| Message ID | 20240924204222.246862-30-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reconcile i915's and xe's display power mgt sequences | expand |
-----Original Message----- From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Rodrigo Vivi Sent: Tuesday, September 24, 2024 1:36 PM To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org Cc: Deak, Imre <imre.deak@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com> 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 <jonathan.cavitt@intel.com> -Jonathan Cavitt > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > 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 > >
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);
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. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/xe/display/xe_display.c | 23 ----------------------- 1 file changed, 23 deletions(-)