Re: [PATCH] hantro_v4l2: ignore enum_framesizes for non-coded formats without postproc

[Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>]

Hi Michael,

On Tue, Jun 21, 2022 at 12:31:50AM +0200, Michael Grzeschik wrote:
> When codec_mode is HANTRO_MODE_NONE, then vidioc_enum_framesizes should
> return with -EINVAL. At least when hantro_needs_postproc returns false.
> Which is the case for encoders. But with the latest postproc scaling
> patch this is not the case anymore. This patch is fixing this.
> 

The fix looks correct, but the commit description looks a bit confusing,
do you think you could reword it and make it more readable?

For commits that fix regressions, it's often handy to include the word "fix"
in the commit subject. How about:

  hantro: fix VIDIOC_ENUM_FRAMESIZES for non-coded formats without postproc

> Fixes: 79c987de8b35 ("media: hantro: Use post processor scaling capacities")
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

Thanks,
Ezequiel

> ---
>  drivers/staging/media/hantro/hantro_v4l2.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> index 22ad182ee972ca..0eb0873d383678 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.c
> +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> @@ -124,8 +124,11 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
>  	}
>  
>  	/* For non-coded formats check if postprocessing scaling is possible */
> -	if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
> -		return hanto_postproc_enum_framesizes(ctx, fsize);
> +	if (fmt->codec_mode == HANTRO_MODE_NONE) {
> +		if (hantro_needs_postproc(ctx, fmt))
> +			return hanto_postproc_enum_framesizes(ctx, fsize);
> +		else
> +			return -EINVAL;
>  	} else if (fsize->index != 0) {
>  		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
>  			  fsize->index);
> -- 
> 2.30.2
>