| Message ID | 20241129151754.22015-3-gustavo.sousa@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Flush DMC wakelock release work at the end of runtime suspend | expand |
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. -- 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 >
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, 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> > > -- > 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 > >
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 >> >
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); +} + 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);
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(-)