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

From: "Gupta, Anshuman" <anshuman.gupta@intel.com>
To: "Auld, Matthew" <matthew.auld@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Cc: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/dgfx: Temporary hammer to keep autosuspend control 'on'
Date: Fri, 14 Oct 2022 04:31:19 +0000	[thread overview]
Message-ID: <CY5PR11MB6211148A0A9C8E4162D7896F95249@CY5PR11MB6211.namprd11.prod.outlook.com> (raw)
In-Reply-To: <6fb51e71-bc5c-16d1-ad13-33017035be83@intel.com>


> -----Original Message-----
> From: Auld, Matthew <matthew.auld@intel.com>
> Sent: Wednesday, October 12, 2022 3:19 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: joonas.lahtinen@linux.intel.com; tvrtko.ursulin@linux.intel.com; Vivi,
> Rodrigo <rodrigo.vivi@intel.com>
> Subject: Re: [PATCH] drm/i915/dgfx: Temporary hammer to keep autosuspend
> control 'on'
> 
> 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...
Hi Matt,
This patch just keeps autosupend to 'on' as i915 default.
User can change it to 'auto' by echo 'auto' > /sys/bus/pci/devices/bdf/power/control ,
so we need the above mentioned commit and need to fix the issues with it.
Thanks,
Anshuman Gupta.
> 
> > 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.