drm/i915: Fix bw atomic check when switching between SAGV vs. no SAGV

If the only thing that is changing is SAGV vs. no SAGV but
the number of active planes and the total data rates end up
unchanged we currently bail out of intel_bw_atomic_check()
early and forget to actually compute the new WGV point
mask and thus won't actually enable/disable SAGV as requested.
This ends up poorly if we end up running with SAGV enabled
when we shouldn't. Usually ends up in underruns.

To fix this let's go through the QGV point mask computation
if either the data rates/number of planes, or the state
of SAGV is changing.

v2: Check more carefully if things are changing to avoid
    the extra calculations/debugs from introducing unwanted
    overhead

Cc: stable@vger.kernel.org
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> #v1
Fixes: 20f505f22531 ("drm/i915: Restrict qgv points which don't have enough bandwidth.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220218064039.12834-3-ville.syrjala@linux.intel.com
(cherry picked from commit 6b728595ffa51c087343c716bccbfc260f120e72)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 2da4aacc956bb86b824a1d07491ca0484a02291b..8ac196e814d5d314913bab9d44b043ceea1b6490 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -825,6 +825,7 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
        unsigned int max_bw_point = 0, max_bw = 0;
        unsigned int num_qgv_points = dev_priv->max_bw[0].num_qgv_points;
        unsigned int num_psf_gv_points = dev_priv->max_bw[0].num_psf_gv_points;
+       bool changed = false;
        u32 mask = 0;
 
        /* FIXME earlier gens need some checks too */
@@ -868,6 +869,8 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
                new_bw_state->data_rate[crtc->pipe] = new_data_rate;
                new_bw_state->num_active_planes[crtc->pipe] = new_active_planes;
 
+               changed = true;
+
                drm_dbg_kms(&dev_priv->drm,
                            "pipe %c data rate %u num active planes %u\n",
                            pipe_name(crtc->pipe),
@@ -875,7 +878,19 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
                            new_bw_state->num_active_planes[crtc->pipe]);
        }
 
-       if (!new_bw_state)
+       old_bw_state = intel_atomic_get_old_bw_state(state);
+       new_bw_state = intel_atomic_get_new_bw_state(state);
+
+       if (new_bw_state &&
+           intel_can_enable_sagv(dev_priv, old_bw_state) !=
+           intel_can_enable_sagv(dev_priv, new_bw_state))
+               changed = true;
+
+       /*
+        * If none of our inputs (data rates, number of active
+        * planes, SAGV yes/no) changed then nothing to do here.
+        */
+       if (!changed)
                return 0;
 
        ret = intel_atomic_lock_global_state(&new_bw_state->base);
@@ -961,7 +976,6 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
         */
        new_bw_state->qgv_points_mask = ~allowed_points & mask;
 
-       old_bw_state = intel_atomic_get_old_bw_state(state);
        /*
         * If the actual mask had changed we need to make sure that
         * the commits are serialized(in case this is a nomodeset, nonblocking)