[Intel-gfx] [PATCH v2] drm/i915/dgfx: Keep PCI autosuspend control 'on' by default on all dGPU

[Gupta, Anshuman]

> -----Original Message-----
> From: Gupta, Anshuman <anshuman.gupta at intel.com>
> Sent: Friday, October 14, 2022 5:03 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: joonas.lahtinen at linux.intel.com; tvrtko.ursulin at linux.intel.com; Auld,
> Matthew <matthew.auld at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>;
> Gupta, Anshuman <anshuman.gupta at intel.com>
> Subject: [PATCH v2] drm/i915/dgfx: Keep PCI autosuspend control 'on' by
> default on all dGPU
> 
> 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 known 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.
> 
> We need a unique interface to read/write from lmem with runtime PM wakeref
> protection something similar to intel_uncore_{read, write}, keep autosuspend
> control to 'on' on all discrete platforms, until we have a unique interface to
> read/write from lmem.
> 
> This just change the default autosuspend setting of i915 on dGPU, user can still
> change it to 'auto'.
> 
> v2:
> - Modified the commit message and subject with more information.
> - Changed the Fixes tag to LMEM support commit. [Joonas]
> - Changed !HAS_LMEM() Cond to !IS_DGFX(). [Rodrigo]
> 
> Fixes: b908be543e44 ("drm/i915: support creating LMEM objects")
> Suggested-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
Hi Rodrigo ,
I missed to add your conditional RB tag here.
I addressed all comments and changed the Fixes tag to b908be543e44 ("drm/i915: support creating LMEM objects")
CI is green, shall I use your RB and merge this tag if nobody else has any further review comment?
Thanks,
Anshuman.
> ---
>  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..744cca507946 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 (!IS_DGFX(i915))
> +		pm_runtime_allow(kdev);
> 
>  	/*
>  	 * The core calls the driver load handler with an RPM reference held.
> --
> 2.25.1