| Message ID | f0d719af219a7f922bcc0746e9f840c471c56c5d.1533743954.git.leonard.crestez@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On 08.08.2018 18:08, Leonard Crestez wrote: > The main axi clk is disabled at the end of mxsfb_crtc_mode_set_nofb and > immediately reenabled in mxsfb_enable_controller. > > Avoid this by moving the handling of axi clk one level up to > mxsfb_crtc_enable. Do the same for mxsfb_crtc_disable for simmetry > > This shouldn't have any functional effect. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> Thanks, looks good! Reviewed-by: Stefan Agner <stefan@agner.ch> > --- > drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c > b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c > index 0abe77675b76..e4fcbb65b969 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c > @@ -127,11 +127,10 @@ static void mxsfb_enable_controller(struct > mxsfb_drm_private *mxsfb) > u32 reg; > > if (mxsfb->clk_disp_axi) > clk_prepare_enable(mxsfb->clk_disp_axi); > clk_prepare_enable(mxsfb->clk); > - mxsfb_enable_axi_clk(mxsfb); > > /* If it was disabled, re-enable the mode again */ > writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET); > > /* Enable the SYNC signals first, then the DMA engine */ > @@ -157,12 +156,10 @@ static void mxsfb_disable_controller(struct > mxsfb_drm_private *mxsfb) > > reg = readl(mxsfb->base + LCDC_VDCTRL4); > reg &= ~VDCTRL4_SYNC_SIGNALS_ON; > writel(reg, mxsfb->base + LCDC_VDCTRL4); > > - mxsfb_disable_axi_clk(mxsfb); > - > clk_disable_unprepare(mxsfb->clk); > if (mxsfb->clk_disp_axi) > clk_disable_unprepare(mxsfb->clk_disp_axi); > } > > @@ -206,11 +203,10 @@ static void mxsfb_crtc_mode_set_nofb(struct > mxsfb_drm_private *mxsfb) > /* > * It seems, you can't re-program the controller if it is still > * running. This may lead to shifted pictures (FIFO issue?), so > * first stop the controller and drain its FIFOs. > */ > - mxsfb_enable_axi_clk(mxsfb); > > /* Mandatory eLCDIF reset as per the Reference Manual */ > err = mxsfb_reset_block(mxsfb->base); > if (err) > return; > @@ -267,23 +263,23 @@ static void mxsfb_crtc_mode_set_nofb(struct > mxsfb_drm_private *mxsfb) > SET_VERT_WAIT_CNT(m->crtc_vtotal - m->crtc_vsync_start), > mxsfb->base + LCDC_VDCTRL3); > > writel(SET_DOTCLK_H_VALID_DATA_CNT(m->hdisplay), > mxsfb->base + LCDC_VDCTRL4); > - > - mxsfb_disable_axi_clk(mxsfb); > } > > void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb) > { > + mxsfb_enable_axi_clk(mxsfb); > mxsfb_crtc_mode_set_nofb(mxsfb); > mxsfb_enable_controller(mxsfb); > } > > void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb) > { > mxsfb_disable_controller(mxsfb); > + mxsfb_disable_axi_clk(mxsfb); > } > > void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb, > struct drm_plane_state *state) > {
On Wed, 2018-08-08 at 20:57 +0200, Stefan Agner wrote: > On 08.08.2018 18:08, Leonard Crestez wrote: > > The main axi clk is disabled at the end of mxsfb_crtc_mode_set_nofb and > > immediately reenabled in mxsfb_enable_controller. > > > > Avoid this by moving the handling of axi clk one level up to > > mxsfb_crtc_enable. Do the same for mxsfb_crtc_disable for simmetry > > > > This shouldn't have any functional effect. > > > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > > Thanks, looks good! > > Reviewed-by: Stefan Agner <stefan@agner.ch> So what is next now that this entire series was reviewed? My guess is that somebody needs to push it to drm-misc-next. However 4.18 was just released and I'm not sure how this interacts with drm. As far as I can tell drm-misc-next is always open and a patch accepted now will make it into 4.20, based on this graph: https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html#merge-timeline -- Regards, Leonard
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index 0abe77675b76..e4fcbb65b969 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -127,11 +127,10 @@ static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb) u32 reg; if (mxsfb->clk_disp_axi) clk_prepare_enable(mxsfb->clk_disp_axi); clk_prepare_enable(mxsfb->clk); - mxsfb_enable_axi_clk(mxsfb); /* If it was disabled, re-enable the mode again */ writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET); /* Enable the SYNC signals first, then the DMA engine */ @@ -157,12 +156,10 @@ static void mxsfb_disable_controller(struct mxsfb_drm_private *mxsfb) reg = readl(mxsfb->base + LCDC_VDCTRL4); reg &= ~VDCTRL4_SYNC_SIGNALS_ON; writel(reg, mxsfb->base + LCDC_VDCTRL4); - mxsfb_disable_axi_clk(mxsfb); - clk_disable_unprepare(mxsfb->clk); if (mxsfb->clk_disp_axi) clk_disable_unprepare(mxsfb->clk_disp_axi); } @@ -206,11 +203,10 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) /* * It seems, you can't re-program the controller if it is still * running. This may lead to shifted pictures (FIFO issue?), so * first stop the controller and drain its FIFOs. */ - mxsfb_enable_axi_clk(mxsfb); /* Mandatory eLCDIF reset as per the Reference Manual */ err = mxsfb_reset_block(mxsfb->base); if (err) return; @@ -267,23 +263,23 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) SET_VERT_WAIT_CNT(m->crtc_vtotal - m->crtc_vsync_start), mxsfb->base + LCDC_VDCTRL3); writel(SET_DOTCLK_H_VALID_DATA_CNT(m->hdisplay), mxsfb->base + LCDC_VDCTRL4); - - mxsfb_disable_axi_clk(mxsfb); } void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb) { + mxsfb_enable_axi_clk(mxsfb); mxsfb_crtc_mode_set_nofb(mxsfb); mxsfb_enable_controller(mxsfb); } void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb) { mxsfb_disable_controller(mxsfb); + mxsfb_disable_axi_clk(mxsfb); } void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb, struct drm_plane_state *state) {
The main axi clk is disabled at the end of mxsfb_crtc_mode_set_nofb and immediately reenabled in mxsfb_enable_controller. Avoid this by moving the handling of axi clk one level up to mxsfb_crtc_enable. Do the same for mxsfb_crtc_disable for simmetry This shouldn't have any functional effect. Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)