On Wed, May 13, 2026 at 10:58:35AM +0300, Jani Nikula wrote: > xe_display_flush_cleanup_work() is a bit of an oddball function in xe > display code. There shouldn't be anything this specific or xe > specific. While I'm not sure what the correct refactor for the function > should be, move it to shared display code for starters, next to the > eerily similar but slightly different intel_has_pending_fb_unpin() that > is only called from i915 core. > > The main goal here is to unblock some refactors on > for_each_intel_crtc(). > > Signed-off-by: Jani Nikula <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_display.c | 21 +++++++++++++++ > drivers/gpu/drm/i915/display/intel_display.h | 1 + > drivers/gpu/drm/xe/display/xe_display.c | 27 +++----------------- > 3 files changed, 25 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index d5cf1476c7b9..50feca52b962 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -737,6 +737,27 @@ bool intel_has_pending_fb_unpin(struct intel_display > *display) > return false; > } > > +void intel_display_flush_cleanup_work(struct intel_display *display) > +{ > + struct intel_crtc *crtc; > + > + for_each_intel_crtc(display->drm, crtc) { > + struct drm_crtc_commit *commit; > + > + spin_lock(&crtc->base.commit_lock); > + commit = list_first_entry_or_null(&crtc->base.commit_list, > + struct drm_crtc_commit, > commit_entry); > + if (commit) > + drm_crtc_commit_get(commit); > + spin_unlock(&crtc->base.commit_lock); > + > + if (commit) { > + wait_for_completion(&commit->cleanup_done); > + drm_crtc_commit_put(commit); > + } > + } > +} > + > /* > * Finds the encoder associated with the given CRTC. This can only be > * used when we know that the CRTC isn't feeding multiple encoders! > diff --git a/drivers/gpu/drm/i915/display/intel_display.h > b/drivers/gpu/drm/i915/display/intel_display.h > index a43ada0c0502..65f8c81a7bae 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.h > +++ b/drivers/gpu/drm/i915/display/intel_display.h > @@ -402,6 +402,7 @@ void intel_disable_transcoder(const struct > intel_crtc_state *old_crtc_state); > void i830_enable_pipe(struct intel_display *display, enum pipe pipe); > void i830_disable_pipe(struct intel_display *display, enum pipe pipe); > bool intel_has_pending_fb_unpin(struct intel_display *display); > +void intel_display_flush_cleanup_work(struct intel_display *display); > void intel_encoder_destroy(struct drm_encoder *encoder); > struct drm_display_mode * > intel_encoder_current_mode(struct intel_encoder *encoder); > diff --git a/drivers/gpu/drm/xe/display/xe_display.c > b/drivers/gpu/drm/xe/display/xe_display.c > index aa73023b7398..ef27fdfdbab2 100644 > --- a/drivers/gpu/drm/xe/display/xe_display.c > +++ b/drivers/gpu/drm/xe/display/xe_display.c > @@ -258,27 +258,6 @@ static bool suspend_to_idle(void) > return false; > } > > -static void xe_display_flush_cleanup_work(struct xe_device *xe) > -{ > - struct intel_crtc *crtc; > - > - for_each_intel_crtc(&xe->drm, crtc) { > - struct drm_crtc_commit *commit; > - > - spin_lock(&crtc->base.commit_lock); > - commit = list_first_entry_or_null(&crtc->base.commit_list, > - struct drm_crtc_commit, > commit_entry); > - if (commit) > - drm_crtc_commit_get(commit); > - spin_unlock(&crtc->base.commit_lock); > - > - if (commit) { > - wait_for_completion(&commit->cleanup_done); > - drm_crtc_commit_put(commit); > - } > - } > -} > - > static void xe_display_enable_d3cold(struct xe_device *xe) > { > struct intel_display *display = xe->display; > @@ -292,7 +271,7 @@ static void xe_display_enable_d3cold(struct xe_device *xe) > */ > intel_power_domains_disable(display); > > - xe_display_flush_cleanup_work(xe); > + intel_display_flush_cleanup_work(display); > > intel_opregion_suspend(display, PCI_D3cold); > > @@ -347,7 +326,7 @@ void xe_display_pm_suspend(struct xe_device *xe) > intel_display_driver_suspend(display); > } > > - xe_display_flush_cleanup_work(xe); > + intel_display_flush_cleanup_work(display);
intel_display_driver_suspend() already flushes the cleanup wq. So I think this is doing nothing. The correct answer seems to be to nuke the whole thing. We are missing the wq flush from the shutdown() path in i915 however, so I suppose we should add it there. > > intel_encoder_block_all_hpds(display); > > @@ -379,7 +358,7 @@ void xe_display_pm_shutdown(struct xe_device *xe) > intel_display_driver_suspend(display); This should rather be the same atomic helper shutdown that i915 uses. I guess what we want is a intel_display_driver_shutdown() to pair up with intel_display_driver_suspend(). > } > > - xe_display_flush_cleanup_work(xe); > + intel_display_flush_cleanup_work(display); > intel_dp_mst_suspend(display); > intel_encoder_block_all_hpds(display); > intel_hpd_cancel_work(display); > -- > 2.47.3 -- Ville Syrjälä Intel
