diff mbox series

[v3,5/8] media: hantro: hevc: Allow to produce 10-bit frames

Message ID 20210618131526.566762-6-benjamin.gaignard@collabora.com (mailing list archive)
State New, archived
Headers show
Series Additional features for Hantro HEVC | expand

Commit Message

Benjamin Gaignard June 18, 2021, 1:15 p.m. UTC
If Hantro driver receive an 10-bit encoded bitstream allow it
to produce 10-bit frames.
Check that we are not try to produce 10-bit frames from a 8-bit
encoded bitstream.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 drivers/staging/media/hantro/hantro_drv.c      | 18 ++++++++++++++++++
 .../staging/media/hantro/hantro_g2_hevc_dec.c  | 18 ++++++++++++++----
 drivers/staging/media/hantro/hantro_hevc.c     |  2 +-
 drivers/staging/media/hantro/imx8m_vpu_hw.c    |  4 ++++
 4 files changed, 37 insertions(+), 5 deletions(-)

Comments

Ezequiel Garcia June 22, 2021, 1:26 p.m. UTC | #1
Hi Benjamin,

On Fri, 2021-06-18 at 15:15 +0200, Benjamin Gaignard wrote:
> If Hantro driver receive an 10-bit encoded bitstream allow it
> to produce 10-bit frames.
> Check that we are not try to produce 10-bit frames from a 8-bit
> encoded bitstream.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
>  drivers/staging/media/hantro/hantro_drv.c      | 18 ++++++++++++++++++
>  .../staging/media/hantro/hantro_g2_hevc_dec.c  | 18 ++++++++++++++----
>  drivers/staging/media/hantro/hantro_hevc.c     |  2 +-
>  drivers/staging/media/hantro/imx8m_vpu_hw.c    |  4 ++++
>  4 files changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index f6635ceb5111..b6373934734e 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -243,6 +243,16 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>         return vb2_queue_init(dst_vq);
>  }
>  
> +static bool hantro_is_10bit_dst_format(struct hantro_ctx *ctx)
> +{
> +       switch (ctx->vpu_dst_fmt->fourcc) {
> +       case V4L2_PIX_FMT_P010:
> +               return true;
> +       default:
> +               return false;
> +       }
> +}
> +
>  static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
>  {
>         if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
> @@ -259,6 +269,10 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
>                         return -EINVAL;
>         } else if (ctrl->id == V4L2_CID_MPEG_VIDEO_HEVC_SPS) {
>                 const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
> +               struct hantro_ctx *ctx;
> +
> +               ctx = container_of(ctrl->handler,
> +                                  struct hantro_ctx, ctrl_handler);
>  
>                 if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
>                         /* Luma and chroma bit depth mismatch */
> @@ -270,6 +284,10 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
>                 if (sps->flags & V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED)
>                         /* No scaling support */
>                         return -EINVAL;
> +               if (sps->bit_depth_luma_minus8 == 0 &&
> +                   hantro_is_10bit_dst_format(ctx)) {
> +                       return -EINVAL;

I had some more time to think about this,
and I recalled that this topic was already discussed
some time ago [1].

This approach won't work. You need to restrict the pixel
formats returned by TRY/G_FMT after the SPS is set with S_EXT_CTRL,
as per the specification.

https://www.spinics.net/lists/kernel/msg3576779.html

Thanks,
Ezequiel
diff mbox series

Patch

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index f6635ceb5111..b6373934734e 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -243,6 +243,16 @@  queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 	return vb2_queue_init(dst_vq);
 }
 
+static bool hantro_is_10bit_dst_format(struct hantro_ctx *ctx)
+{
+	switch (ctx->vpu_dst_fmt->fourcc) {
+	case V4L2_PIX_FMT_P010:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
 {
 	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
@@ -259,6 +269,10 @@  static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
 			return -EINVAL;
 	} else if (ctrl->id == V4L2_CID_MPEG_VIDEO_HEVC_SPS) {
 		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
+		struct hantro_ctx *ctx;
+
+		ctx = container_of(ctrl->handler,
+				   struct hantro_ctx, ctrl_handler);
 
 		if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
 			/* Luma and chroma bit depth mismatch */
@@ -270,6 +284,10 @@  static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
 		if (sps->flags & V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED)
 			/* No scaling support */
 			return -EINVAL;
+		if (sps->bit_depth_luma_minus8 == 0 &&
+		    hantro_is_10bit_dst_format(ctx)) {
+			return -EINVAL;
+		}
 	}
 	return 0;
 }
diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
index 7c4427d8e49f..41dc89ec926c 100644
--- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
+++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
@@ -143,6 +143,16 @@  static bool is_8bit_dst_format(struct hantro_ctx *ctx)
 	}
 }
 
+static int get_dst_format(struct hantro_ctx *ctx)
+{
+	switch (ctx->vpu_dst_fmt->fourcc) {
+	case V4L2_PIX_FMT_P010:
+		return 0x1;
+	default:
+		return 0x0;
+	}
+}
+
 static void set_params(struct hantro_ctx *ctx)
 {
 	const struct hantro_hevc_dec_ctrls *ctrls = &ctx->hevc_dec.ctrls;
@@ -158,8 +168,8 @@  static void set_params(struct hantro_ctx *ctx)
 	hantro_reg_write(vpu, &g2_bit_depth_y_minus8, sps->bit_depth_luma_minus8);
 	hantro_reg_write(vpu, &g2_bit_depth_c_minus8, sps->bit_depth_chroma_minus8);
 
-	hantro_reg_write(vpu, &g2_output_8_bits, 1);
-	hantro_reg_write(vpu, &g2_output_format, 0);
+	hantro_reg_write(vpu, &g2_output_8_bits, is_8bit_dst_format(ctx));
+	hantro_reg_write(vpu, &g2_output_format, get_dst_format(ctx));
 
 	hantro_reg_write(vpu, &g2_hdr_skip_length, ctrls->hevc_hdr_skip_length);
 
@@ -540,7 +550,7 @@  static size_t hantro_hevc_output_chroma_offset(struct hantro_ctx *ctx)
 	int bytes_per_pixel = is_8bit_dst_format(ctx) ? 1 : 2;
 
 	return sps->pic_width_in_luma_samples *
-		sps->pic_height_in_luma_samples * bytes_per_pixel;
+	       sps->pic_height_in_luma_samples * bytes_per_pixel;
 }
 
 static void set_buffers(struct hantro_ctx *ctx)
@@ -627,7 +637,7 @@  int hantro_g2_hevc_dec_run(struct hantro_ctx *ctx)
 	/* Compress buffers */
 	hantro_reg_write(vpu, &g2_ref_compress_bypass, 0);
 
-	/* use NV12 as output format */
+	/* Use raster-scan as output format */
 	hantro_reg_write(vpu, &g2_out_rs_e, 1);
 
 	/* Bus width and max burst */
diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/staging/media/hantro/hantro_hevc.c
index 112b12a84df4..b646bd559ffe 100644
--- a/drivers/staging/media/hantro/hantro_hevc.c
+++ b/drivers/staging/media/hantro/hantro_hevc.c
@@ -25,7 +25,7 @@ 
 
 #define UNUSED_REF	-1
 
-#define G2_ALIGN		16
+#define G2_ALIGN	16
 
 #define CBS_SIZE	16 	/* compression table size in bytes */
 #define CBS_LUMA 	8 	/* luminance CBS is composed of 1 8x8 coded block */
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index ab6ac620f0d0..65bcf46740d7 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -135,6 +135,10 @@  static const struct hantro_fmt imx8m_vpu_g2_dec_fmts[] = {
 		.fourcc = V4L2_PIX_FMT_NV12,
 		.codec_mode = HANTRO_MODE_NONE,
 	},
+	{
+		.fourcc = V4L2_PIX_FMT_P010,
+		.codec_mode = HANTRO_MODE_NONE,
+	},
 	{
 		.fourcc = V4L2_PIX_FMT_HEVC_SLICE,
 		.codec_mode = HANTRO_MODE_HEVC_DEC,