drm: rcar-du: Setup planes before enabling CRTC to avoid flicker

Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
start to CRTC resume") changed the order of the plane commit and CRTC
enable operations to accommodate the runtime PM requirements. However,
this introduced corruption in the first displayed frame, as the CRTC is
now enabled without any plane configured. On Gen2 hardware the first
frame will be black and likely unnoticed, but on Gen3 hardware we end up
starting the display before the VSP compositor, which is more
noticeable.

To fix this, revert the order of the commit operations back, and handle
runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
helper operation handlers.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index d82aa67162c61d171cf2dca698e1ce761f51f050..98cf446391dc06314313590722d56be9e8dcc760 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -452,14 +452,8 @@ static void rcar_du_crtc_wait_page_flip(struct rcar_du_crtc *rcrtc)
  * Start/Stop and Suspend/Resume
  */
 
-static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
+static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc)
 {
-       struct drm_crtc *crtc = &rcrtc->crtc;
-       bool interlaced;
-
-       if (rcrtc->started)
-               return;
-
        /* Set display off and background to black */
        rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
        rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0));
@@ -471,6 +465,18 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
        /* Start with all planes disabled. */
        rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
 
+       /* Enable the VSP compositor. */
+       if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
+               rcar_du_vsp_enable(rcrtc);
+
+       /* Turn vertical blanking interrupt reporting on. */
+       drm_crtc_vblank_on(&rcrtc->crtc);
+}
+
+static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
+{
+       bool interlaced;
+
        /*
         * Select master sync mode. This enables display operation in master
         * sync mode (with the HSYNC and VSYNC signals configured as outputs and
@@ -482,24 +488,12 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
                             DSYSR_TVM_MASTER);
 
        rcar_du_group_start_stop(rcrtc->group, true);
-
-       /* Enable the VSP compositor. */
-       if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
-               rcar_du_vsp_enable(rcrtc);
-
-       /* Turn vertical blanking interrupt reporting back on. */
-       drm_crtc_vblank_on(crtc);
-
-       rcrtc->started = true;
 }
 
 static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
 {
        struct drm_crtc *crtc = &rcrtc->crtc;
 
-       if (!rcrtc->started)
-               return;
-
        /*
         * Disable all planes and wait for the change to take effect. This is
         * required as the DSnPR registers are updated on vblank, and no vblank
@@ -533,8 +527,6 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
        rcar_du_crtc_clr_set(rcrtc, DSYSR, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH);
 
        rcar_du_group_start_stop(rcrtc->group, false);
-
-       rcrtc->started = false;
 }
 
 void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc)
@@ -554,12 +546,10 @@ void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc)
                return;
 
        rcar_du_crtc_get(rcrtc);
-       rcar_du_crtc_start(rcrtc);
+       rcar_du_crtc_setup(rcrtc);
 
        /* Commit the planes state. */
-       if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) {
-               rcar_du_vsp_enable(rcrtc);
-       } else {
+       if (!rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) {
                for (i = 0; i < rcrtc->group->num_planes; ++i) {
                        struct rcar_du_plane *plane = &rcrtc->group->planes[i];
 
@@ -571,6 +561,7 @@ void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc)
        }
 
        rcar_du_crtc_update_planes(rcrtc);
+       rcar_du_crtc_start(rcrtc);
 }
 
 /* -----------------------------------------------------------------------------
@@ -582,7 +573,16 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 {
        struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 
-       rcar_du_crtc_get(rcrtc);
+       /*
+        * If the CRTC has already been setup by the .atomic_begin() handler we
+        * can skip the setup stage.
+        */
+       if (!rcrtc->initialized) {
+               rcar_du_crtc_get(rcrtc);
+               rcar_du_crtc_setup(rcrtc);
+               rcrtc->initialized = true;
+       }
+
        rcar_du_crtc_start(rcrtc);
 }
 
@@ -601,6 +601,7 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
        }
        spin_unlock_irq(&crtc->dev->event_lock);
 
+       rcrtc->initialized = false;
        rcrtc->outputs = 0;
 }
 
@@ -609,6 +610,19 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
 {
        struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 
+       WARN_ON(!crtc->state->enable);
+
+       /*
+        * If a mode set is in progress we can be called with the CRTC disabled.
+        * We then need to first setup the CRTC in order to configure planes.
+        * The .atomic_enable() handler will notice and skip the CRTC setup.
+        */
+       if (!rcrtc->initialized) {
+               rcar_du_crtc_get(rcrtc);
+               rcar_du_crtc_setup(rcrtc);
+               rcrtc->initialized = true;
+       }
+
        if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
                rcar_du_vsp_atomic_begin(rcrtc);
 }

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 0b6d26ecfc3831a146fe656902da8633b226322e..3cc37682659218b3bb6de8db1583150d77faff84 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -30,7 +30,7 @@ struct rcar_du_vsp;
  * @extclock: external pixel dot clock (optional)
  * @mmio_offset: offset of the CRTC registers in the DU MMIO block
  * @index: CRTC software and hardware index
- * @started: whether the CRTC has been started and is running
+ * @initialized: whether the CRTC has been initialized and clocks enabled
  * @event: event to post when the pending page flip completes
  * @flip_wait: wait queue used to signal page flip completion
  * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
@@ -45,7 +45,7 @@ struct rcar_du_crtc {
        struct clk *extclock;
        unsigned int mmio_offset;
        unsigned int index;
-       bool started;
+       bool initialized;
 
        struct drm_pending_vblank_event *event;
        wait_queue_head_t flip_wait;

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 99adf531ea72a06a00f0200888f235075dfe84e2..066b62924618334ef90b515bd6ef3d3c8d8a4328 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -257,9 +257,9 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state)
 
        /* Apply the atomic update. */
        drm_atomic_helper_commit_modeset_disables(dev, old_state);
-       drm_atomic_helper_commit_modeset_enables(dev, old_state);
        drm_atomic_helper_commit_planes(dev, old_state,
                                        DRM_PLANE_COMMIT_ACTIVE_ONLY);
+       drm_atomic_helper_commit_modeset_enables(dev, old_state);
 
        drm_atomic_helper_commit_hw_done(old_state);
        drm_atomic_helper_wait_for_vblanks(dev, old_state);