Hello,
This is an attempt at simplifying the async page flip handling in VC4 in order to get rid of vc4_queue_seqno_cb() and its dependencies and rely on fences instead.
The reason I'm sending this as an RFC is because I'm pretty sure we can put some of the code added in patch 1 in drm_atomic_helper.c. Also, I'd like to have feedback from Padovan, Daniel and maybe others who have already thought about handling async page flips generically. And the last reason is that it's not been extensively tested, so it may not work correctly :-).
Regards,
Boris
Boris Brezillon (2): drm/vc4: Handle async page flips in the atomic_commit() path drm/vc4: Get rid of vc4_queue_seqno_cb() and its dependencies
drivers/gpu/drm/vc4/vc4_crtc.c | 103 +---------------------------------------- drivers/gpu/drm/vc4/vc4_drv.h | 14 ------ drivers/gpu/drm/vc4/vc4_gem.c | 39 ---------------- drivers/gpu/drm/vc4/vc4_kms.c | 101 +++++++++++++++++++++++++++++++++------- 4 files changed, 86 insertions(+), 171 deletions(-)
Rework the atomic commit logic to handle async page flip requests in the atomic update path.
Async page flips are a bit more complicated than async cursor updates (already handled in the vc4_atomic_commit() function) in that you need to wait for fences on the new framebuffers to be signaled before doing the flip. In order to ensure that, we schedule a commit_work work to do the async update (note that the existing implementation already uses a work to wait for fences to be signaled, so, the latency shouldn't change that much).
Of course, we have to modify a bit the atomic_complete_commit() implementation to avoid waiting for things we don't care about when doing an async page flip:
* drm_atomic_helper_wait_for_dependencies() waits for flip_done which we don't care about when doing async page flips. Instead we replace that call by a loop that waits for hw_done only * drm_atomic_helper_commit_modeset_disables() and drm_atomic_helper_commit_modeset_enables() are not needed because nothing except the planes' FBs are updated in async page flips * drm_atomic_helper_commit_planes() is replaced by vc4_plane_async_set_fb() which is doing only the FB update and is thus much simpler * drm_atomic_helper_wait_for_vblanks() is not needed because we don't wait for the next VBLANK period to apply the new state, and flip events are signaled manually just after the HW has been updated
Thanks to this rework, we can get rid of the custom vc4_page_flip() implementation and its dependencies and use drm_atomic_helper_page_flip() instead.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com --- drivers/gpu/drm/vc4/vc4_crtc.c | 103 +---------------------------------------- drivers/gpu/drm/vc4/vc4_kms.c | 101 +++++++++++++++++++++++++++++++++------- 2 files changed, 86 insertions(+), 118 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index bf4667481935..3843c39dce61 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -757,107 +757,6 @@ static irqreturn_t vc4_crtc_irq_handler(int irq, void *data) return ret; }
-struct vc4_async_flip_state { - struct drm_crtc *crtc; - struct drm_framebuffer *fb; - struct drm_pending_vblank_event *event; - - struct vc4_seqno_cb cb; -}; - -/* Called when the V3D execution for the BO being flipped to is done, so that - * we can actually update the plane's address to point to it. - */ -static void -vc4_async_page_flip_complete(struct vc4_seqno_cb *cb) -{ - struct vc4_async_flip_state *flip_state = - container_of(cb, struct vc4_async_flip_state, cb); - struct drm_crtc *crtc = flip_state->crtc; - struct drm_device *dev = crtc->dev; - struct vc4_dev *vc4 = to_vc4_dev(dev); - struct drm_plane *plane = crtc->primary; - - vc4_plane_async_set_fb(plane, flip_state->fb); - if (flip_state->event) { - unsigned long flags; - - spin_lock_irqsave(&dev->event_lock, flags); - drm_crtc_send_vblank_event(crtc, flip_state->event); - spin_unlock_irqrestore(&dev->event_lock, flags); - } - - drm_crtc_vblank_put(crtc); - drm_framebuffer_put(flip_state->fb); - kfree(flip_state); - - up(&vc4->async_modeset); -} - -/* Implements async (non-vblank-synced) page flips. - * - * The page flip ioctl needs to return immediately, so we grab the - * modeset semaphore on the pipe, and queue the address update for - * when V3D is done with the BO being flipped to. - */ -static int vc4_async_page_flip(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event, - uint32_t flags) -{ - struct drm_device *dev = crtc->dev; - struct vc4_dev *vc4 = to_vc4_dev(dev); - struct drm_plane *plane = crtc->primary; - int ret = 0; - struct vc4_async_flip_state *flip_state; - struct drm_gem_cma_object *cma_bo = drm_fb_cma_get_gem_obj(fb, 0); - struct vc4_bo *bo = to_vc4_bo(&cma_bo->base); - - flip_state = kzalloc(sizeof(*flip_state), GFP_KERNEL); - if (!flip_state) - return -ENOMEM; - - drm_framebuffer_get(fb); - flip_state->fb = fb; - flip_state->crtc = crtc; - flip_state->event = event; - - /* Make sure all other async modesetes have landed. */ - ret = down_interruptible(&vc4->async_modeset); - if (ret) { - drm_framebuffer_put(fb); - kfree(flip_state); - return ret; - } - - WARN_ON(drm_crtc_vblank_get(crtc) != 0); - - /* Immediately update the plane's legacy fb pointer, so that later - * modeset prep sees the state that will be present when the semaphore - * is released. - */ - drm_atomic_set_fb_for_plane(plane->state, fb); - plane->fb = fb; - - vc4_queue_seqno_cb(dev, &flip_state->cb, bo->seqno, - vc4_async_page_flip_complete); - - /* Driver takes ownership of state on successful async commit. */ - return 0; -} - -static int vc4_page_flip(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event, - uint32_t flags, - struct drm_modeset_acquire_ctx *ctx) -{ - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) - return vc4_async_page_flip(crtc, fb, event, flags); - else - return drm_atomic_helper_page_flip(crtc, fb, event, flags, ctx); -} - static struct drm_crtc_state *vc4_crtc_duplicate_state(struct drm_crtc *crtc) { struct vc4_crtc_state *vc4_state; @@ -902,7 +801,7 @@ vc4_crtc_reset(struct drm_crtc *crtc) static const struct drm_crtc_funcs vc4_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .destroy = vc4_crtc_destroy, - .page_flip = vc4_page_flip, + .page_flip = drm_atomic_helper_page_flip, .set_property = NULL, .cursor_set = NULL, /* handled by drm_mode_cursor_universal */ .cursor_move = NULL, /* handled by drm_mode_cursor_universal */ diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index e791e498a3dd..faa2c26f1235 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -25,33 +25,89 @@ #include "vc4_drv.h"
static void -vc4_atomic_complete_commit(struct drm_atomic_state *state) +vc4_atomic_complete_commit(struct drm_atomic_state *state, bool async) { struct drm_device *dev = state->dev; struct vc4_dev *vc4 = to_vc4_dev(dev);
drm_atomic_helper_wait_for_fences(dev, state, false);
- drm_atomic_helper_wait_for_dependencies(state); + if (async) { + struct drm_plane_state *plane_state; + struct drm_crtc_state *new_crtc_state; + struct drm_plane *plane; + struct drm_crtc *crtc; + int i; + + /* We need to wait for HW done before applying the new FBs + * otherwise our update might be overwritten by the previous + * commit. + */ + for_each_old_plane_in_state(state, plane, plane_state, i) { + struct drm_crtc_commit *commit = plane_state->commit; + int ret; + + if (!commit) + continue; + + ret = wait_for_completion_timeout(&commit->hw_done, + 10 * HZ); + if (!ret) + DRM_ERROR("[PLANE:%d:%s] hw_done timed out\n", + plane->base.id, plane->name); + }
- drm_atomic_helper_commit_modeset_disables(dev, state); + /* FIXME: + * We could use drm_atomic_helper_async_commit() here, but + * VC4's ->atomic_async_update() implementation expects + * plane->state to point to the old_state and old/new states + * have already been swapped. + * Let's just call our custom function for now and see how we + * can make that more generic afterwards. + */ + for_each_new_plane_in_state(state, plane, plane_state, i) + vc4_plane_async_set_fb(plane, plane_state->fb); + + /* Now, send 'fake' VBLANK events to let the user now its + * pageflip is done. By fake I mean they are really VBLANK + * synchronized but it seems to be expected by the core. + */ + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { + unsigned long flags; + + if (!new_crtc_state->event) + continue; + + WARN_ON(drm_crtc_vblank_get(crtc)); + spin_lock_irqsave(&dev->event_lock, flags); + drm_crtc_send_vblank_event(crtc, new_crtc_state->event); + spin_unlock_irqrestore(&dev->event_lock, flags); + drm_crtc_vblank_put(crtc); + } + } else { + drm_atomic_helper_wait_for_dependencies(state);
- drm_atomic_helper_commit_planes(dev, state, 0); + drm_atomic_helper_commit_modeset_disables(dev, state);
- drm_atomic_helper_commit_modeset_enables(dev, state); + drm_atomic_helper_commit_planes(dev, state, 0);
- /* Make sure that drm_atomic_helper_wait_for_vblanks() - * actually waits for vblank. If we're doing a full atomic - * modeset (as opposed to a vc4_update_plane() short circuit), - * then we need to wait for scanout to be done with our - * display lists before we free it and potentially reallocate - * and overwrite the dlist memory with a new modeset. - */ - state->legacy_cursor_update = false; + drm_atomic_helper_commit_modeset_enables(dev, state); + }
drm_atomic_helper_commit_hw_done(state);
- drm_atomic_helper_wait_for_vblanks(dev, state); + if (!async) { + /* Make sure that drm_atomic_helper_wait_for_vblanks() + * actually waits for vblank. If we're doing a full atomic + * modeset (as opposed to a vc4_update_plane() short circuit), + * then we need to wait for scanout to be done with our + * display lists before we free it and potentially reallocate + * and overwrite the dlist memory with a new modeset. + */ + state->legacy_cursor_update = false; + + drm_atomic_helper_wait_for_vblanks(dev, state); + }
drm_atomic_helper_cleanup_planes(dev, state);
@@ -67,7 +123,20 @@ static void commit_work(struct work_struct *work) struct drm_atomic_state *state = container_of(work, struct drm_atomic_state, commit_work); - vc4_atomic_complete_commit(state); + struct drm_crtc_state *new_crtc_state; + bool async_commit = true; + struct drm_crtc *crtc; + int i; + + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { + if (new_crtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC) + continue; + + async_commit = false; + break; + } + + vc4_atomic_complete_commit(state, async_commit); }
/** @@ -163,7 +232,7 @@ static int vc4_atomic_commit(struct drm_device *dev, if (nonblock) queue_work(system_unbound_wq, &state->commit_work); else - vc4_atomic_complete_commit(state); + vc4_atomic_complete_commit(state, false);
return 0; }
On Fri, 30 Mar 2018 17:09:03 +0200 Boris Brezillon boris.brezillon@bootlin.com wrote:
Rework the atomic commit logic to handle async page flip requests in the atomic update path.
Async page flips are a bit more complicated than async cursor updates (already handled in the vc4_atomic_commit() function) in that you need to wait for fences on the new framebuffers to be signaled before doing the flip. In order to ensure that, we schedule a commit_work work to do the async update (note that the existing implementation already uses a work to wait for fences to be signaled, so, the latency shouldn't change that much).
Of course, we have to modify a bit the atomic_complete_commit() implementation to avoid waiting for things we don't care about when doing an async page flip:
- drm_atomic_helper_wait_for_dependencies() waits for flip_done which we don't care about when doing async page flips. Instead we replace that call by a loop that waits for hw_done only
- drm_atomic_helper_commit_modeset_disables() and drm_atomic_helper_commit_modeset_enables() are not needed because nothing except the planes' FBs are updated in async page flips
- drm_atomic_helper_commit_planes() is replaced by vc4_plane_async_set_fb() which is doing only the FB update and is thus much simpler
- drm_atomic_helper_wait_for_vblanks() is not needed because we don't wait for the next VBLANK period to apply the new state, and flip events are signaled manually just after the HW has been updated
Thanks to this rework, we can get rid of the custom vc4_page_flip() implementation and its dependencies and use drm_atomic_helper_page_flip() instead.
Another good reason for moving async page flip to the atomic commit path is that is fixes a bug we had: drm_atomic_helper_prepare/cleanup_planes() were not called in the async page flip path, which can lead to unbalanced vc4_inc/dec_usecnt() in some situations (when the plane is updated once with an async page flip and then with a regular update) or trigger a use after free if the BO passed to the plane is marked purgeable and the kernel decides to purge its cache.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com
drivers/gpu/drm/vc4/vc4_crtc.c | 103 +---------------------------------------- drivers/gpu/drm/vc4/vc4_kms.c | 101 +++++++++++++++++++++++++++++++++------- 2 files changed, 86 insertions(+), 118 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index bf4667481935..3843c39dce61 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -757,107 +757,6 @@ static irqreturn_t vc4_crtc_irq_handler(int irq, void *data) return ret; }
-struct vc4_async_flip_state {
- struct drm_crtc *crtc;
- struct drm_framebuffer *fb;
- struct drm_pending_vblank_event *event;
- struct vc4_seqno_cb cb;
-};
-/* Called when the V3D execution for the BO being flipped to is done, so that
- we can actually update the plane's address to point to it.
- */
-static void -vc4_async_page_flip_complete(struct vc4_seqno_cb *cb) -{
- struct vc4_async_flip_state *flip_state =
container_of(cb, struct vc4_async_flip_state, cb);- struct drm_crtc *crtc = flip_state->crtc;
- struct drm_device *dev = crtc->dev;
- struct vc4_dev *vc4 = to_vc4_dev(dev);
- struct drm_plane *plane = crtc->primary;
- vc4_plane_async_set_fb(plane, flip_state->fb);
- if (flip_state->event) {
unsigned long flags;spin_lock_irqsave(&dev->event_lock, flags);drm_crtc_send_vblank_event(crtc, flip_state->event);spin_unlock_irqrestore(&dev->event_lock, flags);- }
- drm_crtc_vblank_put(crtc);
- drm_framebuffer_put(flip_state->fb);
- kfree(flip_state);
- up(&vc4->async_modeset);
-}
-/* Implements async (non-vblank-synced) page flips.
- The page flip ioctl needs to return immediately, so we grab the
- modeset semaphore on the pipe, and queue the address update for
- when V3D is done with the BO being flipped to.
- */
-static int vc4_async_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,struct drm_pending_vblank_event *event,uint32_t flags)-{
- struct drm_device *dev = crtc->dev;
- struct vc4_dev *vc4 = to_vc4_dev(dev);
- struct drm_plane *plane = crtc->primary;
- int ret = 0;
- struct vc4_async_flip_state *flip_state;
- struct drm_gem_cma_object *cma_bo = drm_fb_cma_get_gem_obj(fb, 0);
- struct vc4_bo *bo = to_vc4_bo(&cma_bo->base);
- flip_state = kzalloc(sizeof(*flip_state), GFP_KERNEL);
- if (!flip_state)
return -ENOMEM;- drm_framebuffer_get(fb);
- flip_state->fb = fb;
- flip_state->crtc = crtc;
- flip_state->event = event;
- /* Make sure all other async modesetes have landed. */
- ret = down_interruptible(&vc4->async_modeset);
- if (ret) {
drm_framebuffer_put(fb);kfree(flip_state);return ret;- }
- WARN_ON(drm_crtc_vblank_get(crtc) != 0);
- /* Immediately update the plane's legacy fb pointer, so that later
* modeset prep sees the state that will be present when the semaphore* is released.*/- drm_atomic_set_fb_for_plane(plane->state, fb);
- plane->fb = fb;
- vc4_queue_seqno_cb(dev, &flip_state->cb, bo->seqno,
vc4_async_page_flip_complete);- /* Driver takes ownership of state on successful async commit. */
- return 0;
-}
-static int vc4_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,struct drm_pending_vblank_event *event,uint32_t flags,struct drm_modeset_acquire_ctx *ctx)-{
- if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
return vc4_async_page_flip(crtc, fb, event, flags);- else
return drm_atomic_helper_page_flip(crtc, fb, event, flags, ctx);-}
static struct drm_crtc_state *vc4_crtc_duplicate_state(struct drm_crtc *crtc) { struct vc4_crtc_state *vc4_state; @@ -902,7 +801,7 @@ vc4_crtc_reset(struct drm_crtc *crtc) static const struct drm_crtc_funcs vc4_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .destroy = vc4_crtc_destroy,
- .page_flip = vc4_page_flip,
- .page_flip = drm_atomic_helper_page_flip, .set_property = NULL, .cursor_set = NULL, /* handled by drm_mode_cursor_universal */ .cursor_move = NULL, /* handled by drm_mode_cursor_universal */
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index e791e498a3dd..faa2c26f1235 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -25,33 +25,89 @@ #include "vc4_drv.h"
static void -vc4_atomic_complete_commit(struct drm_atomic_state *state) +vc4_atomic_complete_commit(struct drm_atomic_state *state, bool async) { struct drm_device *dev = state->dev; struct vc4_dev *vc4 = to_vc4_dev(dev);
drm_atomic_helper_wait_for_fences(dev, state, false);
- drm_atomic_helper_wait_for_dependencies(state);
- if (async) {
struct drm_plane_state *plane_state;struct drm_crtc_state *new_crtc_state;struct drm_plane *plane;struct drm_crtc *crtc;int i;/* We need to wait for HW done before applying the new FBs* otherwise our update might be overwritten by the previous* commit.*/for_each_old_plane_in_state(state, plane, plane_state, i) {struct drm_crtc_commit *commit = plane_state->commit;int ret;if (!commit)continue;ret = wait_for_completion_timeout(&commit->hw_done,10 * HZ);if (!ret)DRM_ERROR("[PLANE:%d:%s] hw_done timed out\n",plane->base.id, plane->name);}
- drm_atomic_helper_commit_modeset_disables(dev, state);
/* FIXME:* We could use drm_atomic_helper_async_commit() here, but* VC4's ->atomic_async_update() implementation expects* plane->state to point to the old_state and old/new states* have already been swapped.* Let's just call our custom function for now and see how we* can make that more generic afterwards.*/for_each_new_plane_in_state(state, plane, plane_state, i)vc4_plane_async_set_fb(plane, plane_state->fb);/* Now, send 'fake' VBLANK events to let the user now its* pageflip is done. By fake I mean they are really VBLANK* synchronized but it seems to be expected by the core.*/for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {unsigned long flags;if (!new_crtc_state->event)continue;WARN_ON(drm_crtc_vblank_get(crtc));spin_lock_irqsave(&dev->event_lock, flags);drm_crtc_send_vblank_event(crtc, new_crtc_state->event);spin_unlock_irqrestore(&dev->event_lock, flags);drm_crtc_vblank_put(crtc);}- } else {
drm_atomic_helper_wait_for_dependencies(state);
- drm_atomic_helper_commit_planes(dev, state, 0);
drm_atomic_helper_commit_modeset_disables(dev, state);
- drm_atomic_helper_commit_modeset_enables(dev, state);
drm_atomic_helper_commit_planes(dev, state, 0);
- /* Make sure that drm_atomic_helper_wait_for_vblanks()
* actually waits for vblank. If we're doing a full atomic* modeset (as opposed to a vc4_update_plane() short circuit),* then we need to wait for scanout to be done with our* display lists before we free it and potentially reallocate* and overwrite the dlist memory with a new modeset.*/- state->legacy_cursor_update = false;
drm_atomic_helper_commit_modeset_enables(dev, state);}
drm_atomic_helper_commit_hw_done(state);
- drm_atomic_helper_wait_for_vblanks(dev, state);
if (!async) {
/* Make sure that drm_atomic_helper_wait_for_vblanks()* actually waits for vblank. If we're doing a full atomic* modeset (as opposed to a vc4_update_plane() short circuit),* then we need to wait for scanout to be done with our* display lists before we free it and potentially reallocate* and overwrite the dlist memory with a new modeset.*/state->legacy_cursor_update = false;drm_atomic_helper_wait_for_vblanks(dev, state);}
drm_atomic_helper_cleanup_planes(dev, state);
@@ -67,7 +123,20 @@ static void commit_work(struct work_struct *work) struct drm_atomic_state *state = container_of(work, struct drm_atomic_state, commit_work);
- vc4_atomic_complete_commit(state);
- struct drm_crtc_state *new_crtc_state;
- bool async_commit = true;
- struct drm_crtc *crtc;
- int i;
- for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
if (new_crtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC)continue;async_commit = false;break;- }
- vc4_atomic_complete_commit(state, async_commit);
}
/** @@ -163,7 +232,7 @@ static int vc4_atomic_commit(struct drm_device *dev, if (nonblock) queue_work(system_unbound_wq, &state->commit_work); else
vc4_atomic_complete_commit(state);
vc4_atomic_complete_commit(state, false);return 0;
}
The last user of vc4_queue_seqno_cb() (async page flip handling code) is gone. Get rid of this function and all the related structs and fields.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com --- drivers/gpu/drm/vc4/vc4_drv.h | 14 -------------- drivers/gpu/drm/vc4/vc4_gem.c | 39 --------------------------------------- 2 files changed, 53 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 1b4cd1fabf56..80ed7d02fcee 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -158,11 +158,6 @@ struct vc4_dev { */ struct vc4_perfmon *active_perfmon;
- /* List of struct vc4_seqno_cb for callbacks to be made from a - * workqueue when the given seqno is passed. - */ - struct list_head seqno_cb_list; - /* The memory used for storing binner tile alloc, tile state, * and overflow memory allocations. This is freed when V3D * powers down. @@ -271,12 +266,6 @@ to_vc4_fence(struct dma_fence *fence) return (struct vc4_fence *)fence; }
-struct vc4_seqno_cb { - struct work_struct work; - uint64_t seqno; - void (*func)(struct vc4_seqno_cb *cb); -}; - struct vc4_v3d { struct vc4_dev *vc4; struct platform_device *pdev; @@ -692,9 +681,6 @@ void vc4_move_job_to_render(struct drm_device *dev, struct vc4_exec_info *exec); int vc4_wait_for_seqno(struct drm_device *dev, uint64_t seqno, uint64_t timeout_ns, bool interruptible); void vc4_job_handle_completed(struct vc4_dev *vc4); -int vc4_queue_seqno_cb(struct drm_device *dev, - struct vc4_seqno_cb *cb, uint64_t seqno, - void (*func)(struct vc4_seqno_cb *cb)); int vc4_gem_madvise_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 2107b0daf8ef..93d52be1cd37 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -973,7 +973,6 @@ void vc4_job_handle_completed(struct vc4_dev *vc4) { unsigned long irqflags; - struct vc4_seqno_cb *cb, *cb_temp;
spin_lock_irqsave(&vc4->job_lock, irqflags); while (!list_empty(&vc4->job_done_list)) { @@ -987,46 +986,9 @@ vc4_job_handle_completed(struct vc4_dev *vc4) spin_lock_irqsave(&vc4->job_lock, irqflags); }
- list_for_each_entry_safe(cb, cb_temp, &vc4->seqno_cb_list, work.entry) { - if (cb->seqno <= vc4->finished_seqno) { - list_del_init(&cb->work.entry); - schedule_work(&cb->work); - } - } - spin_unlock_irqrestore(&vc4->job_lock, irqflags); }
-static void vc4_seqno_cb_work(struct work_struct *work) -{ - struct vc4_seqno_cb *cb = container_of(work, struct vc4_seqno_cb, work); - - cb->func(cb); -} - -int vc4_queue_seqno_cb(struct drm_device *dev, - struct vc4_seqno_cb *cb, uint64_t seqno, - void (*func)(struct vc4_seqno_cb *cb)) -{ - struct vc4_dev *vc4 = to_vc4_dev(dev); - int ret = 0; - unsigned long irqflags; - - cb->func = func; - INIT_WORK(&cb->work, vc4_seqno_cb_work); - - spin_lock_irqsave(&vc4->job_lock, irqflags); - if (seqno > vc4->finished_seqno) { - cb->seqno = seqno; - list_add_tail(&cb->work.entry, &vc4->seqno_cb_list); - } else { - schedule_work(&cb->work); - } - spin_unlock_irqrestore(&vc4->job_lock, irqflags); - - return ret; -} - /* Scheduled when any job has been completed, this walks the list of * jobs that had completed and unrefs their BOs and frees their exec * structs. @@ -1211,7 +1173,6 @@ vc4_gem_init(struct drm_device *dev) INIT_LIST_HEAD(&vc4->bin_job_list); INIT_LIST_HEAD(&vc4->render_job_list); INIT_LIST_HEAD(&vc4->job_done_list); - INIT_LIST_HEAD(&vc4->seqno_cb_list); spin_lock_init(&vc4->job_lock);
INIT_WORK(&vc4->hangcheck.reset_work, vc4_reset_work);
Boris Brezillon boris.brezillon@bootlin.com writes:
Hello,
This is an attempt at simplifying the async page flip handling in VC4 in order to get rid of vc4_queue_seqno_cb() and its dependencies and rely on fences instead.
The reason I'm sending this as an RFC is because I'm pretty sure we can put some of the code added in patch 1 in drm_atomic_helper.c. Also, I'd like to have feedback from Padovan, Daniel and maybe others who have already thought about handling async page flips generically. And the last reason is that it's not been extensively tested, so it may not work correctly :-).
I'm certainly eager to see our custom async code die. Any KMS core folks want to comment on this proposal?
dri-devel@lists.freedesktop.org
-
Boris Brezillon -
Eric Anholt