From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751476AbcHGF0O (ORCPT ); Sun, 7 Aug 2016 01:26:14 -0400 Received: from home.keithp.com ([63.227.221.253]:45766 "EHLO elaine.keithp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751091AbcHGF0N (ORCPT ); Sun, 7 Aug 2016 01:26:13 -0400 X-Amavis-Alert: BAD HEADER SECTION, Duplicate header field: "Cc" From: Keith Packard To: Daniel Vetter Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2] In-Reply-To: <20160802135417.GO6232@phenom.ffwll.local> References: <1469952534-6516-1-git-send-email-keithp@keithp.com> <20160802135417.GO6232@phenom.ffwll.local> User-Agent: Notmuch/0.22 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) cc: airlied@linux.ie, keithp@keithp.com Date: Sat, 06 Aug 2016 22:26:09 -0700 Message-ID: <86a8gpkv4e.fsf@hiro.keithp.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="==-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --==-=-= Content-Type: multipart/mixed; boundary="=-=-=" --=-=-= Content-Type: text/plain Daniel Vetter writes: > Hm, I can't see v1 anywhere, but I think it'd be better. You can't store > any transient state related to the current update in struct drm_plane. In > this case the cleanup_buffers from a previous update might overlap (for > nonblocking atomic commits) with the prepare_planes for the next one. > Either we need special cleanup vs. error-path code, or some flag somewhere > in the drm_plane_state. Ok, here's pretty much the previous version, which works now that I've fixed the intel driver. Instead of just comparing the fb's, I'm using the framebuffer_changed function, which seems like a nice bit of documentation if nothing else. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-drm-Don-t-prepare-or-cleanup-unchanging-frame-buffer.patch Content-Transfer-Encoding: quoted-printable From=20a75251d5762b1c200ed2f3dca2a5b00cc85bea95 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sat, 4 Jun 2016 01:16:22 -0700 Subject: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v3] When reconfiguring a plane position (as in moving the cursor), the frame buffer for the cursor isn't changing, so don't call the prepare or cleanup driver functions. This avoids making cursor position updates block on all pending rendering. v3: use drm_atomic_helper_framebuffer_changed in both prepare and cleanup phases instead of keeping state in the plane. cc: dri-devel@lists.freedesktop.org cc: David Airlie cc: Daniel Vetter Signed-off-by: Keith Packard =2D-- drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atom= ic_helper.c index ddfa0d1..72e50bc 100644 =2D-- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1246,18 +1246,19 @@ EXPORT_SYMBOL(drm_atomic_helper_commit); * Returns: * 0 on success, negative error code on failure. */ + int drm_atomic_helper_prepare_planes(struct drm_device *dev, struct drm_atomic_state *state) { =2D int nplanes =3D dev->mode_config.num_total_plane; + struct drm_plane *plane; + struct drm_plane_state *plane_state; int ret, i; + int max_prepared_i =3D 0; =20 =2D for (i =3D 0; i < nplanes; i++) { + for_each_plane_in_state(state, plane, plane_state, i) { const struct drm_plane_helper_funcs *funcs; =2D struct drm_plane *plane =3D state->planes[i]; =2D struct drm_plane_state *plane_state =3D state->plane_states[i]; =20 =2D if (!plane) + if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc= )) continue; =20 funcs =3D plane->helper_private; @@ -1267,24 +1268,25 @@ int drm_atomic_helper_prepare_planes(struct drm_dev= ice *dev, if (ret) goto fail; } + max_prepared_i =3D i; } =20 return 0; =20 fail: =2D for (i--; i >=3D 0; i--) { + for_each_plane_in_state(state, plane, plane_state, i) { const struct drm_plane_helper_funcs *funcs; =2D struct drm_plane *plane =3D state->planes[i]; =2D struct drm_plane_state *plane_state =3D state->plane_states[i]; =20 =2D if (!plane) + if (i > max_prepared_i) + break; + + if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc= )) continue; =20 funcs =3D plane->helper_private; =20 if (funcs->cleanup_fb) funcs->cleanup_fb(plane, plane_state); =2D } =20 return ret; @@ -1527,6 +1529,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_devi= ce *dev, for_each_plane_in_state(old_state, plane, plane_state, i) { const struct drm_plane_helper_funcs *funcs; =20 + if (!drm_atomic_helper_framebuffer_changed(dev, old_state, plane_state->= crtc)) + continue; + funcs =3D plane->helper_private; =20 if (funcs->cleanup_fb) =2D-=20 2.8.1 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable =2D-=20 =2Dkeith --=-=-=-- --==-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIVAwUBV6bGcdsiGmkAAAARAQivUhAAndxXe9SLJgrxAcnOYwbPQCnsFPTl1ZeF 7YrDJFQKCET5KgpjUTR7Q5UrZx44Bx/lFlgTqAHajd9Pl7dNmeTnF+Mdimk/qPzW r5WrwtyaLSF90e83Wyf+NTArMszB0TPR0PLfPDUtHtDev3oluYqGF0AewHBquGXm 8DoMYgv+aR6H37zcFhMsGOFsXSKWEfp7Wg99OZ647BJM9MYZDWBOwmAXP6+bofHI emhz3xJHD4wXa4gklKy+sLJPxuFpr1sF8ddFP/hljGQunbGhmtYeVhwVjZSmlFlG 2wgT+pUkz9snpUjEJ2OQkvt7qbYVcg7tzC6MEGZWCmVkvm9j32f94a9Fg7c0hjK/ mScpdFkIOOXuFOu9Gr2+VH3OOToBvdqIBVnA3pDLT/+RpGoomO5Ck7HTOB/WmQen TDWoCSenhxhlHW2jxP65y2k7Cgm4Cv4zJ9i9D9xlITFk/2AfgJyJK8xhO9zy6z2y ZSn/NCE5wbg4s0JJnujBTUJNRcdiehg/yQdc2aMs6WLJ4EkIIhAzJVKt1Li2iRn7 1hskdQVeQnHWQym6YbytrQcZqYume2tMQPsx+c4ErC0jRPXUX4YdESuQ3W1O+pCp vZ3WlcJXY6pO4qsWTRqQfZ9pSOzJQ37iYDbeQ03z6vmUXNOTdtHqT/bT+yFcd12t sz1Mfec+/DQ= =D7gL -----END PGP SIGNATURE----- --==-=-=--