| Message ID | 1437051471-15397-5-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 16, 2015 at 02:57:50PM +0200, Maarten Lankhorst wrote: > Now that intel_display_suspend is atomic it's safe to remove > wait_for_pending_flips from intel_crtc_disable_noatomic. It > will only be used during hw load or resume, in which case there > will be no pending flips anyway. A WARN_ON(pending_flip) then? (Actually we should start doing DRM_ERROR_ON I guess that would make a lot of complaints go away, and also a lot of genuine bug reports) Or do we have warning coverage elsewhere along the CRTC change path? -Chris
Op 21-07-15 om 13:35 schreef Chris Wilson: > On Thu, Jul 16, 2015 at 02:57:50PM +0200, Maarten Lankhorst wrote: >> Now that intel_display_suspend is atomic it's safe to remove >> wait_for_pending_flips from intel_crtc_disable_noatomic. It >> will only be used during hw load or resume, in which case there >> will be no pending flips anyway. > A WARN_ON(pending_flip) then? (Actually we should start doing > DRM_ERROR_ON I guess that would make a lot of complaints go away, and > also a lot of genuine bug reports) Or do we have warning coverage > elsewhere along the CRTC change path? intel_sanitize_crtc, called during hw readout, is the only caller of intel_crtc_disable_noatomic. During hw readout no sw updates should be queued anyway..
On Tue, Jul 21, 2015 at 02:38:14PM +0200, Maarten Lankhorst wrote: > Op 21-07-15 om 13:35 schreef Chris Wilson: > > On Thu, Jul 16, 2015 at 02:57:50PM +0200, Maarten Lankhorst wrote: > >> Now that intel_display_suspend is atomic it's safe to remove > >> wait_for_pending_flips from intel_crtc_disable_noatomic. It > >> will only be used during hw load or resume, in which case there > >> will be no pending flips anyway. > > A WARN_ON(pending_flip) then? (Actually we should start doing > > DRM_ERROR_ON I guess that would make a lot of complaints go away, and > > also a lot of genuine bug reports) Or do we have warning coverage > > elsewhere along the CRTC change path? > > intel_sanitize_crtc, called during hw readout, is the only caller of intel_crtc_disable_noatomic. > During hw readout no sw updates should be queued anyway.. Is there any documentation to say that this function can't ever be called outside of sanitize_crtc? Perhaps rename the function to reflect its usage? -Chris
Op 21-07-15 om 14:40 schreef Chris Wilson: > On Tue, Jul 21, 2015 at 02:38:14PM +0200, Maarten Lankhorst wrote: >> Op 21-07-15 om 13:35 schreef Chris Wilson: >>> On Thu, Jul 16, 2015 at 02:57:50PM +0200, Maarten Lankhorst wrote: >>>> Now that intel_display_suspend is atomic it's safe to remove >>>> wait_for_pending_flips from intel_crtc_disable_noatomic. It >>>> will only be used during hw load or resume, in which case there >>>> will be no pending flips anyway. >>> A WARN_ON(pending_flip) then? (Actually we should start doing >>> DRM_ERROR_ON I guess that would make a lot of complaints go away, and >>> also a lot of genuine bug reports) Or do we have warning coverage >>> elsewhere along the CRTC change path? >> intel_sanitize_crtc, called during hw readout, is the only caller of intel_crtc_disable_noatomic. >> During hw readout no sw updates should be queued anyway.. > Is there any documentation to say that this function can't ever be > called outside of sanitize_crtc? Perhaps rename the function to reflect > its usage? There's no documentation, but all updates outside this function are done with atomic updates and this function is the only one that doesn't update the atomic state. If you use it, you better have a good reason to.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d3a857ce1cb6..5a462e9a4d14 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6187,10 +6187,8 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) if (!intel_crtc->active) return; - if (to_intel_plane_state(crtc->primary->state)->visible) { - intel_crtc_wait_for_pending_flips(crtc); + if (to_intel_plane_state(crtc->primary->state)->visible) intel_pre_disable_primary(crtc); - } intel_crtc_disable_planes(crtc, crtc->state->plane_mask); dev_priv->display.crtc_disable(crtc);
Now that intel_display_suspend is atomic it's safe to remove wait_for_pending_flips from intel_crtc_disable_noatomic. It will only be used during hw load or resume, in which case there will be no pending flips anyway. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)