Re: [PATCH v3] drm/i915/guc: Simplify/extend platform check for Wa_14018913170

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Feb 20, 2024 at 11:52:04AM -0800, John Harrison wrote:
> On 2/19/2024 12:28, Rodrigo Vivi wrote:
> > On Fri, Feb 16, 2024 at 10:38:41AM -0800, John.C.Harrison@Intel.com wrote:
> > > From: John Harrison <John.C.Harrison@Intel.com>
> > > 
> > > The above w/a is required for every platform that the i915 driver
> > > supports. It is fixed on the latest platforms but they are only
> > > supported by Xe instead of i915. So just remove the platform check
> > > completely and keep the code simple.
> > Well, I was going to say that I would prefer a GMD version greater-than
> > check to be future proof. However if this code gets used in some other
> > new platform a new specific guc support would likely need to be added
> > as well right?
> There is no future for i915. That's the point. The only platforms that have
> the hardware fix are all ones that will only ever be supported by the Xe
> driver. So if such a platform were to be backported to i915 then there would
> be a lot more work than just adding a new GuC firmware platform.
> 
> And going backwards, the bug affects all platforms that have a GuC. So if
> any GuC code is being executed at all, then this w/a is applicable.
> 
> > 
> > Perhaps at least adding a comment in the code?
> Such as this?
>         /*
>          * Wa_14018913170: Applicable to all platforms supported by i915.

I believe only this first line would be enough. but up to you and feel free
to put my rv-b with whatever version you get.

 so
>          * don't bother testing for all X/Y/Z platforms explicitly.
>          */
> 
> John.
> 
> 
> > 
> > with that
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > 
> > > Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gt/uc/intel_guc.c | 3 +--
> > >   1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> > > index 2b450c43bbd7f..a3662edb42032 100644
> > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> > > @@ -321,8 +321,7 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc)
> > >   	/* Wa_14018913170 */
> > >   	if (GUC_FIRMWARE_VER(guc) >= MAKE_GUC_VER(70, 7, 0)) {
> > > -		if (IS_DG2(gt->i915) || IS_METEORLAKE(gt->i915) || IS_PONTEVECCHIO(gt->i915))
> > > -			flags |= GUC_WA_ENABLE_TSC_CHECK_ON_RC6;
> > > +		flags |= GUC_WA_ENABLE_TSC_CHECK_ON_RC6;
> > >   	}
> > >   	return flags;
> > > -- 
> > > 2.43.0
> > > 
>