diff mbox series

[1/2] media: verisilicon: Store reference frames pixels format

Message ID 01020192f83fdd04-16f415e6-64f6-44f2-8f79-7676d301d4ab-000000@eu-west-1.amazonses.com
State Superseded
Headers show
Series [1/2] media: verisilicon: Store reference frames pixels format | expand

Commit Message

Benjamin Gaignard Nov. 4, 2024, 5:36 p.m. UTC
Hantro decoder always produce tiled pixels formats but
when the post-processor is used the destination pixels
format is a non tiled pixels format. This led to compute
wrong reference frame size and offsets.
Getting and saving the correct tiled pixels format for 8
and 10 bit stream solve the computation issues.

Fluster VP9 score increase to 166/305 (vs 145/305).
HEVC score is still 141/147.


Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 drivers/media/platform/verisilicon/hantro.h   |  2 ++
 .../media/platform/verisilicon/hantro_g2.c    |  2 +-
 .../platform/verisilicon/hantro_postproc.c    | 28 ++++++-------------
 .../media/platform/verisilicon/hantro_v4l2.c  | 21 ++++++++++++++
 4 files changed, 33 insertions(+), 20 deletions(-)

Comments

Nicolas Dufresne Dec. 12, 2024, 2:38 p.m. UTC | #1
Le lundi 04 novembre 2024 à 17:36 +0000, Benjamin Gaignard a écrit :
> Hantro decoder always produce tiled pixels formats but
> when the post-processor is used the destination pixels
> format is a non tiled pixels format. This led to compute
> wrong reference frame size and offsets.
> Getting and saving the correct tiled pixels format for 8
> and 10 bit stream solve the computation issues.
> 
> Fluster VP9 score increase to 166/305 (vs 145/305).
> HEVC score is still 141/147.
> 
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

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

> ---
>  drivers/media/platform/verisilicon/hantro.h   |  2 ++
>  .../media/platform/verisilicon/hantro_g2.c    |  2 +-
>  .../platform/verisilicon/hantro_postproc.c    | 28 ++++++-------------
>  .../media/platform/verisilicon/hantro_v4l2.c  | 21 ++++++++++++++
>  4 files changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> index 811260dc3c77..14fc6a3e2878 100644
> --- a/drivers/media/platform/verisilicon/hantro.h
> +++ b/drivers/media/platform/verisilicon/hantro.h
> @@ -227,6 +227,7 @@ struct hantro_dev {
>   * @src_fmt:		V4L2 pixel format of active source format.
>   * @vpu_dst_fmt:	Descriptor of active destination format.
>   * @dst_fmt:		V4L2 pixel format of active destination format.
> + * @ref_fmt:		V4L2 pixel format of the reference frames format.
>   *
>   * @ctrl_handler:	Control handler used to register controls.
>   * @jpeg_quality:	User-specified JPEG compression quality.
> @@ -255,6 +256,7 @@ struct hantro_ctx {
>  	struct v4l2_pix_format_mplane src_fmt;
>  	const struct hantro_fmt *vpu_dst_fmt;
>  	struct v4l2_pix_format_mplane dst_fmt;
> +	struct v4l2_pix_format_mplane ref_fmt;
>  
>  	struct v4l2_ctrl_handler ctrl_handler;
>  	int jpeg_quality;
> diff --git a/drivers/media/platform/verisilicon/hantro_g2.c b/drivers/media/platform/verisilicon/hantro_g2.c
> index b880a6849d58..5d33d745d346 100644
> --- a/drivers/media/platform/verisilicon/hantro_g2.c
> +++ b/drivers/media/platform/verisilicon/hantro_g2.c
> @@ -47,7 +47,7 @@ irqreturn_t hantro_g2_irq(int irq, void *dev_id)
>  
>  size_t hantro_g2_chroma_offset(struct hantro_ctx *ctx)
>  {
> -	return ctx->dst_fmt.width * ctx->dst_fmt.height * ctx->bit_depth / 8;
> +	return ctx->ref_fmt.plane_fmt[0].bytesperline *	ctx->ref_fmt.height;
>  }
>  
>  size_t hantro_g2_motion_vectors_offset(struct hantro_ctx *ctx)
> diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
> index 41e93176300b..4ba29682dbd6 100644
> --- a/drivers/media/platform/verisilicon/hantro_postproc.c
> +++ b/drivers/media/platform/verisilicon/hantro_postproc.c
> @@ -194,31 +194,21 @@ void hantro_postproc_free(struct hantro_ctx *ctx)
>  
>  static unsigned int hantro_postproc_buffer_size(struct hantro_ctx *ctx)
>  {
> -	struct v4l2_pix_format_mplane pix_mp;
> -	const struct hantro_fmt *fmt;
>  	unsigned int buf_size;
>  
> -	/* this should always pick native format */
> -	fmt = hantro_get_default_fmt(ctx, false, ctx->bit_depth, HANTRO_AUTO_POSTPROC);
> -	if (!fmt)
> -		return 0;
> -
> -	v4l2_fill_pixfmt_mp(&pix_mp, fmt->fourcc, ctx->src_fmt.width,
> -			    ctx->src_fmt.height);
> -
> -	buf_size = pix_mp.plane_fmt[0].sizeimage;
> +	buf_size = ctx->ref_fmt.plane_fmt[0].sizeimage;
>  	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE)
> -		buf_size += hantro_h264_mv_size(pix_mp.width,
> -						pix_mp.height);
> +		buf_size += hantro_h264_mv_size(ctx->ref_fmt.width,
> +						ctx->ref_fmt.height);
>  	else if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_VP9_FRAME)
> -		buf_size += hantro_vp9_mv_size(pix_mp.width,
> -					       pix_mp.height);
> +		buf_size += hantro_vp9_mv_size(ctx->ref_fmt.width,
> +					       ctx->ref_fmt.height);
>  	else if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_HEVC_SLICE)
> -		buf_size += hantro_hevc_mv_size(pix_mp.width,
> -						pix_mp.height);
> +		buf_size += hantro_hevc_mv_size(ctx->ref_fmt.width,
> +						ctx->ref_fmt.height);
>  	else if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_AV1_FRAME)
> -		buf_size += hantro_av1_mv_size(pix_mp.width,
> -					       pix_mp.height);
> +		buf_size += hantro_av1_mv_size(ctx->ref_fmt.width,
> +					       ctx->ref_fmt.height);
>  
>  	return buf_size;
>  }
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index df6f2536263b..a9a54f177405 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -126,6 +126,24 @@ hantro_find_format(const struct hantro_ctx *ctx, u32 fourcc)
>  	return NULL;
>  }
>  
> +static int
> +hantro_set_reference_frames_format(struct hantro_ctx *ctx)
> +{
> +	const struct hantro_fmt *fmt;
> +	int dst_bit_depth = hantro_get_format_depth(ctx->vpu_dst_fmt->fourcc);
> +
> +	fmt = hantro_get_default_fmt(ctx, false, dst_bit_depth, HANTRO_AUTO_POSTPROC);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	ctx->ref_fmt.width = ctx->src_fmt.width;
> +	ctx->ref_fmt.height = ctx->src_fmt.height;
> +
> +	v4l2_apply_frmsize_constraints(&ctx->ref_fmt.width, &ctx->ref_fmt.height, &fmt->frmsize);
> +	return v4l2_fill_pixfmt_mp(&ctx->ref_fmt, fmt->fourcc,
> +				   ctx->ref_fmt.width, ctx->ref_fmt.height);
> +}
> +
>  const struct hantro_fmt *
>  hantro_get_default_fmt(const struct hantro_ctx *ctx, bool bitstream,
>  		       int bit_depth, bool need_postproc)
> @@ -591,6 +609,9 @@ static int hantro_set_fmt_cap(struct hantro_ctx *ctx,
>  
>  	ctx->vpu_dst_fmt = hantro_find_format(ctx, pix_mp->pixelformat);
>  	ctx->dst_fmt = *pix_mp;
> +	ret = hantro_set_reference_frames_format(ctx);
> +	if (ret)
> +		return ret;
>  
>  	/*
>  	 * Current raw format might have become invalid with newly
diff mbox series

Patch

diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
index 811260dc3c77..14fc6a3e2878 100644
--- a/drivers/media/platform/verisilicon/hantro.h
+++ b/drivers/media/platform/verisilicon/hantro.h
@@ -227,6 +227,7 @@  struct hantro_dev {
  * @src_fmt:		V4L2 pixel format of active source format.
  * @vpu_dst_fmt:	Descriptor of active destination format.
  * @dst_fmt:		V4L2 pixel format of active destination format.
+ * @ref_fmt:		V4L2 pixel format of the reference frames format.
  *
  * @ctrl_handler:	Control handler used to register controls.
  * @jpeg_quality:	User-specified JPEG compression quality.
@@ -255,6 +256,7 @@  struct hantro_ctx {
 	struct v4l2_pix_format_mplane src_fmt;
 	const struct hantro_fmt *vpu_dst_fmt;
 	struct v4l2_pix_format_mplane dst_fmt;
+	struct v4l2_pix_format_mplane ref_fmt;
 
 	struct v4l2_ctrl_handler ctrl_handler;
 	int jpeg_quality;
diff --git a/drivers/media/platform/verisilicon/hantro_g2.c b/drivers/media/platform/verisilicon/hantro_g2.c
index b880a6849d58..5d33d745d346 100644
--- a/drivers/media/platform/verisilicon/hantro_g2.c
+++ b/drivers/media/platform/verisilicon/hantro_g2.c
@@ -47,7 +47,7 @@  irqreturn_t hantro_g2_irq(int irq, void *dev_id)
 
 size_t hantro_g2_chroma_offset(struct hantro_ctx *ctx)
 {
-	return ctx->dst_fmt.width * ctx->dst_fmt.height * ctx->bit_depth / 8;
+	return ctx->ref_fmt.plane_fmt[0].bytesperline *	ctx->ref_fmt.height;
 }
 
 size_t hantro_g2_motion_vectors_offset(struct hantro_ctx *ctx)
diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
index 41e93176300b..4ba29682dbd6 100644
--- a/drivers/media/platform/verisilicon/hantro_postproc.c
+++ b/drivers/media/platform/verisilicon/hantro_postproc.c
@@ -194,31 +194,21 @@  void hantro_postproc_free(struct hantro_ctx *ctx)
 
 static unsigned int hantro_postproc_buffer_size(struct hantro_ctx *ctx)
 {
-	struct v4l2_pix_format_mplane pix_mp;
-	const struct hantro_fmt *fmt;
 	unsigned int buf_size;
 
-	/* this should always pick native format */
-	fmt = hantro_get_default_fmt(ctx, false, ctx->bit_depth, HANTRO_AUTO_POSTPROC);
-	if (!fmt)
-		return 0;
-
-	v4l2_fill_pixfmt_mp(&pix_mp, fmt->fourcc, ctx->src_fmt.width,
-			    ctx->src_fmt.height);
-
-	buf_size = pix_mp.plane_fmt[0].sizeimage;
+	buf_size = ctx->ref_fmt.plane_fmt[0].sizeimage;
 	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE)
-		buf_size += hantro_h264_mv_size(pix_mp.width,
-						pix_mp.height);
+		buf_size += hantro_h264_mv_size(ctx->ref_fmt.width,
+						ctx->ref_fmt.height);
 	else if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_VP9_FRAME)
-		buf_size += hantro_vp9_mv_size(pix_mp.width,
-					       pix_mp.height);
+		buf_size += hantro_vp9_mv_size(ctx->ref_fmt.width,
+					       ctx->ref_fmt.height);
 	else if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_HEVC_SLICE)
-		buf_size += hantro_hevc_mv_size(pix_mp.width,
-						pix_mp.height);
+		buf_size += hantro_hevc_mv_size(ctx->ref_fmt.width,
+						ctx->ref_fmt.height);
 	else if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_AV1_FRAME)
-		buf_size += hantro_av1_mv_size(pix_mp.width,
-					       pix_mp.height);
+		buf_size += hantro_av1_mv_size(ctx->ref_fmt.width,
+					       ctx->ref_fmt.height);
 
 	return buf_size;
 }
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index df6f2536263b..a9a54f177405 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -126,6 +126,24 @@  hantro_find_format(const struct hantro_ctx *ctx, u32 fourcc)
 	return NULL;
 }
 
+static int
+hantro_set_reference_frames_format(struct hantro_ctx *ctx)
+{
+	const struct hantro_fmt *fmt;
+	int dst_bit_depth = hantro_get_format_depth(ctx->vpu_dst_fmt->fourcc);
+
+	fmt = hantro_get_default_fmt(ctx, false, dst_bit_depth, HANTRO_AUTO_POSTPROC);
+	if (!fmt)
+		return -EINVAL;
+
+	ctx->ref_fmt.width = ctx->src_fmt.width;
+	ctx->ref_fmt.height = ctx->src_fmt.height;
+
+	v4l2_apply_frmsize_constraints(&ctx->ref_fmt.width, &ctx->ref_fmt.height, &fmt->frmsize);
+	return v4l2_fill_pixfmt_mp(&ctx->ref_fmt, fmt->fourcc,
+				   ctx->ref_fmt.width, ctx->ref_fmt.height);
+}
+
 const struct hantro_fmt *
 hantro_get_default_fmt(const struct hantro_ctx *ctx, bool bitstream,
 		       int bit_depth, bool need_postproc)
@@ -591,6 +609,9 @@  static int hantro_set_fmt_cap(struct hantro_ctx *ctx,
 
 	ctx->vpu_dst_fmt = hantro_find_format(ctx, pix_mp->pixelformat);
 	ctx->dst_fmt = *pix_mp;
+	ret = hantro_set_reference_frames_format(ctx);
+	if (ret)
+		return ret;
 
 	/*
 	 * Current raw format might have become invalid with newly