[PATCH 1/2] drm/vc4: Correct lbm size and calculation

[Dave Stevenson]

Hi Maxime

On Thu, 21 Jan 2021 at 10:58, Maxime Ripard <maxime at cerno.tech> wrote:
>
> From: Dom Cobley <popcornmix at gmail.com>
>
> LBM base address is measured in units of pixels per cycle.
> That is 4 for 2711 (hvs5) and 2 for 2708.
>
> We are wasting 75% of lbm by indexing without the scaling.
> But we were also using too high a size for the lbm resulting
> in partial corruption (right hand side) of vertically
> scaled images, usually at 4K or lower resolutions with more layers.
>
> The physical RAM of LBM on 2711 is 8 * 1920 * 16 * 12-bit
> (pixels are stored 12-bits per component regardless of format).
>
> The LBM adress indexes work in units of pixels per clock,
> so for 4 pixels per clock that means we have 32 * 1920 = 60K
>
> Fixes: c54619b0bfb3 ("drm/vc4: Add support for the BCM2711 HVS5")
> Signed-off-by: Dom Cobley <popcornmix at gmail.com>
> Signed-off-by: Maxime Ripard <maxime at cerno.tech>

Reviewed-by: Dave Stevenson <dave.stevenson at raspberrypi.com>

> ---
>  drivers/gpu/drm/vc4/vc4_hvs.c   | 8 ++++----
>  drivers/gpu/drm/vc4/vc4_plane.c | 7 ++++++-
>  2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 2b3a597fa65f..c239045e05d6 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -622,11 +622,11 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
>          * for now we just allocate globally.
>          */
>         if (!hvs->hvs5)
> -               /* 96kB */
> -               drm_mm_init(&hvs->lbm_mm, 0, 96 * 1024);
> +               /* 48k words of 2x12-bit pixels */
> +               drm_mm_init(&hvs->lbm_mm, 0, 48 * 1024);
>         else
> -               /* 70k words */
> -               drm_mm_init(&hvs->lbm_mm, 0, 70 * 2 * 1024);
> +               /* 60k words of 4x12-bit pixels */
> +               drm_mm_init(&hvs->lbm_mm, 0, 60 * 1024);
>
>         /* Upload filter kernels.  We only have the one for now, so we
>          * keep it around for the lifetime of the driver.
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 6bd8260aa9f2..b98eabb52920 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -437,6 +437,7 @@ static void vc4_write_ppf(struct vc4_plane_state *vc4_state, u32 src, u32 dst)
>  static u32 vc4_lbm_size(struct drm_plane_state *state)
>  {
>         struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
> +       struct vc4_dev *vc4 = to_vc4_dev(state->plane->dev);
>         u32 pix_per_line;
>         u32 lbm;
>
> @@ -472,7 +473,11 @@ static u32 vc4_lbm_size(struct drm_plane_state *state)
>                 lbm = pix_per_line * 16;
>         }
>
> -       lbm = roundup(lbm, 32);
> +       /* Align it to 64 or 128 (hvs5) bytes */
> +       lbm = roundup(lbm, vc4->hvs->hvs5 ? 128 : 64);
> +
> +       /* Each "word" of the LBM memory contains 2 or 4 (hvs5) pixels */
> +       lbm /= vc4->hvs->hvs5 ? 4 : 2;
>
>         return lbm;
>  }
> --
> 2.29.2
>