| Message ID | 1465388359-8070-8-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 08-06-16 om 14:19 schreef Daniel Vetter: > Split out from my big nonblocking atomic commit helper code as prep > work. While add it, also add some neat asciiart to document how it's > supposed to be used. > > v2: Resurrect misplaced hunk in the kerneldoc. > > Tested-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Tomeu Vizoso <tomeu.vizoso@gmail.com> > Cc: Daniel Stone <daniels@collabora.com> > Tested-by: Liviu Dudau <Liviu.Dudau@arm.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
On Wed, Jun 08, 2016 at 02:19:00PM +0200, Daniel Vetter wrote: > Split out from my big nonblocking atomic commit helper code as prep > work. While add it, also add some neat asciiart to document how it's > supposed to be used. > > v2: Resurrect misplaced hunk in the kerneldoc. > > Tested-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Tomeu Vizoso <tomeu.vizoso@gmail.com> > Cc: Daniel Stone <daniels@collabora.com> > Tested-by: Liviu Dudau <Liviu.Dudau@arm.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/drm_atomic.c | 22 +++++++ > drivers/gpu/drm/drm_crtc.c | 3 + > drivers/gpu/drm/drm_fops.c | 6 ++ > include/drm/drmP.h | 1 + > include/drm/drm_atomic.h | 6 ++ > include/drm/drm_crtc.h | 149 +++++++++++++++++++++++++++++++++++++++++-- > 6 files changed, 181 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 5e4b820a977c..d99ab2f6663f 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -33,6 +33,20 @@ > > #include "drm_crtc_internal.h" > > +static void crtc_commit_free(struct kref *kref) > +{ > + struct drm_crtc_commit *commit = > + container_of(kref, struct drm_crtc_commit, ref); > + > + kfree(commit); > +} > + > +void drm_crtc_commit_put(struct drm_crtc_commit *commit) > +{ > + kref_put(&commit->ref, crtc_commit_free); > +} > +EXPORT_SYMBOL(drm_crtc_commit_put); > + > /** > * drm_atomic_state_default_release - > * release memory initialized by drm_atomic_state_init > @@ -148,6 +162,14 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) > > crtc->funcs->atomic_destroy_state(crtc, > state->crtcs[i].state); > + > + if (state->crtcs[i].commit) { > + kfree(state->crtcs[i].commit->event); > + state->crtcs[i].commit->event = NULL; > + drm_crtc_commit_put(state->crtcs[i].commit); > + } > + > + state->crtcs[i].commit = NULL; > state->crtcs[i].ptr = NULL; > state->crtcs[i].state = NULL; > } > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index d3d0b4162e76..ce0569c3f942 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -669,6 +669,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, > crtc->dev = dev; > crtc->funcs = funcs; > > + INIT_LIST_HEAD(&crtc->commit_list); > + spin_lock_init(&crtc->commit_lock); > + > drm_modeset_lock_init(&crtc->mutex); > ret = drm_mode_object_get(dev, &crtc->base, DRM_MODE_OBJECT_CRTC); > if (ret) > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c > index 44b3ecdeca63..323c238fcac7 100644 > --- a/drivers/gpu/drm/drm_fops.c > +++ b/drivers/gpu/drm/drm_fops.c > @@ -686,6 +686,12 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) > { > assert_spin_locked(&dev->event_lock); > > + if (e->completion) { > + /* ->completion might disappear as soon as it signalled. */ > + complete_all(e->completion); > + e->completion = NULL; > + } > + > if (e->fence) { > fence_signal(e->fence); > fence_put(e->fence); > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 15fe4a21a9da..dce655abd23d 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -283,6 +283,7 @@ struct drm_ioctl_desc { > > /* Event queued up for userspace to read */ > struct drm_pending_event { > + struct completion *completion; > struct drm_event *event; > struct fence *fence; > struct list_head link; > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index a16861c882aa..856a9c85a838 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -30,6 +30,12 @@ > > #include <drm/drm_crtc.h> > > +void drm_crtc_commit_put(struct drm_crtc_commit *commit); > +static inline void drm_crtc_commit_get(struct drm_crtc_commit *commit) > +{ > + kref_get(&commit->ref); > +} > + > struct drm_atomic_state * __must_check > drm_atomic_state_alloc(struct drm_device *dev); > void drm_atomic_state_clear(struct drm_atomic_state *state); > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 520abafc9d00..14aa8212e30f 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -727,9 +727,6 @@ struct drm_crtc_funcs { > * @gamma_store: gamma ramp values > * @helper_private: mid-layer private data > * @properties: property tracking for this CRTC > - * @state: current atomic state for this CRTC > - * @acquire_ctx: per-CRTC implicit acquire context used by atomic drivers for > - * legacy IOCTLs > * > * Each CRTC may have one or more connectors associated with it. This structure > * allows the CRTC to be controlled. > @@ -786,11 +783,37 @@ struct drm_crtc { > > struct drm_object_properties properties; > > + /** > + * @state: > + * > + * Current atomic state for this CRTC. > + */ > struct drm_crtc_state *state; > > - /* > - * For legacy crtc IOCTLs so that atomic drivers can get at the locking > - * acquire context. > + /** > + * @commit_list: > + * > + * List of &drm_crtc_commit structures tracking pending commits. > + * Protected by @commit_lock. This list doesn't hold its own full > + * reference, but burrows it from the ongoing commit. Commit entries > + * must be removed from this list once the commit is fully completed, > + * but before it's correspoding &drm_atomic_state gets destroyed. > + */ > + struct list_head commit_list; > + > + /** > + * @commit_lock: > + * > + * Spinlock to protect @commit_list. > + */ > + spinlock_t commit_lock; > + > + /** > + * @acquire_ctx: > + * > + * Per-CRTC implicit acquire context used by atomic drivers for legacy > + * IOCTLs, so that atomic drivers can get at the locking acquire > + * context. > */ > struct drm_modeset_acquire_ctx *acquire_ctx; > }; > @@ -1732,6 +1755,111 @@ struct drm_bridge { > void *driver_private; > }; > > +/** > + * struct drm_crtc_commit - track modeset commits on a CRTC > + * > + * This structure is used to track pending modeset changes and atomic commit on > + * a per-CRTC basis. Since updating the list should never block this structure > + * is reference counted to allow waiters to safely wait on an event to complete, > + * without holding any locks. > + * > + * It has 3 different events in total to allow a fine-grained synchronoization spelling police: synchronization > + * between outstanding updates:: > + * > + * atomic commit thread hardware > + * > + * write new state into hardware ----> ... > + * signal hw_done > + * switch to new state on next > + * ... v/hblank > + * > + * wait for buffers to show up ... > + * > + * ... send completion irq > + * irq handler signals flip_done > + * cleanup old buffers > + * > + * signal cleanup_done > + * > + * wait for flip_done <---- > + * clean up atomic state > + * > + * The important bit to know is that cleanup_done is the terminal event, but the > + * ordering between flip_done and hw_done is entirely up to the specific driver > + * and modeset state change. > + * > + * For an implementation of how to use this look at > + * drm_atomic_helper_setup_commit() from the atomic helper library. > + */ > +struct drm_crtc_commit { > + /** > + * @crtc: > + * > + * DRM CRTC for this commit. > + */ > + struct drm_crtc *crtc; > + > + /** > + * @ref: > + * > + * Reference count for this structure. Needed to allow blocking on > + * completions without the risk of the completion disappearing > + * meanwhile. > + */ > + struct kref ref; > + > + /** > + * @flip_done: > + * > + * Will be signaled when the hardware has flipped to the new set of > + * buffers. Signals at the same time as when the drm event for this > + * commit is sent to userspace, or when an out-fence is singalled. Note > + * that for most hardware, in most cases this happens after @hw_done is > + * signalled. > + */ > + struct completion flip_done; > + > + /** > + * @hw_done: > + * > + * Will be signalled when all hw register changes for this commit have > + * been written out. Especially when disabling a pipe this can be much > + * later than than @flip_done, since that can signal already when the > + * screen goes black, whereas to fully shut down a pipe more register > + * I/O is required. > + * > + * Note that this does not need to include separately reference-counted > + * resources like backing storage buffer pinning, or runtime pm > + * management. > + */ > + struct completion hw_done; > + > + /** > + * @cleanup_done: > + * > + * Will be signalled after old buffers have been cleaned up again by what do you mean by "cleaned up *again*" ? If you are trying to emphasize the following sentence, I think you can drop "again" from it, but again, I'm only a software developer :) Best regards, Liviu > + * calling drm_atomic_helper_cleanup_planes(). Since this can only > + * happen after a vblank wait completed it might be a bit later. This > + * completion is useful to throttle updates and avoid hardware updates > + * getting ahead of the buffer cleanup too much. > + */ > + struct completion cleanup_done; > + > + /** > + * @commit_entry: > + * > + * Entry on the per-CRTC commit_list. Protected by crtc->commit_lock. > + */ > + struct list_head commit_entry; > + > + /** > + * @event: > + * > + * &drm_pending_vblank_event pointer to clean up private events. > + */ > + struct drm_pending_vblank_event *event; > +}; > + > struct __drm_planes_state { > struct drm_plane *ptr; > struct drm_plane_state *state; > @@ -1740,6 +1868,7 @@ struct __drm_planes_state { > struct __drm_crtcs_state { > struct drm_crtc *ptr; > struct drm_crtc_state *state; > + struct drm_crtc_commit *commit; > }; > > struct __drm_connnectors_state { > @@ -1770,6 +1899,14 @@ struct drm_atomic_state { > struct __drm_connnectors_state *connectors; > > struct drm_modeset_acquire_ctx *acquire_ctx; > + > + /** > + * @commit_work: > + * > + * Work item which can be used by the driver or helpers to execute the > + * commit without blocking. > + */ > + struct work_struct commit_work; > }; > > > -- > 2.8.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 5e4b820a977c..d99ab2f6663f 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -33,6 +33,20 @@ #include "drm_crtc_internal.h" +static void crtc_commit_free(struct kref *kref) +{ + struct drm_crtc_commit *commit = + container_of(kref, struct drm_crtc_commit, ref); + + kfree(commit); +} + +void drm_crtc_commit_put(struct drm_crtc_commit *commit) +{ + kref_put(&commit->ref, crtc_commit_free); +} +EXPORT_SYMBOL(drm_crtc_commit_put); + /** * drm_atomic_state_default_release - * release memory initialized by drm_atomic_state_init @@ -148,6 +162,14 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) crtc->funcs->atomic_destroy_state(crtc, state->crtcs[i].state); + + if (state->crtcs[i].commit) { + kfree(state->crtcs[i].commit->event); + state->crtcs[i].commit->event = NULL; + drm_crtc_commit_put(state->crtcs[i].commit); + } + + state->crtcs[i].commit = NULL; state->crtcs[i].ptr = NULL; state->crtcs[i].state = NULL; } diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d3d0b4162e76..ce0569c3f942 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -669,6 +669,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, crtc->dev = dev; crtc->funcs = funcs; + INIT_LIST_HEAD(&crtc->commit_list); + spin_lock_init(&crtc->commit_lock); + drm_modeset_lock_init(&crtc->mutex); ret = drm_mode_object_get(dev, &crtc->base, DRM_MODE_OBJECT_CRTC); if (ret) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 44b3ecdeca63..323c238fcac7 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -686,6 +686,12 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) { assert_spin_locked(&dev->event_lock); + if (e->completion) { + /* ->completion might disappear as soon as it signalled. */ + complete_all(e->completion); + e->completion = NULL; + } + if (e->fence) { fence_signal(e->fence); fence_put(e->fence); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 15fe4a21a9da..dce655abd23d 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -283,6 +283,7 @@ struct drm_ioctl_desc { /* Event queued up for userspace to read */ struct drm_pending_event { + struct completion *completion; struct drm_event *event; struct fence *fence; struct list_head link; diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index a16861c882aa..856a9c85a838 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -30,6 +30,12 @@ #include <drm/drm_crtc.h> +void drm_crtc_commit_put(struct drm_crtc_commit *commit); +static inline void drm_crtc_commit_get(struct drm_crtc_commit *commit) +{ + kref_get(&commit->ref); +} + struct drm_atomic_state * __must_check drm_atomic_state_alloc(struct drm_device *dev); void drm_atomic_state_clear(struct drm_atomic_state *state); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 520abafc9d00..14aa8212e30f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -727,9 +727,6 @@ struct drm_crtc_funcs { * @gamma_store: gamma ramp values * @helper_private: mid-layer private data * @properties: property tracking for this CRTC - * @state: current atomic state for this CRTC - * @acquire_ctx: per-CRTC implicit acquire context used by atomic drivers for - * legacy IOCTLs * * Each CRTC may have one or more connectors associated with it. This structure * allows the CRTC to be controlled. @@ -786,11 +783,37 @@ struct drm_crtc { struct drm_object_properties properties; + /** + * @state: + * + * Current atomic state for this CRTC. + */ struct drm_crtc_state *state; - /* - * For legacy crtc IOCTLs so that atomic drivers can get at the locking - * acquire context. + /** + * @commit_list: + * + * List of &drm_crtc_commit structures tracking pending commits. + * Protected by @commit_lock. This list doesn't hold its own full + * reference, but burrows it from the ongoing commit. Commit entries + * must be removed from this list once the commit is fully completed, + * but before it's correspoding &drm_atomic_state gets destroyed. + */ + struct list_head commit_list; + + /** + * @commit_lock: + * + * Spinlock to protect @commit_list. + */ + spinlock_t commit_lock; + + /** + * @acquire_ctx: + * + * Per-CRTC implicit acquire context used by atomic drivers for legacy + * IOCTLs, so that atomic drivers can get at the locking acquire + * context. */ struct drm_modeset_acquire_ctx *acquire_ctx; }; @@ -1732,6 +1755,111 @@ struct drm_bridge { void *driver_private; }; +/** + * struct drm_crtc_commit - track modeset commits on a CRTC + * + * This structure is used to track pending modeset changes and atomic commit on + * a per-CRTC basis. Since updating the list should never block this structure + * is reference counted to allow waiters to safely wait on an event to complete, + * without holding any locks. + * + * It has 3 different events in total to allow a fine-grained synchronoization + * between outstanding updates:: + * + * atomic commit thread hardware + * + * write new state into hardware ----> ... + * signal hw_done + * switch to new state on next + * ... v/hblank + * + * wait for buffers to show up ... + * + * ... send completion irq + * irq handler signals flip_done + * cleanup old buffers + * + * signal cleanup_done + * + * wait for flip_done <---- + * clean up atomic state + * + * The important bit to know is that cleanup_done is the terminal event, but the + * ordering between flip_done and hw_done is entirely up to the specific driver + * and modeset state change. + * + * For an implementation of how to use this look at + * drm_atomic_helper_setup_commit() from the atomic helper library. + */ +struct drm_crtc_commit { + /** + * @crtc: + * + * DRM CRTC for this commit. + */ + struct drm_crtc *crtc; + + /** + * @ref: + * + * Reference count for this structure. Needed to allow blocking on + * completions without the risk of the completion disappearing + * meanwhile. + */ + struct kref ref; + + /** + * @flip_done: + * + * Will be signaled when the hardware has flipped to the new set of + * buffers. Signals at the same time as when the drm event for this + * commit is sent to userspace, or when an out-fence is singalled. Note + * that for most hardware, in most cases this happens after @hw_done is + * signalled. + */ + struct completion flip_done; + + /** + * @hw_done: + * + * Will be signalled when all hw register changes for this commit have + * been written out. Especially when disabling a pipe this can be much + * later than than @flip_done, since that can signal already when the + * screen goes black, whereas to fully shut down a pipe more register + * I/O is required. + * + * Note that this does not need to include separately reference-counted + * resources like backing storage buffer pinning, or runtime pm + * management. + */ + struct completion hw_done; + + /** + * @cleanup_done: + * + * Will be signalled after old buffers have been cleaned up again by + * calling drm_atomic_helper_cleanup_planes(). Since this can only + * happen after a vblank wait completed it might be a bit later. This + * completion is useful to throttle updates and avoid hardware updates + * getting ahead of the buffer cleanup too much. + */ + struct completion cleanup_done; + + /** + * @commit_entry: + * + * Entry on the per-CRTC commit_list. Protected by crtc->commit_lock. + */ + struct list_head commit_entry; + + /** + * @event: + * + * &drm_pending_vblank_event pointer to clean up private events. + */ + struct drm_pending_vblank_event *event; +}; + struct __drm_planes_state { struct drm_plane *ptr; struct drm_plane_state *state; @@ -1740,6 +1868,7 @@ struct __drm_planes_state { struct __drm_crtcs_state { struct drm_crtc *ptr; struct drm_crtc_state *state; + struct drm_crtc_commit *commit; }; struct __drm_connnectors_state { @@ -1770,6 +1899,14 @@ struct drm_atomic_state { struct __drm_connnectors_state *connectors; struct drm_modeset_acquire_ctx *acquire_ctx; + + /** + * @commit_work: + * + * Work item which can be used by the driver or helpers to execute the + * commit without blocking. + */ + struct work_struct commit_work; };