linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] media: hantro: Auto generate the AXI ID to avoid conflicts
@ 2021-09-24 13:24 Benjamin Gaignard
  2021-10-14  9:04 ` Ezequiel Garcia
  0 siblings, 1 reply; 2+ messages in thread
From: Benjamin Gaignard @ 2021-09-24 13:24 UTC (permalink / raw)
  To: p.zabel, mchehab, gregkh
  Cc: linux-media, linux-rockchip, linux-staging, linux-kernel,
	Benjamin Gaignard, Enric Balletbo i Serra

The AXI ID is an AXI bus configuration for improve bus performance. 
If read and write operations use different ID the operations can be paralleled,
whereas when they have the same ID the operations will be serialized. 
Right now, the write ID is fixed to 0 but we can set it to 0xff to get auto
generated ID to avoid possible conflicts.

This change has no functional changes, but seems reasonable to let the
hardware to autogenerate the ID instead of hardcoding in software.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
changes in version 2:
- Add a macro with comment about the value.
- Make VP8 and H264 codecs use the macro.
- fluster tests on the both codecs show no regressions.
  ./fluster.py run -ts VP8-TEST-VECTORS -d GStreamer-VP8-V4L2SL-Gst1.0
  ./fluster.py run -ts JVT-AVC_V1 -d GStreamer-H.264-V4L2SL-Gst1.0
- The both codec write other bits in the same configuration register
  so the simplest solution is to use the macro in the both cases.

 drivers/staging/media/hantro/hantro_g1_h264_dec.c | 2 +-
 drivers/staging/media/hantro/hantro_g1_regs.h     | 2 ++
 drivers/staging/media/hantro/hantro_g1_vp8_dec.c  | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
index 236ce24ca00c..f49dbfb8a843 100644
--- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
@@ -29,7 +29,7 @@ static void set_params(struct hantro_ctx *ctx, struct vb2_v4l2_buffer *src_buf)
 	u32 reg;
 
 	/* Decoder control register 0. */
-	reg = G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(0x0);
+	reg = G1_REG_DEC_CTRL0_DEC_AXI_AUTO;
 	if (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD)
 		reg |= G1_REG_DEC_CTRL0_SEQ_MBAFF_E;
 	if (sps->profile_idc > 66) {
diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
index c1756e3d5391..c623b3b0be18 100644
--- a/drivers/staging/media/hantro/hantro_g1_regs.h
+++ b/drivers/staging/media/hantro/hantro_g1_regs.h
@@ -68,6 +68,8 @@
 #define     G1_REG_DEC_CTRL0_PICORD_COUNT_E		BIT(9)
 #define     G1_REG_DEC_CTRL0_DEC_AHB_HLOCK_E		BIT(8)
 #define     G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(x)		(((x) & 0xff) << 0)
+/* Setting AXI ID to 0xff to get auto generated ID to avoid possible conflicts */
+#define     G1_REG_DEC_CTRL0_DEC_AXI_AUTO		G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(0xff)
 #define G1_REG_DEC_CTRL1				0x010
 #define     G1_REG_DEC_CTRL1_PIC_MB_WIDTH(x)		(((x) & 0x1ff) << 23)
 #define     G1_REG_DEC_CTRL1_MB_WIDTH_OFF(x)		(((x) & 0xf) << 19)
diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
index 6180b23e7d94..851eb67f19f5 100644
--- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
@@ -463,7 +463,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
 	      G1_REG_CONFIG_DEC_MAX_BURST(16);
 	vdpu_write_relaxed(vpu, reg, G1_REG_CONFIG);
 
-	reg = G1_REG_DEC_CTRL0_DEC_MODE(10);
+	reg = G1_REG_DEC_CTRL0_DEC_MODE(10) |
+	      G1_REG_DEC_CTRL0_DEC_AXI_AUTO;
 	if (!V4L2_VP8_FRAME_IS_KEY_FRAME(hdr))
 		reg |= G1_REG_DEC_CTRL0_PIC_INTER_E;
 	if (!(hdr->flags & V4L2_VP8_FRAME_FLAG_MB_NO_SKIP_COEFF))
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] media: hantro: Auto generate the AXI ID to avoid conflicts
  2021-09-24 13:24 [PATCH v2] media: hantro: Auto generate the AXI ID to avoid conflicts Benjamin Gaignard
@ 2021-10-14  9:04 ` Ezequiel Garcia
  0 siblings, 0 replies; 2+ messages in thread
From: Ezequiel Garcia @ 2021-10-14  9:04 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: p.zabel, mchehab, gregkh, linux-media, linux-rockchip,
	linux-staging, linux-kernel, Enric Balletbo i Serra

Hi Benjamin,

Thanks for picking up the patch.

On Fri, Sep 24, 2021 at 03:24:47PM +0200, Benjamin Gaignard wrote:
> The AXI ID is an AXI bus configuration for improve bus performance. 
> If read and write operations use different ID the operations can be paralleled,
> whereas when they have the same ID the operations will be serialized. 
> Right now, the write ID is fixed to 0 but we can set it to 0xff to get auto
> generated ID to avoid possible conflicts.
> 
> This change has no functional changes, but seems reasonable to let the
> hardware to autogenerate the ID instead of hardcoding in software.
> 

Well, it may not have functional changes, but it is expected to have
performance impact on a contended AXI bus.

> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> changes in version 2:
> - Add a macro with comment about the value.
> - Make VP8 and H264 codecs use the macro.
> - fluster tests on the both codecs show no regressions.
>   ./fluster.py run -ts VP8-TEST-VECTORS -d GStreamer-VP8-V4L2SL-Gst1.0
>   ./fluster.py run -ts JVT-AVC_V1 -d GStreamer-H.264-V4L2SL-Gst1.0
> - The both codec write other bits in the same configuration register
>   so the simplest solution is to use the macro in the both cases.
> 

Please describe the performance test that you did to make
sure it's not regressing.

>  drivers/staging/media/hantro/hantro_g1_h264_dec.c | 2 +-
>  drivers/staging/media/hantro/hantro_g1_regs.h     | 2 ++
>  drivers/staging/media/hantro/hantro_g1_vp8_dec.c  | 3 ++-

This covers VP8 and H264 on some of the cores (i.MX8 and Rockchip VPU1).
I believe you are missing the VPU2 cores (RK3399), see for instance
rockchip_vpu2_hw_h264_dec.c.

What about MPEG-2? If the tests show no performance impact,
then please do the change for MPEG-2 as well.

Thanks,
Ezequiel

>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> index 236ce24ca00c..f49dbfb8a843 100644
> --- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> +++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> @@ -29,7 +29,7 @@ static void set_params(struct hantro_ctx *ctx, struct vb2_v4l2_buffer *src_buf)
>  	u32 reg;
>  
>  	/* Decoder control register 0. */
> -	reg = G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(0x0);
> +	reg = G1_REG_DEC_CTRL0_DEC_AXI_AUTO;
>  	if (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD)
>  		reg |= G1_REG_DEC_CTRL0_SEQ_MBAFF_E;
>  	if (sps->profile_idc > 66) {
> diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
> index c1756e3d5391..c623b3b0be18 100644
> --- a/drivers/staging/media/hantro/hantro_g1_regs.h
> +++ b/drivers/staging/media/hantro/hantro_g1_regs.h
> @@ -68,6 +68,8 @@
>  #define     G1_REG_DEC_CTRL0_PICORD_COUNT_E		BIT(9)
>  #define     G1_REG_DEC_CTRL0_DEC_AHB_HLOCK_E		BIT(8)
>  #define     G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(x)		(((x) & 0xff) << 0)
> +/* Setting AXI ID to 0xff to get auto generated ID to avoid possible conflicts */
> +#define     G1_REG_DEC_CTRL0_DEC_AXI_AUTO		G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(0xff)
>  #define G1_REG_DEC_CTRL1				0x010
>  #define     G1_REG_DEC_CTRL1_PIC_MB_WIDTH(x)		(((x) & 0x1ff) << 23)
>  #define     G1_REG_DEC_CTRL1_MB_WIDTH_OFF(x)		(((x) & 0xf) << 19)
> diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> index 6180b23e7d94..851eb67f19f5 100644
> --- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> +++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> @@ -463,7 +463,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>  	      G1_REG_CONFIG_DEC_MAX_BURST(16);
>  	vdpu_write_relaxed(vpu, reg, G1_REG_CONFIG);
>  
> -	reg = G1_REG_DEC_CTRL0_DEC_MODE(10);
> +	reg = G1_REG_DEC_CTRL0_DEC_MODE(10) |
> +	      G1_REG_DEC_CTRL0_DEC_AXI_AUTO;
>  	if (!V4L2_VP8_FRAME_IS_KEY_FRAME(hdr))
>  		reg |= G1_REG_DEC_CTRL0_PIC_INTER_E;
>  	if (!(hdr->flags & V4L2_VP8_FRAME_FLAG_MB_NO_SKIP_COEFF))
> -- 
> 2.30.2
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-10-14  9:04 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 13:24 [PATCH v2] media: hantro: Auto generate the AXI ID to avoid conflicts Benjamin Gaignard
2021-10-14  9:04 ` Ezequiel Garcia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).