diff mbox series

[v2] drm/i915/mtl: do not enable render power-gating on MTL

Message ID 20230517-mtl_disable_render_pg-v2-1-0b51180a43f0@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915/mtl: do not enable render power-gating on MTL | expand

Commit Message

Andrzej Hajda May 18, 2023, 2:50 p.m. UTC 
Multiple CI tests fails with forcewake ack timeouts if render
power gating is enabled.
BSpec 52698 states it should be 0 for MTL, but apparently
this info is outdated. Anyway since the patch makes MTL pass basic
tests added FIXME tag informing this is temporary workaround.

v2: added FIXME tag

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4983
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
Changes in v2:
- added FIXME tag
- Link to v1: https://lore.kernel.org/r/20230517-mtl_disable_render_pg-v1-1-6495eebbfb24@intel.com
---
 drivers/gpu/drm/i915/gt/intel_rc6.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


---
base-commit: 641646b29337c97681e0edb67ad2e08aef3fb850
change-id: 20230517-mtl_disable_render_pg-b9f9f1567f9e

Best regards,

Comments

Rodrigo Vivi May 18, 2023, 3:09 p.m. UTC | #1 
On Thu, May 18, 2023 at 04:50:52PM +0200, Andrzej Hajda wrote:
> Multiple CI tests fails with forcewake ack timeouts if render
> power gating is enabled.
> BSpec 52698 states it should be 0 for MTL, but apparently
> this info is outdated. Anyway since the patch makes MTL pass basic
> tests added FIXME tag informing this is temporary workaround.
> 
> v2: added FIXME tag
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4983

No change in the patch is needed, but do we have another
(can be internal) ticket with the work to get this properly
fix without wasting power?

> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> ---
> Changes in v2:
> - added FIXME tag
> - Link to v1: https://lore.kernel.org/r/20230517-mtl_disable_render_pg-v1-1-6495eebbfb24@intel.com
> ---
>  drivers/gpu/drm/i915/gt/intel_rc6.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index 908a3d0f2343f4..58bb1c55294c93 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -117,8 +117,14 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6)
>  			GEN6_RC_CTL_RC6_ENABLE |
>  			GEN6_RC_CTL_EI_MODE(1);
>  
> -	/* Wa_16011777198 - Render powergating must remain disabled */
> -	if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
> +	/*
> +	 * Wa_16011777198 and BSpec 52698 - Render powergating must be off.
> +	 * FIXME BSpec is outdated, disabling powergating for MTL is just
> +	 * temporary wa and should be removed after fixing real cause
> +	 * of forcewake timeouts.
> +	 */
> +	if (IS_METEORLAKE(gt->i915) ||
> +	    IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
>  	    IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))
>  		pg_enable =
>  			GEN9_MEDIA_PG_ENABLE |
> 
> ---
> base-commit: 641646b29337c97681e0edb67ad2e08aef3fb850
> change-id: 20230517-mtl_disable_render_pg-b9f9f1567f9e
> 
> Best regards,
> -- 
> Andrzej Hajda <andrzej.hajda@intel.com>
>
Andrzej Hajda May 18, 2023, 3:33 p.m. UTC | #2 
On 18.05.2023 17:09, Rodrigo Vivi wrote:
> On Thu, May 18, 2023 at 04:50:52PM +0200, Andrzej Hajda wrote:
>> Multiple CI tests fails with forcewake ack timeouts if render
>> power gating is enabled.
>> BSpec 52698 states it should be 0 for MTL, but apparently
>> this info is outdated. Anyway since the patch makes MTL pass basic
>> tests added FIXME tag informing this is temporary workaround.
>>
>> v2: added FIXME tag
>>
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4983
> No change in the patch is needed, but do we have another
> (can be internal) ticket with the work to get this properly
> fix without wasting power?

Yes there are jiras and related hsdes. In fact this tag is not fully 
correct, as the issue is about MTL and RPL. I wanted to use 
"References:" tag but "dim checkpatch" complains, so I have ended with 
Closes.
Regarding your "No change in the patch is needed", do you prefer to 
merge v1 or v2?

[1]:
Regards
Andrzej

>
>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
>> Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
>> ---
>> Changes in v2:
>> - added FIXME tag
>> - Link to v1: https://lore.kernel.org/r/20230517-mtl_disable_render_pg-v1-1-6495eebbfb24@intel.com
>> ---
>>   drivers/gpu/drm/i915/gt/intel_rc6.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> index 908a3d0f2343f4..58bb1c55294c93 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> @@ -117,8 +117,14 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6)
>>   			GEN6_RC_CTL_RC6_ENABLE |
>>   			GEN6_RC_CTL_EI_MODE(1);
>>   
>> -	/* Wa_16011777198 - Render powergating must remain disabled */
>> -	if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
>> +	/*
>> +	 * Wa_16011777198 and BSpec 52698 - Render powergating must be off.
>> +	 * FIXME BSpec is outdated, disabling powergating for MTL is just
>> +	 * temporary wa and should be removed after fixing real cause
>> +	 * of forcewake timeouts.
>> +	 */
>> +	if (IS_METEORLAKE(gt->i915) ||
>> +	    IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
>>   	    IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))
>>   		pg_enable =
>>   			GEN9_MEDIA_PG_ENABLE |
>>
>> ---
>> base-commit: 641646b29337c97681e0edb67ad2e08aef3fb850
>> change-id: 20230517-mtl_disable_render_pg-b9f9f1567f9e
>>
>> Best regards,
>> -- 
>> Andrzej Hajda <andrzej.hajda@intel.com>
>>
Rodrigo Vivi May 18, 2023, 4:11 p.m. UTC | #3 
On Thu, May 18, 2023 at 05:33:55PM +0200, Andrzej Hajda wrote:
> 
> 
> On 18.05.2023 17:09, Rodrigo Vivi wrote:
> > On Thu, May 18, 2023 at 04:50:52PM +0200, Andrzej Hajda wrote:
> > > Multiple CI tests fails with forcewake ack timeouts if render
> > > power gating is enabled.
> > > BSpec 52698 states it should be 0 for MTL, but apparently
> > > this info is outdated. Anyway since the patch makes MTL pass basic
> > > tests added FIXME tag informing this is temporary workaround.
> > > 
> > > v2: added FIXME tag
> > > 
> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4983
> > No change in the patch is needed, but do we have another
> > (can be internal) ticket with the work to get this properly
> > fix without wasting power?
> 
> Yes there are jiras and related hsdes. In fact this tag is not fully
> correct, as the issue is about MTL and RPL. I wanted to use "References:"
> tag but "dim checkpatch" complains, so I have ended with Closes.
> Regarding your "No change in the patch is needed", do you prefer to merge v1
> or v2?

please go ahead with the v2

> 
> [1]:
> Regards
> Andrzej
> 
> > 
> > > Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> > > Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> > > ---
> > > Changes in v2:
> > > - added FIXME tag
> > > - Link to v1: https://lore.kernel.org/r/20230517-mtl_disable_render_pg-v1-1-6495eebbfb24@intel.com
> > > ---
> > >   drivers/gpu/drm/i915/gt/intel_rc6.c | 10 ++++++++--
> > >   1 file changed, 8 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > > index 908a3d0f2343f4..58bb1c55294c93 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > > @@ -117,8 +117,14 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6)
> > >   			GEN6_RC_CTL_RC6_ENABLE |
> > >   			GEN6_RC_CTL_EI_MODE(1);
> > > -	/* Wa_16011777198 - Render powergating must remain disabled */
> > > -	if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
> > > +	/*
> > > +	 * Wa_16011777198 and BSpec 52698 - Render powergating must be off.
> > > +	 * FIXME BSpec is outdated, disabling powergating for MTL is just
> > > +	 * temporary wa and should be removed after fixing real cause
> > > +	 * of forcewake timeouts.
> > > +	 */
> > > +	if (IS_METEORLAKE(gt->i915) ||
> > > +	    IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
> > >   	    IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))
> > >   		pg_enable =
> > >   			GEN9_MEDIA_PG_ENABLE |
> > > 
> > > ---
> > > base-commit: 641646b29337c97681e0edb67ad2e08aef3fb850
> > > change-id: 20230517-mtl_disable_render_pg-b9f9f1567f9e
> > > 
> > > Best regards,
> > > -- 
> > > Andrzej Hajda <andrzej.hajda@intel.com>
> > > 
>
Andrzej Hajda May 18, 2023, 4:56 p.m. UTC | #4 
On 18.05.2023 18:11, Rodrigo Vivi wrote:
> On Thu, May 18, 2023 at 05:33:55PM +0200, Andrzej Hajda wrote:
>>
>> On 18.05.2023 17:09, Rodrigo Vivi wrote:
>>> On Thu, May 18, 2023 at 04:50:52PM +0200, Andrzej Hajda wrote:
>>>> Multiple CI tests fails with forcewake ack timeouts if render
>>>> power gating is enabled.
>>>> BSpec 52698 states it should be 0 for MTL, but apparently
>>>> this info is outdated. Anyway since the patch makes MTL pass basic
>>>> tests added FIXME tag informing this is temporary workaround.
>>>>
>>>> v2: added FIXME tag
>>>>
>>>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4983
>>> No change in the patch is needed, but do we have another
>>> (can be internal) ticket with the work to get this properly
>>> fix without wasting power?
>> Yes there are jiras and related hsdes. In fact this tag is not fully
>> correct, as the issue is about MTL and RPL. I wanted to use "References:"
>> tag but "dim checkpatch" complains, so I have ended with Closes.
>> Regarding your "No change in the patch is needed", do you prefer to merge v1
>> or v2?
> please go ahead with the v2

Pushed to drm-intel-gt-next

Thx
Andrzej

>> [1]:
>> Regards
>> Andrzej
>>
>>>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
>>>> Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
>>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
>>>> ---
>>>> Changes in v2:
>>>> - added FIXME tag
>>>> - Link to v1: https://lore.kernel.org/r/20230517-mtl_disable_render_pg-v1-1-6495eebbfb24@intel.com
>>>> ---
>>>>    drivers/gpu/drm/i915/gt/intel_rc6.c | 10 ++++++++--
>>>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
>>>> index 908a3d0f2343f4..58bb1c55294c93 100644
>>>> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
>>>> @@ -117,8 +117,14 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6)
>>>>    			GEN6_RC_CTL_RC6_ENABLE |
>>>>    			GEN6_RC_CTL_EI_MODE(1);
>>>> -	/* Wa_16011777198 - Render powergating must remain disabled */
>>>> -	if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
>>>> +	/*
>>>> +	 * Wa_16011777198 and BSpec 52698 - Render powergating must be off.
>>>> +	 * FIXME BSpec is outdated, disabling powergating for MTL is just
>>>> +	 * temporary wa and should be removed after fixing real cause
>>>> +	 * of forcewake timeouts.
>>>> +	 */
>>>> +	if (IS_METEORLAKE(gt->i915) ||
>>>> +	    IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
>>>>    	    IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))
>>>>    		pg_enable =
>>>>    			GEN9_MEDIA_PG_ENABLE |
>>>>
>>>> ---
>>>> base-commit: 641646b29337c97681e0edb67ad2e08aef3fb850
>>>> change-id: 20230517-mtl_disable_render_pg-b9f9f1567f9e
>>>>
>>>> Best regards,
>>>> -- 
>>>> Andrzej Hajda <andrzej.hajda@intel.com>
>>>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
index 908a3d0f2343f4..58bb1c55294c93 100644
--- a/drivers/gpu/drm/i915/gt/intel_rc6.c
+++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
@@ -117,8 +117,14 @@  static void gen11_rc6_enable(struct intel_rc6 *rc6)
 			GEN6_RC_CTL_RC6_ENABLE |
 			GEN6_RC_CTL_EI_MODE(1);
 
-	/* Wa_16011777198 - Render powergating must remain disabled */
-	if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
+	/*
+	 * Wa_16011777198 and BSpec 52698 - Render powergating must be off.
+	 * FIXME BSpec is outdated, disabling powergating for MTL is just
+	 * temporary wa and should be removed after fixing real cause
+	 * of forcewake timeouts.
+	 */
+	if (IS_METEORLAKE(gt->i915) ||
+	    IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
 	    IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))
 		pg_enable =
 			GEN9_MEDIA_PG_ENABLE |