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.
> 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.
Matt
>
> This will have an official WA soon so adding a FIXME in the comments.
>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>
> ---
> 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
