Re: [PATCH v2 1/1] drm/i915/mtl: Disable MC6 for MTL A step

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> Rodrigo Vivi
> Sent: Friday, March 10, 2023 4:43 PM
> To: Nilawar, Badal <badal.nilawar@xxxxxxxxx>
> Cc: Germano, Gregory F <gregory.f.germano@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx; Nandamuri, Srikanth
> <srikanth.nandamuri@xxxxxxxxx>; Chilmakuru, Hima B
> <hima.b.chilmakuru@xxxxxxxxx>
> Subject: Re:  [PATCH v2 1/1] drm/i915/mtl: Disable MC6 for MTL A
> step
> 
> On Fri, Mar 10, 2023 at 11:43:39AM +0530, Badal Nilawar wrote:
> > The Wa_14017073508 require to send Media Busy/Idle mailbox while
> > accessing Media tile. As of now it is getting handled while
> > __gt_unpark, __gt_park. But there are various corner cases where
> > forcewakes are taken without __gt_unpark i.e. without sending Busy
> > Mailbox especially during register reads. Forcewakes are taken without
> > busy mailbox leads to GPU HANG. So bringing mailbox calls under
> > forcewake calls are no feasible option as forcewake calls are atomic and
> mailbox calls are blocking.
> > The issue already fixed in B step so disabling MC6 on A step and
> > reverting previous commit which handles Wa_14017073508
> >
> > Fixes: 8f70f1ec587d ("drm/i915/mtl: Add Wa_14017073508 for SAMedia")
> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Signed-off-by: Badal Nilawar <badal.nilawar@xxxxxxxxx>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> 
> I confirm my rv-b here although this came from a combination of 2 patches
> and has a different msg.
> 
> However, depending on when we got the CI results back I won't be available
> for pushing it and I will be out next week. I hope another committer can push
> this to drm-intel-gt-next.
Pushed to drm-intel-gt-next.
Thanks for patch and Review.
Br,
Anshuman Gupta.
> 
> BTW, no need for cover letter in single patches.
> 
> Thanks for the patch,
> Rodrigo.
> 
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_pm.c     | 27 -----------------------
> >  drivers/gpu/drm/i915/gt/intel_rc6.c       |  8 +++++++
> >  drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 13 +----------
> >  drivers/gpu/drm/i915/i915_reg.h           |  9 --------
> >  4 files changed, 9 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> > b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> > index 85ae7dc079f2..e02cb90723ae 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> > @@ -20,31 +20,10 @@
> >  #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)
> >
> > -static void mtl_media_busy(struct intel_gt *gt) -{
> > -	/* Wa_14017073508: mtl */
> > -	if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
> > -	    gt->type == GT_MEDIA)
> > -		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_media_idle(struct intel_gt *gt) -{
> > -	/* Wa_14017073508: mtl */
> > -	if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
> > -	    gt->type == GT_MEDIA)
> > -		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); @@ -92,9 +71,6 @@
> static
> > int __gt_unpark(struct intel_wakeref *wf)
> >
> >  	GT_TRACE(gt, "\n");
> >
> > -	/* Wa_14017073508: mtl */
> > -	mtl_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 @@ -144,9 +120,6 @@ static int __gt_park(struct intel_wakeref
> *wf)
> >  	GEM_BUG_ON(!wakeref);
> >  	intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ,
> wakeref);
> >
> > -	/* Wa_14017073508: mtl */
> > -	mtl_media_idle(gt);
> > -
> >  	return 0;
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c
> > b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > index 5c91622dfca4..f4150f61f39c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > @@ -486,6 +486,7 @@ static bool bxt_check_bios_rc6_setup(struct
> > intel_rc6 *rc6)  static bool rc6_supported(struct intel_rc6 *rc6)  {
> >  	struct drm_i915_private *i915 = rc6_to_i915(rc6);
> > +	struct intel_gt *gt = rc6_to_gt(rc6);
> >
> >  	if (!HAS_RC6(i915))
> >  		return false;
> > @@ -502,6 +503,13 @@ static bool rc6_supported(struct intel_rc6 *rc6)
> >  		return false;
> >  	}
> >
> > +	if (IS_MTL_MEDIA_STEP(gt->i915, STEP_A0, STEP_B0) &&
> > +	    gt->type == GT_MEDIA) {
> > +		drm_notice(&i915->drm,
> > +			   "Media RC6 disabled on A step\n");
> > +		return false;
> > +	}
> > +
> >  	return true;
> >  }
> >
> > 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 fcf51614f9a4..1adec6de223c 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> > @@ -12,20 +12,9 @@
> >
> >  static bool __guc_rc_supported(struct intel_guc *guc)  {
> > -	struct intel_gt *gt = guc_to_gt(guc);
> > -
> > -	/*
> > -	 * Wa_14017073508: 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(gt->i915) >= 12;
> > +		GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12;
> >  }
> >
> >  static bool __guc_rc_selected(struct intel_guc *guc) diff --git
> > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index b9b6d46dfe1b..9db6b3f06a74 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6460,15 +6460,6 @@
> >  /*   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
> >




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux