Skip to content

Commit ec663bc

Browse files
vsyrjalatursulin
authored andcommitted
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: 20f505f ("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 6b72859) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
1 parent afc189d commit ec663bc

1 file changed

Lines changed: 16 additions & 2 deletions

File tree

drivers/gpu/drm/i915/display/intel_bw.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,7 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
825825
unsigned int max_bw_point = 0, max_bw = 0;
826826
unsigned int num_qgv_points = dev_priv->max_bw[0].num_qgv_points;
827827
unsigned int num_psf_gv_points = dev_priv->max_bw[0].num_psf_gv_points;
828+
bool changed = false;
828829
u32 mask = 0;
829830

830831
/* FIXME earlier gens need some checks too */
@@ -868,14 +869,28 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
868869
new_bw_state->data_rate[crtc->pipe] = new_data_rate;
869870
new_bw_state->num_active_planes[crtc->pipe] = new_active_planes;
870871

872+
changed = true;
873+
871874
drm_dbg_kms(&dev_priv->drm,
872875
"pipe %c data rate %u num active planes %u\n",
873876
pipe_name(crtc->pipe),
874877
new_bw_state->data_rate[crtc->pipe],
875878
new_bw_state->num_active_planes[crtc->pipe]);
876879
}
877880

878-
if (!new_bw_state)
881+
old_bw_state = intel_atomic_get_old_bw_state(state);
882+
new_bw_state = intel_atomic_get_new_bw_state(state);
883+
884+
if (new_bw_state &&
885+
intel_can_enable_sagv(dev_priv, old_bw_state) !=
886+
intel_can_enable_sagv(dev_priv, new_bw_state))
887+
changed = true;
888+
889+
/*
890+
* If none of our inputs (data rates, number of active
891+
* planes, SAGV yes/no) changed then nothing to do here.
892+
*/
893+
if (!changed)
879894
return 0;
880895

881896
ret = intel_atomic_lock_global_state(&new_bw_state->base);
@@ -961,7 +976,6 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
961976
*/
962977
new_bw_state->qgv_points_mask = ~allowed_points & mask;
963978

964-
old_bw_state = intel_atomic_get_old_bw_state(state);
965979
/*
966980
* If the actual mask had changed we need to make sure that
967981
* the commits are serialized(in case this is a nomodeset, nonblocking)

0 commit comments

Comments
 (0)