From patchwork Mon Mar 4 14:49:05 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: 10837849 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 8E8A217E9 for ; Mon, 4 Mar 2019 14:49:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7C0BF2A00A for ; Mon, 4 Mar 2019 14:49:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6FD122A0F8; Mon, 4 Mar 2019 14:49:31 +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 F0D982A00A for ; Mon, 4 Mar 2019 14:49:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AA5A989CE1; Mon, 4 Mar 2019 14:49:29 +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 781EB89CE1 for ; Mon, 4 Mar 2019 14:49:28 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a1b1:9d17:4c45:79a4:9c61]) (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 58BB1261176; Mon, 4 Mar 2019 14:49:23 +0000 (GMT) From: Helen Koike To: dri-devel@lists.freedesktop.org, nicholas.kazlauskas@amd.com Subject: [PATCH 1/5] drm: don't block fb changes for async plane updates Date: Mon, 4 Mar 2019 11:49:05 -0300 Message-Id: <20190304144909.6267-2-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190304144909.6267-1-helen.koike@collabora.com> References: <20190304144909.6267-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: =?utf-8?q?St=C3=A9phane_Marchesin?= , Sean Paul , David Airlie , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, Tomasz Figa , boris.brezillon@collabora.com, kernel@collabora.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP In the case of a normal sync update, the preparation of framebuffers (be it calling drm_atomic_helper_prepare_planes() or doing setups with drm_framebuffer_get()) are performed in the new_state and the respective cleanups are performed in the old_state. In the case of async updates, the preparation is also done in the new_state but the cleanups are done in the new_state (because updates are performed in place, i.e. in the current state). The current code blocks async udpates when the fb is changed, turning async updates into sync updates, slowing down cursor updates and introducing regressions in igt tests with errors of type: "CRITICAL: completed 97 cursor updated in a period of 30 flips, we expect to complete approximately 15360 updates, with the threshold set at 7680" Fb changes in async updates were prevented to avoid the following scenario: - Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1 - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2 - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 (wrong) Where we have a single call to prepare fb2 but double cleanup call to fb2. To solve the above problems, instead of blocking async fb changes, we place the old framebuffer in the new_state object, so when the code performs cleanups in the new_state it will cleanup the old_fb and we will have the following scenario instead: - Async update, oldfb = NULL, newfb = fb1, prepare fb1, no cleanup - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb1 - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 Where calls to prepare/cleanup are ballanced. Cc: # v4.14+: 25dc194b34dd: drm: Block fb changes for async plane updates Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates") Suggested-by: Boris Brezillon Signed-off-by: Helen Koike --- Hello, As mentioned in the cover letter, I tested on the rockchip and on i915 (with a patch I am still working on for replacing cursors by async update), with igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any regressions. I couldn't test on MSM and AMD because I don't have the hardware (and I am having some issues testing on vc4) and I would appreciate if anyone could help me testing those. I also think it would be a better solution if, instead of having async to do in-place updates in the current state, the async path should be equivalent to a syncronous update, i.e., modifying new_state and performing a flip IMHO, the only difference between sync and async should be that async update doesn't wait for vblank and applies the changes immeditally to the hw, but the code path could be almost the same. But for now I think this solution is ok (swaping new_fb/old_fb), and then we can adjust things little by little, what do you think? Thanks! Helen drivers/gpu/drm/drm_atomic_helper.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 540a77a2ade9..e7eb96f1efc2 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1608,15 +1608,6 @@ int drm_atomic_helper_async_check(struct drm_device *dev, old_plane_state->crtc != new_plane_state->crtc) return -EINVAL; - /* - * FIXME: Since prepare_fb and cleanup_fb are always called on - * the new_plane_state for async updates we need to block framebuffer - * changes. This prevents use of a fb that's been cleaned up and - * double cleanups from occuring. - */ - if (old_plane_state->fb != new_plane_state->fb) - return -EINVAL; - funcs = plane->helper_private; if (!funcs->atomic_async_update) return -EINVAL; @@ -1657,6 +1648,9 @@ void drm_atomic_helper_async_commit(struct drm_device *dev, int i; for_each_new_plane_in_state(state, plane, plane_state, i) { + struct drm_framebuffer *new_fb = plane_state->fb; + struct drm_framebuffer *old_fb = plane->state->fb; + funcs = plane->helper_private; funcs->atomic_async_update(plane, plane_state); @@ -1665,11 +1659,17 @@ void drm_atomic_helper_async_commit(struct drm_device *dev, * plane->state in-place, make sure at least common * properties have been properly updated. */ - WARN_ON_ONCE(plane->state->fb != plane_state->fb); + WARN_ON_ONCE(plane->state->fb != new_fb); WARN_ON_ONCE(plane->state->crtc_x != plane_state->crtc_x); WARN_ON_ONCE(plane->state->crtc_y != plane_state->crtc_y); WARN_ON_ONCE(plane->state->src_x != plane_state->src_x); WARN_ON_ONCE(plane->state->src_y != plane_state->src_y); + + /* + * Make sure the FBs have been swapped so that cleanups in the + * new_state performs a cleanup in the old FB. + */ + WARN_ON_ONCE(plane_state->fb != old_fb); } } EXPORT_SYMBOL(drm_atomic_helper_async_commit); From patchwork Mon Mar 4 14:49:06 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: 10837851 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 D4DC117E9 for ; Mon, 4 Mar 2019 14:49:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C2EFC2A00A for ; Mon, 4 Mar 2019 14:49:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B66562A0F8; Mon, 4 Mar 2019 14:49:37 +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 5334D2A00A for ; Mon, 4 Mar 2019 14:49:37 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5E7C889CF8; Mon, 4 Mar 2019 14:49:36 +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 [46.235.227.227]) by gabe.freedesktop.org (Postfix) with ESMTPS id 73B5489CF4 for ; Mon, 4 Mar 2019 14:49:34 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a1b1:9d17:4c45:79a4:9c61]) (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 12B432807AF; Mon, 4 Mar 2019 14:49:27 +0000 (GMT) From: Helen Koike To: dri-devel@lists.freedesktop.org, nicholas.kazlauskas@amd.com Subject: [PATCH 2/5] drm/rockchip: fix fb references in async update Date: Mon, 4 Mar 2019 11:49:06 -0300 Message-Id: <20190304144909.6267-3-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190304144909.6267-1-helen.koike@collabora.com> References: <20190304144909.6267-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: =?utf-8?q?St=C3=A9phane_Marchesin?= , Sean Paul , David Airlie , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, Tomasz Figa , boris.brezillon@collabora.com, kernel@collabora.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP In the case of async update, modifications are done in place, i.e. in the current plane state, so the new_state is prepared and the new_state is cleanup up (instead of the old_state, diferrently on what happen in a normal sync update). To cleanup the old_fb properly, it needs to be placed in the new_state in the end of async_update, so cleanup call will unreference the old_fb correctly. Also, the previous code had a: plane_state = plane->funcs->atomic_duplicate_state(plane); ... swap(plane_state, plane->state); if (plane->state->fb && plane->state->fb != new_state->fb) { ... } Which was wrong, as the fb were just assigned to be equal, so this if statement nevers evaluates to true. Another details is that the function drm_crtc_vblank_get() can only be called when vop->is_enabled is true, otherwise it has no effect and trows a WARN_ON(). Calling drm_atomic_set_fb_for_plane() (which get a referent of the new fb and pus the old fb) is not required, as it is taken care by drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane(). Signed-off-by: Helen Koike --- Hello, I tested on the rockchip ficus v1.1 using igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any regressions. It was me who worked on the first version and I am really sorry about that dead code in the if statement. Now I understand drm better and I know better how to properly test things with more care/details. Also, I didn't CC to stable here as I saw the async_update function was only added on v4.20, please let me know if I should CC to stable. Thanks! Helen drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 42 ++++++++++++--------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index c7d4c6073ea5..a1ee8c156a7b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -912,30 +912,31 @@ static void vop_plane_atomic_async_update(struct drm_plane *plane, struct drm_plane_state *new_state) { struct vop *vop = to_vop(plane->state->crtc); - struct drm_plane_state *plane_state; + struct drm_framebuffer *old_fb = plane->state->fb; - plane_state = plane->funcs->atomic_duplicate_state(plane); - plane_state->crtc_x = new_state->crtc_x; - plane_state->crtc_y = new_state->crtc_y; - plane_state->crtc_h = new_state->crtc_h; - plane_state->crtc_w = new_state->crtc_w; - plane_state->src_x = new_state->src_x; - plane_state->src_y = new_state->src_y; - plane_state->src_h = new_state->src_h; - plane_state->src_w = new_state->src_w; - - if (plane_state->fb != new_state->fb) - drm_atomic_set_fb_for_plane(plane_state, new_state->fb); - - swap(plane_state, plane->state); - - if (plane->state->fb && plane->state->fb != new_state->fb) { + /* + * A scanout can still be occurring, so we can't drop the reference to + * the old framebuffer. To solve this we get a reference to old_fb and + * set a worker to release it later. + */ + if (vop->is_enabled && + plane->state->fb && plane->state->fb != new_state->fb) { drm_framebuffer_get(plane->state->fb); WARN_ON(drm_crtc_vblank_get(plane->state->crtc) != 0); drm_flip_work_queue(&vop->fb_unref_work, plane->state->fb); set_bit(VOP_PENDING_FB_UNREF, &vop->pending); } + plane->state->crtc_x = new_state->crtc_x; + plane->state->crtc_y = new_state->crtc_y; + plane->state->crtc_h = new_state->crtc_h; + plane->state->crtc_w = new_state->crtc_w; + plane->state->src_x = new_state->src_x; + plane->state->src_y = new_state->src_y; + plane->state->src_h = new_state->src_h; + plane->state->src_w = new_state->src_w; + plane->state->fb = new_state->fb; + if (vop->is_enabled) { rockchip_drm_psr_inhibit_get_state(new_state->state); vop_plane_atomic_update(plane, plane->state); @@ -945,7 +946,12 @@ static void vop_plane_atomic_async_update(struct drm_plane *plane, rockchip_drm_psr_inhibit_put_state(new_state->state); } - plane->funcs->atomic_destroy_state(plane, plane_state); + /* + * In async update we perform inplace modifications and release the + * new_state. The following is required so we release the reference of + * the old framebuffer. + */ + new_state->fb = old_fb; } static const struct drm_plane_helper_funcs plane_helper_funcs = { From patchwork Mon Mar 4 14:49:07 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: 10837853 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 6552117E0 for ; Mon, 4 Mar 2019 14:49:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 51CF02A0D1 for ; Mon, 4 Mar 2019 14:49:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 45E342A126; Mon, 4 Mar 2019 14:49:40 +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 00C9A2A0D1 for ; Mon, 4 Mar 2019 14:49:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E629089CE2; Mon, 4 Mar 2019 14:49:38 +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 [46.235.227.227]) by gabe.freedesktop.org (Postfix) with ESMTPS id AADAD89CF6 for ; Mon, 4 Mar 2019 14:49:37 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a1b1:9d17:4c45:79a4:9c61]) (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 8DC2C2807B0; Mon, 4 Mar 2019 14:49:32 +0000 (GMT) From: Helen Koike To: dri-devel@lists.freedesktop.org, nicholas.kazlauskas@amd.com Subject: [PATCH 3/5] drm/amd: fix fb references in async update Date: Mon, 4 Mar 2019 11:49:07 -0300 Message-Id: <20190304144909.6267-4-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190304144909.6267-1-helen.koike@collabora.com> References: <20190304144909.6267-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: =?utf-8?q?St=C3=A9phane_Marchesin?= , Sean Paul , David Airlie , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, Tomasz Figa , boris.brezillon@collabora.com, kernel@collabora.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Async update callbacks are expected to set the old_fb in the new_state so prepare/cleanup framebuffers are balanced. Calling drm_atomic_set_fb_for_plane() (which gets a reference of the new fb and put the old fb) is not required, as it's taken care by drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane(). Suggested-by: Boris Brezillon Signed-off-by: Helen Koike Reviewed-by: Nicholas Kazlauskas --- Hello, As mentioned in the cover letter, I tested on the rockchip and on i915 using igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any regressions. But I couldn't test on AMD because I don't have the hardware and I would appreciate if anyone could test it. Also, I didn't CC to stable here as I saw the async_update function was only added on v4.20, please let me know if I should CC to stable. Thanks! Helen drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-- 1 file changed, 1 insertion(+), 2 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 3a6f595f295e..bc02800254bf 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3760,8 +3760,7 @@ static void dm_plane_atomic_async_update(struct drm_plane *plane, struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(new_state->state, plane); - if (plane->state->fb != new_state->fb) - drm_atomic_set_fb_for_plane(plane->state, new_state->fb); + swap(plane->state->fb, new_state->fb); plane->state->src_x = new_state->src_x; plane->state->src_y = new_state->src_y; From patchwork Mon Mar 4 14:49:08 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: 10837855 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 A7EBA1803 for ; Mon, 4 Mar 2019 14:49:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9501D2A0D1 for ; Mon, 4 Mar 2019 14:49:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 88B952A15E; Mon, 4 Mar 2019 14:49:45 +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 3FD2D2A0D1 for ; Mon, 4 Mar 2019 14:49:45 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4A7E089CF5; Mon, 4 Mar 2019 14:49:44 +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 [46.235.227.227]) by gabe.freedesktop.org (Postfix) with ESMTPS id E0DF689CF5 for ; Mon, 4 Mar 2019 14:49:42 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a1b1:9d17:4c45:79a4:9c61]) (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 C37A328079A; Mon, 4 Mar 2019 14:49:36 +0000 (GMT) From: Helen Koike To: dri-devel@lists.freedesktop.org, nicholas.kazlauskas@amd.com Subject: [PATCH 4/5] drm/msm: fix fb references in async update Date: Mon, 4 Mar 2019 11:49:08 -0300 Message-Id: <20190304144909.6267-5-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190304144909.6267-1-helen.koike@collabora.com> References: <20190304144909.6267-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: =?utf-8?q?St=C3=A9phane_Marchesin?= , Sean Paul , David Airlie , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, Tomasz Figa , boris.brezillon@collabora.com, kernel@collabora.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Async update callbacks are expected to set the old_fb in the new_state so prepare/cleanup framebuffers are balanced. Cc: # v4.14+: 25dc194b34dd: drm: Block fb changes for async plane updates Cc: # v4.14+: 8105bbaf9afd: drm: don't block fb changes for async plane updates Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates") Suggested-by: Boris Brezillon Signed-off-by: Helen Koike --- Hello, As mentioned in the cover letter, I tested on the rockchip and on i915 using igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any regressions. But I couldn't test on MSM because I don't have the hardware and I would appreciate if anyone could test it. In other platforms (VC4, AMD, Rockchip), there is a hidden drm_framebuffer_get(new_fb)/drm_framebuffer_put(old_fb) in async_update that is wrong, but I couldn't identify those here, not sure if it is hidden somewhere else, but if tests fail this is probably the cause. Thanks! Helen drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c index be13140967b4..b854f471e9e5 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c @@ -502,6 +502,8 @@ static int mdp5_plane_atomic_async_check(struct drm_plane *plane, static void mdp5_plane_atomic_async_update(struct drm_plane *plane, struct drm_plane_state *new_state) { + struct drm_framebuffer *old_fb = plane->state->fb; + plane->state->src_x = new_state->src_x; plane->state->src_y = new_state->src_y; plane->state->crtc_x = new_state->crtc_x; @@ -524,6 +526,8 @@ static void mdp5_plane_atomic_async_update(struct drm_plane *plane, *to_mdp5_plane_state(plane->state) = *to_mdp5_plane_state(new_state); + + new_state->fb = old_fb; } static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = { From patchwork Mon Mar 4 14:49:09 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: 10837857 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 C5BA91869 for ; Mon, 4 Mar 2019 14:49:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B144E2A0D1 for ; Mon, 4 Mar 2019 14:49:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A55AD2A15E; Mon, 4 Mar 2019 14:49:49 +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 5A8372A0D1 for ; Mon, 4 Mar 2019 14:49:49 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7FD5289CF6; Mon, 4 Mar 2019 14:49:48 +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 EAA7C89CF6 for ; Mon, 4 Mar 2019 14:49:46 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2804:431:9718:a1b1:9d17:4c45:79a4:9c61]) (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 585DD2807B1; Mon, 4 Mar 2019 14:49:42 +0000 (GMT) From: Helen Koike To: dri-devel@lists.freedesktop.org, nicholas.kazlauskas@amd.com Subject: [PATCH 5/5] drm/vc4: fix fb references in async update Date: Mon, 4 Mar 2019 11:49:09 -0300 Message-Id: <20190304144909.6267-6-helen.koike@collabora.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190304144909.6267-1-helen.koike@collabora.com> References: <20190304144909.6267-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: =?utf-8?q?St=C3=A9phane_Marchesin?= , Sean Paul , David Airlie , daniel.vetter@ffwll.ch, linux-kernel@vger.kernel.org, Tomasz Figa , boris.brezillon@collabora.com, kernel@collabora.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Async update callbacks are expected to set the old_fb in the new_state so prepare/cleanup framebuffers are balanced. Calling drm_atomic_set_fb_for_plane() (which gets a reference of the new fb and put the old fb) is not required, as it's taken care by drm_mode_cursor_universal() when calling drm_atomic_helper_update_plane(). Cc: # v4.19+: 25dc194b34dd: drm: Block fb changes for async plane updates Cc: # v4.19+: 8105bbaf9afd: drm: don't block fb changes for async plane updates Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates") Suggested-by: Boris Brezillon Signed-off-by: Helen Koike Reviewed-by: Boris Brezillon --- Hello, As mentioned in the cover letter, I tested on the rockchip and on i915 using igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any regressions. But I couldn't test on VC4. I have a Raspberry pi model B rev2, when FB_SIMPLE is running I can see output on the screen, but when vc4 is loaded my hdmi display is not detected anymore, I am still debugging this, probably some config in the firmware, but I would appreciate if anyone could help me testing it. Also the Cc statble commit hash dependency needs to be updated once the refered commit is merged. Thanks! Helen drivers/gpu/drm/vc4/vc4_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 1babfeca0c92..1235e53b22a3 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -968,7 +968,7 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, { struct vc4_plane_state *vc4_state, *new_vc4_state; - drm_atomic_set_fb_for_plane(plane->state, state->fb); + swap(plane->state->fb, state->fb); plane->state->crtc_x = state->crtc_x; plane->state->crtc_y = state->crtc_y; plane->state->crtc_w = state->crtc_w;