drm/i915: Tighten SAGV constraint for pre-tgl
authorVille Syrjälä <ville.syrjala@linux.intel.com>
Fri, 5 Mar 2021 15:36:06 +0000 (17:36 +0200)
committerVille Syrjälä <ville.syrjala@linux.intel.com>
Fri, 12 Mar 2021 16:18:30 +0000 (18:18 +0200)
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@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210305153610.12177-3-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
drivers/gpu/drm/i915/intel_pm.c

index 00f1ab5a8aeb7801a33ae409a5042f79a55321bf..bf33f73ed2adb327fa6843eb330a0e07e3b03474 100644 (file)
@@ -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;
        }