diff mbox series

[1/1] drm/i915/mtl: Enable Idle Messaging for GSC CS

Message ID 20221115134440.3669021-2-badal.nilawar@intel.com (mailing list archive)
State New, archived
Headers show
Series i915/mtl: Enable idle messaging for GSC CS | expand

Commit Message

Nilawar, Badal Nov. 15, 2022, 1:44 p.m. UTC
From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

By defaut idle mesaging is disabled for GSC CS so to unblock RC6
entry on media tile idle messaging need to be enabled.

v2:
 - Fix review comments (Vinay)
 - Set GSC idle hysterisis to 5 us (Badal)

Bspec: 71496

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 ++++
 2 files changed, 22 insertions(+)

Comments

Rodrigo Vivi Nov. 15, 2022, 3:27 p.m. UTC | #1
On Tue, Nov 15, 2022 at 07:14:40PM +0530, Badal Nilawar wrote:
> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> 
> By defaut idle mesaging is disabled for GSC CS so to unblock RC6
> entry on media tile idle messaging need to be enabled.
> 
> v2:
>  - Fix review comments (Vinay)
>  - Set GSC idle hysterisis to 5 us (Badal)

btw, no need for the cover letter in single/standalone patches.
This history here is enough.

> 
> Bspec: 71496
> 
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 ++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index b0a4a2dbe3ee..5522885b2db0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -15,6 +15,22 @@
>  #include "intel_rc6.h"
>  #include "intel_ring.h"
>  #include "shmem_utils.h"
> +#include "intel_gt_regs.h"
> +
> +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine)
> +{
> +	struct drm_i915_private *i915 = engine->i915;
> +
> +	if (IS_METEORLAKE(i915) && engine->id == GSC0) {
> +		intel_uncore_write(engine->gt->uncore,
> +				   RC_PSMI_CTRL_GSCCS,
> +				   _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
> +		/* 5 us hysterisis */

typo

> +		intel_uncore_write(engine->gt->uncore,
> +				   PWRCTX_MAXCNT_GSCCS,
> +				   0xA);

you said 5 above, but used 10 here, why?

> +	}
> +}
>  
>  static void dbg_poison_ce(struct intel_context *ce)
>  {
> @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
>  
>  	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>  	intel_engine_init_heartbeat(engine);
> +
> +	intel_gsc_idle_msg_enable(engine);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 07031e03f80c..20472eb15364 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -913,6 +913,10 @@
>  #define  MSG_IDLE_FW_MASK	REG_GENMASK(13, 9)
>  #define  MSG_IDLE_FW_SHIFT	9
>  
> +#define	RC_PSMI_CTRL_GSCCS	_MMIO(0x11a050)
> +#define	  IDLE_MSG_DISABLE	BIT(0)

REG_BIT should be preferred.

> +#define PWRCTX_MAXCNT_GSCCS	_MMIO(0x11a054)
> +
>  #define FORCEWAKE_MEDIA_GEN9			_MMIO(0xa270)
>  #define FORCEWAKE_RENDER_GEN9			_MMIO(0xa278)
>  
> -- 
> 2.25.1
>
Vinay Belgaumkar Nov. 16, 2022, 6:05 p.m. UTC | #2
On 11/15/2022 5:44 AM, Badal Nilawar wrote:
> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
> By defaut idle mesaging is disabled for GSC CS so to unblock RC6
> entry on media tile idle messaging need to be enabled.
>
> v2:
>   - Fix review comments (Vinay)
>   - Set GSC idle hysterisis to 5 us (Badal)
>
> Bspec: 71496
>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
>   drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 ++++
>   2 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index b0a4a2dbe3ee..5522885b2db0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -15,6 +15,22 @@
>   #include "intel_rc6.h"
>   #include "intel_ring.h"
>   #include "shmem_utils.h"
> +#include "intel_gt_regs.h"
> +
> +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine)
> +{
> +	struct drm_i915_private *i915 = engine->i915;
> +
> +	if (IS_METEORLAKE(i915) && engine->id == GSC0) {
> +		intel_uncore_write(engine->gt->uncore,
> +				   RC_PSMI_CTRL_GSCCS,
> +				   _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
> +		/* 5 us hysterisis */
> +		intel_uncore_write(engine->gt->uncore,
> +				   PWRCTX_MAXCNT_GSCCS,
> +				   0xA);
> +	}
> +}
>   
>   static void dbg_poison_ce(struct intel_context *ce)
>   {
> @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
>   
>   	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>   	intel_engine_init_heartbeat(engine);
> +
> +	intel_gsc_idle_msg_enable(engine);
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 07031e03f80c..20472eb15364 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -913,6 +913,10 @@
>   #define  MSG_IDLE_FW_MASK	REG_GENMASK(13, 9)
>   #define  MSG_IDLE_FW_SHIFT	9
>   
> +#define	RC_PSMI_CTRL_GSCCS	_MMIO(0x11a050)

Alignment still seems off? Other than that,

Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

> +#define	  IDLE_MSG_DISABLE	BIT(0)
> +#define PWRCTX_MAXCNT_GSCCS	_MMIO(0x11a054)
> +
>   #define FORCEWAKE_MEDIA_GEN9			_MMIO(0xa270)
>   #define FORCEWAKE_RENDER_GEN9			_MMIO(0xa278)
>
Nilawar, Badal Nov. 17, 2022, 6:45 a.m. UTC | #3
Hi Rodrigo,

On 15-11-2022 20:57, Rodrigo Vivi wrote:
> On Tue, Nov 15, 2022 at 07:14:40PM +0530, Badal Nilawar wrote:
>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>
>> By defaut idle mesaging is disabled for GSC CS so to unblock RC6
>> entry on media tile idle messaging need to be enabled.
>>
>> v2:
>>   - Fix review comments (Vinay)
>>   - Set GSC idle hysterisis to 5 us (Badal)
> 
> btw, no need for the cover letter in single/standalone patches.
> This history here is enough.
Sure, next revision I will take care of this.
> 
>>
>> Bspec: 71496
>>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
>>   drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 ++++
>>   2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>> index b0a4a2dbe3ee..5522885b2db0 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>> @@ -15,6 +15,22 @@
>>   #include "intel_rc6.h"
>>   #include "intel_ring.h"
>>   #include "shmem_utils.h"
>> +#include "intel_gt_regs.h"
>> +
>> +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine)
>> +{
>> +	struct drm_i915_private *i915 = engine->i915;
>> +
>> +	if (IS_METEORLAKE(i915) && engine->id == GSC0) {
>> +		intel_uncore_write(engine->gt->uncore,
>> +				   RC_PSMI_CTRL_GSCCS,
>> +				   _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
>> +		/* 5 us hysterisis */
> 
> typo
> 
>> +		intel_uncore_write(engine->gt->uncore,
>> +				   PWRCTX_MAXCNT_GSCCS,
>> +				   0xA);
> 
> you said 5 above, but used 10 here, why?
Bspec:71496 specifies 0xA = 5us.
> 
>> +	}
>> +}
>>   
>>   static void dbg_poison_ce(struct intel_context *ce)
>>   {
>> @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
>>   
>>   	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>   	intel_engine_init_heartbeat(engine);
>> +
>> +	intel_gsc_idle_msg_enable(engine);
>>   }
>>   
>>   /**
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> index 07031e03f80c..20472eb15364 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> @@ -913,6 +913,10 @@
>>   #define  MSG_IDLE_FW_MASK	REG_GENMASK(13, 9)
>>   #define  MSG_IDLE_FW_SHIFT	9
>>   
>> +#define	RC_PSMI_CTRL_GSCCS	_MMIO(0x11a050)
>> +#define	  IDLE_MSG_DISABLE	BIT(0)
> 
> REG_BIT should be preferred.
I will take care of this and above comments in next revision.

Regards,
Badal
> 
>> +#define PWRCTX_MAXCNT_GSCCS	_MMIO(0x11a054)
>> +
>>   #define FORCEWAKE_MEDIA_GEN9			_MMIO(0xa270)
>>   #define FORCEWAKE_RENDER_GEN9			_MMIO(0xa278)
>>   
>> -- 
>> 2.25.1
>>
Rodrigo Vivi Nov. 17, 2022, 10:14 p.m. UTC | #4
On Tue, Nov 15, 2022 at 07:14:40PM +0530, Badal Nilawar wrote:
> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> 
> By defaut idle mesaging is disabled for GSC CS so to unblock RC6
> entry on media tile idle messaging need to be enabled.
> 
> v2:
>  - Fix review comments (Vinay)
>  - Set GSC idle hysterisis to 5 us (Badal)
> 
> Bspec: 71496
> 
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 ++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index b0a4a2dbe3ee..5522885b2db0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -15,6 +15,22 @@
>  #include "intel_rc6.h"
>  #include "intel_ring.h"
>  #include "shmem_utils.h"
> +#include "intel_gt_regs.h"
> +
> +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine)
> +{
> +	struct drm_i915_private *i915 = engine->i915;
> +
> +	if (IS_METEORLAKE(i915) && engine->id == GSC0) {
> +		intel_uncore_write(engine->gt->uncore,
> +				   RC_PSMI_CTRL_GSCCS,
> +				   _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));

disable the disable? shouldn't be enable the disable?
1 = disable, no?

> +		/* 5 us hysterisis */

could you please mention here in the comment that 0xA = 5 us per spec?
I got confused again even though you had explained already...

BTW, how reliable that spec is? Because according to that same line
we should be setting the bit 16, not the bit 0 in the previous reg!

> +		intel_uncore_write(engine->gt->uncore,
> +				   PWRCTX_MAXCNT_GSCCS,
> +				   0xA);
> +	}
> +}
>  
>  static void dbg_poison_ce(struct intel_context *ce)
>  {
> @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
>  
>  	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>  	intel_engine_init_heartbeat(engine);
> +
> +	intel_gsc_idle_msg_enable(engine);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 07031e03f80c..20472eb15364 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -913,6 +913,10 @@
>  #define  MSG_IDLE_FW_MASK	REG_GENMASK(13, 9)
>  #define  MSG_IDLE_FW_SHIFT	9
>  
> +#define	RC_PSMI_CTRL_GSCCS	_MMIO(0x11a050)
> +#define	  IDLE_MSG_DISABLE	BIT(0)
> +#define PWRCTX_MAXCNT_GSCCS	_MMIO(0x11a054)
> +
>  #define FORCEWAKE_MEDIA_GEN9			_MMIO(0xa270)
>  #define FORCEWAKE_RENDER_GEN9			_MMIO(0xa278)
>  
> -- 
> 2.25.1
>
Nilawar, Badal Nov. 18, 2022, 4:05 a.m. UTC | #5
On 18-11-2022 03:44, Rodrigo Vivi wrote:
> On Tue, Nov 15, 2022 at 07:14:40PM +0530, Badal Nilawar wrote:
>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>
>> By defaut idle mesaging is disabled for GSC CS so to unblock RC6
>> entry on media tile idle messaging need to be enabled.
>>
>> v2:
>>   - Fix review comments (Vinay)
>>   - Set GSC idle hysterisis to 5 us (Badal)
>>
>> Bspec: 71496
>>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
>>   drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 ++++
>>   2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>> index b0a4a2dbe3ee..5522885b2db0 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>> @@ -15,6 +15,22 @@
>>   #include "intel_rc6.h"
>>   #include "intel_ring.h"
>>   #include "shmem_utils.h"
>> +#include "intel_gt_regs.h"
>> +
>> +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine)
>> +{
>> +	struct drm_i915_private *i915 = engine->i915;
>> +
>> +	if (IS_METEORLAKE(i915) && engine->id == GSC0) {
>> +		intel_uncore_write(engine->gt->uncore,
>> +				   RC_PSMI_CTRL_GSCCS,
>> +				   _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
> 
> disable the disable? shouldn't be enable the disable?
> 1 = disable, no?
> 
>> +		/* 5 us hysterisis */
> 
> could you please mention here in the comment that 0xA = 5 us per spec?
> I got confused again even though you had explained already...
Sure I will add the comment "0xA=5 us as per spec"
> 
> BTW, how reliable that spec is? Because according to that same line
> we should be setting the bit 16, not the bit 0 in the previous reg!
Bit 16 is mask bit. Bit 0 need to be cleared to enable Idle messaging.
Bit[0] = 1 Disable Idle Messaging / 0 Enable Idle Messaging.

Regards,
Badal
> 
>> +		intel_uncore_write(engine->gt->uncore,
>> +				   PWRCTX_MAXCNT_GSCCS,
>> +				   0xA);
>> +	}
>> +}
>>   
>>   static void dbg_poison_ce(struct intel_context *ce)
>>   {
>> @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
>>   
>>   	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>   	intel_engine_init_heartbeat(engine);
>> +
>> +	intel_gsc_idle_msg_enable(engine);
>>   }
>>   
>>   /**
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> index 07031e03f80c..20472eb15364 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> @@ -913,6 +913,10 @@
>>   #define  MSG_IDLE_FW_MASK	REG_GENMASK(13, 9)
>>   #define  MSG_IDLE_FW_SHIFT	9
>>   
>> +#define	RC_PSMI_CTRL_GSCCS	_MMIO(0x11a050)
>> +#define	  IDLE_MSG_DISABLE	BIT(0)
>> +#define PWRCTX_MAXCNT_GSCCS	_MMIO(0x11a054)
>> +
>>   #define FORCEWAKE_MEDIA_GEN9			_MMIO(0xa270)
>>   #define FORCEWAKE_RENDER_GEN9			_MMIO(0xa278)
>>   
>> -- 
>> 2.25.1
>>
Rodrigo Vivi Nov. 18, 2022, 2:32 p.m. UTC | #6
On Fri, Nov 18, 2022 at 09:35:41AM +0530, Nilawar, Badal wrote:
> 
> 
> On 18-11-2022 03:44, Rodrigo Vivi wrote:
> > On Tue, Nov 15, 2022 at 07:14:40PM +0530, Badal Nilawar wrote:
> > > From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > 
> > > By defaut idle mesaging is disabled for GSC CS so to unblock RC6
> > > entry on media tile idle messaging need to be enabled.
> > > 
> > > v2:
> > >   - Fix review comments (Vinay)
> > >   - Set GSC idle hysterisis to 5 us (Badal)
> > > 
> > > Bspec: 71496
> > > 
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
> > >   drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 ++++
> > >   2 files changed, 22 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > index b0a4a2dbe3ee..5522885b2db0 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > @@ -15,6 +15,22 @@
> > >   #include "intel_rc6.h"
> > >   #include "intel_ring.h"
> > >   #include "shmem_utils.h"
> > > +#include "intel_gt_regs.h"
> > > +
> > > +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine)
> > > +{
> > > +	struct drm_i915_private *i915 = engine->i915;
> > > +
> > > +	if (IS_METEORLAKE(i915) && engine->id == GSC0) {
> > > +		intel_uncore_write(engine->gt->uncore,
> > > +				   RC_PSMI_CTRL_GSCCS,
> > > +				   _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
> > 
> > disable the disable? shouldn't be enable the disable?
> > 1 = disable, no?

doh! the function here is enable so the disable of disable is the
right thing for this bit... Sorry for that.


> > 
> > > +		/* 5 us hysterisis */
> > 
> > could you please mention here in the comment that 0xA = 5 us per spec?
> > I got confused again even though you had explained already...
> Sure I will add the comment "0xA=5 us as per spec"

Thank you

> > 
> > BTW, how reliable that spec is? Because according to that same line
> > we should be setting the bit 16, not the bit 0 in the previous reg!
> Bit 16 is mask bit.

Okay, and we need to clear the bit 0. It makes sense. However the spec
seems to ask us to set the mask, but we are not. Should we?

Also from the register page:
"Must be set to modify corresponding bit in Bits 15:0. (All implemented bits)"

So it looks to me that we do need to set the bit16 to ensure that the
clear of the bit 0 is valid, otherwise this is a bogus call.

> Bit 0 need to be cleared to enable Idle messaging.
> Bit[0] = 1 Disable Idle Messaging / 0 Enable Idle Messaging.
> 
> Regards,
> Badal
> > 
> > > +		intel_uncore_write(engine->gt->uncore,
> > > +				   PWRCTX_MAXCNT_GSCCS,
> > > +				   0xA);
> > > +	}
> > > +}
> > >   static void dbg_poison_ce(struct intel_context *ce)
> > >   {
> > > @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
> > >   	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > >   	intel_engine_init_heartbeat(engine);
> > > +
> > > +	intel_gsc_idle_msg_enable(engine);
> > >   }
> > >   /**
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > index 07031e03f80c..20472eb15364 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > @@ -913,6 +913,10 @@
> > >   #define  MSG_IDLE_FW_MASK	REG_GENMASK(13, 9)
> > >   #define  MSG_IDLE_FW_SHIFT	9
> > > +#define	RC_PSMI_CTRL_GSCCS	_MMIO(0x11a050)
> > > +#define	  IDLE_MSG_DISABLE	BIT(0)
> > > +#define PWRCTX_MAXCNT_GSCCS	_MMIO(0x11a054)
> > > +
> > >   #define FORCEWAKE_MEDIA_GEN9			_MMIO(0xa270)
> > >   #define FORCEWAKE_RENDER_GEN9			_MMIO(0xa278)
> > > -- 
> > > 2.25.1
> > >
Rodrigo Vivi Nov. 18, 2022, 2:43 p.m. UTC | #7
On Fri, Nov 18, 2022 at 09:32:37AM -0500, Rodrigo Vivi wrote:
> On Fri, Nov 18, 2022 at 09:35:41AM +0530, Nilawar, Badal wrote:
> > 
> > 
> > On 18-11-2022 03:44, Rodrigo Vivi wrote:
> > > On Tue, Nov 15, 2022 at 07:14:40PM +0530, Badal Nilawar wrote:
> > > > From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > > 
> > > > By defaut idle mesaging is disabled for GSC CS so to unblock RC6
> > > > entry on media tile idle messaging need to be enabled.
> > > > 
> > > > v2:
> > > >   - Fix review comments (Vinay)
> > > >   - Set GSC idle hysterisis to 5 us (Badal)
> > > > 
> > > > Bspec: 71496
> > > > 
> > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
> > > >   drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 ++++
> > > >   2 files changed, 22 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > index b0a4a2dbe3ee..5522885b2db0 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > @@ -15,6 +15,22 @@
> > > >   #include "intel_rc6.h"
> > > >   #include "intel_ring.h"
> > > >   #include "shmem_utils.h"
> > > > +#include "intel_gt_regs.h"
> > > > +
> > > > +static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine)
> > > > +{
> > > > +	struct drm_i915_private *i915 = engine->i915;
> > > > +
> > > > +	if (IS_METEORLAKE(i915) && engine->id == GSC0) {
> > > > +		intel_uncore_write(engine->gt->uncore,
> > > > +				   RC_PSMI_CTRL_GSCCS,
> > > > +				   _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
> > > 
> > > disable the disable? shouldn't be enable the disable?
> > > 1 = disable, no?
> 
> doh! the function here is enable so the disable of disable is the
> right thing for this bit... Sorry for that.
> 
> 
> > > 
> > > > +		/* 5 us hysterisis */
> > > 
> > > could you please mention here in the comment that 0xA = 5 us per spec?
> > > I got confused again even though you had explained already...
> > Sure I will add the comment "0xA=5 us as per spec"
> 
> Thank you
> 
> > > 
> > > BTW, how reliable that spec is? Because according to that same line
> > > we should be setting the bit 16, not the bit 0 in the previous reg!
> > Bit 16 is mask bit.
> 
> Okay, and we need to clear the bit 0. It makes sense. However the spec
> seems to ask us to set the mask, but we are not. Should we?
> 
> Also from the register page:
> "Must be set to modify corresponding bit in Bits 15:0. (All implemented bits)"
> 
> So it looks to me that we do need to set the bit16 to ensure that the
> clear of the bit 0 is valid, otherwise this is a bogus call.

oh, I hate these macros hiding things up... bit 16 is there on that
_MASKED_BIT_DISABLED as you showed me offline...

So, with the comment to the 5usec added, feel free to use:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> > Bit 0 need to be cleared to enable Idle messaging.
> > Bit[0] = 1 Disable Idle Messaging / 0 Enable Idle Messaging.
> > 
> > Regards,
> > Badal
> > > 
> > > > +		intel_uncore_write(engine->gt->uncore,
> > > > +				   PWRCTX_MAXCNT_GSCCS,
> > > > +				   0xA);
> > > > +	}
> > > > +}
> > > >   static void dbg_poison_ce(struct intel_context *ce)
> > > >   {
> > > > @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
> > > >   	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > > >   	intel_engine_init_heartbeat(engine);
> > > > +
> > > > +	intel_gsc_idle_msg_enable(engine);
> > > >   }
> > > >   /**
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > index 07031e03f80c..20472eb15364 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > @@ -913,6 +913,10 @@
> > > >   #define  MSG_IDLE_FW_MASK	REG_GENMASK(13, 9)
> > > >   #define  MSG_IDLE_FW_SHIFT	9
> > > > +#define	RC_PSMI_CTRL_GSCCS	_MMIO(0x11a050)
> > > > +#define	  IDLE_MSG_DISABLE	BIT(0)
> > > > +#define PWRCTX_MAXCNT_GSCCS	_MMIO(0x11a054)
> > > > +
> > > >   #define FORCEWAKE_MEDIA_GEN9			_MMIO(0xa270)
> > > >   #define FORCEWAKE_RENDER_GEN9			_MMIO(0xa278)
> > > > -- 
> > > > 2.25.1
> > > >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index b0a4a2dbe3ee..5522885b2db0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -15,6 +15,22 @@ 
 #include "intel_rc6.h"
 #include "intel_ring.h"
 #include "shmem_utils.h"
+#include "intel_gt_regs.h"
+
+static void intel_gsc_idle_msg_enable(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *i915 = engine->i915;
+
+	if (IS_METEORLAKE(i915) && engine->id == GSC0) {
+		intel_uncore_write(engine->gt->uncore,
+				   RC_PSMI_CTRL_GSCCS,
+				   _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
+		/* 5 us hysterisis */
+		intel_uncore_write(engine->gt->uncore,
+				   PWRCTX_MAXCNT_GSCCS,
+				   0xA);
+	}
+}
 
 static void dbg_poison_ce(struct intel_context *ce)
 {
@@ -275,6 +291,8 @@  void intel_engine_init__pm(struct intel_engine_cs *engine)
 
 	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
 	intel_engine_init_heartbeat(engine);
+
+	intel_gsc_idle_msg_enable(engine);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 07031e03f80c..20472eb15364 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -913,6 +913,10 @@ 
 #define  MSG_IDLE_FW_MASK	REG_GENMASK(13, 9)
 #define  MSG_IDLE_FW_SHIFT	9
 
+#define	RC_PSMI_CTRL_GSCCS	_MMIO(0x11a050)
+#define	  IDLE_MSG_DISABLE	BIT(0)
+#define PWRCTX_MAXCNT_GSCCS	_MMIO(0x11a054)
+
 #define FORCEWAKE_MEDIA_GEN9			_MMIO(0xa270)
 #define FORCEWAKE_RENDER_GEN9			_MMIO(0xa278)