Re: [PATCH v10 8/9] media: hantro: Introduce G2/HEVC decoder

2021-05-05 Thread Hans Verkuil
Hi Benjamin,

Some comments:

On 20/04/2021 14:10, Benjamin Gaignard wrote:
> Implement all the logic to get G2 hardware decoding HEVC frames.
> It supports up level 5.1 HEVC stream.
> It doesn't support yet 10 bits formats or the scaling feature.
> 
> Add HANTRO HEVC dedicated control to skip some bits at the beginning
> of the slice header. That is very specific to this hardware so can't
> go into uapi structures. Computing the needed value is complex and requires
> information from the stream that only the userland knows so let it
> provide the correct value to the driver.
> 
> Signed-off-by: Benjamin Gaignard 
> Co-developed-by: Adrian Ratiu 
> Signed-off-by: Adrian Ratiu 
> Co-developed-by: Ezequiel Garcia 
> Signed-off-by: Ezequiel Garcia 
> ---
>  drivers/staging/media/hantro/Makefile |   2 +
>  drivers/staging/media/hantro/hantro.h |   2 +
>  drivers/staging/media/hantro/hantro_drv.c |  36 ++
>  .../staging/media/hantro/hantro_g2_hevc_dec.c | 587 ++
>  drivers/staging/media/hantro/hantro_g2_regs.h | 198 ++
>  drivers/staging/media/hantro/hantro_hevc.c| 327 ++
>  drivers/staging/media/hantro/hantro_hw.h  |  49 ++
>  7 files changed, 1201 insertions(+)
>  create mode 100644 drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>  create mode 100644 drivers/staging/media/hantro/hantro_g2_regs.h
>  create mode 100644 drivers/staging/media/hantro/hantro_hevc.c
> 
> diff --git a/drivers/staging/media/hantro/Makefile 
> b/drivers/staging/media/hantro/Makefile
> index 743ce08eb184..0357f1772267 100644
> --- a/drivers/staging/media/hantro/Makefile
> +++ b/drivers/staging/media/hantro/Makefile
> @@ -9,12 +9,14 @@ hantro-vpu-y += \
>   hantro_h1_jpeg_enc.o \
>   hantro_g1_h264_dec.o \
>   hantro_g1_mpeg2_dec.o \
> + hantro_g2_hevc_dec.o \
>   hantro_g1_vp8_dec.o \
>   rk3399_vpu_hw_jpeg_enc.o \
>   rk3399_vpu_hw_mpeg2_dec.o \
>   rk3399_vpu_hw_vp8_dec.o \
>   hantro_jpeg.o \
>   hantro_h264.o \
> + hantro_hevc.o \
>   hantro_mpeg2.o \
>   hantro_vp8.o
>  
> diff --git a/drivers/staging/media/hantro/hantro.h 
> b/drivers/staging/media/hantro/hantro.h
> index e50d39b51902..a70c386de6f1 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -221,6 +221,7 @@ struct hantro_dev {
>   * @jpeg_enc:JPEG-encoding context.
>   * @mpeg2_dec:   MPEG-2-decoding context.
>   * @vp8_dec: VP8-decoding context.
> + * @hevc_dec:HEVC-decoding context.
>   */
>  struct hantro_ctx {
>   struct hantro_dev *dev;
> @@ -247,6 +248,7 @@ struct hantro_ctx {
>   struct hantro_jpeg_enc_hw_ctx jpeg_enc;
>   struct hantro_mpeg2_dec_hw_ctx mpeg2_dec;
>   struct hantro_vp8_dec_hw_ctx vp8_dec;
> + struct hantro_hevc_dec_hw_ctx hevc_dec;
>   };
>  };
>  
> diff --git a/drivers/staging/media/hantro/hantro_drv.c 
> b/drivers/staging/media/hantro/hantro_drv.c
> index d9a3a5ef9330..905a69758b37 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -281,6 +281,26 @@ static int hantro_jpeg_s_ctrl(struct v4l2_ctrl *ctrl)
>   return 0;
>  }
>  
> +static int hantro_hevc_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct hantro_ctx *ctx;
> +
> + ctx = container_of(ctrl->handler,
> +struct hantro_ctx, ctrl_handler);
> +
> + vpu_debug(1, "s_ctrl: id = %d, val = %d\n", ctrl->id, ctrl->val);
> +
> + switch (ctrl->id) {
> + case V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP:
> + ctx->hevc_dec.ctrls.hevc_hdr_skip_length = ctrl->val;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
>  static const struct v4l2_ctrl_ops hantro_ctrl_ops = {
>   .try_ctrl = hantro_try_ctrl,
>  };
> @@ -289,6 +309,10 @@ static const struct v4l2_ctrl_ops hantro_jpeg_ctrl_ops = 
> {
>   .s_ctrl = hantro_jpeg_s_ctrl,
>  };
>  
> +static const struct v4l2_ctrl_ops hantro_hevc_ctrl_ops = {
> + .s_ctrl = hantro_hevc_s_ctrl,
> +};
> +
>  static const struct hantro_ctrl controls[] = {
>   {
>   .codec = HANTRO_JPEG_ENCODER,
> @@ -409,6 +433,18 @@ static const struct hantro_ctrl controls[] = {
>   .cfg = {
>   .id = V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS,
>   },
> + }, {
> + .codec = HANTRO_HEVC_DECODER,
> + .cfg = {
> + .id = V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP,
> + .name = "Hantro HEVC slice header skip bytes",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 0,
> + .def = 0,
> + .max = 0x100,
> + .step = 1,
> +   

[PATCH v10 8/9] media: hantro: Introduce G2/HEVC decoder

2021-04-20 Thread Benjamin Gaignard
Implement all the logic to get G2 hardware decoding HEVC frames.
It supports up level 5.1 HEVC stream.
It doesn't support yet 10 bits formats or the scaling feature.

Add HANTRO HEVC dedicated control to skip some bits at the beginning
of the slice header. That is very specific to this hardware so can't
go into uapi structures. Computing the needed value is complex and requires
information from the stream that only the userland knows so let it
provide the correct value to the driver.

Signed-off-by: Benjamin Gaignard 
Co-developed-by: Adrian Ratiu 
Signed-off-by: Adrian Ratiu 
Co-developed-by: Ezequiel Garcia 
Signed-off-by: Ezequiel Garcia 
---
 drivers/staging/media/hantro/Makefile |   2 +
 drivers/staging/media/hantro/hantro.h |   2 +
 drivers/staging/media/hantro/hantro_drv.c |  36 ++
 .../staging/media/hantro/hantro_g2_hevc_dec.c | 587 ++
 drivers/staging/media/hantro/hantro_g2_regs.h | 198 ++
 drivers/staging/media/hantro/hantro_hevc.c| 327 ++
 drivers/staging/media/hantro/hantro_hw.h  |  49 ++
 7 files changed, 1201 insertions(+)
 create mode 100644 drivers/staging/media/hantro/hantro_g2_hevc_dec.c
 create mode 100644 drivers/staging/media/hantro/hantro_g2_regs.h
 create mode 100644 drivers/staging/media/hantro/hantro_hevc.c

diff --git a/drivers/staging/media/hantro/Makefile 
b/drivers/staging/media/hantro/Makefile
index 743ce08eb184..0357f1772267 100644
--- a/drivers/staging/media/hantro/Makefile
+++ b/drivers/staging/media/hantro/Makefile
@@ -9,12 +9,14 @@ hantro-vpu-y += \
hantro_h1_jpeg_enc.o \
hantro_g1_h264_dec.o \
hantro_g1_mpeg2_dec.o \
+   hantro_g2_hevc_dec.o \
hantro_g1_vp8_dec.o \
rk3399_vpu_hw_jpeg_enc.o \
rk3399_vpu_hw_mpeg2_dec.o \
rk3399_vpu_hw_vp8_dec.o \
hantro_jpeg.o \
hantro_h264.o \
+   hantro_hevc.o \
hantro_mpeg2.o \
hantro_vp8.o
 
diff --git a/drivers/staging/media/hantro/hantro.h 
b/drivers/staging/media/hantro/hantro.h
index e50d39b51902..a70c386de6f1 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -221,6 +221,7 @@ struct hantro_dev {
  * @jpeg_enc:  JPEG-encoding context.
  * @mpeg2_dec: MPEG-2-decoding context.
  * @vp8_dec:   VP8-decoding context.
+ * @hevc_dec:  HEVC-decoding context.
  */
 struct hantro_ctx {
struct hantro_dev *dev;
@@ -247,6 +248,7 @@ struct hantro_ctx {
struct hantro_jpeg_enc_hw_ctx jpeg_enc;
struct hantro_mpeg2_dec_hw_ctx mpeg2_dec;
struct hantro_vp8_dec_hw_ctx vp8_dec;
+   struct hantro_hevc_dec_hw_ctx hevc_dec;
};
 };
 
diff --git a/drivers/staging/media/hantro/hantro_drv.c 
b/drivers/staging/media/hantro/hantro_drv.c
index d9a3a5ef9330..905a69758b37 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -281,6 +281,26 @@ static int hantro_jpeg_s_ctrl(struct v4l2_ctrl *ctrl)
return 0;
 }
 
+static int hantro_hevc_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct hantro_ctx *ctx;
+
+   ctx = container_of(ctrl->handler,
+  struct hantro_ctx, ctrl_handler);
+
+   vpu_debug(1, "s_ctrl: id = %d, val = %d\n", ctrl->id, ctrl->val);
+
+   switch (ctrl->id) {
+   case V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP:
+   ctx->hevc_dec.ctrls.hevc_hdr_skip_length = ctrl->val;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static const struct v4l2_ctrl_ops hantro_ctrl_ops = {
.try_ctrl = hantro_try_ctrl,
 };
@@ -289,6 +309,10 @@ static const struct v4l2_ctrl_ops hantro_jpeg_ctrl_ops = {
.s_ctrl = hantro_jpeg_s_ctrl,
 };
 
+static const struct v4l2_ctrl_ops hantro_hevc_ctrl_ops = {
+   .s_ctrl = hantro_hevc_s_ctrl,
+};
+
 static const struct hantro_ctrl controls[] = {
{
.codec = HANTRO_JPEG_ENCODER,
@@ -409,6 +433,18 @@ static const struct hantro_ctrl controls[] = {
.cfg = {
.id = V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS,
},
+   }, {
+   .codec = HANTRO_HEVC_DECODER,
+   .cfg = {
+   .id = V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP,
+   .name = "Hantro HEVC slice header skip bytes",
+   .type = V4L2_CTRL_TYPE_INTEGER,
+   .min = 0,
+   .def = 0,
+   .max = 0x100,
+   .step = 1,
+   .ops = &hantro_hevc_ctrl_ops,
+   },
},
 };
 
diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c 
b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
new file mode 100644
index ..03a193a50c09
--- /