Re: [PATCH v4 05/11] media: hantro: Add a field to distinguish the hardware versions

[Benjamin Gaignard]

Le 03/03/2021 à 23:05, Ezequiel Garcia a écrit :

On Wed, 2021-03-03 at 12:39 +0100, Benjamin Gaignard wrote:

Decoders hardware blocks could exist in multiple versions: add
a field to distinguish them at runtime.
G2 hardware block doesn't have postprocessor hantro_needs_postproc
function should always returns false in for this hardware.
hantro_needs_postproc function becoming to much complex to
stay inline in .h file move it to .c file.

Note that I already questioned this patch before:

https://lkml.org/lkml/2021/2/17/722

I think it's better to rely on of_device_id.data for this
type of thing.

In particular, I was expecting that just using
hantro_variant.postproc_regs would be enough.

Can you try if that works and avoid reading swreg(0)
and probing the hardware core?

I have found a way to remove this: if the variant doesn't define
post processor formats, needs_postproc function will always returns
false and that what the only useful usage of this version field.

Benjamin

Thanks!
Ezequiel

Keep the default behavoir to be G1 hardware.

Signed-off-by: Benjamin Gaignard <benjamin.gaign...@collabora.com>
---
  drivers/staging/media/hantro/hantro.h          | 13 +++++++------
  drivers/staging/media/hantro/hantro_drv.c      |  2 ++
  drivers/staging/media/hantro/hantro_postproc.c | 17 +++++++++++++++++
  3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h 
b/drivers/staging/media/hantro/hantro.h
index a76a0d79db9f..05876e426419 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -37,6 +37,9 @@ struct hantro_codec_ops;
  #define HANTRO_HEVC_DECODER    BIT(19)
  #define HANTRO_DECODERS                0xffff0000

+#define HANTRO_G1_REV          0x6731

+#define HANTRO_G2_REV          0x6732
+
  /**
   * struct hantro_irq - irq handler and name
   *
@@ -171,6 +174,7 @@ hantro_vdev_to_func(struct video_device *vdev)
   * @enc_base:          Mapped address of VPU encoder register for convenience.
   * @dec_base:          Mapped address of VPU decoder register for convenience.
   * @ctrl_base:         Mapped address of VPU control block.
+ * @core_hw_dec_rev    Runtime detected HW decoder core revision
   * @vpu_mutex:         Mutex to synchronize V4L2 calls.
   * @irqlock:           Spinlock to synchronize access to data structures
   *                     shared with interrupt handlers.
@@ -190,6 +194,7 @@ struct hantro_dev {
         void __iomem *enc_base;
         void __iomem *dec_base;
         void __iomem *ctrl_base;
+       u32 core_hw_dec_rev;

        struct mutex vpu_mutex; /* video_device lock */

         spinlock_t irqlock;
@@ -412,12 +417,8 @@ hantro_get_dst_buf(struct hantro_ctx *ctx)
         return v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
  }

-static inline bool

-hantro_needs_postproc(const struct hantro_ctx *ctx,
-                     const struct hantro_fmt *fmt)
-{
-       return !ctx->is_encoder && fmt->fourcc != V4L2_PIX_FMT_NV12;
-}
+bool hantro_needs_postproc(const struct hantro_ctx *ctx,
+                          const struct hantro_fmt *fmt);

 static inline dma_addr_t

  hantro_get_dec_buf_addr(struct hantro_ctx *ctx, struct vb2_buffer *vb)
diff --git a/drivers/staging/media/hantro/hantro_drv.c 
b/drivers/staging/media/hantro/hantro_drv.c
index f0b68e16fcc0..e3e6df28f470 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -836,6 +836,8 @@ static int hantro_probe(struct platform_device *pdev)
         }
         vpu->enc_base = vpu->reg_bases[0] + vpu->variant->enc_offset;
         vpu->dec_base = vpu->reg_bases[0] + vpu->variant->dec_offset;
+       /* by default decoder is G1 */
+       vpu->core_hw_dec_rev = HANTRO_G1_REV;

        ret = dma_set_coherent_mask(vpu->dev, DMA_BIT_MASK(32));

         if (ret) {
diff --git a/drivers/staging/media/hantro/hantro_postproc.c 
b/drivers/staging/media/hantro/hantro_postproc.c
index 6d2a8f2a8f0b..050880f720d6 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -50,6 +50,23 @@ const struct hantro_postproc_regs hantro_g1_postproc_regs = {
         .display_width = {G1_REG_PP_DISPLAY_WIDTH, 0, 0xfff},
  };

+bool hantro_needs_postproc(const struct hantro_ctx *ctx,

+                          const struct hantro_fmt *fmt)
+{
+       struct hantro_dev *vpu = ctx->dev;
+
+       if (ctx->is_encoder)
+               return false;
+
+       if (vpu->core_hw_dec_rev == HANTRO_G1_REV):q
+               return fmt->fourcc != V4L2_PIX_FMT_NV12;
+
+       if (vpu->core_hw_dec_rev == HANTRO_G2_REV)
+               return false;
+
+       return false;
+}
+
  void hantro_postproc_enable(struct hantro_ctx *ctx)
  {
         struct hantro_dev *vpu = ctx->dev;