On Tue, 26 May 2026, Ville Syrjälä <[email protected]> wrote:
> 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 */
Fair enough. Can I push with that?
>
>> +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
--
Jani Nikula, Intel
