dri-devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] drm/i915/guc: Simplify/extend platform check for Wa_14018913170
@ 2024-02-16 18:38 John.C.Harrison
  2024-02-19 20:28 ` Rodrigo Vivi
  0 siblings, 1 reply; 5+ messages in thread
From: John.C.Harrison @ 2024-02-16 18:38 UTC (permalink / raw
  To: Intel-GFX; +Cc: DRI-Devel, John Harrison

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.

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] drm/i915/guc: Simplify/extend platform check for Wa_14018913170
  2024-02-16 18:38 [PATCH v3] drm/i915/guc: Simplify/extend platform check for Wa_14018913170 John.C.Harrison
@ 2024-02-19 20:28 ` Rodrigo Vivi
  2024-02-20 19:52   ` John Harrison
  0 siblings, 1 reply; 5+ messages in thread
From: Rodrigo Vivi @ 2024-02-19 20:28 UTC (permalink / raw
  To: John.C.Harrison; +Cc: Intel-GFX, DRI-Devel

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?

Perhaps at least adding a comment in the code?

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
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] drm/i915/guc: Simplify/extend platform check for Wa_14018913170
  2024-02-19 20:28 ` Rodrigo Vivi
@ 2024-02-20 19:52   ` John Harrison
  2024-02-20 22:03     ` Rodrigo Vivi
  0 siblings, 1 reply; 5+ messages in thread
From: John Harrison @ 2024-02-20 19:52 UTC (permalink / raw
  To: Rodrigo Vivi; +Cc: Intel-GFX, DRI-Devel

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 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
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] drm/i915/guc: Simplify/extend platform check for Wa_14018913170
  2024-02-20 19:52   ` John Harrison
@ 2024-02-20 22:03     ` Rodrigo Vivi
  0 siblings, 0 replies; 5+ messages in thread
From: Rodrigo Vivi @ 2024-02-20 22:03 UTC (permalink / raw
  To: John Harrison; +Cc: Intel-GFX, DRI-Devel

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
> > > 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3] drm/i915/guc: Simplify/extend platform check for Wa_14018913170
@ 2024-02-23 20:28 John.C.Harrison
  0 siblings, 0 replies; 5+ messages in thread
From: John.C.Harrison @ 2024-02-23 20:28 UTC (permalink / raw
  To: Intel-GFX; +Cc: DRI-Devel, John Harrison, Rodrigo Vivi

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.

v2: Add extra comment (review feedback from Rodrigo).

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 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..d2b7425bbdcc2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -319,10 +319,12 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc)
 	if (!RCS_MASK(gt))
 		flags |= GUC_WA_RCS_REGS_IN_CCS_REGS_LIST;
 
-	/* Wa_14018913170 */
+	/*
+	 * Wa_14018913170: Applicable to all platforms supported by i915 so
+	 * don't bother testing for all X/Y/Z platforms explicitly.
+	 */
 	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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-02-23 20:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-16 18:38 [PATCH v3] drm/i915/guc: Simplify/extend platform check for Wa_14018913170 John.C.Harrison
2024-02-19 20:28 ` Rodrigo Vivi
2024-02-20 19:52   ` John Harrison
2024-02-20 22:03     ` Rodrigo Vivi
  -- strict thread matches above, loose matches on Subject: below --
2024-02-23 20:28 John.C.Harrison

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).