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>
Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200923084032.218619-2-maxime@cerno.tech

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 7ef20adedee5214b62fb09de226f6ebed43865ff..482219fb4db2146fe042700fdddbc3e17c03088a 100644 (file)
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -863,6 +863,7 @@ void vc4_crtc_reset(struct drm_crtc *crtc)
                return;
        }
 
+       vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
        __drm_atomic_helper_crtc_reset(crtc, &vc4_crtc_state->base);
 }
 

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 8c8d96b6289f40cd89e73b0516258ebebad4c482..90b911fd2a7f3b43191fdba401ea03ecc19809c1 100644 (file)
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -532,6 +532,8 @@ struct vc4_crtc_state {
        } 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 01fa60844695f9ddd94d57f3e7e2bd3deb6f8a00..149825ff5df8d41f2a815c735cb087b41aea536f 100644 (file)
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -616,7 +616,7 @@ static int
 vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 {
        unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0);
-       struct drm_crtc_state *crtc_state;
+       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
        struct drm_crtc *crtc;
        int i, ret;
 
@@ -629,6 +629,8 @@ 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;
 
@@ -637,14 +639,22 @@ 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);
                unsigned int matching_channels;
 
-               if (!crtc_state->enable)
+               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
@@ -674,7 +684,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
                if (matching_channels) {
                        unsigned int channel = ffs(matching_channels) - 1;
 
-                       vc4_crtc_state->assigned_channel = channel;
+                       new_vc4_crtc_state->assigned_channel = channel;
                        unassigned_channels &= ~BIT(channel);
                } else {
                        return -EINVAL;