| Message ID | 20230920103126.2759601-3-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | imx-lcdif modeset changes | expand |
On 9/20/23 12:31, Lucas Stach wrote: > Allow drm_atomic_helper_commit_tail_rpm to setup all the plane > state before the scanout is started. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/gpu/drm/mxsfb/lcdif_kms.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c > index f5bfe8b52920..4acf6914a8d1 100644 > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > @@ -505,6 +505,8 @@ static int lcdif_crtc_atomic_check(struct drm_crtc *crtc, > static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct lcdif_drm_private *lcdif = to_lcdif_drm_private(crtc->dev); > struct drm_pending_vblank_event *event; > u32 reg; > @@ -513,6 +515,9 @@ static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, > reg |= CTRLDESCL0_5_SHADOW_LOAD_EN; > writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5); > > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > + lcdif_enable_controller(lcdif); > + > event = crtc->state->event; > crtc->state->event = NULL; > > @@ -552,7 +557,6 @@ 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); > } > - lcdif_enable_controller(lcdif); > > drm_crtc_vblank_on(crtc); > } Reviewed-by: Marek Vasut <marex@denx.de>
Hi, On Wednesday, September 20, 2023 6:31 PM Lucas Stach <l.stach@pengutronix.de> wrote: > Allow drm_atomic_helper_commit_tail_rpm to setup all the plane > state before the scanout is started. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/gpu/drm/mxsfb/lcdif_kms.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c > b/drivers/gpu/drm/mxsfb/lcdif_kms.c > index f5bfe8b52920..4acf6914a8d1 100644 > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > @@ -505,6 +505,8 @@ static int lcdif_crtc_atomic_check(struct drm_crtc > *crtc, > static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = > drm_atomic_get_new_crtc_state(state, > + crtc); > struct lcdif_drm_private *lcdif = to_lcdif_drm_private(crtc->dev); > struct drm_pending_vblank_event *event; > u32 reg; > @@ -513,6 +515,9 @@ static void lcdif_crtc_atomic_flush(struct drm_crtc > *crtc, > reg |= CTRLDESCL0_5_SHADOW_LOAD_EN; > writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5); > > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > + lcdif_enable_controller(lcdif); Moving lcdif_enable_controller() function call from atomic_enable to atomic_flush would change CRTC vs bridge enablement order, which effectively makes bridge enablement happen prior to CRTC enablement, see drm_atomic_helper_commit_tail_rpm() detail implementation. The reversed order potentially causes malfunctions of the bridge. BTW, even if it's ok to move the function call, it would be better to call lcdif_enable_controller() before CTRLDESCL0_5_SHADOW_LOAD_EN is set so that the original sequence is kept. Also, LCDIF chapter in SoC RMs indicates that the shadow load enablement is the last step in "Software flow diagram". Regards, Liu Ying > + > event = crtc->state->event; > crtc->state->event = NULL; > > @@ -552,7 +557,6 @@ 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); > } > - lcdif_enable_controller(lcdif); > > drm_crtc_vblank_on(crtc); > } > -- > 2.39.2
Am Donnerstag, dem 21.09.2023 um 07:13 +0000 schrieb Ying Liu: > Hi, > > On Wednesday, September 20, 2023 6:31 PM Lucas Stach <l.stach@pengutronix.de> wrote: > > Allow drm_atomic_helper_commit_tail_rpm to setup all the plane > > state before the scanout is started. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > --- > > drivers/gpu/drm/mxsfb/lcdif_kms.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > index f5bfe8b52920..4acf6914a8d1 100644 > > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > @@ -505,6 +505,8 @@ static int lcdif_crtc_atomic_check(struct drm_crtc > > *crtc, > > static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, > > struct drm_atomic_state *state) > > { > > + struct drm_crtc_state *crtc_state = > > drm_atomic_get_new_crtc_state(state, > > + crtc); > > struct lcdif_drm_private *lcdif = to_lcdif_drm_private(crtc->dev); > > struct drm_pending_vblank_event *event; > > u32 reg; > > @@ -513,6 +515,9 @@ static void lcdif_crtc_atomic_flush(struct drm_crtc > > *crtc, > > reg |= CTRLDESCL0_5_SHADOW_LOAD_EN; > > writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5); > > > > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > > + lcdif_enable_controller(lcdif); > > Moving lcdif_enable_controller() function call from atomic_enable to > atomic_flush would change CRTC vs bridge enablement order, which > effectively makes bridge enablement happen prior to CRTC enablement, > see drm_atomic_helper_commit_tail_rpm() detail implementation. The > reversed order potentially causes malfunctions of the bridge. > This has nothing to do with the bridge after the LCDIF controller. The RPM commit_tail implementation enables the CRTC before all the plane state is set up. To avoid having to program the plane state in the CRTC enable this series defers the actual controller enable to the last step of the atomic commit. This way the plane state is programmed the usual way via the atomic update_plane callback and we don't need to duplicate this setup. > BTW, even if it's ok to move the function call, it would be better to call > lcdif_enable_controller() before CTRLDESCL0_5_SHADOW_LOAD_EN is > set so that the original sequence is kept. Also, LCDIF chapter in SoC RMs > indicates that the shadow load enablement is the last step in > "Software flow diagram". This flow chart shows how the double buffered update should work, it doesn't show the initial controller enable sequence. Without the shadow load enable bit being set before the controller enable I could observe sporadic issues on the first frame where the DMA engine would read the wrong memory address. Regards, Lucas > > Regards, > Liu Ying > > > + > > event = crtc->state->event; > > crtc->state->event = NULL; > > > > @@ -552,7 +557,6 @@ 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); > > } > > - lcdif_enable_controller(lcdif); > > > > drm_crtc_vblank_on(crtc); > > } > > -- > > 2.39.2 >
On Thursday, September 21, 2023 3:55 PM Lucas Stach <l.stach@pengutronix.de> wrote: > Am Donnerstag, dem 21.09.2023 um 07:13 +0000 schrieb Ying Liu: > > Hi, > > > > On Wednesday, September 20, 2023 6:31 PM Lucas Stach > <l.stach@pengutronix.de> wrote: > > > Allow drm_atomic_helper_commit_tail_rpm to setup all the plane > > > state before the scanout is started. > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > --- > > > drivers/gpu/drm/mxsfb/lcdif_kms.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > index f5bfe8b52920..4acf6914a8d1 100644 > > > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > @@ -505,6 +505,8 @@ static int lcdif_crtc_atomic_check(struct drm_crtc > > > *crtc, > > > static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, > > > struct drm_atomic_state *state) > > > { > > > + struct drm_crtc_state *crtc_state = > > > drm_atomic_get_new_crtc_state(state, > > > + crtc); > > > struct lcdif_drm_private *lcdif = to_lcdif_drm_private(crtc->dev); > > > struct drm_pending_vblank_event *event; > > > u32 reg; > > > @@ -513,6 +515,9 @@ static void lcdif_crtc_atomic_flush(struct drm_crtc > > > *crtc, > > > reg |= CTRLDESCL0_5_SHADOW_LOAD_EN; > > > writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5); > > > > > > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > > > + lcdif_enable_controller(lcdif); > > > > Moving lcdif_enable_controller() function call from atomic_enable to > > atomic_flush would change CRTC vs bridge enablement order, which > > effectively makes bridge enablement happen prior to CRTC enablement, > > see drm_atomic_helper_commit_tail_rpm() detail implementation. The > > reversed order potentially causes malfunctions of the bridge. > > > This has nothing to do with the bridge after the LCDIF controller. The IMHO, the reserved CRTC vs bridge enablement order is relevant for the overall display pipeline. > RPM commit_tail implementation enables the CRTC before all the plane > state is set up. To avoid having to program the plane state in the CRTC > enable this series defers the actual controller enable to the last step > of the atomic commit. This way the plane state is programmed the usual > way via the atomic update_plane callback and we don't need to duplicate > this setup. I understand the patch avoids some duplications via deferral controller enablement time point, but the reversed CRTC vs bridge enablement order is the concern here. > > > BTW, even if it's ok to move the function call, it would be better to call > > lcdif_enable_controller() before CTRLDESCL0_5_SHADOW_LOAD_EN is > > set so that the original sequence is kept. Also, LCDIF chapter in SoC RMs > > indicates that the shadow load enablement is the last step in > > "Software flow diagram". > > This flow chart shows how the double buffered update should work, it > doesn't show the initial controller enable sequence. Without the shadow You are right. The downstream driver also enables shadow load before the entire controller. Regards, Liu Ying > load enable bit being set before the controller enable I could observe > sporadic issues on the first frame where the DMA engine would read the > wrong memory address. > > Regards, > Lucas > > > > > Regards, > > Liu Ying > > > > > + > > > event = crtc->state->event; > > > crtc->state->event = NULL; > > > > > > @@ -552,7 +557,6 @@ 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); > > > } > > > - lcdif_enable_controller(lcdif); > > > > > > drm_crtc_vblank_on(crtc); > > > } > > > -- > > > 2.39.2 > >
Am Donnerstag, dem 21.09.2023 um 08:18 +0000 schrieb Ying Liu: > On Thursday, September 21, 2023 3:55 PM Lucas Stach <l.stach@pengutronix.de> wrote: > > Am Donnerstag, dem 21.09.2023 um 07:13 +0000 schrieb Ying Liu: > > > Hi, > > > > > > On Wednesday, September 20, 2023 6:31 PM Lucas Stach > > <l.stach@pengutronix.de> wrote: > > > > Allow drm_atomic_helper_commit_tail_rpm to setup all the plane > > > > state before the scanout is started. > > > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > > --- > > > > drivers/gpu/drm/mxsfb/lcdif_kms.c | 6 +++++- > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > > b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > > index f5bfe8b52920..4acf6914a8d1 100644 > > > > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > > @@ -505,6 +505,8 @@ static int lcdif_crtc_atomic_check(struct drm_crtc > > > > *crtc, > > > > static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, > > > > struct drm_atomic_state *state) > > > > { > > > > + struct drm_crtc_state *crtc_state = > > > > drm_atomic_get_new_crtc_state(state, > > > > + crtc); > > > > struct lcdif_drm_private *lcdif = to_lcdif_drm_private(crtc->dev); > > > > struct drm_pending_vblank_event *event; > > > > u32 reg; > > > > @@ -513,6 +515,9 @@ static void lcdif_crtc_atomic_flush(struct drm_crtc > > > > *crtc, > > > > reg |= CTRLDESCL0_5_SHADOW_LOAD_EN; > > > > writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5); > > > > > > > > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > > > > + lcdif_enable_controller(lcdif); > > > > > > Moving lcdif_enable_controller() function call from atomic_enable to > > > atomic_flush would change CRTC vs bridge enablement order, which > > > effectively makes bridge enablement happen prior to CRTC enablement, > > > see drm_atomic_helper_commit_tail_rpm() detail implementation. The > > > reversed order potentially causes malfunctions of the bridge. > > > > > This has nothing to do with the bridge after the LCDIF controller. The > > IMHO, the reserved CRTC vs bridge enablement order is relevant for > the overall display pipeline. > Ah, I see what you mean now. Moving the controller enable into flush is violating the documented flow that bridges might assume the display link to be running in their enable callback. I don't think any of the bridges connected to the LCDIF really care about this, but it's bad nevertheless. This seems to be a generic issue with the RPM commit tail. Enabling the display link in the CRTC atomic_enable without all the plane state being set up in the HW sounds wrong and I think we also don't want to hoist (duplicate) all the plane setup into the CRTC enable. I'll look into this some more to see if we can do something better that doesn't violate the bridge chain constraints. Regards, Lucas > > RPM commit_tail implementation enables the CRTC before all the plane > > state is set up. To avoid having to program the plane state in the CRTC > > enable this series defers the actual controller enable to the last step > > of the atomic commit. This way the plane state is programmed the usual > > way via the atomic update_plane callback and we don't need to duplicate > > this setup. > > I understand the patch avoids some duplications via deferral controller > enablement time point, but the reversed CRTC vs bridge enablement order > is the concern here. > > > > > > BTW, even if it's ok to move the function call, it would be better to call > > > lcdif_enable_controller() before CTRLDESCL0_5_SHADOW_LOAD_EN is > > > set so that the original sequence is kept. Also, LCDIF chapter in SoC RMs > > > indicates that the shadow load enablement is the last step in > > > "Software flow diagram". > > > > This flow chart shows how the double buffered update should work, it > > doesn't show the initial controller enable sequence. Without the shadow > > You are right. The downstream driver also enables shadow load before > the entire controller. > > Regards, > Liu Ying > > > load enable bit being set before the controller enable I could observe > > sporadic issues on the first frame where the DMA engine would read the > > wrong memory address. > > > > Regards, > > Lucas > > > > > > > > Regards, > > > Liu Ying > > > > > > > + > > > > event = crtc->state->event; > > > > crtc->state->event = NULL; > > > > > > > > @@ -552,7 +557,6 @@ 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); > > > > } > > > > - lcdif_enable_controller(lcdif); > > > > > > > > drm_crtc_vblank_on(crtc); > > > > } > > > > -- > > > > 2.39.2 > > > >
On Thursday, September 21, 2023 4:57 PM Lucas Stach <l.stach@pengutronix.de> wrote: > Am Donnerstag, dem 21.09.2023 um 08:18 +0000 schrieb Ying Liu: > > On Thursday, September 21, 2023 3:55 PM Lucas Stach > <l.stach@pengutronix.de> wrote: > > > Am Donnerstag, dem 21.09.2023 um 07:13 +0000 schrieb Ying Liu: > > > > Hi, > > > > > > > > On Wednesday, September 20, 2023 6:31 PM Lucas Stach > > > <l.stach@pengutronix.de> wrote: > > > > > Allow drm_atomic_helper_commit_tail_rpm to setup all the plane > > > > > state before the scanout is started. > > > > > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > > > --- > > > > > drivers/gpu/drm/mxsfb/lcdif_kms.c | 6 +++++- > > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > > > b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > > > index f5bfe8b52920..4acf6914a8d1 100644 > > > > > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > > > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > > > > @@ -505,6 +505,8 @@ static int lcdif_crtc_atomic_check(struct > drm_crtc > > > > > *crtc, > > > > > static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, > > > > > struct drm_atomic_state *state) > > > > > { > > > > > + struct drm_crtc_state *crtc_state = > > > > > drm_atomic_get_new_crtc_state(state, > > > > > + > crtc); > > > > > struct lcdif_drm_private *lcdif = to_lcdif_drm_private(crtc->dev); > > > > > struct drm_pending_vblank_event *event; > > > > > u32 reg; > > > > > @@ -513,6 +515,9 @@ static void lcdif_crtc_atomic_flush(struct > drm_crtc > > > > > *crtc, > > > > > reg |= CTRLDESCL0_5_SHADOW_LOAD_EN; > > > > > writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5); > > > > > > > > > > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > > > > > + lcdif_enable_controller(lcdif); > > > > > > > > Moving lcdif_enable_controller() function call from atomic_enable to > > > > atomic_flush would change CRTC vs bridge enablement order, which > > > > effectively makes bridge enablement happen prior to CRTC enablement, > > > > see drm_atomic_helper_commit_tail_rpm() detail implementation. The > > > > reversed order potentially causes malfunctions of the bridge. > > > > > > > This has nothing to do with the bridge after the LCDIF controller. The > > > > IMHO, the reserved CRTC vs bridge enablement order is relevant for > > the overall display pipeline. > > > Ah, I see what you mean now. Moving the controller enable into flush is > violating the documented flow that bridges might assume the display > link to be running in their enable callback. I don't think any of the > bridges connected to the LCDIF really care about this, but it's bad > nevertheless. Not sure if the first bridges(all in SoCs for now) connected to LCDIF care about the order. But, it's really about the overall display pipeline including wild out-of-SoC bridges in a chain. > > This seems to be a generic issue with the RPM commit tail. Enabling the > display link in the CRTC atomic_enable without all the plane state > being set up in the HW sounds wrong and I think we also don't want to Right, seems to be a generic issue. Regards, Liu Ying > hoist (duplicate) all the plane setup into the CRTC enable. > > I'll look into this some more to see if we can do something better that > doesn't violate the bridge chain constraints. > > Regards, > Lucas > > > > RPM commit_tail implementation enables the CRTC before all the plane > > > state is set up. To avoid having to program the plane state in the CRTC > > > enable this series defers the actual controller enable to the last step > > > of the atomic commit. This way the plane state is programmed the usual > > > way via the atomic update_plane callback and we don't need to duplicate > > > this setup. > > > > I understand the patch avoids some duplications via deferral controller > > enablement time point, but the reversed CRTC vs bridge enablement order > > is the concern here. > > > > > > > > > BTW, even if it's ok to move the function call, it would be better to call > > > > lcdif_enable_controller() before CTRLDESCL0_5_SHADOW_LOAD_EN is > > > > set so that the original sequence is kept. Also, LCDIF chapter in SoC RMs > > > > indicates that the shadow load enablement is the last step in > > > > "Software flow diagram". > > > > > > This flow chart shows how the double buffered update should work, it > > > doesn't show the initial controller enable sequence. Without the shadow > > > > You are right. The downstream driver also enables shadow load before > > the entire controller. > > > > Regards, > > Liu Ying > > > > > load enable bit being set before the controller enable I could observe > > > sporadic issues on the first frame where the DMA engine would read the > > > wrong memory address. > > > > > > Regards, > > > Lucas > > > > > > > > > > > Regards, > > > > Liu Ying > > > > > > > > > + > > > > > event = crtc->state->event; > > > > > crtc->state->event = NULL; > > > > > > > > > > @@ -552,7 +557,6 @@ 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); > > > > > } > > > > > - lcdif_enable_controller(lcdif); > > > > > > > > > > drm_crtc_vblank_on(crtc); > > > > > } > > > > > -- > > > > > 2.39.2 > > > > > >
diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c index f5bfe8b52920..4acf6914a8d1 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c @@ -505,6 +505,8 @@ static int lcdif_crtc_atomic_check(struct drm_crtc *crtc, static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_state *state) { + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, + crtc); struct lcdif_drm_private *lcdif = to_lcdif_drm_private(crtc->dev); struct drm_pending_vblank_event *event; u32 reg; @@ -513,6 +515,9 @@ static void lcdif_crtc_atomic_flush(struct drm_crtc *crtc, reg |= CTRLDESCL0_5_SHADOW_LOAD_EN; writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5); + if (drm_atomic_crtc_needs_modeset(crtc_state)) + lcdif_enable_controller(lcdif); + event = crtc->state->event; crtc->state->event = NULL; @@ -552,7 +557,6 @@ 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); } - lcdif_enable_controller(lcdif); drm_crtc_vblank_on(crtc); }
Allow drm_atomic_helper_commit_tail_rpm to setup all the plane state before the scanout is started. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/gpu/drm/mxsfb/lcdif_kms.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)