Its actions are opportunistic anyway and will be completed on device suspend.
Also restrict the scope of the pm runtime reference to the notifier callback itself to make it easier to follow.
Marking as a fix to simplify backporting of the fix that follows in the series.
Fixes: c6a4d46ec1d7 ("drm/xe: evict user memory in PM notifier") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: stable@vger.kernel.org # v6.16+ Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com --- drivers/gpu/drm/xe/xe_pm.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index a2e85030b7f4..b57b46ad9f7c 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -308,28 +308,22 @@ static int xe_pm_notifier_callback(struct notifier_block *nb, case PM_SUSPEND_PREPARE: xe_pm_runtime_get(xe); err = xe_bo_evict_all_user(xe); - if (err) { + if (err) drm_dbg(&xe->drm, "Notifier evict user failed (%d)\n", err); - xe_pm_runtime_put(xe); - break; - }
err = xe_bo_notifier_prepare_all_pinned(xe); - if (err) { + if (err) drm_dbg(&xe->drm, "Notifier prepare pin failed (%d)\n", err); - xe_pm_runtime_put(xe); - } + xe_pm_runtime_put(xe); break; case PM_POST_HIBERNATION: case PM_POST_SUSPEND: + xe_pm_runtime_get(xe); xe_bo_notifier_unprepare_all_pinned(xe); xe_pm_runtime_put(xe); break; }
- if (err) - return NOTIFY_BAD; - return NOTIFY_DONE; }
On Fri, Aug 29, 2025 at 01:33:49PM +0200, Thomas Hellström wrote:
Its actions are opportunistic anyway and will be completed on device suspend.
Also restrict the scope of the pm runtime reference to the notifier callback itself to make it easier to follow.
Marking as a fix to simplify backporting of the fix that follows in the series.
Fixes: c6a4d46ec1d7 ("drm/xe: evict user memory in PM notifier") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: stable@vger.kernel.org # v6.16+ Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/xe/xe_pm.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index a2e85030b7f4..b57b46ad9f7c 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -308,28 +308,22 @@ static int xe_pm_notifier_callback(struct notifier_block *nb, case PM_SUSPEND_PREPARE: xe_pm_runtime_get(xe); err = xe_bo_evict_all_user(xe);
if (err) {
if (err) drm_dbg(&xe->drm, "Notifier evict user failed (%d)\n", err);
xe_pm_runtime_put(xe);
break;
}
err = xe_bo_notifier_prepare_all_pinned(xe);
if (err) {
if (err) drm_dbg(&xe->drm, "Notifier prepare pin failed (%d)\n", err);
xe_pm_runtime_put(xe);
}
break; case PM_POST_HIBERNATION: case PM_POST_SUSPEND:xe_pm_runtime_put(xe);
xe_bo_notifier_unprepare_all_pinned(xe); xe_pm_runtime_put(xe); break; }xe_pm_runtime_get(xe);
- if (err)
return NOTIFY_BAD;
- return NOTIFY_DONE;
} -- 2.50.1
On 29/08/2025 12:33, Thomas Hellström wrote:
Its actions are opportunistic anyway and will be completed on device suspend.
Also restrict the scope of the pm runtime reference to the notifier callback itself to make it easier to follow.
Marking as a fix to simplify backporting of the fix that follows in the series.
Fixes: c6a4d46ec1d7 ("drm/xe: evict user memory in PM notifier") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: stable@vger.kernel.org # v6.16+ Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com
drivers/gpu/drm/xe/xe_pm.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index a2e85030b7f4..b57b46ad9f7c 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -308,28 +308,22 @@ static int xe_pm_notifier_callback(struct notifier_block *nb, case PM_SUSPEND_PREPARE: xe_pm_runtime_get(xe); err = xe_bo_evict_all_user(xe);
if (err) {
if (err) drm_dbg(&xe->drm, "Notifier evict user failed (%d)\n", err);
xe_pm_runtime_put(xe);
break;
}
err = xe_bo_notifier_prepare_all_pinned(xe);
if (err) {
if (err) drm_dbg(&xe->drm, "Notifier prepare pin failed (%d)\n", err);
xe_pm_runtime_put(xe);
}
xe_pm_runtime_put(xe);
IIRC I was worried that this ends up triggering runtime suspend at some later point and then something wakes it up again before the actual forced suspend triggers, which looks like it would undo all the prepare_all_pinned() work, so I opted for keeping it held over the entire sequence. Is that not a concern here?
break;
case PM_POST_HIBERNATION: case PM_POST_SUSPEND:
xe_bo_notifier_unprepare_all_pinned(xe); xe_pm_runtime_put(xe); break; }xe_pm_runtime_get(xe);
- if (err)
return NOTIFY_BAD;
- return NOTIFY_DONE; }
On Fri, Aug 29, 2025 at 04:50:01PM +0100, Matthew Auld wrote:
On 29/08/2025 12:33, Thomas Hellström wrote:
Its actions are opportunistic anyway and will be completed on device suspend.
Also restrict the scope of the pm runtime reference to the notifier callback itself to make it easier to follow.
Marking as a fix to simplify backporting of the fix that follows in the series.
Fixes: c6a4d46ec1d7 ("drm/xe: evict user memory in PM notifier") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: stable@vger.kernel.org # v6.16+ Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com
drivers/gpu/drm/xe/xe_pm.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index a2e85030b7f4..b57b46ad9f7c 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -308,28 +308,22 @@ static int xe_pm_notifier_callback(struct notifier_block *nb, case PM_SUSPEND_PREPARE: xe_pm_runtime_get(xe); err = xe_bo_evict_all_user(xe);
if (err) {
if (err) drm_dbg(&xe->drm, "Notifier evict user failed (%d)\n", err);
xe_pm_runtime_put(xe);
break;
err = xe_bo_notifier_prepare_all_pinned(xe);}
if (err) {
if (err) drm_dbg(&xe->drm, "Notifier prepare pin failed (%d)\n", err);
xe_pm_runtime_put(xe);
}
xe_pm_runtime_put(xe);
IIRC I was worried that this ends up triggering runtime suspend at some later point and then something wakes it up again before the actual forced suspend triggers, which looks like it would undo all the prepare_all_pinned() work, so I opted for keeping it held over the entire sequence. Is that not a concern here?
I was seeing this more like a umbrella to shut-up our inner callers warnings since runtime pm references will be taken prior to pm state transitions by base/power.
However on a quick look I couldn't connect the core code that takes the runtime pm directly with this notify now. So, perhaps let's indeed play safe and keep our own references here?!...
break;
case PM_POST_HIBERNATION: case PM_POST_SUSPEND:
xe_bo_notifier_unprepare_all_pinned(xe); xe_pm_runtime_put(xe); break; }xe_pm_runtime_get(xe);
- if (err)
return NOTIFY_BAD;
- return NOTIFY_DONE; }
On Fri, 2025-08-29 at 13:15 -0400, Rodrigo Vivi wrote:
On Fri, Aug 29, 2025 at 04:50:01PM +0100, Matthew Auld wrote:
On 29/08/2025 12:33, Thomas Hellström wrote:
Its actions are opportunistic anyway and will be completed on device suspend.
Also restrict the scope of the pm runtime reference to the notifier callback itself to make it easier to follow.
Marking as a fix to simplify backporting of the fix that follows in the series.
Fixes: c6a4d46ec1d7 ("drm/xe: evict user memory in PM notifier") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: stable@vger.kernel.org # v6.16+ Signed-off-by: Thomas Hellström
thomas.hellstrom@linux.intel.com
drivers/gpu/drm/xe/xe_pm.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index a2e85030b7f4..b57b46ad9f7c 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -308,28 +308,22 @@ static int xe_pm_notifier_callback(struct notifier_block *nb, case PM_SUSPEND_PREPARE: xe_pm_runtime_get(xe); err = xe_bo_evict_all_user(xe);
if (err) {
if (err)
drm_dbg(&xe->drm, "Notifier evict user failed (%d)\n", err);
xe_pm_runtime_put(xe);
break;
}
err = xe_bo_notifier_prepare_all_pinned(xe);
if (err) {
if (err)
drm_dbg(&xe->drm, "Notifier prepare pin failed (%d)\n", err);
xe_pm_runtime_put(xe);
}
xe_pm_runtime_put(xe);
IIRC I was worried that this ends up triggering runtime suspend at some later point and then something wakes it up again before the actual forced suspend triggers, which looks like it would undo all the prepare_all_pinned() work, so I opted for keeping it held over the entire sequence. Is that not a concern here?
Good point.
I was seeing this more like a umbrella to shut-up our inner callers warnings since runtime pm references will be taken prior to pm state transitions by base/power.
However on a quick look I couldn't connect the core code that takes the runtime pm directly with this notify now. So, perhaps let's indeed play safe and keep our own references here?!...
I'll keep the references, and perhaps add a comment about that so that nobody tries this again.
Any concerns about ignoring errors?
/Thomas
break; case PM_POST_HIBERNATION: case PM_POST_SUSPEND:
xe_pm_runtime_get(xe);
xe_bo_notifier_unprepare_all_pinned(xe); xe_pm_runtime_put(xe); break; }
- if (err)
return NOTIFY_BAD;
return NOTIFY_DONE; }
linux-stable-mirror@lists.linaro.org