Re: [Intel-gfx] [PATCH] drm/i915/mtl: do not enable render power-gating on MTL

[Rodrigo Vivi <rodrigo.vivi@kernel.org>]

On Wed, May 17, 2023 at 05:36:33PM +0200, Das, Nirmoy wrote:
> 
> On 5/17/2023 5:25 PM, Vivi, Rodrigo wrote:
> > On Wed, 2023-05-17 at 17:12 +0200, Das, Nirmoy wrote:
> > > On 5/17/2023 3:59 PM, Andrzej Hajda wrote:
> > > > Multiple CI tests fails with forcewake ack timeouts
> > > > if render power gating is enabled.
> > > > BSpec 52698 clearly states it should be 0 for MTL.
> > > > 
> > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4983
> > > > Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> > > > ---
> > > >    drivers/gpu/drm/i915/gt/intel_rc6.c | 5 +++--
> > > >    1 file changed, 3 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..ebb2373dd73640 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > > > @@ -117,8 +117,9 @@ 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 */
> > > Nice catch!
> > Indeed! What a mess in the workaround database.
> > It is telling us that no_impact on MTL SKUs while we clearly needs
> > that. I tried to reopen that and get that fixed in the hsds.
> > 
> > 
> > >   instead of bspec you could add Wa_14012655556.
> > not actually.
> > 16011777198 is the right lineage number for 14012655556.
> > Besides, 14012655556 is for DG2 anyway.
> > 
> > Let's keep the way Adrzej put with the BSPec reference besides the
> > lineage.
> 
> Makes sense, didn't realize 14012655556  is much older.
> 
> Thanks!
> 
> > 
> > > 
> > > > +       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: 01d3dd92d1b71421f6ee85e1bea829e0a917d979
> > > > change-id: 20230517-mtl_disable_render_pg-b9f9f1567f9e
> > > ^ unwanted artifacts ?   Otherwise this looks good to me.
> > > 
> > > Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
> > with the artifacts removed:
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Folks, please do not merge this patch.
At least not as it is right now...

We need to root cause this. The hw bug related to this workaround
was really fixed and this workaround should not be needed in MTL.

We need to find the root cause instead of masking it here.
Or at least merge as temporary FIXME/XXX and then work to
get the root cause...

The BSPec will get updated to remove the MTL mention there.

Sorry about that.

> > 
> > 
> > > > Best regards,