[Intel-gfx] [PATCH 3/4] drm/xe: Fix the runtime_idle call and d3cold.allowed decision.

[Gupta, Anshuman]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi at intel.com>
> Sent: Friday, July 21, 2023 2:34 AM
> To: intel-gfx at lists.freedesktop.org
> Cc: Vivi, Rodrigo <rodrigo.vivi at intel.com>; Gupta, Anshuman
> <anshuman.gupta at intel.com>
> Subject: [PATCH 3/4] drm/xe: Fix the runtime_idle call and d3cold.allowed
> decision.
> 
> According to Documentation/power/runtime_pm.txt:
I tried to fix runtime idle https://patchwork.freedesktop.org/patch/543024/?series=119467&rev=1 
But forgot to CC to you.
Anyway some comment below,

> 
> int pm_runtime_put(struct device *dev);
>     - decrement the device's usage counter; if the result is 0 then run
>       pm_request_idle(dev) and return its result
> 
> int pm_runtime_put_autosuspend(struct device *dev);
>     - decrement the device's usage counter; if the result is 0 then run
>       pm_request_autosuspend(dev) and return its result
> 
> We need to ensure that the idle function is called before suspending so we
> take the right d3cold.allowed decision and respect the values set on
> vram_d3cold_threshold sysfs. So we need pm_runtime_put() instead of
> pm_runtime_put_autosuspend().
> 
> Cc: Anshuman Gupta <anshuman.gupta at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index a6459df2599e..73bcb76c2d42 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -144,7 +144,7 @@ static void xe_pm_runtime_init(struct xe_device *xe)
>  	pm_runtime_set_active(dev);
>  	pm_runtime_allow(dev);
>  	pm_runtime_mark_last_busy(dev);
I have not thought of using last_busy() here in _put().
If we mark last_busy during _put then pm core auto-suspend timer will start ticking from _put().
Theoretically  that can lead to idle() and runtime_suspend() call to race with each other ? [1]
[1] Documentation/power/runtime_pm.txt
(1) The callbacks are mutually exclusive (e.g. it is forbidden to execute
    ->runtime_suspend() in parallel with ->runtime_resume() or with another
    instance of ->runtime_suspend() for the same device) with the exception that
    ->runtime_suspend() or ->runtime_resume() can be executed in parallel with
    ->runtime_idle() (although ->runtime_idle() will not be started while any
    of the other callbacks is being executed for the same device).
Thanks,
Anshuman Gupta.
> -	pm_runtime_put_autosuspend(dev);
> +	pm_runtime_put(dev);
We need to fix this in intel_runtime_pm_put()  compat-i915-headers as well.
>  }
> 
>  void xe_pm_init(struct xe_device *xe)
> @@ -273,7 +273,7 @@ int xe_pm_runtime_get(struct xe_device *xe)  int
> xe_pm_runtime_put(struct xe_device *xe)  {
>  	pm_runtime_mark_last_busy(xe->drm.dev);
> -	return pm_runtime_put_autosuspend(xe->drm.dev);
> +	return pm_runtime_put(xe->drm.dev);
>  }
> 
>  int xe_pm_runtime_get_if_active(struct xe_device *xe)
> --
> 2.41.0