drm/vc4: crtc: Keep the previously assigned HVS FIFO

The HVS FIFOs are currently assigned each time we have an atomic_check
for all the enabled CRTCs.

However, if we are running multiple outputs in parallel and we happen to
disable the first (by index) CRTC, we end up changing the assigned FIFO
of the second CRTC without disabling and reenabling the pixelvalve which
ends up in a stall and eventually a VBLANK timeout.

In order to fix this, we can create a special value for our assigned
channel to mark it as disabled, and if our CRTC already had an assigned
channel in its previous state, we keep on using it.

Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
[hoegeun.kwon: Needed to fix page flip issue of dual hdmi.]
Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
Change-Id: I70e0d6b563d5c7f22a1a7e1527a401ac42395b78

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f8fa076..2eabf02 100644 (file)
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1088,12 +1088,18 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
 static void
 vc4_crtc_reset(struct drm_crtc *crtc)
 {
+       struct vc4_crtc_state *vc4_crtc_state;
+
        if (crtc->state)
                vc4_crtc_destroy_state(crtc, crtc->state);
 
-       crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
-       if (crtc->state)
-               crtc->state->crtc = crtc;
+       vc4_crtc_state = kzalloc(sizeof(*vc4_crtc_state), GFP_KERNEL);
+       if (!vc4_crtc_state)
+               return;
+
+       vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
+       crtc->state = &vc4_crtc_state->base;
+       crtc->state->crtc = crtc;
 }
 
 static const struct drm_crtc_funcs vc4_crtc_funcs = {

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 9273595..457fd68 100644 (file)
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -511,6 +511,7 @@ struct vc4_crtc_state {
                unsigned int bottom;
        } margins;
 };
+#define VC4_HVS_CHANNEL_DISABLED ((unsigned int) -1)
 
 static inline struct vc4_crtc_state *
 to_vc4_crtc_state(struct drm_crtc_state *crtc_state)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 96aadcb..77745ad 100644 (file)
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -585,7 +585,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 {
        unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0);
        struct vc4_dev *vc4 = to_vc4_dev(state->dev);
-       struct drm_crtc_state *crtc_state;
+       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
        struct drm_crtc *crtc;
        int i, ret;
 
@@ -598,6 +598,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
         * modified.
         */
        list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+               struct drm_crtc_state *crtc_state;
                if (!crtc->state->enable)
                        continue;
 
@@ -606,15 +607,23 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
                        return PTR_ERR(crtc_state);
        }
 
-       for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
-               struct vc4_crtc_state *vc4_crtc_state =
-                       to_vc4_crtc_state(crtc_state);
+       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+               struct vc4_crtc_state *new_vc4_crtc_state =
+                       to_vc4_crtc_state(new_crtc_state);
                struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
                bool is_assigned = false;
                unsigned int channel;
 
-               if (!crtc_state->enable || vc4->firmware_kms)
+               if (old_crtc_state->enable && !new_crtc_state->enable)
+                       new_vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
+
+               if (!new_crtc_state->enable)
+                       continue;
+
+               if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) {
+                       unassigned_channels &= ~BIT(new_vc4_crtc_state->assigned_channel);
                        continue;
+               }
 
                /*
                 * The problem we have to solve here is that we have
@@ -646,7 +655,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
                        if (!(BIT(channel) & vc4_crtc->data->hvs_available_channels))
                                continue;
 
-                       vc4_crtc_state->assigned_channel = channel;
+                       new_vc4_crtc_state->assigned_channel = channel;
                        unassigned_channels &= ~BIT(channel);
                        is_assigned = true;
                        break;