From cdf64625e8d090db6abd13d1b3d71c8d77844801 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Fri, 5 Mar 2021 17:36:06 +0200 Subject: [PATCH] drm/i915: Tighten SAGV constraint for pre-tgl MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20210305153610.12177-3-ville.syrjala@linux.intel.com Reviewed-by: Stanislav Lisovskiy --- 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 00f1ab5a8aeb..bf33f73ed2ad 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.34.1