From patchwork Fri Mar 30 15:09:03 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 10317821 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4EFEB60212 for ; Fri, 30 Mar 2018 15:09:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3E1F92A5CC for ; Fri, 30 Mar 2018 15:09:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 32E962A5CE; Fri, 30 Mar 2018 15:09:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8A81D2A5CC for ; Fri, 30 Mar 2018 15:09:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6434D6E416; Fri, 30 Mar 2018 15:09:15 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail.bootlin.com (mail.bootlin.com [62.4.15.54]) by gabe.freedesktop.org (Postfix) with ESMTP id 6665F89F5F for ; Fri, 30 Mar 2018 15:09:09 +0000 (UTC) Received: by mail.bootlin.com (Postfix, from userid 110) id A75F320384; Fri, 30 Mar 2018 17:09:08 +0200 (CEST) Received: from localhost.localdomain (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr [90.63.216.87]) by mail.bootlin.com (Postfix) with ESMTPSA id 5B7EB20384; Fri, 30 Mar 2018 17:09:08 +0200 (CEST) From: Boris Brezillon To: David Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org, Eric Anholt , Gustavo Padovan Subject: [RFC 1/2] drm/vc4: Handle async page flips in the atomic_commit() path Date: Fri, 30 Mar 2018 17:09:03 +0200 Message-Id: <20180330150904.30458-2-boris.brezillon@bootlin.com> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20180330150904.30458-1-boris.brezillon@bootlin.com> References: <20180330150904.30458-1-boris.brezillon@bootlin.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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; }