Re: [PATCH v3 1/4] xlnx_dp: fix the wrong register size

From: Alistair Francis <alistair23@gmail.com>
To: frederic.konrad@xilinx.com
Cc: "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	qemu-arm <qemu-arm@nongnu.org>,
	Peter Maydell <peter.maydell@linaro.org>,
	Edgar Iglesias <edgar.iglesias@gmail.com>,
	 Alistair Francis <alistair@alistair23.me>,
	Sai Pavan Boddu <saipava@xilinx.com>,
	Edgar Iglesias <edgari@xilinx.com>,
	fkonrad@amd.com,  "Edgar E . Iglesias" <edgar.iglesias@amd.com>
Subject: Re: [PATCH v3 1/4] xlnx_dp: fix the wrong register size
Date: Thu, 2 Jun 2022 07:14:27 +1000	[thread overview]
Message-ID: <CAKmqyKMUnYrb4o40WCbjAJ4igAjw0W1=S=D1WQEBGU7agba_CQ@mail.gmail.com> (raw)
In-Reply-To: <20220601172353.3220232-2-fkonrad@xilinx.com>

On Thu, Jun 2, 2022 at 3:26 AM <frederic.konrad@xilinx.com> wrote:
>
> From: Frederic Konrad <fkonrad@amd.com>
>
> The core and the vblend registers size are wrong, they should respectively be
> 0x3B0 and 0x1E0 according to:
>   https://www.xilinx.com/htmldocs/registers/ug1087/ug1087-zynq-ultrascale-registers.html.
>
> Let's fix that and use macros when creating the mmio region.
>
> Fixes: 58ac482a66d ("introduce xlnx-dp")
> Signed-off-by: Frederic Konrad <fkonrad@amd.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/display/xlnx_dp.c         | 17 ++++++++++-------
>  include/hw/display/xlnx_dp.h |  9 +++++++--
>  2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index 9bb781e312..0378570459 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -1219,19 +1219,22 @@ static void xlnx_dp_init(Object *obj)
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>      XlnxDPState *s = XLNX_DP(obj);
>
> -    memory_region_init(&s->container, obj, TYPE_XLNX_DP, 0xC050);
> +    memory_region_init(&s->container, obj, TYPE_XLNX_DP, DP_CONTAINER_SIZE);
>
>      memory_region_init_io(&s->core_iomem, obj, &dp_ops, s, TYPE_XLNX_DP
> -                          ".core", 0x3AF);
> -    memory_region_add_subregion(&s->container, 0x0000, &s->core_iomem);
> +                          ".core", sizeof(s->core_registers));
> +    memory_region_add_subregion(&s->container, DP_CORE_REG_OFFSET,
> +                                &s->core_iomem);
>
>      memory_region_init_io(&s->vblend_iomem, obj, &vblend_ops, s, TYPE_XLNX_DP
> -                          ".v_blend", 0x1DF);
> -    memory_region_add_subregion(&s->container, 0xA000, &s->vblend_iomem);
> +                          ".v_blend", sizeof(s->vblend_registers));
> +    memory_region_add_subregion(&s->container, DP_VBLEND_REG_OFFSET,
> +                                &s->vblend_iomem);
>
>      memory_region_init_io(&s->avbufm_iomem, obj, &avbufm_ops, s, TYPE_XLNX_DP
> -                          ".av_buffer_manager", 0x238);
> -    memory_region_add_subregion(&s->container, 0xB000, &s->avbufm_iomem);
> +                          ".av_buffer_manager", sizeof(s->avbufm_registers));
> +    memory_region_add_subregion(&s->container, DP_AVBUF_REG_OFFSET,
> +                                &s->avbufm_iomem);
>
>      memory_region_init_io(&s->audio_iomem, obj, &audio_ops, s, TYPE_XLNX_DP
>                            ".audio", sizeof(s->audio_registers));
> diff --git a/include/hw/display/xlnx_dp.h b/include/hw/display/xlnx_dp.h
> index 8ab4733bb8..1ef5a89ee7 100644
> --- a/include/hw/display/xlnx_dp.h
> +++ b/include/hw/display/xlnx_dp.h
> @@ -39,10 +39,15 @@
>  #define AUD_CHBUF_MAX_DEPTH                 (32 * KiB)
>  #define MAX_QEMU_BUFFER_SIZE                (4 * KiB)
>
> -#define DP_CORE_REG_ARRAY_SIZE              (0x3AF >> 2)
> +#define DP_CORE_REG_OFFSET                  (0x0000)
> +#define DP_CORE_REG_ARRAY_SIZE              (0x3B0 >> 2)
> +#define DP_AVBUF_REG_OFFSET                 (0xB000)
>  #define DP_AVBUF_REG_ARRAY_SIZE             (0x238 >> 2)
> -#define DP_VBLEND_REG_ARRAY_SIZE            (0x1DF >> 2)
> +#define DP_VBLEND_REG_OFFSET                (0xA000)
> +#define DP_VBLEND_REG_ARRAY_SIZE            (0x1E0 >> 2)
> +#define DP_AUDIO_REG_OFFSET                 (0xC000)
>  #define DP_AUDIO_REG_ARRAY_SIZE             (0x50 >> 2)
> +#define DP_CONTAINER_SIZE                   (0xC050)
>
>  struct PixmanPlane {
>      pixman_format_code_t format;
> --
> 2.25.1
>
>