Re: [RFC PATCH 02/12] media: iris: Add platform capabilities for HEVC and VP9 decoders

From: Bryan O'Donoghue
Date: Wed Mar 05 2025 - 19:28:26 EST


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@xxxxxxxxxxx>
---
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@xxxxxxxxxx>