Re: [PATCH v2 3/6] media: qcom: iris: gen2: add support for 10bit decoding

[Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>]

On 4/17/2026 3:07 PM, Neil Armstrong wrote:
> Add the necessary plumbing into the HFi Gen2 to signal the decoder
> the right 10bit pixel format and stride when in compressed mode.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../platform/qcom/iris/iris_hfi_gen2_command.c     | 71 +++++++++++++++++++++-
>  .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>  drivers/media/platform/qcom/iris/iris_utils.c      |  4 +-
>  3 files changed, 72 insertions(+), 4 deletions(-)
> 
> 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 30bfd90d423b..d664e606e886 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> @@ -481,8 +481,20 @@ static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
>  
>  	if (inst->domain == DECODER) {
>  		pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
> -		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
> -			HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
> +		switch (pixelformat) {
> +		case V4L2_PIX_FMT_NV12:
> +			hfi_colorformat = HFI_COLOR_FMT_NV12;
> +			break;
> +		case V4L2_PIX_FMT_QC08C:
> +			hfi_colorformat = HFI_COLOR_FMT_NV12_UBWC;
> +			break;
> +		case V4L2_PIX_FMT_P010:
> +			hfi_colorformat = HFI_COLOR_FMT_P010;
> +			break;
> +		case V4L2_PIX_FMT_QC10C:
> +			hfi_colorformat = HFI_COLOR_FMT_TP10_UBWC;
> +			break;
> +		};
>  	} else {
>  		pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
>  		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
> @@ -517,7 +529,8 @@ static int iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
>  	stride_uv = stride_y;
>  	scanline_uv = scanline_y / 2;
>  
> -	if (pixelformat != V4L2_PIX_FMT_NV12)
> +	if (pixelformat != V4L2_PIX_FMT_NV12 &&
> +	    pixelformat != V4L2_PIX_FMT_P010)
>  		return 0;
>  
>  	payload[0] = stride_y << 16 | scanline_y;
> @@ -532,6 +545,57 @@ static int iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
>  						  sizeof(u64));
>  }
>  
> +static int iris_hfi_gen2_set_ubwc_stride_scanline(struct iris_inst *inst, u32 plane)
> +{
> +	u32 meta_stride_y, meta_scanline_y, meta_stride_uv, meta_scanline_uv;
> +	u32 stride_y, scanline_y, stride_uv, scanline_uv;
> +	u32 port = iris_hfi_gen2_get_port(inst, plane);
> +	u32 pixelformat, width, height;
> +	u32 payload[4];
> +
> +	pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
> +	width = inst->fmt_dst->fmt.pix_mp.width;
> +	height = inst->fmt_dst->fmt.pix_mp.height;
> +
> +	switch (pixelformat) {
> +	case V4L2_PIX_FMT_QC08C:
> +		stride_y = ALIGN(width, 128);
> +		scanline_y = ALIGN(height, 32);
> +		stride_uv = ALIGN(width, 128);
> +		scanline_uv = ALIGN((height + 1) >> 1, 32);
> +		meta_stride_y = ALIGN(DIV_ROUND_UP(width, 32), 64);
> +		meta_scanline_y = ALIGN(DIV_ROUND_UP(height, 8), 16);
> +		meta_stride_uv = ALIGN(DIV_ROUND_UP((width + 1) >> 1, 16), 64);
> +		meta_scanline_uv = ALIGN(DIV_ROUND_UP((height + 1) >> 1, 8), 16);
> +		break;
> +	case V4L2_PIX_FMT_QC10C:
> +		stride_y = ALIGN(width * 4 / 3, 256);
> +		scanline_y = ALIGN(height, 16);
> +		stride_uv = ALIGN(width * 4 / 3, 256);
> +		scanline_uv = ALIGN((height + 1) >> 1, 16);
> +		meta_stride_y = ALIGN(DIV_ROUND_UP(width, 48), 64);
> +		meta_scanline_y = ALIGN(DIV_ROUND_UP(height, 4), 16);
> +		meta_stride_uv = ALIGN(DIV_ROUND_UP((width + 1) >> 1, 24), 64);
> +		meta_scanline_uv = ALIGN(DIV_ROUND_UP((height + 1) >> 1, 4), 16);
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	payload[0] = stride_y << 16 | scanline_y;
> +	payload[1] = stride_uv << 16 | scanline_uv;
> +	payload[2] = meta_stride_y << 16 | meta_scanline_y;
> +	payload[3] = meta_stride_uv << 16 | meta_scanline_uv;
> +
> +	return iris_hfi_gen2_session_set_property(inst,
> +						  HFI_PROP_UBWC_STRIDE_SCANLINE,
> +						  HFI_HOST_FLAGS_NONE,
> +						  port,
> +						  HFI_PAYLOAD_U32_ARRAY,
> +						  &payload[0],
> +						  sizeof(u32) * 4);
> +}

I’m still not convinced this change is needed for non‑AV1 codecs.
I’m concerned this may be masking an underlying alignment issue, since this
property is not intended to be used for non‑AV1 codecs. Even FW team
doesn't recommend setting this property for other codecs as it can lead to
wasted memory due to over‑allocation and padding.

Thanks,
Dikshita