[PATCH] drm/i915/mtl: Wake GT before sending H2G message

[Matt Roper]

On Thu, Jan 18, 2024 at 05:21:23PM -0800, Belgaumkar, Vinay wrote:
> 
> On 1/18/2024 3:50 PM, Matt Roper wrote:
> > On Thu, Jan 18, 2024 at 03:17:28PM -0800, Vinay Belgaumkar wrote:
> > > Instead of waiting until the interrupt reaches GuC, we can grab a
> > > forcewake while triggering the H2G interrupt. GEN11_GUC_HOST_INTERRUPT
> > > is inside an "always on" domain with respect to RC6. However, there
> > A bit of a nitpick, but technically "always on" is a description of GT
> > register ranges that never get powered down.  GEN11_GUC_HOST_INTERRUPT
> > isn't inside the GT at all, but rather is an sgunit register and thus
> > isn't affected by forcewake.  This is just a special case where the
> > sgunit register forwards a message back to the GT's GuC, and the
> > workaround wants us to make sure the GT is awake before that message
> > gets there.
> True, can modify the description to reflect this.
> > 
> > > could be some delays when platform is entering/exiting some higher
> > > level platform sleep states and a H2G is triggered. A forcewake
> > > ensures those sleep states have been fully exited and further
> > > processing occurs as expected.
> > Based on this description, is adding implicit forcewake to this register
> > really enough?  Implicit forcewake powers up before a read/write, but
> > also allows it to power back down as soon as the MMIO operation is
> > complete.  If the GuC is a bit slow to notice the interrupt, then we
> > could wind up with a sequence like
> > 
> >   - Driver grabs forcewake and GT powers up
> >   - Driver writes 0x1901f0 to trigger GuC interrupt
> >   - Driver releases forcewake and GT powers down
> >   - GuC notices interrupt (or maybe fails to notice it because the GT
> >     powered down before it had a chance to process it?)
> > 
> > which I'm guessing isn't actually going to satisfy this workaround.  Do
> > we actually need to keep the GT awake not just through the register
> > operation, but also through the GuC's processing of the interrupt?  If
> > so, then we probably want to do an explicit forcewake get/put to ensure
> > the hardware stays powered up long enough.
> 
> The issue being addressed here is not GT entering C6, but the higher
> platform sleep states. Once we force wake GT while writing to the H2G
> register, that should bring us out of sleep. After clearing the forcewake
> (which would happen after the write for 0x1901f0 goes through), we still
> have C6 hysteresis and the hysteresis counters for the higher platform sleep
> states which should give GuC enough time to process the interrupt before we
> enter C6 and then subsequently these higher sleep states.

Okay, makes sense.  Hopefully the finalize the workaround details and
documentation soon, but this looks reasonable with the information we
have at the moment.

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


Matt

> 
> Thanks,
> 
> Vinay.
> 
> > 
> > 
> > Matt
> > 
> > > This will have an official WA soon so adding a FIXME in the comments.
> > > 
> > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_uncore.c | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > > index dfefad5a5fec..121458a31886 100644
> > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > @@ -1800,7 +1800,10 @@ static const struct intel_forcewake_range __mtl_fw_ranges[] = {
> > >   	GEN_FW_RANGE(0x24000, 0x2ffff, 0), /*
> > >   		0x24000 - 0x2407f: always on
> > >   		0x24080 - 0x2ffff: reserved */
> > > -	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_GT)
> > > +	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_GT),
> > > +	GEN_FW_RANGE(0x40000, 0x1901ec, 0),
> > > +	GEN_FW_RANGE(0x1901f0, 0x1901f0, FORCEWAKE_GT)
> > > +		/* FIXME: WA to wake GT while triggering H2G */
> > >   };
> > >   /*
> > > -- 
> > > 2.38.1
> > > 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation