Re: [PATCH 2/3] drm/xe/display: Extract xe_display_pm_runtime_suspend_late()

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Rodrigo Vivi (2024-11-29 13:43:28-03:00)
>On Fri, Nov 29, 2024 at 01:07:32PM -0300, Gustavo Sousa wrote:
>> Quoting Gustavo Sousa (2024-11-29 12:17:45-03:00)
>> >The current behavior for the runtime suspend case is that
>> >xe_display_pm_suspend_late() is only called when D3cold is allowed.
>> >Let's incorportate that behavior into a function specific to runtime PM
>> >and call it xe_display_pm_runtime_suspend_late().
>> >
>> >With that, we keep stuff a bit more self-contained and allow having a
>> >place for adding more "late display runtime suspend"-related logic that
>> >isn't dependent on the "D3cold allowed" state.
>> >
>> >Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> >---
>> > drivers/gpu/drm/xe/display/xe_display.c | 9 +++++++++
>> > drivers/gpu/drm/xe/display/xe_display.h | 1 +
>> > drivers/gpu/drm/xe/xe_pm.c              | 4 ++--
>> > 3 files changed, 12 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
>> >index 69c828f38cb6..2e189abf8d6f 100644
>> >--- a/drivers/gpu/drm/xe/display/xe_display.c
>> >+++ b/drivers/gpu/drm/xe/display/xe_display.c
>> >@@ -407,6 +407,15 @@ void xe_display_pm_suspend_late(struct xe_device *xe)
>> >         intel_display_power_suspend_late(xe, s2idle);
>> > }
>> > 
>> >+void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
>> >+{
>> >+        if (!xe->info.probe_display)
>> >+                return;
>> >+
>> >+        if (xe->d3cold.allowed)
>> >+                xe_display_pm_runtime_suspend_late(xe);
>> 
>> Whoops... There is a typo here. It should have been
>> xe_display_pm_suspend_late(). I'll send a new version.
>
>I'm glad you found first,

Me too, to attenuate my embarrassment :-)

>I confess I was going to miss it.
>
>Please go ahead with the changes and I will incorporate that
>later whenever I get back to that display-pm-reconcile series
>I have
>
>For the series (with this chunk fixed):
>
>Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thanks!

--
Gustavo Sousa

>
>> 
>> --
>> Gustavo Sousa
>> 
>> >+}
>> >+
>> > void xe_display_pm_shutdown_late(struct xe_device *xe)
>> > {
>> >         if (!xe->info.probe_display)
>> >diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
>> >index 17afa537aee5..954c5a268ef4 100644
>> >--- a/drivers/gpu/drm/xe/display/xe_display.h
>> >+++ b/drivers/gpu/drm/xe/display/xe_display.h
>> >@@ -41,6 +41,7 @@ void xe_display_pm_shutdown_late(struct xe_device *xe);
>> > void xe_display_pm_resume_early(struct xe_device *xe);
>> > void xe_display_pm_resume(struct xe_device *xe);
>> > void xe_display_pm_runtime_suspend(struct xe_device *xe);
>> >+void xe_display_pm_runtime_suspend_late(struct xe_device *xe);
>> > void xe_display_pm_runtime_resume(struct xe_device *xe);
>> > 
>> > #else
>> >diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
>> >index 80699dbeb2e9..a6761cb769b2 100644
>> >--- a/drivers/gpu/drm/xe/xe_pm.c
>> >+++ b/drivers/gpu/drm/xe/xe_pm.c
>> >@@ -414,8 +414,8 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
>> > 
>> >         xe_irq_suspend(xe);
>> > 
>> >-        if (xe->d3cold.allowed)
>> >-                xe_display_pm_suspend_late(xe);
>> >+        xe_display_pm_runtime_suspend_late(xe);
>> >+
>> > out:
>> >         if (err)
>> >                 xe_display_pm_runtime_resume(xe);
>> >-- 
>> >2.47.0
>> >