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.
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
