| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f4354a08-46e2-4bf5-9395-f9b861a9d7c8@linaro.org>
Date: Thu, 6 Mar 2025 00:28:09 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Dikshita Agarwal <quic_dikshita@...cinc.com>, quic_vgarodia@...cinc.com,
quic_abhinavk@...cinc.com, mchehab@...nel.org
Cc: hverkuil@...all.nl, linux-media@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 02/12] media: iris: Add platform capabilities for HEVC
and VP9 decoders
On 05/03/2025 10:43, Dikshita Agarwal wrote:
> Add platform capabilities for HEVC and VP9 codecs in decoder driver
> with related hooks.
>
> Signed-off-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
> ---
> drivers/media/platform/qcom/iris/iris_ctrls.c | 28 ++++++-
> .../qcom/iris/iris_hfi_gen2_command.c | 30 ++++++-
> .../qcom/iris/iris_hfi_gen2_defines.h | 1 +
> .../qcom/iris/iris_hfi_gen2_response.c | 36 ++++++++-
> .../platform/qcom/iris/iris_platform_common.h | 9 ++-
> .../platform/qcom/iris/iris_platform_sm8550.c | 80 ++++++++++++++++++-
> 6 files changed, 170 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c
> index b690578256d5..fb2b818c7c5c 100644
> --- a/drivers/media/platform/qcom/iris/iris_ctrls.c
> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c
> @@ -20,9 +20,19 @@ static enum platform_inst_fw_cap_type iris_get_cap_id(u32 id)
> case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER:
> return DEBLOCK;
> case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
> - return PROFILE;
> + return PROFILE_H264;
> + case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
> + return PROFILE_HEVC;
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> + return PROFILE_VP9;
> case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
> - return LEVEL;
> + return LEVEL_H264;
> + case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> + return LEVEL_HEVC;
> + case V4L2_CID_MPEG_VIDEO_VP9_LEVEL:
> + return LEVEL_VP9;
> + case V4L2_CID_MPEG_VIDEO_HEVC_TIER:
> + return TIER;
> default:
> return INST_FW_CAP_MAX;
> }
> @@ -36,10 +46,20 @@ static u32 iris_get_v4l2_id(enum platform_inst_fw_cap_type cap_id)
> switch (cap_id) {
> case DEBLOCK:
> return V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER;
> - case PROFILE:
> + case PROFILE_H264:
> return V4L2_CID_MPEG_VIDEO_H264_PROFILE;
> - case LEVEL:
> + case PROFILE_HEVC:
> + return V4L2_CID_MPEG_VIDEO_HEVC_PROFILE;
> + case PROFILE_VP9:
> + return V4L2_CID_MPEG_VIDEO_VP9_PROFILE;
> + case LEVEL_H264:
> return V4L2_CID_MPEG_VIDEO_H264_LEVEL;
> + case LEVEL_HEVC:
> + return V4L2_CID_MPEG_VIDEO_HEVC_LEVEL;
> + case LEVEL_VP9:
> + return V4L2_CID_MPEG_VIDEO_VP9_LEVEL;
> + case TIER:
> + return V4L2_CID_MPEG_VIDEO_HEVC_TIER;
> default:
> return 0;
> }
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> index beaf3a051d7c..a3ebcda9a2ba 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> @@ -309,7 +309,20 @@ static int iris_hfi_gen2_set_profile(struct iris_inst *inst)
> {
> struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
> u32 port = iris_hfi_gen2_get_port(V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
> - u32 profile = inst->fw_caps[PROFILE].value;
> + u32 profile;
> +
> + switch (inst->codec) {
> + case V4L2_PIX_FMT_HEVC:
> + profile = inst->fw_caps[PROFILE_HEVC].value;
> + break;
> + case V4L2_PIX_FMT_VP9:
> + profile = inst->fw_caps[PROFILE_VP9].value;
> + break;
> + case V4L2_PIX_FMT_H264:
> + default:
> + profile = inst->fw_caps[PROFILE_H264].value;
> + break;
Following up on my previous comment about returning a 0 default and
running with it instead of erroring it - you then treat default == 0 @
inst->codec assigned in iris_hfi_gen[1|2]_sys_init as H264.
In fact why have a default by the time you get this this point in the
code anyway ?
Just chuck out parameters which aren't expected as errors and then don't
bother with these defaults.
> + }
>
> inst_hfi_gen2->src_subcr_params.profile = profile;
>
> @@ -326,7 +339,20 @@ static int iris_hfi_gen2_set_level(struct iris_inst *inst)
> {
> struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
> u32 port = iris_hfi_gen2_get_port(V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
> - u32 level = inst->fw_caps[LEVEL].value;
> + u32 level;
> +
> + switch (inst->codec) {
> + case V4L2_PIX_FMT_HEVC:
> + level = inst->fw_caps[LEVEL_HEVC].value;
> + break;
> + case V4L2_PIX_FMT_VP9:
> + level = inst->fw_caps[LEVEL_VP9].value;
> + break;
> + case V4L2_PIX_FMT_H264:
> + default:
> + level = inst->fw_caps[LEVEL_H264].value;
> + break;
> + }
>
> inst_hfi_gen2->src_subcr_params.level = level;
>
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> index 2fcf7914b70f..48c507a1ec27 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> @@ -46,6 +46,7 @@
> #define HFI_PROP_CROP_OFFSETS 0x03000105
> #define HFI_PROP_PROFILE 0x03000107
> #define HFI_PROP_LEVEL 0x03000108
> +#define HFI_PROP_TIER 0x03000109
> #define HFI_PROP_STAGE 0x0300010a
> #define HFI_PROP_PIPE 0x0300010b
> #define HFI_PROP_LUMA_CHROMA_BIT_DEPTH 0x0300010f
These seem like - probably bitfields ?
Could we get the bits in a follow on patch/series ?
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> index b75a01641d5d..809bf0f238bd 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> @@ -563,8 +563,22 @@ static void iris_hfi_gen2_read_input_subcr_params(struct iris_inst *inst)
> inst->crop.width = pixmp_ip->width -
> ((subsc_params.crop_offsets[1] >> 16) & 0xFFFF) - inst->crop.left;
>
> - inst->fw_caps[PROFILE].value = subsc_params.profile;
> - inst->fw_caps[LEVEL].value = subsc_params.level;
> + switch (inst->codec) {
> + case V4L2_PIX_FMT_HEVC:
> + inst->fw_caps[PROFILE_HEVC].value = subsc_params.profile;
> + inst->fw_caps[LEVEL_HEVC].value = subsc_params.level;
> + break;
> + case V4L2_PIX_FMT_VP9:
> + inst->fw_caps[PROFILE_VP9].value = subsc_params.profile;
> + inst->fw_caps[LEVEL_VP9].value = subsc_params.level;
> + break;
> + case V4L2_PIX_FMT_H264:
> + default:
> + inst->fw_caps[PROFILE_H264].value = subsc_params.profile;
> + inst->fw_caps[LEVEL_H264].value = subsc_params.level;
> + break;
> + }
> +
> inst->fw_caps[POC].value = subsc_params.pic_order_cnt;
>
> if (subsc_params.bit_depth != BIT_DEPTH_8 ||
> @@ -791,8 +805,22 @@ static void iris_hfi_gen2_init_src_change_param(struct iris_inst *inst)
> full_range, video_format,
> video_signal_type_present_flag);
>
> - subsc_params->profile = inst->fw_caps[PROFILE].value;
> - subsc_params->level = inst->fw_caps[LEVEL].value;
> + switch (inst->codec) {
> + case V4L2_PIX_FMT_HEVC:
> + subsc_params->profile = inst->fw_caps[PROFILE_HEVC].value;
> + subsc_params->level = inst->fw_caps[LEVEL_HEVC].value;
> + break;
> + case V4L2_PIX_FMT_VP9:
> + subsc_params->profile = inst->fw_caps[PROFILE_VP9].value;
> + subsc_params->level = inst->fw_caps[LEVEL_VP9].value;
> + break;
> + case V4L2_PIX_FMT_H264:
> + default:
> + subsc_params->profile = inst->fw_caps[PROFILE_H264].value;
> + subsc_params->level = inst->fw_caps[LEVEL_H264].value;
> + break;
> + }
> +
> subsc_params->pic_order_cnt = inst->fw_caps[POC].value;
> subsc_params->bit_depth = inst->fw_caps[BIT_DEPTH].value;
> if (inst->fw_caps[CODED_FRAMES].value ==
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index f6b15d2805fb..67204cddd44a 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -78,8 +78,12 @@ struct platform_inst_caps {
> };
>
> enum platform_inst_fw_cap_type {
> - PROFILE = 1,
> - LEVEL,
> + PROFILE_H264 = 1,
> + PROFILE_HEVC,
> + PROFILE_VP9,
> + LEVEL_H264,
> + LEVEL_HEVC,
> + LEVEL_VP9,
> INPUT_BUF_HOST_MAX_COUNT,
> STAGE,
> PIPE,
> @@ -88,6 +92,7 @@ enum platform_inst_fw_cap_type {
> BIT_DEPTH,
> RAP_FRAME,
> DEBLOCK,
> + TIER,
> INST_FW_CAP_MAX,
> };
>
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
> index 35d278996c43..29bc50785da5 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
> @@ -14,7 +14,7 @@
>
> static struct platform_inst_fw_cap inst_fw_cap_sm8550[] = {
> {
> - .cap_id = PROFILE,
> + .cap_id = PROFILE_H264,
> .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE,
> .max = V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_HIGH,
> .step_or_mask = BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
> @@ -28,7 +28,29 @@ static struct platform_inst_fw_cap inst_fw_cap_sm8550[] = {
> .set = iris_set_u32_enum,
> },
> {
> - .cap_id = LEVEL,
> + .cap_id = PROFILE_HEVC,
> + .min = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN,
> + .max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE,
> + .step_or_mask = BIT(V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE),
> + .value = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN,
> + .hfi_id = HFI_PROP_PROFILE,
> + .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
> + .set = iris_set_u32_enum,
> + },
> + {
> + .cap_id = PROFILE_VP9,
> + .min = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
> + .max = V4L2_MPEG_VIDEO_VP9_PROFILE_2,
> + .step_or_mask = BIT(V4L2_MPEG_VIDEO_VP9_PROFILE_0) |
> + BIT(V4L2_MPEG_VIDEO_VP9_PROFILE_2),
> + .value = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
> + .hfi_id = HFI_PROP_PROFILE,
> + .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
> + .set = iris_set_u32_enum,
> + },
> + {
> + .cap_id = LEVEL_H264,
> .min = V4L2_MPEG_VIDEO_H264_LEVEL_1_0,
> .max = V4L2_MPEG_VIDEO_H264_LEVEL_6_2,
> .step_or_mask = BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_0) |
> @@ -56,6 +78,60 @@ static struct platform_inst_fw_cap inst_fw_cap_sm8550[] = {
> .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
> .set = iris_set_u32_enum,
> },
> + {
> + .cap_id = LEVEL_HEVC,
> + .min = V4L2_MPEG_VIDEO_HEVC_LEVEL_1,
> + .max = V4L2_MPEG_VIDEO_HEVC_LEVEL_6_2,
> + .step_or_mask = BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_1) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_2) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_2_1) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_3) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_3_1) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_4) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_4_1) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_5) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_5_2) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_6) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_6_1) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_LEVEL_6_2),
> + .value = V4L2_MPEG_VIDEO_HEVC_LEVEL_6_1,
> + .hfi_id = HFI_PROP_LEVEL,
> + .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
> + .set = iris_set_u32_enum,
> + },
> + {
> + .cap_id = LEVEL_VP9,
> + .min = V4L2_MPEG_VIDEO_VP9_LEVEL_1_0,
> + .max = V4L2_MPEG_VIDEO_VP9_LEVEL_6_0,
> + .step_or_mask = BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_1_0) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_1_1) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_2_0) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_2_1) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_3_0) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_3_1) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_4_0) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_4_1) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_5_0) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_5_1) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_5_2) |
> + BIT(V4L2_MPEG_VIDEO_VP9_LEVEL_6_0),
> + .value = V4L2_MPEG_VIDEO_VP9_LEVEL_6_0,
> + .hfi_id = HFI_PROP_LEVEL,
> + .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
> + .set = iris_set_u32_enum,
> + },
> + {
> + .cap_id = TIER,
> + .min = V4L2_MPEG_VIDEO_HEVC_TIER_MAIN,
> + .max = V4L2_MPEG_VIDEO_HEVC_TIER_HIGH,
> + .step_or_mask = BIT(V4L2_MPEG_VIDEO_HEVC_TIER_MAIN) |
> + BIT(V4L2_MPEG_VIDEO_HEVC_TIER_HIGH),
> + .value = V4L2_MPEG_VIDEO_HEVC_TIER_HIGH,
> + .hfi_id = HFI_PROP_TIER,
> + .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
> + .set = iris_set_u32_enum,
> + },
> {
> .cap_id = INPUT_BUF_HOST_MAX_COUNT,
> .min = DEFAULT_MAX_HOST_BUF_COUNT,
Other than those nits
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Powered by blists - more mailing lists