> -----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(>->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
> >
