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

[Sripada, Radhakrishna]

Hi G.G,

> -----Original Message-----
> From: Mun, Gwan-gyeong <gwan-gyeong.mun at intel.com>
> Sent: Tuesday, August 23, 2022 11:14 PM
> To: Roper, Matthew D <matthew.d.roper at intel.com>; Sripada, Radhakrishna
> <radhakrishna.sripada at intel.com>
> Cc: intel-gfx at 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 at intel.com>
> >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at 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 at 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. 

Thanks,
RK
> 
> Br,
> G.G.
> >> +		fw_clear(d, 0xefff);
> >> +	else
> >> +		fw_clear(d, 0xffff);
> >>   }
> >>
> >>   static inline void
> >> --
> >> 2.25.1
> >>
> >