Re: [PATCH] drm/i915/rc6: Disable RPG during workload execution

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Oct 23, 2024 at 11:03:57AM +0530, Nilawar, Badal wrote:
> 
> 
> On 22-10-2024 22:39, Rodrigo Vivi wrote:
> > On Tue, Oct 22, 2024 at 03:28:43PM +0200, Andi Shyti wrote:
> > > Hi Badal,
> > > 
> > > On Tue, Oct 22, 2024 at 06:52:26PM +0530, Badal Nilawar wrote:
> > > > Encountering forcewake errors related to render power gating;
> > > 
> > > Can you please expand your explanation here?
> > 
> > yeap. More explanation please. All platforms? really?
> 
> We are seeing Render forcewake timeouts on ADLP, ADLM, ADLN, TWL, DG1, rpl.

Is this a regression? or a new issue?

Is this happening with Xe on these platforms? or i915 only?

> Issue disappears after disabling RPG. Instead of fully disabling RPG I am
> disabling it during active submissions i.e. during unpark.
> For MTL and ARL RPG is already disabled permanently.

uhm. Interesting. Why that is disabled on these platforms? perhaps we should be
doing the same for all GuC enabled platforms?

> 
> Impact of doing this change should be performance improvement so kept for
> all platform otherwise I will add platform check.

it could cause power consumption and battery life regressions. Better to filter
per platform.

> 
> This is the issue https://gitlab.freedesktop.org/drm/intel/issues/9413. Will
> add it in commit message.

Next time please include the relevant links in the commit msg.

Thanks a lot for the info and for working on this,
Rodrigo.

> 
> Regards,
> Badal
> 
> > 
> > > 
> > > > therefore, disable it during workload execution.
> > > 
> > > ... and here.
> > > 
> > > > Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> > > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/gt/intel_rc6.c       | 18 +++++++++++++++++-
> > > >   drivers/gpu/drm/i915/gt/intel_rc6_types.h |  1 +
> > > >   2 files changed, 18 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > > > index c864d101faf9..459394ab5258 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > > > @@ -140,6 +140,7 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6)
> > > >   					      VDN_MFX_POWERGATE_ENABLE(i));
> > > >   	}
> > > > +	rc6->pg_enable = pg_enable;
> > > 
> > > this looks borderline racy, it's fine only because this function
> > > is called during resume which normally runs in atomic context.
> > > 
> > > >   	intel_uncore_write_fw(uncore, GEN9_PG_ENABLE, pg_enable);
> > > >   }
> > > > @@ -572,8 +573,11 @@ static void __intel_rc6_disable(struct intel_rc6 *rc6)
> > > >   	intel_guc_rc_disable(gt_to_guc(gt));
> > > >   	intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
> > > > -	if (GRAPHICS_VER(i915) >= 9)
> > > > +	if (GRAPHICS_VER(i915) >= 9) {
> > > > +		rc6->pg_enable = 0;
> > > >   		intel_uncore_write_fw(uncore, GEN9_PG_ENABLE, 0);
> > > > +	}
> > > > +
> > > >   	intel_uncore_write_fw(uncore, GEN6_RC_CONTROL, 0);
> > > >   	intel_uncore_write_fw(uncore, GEN6_RC_STATE, 0);
> > > >   	intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
> > > > @@ -687,6 +691,15 @@ void intel_rc6_unpark(struct intel_rc6 *rc6)
> > > >   	/* Restore HW timers for automatic RC6 entry while busy */
> > > >   	intel_uncore_write_fw(uncore, GEN6_RC_CONTROL, rc6->ctl_enable);
> > > > +
> > > > +	/*
> > > > +	 * Seeing render forcewake timeouts during active submissions so disable render PG
> > > > +	 * while workloads are under execution.
> > > 
> > > Can you please improve this sentence? If I never new about the
> > > issue I would be a bit confused.
> > > 
> > > > +	 * FIXME Remove this change once real cause of render force wake timeout is fixed
> > > > +	 */
> > > > +	if (rc6->pg_enable == GEN9_RENDER_PG_ENABLE)
> > > 
> > > is this supposed to be "pg_enable == GEN9_RENDER_PG_ENABLE" or
> > > "pg_enable & GEN9_RENDER_PG_ENABLE" ?
> > > 
> > > Andi
>