On Mon, May 25, 2026 at 02:05:53PM +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().
>
> v2: Add FIXME comment (Ville)
>
> Signed-off-by: Jani Nikula <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++
> drivers/gpu/drm/i915/display/intel_display.h | 1 +
> drivers/gpu/drm/xe/display/xe_display.c | 27 +++-----------------
> 3 files changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 6c8935f69db1..a6cee0f81358 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -737,6 +737,28 @@ bool intel_has_pending_fb_unpin(struct intel_display
> *display)
> return false;
> }
>
> +/* FIXME: All callers need to be audited and unified between i915 and xe */
That makes me think we want to keep this. I was more thinking of
something like
/* FIXME remove this and just flush the cleanup wq where appropriate */
> +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 45a90d2fe6ec..72f33113a5a3 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 8d08da60336d..a5066de3d789 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -244,27 +244,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;
> @@ -278,7 +257,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);
>
> @@ -333,7 +312,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_encoder_block_all_hpds(display);
>
> @@ -365,7 +344,7 @@ void xe_display_pm_shutdown(struct xe_device *xe)
> intel_display_driver_suspend(display);
> }
>
> - 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
