| Message ID | 20170614090842.9669-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 14, 2017 at 11:08:41AM +0200, Maarten Lankhorst wrote: > Moving the wait to a helper allows callers outside of the core to > wait for pending updates to complete. > > This can be used to prevent races against nonblocking modesets. > Only the hw_done call in swap_state is moved to a helper, doing > it for the other callers requires too many changes and I think this > is the only useful one to export. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++---------- > include/drm/drm_atomic_helper.h | 1 + > 2 files changed, 38 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 86d3093c6c9b..1b068ce1aea9 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2067,6 +2067,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, > EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes); > > /** > + * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete. > + * @crtc: crtc to wait on. > + * > + * This function waits for all previous hardware updates queued on @crtc > + * to complete. > + * > + * Returns: > + * 0 on success, negative error code on failure. > + */ > +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc) > +{ > + struct drm_crtc_commit *commit; > + long ret; > + > + spin_lock(&crtc->commit_lock); > + commit = list_first_entry_or_null(&crtc->commit_list, > + struct drm_crtc_commit, commit_entry); > + if (commit) > + drm_crtc_commit_get(commit); > + spin_unlock(&crtc->commit_lock); > + > + if (!commit) > + return 0; > + > + ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ); > + drm_crtc_commit_put(commit); > + > + if (ret == 0) > + return -EBUSY; -ETIMEDOUT ? > + > + return ret > 0 ? 0 : ret; > +} > +EXPORT_SYMBOL(drm_atomic_helper_wait_for_hw_done); > + > +/** > * drm_atomic_helper_swap_state - store atomic state into current sw state > * @state: atomic state > * @stall: stall for proceeding commits > @@ -2107,28 +2142,16 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, > struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct drm_plane *plane; > struct drm_plane_state *old_plane_state, *new_plane_state; > - struct drm_crtc_commit *commit; > void *obj, *obj_state; > const struct drm_private_state_funcs *funcs; > > if (stall) { > for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { > - spin_lock(&crtc->commit_lock); > - commit = list_first_entry_or_null(&crtc->commit_list, > - struct drm_crtc_commit, commit_entry); > - if (commit) > - drm_crtc_commit_get(commit); > - spin_unlock(&crtc->commit_lock); > + ret = drm_atomic_helper_wait_for_hw_done(crtc); > > - if (!commit) > - continue; > - > - ret = wait_for_completion_timeout(&commit->hw_done, > - 10*HZ); > - if (ret == 0) > + if (ret < 0) > DRM_ERROR("[CRTC:%d:%s] hw_done timed out\n", > crtc->base.id, crtc->name); > - drm_crtc_commit_put(commit); > } > } > > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index f0a8678ae98e..175a1a3fed36 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -77,6 +77,7 @@ void > drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state, > bool atomic); > > +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc); > void drm_atomic_helper_swap_state(struct drm_atomic_state *state, > bool stall); > > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 14-06-17 om 16:52 schreef Ville Syrjälä: > On Wed, Jun 14, 2017 at 11:08:41AM +0200, Maarten Lankhorst wrote: >> Moving the wait to a helper allows callers outside of the core to >> wait for pending updates to complete. >> >> This can be used to prevent races against nonblocking modesets. >> Only the hw_done call in swap_state is moved to a helper, doing >> it for the other callers requires too many changes and I think this >> is the only useful one to export. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++---------- >> include/drm/drm_atomic_helper.h | 1 + >> 2 files changed, 38 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c >> index 86d3093c6c9b..1b068ce1aea9 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c >> @@ -2067,6 +2067,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, >> EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes); >> >> /** >> + * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete. >> + * @crtc: crtc to wait on. >> + * >> + * This function waits for all previous hardware updates queued on @crtc >> + * to complete. >> + * >> + * Returns: >> + * 0 on success, negative error code on failure. >> + */ >> +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc) >> +{ >> + struct drm_crtc_commit *commit; >> + long ret; >> + >> + spin_lock(&crtc->commit_lock); >> + commit = list_first_entry_or_null(&crtc->commit_list, >> + struct drm_crtc_commit, commit_entry); >> + if (commit) >> + drm_crtc_commit_get(commit); >> + spin_unlock(&crtc->commit_lock); >> + >> + if (!commit) >> + return 0; >> + >> + ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ); >> + drm_crtc_commit_put(commit); >> + >> + if (ret == 0) >> + return -EBUSY; > -ETIMEDOUT ? ETIMEDOUT -> /* Connection timed out */ No I don't think that would match.
On Wed, Jun 14, 2017 at 08:11:18PM +0200, Maarten Lankhorst wrote: > Op 14-06-17 om 16:52 schreef Ville Syrjälä: > > On Wed, Jun 14, 2017 at 11:08:41AM +0200, Maarten Lankhorst wrote: > >> Moving the wait to a helper allows callers outside of the core to > >> wait for pending updates to complete. > >> > >> This can be used to prevent races against nonblocking modesets. > >> Only the hw_done call in swap_state is moved to a helper, doing > >> it for the other callers requires too many changes and I think this > >> is the only useful one to export. > >> > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >> --- > >> drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++---------- > >> include/drm/drm_atomic_helper.h | 1 + > >> 2 files changed, 38 insertions(+), 14 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > >> index 86d3093c6c9b..1b068ce1aea9 100644 > >> --- a/drivers/gpu/drm/drm_atomic_helper.c > >> +++ b/drivers/gpu/drm/drm_atomic_helper.c > >> @@ -2067,6 +2067,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, > >> EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes); > >> > >> /** > >> + * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete. > >> + * @crtc: crtc to wait on. > >> + * > >> + * This function waits for all previous hardware updates queued on @crtc > >> + * to complete. > >> + * > >> + * Returns: > >> + * 0 on success, negative error code on failure. > >> + */ > >> +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc) > >> +{ > >> + struct drm_crtc_commit *commit; > >> + long ret; > >> + > >> + spin_lock(&crtc->commit_lock); > >> + commit = list_first_entry_or_null(&crtc->commit_list, > >> + struct drm_crtc_commit, commit_entry); > >> + if (commit) > >> + drm_crtc_commit_get(commit); > >> + spin_unlock(&crtc->commit_lock); > >> + > >> + if (!commit) > >> + return 0; > >> + > >> + ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ); > >> + drm_crtc_commit_put(commit); > >> + > >> + if (ret == 0) > >> + return -EBUSY; > > -ETIMEDOUT ? > ETIMEDOUT -> /* Connection timed out */ It's the standard "something timed out" errno. Used all over drm. > > No I don't think that would match.
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 86d3093c6c9b..1b068ce1aea9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2067,6 +2067,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes); /** + * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete. + * @crtc: crtc to wait on. + * + * This function waits for all previous hardware updates queued on @crtc + * to complete. + * + * Returns: + * 0 on success, negative error code on failure. + */ +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc) +{ + struct drm_crtc_commit *commit; + long ret; + + spin_lock(&crtc->commit_lock); + commit = list_first_entry_or_null(&crtc->commit_list, + struct drm_crtc_commit, commit_entry); + if (commit) + drm_crtc_commit_get(commit); + spin_unlock(&crtc->commit_lock); + + if (!commit) + return 0; + + ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ); + drm_crtc_commit_put(commit); + + if (ret == 0) + return -EBUSY; + + return ret > 0 ? 0 : ret; +} +EXPORT_SYMBOL(drm_atomic_helper_wait_for_hw_done); + +/** * drm_atomic_helper_swap_state - store atomic state into current sw state * @state: atomic state * @stall: stall for proceeding commits @@ -2107,28 +2142,16 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; - struct drm_crtc_commit *commit; void *obj, *obj_state; const struct drm_private_state_funcs *funcs; if (stall) { for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { - spin_lock(&crtc->commit_lock); - commit = list_first_entry_or_null(&crtc->commit_list, - struct drm_crtc_commit, commit_entry); - if (commit) - drm_crtc_commit_get(commit); - spin_unlock(&crtc->commit_lock); + ret = drm_atomic_helper_wait_for_hw_done(crtc); - if (!commit) - continue; - - ret = wait_for_completion_timeout(&commit->hw_done, - 10*HZ); - if (ret == 0) + if (ret < 0) DRM_ERROR("[CRTC:%d:%s] hw_done timed out\n", crtc->base.id, crtc->name); - drm_crtc_commit_put(commit); } } diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index f0a8678ae98e..175a1a3fed36 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -77,6 +77,7 @@ void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state, bool atomic); +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc); void drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall);
Moving the wait to a helper allows callers outside of the core to wait for pending updates to complete. This can be used to prevent races against nonblocking modesets. Only the hw_done call in swap_state is moved to a helper, doing it for the other callers requires too many changes and I think this is the only useful one to export. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++---------- include/drm/drm_atomic_helper.h | 1 + 2 files changed, 38 insertions(+), 14 deletions(-)