On Fri, Jun 05, 2026 at 07:04:59PM +0300, Jani Nikula wrote: > On Fri, 29 May 2026, Ville Syrjälä <[email protected]> wrote: > > On Fri, May 29, 2026 at 02:04:08PM +0300, Jani Nikula wrote: > >> With the special d3cold handling in xe runtime suspend/resume separated, > >> unify the non-d3cold paths with i915. There are multiple changes here at > >> once that probably wouldn't make much sense to split out further. > >> > >> Add the call to intel_display_driver_pm_runtime_suspend(), which in turn > >> starts calling intel_display_power_runtime_suspend(). Deep down, this > >> should take care of intel_dmc_wl_flush_release_work(), allowing us to > >> drop the extra call in xe_display_pm_runtime_suspend_late(). > >> > >> Add the call to intel_display_driver_pm_runtime_suspend_late(), which > >> starts calling intel_opregion_notify_adapter(). The > >> intel_hpd_poll_enable() call is also done here, postponed from the > >> previous location in xe_display_pm_runtime_suspend(). > >> > >> Add the call to intel_display_driver_pm_runtime_resume_early(), which > >> contains the intel_display_power_runtime_resume() and > >> intel_opregion_notify_adapter() counterparts found in > >> intel_display_power_runtime_suspend() and > >> intel_display_power_runtime_suspend_late(). (They are not symmetric.) > >> > >> Finally, intel_display_driver_pm_runtime_resume() replaces the direct > >> calls to intel_hpd_init(), intel_hpd_poll_disable(), and > >> skl_watermark_ipc_update(). > >> > >> Signed-off-by: Jani Nikula <[email protected]> > >> --- > >> drivers/gpu/drm/xe/display/xe_display.c | 14 ++++---------- > >> 1 file changed, 4 insertions(+), 10 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/xe/display/xe_display.c > >> b/drivers/gpu/drm/xe/display/xe_display.c > >> index 0fadc62e1cdd..cd7264d7dfe2 100644 > >> --- a/drivers/gpu/drm/xe/display/xe_display.c > >> +++ b/drivers/gpu/drm/xe/display/xe_display.c > >> @@ -363,7 +363,7 @@ void xe_display_pm_runtime_suspend(struct xe_device > >> *xe) > >> return; > >> } > >> > >> - intel_hpd_poll_enable(display); > >> + intel_display_driver_pm_runtime_suspend(display); > >> } > >> > >> void xe_display_pm_runtime_suspend_late(struct xe_device *xe) > >> @@ -379,12 +379,7 @@ void xe_display_pm_runtime_suspend_late(struct > >> xe_device *xe) > >> return; > >> } > >> > >> - /* > >> - * If xe_display_pm_suspend_late() is not called, it is likely > >> - * that we will be on dynamic DC states with DMC wakelock enabled. We > >> - * need to flush the release work in that case. > >> - */ > > > > The entire comment seems wonky. I don't think we have any platforms > > with wakelock+!DC9. Ah, it was added exactly because the D3cold path > > is broken and doesn't go into DC9. Would have been better to fix the > > actual problem instead. I guess you should move the comment to the > > D3cold path in the previous patch to keep it where it actually makes > > some sense. > > Huh, but the comment says, "If xe_display_pm_suspend_late() is not > called", and the d3cold path above calls that. It's the non-d3cold path > that does not call the function. > > Yeah, I don't know what to do with this. > > Commit 731c74e988ff ("drm/xe/display: Flush DMC wakelock release work on > runtime suspend") says, "We currently are not calling display runtime > suspend functions when D3cold is not allowed." But that's not the whole > truth, we're not calling the runtime suspend/resume functions *at all* > on any xe path. (Which is what the patch at hand is trying to fix for > non-d3cold.) > > Why is the fix in the above commit just adding the single DMC wakelock > call, instead of doing the proper runtime suspend/resume for non-d3cold? > > I don't get it.
Yeap, it was never called and we were indeed dropping the DC9 savings out. DC5/DC6 was the only thing that would be working anyway I believe. And Gustavo's patch was more like a temporary workaround at the time that we knew we would be working to unify the sequences. I'm sorry for having dropped that effort :( > > Cc: Gustavo, Rodrigo > > > BR, > Jani. > > > > > > Reviewed-by: Ville Syrjälä <[email protected]> > > > >> - intel_dmc_wl_flush_release_work(display); > >> + intel_display_driver_pm_runtime_suspend_late(display); > >> } > >> > >> void xe_display_pm_runtime_resume(struct xe_device *xe) > >> @@ -399,9 +394,8 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > >> return; > >> } > >> > >> - intel_hpd_init(display); > >> - intel_hpd_poll_disable(display); > >> - skl_watermark_ipc_update(display); > >> + intel_display_driver_pm_runtime_resume_early(display); > >> + intel_display_driver_pm_runtime_resume(display); > >> } > >> > >> > >> -- > >> 2.47.3 > > -- > Jani Nikula, Intel
