Re: [Intel-gfx] [PATCH] drm/i915/dgfx: Temporary hammer to keep autosuspend control 'on'

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

On Wed, Oct 12, 2022 at 01:26:45PM +0300, Joonas Lahtinen wrote:
> I think I commented on this already, but the patch subject should really be as
> informative as possible like: "Disable PCI runtime PM on dGPUs" as that is exactly
> what the patch does.

+1 here.

> 
> Also bit unsure if the Fixes: tag should really point to the runtime PM
> commit but maybe instead to the introduction of LMEM commit.

indeed. if the runtime_pm by default came earlier then the LMEM, the
HAS_LMEM() won't even exist and it will break the ports.

So, I'd say to go to whatever patch is the most recent between the 2.

With those changes:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> Regards, Joonas
> 
> Quoting Anshuman Gupta (2022-10-12 11:34:02)
> > 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")
> > Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  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.
> > -- 
> > 2.25.1
> >