[Intel-gfx] [PATCH v2] drm/i915: Move init_clock_gating() back to where it was

[Ville Syrjälä]

On Wed, Nov 08, 2017 at 04:18:27PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-08 13:35:55)
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Apparently setting up a bunch of GT registers before we've properly
> > initialized the rest of the GT hardware leads to these setting being
> > lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> > Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > by doing init_clock_gating() too early. This should actually affect
> > other platforms as well, but apparently not to such a great degree.
> > 
> > What I was ultimately after in that commit was to move the
> > ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> > move init_clock_gating() back to where it was, and call
> > ilk_init_lp_watermarks() just before the watermark state readout.
> > 
> > This highlights how fragile and messed up our init order really is.
> > I wonder why we even initialize the display before gem. The opposite
> > order would make much more sense to me...
> > 
> > v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
> >     be done before all planes might get disabled.
> > 
> > Cc: stable at vger.kernel.org
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mark Janes <mark.a.janes at intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Oscar Mateo <oscar.mateo at intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Reported-by: Mark Janes <mark.a.janes at intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> CTS remains fixed,
> Tested-by: Chris Wilson <chris at chris-wilson.co.uk>
> 
> I know of no reason why this should work in this order,
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> (but I didn't know about the v2 requirement either!)

It's somewhat baffling how this stuff managed to work before I
moved .init_clock_gating() to happen earlier. AFAICS that plane
disabling vs. the chicken bit should have bitten us back then as
well.

Patch pushed to dinq. Thanks for doing all the heavy lifting on
this one.

-- 
Ville Syrjälä
Intel OTC