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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@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@vger.kernel.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mark Janes <mark.a.janes@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Oscar Mateo <oscar.mateo@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Reported-by: Mark Janes <mark.a.janes@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@linux.intel.com>
> 
> CTS remains fixed,
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> I know of no reason why this should work in this order,
> Reviewed-by: Chris Wilson <chris@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