[PATCH v2] drm/mxsfb: Fix runtime PM for unpowering lcdif block
Philipp Zabel
p.zabel at pengutronix.de
Tue Jul 31 11:54:57 UTC 2018
On Tue, 2018-07-17 at 13:48 +0300, Leonard Crestez wrote:
> 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.
Why does this happen?
> 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 at nxp.com>
Tested-by: Philipp Zabel <p.zabel at pengutronix.de>
on imx6ull-evk, this fixes the initial corrupt frame.
regards
Philipp
>
> ---
>
> 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);
>
More information about the dri-devel
mailing list