> -----Original Message----- > From: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> > Sent: Friday, July 21, 2023 2:34 AM > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Gupta, Anshuman > <anshuman.gupta@xxxxxxxxx> > 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@xxxxxxxxx> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > --- > 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
