From patchwork Fri Apr 12 12:58:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helen Mae Koike Fornazier X-Patchwork-Id: 10898247 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4641D17E6 for ; Fri, 12 Apr 2019 12:59:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2E58F28DB6 for ; Fri, 12 Apr 2019 12:59:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2271F28DBC; Fri, 12 Apr 2019 12:59:02 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 B577B28DB6 for ; Fri, 12 Apr 2019 12:59:01 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6AAFE899DE; Fri, 12 Apr 2019 12:59:00 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 223CE899DE for ; Fri, 12 Apr 2019 12:58:59 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a5e8:c168:522a:50d5:d06d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: koike) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 4F42A2811A6; Fri, 12 Apr 2019 13:58:53 +0100 (BST) From: Helen Koike To: dri-devel@lists.freedesktop.org, David Airlie Subject: [PATCH v3 1/4] drm/uapi: add documentation for atomic flags Date: Fri, 12 Apr 2019 09:58:24 -0300 Message-Id: <20190412125827.5877-2-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190412125827.5877-1-helen.koike@collabora.com> References: <20190412125827.5877-1-helen.koike@collabora.com> MIME-Version: 1.0 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: dnicoara@chromium.org, kernel@collabora.com, daniels@collabora.com, Sean Paul , alexandros.frantzis@collabora.com, Maxime Ripard , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, tomasz Figa , Gustavo Padovan , Helen Koike , boris.brezillon@collabora.com, tina.zhang@intel.com, Sean Paul , nicholas.kazlauskas@amd.com, =?utf-8?q?St=C3=A9phane_Marchesin?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP add a brief description of the flags used in an atomic commit Signed-off-by: Helen Koike --- Changes in v3: None Changes in v2: None Changes in v1: None include/uapi/drm/drm_mode.h | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 83cd1636b9be..88ef2cf04d13 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -729,7 +729,23 @@ struct drm_mode_destroy_dumb { __u32 handle; }; -/* page-flip flags are valid, plus: */ +/* + * drm atomic flags + * + * page-flip flags are valid, plus: + * + * DRM_MODE_ATOMIC_TEST_ONLY + * Used with fences to check if the Sync File is a valid one. + * + * DRM_MODE_ATOMIC_NONBLOCK + * Perform a normal atomic update but do not block the ioctl until the request + * is finished, return the ioctl call immediately. + * + * DRM_MODE_ATOMIC_ALLOW_MODESET + * Indicates whether a full modeset is acceptable or not. + */ + +/* */ #define DRM_MODE_ATOMIC_TEST_ONLY 0x0100 #define DRM_MODE_ATOMIC_NONBLOCK 0x0200 #define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400 From patchwork Fri Apr 12 12:58:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helen Mae Koike Fornazier X-Patchwork-Id: 10898253 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4298117E6 for ; Fri, 12 Apr 2019 12:59:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 21E3D28DB6 for ; Fri, 12 Apr 2019 12:59:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1548228DBC; Fri, 12 Apr 2019 12:59:15 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 0BAF128DB9 for ; Fri, 12 Apr 2019 12:59:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AC566899E8; Fri, 12 Apr 2019 12:59:11 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 080E8899E6; Fri, 12 Apr 2019 12:59:10 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a5e8:c168:522a:50d5:d06d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: koike) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 95480281296; Fri, 12 Apr 2019 13:58:58 +0100 (BST) From: Helen Koike To: dri-devel@lists.freedesktop.org, David Airlie Subject: [PATCH v3 2/4] drm/atomic: rename async_{update, check} to amend_{update, check} Date: Fri, 12 Apr 2019 09:58:25 -0300 Message-Id: <20190412125827.5877-3-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190412125827.5877-1-helen.koike@collabora.com> References: <20190412125827.5877-1-helen.koike@collabora.com> MIME-Version: 1.0 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: Sean Paul , alexandros.frantzis@collabora.com, Maxime Ripard , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, Mamta Shukla , tina.zhang@intel.com, kernel@collabora.com, Anthony Koo , Jonathan Corbet , Sean Paul , David Francis , linux-doc@vger.kernel.org, amd-gfx@lists.freedesktop.org, Gustavo Padovan , linux-rockchip@lists.infradead.org, daniels@collabora.com, Leo Li , linux-arm-msm@vger.kernel.org, Helen Koike , Mikita Lipski , Bhawanpreet Lakha , linux-arm-kernel@lists.infradead.org, dnicoara@chromium.org, =?utf-8?q?St?= =?utf-8?q?=C3=A9phane_Marchesin?= , freedreno@lists.freedesktop.org, tomasz Figa , boris.brezillon@collabora.com, Thomas Zimmermann , Alex Deucher , =?utf-8?q?Christian_K=C3=B6nig?= , Russell King , nicholas.kazlauskas@amd.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Asynchronous update is the ability change the hw state at any time, not only during vblank. Amend mode is the ability to perform 1000 commits to be applied as soon as possible without waiting for 1000 vblanks. async updates can be seen as amend, but the opposite is not true. &drm_plane_helper_funcs.atomic_async_{update,check}() was being used by drivers to implement amend and not async. So rename them to amend. Also improve docs explaining the difference. If asynchronous is required, normal page flip can be performed using DRM_MODE_PAGE_FLIP_ASYNC flag. Signed-off-by: Helen Koike --- Hello, I would like to officially clarify what async update means by adding it in the docs. Please correct me if I am wrong, but the current async_{update,check} callbacks are being used to do amend (the legacy cursor behavior), i.e. to allow 1000 updates without waiting for 1000 vblanks. So I would like to clarify this in the docs and rename the current callbacks to reflect this behaviour. I also see that for real async updates, the flag DRM_MODE_PAGE_FLIP_ASYNC can be used in a normal sync update (it is already being used by some drivers actually, in the atomic path, not only in the legacy page flip, at least is what I understood from amdgpu_dm_atomic_commit_tail() implementation). Please, let me know if I misunderstood anything. Otherwise renaming and improving the docs would put us all in sync. Thanks! Helen Changes in v3: None Changes in v2: None Changes in v1: None Documentation/gpu/drm-kms-helpers.rst | 8 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +- drivers/gpu/drm/drm_atomic_helper.c | 111 +++++++++++++----- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 8 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 8 +- drivers/gpu/drm/vc4/vc4_kms.c | 4 +- drivers/gpu/drm/vc4/vc4_plane.c | 8 +- include/drm/drm_atomic.h | 4 +- include/drm/drm_atomic_helper.h | 4 +- include/drm/drm_modeset_helper_vtables.h | 38 +++--- 10 files changed, 139 insertions(+), 64 deletions(-) diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 58b375e47615..c067a196902d 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -53,12 +53,18 @@ Overview .. kernel-doc:: drivers/gpu/drm/drm_atomic_helper.c :doc: overview -Implementing Asynchronous Atomic Commit +Implementing Nonblocking Atomic Commit --------------------------------------- .. kernel-doc:: drivers/gpu/drm/drm_atomic_helper.c :doc: implementing nonblocking commit +Amend Mode Atomic Commit +------------------------ + +.. kernel-doc:: drivers/gpu/drm/drm_atomic_helper.c + :doc: amend mode atomic commit + Helper Functions Reference -------------------------- diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 2f26581b93ff..711e7715e112 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3779,8 +3779,12 @@ static const struct drm_plane_helper_funcs dm_plane_helper_funcs = { .prepare_fb = dm_plane_helper_prepare_fb, .cleanup_fb = dm_plane_helper_cleanup_fb, .atomic_check = dm_plane_atomic_check, - .atomic_async_check = dm_plane_atomic_async_check, - .atomic_async_update = dm_plane_atomic_async_update + /* + * FIXME: ideally, instead of hooking async updates to amend, + * we could avoid tearing by modifying the pending commit. + */ + .atomic_amend_check = dm_plane_atomic_async_check, + .atomic_amend_update = dm_plane_atomic_async_update }; /* @@ -6140,7 +6144,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, * helper, check if it can be done asynchronously for better * performance. */ - state->async_update = !drm_atomic_helper_async_check(dev, state); + state->amend_update = !drm_atomic_helper_amend_check(dev, state); } /* Must be success */ diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2453678d1186..eb5dcd84fea7 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -948,7 +948,7 @@ int drm_atomic_helper_check(struct drm_device *dev, return ret; if (state->legacy_cursor_update) - state->async_update = !drm_atomic_helper_async_check(dev, state); + state->amend_update = !drm_atomic_helper_amend_check(dev, state); return ret; } @@ -1569,19 +1569,68 @@ static void commit_work(struct work_struct *work) } /** - * drm_atomic_helper_async_check - check if state can be commited asynchronously + * DOC: amend mode atomic commit + * + * The amend feature provides a way to perform 1000 updates to be applied as + * soon as possible without waiting for 1000 vblanks. + + * Currently, only the legacy cursor update uses amend mode, where historically, + * userspace performs several updates before the next vblank and don't want to + * see a delay in the cursor's movement. + * If amend is not supported, legacy cursor falls back to a normal sync update. + * + * To implement the legacy cursor update, drivers should provide + * &drm_plane_helper_funcs.atomic_amend_check() and + * &drm_plane_helper_funcs.atomic_amend_update() + * + * Drivers just need to make sure the last state overrides the previous one, so + * that if X updates were performed, then, in some point in the near future, + * preferentially in the next vblank, the Xth state will be the hardware state. + * + * If the hardware supports asynchronous update, i.e, changing its state without + * waiting for vblank, then &drm_plane_helper_funcs.atomic_amend_update() can be + * implemented using asynchronous update (the amend mode property is held), but + * it can cause tearing in the image. + * + * Otherwise (if async is not supported by the hw), drivers need to override the + * commit to be applied in the next vblank, and also they need to take care of + * framebuffer references when programming a new framebuffer, as hw can still be + * scanning out the old framebuffer. For now drivers must implement their own + * workers for deferring if needed, until a common solution is created. + * + * + * Notes / highlights: + * + * - amend update is performed on legacy cursor updates. + * + * - amend update won't happen if there is an outstanding commit modifying the + * same plane. + * + * - amend update won't happen if atomic_amend_check() returns false. + * + * - if atomic_amend_check() fails, it falls back to a normal synchronous + * update. + * + * - if userspace wants to ensure an asynchronous page flip, i.e. change hw + * state immediately, see DRM_MODE_PAGE_FLIP_ASYNC flag + * (asynchronous page flip maintains the amend property by definition). + * + * - Asynchronous modeset doesn't make sense, only asynchronous page flip. + */ + +/** + * drm_atomic_helper_amend_check - check if state can be amended. * @dev: DRM device * @state: the driver state object * - * This helper will check if it is possible to commit the state asynchronously. - * Async commits are not supposed to swap the states like normal sync commits - * but just do in-place changes on the current state. + * This helper will check if it is possible perform a commit in amend mode. + * For amend mode definition see :doc: amend mode atomic commit * - * It will return 0 if the commit can happen in an asynchronous fashion or error - * if not. Note that error just mean it can't be commited asynchronously, if it - * fails the commit should be treated like a normal synchronous commit. + * It will return 0 if the commit can happen in an amend fashion or error + * if not. Note that error just mean it can't be committed in amend mode, if it + * fails the commit should be treated like a normal commit. */ -int drm_atomic_helper_async_check(struct drm_device *dev, +int drm_atomic_helper_amend_check(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_crtc *crtc; @@ -1610,7 +1659,7 @@ int drm_atomic_helper_async_check(struct drm_device *dev, /* * FIXME: Since prepare_fb and cleanup_fb are always called on - * the new_plane_state for async updates we need to block framebuffer + * the new_plane_state for amend updates, we need to block framebuffer * changes. This prevents use of a fb that's been cleaned up and * double cleanups from occuring. */ @@ -1618,37 +1667,43 @@ int drm_atomic_helper_async_check(struct drm_device *dev, return -EINVAL; funcs = plane->helper_private; - if (!funcs->atomic_async_update) + if (!funcs->atomic_amend_update) return -EINVAL; if (new_plane_state->fence) return -EINVAL; /* - * Don't do an async update if there is an outstanding commit modifying - * the plane. This prevents our async update's changes from getting - * overridden by a previous synchronous update's state. + * Don't do an amend update if there is an outstanding commit modifying + * the plane. + * TODO: add support for modifying the outstanding commit. */ if (old_plane_state->commit && !try_wait_for_completion(&old_plane_state->commit->hw_done)) return -EBUSY; - return funcs->atomic_async_check(plane, new_plane_state); + return funcs->atomic_amend_check(plane, new_plane_state); } -EXPORT_SYMBOL(drm_atomic_helper_async_check); +EXPORT_SYMBOL(drm_atomic_helper_amend_check); /** - * drm_atomic_helper_async_commit - commit state asynchronously + * drm_atomic_helper_amend_commit - commit state in amend mode * @dev: DRM device * @state: the driver state object * - * This function commits a state asynchronously, i.e., not vblank - * synchronized. It should be used on a state only when - * drm_atomic_async_check() succeeds. Async commits are not supposed to swap - * the states like normal sync commits, but just do in-place changes on the - * current state. + * This function commits a state in amend mode. + * For amend mode definition see :doc: amend mode atomic commit + * + * It should be used on a state only when drm_atomic_amend_check() succeeds. + * + * Amend commits are not supposed to swap the states like normal sync commits, + * but just do in-place changes on the current state. + * + * TODO: instead of doing in-place changes, modify the new_state and perform an + * immediate flip. Drivers could reuse the page_flip code and even use the + * DRM_MODE_PAGE_FLIP_ASYNC flag if the hardware supports asyncronous update. */ -void drm_atomic_helper_async_commit(struct drm_device *dev, +void drm_atomic_helper_amend_commit(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_plane *plane; @@ -1658,10 +1713,10 @@ void drm_atomic_helper_async_commit(struct drm_device *dev, for_each_new_plane_in_state(state, plane, plane_state, i) { funcs = plane->helper_private; - funcs->atomic_async_update(plane, plane_state); + funcs->atomic_amend_update(plane, plane_state); /* - * ->atomic_async_update() is supposed to update the + * ->atomic_amend_update() is supposed to update the * plane->state in-place, make sure at least common * properties have been properly updated. */ @@ -1672,7 +1727,7 @@ void drm_atomic_helper_async_commit(struct drm_device *dev, WARN_ON_ONCE(plane->state->src_y != plane_state->src_y); } } -EXPORT_SYMBOL(drm_atomic_helper_async_commit); +EXPORT_SYMBOL(drm_atomic_helper_amend_commit); /** * drm_atomic_helper_commit - commit validated state object @@ -1698,12 +1753,12 @@ int drm_atomic_helper_commit(struct drm_device *dev, { int ret; - if (state->async_update) { + if (state->amend_update) { ret = drm_atomic_helper_prepare_planes(dev, state); if (ret) return ret; - drm_atomic_helper_async_commit(dev, state); + drm_atomic_helper_amend_commit(dev, state); drm_atomic_helper_cleanup_planes(dev, state); return 0; diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c index be13140967b4..814e8230cec6 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c @@ -531,8 +531,12 @@ static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = { .cleanup_fb = mdp5_plane_cleanup_fb, .atomic_check = mdp5_plane_atomic_check, .atomic_update = mdp5_plane_atomic_update, - .atomic_async_check = mdp5_plane_atomic_async_check, - .atomic_async_update = mdp5_plane_atomic_async_update, + /* + * FIXME: ideally, instead of hooking async updates to amend, + * we could avoid tearing by modifying the pending commit. + */ + .atomic_amend_check = mdp5_plane_atomic_async_check, + .atomic_amend_update = mdp5_plane_atomic_async_update, }; static void set_scanout_locked(struct mdp5_kms *mdp5_kms, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index a7cbf6c9a153..216ad76118dc 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -877,7 +877,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane, spin_unlock(&vop->reg_lock); } -static int vop_plane_atomic_async_check(struct drm_plane *plane, +static int vop_plane_atomic_amend_check(struct drm_plane *plane, struct drm_plane_state *state) { struct vop_win *vop_win = to_vop_win(plane); @@ -908,7 +908,7 @@ static int vop_plane_atomic_async_check(struct drm_plane *plane, true, true); } -static void vop_plane_atomic_async_update(struct drm_plane *plane, +static void vop_plane_atomic_amend_update(struct drm_plane *plane, struct drm_plane_state *new_state) { struct vop *vop = to_vop(plane->state->crtc); @@ -952,8 +952,8 @@ static const struct drm_plane_helper_funcs plane_helper_funcs = { .atomic_check = vop_plane_atomic_check, .atomic_update = vop_plane_atomic_update, .atomic_disable = vop_plane_atomic_disable, - .atomic_async_check = vop_plane_atomic_async_check, - .atomic_async_update = vop_plane_atomic_async_update, + .atomic_amend_check = vop_plane_atomic_amend_check, + .atomic_amend_update = vop_plane_atomic_amend_update, .prepare_fb = drm_gem_fb_prepare_fb, }; diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 295dacc8bcb9..229032b1b315 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -216,7 +216,7 @@ static int vc4_atomic_commit(struct drm_device *dev, struct vc4_dev *vc4 = to_vc4_dev(dev); int ret; - if (state->async_update) { + if (state->amend_update) { ret = down_interruptible(&vc4->async_modeset); if (ret) return ret; @@ -227,7 +227,7 @@ static int vc4_atomic_commit(struct drm_device *dev, return ret; } - drm_atomic_helper_async_commit(dev, state); + drm_atomic_helper_amend_commit(dev, state); drm_atomic_helper_cleanup_planes(dev, state); diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 4d918d3e4858..ea560000222d 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -1169,8 +1169,12 @@ static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = { .atomic_update = vc4_plane_atomic_update, .prepare_fb = vc4_prepare_fb, .cleanup_fb = vc4_cleanup_fb, - .atomic_async_check = vc4_plane_atomic_async_check, - .atomic_async_update = vc4_plane_atomic_async_update, + /* + * FIXME: ideally, instead of hooking async updates to amend, + * we could avoid tearing by modifying the pending commit. + */ + .atomic_amend_check = vc4_plane_atomic_async_check, + .atomic_amend_update = vc4_plane_atomic_async_update, }; static void vc4_plane_destroy(struct drm_plane *plane) diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 824a5ed4e216..b1ced069ffc1 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -300,7 +300,7 @@ struct __drm_private_objs_state { * @ref: count of all references to this state (will not be freed until zero) * @dev: parent DRM device * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics - * @async_update: hint for asynchronous plane update + * @amend_update: hint for amend plane update * @planes: pointer to array of structures with per-plane data * @crtcs: pointer to array of CRTC pointers * @num_connector: size of the @connectors and @connector_states arrays @@ -328,7 +328,7 @@ struct drm_atomic_state { */ bool allow_modeset : 1; bool legacy_cursor_update : 1; - bool async_update : 1; + bool amend_update : 1; /** * @duplicated: * diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 58214be3bf3d..8ce0594ccfb9 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -55,9 +55,9 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *state); int drm_atomic_helper_commit(struct drm_device *dev, struct drm_atomic_state *state, bool nonblock); -int drm_atomic_helper_async_check(struct drm_device *dev, +int drm_atomic_helper_amend_check(struct drm_device *dev, struct drm_atomic_state *state); -void drm_atomic_helper_async_commit(struct drm_device *dev, +void drm_atomic_helper_amend_commit(struct drm_device *dev, struct drm_atomic_state *state); int drm_atomic_helper_wait_for_fences(struct drm_device *dev, diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index cfb7be40bed7..d92e62dd76c4 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -1136,53 +1136,55 @@ struct drm_plane_helper_funcs { struct drm_plane_state *old_state); /** - * @atomic_async_check: + * @atomic_amend_check: * * Drivers should set this function pointer to check if the plane state - * can be updated in a async fashion. Here async means "not vblank - * synchronized". + * can be updated in amend mode. + * For amend mode definition see :doc: amend mode atomic commit * - * This hook is called by drm_atomic_async_check() to establish if a - * given update can be committed asynchronously, that is, if it can + * This hook is called by drm_atomic_amend_check() to establish if a + * given update can be committed in amend mode, that is, if it can * jump ahead of the state currently queued for update. * * RETURNS: * * Return 0 on success and any error returned indicates that the update - * can not be applied in asynchronous manner. + * can not be applied in amend mode. */ - int (*atomic_async_check)(struct drm_plane *plane, + int (*atomic_amend_check)(struct drm_plane *plane, struct drm_plane_state *state); /** - * @atomic_async_update: + * @atomic_amend_update: * - * Drivers should set this function pointer to perform asynchronous - * updates of planes, that is, jump ahead of the currently queued - * state and update the plane. Here async means "not vblank - * synchronized". + * Drivers should set this function pointer to perform amend + * updates of planes. + * The amend feature provides a way to perform 1000 commits, without + * waiting for 1000 vblanks to get the last state applied. * - * This hook is called by drm_atomic_helper_async_commit(). + * For amend mode definition see :doc: amend mode atomic commit * - * An async update will happen on legacy cursor updates. An async + * This hook is called by drm_atomic_helper_amend_commit(). + * + * An amend update will happen on legacy cursor updates. An amend * update won't happen if there is an outstanding commit modifying * the same plane. * * Note that unlike &drm_plane_helper_funcs.atomic_update this hook - * takes the new &drm_plane_state as parameter. When doing async_update + * takes the new &drm_plane_state as parameter. When doing amend_update * drivers shouldn't replace the &drm_plane_state but update the * current one with the new plane configurations in the new * plane_state. * * FIXME: * - It only works for single plane updates - * - Async Pageflips are not supported yet - * - Some hw might still scan out the old buffer until the next + * - If hardware don't support asyncronous update to implement amend, + * some hw might still scan out the old buffer until the next * vblank, however we let go of the fb references as soon as * we run this hook. For now drivers must implement their own workers * for deferring if needed, until a common solution is created. */ - void (*atomic_async_update)(struct drm_plane *plane, + void (*atomic_amend_update)(struct drm_plane *plane, struct drm_plane_state *new_state); }; From patchwork Fri Apr 12 12:58:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helen Mae Koike Fornazier X-Patchwork-Id: 10898255 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 59B3717E0 for ; Fri, 12 Apr 2019 12:59:19 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 36F2A28DB9 for ; Fri, 12 Apr 2019 12:59:19 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2B19028DC0; Fri, 12 Apr 2019 12:59:19 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 7F1E128DB9 for ; Fri, 12 Apr 2019 12:59:18 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 57B2C89944; Fri, 12 Apr 2019 12:59:17 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id D28C98993B for ; Fri, 12 Apr 2019 12:59:15 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a5e8:c168:522a:50d5:d06d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: koike) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 7785B2811D1; Fri, 12 Apr 2019 13:59:09 +0100 (BST) From: Helen Koike To: dri-devel@lists.freedesktop.org, David Airlie Subject: [PATCH RFC v3 3/4] drm/atomic: add ATOMIC_AMEND flag to the Atomic IOCTL. Date: Fri, 12 Apr 2019 09:58:26 -0300 Message-Id: <20190412125827.5877-4-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190412125827.5877-1-helen.koike@collabora.com> References: <20190412125827.5877-1-helen.koike@collabora.com> MIME-Version: 1.0 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: dnicoara@chromium.org, kernel@collabora.com, daniels@collabora.com, Sean Paul , alexandros.frantzis@collabora.com, Maxime Ripard , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, tomasz Figa , Gustavo Padovan , Helen Koike , boris.brezillon@collabora.com, Enric Balletbo i Serra , tina.zhang@intel.com, Sean Paul , nicholas.kazlauskas@amd.com, =?utf-8?q?St?= =?utf-8?q?=C3=A9phane_Marchesin?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP This flag tells core to jump ahead the queued update if the conditions in drm_atomic_amend_check() are met. That means we are only able to do an amend update if no modeset is pending and update for the same plane is not queued. It uses the already in place infrastructure for amend updates. It is useful for cursor updates over the atomic ioctl. Otherwise in some cases updates may be delayed to the point the user will notice it. Note that for now it's only enabled for cursor planes. DRM_MODE_ATOMIC_AMEND should be passed to the Atomic IOCTL to use this feature. Signed-off-by: Gustavo Padovan Signed-off-by: Enric Balletbo i Serra [updated for upstream] Signed-off-by: Helen Koike --- Hi, This is the third attempt to introduce the new ATOMIC_AMEND flag for atomic operations, see the commit message for a more detailed description. I am sending this patch in the series as an RFC as I still have things to define/discuss. The main reasons to expose this through atomic api is to avoid mixing legacy with modern/atomic API (since their interactions are not well defined) and to be able to explicitly manage the cursor plane. The way I envision it working is: 1) userspace just use DRM_MODE_PAGE_FLIP_ASYNC if true async is desired. 2) if it fails, then userspace can try DRM_MODE_ATOMIC_AMEND if true async is not required, but a legacy cursor behavior is required 3) if amend is not possible, we can or: A) fallback to a non amend update B) fail C) add another flag to define what it should do. Right now the code does A for legacy cursor and B for atomic api using the DRM_MODE_ATOMIC_AMEND flag. Discussing with some people it seems that failing is better for Xorg, but Ozone (chrome compositor) doesn't expect the amend to fail, but I think it could just retry the same atomic commit without the AMEND flag if it wants to fallback. Alexandros also did a proof-of-concept to use this flag and draw cursors using atomic if possible on ozone (Chrome compositor) [1]. I'm sending this as an RFC for now as I still need to verify the requirements from other compositors and make some tests (and also to justify the s/async/amend renaming). Please, let me know if you detect issues with this. If not, I'll start implementing tests in igt and push the adoption in other compositors. Thanks Helen [1] https://chromium-review.googlesource.com/c/chromium/src/+/1092711 [2] https://gitlab.collabora.com/eballetbo/drm-cursor/commits/async-capability Changes in v3: - rebase tree - rebase on top of renaming async_update to amend_update - improve documentation - don't fall back to a normal commit if amend is not possible when requested through the atomic api Changes in v2: - rebase tree - do not fall back to a non-async update if if there isn't any pending commit to amend Changes in v1: - https://patchwork.freedesktop.org/patch/243088/ - Only enable it if userspace requests it. - Only allow async update for cursor type planes. - Rename ASYNC_UPDATE for ATOMIC_AMEND. drivers/gpu/drm/drm_atomic_helper.c | 39 +++++++++++++++++++++++------ drivers/gpu/drm/drm_atomic_uapi.c | 8 ++++++ include/uapi/drm/drm_mode.h | 9 ++++++- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index eb5dcd84fea7..9b0df08836c3 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -947,10 +947,18 @@ int drm_atomic_helper_check(struct drm_device *dev, if (ret) return ret; + /* + * If amend was explicitly requested, but it is not possible, + * return error instead of falling back to a normal commit. + */ + if (state->amend_update) + return drm_atomic_helper_amend_check(dev, state); + + /* Legacy mode falls back to a normal commit if amend isn't possible. */ if (state->legacy_cursor_update) state->amend_update = !drm_atomic_helper_amend_check(dev, state); - return ret; + return 0; } EXPORT_SYMBOL(drm_atomic_helper_check); @@ -1574,12 +1582,20 @@ static void commit_work(struct work_struct *work) * The amend feature provides a way to perform 1000 updates to be applied as * soon as possible without waiting for 1000 vblanks. - * Currently, only the legacy cursor update uses amend mode, where historically, + * Legacy cursor update uses amend mode, where historically, * userspace performs several updates before the next vblank and don't want to * see a delay in the cursor's movement. * If amend is not supported, legacy cursor falls back to a normal sync update. * - * To implement the legacy cursor update, drivers should provide + * Amend can also be performed through the atomic API using the flag + * DRM_MODE_ATOMIC_AMEND. The atomic commit will fail if this flag is set but + * the driver doesn't support it. + * Amend can't perform modeset, thus atomic API will fail if + * DRM_MODE_ATOMIC_AMEND is used in conjunction with + * DRM_MODE_ATOMIC_ALLOW_MODESET flag. + * + * To implement the legacy cursor update and amend mode through atomic, drivers + * should provide: * &drm_plane_helper_funcs.atomic_amend_check() and * &drm_plane_helper_funcs.atomic_amend_update() * @@ -1601,16 +1617,21 @@ static void commit_work(struct work_struct *work) * * Notes / highlights: * - * - amend update is performed on legacy cursor updates. + * - amend update is performed on legacy cursor updates, but it will fallback to + * a normal commit if amend is not possible. However, if amend was requested + * through atomic, the ioctl will fail instead of fall back if it can't be + * amended. + * + * - atomic api will reject amend if DRM_MODE_ATOMIC_ALLOW_MODESET flag is used. + * + * - If DRM_MODE_PAGE_FLIP_ASYNC is set, then the DRM_MODE_ATOMIC_AMEND flag + * will be ignored. * * - amend update won't happen if there is an outstanding commit modifying the * same plane. * * - amend update won't happen if atomic_amend_check() returns false. * - * - if atomic_amend_check() fails, it falls back to a normal synchronous - * update. - * * - if userspace wants to ensure an asynchronous page flip, i.e. change hw * state immediately, see DRM_MODE_PAGE_FLIP_ASYNC flag * (asynchronous page flip maintains the amend property by definition). @@ -1673,6 +1694,10 @@ int drm_atomic_helper_amend_check(struct drm_device *dev, if (new_plane_state->fence) return -EINVAL; + /* Only allow amend update for cursor type planes. */ + if (plane->type != DRM_PLANE_TYPE_CURSOR) + return -EINVAL; + /* * Don't do an amend update if there is an outstanding commit modifying * the plane. diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 4eb81f10bc54..d1962cdea602 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -1300,6 +1301,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) return -EINVAL; + if ((arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET) && + (arg->flags & DRM_MODE_ATOMIC_AMEND)) + return -EINVAL; + state = drm_atomic_state_alloc(dev); if (!state) return -ENOMEM; @@ -1307,6 +1312,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE); state->acquire_ctx = &ctx; state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET); + /* async takes precedence over amend */ + state->amend_update = arg->flags & DRM_MODE_PAGE_FLIP_ASYNC ? 0 : + !!(arg->flags & DRM_MODE_ATOMIC_AMEND); retry: copied_objs = 0; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 88ef2cf04d13..3831d04f3b38 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -743,19 +743,26 @@ struct drm_mode_destroy_dumb { * * DRM_MODE_ATOMIC_ALLOW_MODESET * Indicates whether a full modeset is acceptable or not. + * + * DRM_MODE_ATOMIC_AMEND + * Used to perform an atomic amend. See "DOC: amend mode atomic commit" in + * drm_atomic_helper.c for more details. + * This flag can't be used with DRM_MODE_ATOMIC_ALLOW_MODESET. */ /* */ #define DRM_MODE_ATOMIC_TEST_ONLY 0x0100 #define DRM_MODE_ATOMIC_NONBLOCK 0x0200 #define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400 +#define DRM_MODE_ATOMIC_AMEND 0x0800 #define DRM_MODE_ATOMIC_FLAGS (\ DRM_MODE_PAGE_FLIP_EVENT |\ DRM_MODE_PAGE_FLIP_ASYNC |\ DRM_MODE_ATOMIC_TEST_ONLY |\ DRM_MODE_ATOMIC_NONBLOCK |\ - DRM_MODE_ATOMIC_ALLOW_MODESET) + DRM_MODE_ATOMIC_ALLOW_MODESET |\ + DRM_MODE_ATOMIC_AMEND) struct drm_mode_atomic { __u32 flags; From patchwork Fri Apr 12 12:58:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helen Mae Koike Fornazier X-Patchwork-Id: 10898263 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4B2D9186E for ; Fri, 12 Apr 2019 12:59:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2EA3F28DB6 for ; Fri, 12 Apr 2019 12:59:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2185828DBC; Fri, 12 Apr 2019 12:59:30 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 5D78E28DB6 for ; Fri, 12 Apr 2019 12:59:29 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C79608993B; Fri, 12 Apr 2019 12:59:27 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1FF1F89113; Fri, 12 Apr 2019 12:59:26 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a5e8:c168:522a:50d5:d06d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: koike) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 533B22811D2; Fri, 12 Apr 2019 13:59:15 +0100 (BST) From: Helen Koike To: dri-devel@lists.freedesktop.org, David Airlie Subject: [PATCH RFC v3 4/4] drm/atomic: hook atomic_async_{check, update} with PAGE_FLIP_ASYNC flag Date: Fri, 12 Apr 2019 09:58:27 -0300 Message-Id: <20190412125827.5877-5-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190412125827.5877-1-helen.koike@collabora.com> References: <20190412125827.5877-1-helen.koike@collabora.com> MIME-Version: 1.0 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: Sean Paul , alexandros.frantzis@collabora.com, Maxime Ripard , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, Mamta Shukla , tina.zhang@intel.com, kernel@collabora.com, Anthony Koo , linux-rockchip@lists.infradead.org, Sean Paul , David Francis , amd-gfx@lists.freedesktop.org, Gustavo Padovan , daniels@collabora.com, Leo Li , linux-arm-msm@vger.kernel.org, Helen Koike , Mikita Lipski , Bhawanpreet Lakha , linux-arm-kernel@lists.infradead.org, dnicoara@chromium.org, =?utf-8?q?St?= =?utf-8?q?=C3=A9phane_Marchesin?= , freedreno@lists.freedesktop.org, tomasz Figa , boris.brezillon@collabora.com, Thomas Zimmermann , Alex Deucher , =?utf-8?q?Christian_K=C3=B6nig?= , Russell King , nicholas.kazlauskas@amd.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Add atomic_async_{check,update} hooks in drm_plane_helper_funcs. These hooks are called when userspace requests asyncronous page flip in the atomic api through the flag DRM_MODE_PAGE_FLIP_ASYNC. Update those hooks in the drivers that implement async functions, except for amdgpu who handles the flag in the normal path, and rockchip who doesn't support async page flip. Signed-off-by: Helen Koike --- Hi, This patch is an attempt to expose the implementation that already exist for true async page flips updates through atomic api when the DRM_MODE_PAGE_FLIP_ASYNC is used. In this commit I'm re-introducing the atomic_async_{check,update} hooks. I know it is a bit weird to remove them first and them re-add them, but I did this in the last commit to avoid any state of inconsistency between commits, as rockchip doesn't support async page flip and they were being used as amend. So I reverted to amend first and then re-introced async again tied to the DRM_MODE_PAGE_FLIP_ASYNC flag (I also think this is easier to read). To use async, it is required to have dev->mode_config.async_page_flip = true; but I see that only amdgpu and vc4 have this, so this patch won't take effect on mdp5. Nouveau and radeon also have this flag, but they don't implement the async hooks yet. Please let me know what you think. Thanks Helen Changes in v3: None Changes in v2: None Changes in v1: None .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++ drivers/gpu/drm/drm_atomic_helper.c | 31 ++++++++++++---- drivers/gpu/drm/drm_atomic_uapi.c | 3 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 2 + drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++ drivers/gpu/drm/vc4/vc4_plane.c | 2 + include/drm/drm_atomic.h | 2 + include/drm/drm_atomic_helper.h | 9 +++-- include/drm/drm_modeset_helper_vtables.h | 37 +++++++++++++++++++ 9 files changed, 83 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 711e7715e112..bb8a5f1997d6 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3785,6 +3785,11 @@ static const struct drm_plane_helper_funcs dm_plane_helper_funcs = { */ .atomic_amend_check = dm_plane_atomic_async_check, .atomic_amend_update = dm_plane_atomic_async_update + /* + * Note: amdgpu takes care of DRM_MODE_PAGE_FLIP_ASYNC flag in the + * normal commit path, thus .atomic_async_check and .atomic_async_update + * are not provided here. + */ }; /* diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 9b0df08836c3..bfcf88359de5 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -947,16 +947,31 @@ int drm_atomic_helper_check(struct drm_device *dev, if (ret) return ret; + /* + * If async page flip was explicitly requested, but it is not possible, + * return error instead of falling back to a normal commit. + * If async_amend_check returns -EOPNOTSUPP, it means + * ->atomic_async_update hook doesn't exist, so fallback to normal + * commit and let the driver handle DRM_MODE_PAGE_FLIP_ASYNC flag + * through normal commit code path. + */ + if (state->async_update) { + ret = drm_atomic_helper_async_amend_check(dev, state, false); + state->async_update = !ret; + return !ret || ret == -EOPNOTSUPP ? 0 : ret; + } + /* * If amend was explicitly requested, but it is not possible, * return error instead of falling back to a normal commit. */ if (state->amend_update) - return drm_atomic_helper_amend_check(dev, state); + return drm_atomic_helper_async_amend_check(dev, state, true); /* Legacy mode falls back to a normal commit if amend isn't possible. */ if (state->legacy_cursor_update) - state->amend_update = !drm_atomic_helper_amend_check(dev, state); + state->amend_update = + !drm_atomic_helper_async_amend_check(dev, state, true); return 0; } @@ -1651,8 +1666,9 @@ static void commit_work(struct work_struct *work) * if not. Note that error just mean it can't be committed in amend mode, if it * fails the commit should be treated like a normal commit. */ -int drm_atomic_helper_amend_check(struct drm_device *dev, - struct drm_atomic_state *state) +int drm_atomic_helper_async_amend_check(struct drm_device *dev, + struct drm_atomic_state *state, + bool amend) { struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; @@ -1695,7 +1711,7 @@ int drm_atomic_helper_amend_check(struct drm_device *dev, return -EINVAL; /* Only allow amend update for cursor type planes. */ - if (plane->type != DRM_PLANE_TYPE_CURSOR) + if (amend && plane->type != DRM_PLANE_TYPE_CURSOR) return -EINVAL; /* @@ -1707,9 +1723,10 @@ int drm_atomic_helper_amend_check(struct drm_device *dev, !try_wait_for_completion(&old_plane_state->commit->hw_done)) return -EBUSY; - return funcs->atomic_amend_check(plane, new_plane_state); + return amend ? funcs->atomic_amend_check(plane, new_plane_state) : + funcs->atomic_async_check(plane, new_plane_state); } -EXPORT_SYMBOL(drm_atomic_helper_amend_check); +EXPORT_SYMBOL(drm_atomic_helper_async_amend_check); /** * drm_atomic_helper_amend_commit - commit state in amend mode diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index d1962cdea602..1d9a6142218e 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1312,8 +1312,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE); state->acquire_ctx = &ctx; state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET); + state->async_update = !!(arg->flags & DRM_MODE_PAGE_FLIP_ASYNC); /* async takes precedence over amend */ - state->amend_update = arg->flags & DRM_MODE_PAGE_FLIP_ASYNC ? 0 : + state->amend_update = state->async_update ? 0 : !!(arg->flags & DRM_MODE_ATOMIC_AMEND); retry: diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c index 814e8230cec6..e3b2a2c74852 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c @@ -531,6 +531,8 @@ static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = { .cleanup_fb = mdp5_plane_cleanup_fb, .atomic_check = mdp5_plane_atomic_check, .atomic_update = mdp5_plane_atomic_update, + .atomic_async_check = mdp5_plane_atomic_async_check, + .atomic_async_update = mdp5_plane_atomic_async_update, /* * FIXME: ideally, instead of hooking async updates to amend, * we could avoid tearing by modifying the pending commit. diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 216ad76118dc..c2f7201e52a9 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -954,6 +954,10 @@ static const struct drm_plane_helper_funcs plane_helper_funcs = { .atomic_disable = vop_plane_atomic_disable, .atomic_amend_check = vop_plane_atomic_amend_check, .atomic_amend_update = vop_plane_atomic_amend_update, + /* + * Note: rockchip doesn't support async page flip, thus + * .atomic_async_update and .atomic_async_check are not provided. + */ .prepare_fb = drm_gem_fb_prepare_fb, }; diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index ea560000222d..24a9befe89e6 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -1175,6 +1175,8 @@ static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = { */ .atomic_amend_check = vc4_plane_atomic_async_check, .atomic_amend_update = vc4_plane_atomic_async_update, + .atomic_async_check = vc4_plane_atomic_async_check, + .atomic_async_update = vc4_plane_atomic_async_update, }; static void vc4_plane_destroy(struct drm_plane *plane) diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index b1ced069ffc1..05756a09e762 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -300,6 +300,7 @@ struct __drm_private_objs_state { * @ref: count of all references to this state (will not be freed until zero) * @dev: parent DRM device * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics + * @async_update: hint for asyncronous page flip update * @amend_update: hint for amend plane update * @planes: pointer to array of structures with per-plane data * @crtcs: pointer to array of CRTC pointers @@ -328,6 +329,7 @@ struct drm_atomic_state { */ bool allow_modeset : 1; bool legacy_cursor_update : 1; + bool async_update : 1; bool amend_update : 1; /** * @duplicated: diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 8ce0594ccfb9..39e57d559a30 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -55,10 +55,11 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *state); int drm_atomic_helper_commit(struct drm_device *dev, struct drm_atomic_state *state, bool nonblock); -int drm_atomic_helper_amend_check(struct drm_device *dev, - struct drm_atomic_state *state); -void drm_atomic_helper_amend_commit(struct drm_device *dev, - struct drm_atomic_state *state); +int drm_atomic_helper_async_amend_check(struct drm_device *dev, + struct drm_atomic_state *state, + bool amend); +void drm_atomic_helper_async_amend_commit(struct drm_device *dev, + struct drm_atomic_state *state); int drm_atomic_helper_wait_for_fences(struct drm_device *dev, struct drm_atomic_state *state, diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index d92e62dd76c4..c2863d62f160 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -1135,6 +1135,43 @@ struct drm_plane_helper_funcs { void (*atomic_disable)(struct drm_plane *plane, struct drm_plane_state *old_state); + /** + * @atomic_async_check: + * + * Drivers should set this function pointer to check if a page flip can + * be performed asynchronously, i.e., immediately without waiting for a + * vblank. + * + * This hook is called by drm_atomic_async_check() to establish if a + * given update can be committed in async mode, that is, if it can + * jump ahead of the state currently queued for update. + * + * RETURNS: + * + * Return 0 on success and any error returned indicates that the update + * can not be applied in asynd mode. + */ + int (*atomic_async_check)(struct drm_plane *plane, + struct drm_plane_state *state); + + /** + * @atomic_async_update: + * + * Drivers should set this function pointer to perform asynchronous page + * flips through the atomic api, i.e. not vblank synchronized. + * + * Note that unlike &drm_plane_helper_funcs.atomic_update this hook + * takes the new &drm_plane_state as parameter. When doing async_update + * drivers shouldn't replace the &drm_plane_state but update the + * current one with the new plane configurations in the new + * plane_state. + * + * FIXME: + * - It only works for single plane updates + */ + void (*atomic_async_update)(struct drm_plane *plane, + struct drm_plane_state *new_state); + /** * @atomic_amend_check: *