Adding lcdif nodes to a power domain currently does work, it results in black/corrupted screens or hangs. While the driver does enable runtime pm it does not deal correctly with the block being unpowered. Ensure power is on when required by adding pm_runtime_get/put_sync to mxsfb_pipe_enable/disable. Since power is lost on suspend implement PM_SLEEP_OPS using drm_mode_config_helper_suspend/resume. The mxsfb_plane_atomic_update function can get called before mxsfb_pipe_enable while the block is not yet powered. When this happens the write to LCDIF_NEXT_BUF is lost causing corrupt display on unblank until a refresh. Fix this by not writing gem->paddr if the block is not enabled and instead delaying the write until the next mxsfb_crtc_mode_set_nofb call. At that point also update cur_buf to avoid an initial corrupt frame after resume. Signed-off-by: Leonard Crestez <leonard.crestez@xxxxxxx> --- The purpose of this patch is to prepare for enabling power gating on DISPLAY power domains. Changes since v1: * Drop mxsfb_runtime_suspend/mxsfb_runtime_resume, calling pm_runtime_get/put in pipe enable/disable is enough. * Use drm_mode_config_helper_suspend/resume instead of attempting to track state manually. * Don't touch NEXT_BUF if atomic_update called with crtc disabled * Also update CUR_BUF on enable, this avoids initial corrupt frames. Previous discussion: https://lkml.org/lkml/2018/7/17/329 --- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 51 +++++++++++++++++++++++------- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 25 +++++++++++++++ drivers/gpu/drm/mxsfb/mxsfb_drv.h | 2 ++ 3 files changed, 67 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index 0abe77675b76..10153da77c40 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -194,15 +194,31 @@ static int mxsfb_reset_block(void __iomem *reset_addr) return ret; return clear_poll_bit(reset_addr, MODULE_CLKGATE); } +static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb) +{ + struct drm_framebuffer *fb = mxsfb->pipe.plane.state->fb; + struct drm_gem_cma_object *gem; + + if (!fb) + return 0; + + gem = drm_fb_cma_get_gem_obj(fb, 0); + if (!gem) + return 0; + + return gem->paddr; +} + static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) { struct drm_display_mode *m = &mxsfb->pipe.crtc.state->adjusted_mode; const u32 bus_flags = mxsfb->connector.display_info.bus_flags; u32 vdctrl0, vsync_pulse_len, hsync_pulse_len; + dma_addr_t paddr; int err; /* * It seems, you can't re-program the controller if it is still * running. This may lead to shifted pictures (FIFO issue?), so @@ -268,35 +284,47 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) mxsfb->base + LCDC_VDCTRL3); writel(SET_DOTCLK_H_VALID_DATA_CNT(m->hdisplay), mxsfb->base + LCDC_VDCTRL4); + /* Update cur_buf as well to avoid an initial corrupt frame */ + paddr = mxsfb_get_fb_paddr(mxsfb); + if (paddr) { + writel(paddr, mxsfb->base + mxsfb->devdata->cur_buf); + writel(paddr, mxsfb->base + mxsfb->devdata->next_buf); + } mxsfb_disable_axi_clk(mxsfb); } void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb) { + if (mxsfb->enabled) + return; + mxsfb_crtc_mode_set_nofb(mxsfb); mxsfb_enable_controller(mxsfb); + + mxsfb->enabled = true; } void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb) { + if (!mxsfb->enabled) + return; + mxsfb_disable_controller(mxsfb); + + mxsfb->enabled = false; } void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb, struct drm_plane_state *state) { struct drm_simple_display_pipe *pipe = &mxsfb->pipe; struct drm_crtc *crtc = &pipe->crtc; - struct drm_framebuffer *fb = pipe->plane.state->fb; struct drm_pending_vblank_event *event; - struct drm_gem_cma_object *gem; - - if (!crtc) - return; + dma_addr_t paddr; spin_lock_irq(&crtc->dev->event_lock); event = crtc->state->event; if (event) { crtc->state->event = NULL; @@ -307,14 +335,15 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb, drm_crtc_send_vblank_event(crtc, event); } } spin_unlock_irq(&crtc->dev->event_lock); - if (!fb) + if (!mxsfb->enabled) return; - gem = drm_fb_cma_get_gem_obj(fb, 0); - - mxsfb_enable_axi_clk(mxsfb); - writel(gem->paddr, mxsfb->base + mxsfb->devdata->next_buf); - mxsfb_disable_axi_clk(mxsfb); + paddr = mxsfb_get_fb_paddr(mxsfb); + if (paddr) { + mxsfb_enable_axi_clk(mxsfb); + writel(paddr, mxsfb->base + mxsfb->devdata->next_buf); + mxsfb_disable_axi_clk(mxsfb); + } } diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index dd1dd58e4956..a5269fccbed9 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -101,23 +101,27 @@ static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = { static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc_state *crtc_state, struct drm_plane_state *plane_state) { struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe); + struct drm_device *drm = pipe->plane.dev; + pm_runtime_get_sync(drm->dev); drm_panel_prepare(mxsfb->panel); mxsfb_crtc_enable(mxsfb); drm_panel_enable(mxsfb->panel); } static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe) { struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe); + struct drm_device *drm = pipe->plane.dev; drm_panel_disable(mxsfb->panel); mxsfb_crtc_disable(mxsfb); drm_panel_unprepare(mxsfb->panel); + pm_runtime_put_sync(drm->dev); } static void mxsfb_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_plane_state *plane_state) { @@ -412,17 +416,38 @@ static int mxsfb_remove(struct platform_device *pdev) drm_dev_unref(drm); return 0; } +#ifdef CONFIG_PM +static int mxsfb_suspend(struct device *dev) +{ + struct drm_device *drm = dev_get_drvdata(dev); + + return drm_mode_config_helper_suspend(drm); +} + +static int mxsfb_resume(struct device *dev) +{ + struct drm_device *drm = dev_get_drvdata(dev); + + return drm_mode_config_helper_resume(drm); +} +#endif + +static const struct dev_pm_ops mxsfb_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(mxsfb_suspend, mxsfb_resume) +}; + static struct platform_driver mxsfb_platform_driver = { .probe = mxsfb_probe, .remove = mxsfb_remove, .id_table = mxsfb_devtype, .driver = { .name = "mxsfb-drm", .of_match_table = mxsfb_dt_ids, + .pm = &mxsfb_pm_ops, }, }; module_platform_driver(mxsfb_platform_driver); diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h index 5d0883fc805b..e539d4b05c48 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h @@ -36,10 +36,12 @@ struct mxsfb_drm_private { struct drm_simple_display_pipe pipe; struct drm_connector connector; struct drm_panel *panel; struct drm_fbdev_cma *fbdev; + + bool enabled; }; int mxsfb_setup_crtc(struct drm_device *dev); int mxsfb_create_output(struct drm_device *dev); -- 2.17.1 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel