Attachment 129720 Details for Bug 96645 – [PATCH] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks

[patch] [PATCH] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks

0001-drm-i915-Do-.init_clock_gating-earlier-to-avoid-it-c.patch (text/plain), 3.30 KB, created by Ville Syrjala on 2017-02-17 20:20:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ville Syrjala

Created: 2017-02-17 20:20:35 UTC

Size: 3.30 KB

patch

obsolete

>From 672acf191ad991645f848c0532692caddac628c1 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
>Date: Fri, 17 Feb 2017 21:45:17 +0200
>Subject: [PATCH] drm/i915: Do .init_clock_gating() earlier to avoid it
> clobbering watermarks
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Currently ILK-BDW explicitly disable LP1+ watermarks from their
>.init_clock_gating() hooks. Unfortunately that hook gets called way too
>late since by that time we've already initialized all the watermark
>state tracking which then gets out of sync with the hardware state.
>
>We may eventually want to consider killing off the explicit LP1+
>disable from .init_clock_gating(). In the meantime however, we can
>avoid the problem by reordering the init sequence such that
>intel_modeset_init_hw()->intel_init_clock_gating() gets called
>prior to the hardware state takeover.
>
>I suppose prior to the two stage watermark programming we were
>magically saved by something that forced the watermarks to be
>reprogrammed fully after .init_clock_gating() got called. But
>now that no longer happens.
>
>Note that the diff might look a bit odd as it kills off one
>call of intel_update_cdclk(), but that's fine because
>intel_modeset_init_hw() does the exact same thing. Previously
>we just did it twice.
>
>Actually even this new init sequence is pretty bogus as
>.init_clock_gating() really should be called before any gem
>hardware init since it can  configure various clock gating
>workarounds and whatnot that affect the GT side as well. Also
>intel_modeset_init() really should get split up into better
>defined init stages. Another "fun" detail is that
>intel_modeset_gem_init() is where RPS/RC6 gets configured.
>Why that is done from the display code is beyond me. I've
>decided to leave all this be for now, and just try to fix
>the init sequence enough for watermarks to work.
>
>Cc: stable@vger.kernel.org
>Cc: Gabriele Mazzotta <gabriele.mzt@gmail.com>
>Cc: David Purton <dcpurton@marshwiggle.net>
>Cc: Matt Roper <matthew.d.roper@intel.com>
>Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>Reported-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
>Reported-by: David Purton <dcpurton@marshwiggle.net>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96645
>Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
>Signed-off-by: Ville SyrjÃ¤lÃ¤ <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/intel_display.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index 730aee755c80..0466db16f193 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -15028,12 +15028,11 @@ int intel_modeset_init(struct drm_device *dev)
> 		}
> 	}
> 
>-	intel_update_czclk(dev_priv);
>-	intel_update_cdclk(dev_priv);
>-	dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
>-
> 	intel_shared_dpll_init(dev);
> 
>+	intel_update_czclk(dev_priv);
>+	intel_modeset_init_hw(dev);
>+
> 	if (dev_priv->max_cdclk_freq == 0)
> 		intel_update_max_cdclk(dev_priv);
> 
>@@ -15591,8 +15590,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
> 
> 	intel_init_gt_powersave(dev_priv);
> 
>-	intel_modeset_init_hw(dev);
>-
> 	intel_setup_overlay(dev_priv);
> }
> 
>-- 
>2.10.2
>

Actions: View | Splinter Review

Attachments on bug 96645: 124794 | 129720

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.