drm/vc4: kms: Fix VBLANK reporting on a disabled CRTC
[platform/kernel/linux-rpi.git] / drivers / gpu / drm / vc4 / vc4_kms.c
index 77745ad..a552221 100644 (file)
@@ -157,23 +157,79 @@ static void vc4_hvs_pv_muxing_commit(struct vc4_dev *vc4,
 {
        struct drm_crtc_state *crtc_state;
        struct drm_crtc *crtc;
+       unsigned int i;
+
+       for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+               struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state);
+               u32 dispctrl;
+               u32 dsp3_mux;
+
+               if (!crtc_state->active)
+                       continue;
+
+               if (vc4_state->assigned_channel != 2)
+                       continue;
+
+               /*
+                * SCALER_DISPCTRL_DSP3 = X, where X < 2 means 'connect DSP3 to
+                * FIFO X'.
+                * SCALER_DISPCTRL_DSP3 = 3 means 'disable DSP 3'.
+                *
+                * DSP3 is connected to FIFO2 unless the transposer is
+                * enabled. In this case, FIFO 2 is directly accessed by the
+                * TXP IP, and we need to disable the FIFO2 -> pixelvalve1
+                * route.
+                */
+               if (vc4_state->feed_txp)
+                       dsp3_mux = VC4_SET_FIELD(3, SCALER_DISPCTRL_DSP3_MUX);
+               else
+                       dsp3_mux = VC4_SET_FIELD(2, SCALER_DISPCTRL_DSP3_MUX);
+
+               dispctrl = HVS_READ(SCALER_DISPCTRL) &
+                          ~SCALER_DISPCTRL_DSP3_MUX_MASK;
+               HVS_WRITE(SCALER_DISPCTRL, dispctrl | dsp3_mux);
+       }
+}
+
+static struct drm_crtc_state *
+drm_atomic_get_new_or_current_crtc_state(struct drm_atomic_state *state,
+                                        struct drm_crtc *crtc)
+{
+       struct drm_crtc_state *crtc_state;
+
+       crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+       if (crtc_state)
+               return crtc_state;
+
+       return crtc->state;
+}
+
+#define for_each_new_or_current_crtc_state(__state, crtc, crtc_state)  \
+       list_for_each_entry(crtc, &__state->dev->mode_config.crtc_list, head) \
+               for_each_if(crtc_state = drm_atomic_get_new_or_current_crtc_state(__state, crtc))
+
+static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4,
+                                    struct drm_atomic_state *state)
+{
+       struct drm_crtc_state *crtc_state;
+       struct drm_crtc *crtc;
        unsigned char dsp2_mux = 0;
        unsigned char dsp3_mux = 3;
        unsigned char dsp4_mux = 3;
        unsigned char dsp5_mux = 3;
-       unsigned int i;
        u32 reg;
 
-       for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
-               struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state);
+       for_each_new_or_current_crtc_state(state, crtc, crtc_state) {
                struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
+               struct vc4_crtc_state *vc4_state;
 
                if (!crtc_state->active)
                        continue;
 
+               vc4_state = to_vc4_crtc_state(crtc_state);
                switch (vc4_crtc->data->hvs_output) {
                case 2:
-                       dsp2_mux = (vc4_state->assigned_channel == 2) ? 1 : 0;
+                       dsp2_mux = (vc4_state->assigned_channel == 2) ? 0 : 1;
                        break;
 
                case 3:
@@ -194,30 +250,27 @@ static void vc4_hvs_pv_muxing_commit(struct vc4_dev *vc4,
        }
 
        reg = HVS_READ(SCALER_DISPECTRL);
-       if (FIELD_GET(SCALER_DISPECTRL_DSP2_MUX_MASK, reg) != dsp2_mux)
-               HVS_WRITE(SCALER_DISPECTRL,
-                         (reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) |
-                         VC4_SET_FIELD(dsp2_mux, SCALER_DISPECTRL_DSP2_MUX));
+       HVS_WRITE(SCALER_DISPECTRL,
+                 (reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) |
+                 VC4_SET_FIELD(dsp2_mux, SCALER_DISPECTRL_DSP2_MUX));
 
        reg = HVS_READ(SCALER_DISPCTRL);
-       if (FIELD_GET(SCALER_DISPCTRL_DSP3_MUX_MASK, reg) != dsp3_mux)
-               HVS_WRITE(SCALER_DISPCTRL,
-                         (reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) |
-                         VC4_SET_FIELD(dsp3_mux, SCALER_DISPCTRL_DSP3_MUX));
+       HVS_WRITE(SCALER_DISPCTRL,
+                 (reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) |
+                 VC4_SET_FIELD(dsp3_mux, SCALER_DISPCTRL_DSP3_MUX));
 
        reg = HVS_READ(SCALER_DISPEOLN);
-       if (FIELD_GET(SCALER_DISPEOLN_DSP4_MUX_MASK, reg) != dsp4_mux)
-               HVS_WRITE(SCALER_DISPEOLN,
-                         (reg & ~SCALER_DISPEOLN_DSP4_MUX_MASK) |
-                         VC4_SET_FIELD(dsp4_mux, SCALER_DISPEOLN_DSP4_MUX));
+       HVS_WRITE(SCALER_DISPEOLN,
+                 (reg & ~SCALER_DISPEOLN_DSP4_MUX_MASK) |
+                 VC4_SET_FIELD(dsp4_mux, SCALER_DISPEOLN_DSP4_MUX));
 
        reg = HVS_READ(SCALER_DISPDITHER);
-       if (FIELD_GET(SCALER_DISPDITHER_DSP5_MUX_MASK, reg) != dsp5_mux)
-               HVS_WRITE(SCALER_DISPDITHER,
-                         (reg & ~SCALER_DISPDITHER_DSP5_MUX_MASK) |
-                         VC4_SET_FIELD(dsp5_mux, SCALER_DISPDITHER_DSP5_MUX));
+       HVS_WRITE(SCALER_DISPDITHER,
+                 (reg & ~SCALER_DISPDITHER_DSP5_MUX_MASK) |
+                 VC4_SET_FIELD(dsp5_mux, SCALER_DISPDITHER_DSP5_MUX));
 }
 
+
 static void
 vc4_atomic_complete_commit(struct drm_atomic_state *state)
 {
@@ -248,10 +301,12 @@ vc4_atomic_complete_commit(struct drm_atomic_state *state)
 
        drm_atomic_helper_commit_modeset_disables(dev, state);
 
-       if (!vc4->firmware_kms) {
-               vc4_ctm_commit(vc4, state);
+       vc4_ctm_commit(vc4, state);
+
+       if (vc4->hvs->hvs5)
+               vc5_hvs_pv_muxing_commit(vc4, state);
+       else
                vc4_hvs_pv_muxing_commit(vc4, state);
-       }
 
        drm_atomic_helper_commit_planes(dev, state, 0);
 
@@ -580,14 +635,44 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = {
 #define NUM_OUTPUTS  6
 #define NUM_CHANNELS 3
 
-static int
-vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
+/*
+ * The BCM2711 HVS has up to 7 output connected to the pixelvalves and
+ * the TXP (and therefore all the CRTCs found on that platform).
+ *
+ * The naive (and our initial) implementation would just iterate over
+ * all the active CRTCs, try to find a suitable FIFO, and then remove it
+ * from the available FIFOs pool. However, there's a few corner cases
+ * that need to be considered:
+ *
+ * - When running in a dual-display setup (so with two CRTCs involved),
+ *   we can update the state of a single CRTC (for example by changing
+ *   its mode using xrandr under X11) without affecting the other. In
+ *   this case, the other CRTC wouldn't be in the state at all, so we
+ *   need to consider all the running CRTCs in the DRM device to assign
+ *   a FIFO, not just the one in the state.
+ *
+ * - To fix the above, we can't use drm_atomic_get_crtc_state on all
+ *   enabled CRTCs to pull their CRTC state into the global state, since
+ *   a page flip would start considering their vblank to complete. Since
+ *   we don't have a guarantee that they are actually active, that
+ *   vblank might never happen, and shouldn't even be considered if we
+ *   want to do a page flip on a single CRTC. That can be tested by
+ *   doing a modetest -v first on HDMI1 and then on HDMI0.
+ *
+ * - Since we need the pixelvalve to be disabled and enabled back when
+ *   the FIFO is changed, we should keep the FIFO assigned for as long
+ *   as the CRTC is enabled, only considering it free again once that
+ *   CRTC has been disabled. This can be tested by booting X11 on a
+ *   single display, and changing the resolution down and then back up.
+ */
+static int vc4_pv_muxing_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 *old_crtc_state, *new_crtc_state;
+       struct drm_crtc_state *crtc_state;
        struct drm_crtc *crtc;
-       int i, ret;
+       unsigned int i;
 
        /*
         * Since the HVS FIFOs are shared across all the pixelvalves and
@@ -597,14 +682,13 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
         * the same CRTCs, instead of evaluating only the CRTC being
         * modified.
         */
-       list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-               struct drm_crtc_state *crtc_state;
-               if (!crtc->state->enable)
+       for_each_new_or_current_crtc_state(state, crtc, crtc_state) {
+               struct vc4_crtc_state *vc4_crtc_state;
+               if (!crtc_state->enable)
                        continue;
 
-               crtc_state = drm_atomic_get_crtc_state(state, crtc);
-               if (IS_ERR(crtc_state))
-                       return PTR_ERR(crtc_state);
+               vc4_crtc_state = to_vc4_crtc_state(crtc_state);
+               unassigned_channels &= ~BIT(vc4_crtc_state->assigned_channel);
        }
 
        for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
@@ -620,10 +704,8 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
                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);
+               if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED)
                        continue;
-               }
 
                /*
                 * The problem we have to solve here is that we have
@@ -665,6 +747,18 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
                        return -EINVAL;
        }
 
+       return 0;
+}
+
+static int
+vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
+{
+       int ret;
+
+       ret = vc4_pv_muxing_atomic_check(dev, state);
+       if (ret)
+               return ret;
+
        ret = vc4_ctm_atomic_check(dev, state);
        if (ret < 0)
                return ret;