Re: [PATCH v2 2/7] media: hantro: Support format filtering by depth

[Ezequiel Garcia]

Hi Jernej,

On Thu, Jun 16, 2022 at 10:25:08PM +0200, Jernej Skrabec wrote:
> In preparation for supporting 10-bit formats, add mechanism which will
> filter formats based on pixel depth.
> 
> Hantro G2 supports only one decoding format natively and that is based
> on bit depth of current video frame. Additionally, it makes no sense to
> upconvert bitness, so filter those out too.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>

Reviewed-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxxxxxxxxx>

Thanks,
Ezequiel

> ---
>  drivers/staging/media/hantro/hantro.h      |  4 ++
>  drivers/staging/media/hantro/hantro_v4l2.c | 48 ++++++++++++++++++++--
>  drivers/staging/media/hantro/hantro_v4l2.h |  1 +
>  3 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
> index 26308bb29adc..2989ebc631cc 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -227,6 +227,7 @@ struct hantro_dev {
>   *
>   * @ctrl_handler:	Control handler used to register controls.
>   * @jpeg_quality:	User-specified JPEG compression quality.
> + * @bit_depth:		Bit depth of current frame
>   *
>   * @codec_ops:		Set of operations related to codec mode.
>   * @postproc:		Post-processing context.
> @@ -252,6 +253,7 @@ struct hantro_ctx {
>  
>  	struct v4l2_ctrl_handler ctrl_handler;
>  	int jpeg_quality;
> +	int bit_depth;
>  
>  	const struct hantro_codec_ops *codec_ops;
>  	struct hantro_postproc_ctx postproc;
> @@ -277,6 +279,7 @@ struct hantro_ctx {
>   * @enc_fmt:	Format identifier for encoder registers.
>   * @frmsize:	Supported range of frame sizes (only for bitstream formats).
>   * @postprocessed: Indicates if this format needs the post-processor.
> + * @match_depth: Indicates if format bit depth must match video bit depth
>   */
>  struct hantro_fmt {
>  	char *name;
> @@ -287,6 +290,7 @@ struct hantro_fmt {
>  	enum hantro_enc_fmt enc_fmt;
>  	struct v4l2_frmsize_stepwise frmsize;
>  	bool postprocessed;
> +	bool match_depth;
>  };
>  
>  struct hantro_reg {
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> index 29cc61d53b71..334f18a4120d 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.c
> +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> @@ -64,6 +64,42 @@ hantro_get_postproc_formats(const struct hantro_ctx *ctx,
>  	return ctx->dev->variant->postproc_fmts;
>  }
>  
> +int hantro_get_format_depth(u32 fourcc)
> +{
> +	switch (fourcc) {
> +	case V4L2_PIX_FMT_P010:
> +	case V4L2_PIX_FMT_P010_4L4:
> +		return 10;
> +	default:
> +		return 8;
> +	}
> +}
> +
> +static bool
> +hantro_check_depth_match(const struct hantro_ctx *ctx,
> +			 const struct hantro_fmt *fmt)
> +{
> +	int fmt_depth, ctx_depth = 8;
> +
> +	if (!fmt->match_depth && !fmt->postprocessed)
> +		return true;
> +
> +	/* 0 means default depth, which is 8 */
> +	if (ctx->bit_depth)
> +		ctx_depth = ctx->bit_depth;
> +
> +	fmt_depth = hantro_get_format_depth(fmt->fourcc);
> +
> +	/*
> +	 * Allow only downconversion for postproc formats for now.
> +	 * It may be possible to relax that on some HW.
> +	 */
> +	if (!fmt->match_depth)
> +		return fmt_depth <= ctx_depth;
> +
> +	return fmt_depth == ctx_depth;
> +}
> +
>  static const struct hantro_fmt *
>  hantro_find_format(const struct hantro_ctx *ctx, u32 fourcc)
>  {
> @@ -91,7 +127,8 @@ hantro_get_default_fmt(const struct hantro_ctx *ctx, bool bitstream)
>  	formats = hantro_get_formats(ctx, &num_fmts);
>  	for (i = 0; i < num_fmts; i++) {
>  		if (bitstream == (formats[i].codec_mode !=
> -				  HANTRO_MODE_NONE))
> +				  HANTRO_MODE_NONE) &&
> +		    hantro_check_depth_match(ctx, &formats[i]))
>  			return &formats[i];
>  	}
>  	return NULL;
> @@ -162,11 +199,13 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
>  	formats = hantro_get_formats(ctx, &num_fmts);
>  	for (i = 0; i < num_fmts; i++) {
>  		bool mode_none = formats[i].codec_mode == HANTRO_MODE_NONE;
> +		fmt = &formats[i];
>  
>  		if (skip_mode_none == mode_none)
>  			continue;
> +		if (!hantro_check_depth_match(ctx, fmt))
> +			continue;
>  		if (j == f->index) {
> -			fmt = &formats[i];
>  			f->pixelformat = fmt->fourcc;
>  			return 0;
>  		}
> @@ -182,8 +221,11 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
>  		return -EINVAL;
>  	formats = hantro_get_postproc_formats(ctx, &num_fmts);
>  	for (i = 0; i < num_fmts; i++) {
> +		fmt = &formats[i];
> +
> +		if (!hantro_check_depth_match(ctx, fmt))
> +			continue;
>  		if (j == f->index) {
> -			fmt = &formats[i];
>  			f->pixelformat = fmt->fourcc;
>  			return 0;
>  		}
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.h b/drivers/staging/media/hantro/hantro_v4l2.h
> index 18bc682c8556..b17e84c82582 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.h
> +++ b/drivers/staging/media/hantro/hantro_v4l2.h
> @@ -22,5 +22,6 @@ extern const struct v4l2_ioctl_ops hantro_ioctl_ops;
>  extern const struct vb2_ops hantro_queue_ops;
>  
>  void hantro_reset_fmts(struct hantro_ctx *ctx);
> +int hantro_get_format_depth(u32 fourcc);
>  
>  #endif /* HANTRO_V4L2_H_ */
> -- 
> 2.36.1
>