diff mbox series

drm/i915/mtl: Add MC6 Wa_14017210380 for SAMedia

Message ID 20221028192935.1458271-1-badal.nilawar@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/mtl: Add MC6 Wa_14017210380 for SAMedia | expand

Commit Message

Nilawar, Badal Oct. 28, 2022, 7:29 p.m. UTC
This workaround is added for Media Tile of MTL A step. It is to help
pcode workaround which handles the hardware bug seen on CXL splitter
during package C2/C3 transitins due to MC6 entry/exit. As a part of
workaround pcode expect kmd to send mailbox message "media busy" when
components of Media tile is in use and "media not busy" when not in use.
As per workaround description gucrc need to be disabled so enabled
host based RC for Media tile.

HSD: 14017210380

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_pm.c     | 33 +++++++++++++++++++++++
 drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 13 ++++++++-
 drivers/gpu/drm/i915/i915_drv.h           |  4 +++
 drivers/gpu/drm/i915/i915_reg.h           |  9 +++++++
 4 files changed, 58 insertions(+), 1 deletion(-)

Comments

Rodrigo Vivi Oct. 31, 2022, 9:49 a.m. UTC | #1
On Sat, Oct 29, 2022 at 12:59:35AM +0530, Badal Nilawar wrote:
> This workaround is added for Media Tile of MTL A step. It is to help
> pcode workaround which handles the hardware bug seen on CXL splitter
> during package C2/C3 transitins due to MC6 entry/exit. As a part of
> workaround pcode expect kmd to send mailbox message "media busy" when
> components of Media tile is in use and "media not busy" when not in use.
> As per workaround description gucrc need to be disabled so enabled
> host based RC for Media tile.
> 
> HSD: 14017210380
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c     | 33 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 13 ++++++++-
>  drivers/gpu/drm/i915/i915_drv.h           |  4 +++
>  drivers/gpu/drm/i915/i915_reg.h           |  9 +++++++
>  4 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index f553e2173bda..398dbeb298ca 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -19,10 +19,37 @@
>  #include "intel_rc6.h"
>  #include "intel_rps.h"
>  #include "intel_wakeref.h"
> +#include "intel_pcode.h"
>  #include "pxp/intel_pxp_pm.h"
>  
>  #define I915_GT_SUSPEND_IDLE_TIMEOUT (HZ / 2)
>  
> +/*
> + * Wa_14017210380: mtl
> + */
> +
> +static bool mtl_needs_media_mc6_wa(struct intel_gt *gt)

+Ashutosh since we recently discussed about the term "MC6".

in that discussion we have concluded to not introduce a new term
since it is not used in any kind of spec and might only bring
more doubts then answers, although "Render" in the media gt
makes no sense as well.

In the end, most of the code is common and is called as rc6.
so we should maybe s/mc6/media_rc6 here?

The rest of the patch looks good to me. But we need to check the IGT
failure on a pm related test that failed... just to be sure.

> +{
> +	return (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
> +		gt->type == GT_MEDIA);
> +}
> +
> +static void mtl_mc6_wa_media_busy(struct intel_gt *gt)
> +{
> +	if (mtl_needs_media_mc6_wa(gt))
> +		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
> +				  PCODE_MBOX_GT_STATE_MEDIA_BUSY,
> +				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
> +}
> +
> +static void mtl_mc6_wa_media_not_busy(struct intel_gt *gt)
> +{
> +	if (mtl_needs_media_mc6_wa(gt))
> +		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
> +				  PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY,
> +				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
> +}
> +
>  static void user_forcewake(struct intel_gt *gt, bool suspend)
>  {
>  	int count = atomic_read(&gt->user_wakeref);
> @@ -70,6 +97,9 @@ static int __gt_unpark(struct intel_wakeref *wf)
>  
>  	GT_TRACE(gt, "\n");
>  
> +	/* Wa_14017210380: mtl */
> +	mtl_mc6_wa_media_busy(gt);
> +
>  	/*
>  	 * It seems that the DMC likes to transition between the DC states a lot
>  	 * when there are no connected displays (no active power domains) during
> @@ -119,6 +149,9 @@ static int __gt_park(struct intel_wakeref *wf)
>  	GEM_BUG_ON(!wakeref);
>  	intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref);
>  
> +	/* Wa_14017210380: mtl */
> +	mtl_mc6_wa_media_not_busy(gt);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> index 8f8dd05835c5..cc6356ff84a5 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> @@ -11,9 +11,20 @@
>  
>  static bool __guc_rc_supported(struct intel_guc *guc)
>  {
> +	struct intel_gt *gt = guc_to_gt(guc);
> +
> +	/*
> +	 * Wa_14017210380: mtl
> +	 * Do not enable gucrc to avoid additional interrupts which
> +	 * may disrupt pcode wa.
> +	 */
> +	if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
> +	    gt->type == GT_MEDIA)
> +		return false;
> +
>  	/* GuC RC is unavailable for pre-Gen12 */
>  	return guc->submission_supported &&
> -		GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12;
> +		GRAPHICS_VER(gt->i915) >= 12;
>  }
>  
>  static bool __guc_rc_selected(struct intel_guc *guc)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 05b3300cc4ed..659b92382ff2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -740,6 +740,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define IS_XEHPSDV_GRAPHICS_STEP(__i915, since, until) \
>  	(IS_XEHPSDV(__i915) && IS_GRAPHICS_STEP(__i915, since, until))
>  
> +#define IS_MTL_GRAPHICS_STEP(__i915, variant, since, until) \
> +	(IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \
> +	 IS_GRAPHICS_STEP(__i915, since, until))
> +
>  /*
>   * DG2 hardware steppings are a bit unusual.  The hardware design was forked to
>   * create three variants (G10, G11, and G12) which each have distinct
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1c0da50c0dc7..abe62cea083d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6678,6 +6678,15 @@
>  /*   XEHP_PCODE_FREQUENCY_CONFIG param2 */
>  #define     PCODE_MBOX_DOMAIN_NONE		0x0
>  #define     PCODE_MBOX_DOMAIN_MEDIAFF		0x3
> +
> +/* Wa_14017210380: mtl */
> +#define   PCODE_MBOX_GT_STATE			0x50
> +/* sub-commands (param1) */
> +#define     PCODE_MBOX_GT_STATE_MEDIA_BUSY	0x1
> +#define     PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY	0x2
> +/* param2 */
> +#define     PCODE_MBOX_GT_STATE_DOMAIN_MEDIA	0x1
> +
>  #define GEN6_PCODE_DATA				_MMIO(0x138128)
>  #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
>  #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
> -- 
> 2.25.1
>
Nilawar, Badal Oct. 31, 2022, 1:52 p.m. UTC | #2
Hi Rodrigo,

On 31-10-2022 15:19, Rodrigo Vivi wrote:
> On Sat, Oct 29, 2022 at 12:59:35AM +0530, Badal Nilawar wrote:
>> This workaround is added for Media Tile of MTL A step. It is to help
>> pcode workaround which handles the hardware bug seen on CXL splitter
>> during package C2/C3 transitins due to MC6 entry/exit. As a part of
>> workaround pcode expect kmd to send mailbox message "media busy" when
>> components of Media tile is in use and "media not busy" when not in use.
>> As per workaround description gucrc need to be disabled so enabled
>> host based RC for Media tile.
>>
>> HSD: 14017210380
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_gt_pm.c     | 33 +++++++++++++++++++++++
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 13 ++++++++-
>>   drivers/gpu/drm/i915/i915_drv.h           |  4 +++
>>   drivers/gpu/drm/i915/i915_reg.h           |  9 +++++++
>>   4 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>> index f553e2173bda..398dbeb298ca 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>> @@ -19,10 +19,37 @@
>>   #include "intel_rc6.h"
>>   #include "intel_rps.h"
>>   #include "intel_wakeref.h"
>> +#include "intel_pcode.h"
>>   #include "pxp/intel_pxp_pm.h"
>>   
>>   #define I915_GT_SUSPEND_IDLE_TIMEOUT (HZ / 2)
>>   
>> +/*
>> + * Wa_14017210380: mtl
>> + */
>> +
>> +static bool mtl_needs_media_mc6_wa(struct intel_gt *gt)
> 
> +Ashutosh since we recently discussed about the term "MC6".
> 
> in that discussion we have concluded to not introduce a new term
> since it is not used in any kind of spec and might only bring
> more doubts then answers, although "Render" in the media gt
> makes no sense as well.
> 
> In the end, most of the code is common and is called as rc6.
> so we should maybe s/mc6/media_rc6 here?
Sure I will make above change and resend the patch.
> 
> The rest of the patch looks good to me. But we need to check the IGT
> failure on a pm related test that failed... just to be sure.
Failures reported in IGT so far are not related to this change.

Regards,
Badal
> 
>> +{
>> +	return (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
>> +		gt->type == GT_MEDIA);
>> +}
>> +
>> +static void mtl_mc6_wa_media_busy(struct intel_gt *gt)
>> +{
>> +	if (mtl_needs_media_mc6_wa(gt))
>> +		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
>> +				  PCODE_MBOX_GT_STATE_MEDIA_BUSY,
>> +				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
>> +}
>> +
>> +static void mtl_mc6_wa_media_not_busy(struct intel_gt *gt)
>> +{
>> +	if (mtl_needs_media_mc6_wa(gt))
>> +		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
>> +				  PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY,
>> +				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
>> +}
>> +
>>   static void user_forcewake(struct intel_gt *gt, bool suspend)
>>   {
>>   	int count = atomic_read(&gt->user_wakeref);
>> @@ -70,6 +97,9 @@ static int __gt_unpark(struct intel_wakeref *wf)
>>   
>>   	GT_TRACE(gt, "\n");
>>   
>> +	/* Wa_14017210380: mtl */
>> +	mtl_mc6_wa_media_busy(gt);
>> +
>>   	/*
>>   	 * It seems that the DMC likes to transition between the DC states a lot
>>   	 * when there are no connected displays (no active power domains) during
>> @@ -119,6 +149,9 @@ static int __gt_park(struct intel_wakeref *wf)
>>   	GEM_BUG_ON(!wakeref);
>>   	intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref);
>>   
>> +	/* Wa_14017210380: mtl */
>> +	mtl_mc6_wa_media_not_busy(gt);
>> +
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
>> index 8f8dd05835c5..cc6356ff84a5 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
>> @@ -11,9 +11,20 @@
>>   
>>   static bool __guc_rc_supported(struct intel_guc *guc)
>>   {
>> +	struct intel_gt *gt = guc_to_gt(guc);
>> +
>> +	/*
>> +	 * Wa_14017210380: mtl
>> +	 * Do not enable gucrc to avoid additional interrupts which
>> +	 * may disrupt pcode wa.
>> +	 */
>> +	if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
>> +	    gt->type == GT_MEDIA)
>> +		return false;
>> +
>>   	/* GuC RC is unavailable for pre-Gen12 */
>>   	return guc->submission_supported &&
>> -		GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12;
>> +		GRAPHICS_VER(gt->i915) >= 12;
>>   }
>>   
>>   static bool __guc_rc_selected(struct intel_guc *guc)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 05b3300cc4ed..659b92382ff2 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -740,6 +740,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>>   #define IS_XEHPSDV_GRAPHICS_STEP(__i915, since, until) \
>>   	(IS_XEHPSDV(__i915) && IS_GRAPHICS_STEP(__i915, since, until))
>>   
>> +#define IS_MTL_GRAPHICS_STEP(__i915, variant, since, until) \
>> +	(IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \
>> +	 IS_GRAPHICS_STEP(__i915, since, until))
>> +
>>   /*
>>    * DG2 hardware steppings are a bit unusual.  The hardware design was forked to
>>    * create three variants (G10, G11, and G12) which each have distinct
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 1c0da50c0dc7..abe62cea083d 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6678,6 +6678,15 @@
>>   /*   XEHP_PCODE_FREQUENCY_CONFIG param2 */
>>   #define     PCODE_MBOX_DOMAIN_NONE		0x0
>>   #define     PCODE_MBOX_DOMAIN_MEDIAFF		0x3
>> +
>> +/* Wa_14017210380: mtl */
>> +#define   PCODE_MBOX_GT_STATE			0x50
>> +/* sub-commands (param1) */
>> +#define     PCODE_MBOX_GT_STATE_MEDIA_BUSY	0x1
>> +#define     PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY	0x2
>> +/* param2 */
>> +#define     PCODE_MBOX_GT_STATE_DOMAIN_MEDIA	0x1
>> +
>>   #define GEN6_PCODE_DATA				_MMIO(0x138128)
>>   #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
>>   #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
>> -- 
>> 2.25.1
>>
Matt Roper Nov. 1, 2022, 3:17 p.m. UTC | #3
On Sat, Oct 29, 2022 at 12:59:35AM +0530, Badal Nilawar wrote:
> This workaround is added for Media Tile of MTL A step. It is to help
> pcode workaround which handles the hardware bug seen on CXL splitter
> during package C2/C3 transitins due to MC6 entry/exit. As a part of
> workaround pcode expect kmd to send mailbox message "media busy" when
> components of Media tile is in use and "media not busy" when not in use.
> As per workaround description gucrc need to be disabled so enabled
> host based RC for Media tile.
> 
> HSD: 14017210380
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c     | 33 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 13 ++++++++-
>  drivers/gpu/drm/i915/i915_drv.h           |  4 +++
>  drivers/gpu/drm/i915/i915_reg.h           |  9 +++++++
>  4 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index f553e2173bda..398dbeb298ca 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -19,10 +19,37 @@
>  #include "intel_rc6.h"
>  #include "intel_rps.h"
>  #include "intel_wakeref.h"
> +#include "intel_pcode.h"
>  #include "pxp/intel_pxp_pm.h"
>  
>  #define I915_GT_SUSPEND_IDLE_TIMEOUT (HZ / 2)
>  
> +/*
> + * Wa_14017210380: mtl
> + */

This doesn't appear to be a valid workaround number; workaround numbers
are always supposed to be the "lineage" numbers from the workaround
database.  Wa_14017073508 seems to be related; is that the one you're
implementing?

> +
> +static bool mtl_needs_media_mc6_wa(struct intel_gt *gt)

Drive-by comment:  names like this aren't great since even though
there's only one "media MC6" workaround today, that may not be true in
the future.


Matt

> +{
> +	return (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
> +		gt->type == GT_MEDIA);
> +}
> +
> +static void mtl_mc6_wa_media_busy(struct intel_gt *gt)
> +{
> +	if (mtl_needs_media_mc6_wa(gt))
> +		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
> +				  PCODE_MBOX_GT_STATE_MEDIA_BUSY,
> +				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
> +}
> +
> +static void mtl_mc6_wa_media_not_busy(struct intel_gt *gt)
> +{
> +	if (mtl_needs_media_mc6_wa(gt))
> +		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
> +				  PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY,
> +				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
> +}
> +
>  static void user_forcewake(struct intel_gt *gt, bool suspend)
>  {
>  	int count = atomic_read(&gt->user_wakeref);
> @@ -70,6 +97,9 @@ static int __gt_unpark(struct intel_wakeref *wf)
>  
>  	GT_TRACE(gt, "\n");
>  
> +	/* Wa_14017210380: mtl */
> +	mtl_mc6_wa_media_busy(gt);
> +
>  	/*
>  	 * It seems that the DMC likes to transition between the DC states a lot
>  	 * when there are no connected displays (no active power domains) during
> @@ -119,6 +149,9 @@ static int __gt_park(struct intel_wakeref *wf)
>  	GEM_BUG_ON(!wakeref);
>  	intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref);
>  
> +	/* Wa_14017210380: mtl */
> +	mtl_mc6_wa_media_not_busy(gt);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> index 8f8dd05835c5..cc6356ff84a5 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> @@ -11,9 +11,20 @@
>  
>  static bool __guc_rc_supported(struct intel_guc *guc)
>  {
> +	struct intel_gt *gt = guc_to_gt(guc);
> +
> +	/*
> +	 * Wa_14017210380: mtl
> +	 * Do not enable gucrc to avoid additional interrupts which
> +	 * may disrupt pcode wa.
> +	 */
> +	if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
> +	    gt->type == GT_MEDIA)
> +		return false;
> +
>  	/* GuC RC is unavailable for pre-Gen12 */
>  	return guc->submission_supported &&
> -		GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12;
> +		GRAPHICS_VER(gt->i915) >= 12;
>  }
>  
>  static bool __guc_rc_selected(struct intel_guc *guc)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 05b3300cc4ed..659b92382ff2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -740,6 +740,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define IS_XEHPSDV_GRAPHICS_STEP(__i915, since, until) \
>  	(IS_XEHPSDV(__i915) && IS_GRAPHICS_STEP(__i915, since, until))
>  
> +#define IS_MTL_GRAPHICS_STEP(__i915, variant, since, until) \
> +	(IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \
> +	 IS_GRAPHICS_STEP(__i915, since, until))
> +
>  /*
>   * DG2 hardware steppings are a bit unusual.  The hardware design was forked to
>   * create three variants (G10, G11, and G12) which each have distinct
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1c0da50c0dc7..abe62cea083d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6678,6 +6678,15 @@
>  /*   XEHP_PCODE_FREQUENCY_CONFIG param2 */
>  #define     PCODE_MBOX_DOMAIN_NONE		0x0
>  #define     PCODE_MBOX_DOMAIN_MEDIAFF		0x3
> +
> +/* Wa_14017210380: mtl */
> +#define   PCODE_MBOX_GT_STATE			0x50
> +/* sub-commands (param1) */
> +#define     PCODE_MBOX_GT_STATE_MEDIA_BUSY	0x1
> +#define     PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY	0x2
> +/* param2 */
> +#define     PCODE_MBOX_GT_STATE_DOMAIN_MEDIA	0x1
> +
>  #define GEN6_PCODE_DATA				_MMIO(0x138128)
>  #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
>  #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
> -- 
> 2.25.1
>
Nilawar, Badal Nov. 1, 2022, 4:26 p.m. UTC | #4
Hi Matt,

On 01-11-2022 20:47, Matt Roper wrote:
> On Sat, Oct 29, 2022 at 12:59:35AM +0530, Badal Nilawar wrote:
>> This workaround is added for Media Tile of MTL A step. It is to help
>> pcode workaround which handles the hardware bug seen on CXL splitter
>> during package C2/C3 transitins due to MC6 entry/exit. As a part of
>> workaround pcode expect kmd to send mailbox message "media busy" when
>> components of Media tile is in use and "media not busy" when not in use.
>> As per workaround description gucrc need to be disabled so enabled
>> host based RC for Media tile.
>>
>> HSD: 14017210380
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_gt_pm.c     | 33 +++++++++++++++++++++++
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 13 ++++++++-
>>   drivers/gpu/drm/i915/i915_drv.h           |  4 +++
>>   drivers/gpu/drm/i915/i915_reg.h           |  9 +++++++
>>   4 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>> index f553e2173bda..398dbeb298ca 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>> @@ -19,10 +19,37 @@
>>   #include "intel_rc6.h"
>>   #include "intel_rps.h"
>>   #include "intel_wakeref.h"
>> +#include "intel_pcode.h"
>>   #include "pxp/intel_pxp_pm.h"
>>   
>>   #define I915_GT_SUSPEND_IDLE_TIMEOUT (HZ / 2)
>>   
>> +/*
>> + * Wa_14017210380: mtl
>> + */
> 
> This doesn't appear to be a valid workaround number; workaround numbers
> are always supposed to be the "lineage" numbers from the workaround
> database.  Wa_14017073508 seems to be related; is that the one you're
> implementing?
Thanks for pointing out I will correct the workaround number in next 
revision.
> 
>> +
>> +static bool mtl_needs_media_mc6_wa(struct intel_gt *gt)
> 
> Drive-by comment:  names like this aren't great since even though
> there's only one "media MC6" workaround today, that may not be true in
> the future.
I think its better to drop this function and replace its calls below 
with (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) && gt->type == 
GT_MEDIA)

Regards,
Badal
> 
> 
> Matt
> 
>> +{
>> +	return (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
>> +		gt->type == GT_MEDIA);
>> +}
>> +
>> +static void mtl_mc6_wa_media_busy(struct intel_gt *gt)
>> +{
>> +	if (mtl_needs_media_mc6_wa(gt))
>> +		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
>> +				  PCODE_MBOX_GT_STATE_MEDIA_BUSY,
>> +				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
>> +}
>> +
>> +static void mtl_mc6_wa_media_not_busy(struct intel_gt *gt)
>> +{
>> +	if (mtl_needs_media_mc6_wa(gt))
>> +		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
>> +				  PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY,
>> +				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
>> +}
>> +
>>   static void user_forcewake(struct intel_gt *gt, bool suspend)
>>   {
>>   	int count = atomic_read(&gt->user_wakeref);
>> @@ -70,6 +97,9 @@ static int __gt_unpark(struct intel_wakeref *wf)
>>   
>>   	GT_TRACE(gt, "\n");
>>   
>> +	/* Wa_14017210380: mtl */
>> +	mtl_mc6_wa_media_busy(gt);
>> +
>>   	/*
>>   	 * It seems that the DMC likes to transition between the DC states a lot
>>   	 * when there are no connected displays (no active power domains) during
>> @@ -119,6 +149,9 @@ static int __gt_park(struct intel_wakeref *wf)
>>   	GEM_BUG_ON(!wakeref);
>>   	intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref);
>>   
>> +	/* Wa_14017210380: mtl */
>> +	mtl_mc6_wa_media_not_busy(gt);
>> +
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
>> index 8f8dd05835c5..cc6356ff84a5 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
>> @@ -11,9 +11,20 @@
>>   
>>   static bool __guc_rc_supported(struct intel_guc *guc)
>>   {
>> +	struct intel_gt *gt = guc_to_gt(guc);
>> +
>> +	/*
>> +	 * Wa_14017210380: mtl
>> +	 * Do not enable gucrc to avoid additional interrupts which
>> +	 * may disrupt pcode wa.
>> +	 */
>> +	if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
>> +	    gt->type == GT_MEDIA)
>> +		return false;
>> +
>>   	/* GuC RC is unavailable for pre-Gen12 */
>>   	return guc->submission_supported &&
>> -		GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12;
>> +		GRAPHICS_VER(gt->i915) >= 12;
>>   }
>>   
>>   static bool __guc_rc_selected(struct intel_guc *guc)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 05b3300cc4ed..659b92382ff2 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -740,6 +740,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>>   #define IS_XEHPSDV_GRAPHICS_STEP(__i915, since, until) \
>>   	(IS_XEHPSDV(__i915) && IS_GRAPHICS_STEP(__i915, since, until))
>>   
>> +#define IS_MTL_GRAPHICS_STEP(__i915, variant, since, until) \
>> +	(IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \
>> +	 IS_GRAPHICS_STEP(__i915, since, until))
>> +
>>   /*
>>    * DG2 hardware steppings are a bit unusual.  The hardware design was forked to
>>    * create three variants (G10, G11, and G12) which each have distinct
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 1c0da50c0dc7..abe62cea083d 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6678,6 +6678,15 @@
>>   /*   XEHP_PCODE_FREQUENCY_CONFIG param2 */
>>   #define     PCODE_MBOX_DOMAIN_NONE		0x0
>>   #define     PCODE_MBOX_DOMAIN_MEDIAFF		0x3
>> +
>> +/* Wa_14017210380: mtl */
>> +#define   PCODE_MBOX_GT_STATE			0x50
>> +/* sub-commands (param1) */
>> +#define     PCODE_MBOX_GT_STATE_MEDIA_BUSY	0x1
>> +#define     PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY	0x2
>> +/* param2 */
>> +#define     PCODE_MBOX_GT_STATE_DOMAIN_MEDIA	0x1
>> +
>>   #define GEN6_PCODE_DATA				_MMIO(0x138128)
>>   #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
>>   #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
>> -- 
>> 2.25.1
>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index f553e2173bda..398dbeb298ca 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -19,10 +19,37 @@ 
 #include "intel_rc6.h"
 #include "intel_rps.h"
 #include "intel_wakeref.h"
+#include "intel_pcode.h"
 #include "pxp/intel_pxp_pm.h"
 
 #define I915_GT_SUSPEND_IDLE_TIMEOUT (HZ / 2)
 
+/*
+ * Wa_14017210380: mtl
+ */
+
+static bool mtl_needs_media_mc6_wa(struct intel_gt *gt)
+{
+	return (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
+		gt->type == GT_MEDIA);
+}
+
+static void mtl_mc6_wa_media_busy(struct intel_gt *gt)
+{
+	if (mtl_needs_media_mc6_wa(gt))
+		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
+				  PCODE_MBOX_GT_STATE_MEDIA_BUSY,
+				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
+}
+
+static void mtl_mc6_wa_media_not_busy(struct intel_gt *gt)
+{
+	if (mtl_needs_media_mc6_wa(gt))
+		snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
+				  PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY,
+				  PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
+}
+
 static void user_forcewake(struct intel_gt *gt, bool suspend)
 {
 	int count = atomic_read(&gt->user_wakeref);
@@ -70,6 +97,9 @@  static int __gt_unpark(struct intel_wakeref *wf)
 
 	GT_TRACE(gt, "\n");
 
+	/* Wa_14017210380: mtl */
+	mtl_mc6_wa_media_busy(gt);
+
 	/*
 	 * It seems that the DMC likes to transition between the DC states a lot
 	 * when there are no connected displays (no active power domains) during
@@ -119,6 +149,9 @@  static int __gt_park(struct intel_wakeref *wf)
 	GEM_BUG_ON(!wakeref);
 	intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref);
 
+	/* Wa_14017210380: mtl */
+	mtl_mc6_wa_media_not_busy(gt);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
index 8f8dd05835c5..cc6356ff84a5 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
@@ -11,9 +11,20 @@ 
 
 static bool __guc_rc_supported(struct intel_guc *guc)
 {
+	struct intel_gt *gt = guc_to_gt(guc);
+
+	/*
+	 * Wa_14017210380: mtl
+	 * Do not enable gucrc to avoid additional interrupts which
+	 * may disrupt pcode wa.
+	 */
+	if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
+	    gt->type == GT_MEDIA)
+		return false;
+
 	/* GuC RC is unavailable for pre-Gen12 */
 	return guc->submission_supported &&
-		GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12;
+		GRAPHICS_VER(gt->i915) >= 12;
 }
 
 static bool __guc_rc_selected(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 05b3300cc4ed..659b92382ff2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -740,6 +740,10 @@  IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define IS_XEHPSDV_GRAPHICS_STEP(__i915, since, until) \
 	(IS_XEHPSDV(__i915) && IS_GRAPHICS_STEP(__i915, since, until))
 
+#define IS_MTL_GRAPHICS_STEP(__i915, variant, since, until) \
+	(IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \
+	 IS_GRAPHICS_STEP(__i915, since, until))
+
 /*
  * DG2 hardware steppings are a bit unusual.  The hardware design was forked to
  * create three variants (G10, G11, and G12) which each have distinct
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1c0da50c0dc7..abe62cea083d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6678,6 +6678,15 @@ 
 /*   XEHP_PCODE_FREQUENCY_CONFIG param2 */
 #define     PCODE_MBOX_DOMAIN_NONE		0x0
 #define     PCODE_MBOX_DOMAIN_MEDIAFF		0x3
+
+/* Wa_14017210380: mtl */
+#define   PCODE_MBOX_GT_STATE			0x50
+/* sub-commands (param1) */
+#define     PCODE_MBOX_GT_STATE_MEDIA_BUSY	0x1
+#define     PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY	0x2
+/* param2 */
+#define     PCODE_MBOX_GT_STATE_DOMAIN_MEDIA	0x1
+
 #define GEN6_PCODE_DATA				_MMIO(0x138128)
 #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
 #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16