diff mbox series

[v2,6/6] drm/i915: Remove special handling for !RCS_MASK()

Message ID 20240312235145.2443975-7-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: cleanup dead code | expand

Commit Message

Lucas De Marchi March 12, 2024, 11:51 p.m. UTC 
With both XEHPSDV and PVC removed (as platforms, most of their code
remain used by others), there's no need to handle !RCS_MASK() as
other platforms don't ever have fused-off render. Remove those code
paths and the special WA flag when initializing GuC.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c   | 5 ++---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c      | 4 ----
 drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 -
 3 files changed, 2 insertions(+), 8 deletions(-)

Comments

Matt Roper March 14, 2024, 8:57 p.m. UTC | #1 
On Tue, Mar 12, 2024 at 04:51:45PM -0700, Lucas De Marchi wrote:
> With both XEHPSDV and PVC removed (as platforms, most of their code
> remain used by others), there's no need to handle !RCS_MASK() as
> other platforms don't ever have fused-off render. Remove those code
> paths and the special WA flag when initializing GuC.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c   | 5 ++---
>  drivers/gpu/drm/i915/gt/uc/intel_guc.c      | 4 ----
>  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 -
>  3 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 396f5fe993c3..476651bd0a21 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -497,9 +497,8 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id,
>  	engine->logical_mask = BIT(logical_instance);
>  	__sprint_engine_name(engine);
>  
> -	if ((engine->class == COMPUTE_CLASS && !RCS_MASK(engine->gt) &&
> -	     __ffs(CCS_MASK(engine->gt)) == engine->instance) ||
> -	     engine->class == RENDER_CLASS)
> +	if ((engine->class == COMPUTE_CLASS || engine->class == RENDER_CLASS) &&
> +	    __ffs(CCS_MASK(engine->gt) | RCS_MASK(engine->gt)) == engine->instance)
>  		engine->flags |= I915_ENGINE_FIRST_RENDER_COMPUTE;

Given that we expect all i915 platforms to have a render engine now, we
could simplify this down to just

        if (engine->class == RENDER)
                engine->flags |= I915_ENGINE_FIRST_RENDER_COMPUTE;

But either way,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

>  
>  	/* features common between engines sharing EUs */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 217277329546..3dd7699f2ad3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -320,10 +320,6 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc)
>  	if (IS_DG2_G11(gt->i915))
>  		flags |= GUC_WA_CONTEXT_ISOLATION;
>  
> -	/* Wa_18020744125 */
> -	if (!RCS_MASK(gt))
> -		flags |= GUC_WA_RCS_REGS_IN_CCS_REGS_LIST;
> -
>  	/*
>  	 * Wa_14018913170: Applicable to all platforms supported by i915 so
>  	 * don't bother testing for all X/Y/Z platforms explicitly.
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> index 14797e80bc92..1ad31a743197 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> @@ -101,7 +101,6 @@
>  #define   GUC_WA_RCS_CCS_SWITCHOUT	BIT(16)
>  #define   GUC_WA_HOLD_CCS_SWITCHOUT	BIT(17)
>  #define   GUC_WA_POLLCS			BIT(18)
> -#define   GUC_WA_RCS_REGS_IN_CCS_REGS_LIST	BIT(21)
>  #define   GUC_WA_ENABLE_TSC_CHECK_ON_RC6	BIT(22)
>  
>  #define GUC_CTL_FEATURE			2
> -- 
> 2.43.0
>
Lucas De Marchi March 20, 2024, 5:30 a.m. UTC | #2 
On Thu, Mar 14, 2024 at 01:57:46PM -0700, Matt Roper wrote:
>On Tue, Mar 12, 2024 at 04:51:45PM -0700, Lucas De Marchi wrote:
>> With both XEHPSDV and PVC removed (as platforms, most of their code
>> remain used by others), there's no need to handle !RCS_MASK() as
>> other platforms don't ever have fused-off render. Remove those code
>> paths and the special WA flag when initializing GuC.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_engine_cs.c   | 5 ++---
>>  drivers/gpu/drm/i915/gt/uc/intel_guc.c      | 4 ----
>>  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 -
>>  3 files changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> index 396f5fe993c3..476651bd0a21 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> @@ -497,9 +497,8 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id,
>>  	engine->logical_mask = BIT(logical_instance);
>>  	__sprint_engine_name(engine);
>>
>> -	if ((engine->class == COMPUTE_CLASS && !RCS_MASK(engine->gt) &&
>> -	     __ffs(CCS_MASK(engine->gt)) == engine->instance) ||
>> -	     engine->class == RENDER_CLASS)
>> +	if ((engine->class == COMPUTE_CLASS || engine->class == RENDER_CLASS) &&
>> +	    __ffs(CCS_MASK(engine->gt) | RCS_MASK(engine->gt)) == engine->instance)
>>  		engine->flags |= I915_ENGINE_FIRST_RENDER_COMPUTE;
>
>Given that we expect all i915 platforms to have a render engine now, we
>could simplify this down to just
>
>        if (engine->class == RENDER)
>                engine->flags |= I915_ENGINE_FIRST_RENDER_COMPUTE;

makes sense, but I'd rather add a patch on top to get rid of I915_ENGINE_FIRST_RENDER_COMPUTE
and make this assumption "we always have a render engine" explicit. It
seems that there's more that can be simplified when we do that.

>
>But either way,
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

I will keep this patch as is for now. Thanks

Lucas De Marchi

>
>>
>>  	/* features common between engines sharing EUs */
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> index 217277329546..3dd7699f2ad3 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> @@ -320,10 +320,6 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc)
>>  	if (IS_DG2_G11(gt->i915))
>>  		flags |= GUC_WA_CONTEXT_ISOLATION;
>>
>> -	/* Wa_18020744125 */
>> -	if (!RCS_MASK(gt))
>> -		flags |= GUC_WA_RCS_REGS_IN_CCS_REGS_LIST;
>> -
>>  	/*
>>  	 * Wa_14018913170: Applicable to all platforms supported by i915 so
>>  	 * don't bother testing for all X/Y/Z platforms explicitly.
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>> index 14797e80bc92..1ad31a743197 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>> @@ -101,7 +101,6 @@
>>  #define   GUC_WA_RCS_CCS_SWITCHOUT	BIT(16)
>>  #define   GUC_WA_HOLD_CCS_SWITCHOUT	BIT(17)
>>  #define   GUC_WA_POLLCS			BIT(18)
>> -#define   GUC_WA_RCS_REGS_IN_CCS_REGS_LIST	BIT(21)
>>  #define   GUC_WA_ENABLE_TSC_CHECK_ON_RC6	BIT(22)
>>
>>  #define GUC_CTL_FEATURE			2
>> --
>> 2.43.0
>>
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 396f5fe993c3..476651bd0a21 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -497,9 +497,8 @@  static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id,
 	engine->logical_mask = BIT(logical_instance);
 	__sprint_engine_name(engine);
 
-	if ((engine->class == COMPUTE_CLASS && !RCS_MASK(engine->gt) &&
-	     __ffs(CCS_MASK(engine->gt)) == engine->instance) ||
-	     engine->class == RENDER_CLASS)
+	if ((engine->class == COMPUTE_CLASS || engine->class == RENDER_CLASS) &&
+	    __ffs(CCS_MASK(engine->gt) | RCS_MASK(engine->gt)) == engine->instance)
 		engine->flags |= I915_ENGINE_FIRST_RENDER_COMPUTE;
 
 	/* features common between engines sharing EUs */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 217277329546..3dd7699f2ad3 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -320,10 +320,6 @@  static u32 guc_ctl_wa_flags(struct intel_guc *guc)
 	if (IS_DG2_G11(gt->i915))
 		flags |= GUC_WA_CONTEXT_ISOLATION;
 
-	/* Wa_18020744125 */
-	if (!RCS_MASK(gt))
-		flags |= GUC_WA_RCS_REGS_IN_CCS_REGS_LIST;
-
 	/*
 	 * Wa_14018913170: Applicable to all platforms supported by i915 so
 	 * don't bother testing for all X/Y/Z platforms explicitly.
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
index 14797e80bc92..1ad31a743197 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
@@ -101,7 +101,6 @@ 
 #define   GUC_WA_RCS_CCS_SWITCHOUT	BIT(16)
 #define   GUC_WA_HOLD_CCS_SWITCHOUT	BIT(17)
 #define   GUC_WA_POLLCS			BIT(18)
-#define   GUC_WA_RCS_REGS_IN_CCS_REGS_LIST	BIT(21)
 #define   GUC_WA_ENABLE_TSC_CHECK_ON_RC6	BIT(22)
 
 #define GUC_CTL_FEATURE			2