Re: [PATCH v9 4/8] media: qcom: venus: add codec blacklist mechanism

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The newly introduced codec blacklist mechanism is silently bypassed and ignored for newer HFI platforms (V4, V6).

Pre-existing issues:
- [High] Unmapped firmware codecs cause a fatal probe failure instead of being gracefully skipped.
--

commit 940596b130019709f06d3571e5a70d611a9c2f28
Author: Erikas Bitovtas <xerikasxx@gmail.com>

media: qcom: venus: add codec blacklist mechanism

This commit introduces dec_codec_blacklist and enc_codec_blacklist
properties to allow disabling different codecs per Venus device. It updates
parse_codecs() to apply these blacklists instead of checking the HFI version.

> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index a87e8afb23dfb..bd88e2b484a53 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
[ ... ]
> @@ -178,6 +179,8 @@ static void venus_sys_error_handler(struct work_struct *work)
>  static u32 to_v4l2_codec_type(u32 codec)
>  {
>  	switch (codec) {
> +	case HFI_VIDEO_CODEC_HEVC:
> +		return V4L2_PIX_FMT_HEVC;

[Severity: High]
This is a pre-existing issue, but does an unmapped firmware codec cause a
fatal probe failure instead of being gracefully skipped?

If a codec bit is set by the firmware but is not explicitly mapped in
to_v4l2_codec_type() (such as HFI_VIDEO_CODEC_SPARK if it were not explicitly
blacklisted), it hits the default case and returns 0.

This 0 is then passed to hfi_session_init(), which converts it back to codec=0.
pkt_session_init() correctly rejects codec=0 with -EINVAL. This error is
unconditionally propagated up to venus_probe(), causing the entire device probe
to fail and the driver to become unusable.

Should unknown codecs simply be skipped instead?

[ ... ]
> diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c
> index 92765f9c88730..8e762cf77968b 100644
> --- a/drivers/media/platform/qcom/venus/hfi_parser.c
> +++ b/drivers/media/platform/qcom/venus/hfi_parser.c
> @@ -206,11 +206,11 @@ static int parse_codecs(struct venus_core *core, void *data)
>  	core->dec_codecs = codecs->dec_codecs;
>  	core->enc_codecs = codecs->enc_codecs;
>  
> -	if (IS_V1(core)) {
> -		core->dec_codecs &= ~HFI_VIDEO_CODEC_HEVC;
> -		core->dec_codecs &= ~HFI_VIDEO_CODEC_SPARK;
> -		core->enc_codecs &= ~HFI_VIDEO_CODEC_HEVC;
> -	}
> +	if (core->res->dec_codec_blacklist)
> +		core->dec_codecs &= ~core->res->dec_codec_blacklist;

[Severity: Medium]
Will this newly introduced codec blacklist mechanism be silently bypassed and
ignored for newer HFI platforms like V4 and V6?

For newer platforms, hfi_platform_parser() directly sets the codecs from
hardcoded driver capabilities and returns 0. This causes hfi_parser() to return
early and skip the entire firmware parsing loop where parse_codecs() resides.

If a V4 or V6 device is configured with these blacklists, they appear to be
completely ignored.

> +
> +	if (core->res->enc_codec_blacklist)
> +		core->enc_codecs &= ~core->res->enc_codec_blacklist;
>  
>  	return sizeof(*codecs);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-msm8939-venus-rfc-v9-0-bb1069f3fe02@gmail.com?part=4