diff mbox series

venus: venc: Fix setting of profile and level

Message ID 20201024163436.19638-1-stanimir.varbanov@linaro.org (mailing list archive)
State Superseded, archived
Headers show
Series venus: venc: Fix setting of profile and level | expand

Commit Message

Stanimir Varbanov Oct. 24, 2020, 4:34 p.m. UTC 
The profile and level in op_set_ctrl was recently changed but during
v4l2_ctrl_handler_setup profile and level control values are mangled.

Fixes: 435c53c3698f ("media: venus: venc: Use helper to set profile and level")
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/core.h      | 15 +++++++--
 drivers/media/platform/qcom/venus/venc.c      | 32 ++++++++++++++++++-
 .../media/platform/qcom/venus/venc_ctrls.c    | 14 ++++++--
 3 files changed, 56 insertions(+), 5 deletions(-)

Comments

kernel test robot Oct. 26, 2020, 5:47 p.m. UTC | #1 
Hi Stanimir,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on linus/master v5.10-rc1 next-20201026]
[cannot apply to linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Stanimir-Varbanov/venus-venc-Fix-setting-of-profile-and-level/20201025-003549
base:   git://linuxtv.org/media_tree.git master
config: arm-randconfig-r023-20201026 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project f2c25c70791de95d2466e09b5b58fc37f6ccd7a4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/11157ae4c1a8ce951c7b93f2bd240b9d96779965
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Stanimir-Varbanov/venus-venc-Fix-setting-of-profile-and-level/20201025-003549
        git checkout 11157ae4c1a8ce951c7b93f2bd240b9d96779965
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/platform/qcom/venus/venc.c:713:2: warning: variable 'profile' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
           default:
           ^~~~~~~
   drivers/media/platform/qcom/venus/venc.c:717:45: note: uninitialized use occurs here
           ret = venus_helper_set_profile_level(inst, profile, level);
                                                      ^~~~~~~
   drivers/media/platform/qcom/venus/venc.c:540:13: note: initialize the variable 'profile' to silence this warning
           u32 profile, level;
                      ^
                       = 0
>> drivers/media/platform/qcom/venus/venc.c:713:2: warning: variable 'level' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
           default:
           ^~~~~~~
   drivers/media/platform/qcom/venus/venc.c:717:54: note: uninitialized use occurs here
           ret = venus_helper_set_profile_level(inst, profile, level);
                                                               ^~~~~
   drivers/media/platform/qcom/venus/venc.c:540:20: note: initialize the variable 'level' to silence this warning
           u32 profile, level;
                             ^
                              = 0
   2 warnings generated.

vim +/profile +713 drivers/media/platform/qcom/venus/venc.c

   529	
   530	static int venc_set_properties(struct venus_inst *inst)
   531	{
   532		struct venc_controls *ctr = &inst->controls.enc;
   533		struct hfi_intra_period intra_period;
   534		struct hfi_framerate frate;
   535		struct hfi_bitrate brate;
   536		struct hfi_idr_period idrp;
   537		struct hfi_quantization quant;
   538		struct hfi_quantization_range quant_range;
   539		u32 ptype, rate_control, bitrate;
   540		u32 profile, level;
   541		int ret;
   542	
   543		ret = venus_helper_set_work_mode(inst, VIDC_WORK_MODE_2);
   544		if (ret)
   545			return ret;
   546	
   547		ptype = HFI_PROPERTY_CONFIG_FRAME_RATE;
   548		frate.buffer_type = HFI_BUFFER_OUTPUT;
   549		frate.framerate = inst->fps * (1 << 16);
   550	
   551		ret = hfi_session_set_property(inst, ptype, &frate);
   552		if (ret)
   553			return ret;
   554	
   555		if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_H264) {
   556			struct hfi_h264_vui_timing_info info;
   557			struct hfi_h264_entropy_control entropy;
   558			struct hfi_h264_db_control deblock;
   559	
   560			ptype = HFI_PROPERTY_PARAM_VENC_H264_VUI_TIMING_INFO;
   561			info.enable = 1;
   562			info.fixed_framerate = 1;
   563			info.time_scale = NSEC_PER_SEC;
   564	
   565			ret = hfi_session_set_property(inst, ptype, &info);
   566			if (ret)
   567				return ret;
   568	
   569			ptype = HFI_PROPERTY_PARAM_VENC_H264_ENTROPY_CONTROL;
   570			entropy.entropy_mode = venc_v4l2_to_hfi(
   571						  V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE,
   572						  ctr->h264_entropy_mode);
   573			entropy.cabac_model = HFI_H264_CABAC_MODEL_0;
   574	
   575			ret = hfi_session_set_property(inst, ptype, &entropy);
   576			if (ret)
   577				return ret;
   578	
   579			ptype = HFI_PROPERTY_PARAM_VENC_H264_DEBLOCK_CONTROL;
   580			deblock.mode = venc_v4l2_to_hfi(
   581					      V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_MODE,
   582					      ctr->h264_loop_filter_mode);
   583			deblock.slice_alpha_offset = ctr->h264_loop_filter_alpha;
   584			deblock.slice_beta_offset = ctr->h264_loop_filter_beta;
   585	
   586			ret = hfi_session_set_property(inst, ptype, &deblock);
   587			if (ret)
   588				return ret;
   589		}
   590	
   591		/* IDR periodicity, n:
   592		 * n = 0 - only the first I-frame is IDR frame
   593		 * n = 1 - all I-frames will be IDR frames
   594		 * n > 1 - every n-th I-frame will be IDR frame
   595		 */
   596		ptype = HFI_PROPERTY_CONFIG_VENC_IDR_PERIOD;
   597		idrp.idr_period = 0;
   598		ret = hfi_session_set_property(inst, ptype, &idrp);
   599		if (ret)
   600			return ret;
   601	
   602		if (ctr->num_b_frames) {
   603			u32 max_num_b_frames = NUM_B_FRAMES_MAX;
   604	
   605			ptype = HFI_PROPERTY_PARAM_VENC_MAX_NUM_B_FRAMES;
   606			ret = hfi_session_set_property(inst, ptype, &max_num_b_frames);
   607			if (ret)
   608				return ret;
   609		}
   610	
   611		ptype = HFI_PROPERTY_CONFIG_VENC_INTRA_PERIOD;
   612		intra_period.pframes = ctr->num_p_frames;
   613		intra_period.bframes = ctr->num_b_frames;
   614	
   615		ret = hfi_session_set_property(inst, ptype, &intra_period);
   616		if (ret)
   617			return ret;
   618	
   619		if (!ctr->rc_enable)
   620			rate_control = HFI_RATE_CONTROL_OFF;
   621		else if (ctr->bitrate_mode == V4L2_MPEG_VIDEO_BITRATE_MODE_VBR)
   622			rate_control = ctr->frame_skip_mode ? HFI_RATE_CONTROL_VBR_VFR :
   623							      HFI_RATE_CONTROL_VBR_CFR;
   624		else if (ctr->bitrate_mode == V4L2_MPEG_VIDEO_BITRATE_MODE_CBR)
   625			rate_control = ctr->frame_skip_mode ? HFI_RATE_CONTROL_CBR_VFR :
   626							      HFI_RATE_CONTROL_CBR_CFR;
   627		else if (ctr->bitrate_mode == V4L2_MPEG_VIDEO_BITRATE_MODE_CQ)
   628			rate_control = HFI_RATE_CONTROL_CQ;
   629	
   630		ptype = HFI_PROPERTY_PARAM_VENC_RATE_CONTROL;
   631		ret = hfi_session_set_property(inst, ptype, &rate_control);
   632		if (ret)
   633			return ret;
   634	
   635		if (rate_control == HFI_RATE_CONTROL_CQ && ctr->const_quality) {
   636			struct hfi_heic_frame_quality quality = {};
   637	
   638			ptype = HFI_PROPERTY_CONFIG_HEIC_FRAME_QUALITY;
   639			quality.frame_quality = ctr->const_quality;
   640			ret = hfi_session_set_property(inst, ptype, &quality);
   641			if (ret)
   642				return ret;
   643		}
   644	
   645		if (!ctr->bitrate)
   646			bitrate = 64000;
   647		else
   648			bitrate = ctr->bitrate;
   649	
   650		ptype = HFI_PROPERTY_CONFIG_VENC_TARGET_BITRATE;
   651		brate.bitrate = bitrate;
   652		brate.layer_id = 0;
   653	
   654		ret = hfi_session_set_property(inst, ptype, &brate);
   655		if (ret)
   656			return ret;
   657	
   658		if (!ctr->bitrate_peak)
   659			bitrate *= 2;
   660		else
   661			bitrate = ctr->bitrate_peak;
   662	
   663		ptype = HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE;
   664		brate.bitrate = bitrate;
   665		brate.layer_id = 0;
   666	
   667		ret = hfi_session_set_property(inst, ptype, &brate);
   668		if (ret)
   669			return ret;
   670	
   671		ptype = HFI_PROPERTY_PARAM_VENC_SESSION_QP;
   672		quant.qp_i = ctr->h264_i_qp;
   673		quant.qp_p = ctr->h264_p_qp;
   674		quant.qp_b = ctr->h264_b_qp;
   675		quant.layer_id = 0;
   676		ret = hfi_session_set_property(inst, ptype, &quant);
   677		if (ret)
   678			return ret;
   679	
   680		ptype = HFI_PROPERTY_PARAM_VENC_SESSION_QP_RANGE;
   681		quant_range.min_qp = ctr->h264_min_qp;
   682		quant_range.max_qp = ctr->h264_max_qp;
   683		quant_range.layer_id = 0;
   684		ret = hfi_session_set_property(inst, ptype, &quant_range);
   685		if (ret)
   686			return ret;
   687	
   688		switch (inst->hfi_codec) {
   689		case HFI_VIDEO_CODEC_H264:
   690			profile = ctr->profile.h264;
   691			level = ctr->level.h264;
   692			break;
   693		case HFI_VIDEO_CODEC_MPEG2:
   694			profile = 0;
   695			level = 0;
   696			break;
   697		case HFI_VIDEO_CODEC_MPEG4:
   698			profile = ctr->profile.mpeg4;
   699			level = ctr->level.mpeg4;
   700			break;
   701		case HFI_VIDEO_CODEC_VP8:
   702			profile = ctr->profile.vp8;
   703			level = 0;
   704			break;
   705		case HFI_VIDEO_CODEC_VP9:
   706			profile = ctr->profile.vp9;
   707			level = ctr->level.vp9;
   708			break;
   709		case HFI_VIDEO_CODEC_HEVC:
   710			profile = ctr->profile.hevc;
   711			level = ctr->level.hevc;
   712			break;
 > 713		default:
   714			break;
   715		}
   716	
   717		ret = venus_helper_set_profile_level(inst, profile, level);
   718		if (ret)
   719			return ret;
   720	
   721		return 0;
   722	}
   723	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 7b79a33dc9d6..05c9fbd51f0c 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -243,8 +243,19 @@  struct venc_controls {
 
 	u32 header_mode;
 
-	u32 profile;
-	u32 level;
+	struct {
+		u32 h264;
+		u32 mpeg4;
+		u32 hevc;
+		u32 vp8;
+		u32 vp9;
+	} profile;
+	struct {
+		u32 h264;
+		u32 mpeg4;
+		u32 hevc;
+		u32 vp9;
+	} level;
 };
 
 struct venus_buffer {
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index f8b1484e7dcd..7322b01f2e3a 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -537,6 +537,7 @@  static int venc_set_properties(struct venus_inst *inst)
 	struct hfi_quantization quant;
 	struct hfi_quantization_range quant_range;
 	u32 ptype, rate_control, bitrate;
+	u32 profile, level;
 	int ret;
 
 	ret = venus_helper_set_work_mode(inst, VIDC_WORK_MODE_2);
@@ -684,7 +685,36 @@  static int venc_set_properties(struct venus_inst *inst)
 	if (ret)
 		return ret;
 
-	ret = venus_helper_set_profile_level(inst, ctr->profile, ctr->level);
+	switch (inst->hfi_codec) {
+	case HFI_VIDEO_CODEC_H264:
+		profile = ctr->profile.h264;
+		level = ctr->level.h264;
+		break;
+	case HFI_VIDEO_CODEC_MPEG2:
+		profile = 0;
+		level = 0;
+		break;
+	case HFI_VIDEO_CODEC_MPEG4:
+		profile = ctr->profile.mpeg4;
+		level = ctr->level.mpeg4;
+		break;
+	case HFI_VIDEO_CODEC_VP8:
+		profile = ctr->profile.vp8;
+		level = 0;
+		break;
+	case HFI_VIDEO_CODEC_VP9:
+		profile = ctr->profile.vp9;
+		level = ctr->level.vp9;
+		break;
+	case HFI_VIDEO_CODEC_HEVC:
+		profile = ctr->profile.hevc;
+		level = ctr->level.hevc;
+		break;
+	default:
+		break;
+	}
+
+	ret = venus_helper_set_profile_level(inst, profile, level);
 	if (ret)
 		return ret;
 
diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
index 0708b3b89d0c..cf860e6446c0 100644
--- a/drivers/media/platform/qcom/venus/venc_ctrls.c
+++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
@@ -103,15 +103,25 @@  static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
 		ctr->h264_entropy_mode = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
+		ctr->profile.mpeg4 = ctrl->val;
+		break;
 	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
+		ctr->profile.h264 = ctrl->val;
+		break;
 	case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
+		ctr->profile.hevc = ctrl->val;
+		break;
 	case V4L2_CID_MPEG_VIDEO_VP8_PROFILE:
-		ctr->profile = ctrl->val;
+		ctr->profile.vp8 = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
+		ctr->level.mpeg4 = ctrl->val;
+		break;
 	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
+		ctr->level.h264 = ctrl->val;
+		break;
 	case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
-		ctr->level = ctrl->val;
+		ctr->level.hevc = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_H264_I_FRAME_QP:
 		ctr->h264_i_qp = ctrl->val;