-----Original Message----- From: Intel-gfx <[email protected]> On Behalf Of Rodrigo Vivi Sent: Tuesday, September 24, 2024 1:36 PM To: [email protected]; [email protected] Cc: Deak, Imre <[email protected]>; Vivi, Rodrigo <[email protected]> Subject: [PATCH 29/31] drm/xe/display: Kill crtc commit flush > > This flush was needed in regular suspend cases in the past. > After the clean-up and reconciliation with the i915 it was > not needed anymore and removed. However this remained here > in the runtime suspend path. > > However, the runtime pm flow ensures that there won't be any > flying or pending crtc commit when the runtime_suspend is > called. So this is not needed here. Clean it up. >
LGTM, though maybe the commit message could use some refactoring. Something like: """ xe_display_flush_cleanup_work was originally needed for regular suspend cases. After the clean-up and reconciliation with the i915, however, it was no longer needed and removed. Despite this, the function remained as a part of the runtime suspend path. Since the runtime pm flow ensures that there won't be any flying or pending crtc commits when the runtime_suspend is called, calling xe_display_flush_cleanup_work here is no longer necessary. With no other use cases, this function can safely be removed. """ Just a suggestion. Not blocking. Reviewed-by: Jonathan Cavitt <[email protected]> -Jonathan Cavitt > Signed-off-by: Rodrigo Vivi <[email protected]> > --- > drivers/gpu/drm/xe/display/xe_display.c | 23 ----------------------- > 1 file changed, 23 deletions(-) > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c > b/drivers/gpu/drm/xe/display/xe_display.c > index 780c8d7f392d..23bdd8904c44 100644 > --- a/drivers/gpu/drm/xe/display/xe_display.c > +++ b/drivers/gpu/drm/xe/display/xe_display.c > @@ -283,27 +283,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_to_d3cold(struct xe_device *xe) > { > struct intel_display *display = &xe->display; > @@ -311,8 +290,6 @@ static void xe_display_to_d3cold(struct xe_device *xe) > /* We do a lot of poking in a lot of registers, make sure they work > properly. */ > intel_power_domains_disable(xe); > > - xe_display_flush_cleanup_work(xe); > - > intel_hpd_cancel_work(xe); > > intel_opregion_suspend(display, PCI_D3cold); > -- > 2.46.0 > >
