Hi Lucas, On Thu, Jun 1, 2023 at 2:45 AM Lucas Stach <l.stach@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> > --- > 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 >
