Re: [PATCH v4 02/16] media: iris: Filter UBWC raw formats based on hardware capabilities

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The logic to dynamically filter out UBWC raw formats in the video decoder is incomplete and fails to omit the 8-bit UBWC format (QC08C).
- [Medium] Incorrect use of the encoder UBWC version field to validate decoder capabilities.
--

commit a8db7d07b5e1a7229c20919b2735cd3483c3c3ab
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

media: iris: Filter UBWC raw formats based on hardware capabilities

The raw formats supported by Iris were previously advertised unconditionally,
assuming UBWC support on all platforms. Use the UBWC configuration provided
by the platform to dynamically filter raw formats at runtime. If UBWC is
not supported, UBWC-based formats are omitted from the advertised capability
list.

> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index 9e228b70420e4..7da43f312ba98 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
[ ... ]
> @@ -82,6 +84,9 @@ static bool check_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>  		fmt = iris_vdec_formats_cap;
>  		size = ARRAY_SIZE(iris_vdec_formats_cap);
> +		/* Last format is UBWC; drop it if UBWC is unsupported */
> +		if (!ubwc->ubwc_enc_version)
> +			size--;

[Severity: High]
Does this logic successfully drop all UBWC formats?

Looking at the definition of iris_vdec_formats_cap, it appears there are
multiple UBWC formats interspersed:

static const u32 iris_vdec_formats_cap[] = {
	[IRIS_FMT_NV12] = V4L2_PIX_FMT_NV12,
	[IRIS_FMT_QC08C] = V4L2_PIX_FMT_QC08C,
	[IRIS_FMT_TP10] = V4L2_PIX_FMT_P010,
	[IRIS_FMT_QC10C] = V4L2_PIX_FMT_QC10C,
};

By just decrementing the size, it seems we only omit the last format
(QC10C), but QC08C would remain within the array bounds. Could this
cause QC08C to be incorrectly accepted by check_format() on platforms
without UBWC support?

[Severity: Medium]
Should this check use the decoder version instead?

Since this is the video decoder implementation in iris_vdec.c, checking
ubwc->ubwc_enc_version might lead to incorrect filtering on future hardware
platforms where UBWC support differs between the encoder and decoder. Would
ubwc->ubwc_dec_version be more appropriate here?

>  		break;
>  	default:
>  		return false;
> @@ -121,6 +127,9 @@ static u32 find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>  		fmt = iris_vdec_formats_cap;
>  		size = ARRAY_SIZE(iris_vdec_formats_cap);
> +		/* Last format is UBWC; drop it if UBWC is unsupported */
> +		if (!ubwc->ubwc_enc_version)
> +			size--;

[Severity: High]
Does this also result in find_format_by_index() returning a UBWC format?

Because QC08C remains within the decremented array bounds, would querying
index 1 still return V4L2_PIX_FMT_QC08C to userspace even when UBWC is
unsupported?

[Severity: Medium]
Similar to the issue above, should this use ubwc->ubwc_dec_version instead
of ubwc->ubwc_enc_version to properly validate decoder capabilities?

>  		break;
>  	default:
>  		return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-iris-ar50lt-v4-0-0abfb74d5b3c@oss.qualcomm.com?part=2