[PATCH 1/2] drm: lcdif: move pitch setup to plane atomic update

[Ying Liu]

Hi Lucas,

On Thu, Jun 1, 2023 at 2:45 AM Lucas Stach <l.stach at pengutronix.de> wrote:
>
> The buffer pitch may change when switching the buffer on a
> atomic update. As the register is double buffered it can be
> safely changed while the display is active.
>
> Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
> ---
>  drivers/gpu/drm/mxsfb/lcdif_kms.c | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
>

It seems that this patch set doesn't show up in dri-devel patchwork.
Not sure why.

> diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> index 262bc43b1079..bbea44ee7a66 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> @@ -314,19 +314,6 @@ static void lcdif_set_mode(struct lcdif_drm_private *lcdif, u32 bus_flags)
>         writel(CTRLDESCL0_1_HEIGHT(m->vdisplay) |
>                CTRLDESCL0_1_WIDTH(m->hdisplay),
>                lcdif->base + LCDC_V8_CTRLDESCL0_1);
> -
> -       /*
> -        * Undocumented P_SIZE and T_SIZE register but those written in the
> -        * downstream kernel those registers control the AXI burst size. As of
> -        * now there are two known values:
> -        *  1 - 128Byte
> -        *  2 - 256Byte
> -        * Downstream set it to 256B burst size to improve the memory
> -        * efficiency so set it here too.
> -        */
> -       ctrl = CTRLDESCL0_3_P_SIZE(2) | CTRLDESCL0_3_T_SIZE(2) |
> -              CTRLDESCL0_3_PITCH(lcdif->crtc.primary->state->fb->pitches[0]);
> -       writel(ctrl, lcdif->base + LCDC_V8_CTRLDESCL0_3);
>  }
>
>  static void lcdif_enable_controller(struct lcdif_drm_private *lcdif)
> @@ -502,6 +489,10 @@ static void lcdif_crtc_atomic_enable(struct drm_crtc *crtc,
>                 writel(CTRLDESCL_HIGH0_4_ADDR_HIGH(upper_32_bits(paddr)),
>                        lcdif->base + LCDC_V8_CTRLDESCL_HIGH0_4);
>         }
> +       writel(CTRLDESCL0_3_P_SIZE(2) | CTRLDESCL0_3_T_SIZE(2) |
> +              CTRLDESCL0_3_PITCH(new_pstate->fb->pitches[0]),
> +              lcdif->base + LCDC_V8_CTRLDESCL0_3);

The patch subject says pitch setup is moved to plane atomic update,
but it is still set in crtc atomic enable.  Care to improve the subject a bit?

With this patch, plane atomic update does the same to set FB address
and pitch.  It looks fine to call lcdif_plane_primary_atomic_update()
here to save some lines, doesn't it?

> +
>         lcdif_enable_controller(lcdif);
>
>         drm_crtc_vblank_on(crtc);
> @@ -611,6 +602,19 @@ static void lcdif_plane_primary_atomic_update(struct drm_plane *plane,
>                 writel(CTRLDESCL_HIGH0_4_ADDR_HIGH(upper_32_bits(paddr)),
>                        lcdif->base + LCDC_V8_CTRLDESCL_HIGH0_4);
>         }
> +
> +       /*
> +        * Undocumented P_SIZE and T_SIZE bitfields written in the downstream
> +        * driver. Those bitfields control the AXI burst size. As of now there
> +        * are two known values:

The comment is updated and looks better than the one removed from
lcdif_set_mode().  I'm ok with doing that in this patch, but if it's done in
a separate patch, fine too.

Regards,
Liu Ying

> +        *  1 - 128Byte
> +        *  2 - 256Byte
> +        * Downstream set it to 256B burst size to improve the memory
> +        * efficiency so set it here too.
> +        */
> +       writel(CTRLDESCL0_3_P_SIZE(2) | CTRLDESCL0_3_T_SIZE(2) |
> +              CTRLDESCL0_3_PITCH(new_pstate->fb->pitches[0]),
> +              lcdif->base + LCDC_V8_CTRLDESCL0_3);
>  }
>
>  static bool lcdif_format_mod_supported(struct drm_plane *plane,
> --
> 2.39.2
>