diff mbox series

[1/3] drm/i915: Remove redundant check for DG1

Message ID 20230306204954.753739-1-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/i915: Remove redundant check for DG1 | expand

Commit Message

Lucas De Marchi March 6, 2023, 8:49 p.m. UTC
dg1_gt_workarounds_init() is only ever called for DG1, so there is no
point checking it again.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Comments

Matt Roper March 6, 2023, 11:15 p.m. UTC | #1
On Mon, Mar 06, 2023 at 12:49:52PM -0800, Lucas De Marchi wrote:
> dg1_gt_workarounds_init() is only ever called for DG1, so there is no
> point checking it again.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

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

> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 32aa1647721a..eb6cc4867d67 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1472,21 +1472,15 @@ gen12_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  static void
>  dg1_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  {
> -	struct drm_i915_private *i915 = gt->i915;
> -
>  	gen12_gt_workarounds_init(gt, wal);
>  
>  	/* Wa_1409420604:dg1 */
> -	if (IS_DG1(i915))
> -		wa_mcr_write_or(wal,
> -				SUBSLICE_UNIT_LEVEL_CLKGATE2,
> -				CPSSUNIT_CLKGATE_DIS);
> +	wa_mcr_write_or(wal, SUBSLICE_UNIT_LEVEL_CLKGATE2,
> +			CPSSUNIT_CLKGATE_DIS);
>  
>  	/* Wa_1408615072:dg1 */
>  	/* Empirical testing shows this register is unaffected by engine reset. */
> -	if (IS_DG1(i915))
> -		wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
> -			    VSUNIT_CLKGATE_DIS_TGL);
> +	wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2, VSUNIT_CLKGATE_DIS_TGL);
>  }
>  
>  static void
> -- 
> 2.39.0
>
Ville Syrjälä March 13, 2023, 9:10 a.m. UTC | #2
On Mon, Mar 06, 2023 at 12:49:52PM -0800, Lucas De Marchi wrote:
> dg1_gt_workarounds_init() is only ever called for DG1, so there is no
> point checking it again.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 32aa1647721a..eb6cc4867d67 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1472,21 +1472,15 @@ gen12_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  static void
>  dg1_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  {
> -	struct drm_i915_private *i915 = gt->i915;
> -

Looks like you pushed some stale version of this patch which
didn't remove this variable. Now the CONFIG_DRM_I915_WERROR=y
build is broken.

Did you lose that from your pre-push build .config?

>  	gen12_gt_workarounds_init(gt, wal);
>  
>  	/* Wa_1409420604:dg1 */
> -	if (IS_DG1(i915))
> -		wa_mcr_write_or(wal,
> -				SUBSLICE_UNIT_LEVEL_CLKGATE2,
> -				CPSSUNIT_CLKGATE_DIS);
> +	wa_mcr_write_or(wal, SUBSLICE_UNIT_LEVEL_CLKGATE2,
> +			CPSSUNIT_CLKGATE_DIS);
>  
>  	/* Wa_1408615072:dg1 */
>  	/* Empirical testing shows this register is unaffected by engine reset. */
> -	if (IS_DG1(i915))
> -		wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
> -			    VSUNIT_CLKGATE_DIS_TGL);
> +	wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2, VSUNIT_CLKGATE_DIS_TGL);
>  }
>  
>  static void
> -- 
> 2.39.0
Lucas De Marchi March 13, 2023, 1:23 p.m. UTC | #3
On Mon, Mar 13, 2023 at 11:10:26AM +0200, Ville Syrjälä wrote:
>On Mon, Mar 06, 2023 at 12:49:52PM -0800, Lucas De Marchi wrote:
>> dg1_gt_workarounds_init() is only ever called for DG1, so there is no
>> point checking it again.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 12 +++---------
>>  1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> index 32aa1647721a..eb6cc4867d67 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> @@ -1472,21 +1472,15 @@ gen12_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>>  static void
>>  dg1_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>>  {
>> -	struct drm_i915_private *i915 = gt->i915;
>> -
>
>Looks like you pushed some stale version of this patch which
>didn't remove this variable. Now the CONFIG_DRM_I915_WERROR=y
>build is broken.
>
>Did you lose that from your pre-push build .config?

no, looks like a conflict between drm-intel-gt-next and drm-intel-next

69ea87e1591a ("drm/i915/dg1: Drop support for pre-production steppings")
merged in drm-intel-next dropped the only user.

This patch was merged in drm-intel-gt-next and I didn't realize the
race was with the other branch rather than with the same branch due to
taking a long time for me to apply. Let me rebuild drm-tip to fix it up.

Lucas De Marchi

>
>>  	gen12_gt_workarounds_init(gt, wal);
>>
>>  	/* Wa_1409420604:dg1 */
>> -	if (IS_DG1(i915))
>> -		wa_mcr_write_or(wal,
>> -				SUBSLICE_UNIT_LEVEL_CLKGATE2,
>> -				CPSSUNIT_CLKGATE_DIS);
>> +	wa_mcr_write_or(wal, SUBSLICE_UNIT_LEVEL_CLKGATE2,
>> +			CPSSUNIT_CLKGATE_DIS);
>>
>>  	/* Wa_1408615072:dg1 */
>>  	/* Empirical testing shows this register is unaffected by engine reset. */
>> -	if (IS_DG1(i915))
>> -		wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
>> -			    VSUNIT_CLKGATE_DIS_TGL);
>> +	wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2, VSUNIT_CLKGATE_DIS_TGL);
>>  }
>>
>>  static void
>> --
>> 2.39.0
>
>-- 
>Ville Syrjälä
>Intel
Ville Syrjälä March 13, 2023, 1:32 p.m. UTC | #4
On Mon, Mar 13, 2023 at 06:23:59AM -0700, Lucas De Marchi wrote:
> On Mon, Mar 13, 2023 at 11:10:26AM +0200, Ville Syrjälä wrote:
> >On Mon, Mar 06, 2023 at 12:49:52PM -0800, Lucas De Marchi wrote:
> >> dg1_gt_workarounds_init() is only ever called for DG1, so there is no
> >> point checking it again.
> >>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 12 +++---------
> >>  1 file changed, 3 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >> index 32aa1647721a..eb6cc4867d67 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >> @@ -1472,21 +1472,15 @@ gen12_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> >>  static void
> >>  dg1_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> >>  {
> >> -	struct drm_i915_private *i915 = gt->i915;
> >> -
> >
> >Looks like you pushed some stale version of this patch which
> >didn't remove this variable. Now the CONFIG_DRM_I915_WERROR=y
> >build is broken.
> >
> >Did you lose that from your pre-push build .config?
> 
> no, looks like a conflict between drm-intel-gt-next and drm-intel-next
> 
> 69ea87e1591a ("drm/i915/dg1: Drop support for pre-production steppings")
> merged in drm-intel-next dropped the only user.
> 
> This patch was merged in drm-intel-gt-next and I didn't realize the
> race was with the other branch rather than with the same branch due to
> taking a long time for me to apply. Let me rebuild drm-tip to fix it up.

I already fixed it. Just forgot to note that here.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 32aa1647721a..eb6cc4867d67 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1472,21 +1472,15 @@  gen12_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
 static void
 dg1_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
 {
-	struct drm_i915_private *i915 = gt->i915;
-
 	gen12_gt_workarounds_init(gt, wal);
 
 	/* Wa_1409420604:dg1 */
-	if (IS_DG1(i915))
-		wa_mcr_write_or(wal,
-				SUBSLICE_UNIT_LEVEL_CLKGATE2,
-				CPSSUNIT_CLKGATE_DIS);
+	wa_mcr_write_or(wal, SUBSLICE_UNIT_LEVEL_CLKGATE2,
+			CPSSUNIT_CLKGATE_DIS);
 
 	/* Wa_1408615072:dg1 */
 	/* Empirical testing shows this register is unaffected by engine reset. */
-	if (IS_DG1(i915))
-		wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
-			    VSUNIT_CLKGATE_DIS_TGL);
+	wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2, VSUNIT_CLKGATE_DIS_TGL);
 }
 
 static void