Re: [PATCH] media: verisilicon: Export only needed pixels formats.

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]

Hi Benjamin,

Le mardi 24 juin 2025 à 14:29 +0200, Benjamin Gaignard a écrit :
> When enumerating the pixels formats check if the context
> request to only export post-processed pixels formats.
> The exception is when V4L2_FMTDESC_FLAG_ENUM_ALL is set, we
> need to export all pixels formats.

The change looks good. The explanation could perhaps be improved. Perhaps
something along this instead ?

	Some pixel formats can only be produced if the decoder outputs 
	reference pictures directly. In some cases, such as AV1 film-grain,
	the use of the post-processor is strictly required. In this case,
	only enumerate the post-processor supported formats. The exception is
	when V4L2_FMTDESC_FLAG_ENUM_ALL is set, in this case, we enumerate
	everything regardless of the state.

Perhaps long term we should rename need_postproc by force_postproc. I think this
would help reading the code.

> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Fixes: bcd4f091cf1e ("media: verisilicon: Use V4L2_FMTDESC_FLAG_ENUM_ALL flag")

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  drivers/media/platform/verisilicon/hantro_v4l2.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index 7c3515cf7d64..7869faf921f4 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -222,6 +222,7 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
>  	unsigned int num_fmts, i, j = 0;
>  	bool skip_mode_none, enum_all_formats;
>  	u32 index = f->index & ~V4L2_FMTDESC_FLAG_ENUM_ALL;
> +	bool need_postproc = ctx->need_postproc;
>  
>  	/*
>  	 * If the V4L2_FMTDESC_FLAG_ENUM_ALL flag is set, we want to enumerate all
> @@ -230,6 +231,9 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
>  	enum_all_formats = !!(f->index & V4L2_FMTDESC_FLAG_ENUM_ALL);
>  	f->index = index;
>  
> +	if (enum_all_formats)
> +		need_postproc = HANTRO_AUTO_POSTPROC;
> +
>  	/*
>  	 * When dealing with an encoder:
>  	 *  - on the capture side we want to filter out all MODE_NONE formats.
> @@ -242,7 +246,7 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
>  	 */
>  	skip_mode_none = capture == ctx->is_encoder;
>  
> -	formats = hantro_get_formats(ctx, &num_fmts, HANTRO_AUTO_POSTPROC);
> +	formats = hantro_get_formats(ctx, &num_fmts, need_postproc);
>  	for (i = 0; i < num_fmts; i++) {
>  		bool mode_none = formats[i].codec_mode == HANTRO_MODE_NONE;
>  		fmt = &formats[i];

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]