Re: [Intel-gfx] [PATCH] drm/i915: Skip Bit12 fw domain reset for gen12+

[Matt Roper <matthew.d.roper@intel.com>]

On Thu, Dec 15, 2022 at 05:38:22PM +0000, Tvrtko Ursulin wrote:
> 
> On 25/08/2022 18:49, Sripada, Radhakrishna wrote:
> > Hi G.G,
> > 
> > > -----Original Message-----
> > > From: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>
> > > Sent: Tuesday, August 23, 2022 11:14 PM
> > > To: Roper, Matthew D <matthew.d.roper@intel.com>; Sripada, Radhakrishna
> > > <radhakrishna.sripada@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Skip Bit12 fw domain reset for gen12+
> > > 
> > > 
> > > 
> > > On 8/18/22 3:00 PM, Matt Roper wrote:
> > > > On Wed, Aug 17, 2022 at 03:43:04PM -0700, Radhakrishna Sripada wrote:
> > > > > Bit12 of the Forcewake request register should not be cleared post
> > > > > gen12. Do not touch this bit while clearing during fw domain reset.
> > > > > 
> > > > > Bspec: 52542
> > > > > 
> > > > > Signed-off-by: Sushma Venkatesh Reddy
> > > <sushma.venkatesh.reddy@intel.com>
> > > > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > > ---
> > > > >    drivers/gpu/drm/i915/intel_uncore.c | 5 ++++-
> > > > >    1 file changed, 4 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> > > b/drivers/gpu/drm/i915/intel_uncore.c
> > > > > index a852c471d1b3..c85e2b686c95 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > > > @@ -113,7 +113,10 @@ fw_domain_reset(const struct
> > > intel_uncore_forcewake_domain *d)
> > > > >    	 * off in ICL+), so no waiting for acks
> > > > >    	 */
> > > > >    	/* WaRsClearFWBitsAtReset:bdw,skl */
> > > > 
> > > > While we're at it, let's remove the "bdw,skl" from this comment since
> > > > it's misleading and doesn't match the code.  We do still apply this
> > > > workaround on other pre-gen12 platforms than just those two.
> > > > 
> > > > Aside from the comment tweak,
> > > > 
> > > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > > > 
> > > > > -	fw_clear(d, 0xffff);
> > > > > +	if (GRAPHICS_VER(d->uncore->i915) >= 12)
> > > Hi Radhakrishna Sripada,
> > > 
> > > In bspec 52542, there is an explanation that BIT12 should not be set for
> > > address 0xA188 corresponding to FORCEWAKE_MT/FORCEWAKE_GT_GEN9, but
> > > in
> > > bspec 52466, there is no explanation that BIT12 should not be set for
> > > address 0xA278, address of FORCEWAKE_RENDER_GEN9.
> > > 
> > > I ask if fw_domain_reset() should perform fw_clear() by comparing not
> > > only GRAPHICS_VER() >= 12 but also checking of FW_DOMAIN_ID_RENDER and
> > > FW_DOMAIN_ID_GT values.
> > Based on the note in 52542, all other WA notes are overridden by the comment. So unless stated
> > otherwise, it should apply to this register as well.
> > 
> > Created a bspec issue to request for additional clarification just to be safe. Will send an additional
> > patch if the comment contradicts our understanding.
> 
> How important was this patch - should it be sent to stable?

It shouldn't be needed for stable; clearing the bit isn't necessary from
gen12 onward, but as far as we know it doesn't cause any problems either
(except maybe on MTL, which is still under force_probe and not supported
on stable kernels).  So according to stable-kernel-rules.rst, this patch
would not qualify.


Matt

> 
> Regards,
> 
> Tvrtko

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation