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@xxxxxxxxxxxxx>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
> ---
> 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
>
