Re: [PATCH] drm/i915: Move hsw GT w/a to engine initialisation

From: Chris Wilson <chris@chris-wilson.co.uk>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Move hsw GT w/a to engine initialisation
Date: Fri, 03 Nov 2017 10:38:38 +0000	[thread overview]
Message-ID: <150970551808.13980.8183733825087038900@mail.alporthouse.com> (raw)
In-Reply-To: <20171103102517.GU10981@intel.com>

Quoting Ville Syrjälä (2017-11-03 10:25:17)
> On Fri, Nov 03, 2017 at 02:56:28AM +0000, Chris Wilson wrote:
> > In commit b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to
> > avoid it clobbering watermarks") init_clock_gating was called earlier in
> > the module load sequence, moving it before we acquired the forcewake
> > used to initialise the engines. This revealed that on Haswell, at least,
> > some of those GT w/as had been moved into the power context, and so as
> > we were now setting them outside of the power context, those settings
> > were being lost.
> 
> Hmm. Writes shouldn't need forcewake as they go through the wake FIFO,
> And the power context should have been set up by the BIOS. So I'm not
> sure that explanation is entirely satisfactory, for masked registers
> at least. For the ones that do RMW it could well be a problem.
> 
> Also there are some registers on the list that IIRC live in the
> logical context, like GT_MODE/CACHE_MODE. I guess if the BIOS would
> already enable rc6 those would be lost until we have a context set up.

We don't overwrite the state when loading the first context, so that
itself shouldn't be an issue. It definitely does seem like some context
registers are being lost; and that is a behavior we have associated with
the "powercontext" (e.g. so many registers for execlists :(.

> This problem doesn't seem like it should be specific to HSW. So I wonder
> if we should start by just reverting that offending patch and move just
> the watermark thing out to some earlier position in the sequence.

Whatever makes the simpler cc:stable patch. We have to overhaul these
register initialisations, and I definitely pity the poor soul who has to
navigate all the old bspecs to work out where each register needs to
live.

Ville do you want to take a pass at splitting the wm from clock-gating?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx