From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Say we have two planes enabled with watermarks configured
as follows:
plane A: wm0=enabled/can_sagv=false, wm1=enabled/can_sagv=true
plane B: wm0=enabled/can_sagv=true, wm1=disabled
This is possible since the latency we use to calculate
can_sagv may not be the same for both planes due to
skl_needs_memory_bw_wa().
In this case skl_crtc_can_enable_sagv() will see that
both planes have enabled at least one watermark level
with can_sagv==true, and thus proceeds to allow SAGV.
However, since plane B does not have wm1 enabled
plane A can't actually use it either. Thus we are
now running with SAGV enabled, but plane A can't
actually tolerate the extra latency it imposes.
To remedy this only allow SAGV on if the highest common
enabled watermark level for all active planes can tolerate
the extra SAGV latency.
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 854ffecd98d9..b6e34d1701a0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3876,6 +3876,7 @@ static bool skl_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state)
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
enum plane_id plane_id;
+ int max_level = INT_MAX;
if (!intel_has_sagv(dev_priv))
return false;
@@ -3900,12 +3901,23 @@ static bool skl_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state)
!wm->wm[level].plane_en; --level)
{ }
+ /* Highest common enabled wm level for all planes */
+ max_level = min(level, max_level);
+ }
+
+ /* No enabled planes? */
+ if (max_level == INT_MAX)
+ return true;
+
+ for_each_plane_id_on_crtc(crtc, plane_id) {
+ const struct skl_plane_wm *wm =
+ &crtc_state->wm.skl.optimal.planes[plane_id];
+
/*
- * If any of the planes on this pipe don't enable wm levels that
- * incur memory latencies higher than sagv_block_time_us we
- * can't enable SAGV.
+ * All enabled planes must have enabled a common wm level that
+ * can tolerate memory latencies higher than sagv_block_time_us
*/
- if (!wm->wm[level].can_sagv)
+ if (wm->wm[0].plane_en && !wm->wm[max_level].can_sagv)
return false;
}
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
