Re: [PATCH 1/9] media: iris: translate Gen1 profile/level to HFI enumerants

[Vishnu Reddy <busanna.reddy@oss.qualcomm.com>]

On 7/10/2026 1:06 AM, Dmitry Baryshkov wrote:
> The Gen1 firmware expects the profile and level in the
> HFI_PROPERTY_PARAM_PROFILE_LEVEL_CURRENT property as HFI-specific
> enumerants (a one-hot bitmask), but the iris driver passed the raw V4L2
> profile and level values straight through. Only the V4L2 H264 high
> profile happened to match its HFI value (both 0x4); every other profile
> and all levels were programmed with a wrong value.
>
> Translate the V4L2 profile and level to the HFI enumerants per codec
> before building the property, mirroring the mapping the venus driver
> performs for the same firmware interface.
>
> Fixes: d22037f3fd33 ("media: iris: Set platform capabilities to firmware for encoder video device")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  .../platform/qcom/iris/iris_hfi_gen1_command.c     | 109 +++++++++++++++++++--
>  .../platform/qcom/iris/iris_hfi_gen1_defines.h     |  44 +++++++++
>  2 files changed, 144 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index c4baabbacefd..ed9fef2aec08 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -479,6 +479,105 @@ static int iris_hfi_gen1_session_drain(struct iris_inst *inst, u32 plane)
>  	return -EINVAL;
>  }
>  
> +struct iris_hfi_gen1_id_mapping {
> +	u32 hfi_id;
> +	u32 v4l2_id;
> +};
> +
> +static const struct iris_hfi_gen1_id_mapping iris_hfi_gen1_h264_profiles[] = {
> +	{ HFI_H264_PROFILE_BASELINE, V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE },
> +	{ HFI_H264_PROFILE_MAIN, V4L2_MPEG_VIDEO_H264_PROFILE_MAIN },
> +	{ HFI_H264_PROFILE_HIGH, V4L2_MPEG_VIDEO_H264_PROFILE_HIGH },
> +	{ HFI_H264_PROFILE_STEREO_HIGH, V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH },
> +	{ HFI_H264_PROFILE_MULTIVIEW_HIGH, V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH },
> +	{ HFI_H264_PROFILE_CONSTRAINED_BASE, V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE },
> +	{ HFI_H264_PROFILE_CONSTRAINED_HIGH, V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_HIGH },
> +};
> +
> +static const struct iris_hfi_gen1_id_mapping iris_hfi_gen1_h264_levels[] = {
> +	{ HFI_H264_LEVEL_1, V4L2_MPEG_VIDEO_H264_LEVEL_1_0 },
> +	{ HFI_H264_LEVEL_1b, V4L2_MPEG_VIDEO_H264_LEVEL_1B },
> +	{ HFI_H264_LEVEL_11, V4L2_MPEG_VIDEO_H264_LEVEL_1_1 },
> +	{ HFI_H264_LEVEL_12, V4L2_MPEG_VIDEO_H264_LEVEL_1_2 },
> +	{ HFI_H264_LEVEL_13, V4L2_MPEG_VIDEO_H264_LEVEL_1_3 },
> +	{ HFI_H264_LEVEL_2, V4L2_MPEG_VIDEO_H264_LEVEL_2_0 },
> +	{ HFI_H264_LEVEL_21, V4L2_MPEG_VIDEO_H264_LEVEL_2_1 },
> +	{ HFI_H264_LEVEL_22, V4L2_MPEG_VIDEO_H264_LEVEL_2_2 },
> +	{ HFI_H264_LEVEL_3, V4L2_MPEG_VIDEO_H264_LEVEL_3_0 },
> +	{ HFI_H264_LEVEL_31, V4L2_MPEG_VIDEO_H264_LEVEL_3_1 },
> +	{ HFI_H264_LEVEL_32, V4L2_MPEG_VIDEO_H264_LEVEL_3_2 },
> +	{ HFI_H264_LEVEL_4, V4L2_MPEG_VIDEO_H264_LEVEL_4_0 },
> +	{ HFI_H264_LEVEL_41, V4L2_MPEG_VIDEO_H264_LEVEL_4_1 },
> +	{ HFI_H264_LEVEL_42, V4L2_MPEG_VIDEO_H264_LEVEL_4_2 },
> +	{ HFI_H264_LEVEL_5, V4L2_MPEG_VIDEO_H264_LEVEL_5_0 },
> +	{ HFI_H264_LEVEL_51, V4L2_MPEG_VIDEO_H264_LEVEL_5_1 },
> +	{ HFI_H264_LEVEL_52, V4L2_MPEG_VIDEO_H264_LEVEL_5_1 },

Is it correct that both HFI levels 51 and 52 map to V4L2 level 5_1?

> +};
> +
> +static const struct iris_hfi_gen1_id_mapping iris_hfi_gen1_hevc_profiles[] = {
> +	{ HFI_HEVC_PROFILE_MAIN, V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN },
> +	{ HFI_HEVC_PROFILE_MAIN_STILL_PIC, V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE },
> +	{ HFI_HEVC_PROFILE_MAIN10, V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10 },
> +};
> +
> +static const struct iris_hfi_gen1_id_mapping iris_hfi_gen1_hevc_levels[] = {
> +	{ HFI_HEVC_LEVEL_1, V4L2_MPEG_VIDEO_HEVC_LEVEL_1 },
> +	{ HFI_HEVC_LEVEL_2, V4L2_MPEG_VIDEO_HEVC_LEVEL_2 },
> +	{ HFI_HEVC_LEVEL_21, V4L2_MPEG_VIDEO_HEVC_LEVEL_2_1 },
> +	{ HFI_HEVC_LEVEL_3, V4L2_MPEG_VIDEO_HEVC_LEVEL_3 },
> +	{ HFI_HEVC_LEVEL_31, V4L2_MPEG_VIDEO_HEVC_LEVEL_3_1 },
> +	{ HFI_HEVC_LEVEL_4, V4L2_MPEG_VIDEO_HEVC_LEVEL_4 },
> +	{ HFI_HEVC_LEVEL_41, V4L2_MPEG_VIDEO_HEVC_LEVEL_4_1 },
> +	{ HFI_HEVC_LEVEL_5, V4L2_MPEG_VIDEO_HEVC_LEVEL_5 },
> +	{ HFI_HEVC_LEVEL_51, V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1 },
> +	{ HFI_HEVC_LEVEL_52, V4L2_MPEG_VIDEO_HEVC_LEVEL_5_2 },
> +	{ HFI_HEVC_LEVEL_6, V4L2_MPEG_VIDEO_HEVC_LEVEL_6 },
> +	{ HFI_HEVC_LEVEL_61, V4L2_MPEG_VIDEO_HEVC_LEVEL_6_1 },
> +	{ HFI_HEVC_LEVEL_62, V4L2_MPEG_VIDEO_HEVC_LEVEL_6_2 },
> +};
> +
> +static u32 iris_hfi_gen1_find_hfi_id(u32 v4l2_id,
> +				     const struct iris_hfi_gen1_id_mapping *array,
> +				     unsigned int array_sz)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < array_sz; i++)
> +		if (v4l2_id == array[i].v4l2_id)
> +			return array[i].hfi_id;
> +
> +	return 0;
> +}
> +
> +/*
> + * The Gen1 firmware expects profile and level as HFI enumerants, while the
> + * client provides raw V4L2 values. Translate them per codec, mirroring the
> + * mapping performed by the venus driver for the same firmware interface.
> + */
> +static void
> +iris_hfi_gen1_profile_level(struct iris_inst *inst, u32 v4l2_pf, u32 v4l2_lvl,
> +			    struct hfi_profile_level *pl)
> +{
> +	switch (inst->codec) {
> +	case V4L2_PIX_FMT_H264:
> +		pl->profile = iris_hfi_gen1_find_hfi_id(v4l2_pf, iris_hfi_gen1_h264_profiles,
> +							ARRAY_SIZE(iris_hfi_gen1_h264_profiles));
> +		pl->level = iris_hfi_gen1_find_hfi_id(v4l2_lvl, iris_hfi_gen1_h264_levels,
> +						      ARRAY_SIZE(iris_hfi_gen1_h264_levels));
> +		break;
> +	case V4L2_PIX_FMT_HEVC:
> +		pl->profile = iris_hfi_gen1_find_hfi_id(v4l2_pf, iris_hfi_gen1_hevc_profiles,
> +							ARRAY_SIZE(iris_hfi_gen1_hevc_profiles));
> +		pl->level = iris_hfi_gen1_find_hfi_id(v4l2_lvl, iris_hfi_gen1_hevc_levels,
> +						      ARRAY_SIZE(iris_hfi_gen1_hevc_levels));
> +		break;
> +	default:
> +		pl->profile = 0;
> +		pl->level = 0;
> +		break;
> +	}
> +}
> +
>  static int
>  iris_hfi_gen1_packet_session_set_property(struct hfi_session_set_property_pkt *packet,
>  					  struct iris_inst *inst, u32 ptype, void *pdata)
> @@ -582,15 +681,7 @@ iris_hfi_gen1_packet_session_set_property(struct hfi_session_set_property_pkt *p
>  	case HFI_PROPERTY_PARAM_PROFILE_LEVEL_CURRENT: {
>  		struct hfi_profile_level *in = pdata, *pl = prop_data;
>  
> -		pl->level = in->level;
> -		pl->profile = in->profile;
> -		if (pl->profile <= 0)
> -			/* Profile not supported, falling back to high */
> -			pl->profile = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH;
> -
> -		if (!pl->level)
> -			/* Level not supported, falling back to 1 */
> -			pl->level = 1;
> +		iris_hfi_gen1_profile_level(inst, in->profile, in->level, pl);
>  
>  		packet->shdr.hdr.size += sizeof(*pl);
>  		break;
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> index bb495a1d2623..2702f255bc04 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> @@ -17,6 +17,50 @@
>  #define HFI_VIDEO_CODEC_HEVC				0x00002000
>  #define HFI_VIDEO_CODEC_VP9				0x00004000
>  
> +#define HFI_H264_PROFILE_BASELINE			0x00000001
> +#define HFI_H264_PROFILE_MAIN				0x00000002
> +#define HFI_H264_PROFILE_HIGH				0x00000004
> +#define HFI_H264_PROFILE_STEREO_HIGH			0x00000008
> +#define HFI_H264_PROFILE_MULTIVIEW_HIGH			0x00000010
> +#define HFI_H264_PROFILE_CONSTRAINED_BASE		0x00000020
> +#define HFI_H264_PROFILE_CONSTRAINED_HIGH		0x00000040
> +
> +#define HFI_H264_LEVEL_1				0x00000001
> +#define HFI_H264_LEVEL_1b				0x00000002
> +#define HFI_H264_LEVEL_11				0x00000004
> +#define HFI_H264_LEVEL_12				0x00000008
> +#define HFI_H264_LEVEL_13				0x00000010
> +#define HFI_H264_LEVEL_2				0x00000020
> +#define HFI_H264_LEVEL_21				0x00000040
> +#define HFI_H264_LEVEL_22				0x00000080
> +#define HFI_H264_LEVEL_3				0x00000100
> +#define HFI_H264_LEVEL_31				0x00000200
> +#define HFI_H264_LEVEL_32				0x00000400
> +#define HFI_H264_LEVEL_4				0x00000800
> +#define HFI_H264_LEVEL_41				0x00001000
> +#define HFI_H264_LEVEL_42				0x00002000
> +#define HFI_H264_LEVEL_5				0x00004000
> +#define HFI_H264_LEVEL_51				0x00008000
> +#define HFI_H264_LEVEL_52				0x00010000
> +
> +#define HFI_HEVC_PROFILE_MAIN				0x00000001
> +#define HFI_HEVC_PROFILE_MAIN10				0x00000002
> +#define HFI_HEVC_PROFILE_MAIN_STILL_PIC			0x00000004
> +
> +#define HFI_HEVC_LEVEL_1				0x00000001
> +#define HFI_HEVC_LEVEL_2				0x00000002
> +#define HFI_HEVC_LEVEL_21				0x00000004
> +#define HFI_HEVC_LEVEL_3				0x00000008
> +#define HFI_HEVC_LEVEL_31				0x00000010
> +#define HFI_HEVC_LEVEL_4				0x00000020
> +#define HFI_HEVC_LEVEL_41				0x00000040
> +#define HFI_HEVC_LEVEL_5				0x00000080
> +#define HFI_HEVC_LEVEL_51				0x00000100
> +#define HFI_HEVC_LEVEL_52				0x00000200
> +#define HFI_HEVC_LEVEL_6				0x00000400
> +#define HFI_HEVC_LEVEL_61				0x00000800
> +#define HFI_HEVC_LEVEL_62				0x00001000
> +
>  #define HFI_ERR_NONE					0x0
>  
>  #define HFI_CMD_SYS_INIT				0x10001

Reviewed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>