On Wed, Oct 12, 2022 at 10:48:30AM +0100, Matthew Auld wrote:
> On 12/10/2022 09:34, Anshuman Gupta wrote:
> > DGFX platforms has lmem and cpu can access the lmem objects
> > via mmap and i915 internal i915_gem_object_pin_map() for
> > i915 own usages. Both of these methods has pre-requisite
> > requirement to keep GFX PCI endpoint in D0 for a supported
> > iomem transaction over PCI link. (Refer PCIe specs 5.3.1.4.1)
> >
> > Both DG1/DG2 have a hardware bug that violates the PCIe specs
> > and support the iomem read write transaction over PCIe bus despite
> > endpoint is D3 state.
> > Due to above H/W bug, we had never observed any issue with i915 runtime
> > PM versus lmem access.
> > But this issue becomes visible when PCIe gfx endpoint's upstream
> > bridge enters to D3, at this point any lmem read/write access will be
> > returned as unsupported request. But again this issue is not observed
> > on every platform because it has been observed on few host machines
> > DG1/DG2 endpoint's upstream bridge does not bind with pcieport driver.
> > which really disables the PCIe power savings and leaves the bridge
> > at D0 state.
> >
> > Till we fix all issues related to runtime PM, we need
> > to keep autosupend control to 'on' on all discrete platforms with lmem.
> >
> > Fixes: 527bab0473f2 ("drm/i915/rpm: Enable runtime pm autosuspend by default")
>
> So with this change all the runtime pm stuff is disabled on dgfx? i.e
> intel_runtime_pm_get() always returns zero or so? Wondering if we should
> also revert ad74457a6b5a ("drm/i915/dgfx: Release mmap on rpm suspend") for
> now, since that still needs some more fixes...
I don't believe we need to revert that. That's already one step forward towards
the final solution. It is not complete but it is not wrong.
And it is orthogonal to this protection right now.
>
> > Suggested-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 6ed5786bcd29..410a5cb58a61 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -591,8 +591,15 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
> > pm_runtime_use_autosuspend(kdev);
> > }
> > - /* Enable by default */
> > - pm_runtime_allow(kdev);
> > + /*
> > + * FIXME: Temp hammer to keep autosupend disable on lmem supported platforms.
> > + * As per PCIe specs 5.3.1.4.1, all iomem read write request over a PCIe
> > + * function will be unsupported in case PCIe endpoint function is in D3.
> > + * Let's keep i915 autosuspend control 'on' till we fix all known issue
> > + * with lmem access in D3.
> > + */
> > + if (!HAS_LMEM(i915))
> > + pm_runtime_allow(kdev);
> > /*
> > * The core calls the driver load handler with an RPM reference held.
