From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f65.google.com ([209.85.208.65]:35322 "EHLO mail-ed1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753155AbeGEKKu (ORCPT ); Thu, 5 Jul 2018 06:10:50 -0400 Received: by mail-ed1-f65.google.com with SMTP id b10-v6so5943473edi.2 for ; Thu, 05 Jul 2018 03:10:49 -0700 (PDT) From: Daniel Vetter To: DRI Development Cc: Intel Graphics Development , Daniel Vetter , Daniel Stone , Pekka Paalanen , stable@vger.kernel.org, Daniel Vetter Subject: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets Date: Thu, 5 Jul 2018 12:10:43 +0200 Message-Id: <20180705101043.4883-1-daniel.vetter@ffwll.ch> Sender: stable-owner@vger.kernel.org List-ID: When doing an atomic modeset with ALLOW_MODESET drivers are allowed to pull in arbitrary other resources, including CRTCs (e.g. when reconfiguring global resources). But in nonblocking mode userspace has then no idea this happened, which can lead to spurious EBUSY calls, both: - when that other CRTC is currently busy doing a page_flip the ALLOW_MODESET commit can fail with an EBUSY - on the other CRTC a normal atomic flip can fail with EBUSY because of the additional commit inserted by the kernel without userspace's knowledge For blocking commits this isn't a problem, because everyone else will just block until all the CRTC are reconfigured. Only thing userspace can notice is the dropped frames without any reason for why frames got dropped. Consensus is that we need new uapi to handle this properly, but no one has any idea what exactly the new uapi should look like. As a stop-gap plug this problem by demoting nonblocking commits which might cause issues by including CRTCs not in the original request to blocking commits. v2: Add comments and a WARN_ON to enforce this only when allowed - we don't want to silently convert page flips into blocking plane updates just because the driver is buggy. References: https://lists.freedesktop.org/archives/dri-devel/2018-July/182281.html Bugzilla: https://gitlab.freedesktop.org/wayland/weston/issues/24#note_9568 Cc: Daniel Stone Cc: Pekka Paalanen Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index d5cefb1cb2a2..058512f14772 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -2018,15 +2018,43 @@ EXPORT_SYMBOL(drm_atomic_commit); int drm_atomic_nonblocking_commit(struct drm_atomic_state *state) { struct drm_mode_config *config = &state->dev->mode_config; - int ret; + unsigned requested_crtc = 0; + unsigned affected_crtc = 0; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + bool nonblocking = true; + int ret, i; + + /* + * For commits that allow modesets drivers can add other CRTCs to the + * atomic commit, e.g. when they need to reallocate global resources. + * + * But when userspace also requests a nonblocking commit then userspace + * cannot know that the commit affects other CRTCs, which can result in + * spurious EBUSY failures. Until we have better uapi plug this by + * demoting such commits to blocking mode. + */ + for_each_new_crtc_in_state(state, crtc, crtc_state, i) + requested_crtc |= drm_crtc_mask(crtc); ret = drm_atomic_check_only(state); if (ret) return ret; - DRM_DEBUG_ATOMIC("committing %p nonblocking\n", state); + for_each_new_crtc_in_state(state, crtc, crtc_state, i) + affected_crtc |= drm_crtc_mask(crtc); + + if (affected_crtc != requested_crtc) { + /* adding other CRTC is only allowed for modeset commits */ + WARN_ON(state->allow_modeset); + + DRM_DEBUG_ATOMIC("demoting %p to blocking mode to avoid EBUSY\n", state); + nonblocking = false; + } else { + DRM_DEBUG_ATOMIC("committing %p nonblocking\n", state); + } - return config->funcs->atomic_commit(state->dev, state, true); + return config->funcs->atomic_commit(state->dev, state, nonblocking); } EXPORT_SYMBOL(drm_atomic_nonblocking_commit); -- 2.18.0 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets Date: Thu, 5 Jul 2018 12:10:43 +0200 Message-ID: <20180705101043.4883-1-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9C22A6ECD2 for ; Thu, 5 Jul 2018 10:10:50 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id e19-v6so5939787edq.7 for ; Thu, 05 Jul 2018 03:10:50 -0700 (PDT) List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: DRI Development Cc: Daniel Vetter , Intel Graphics Development , stable@vger.kernel.org, Pekka Paalanen , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org V2hlbiBkb2luZyBhbiBhdG9taWMgbW9kZXNldCB3aXRoIEFMTE9XX01PREVTRVQgZHJpdmVycyBh cmUgYWxsb3dlZCB0bwpwdWxsIGluIGFyYml0cmFyeSBvdGhlciByZXNvdXJjZXMsIGluY2x1ZGlu ZyBDUlRDcyAoZS5nLiB3aGVuCnJlY29uZmlndXJpbmcgZ2xvYmFsIHJlc291cmNlcykuCgpCdXQg aW4gbm9uYmxvY2tpbmcgbW9kZSB1c2Vyc3BhY2UgaGFzIHRoZW4gbm8gaWRlYSB0aGlzIGhhcHBl bmVkLAp3aGljaCBjYW4gbGVhZCB0byBzcHVyaW91cyBFQlVTWSBjYWxscywgYm90aDoKLSB3aGVu IHRoYXQgb3RoZXIgQ1JUQyBpcyBjdXJyZW50bHkgYnVzeSBkb2luZyBhIHBhZ2VfZmxpcCB0aGUK ICBBTExPV19NT0RFU0VUIGNvbW1pdCBjYW4gZmFpbCB3aXRoIGFuIEVCVVNZCi0gb24gdGhlIG90 aGVyIENSVEMgYSBub3JtYWwgYXRvbWljIGZsaXAgY2FuIGZhaWwgd2l0aCBFQlVTWSBiZWNhdXNl CiAgb2YgdGhlIGFkZGl0aW9uYWwgY29tbWl0IGluc2VydGVkIGJ5IHRoZSBrZXJuZWwgd2l0aG91 dCB1c2Vyc3BhY2UncwogIGtub3dsZWRnZQoKRm9yIGJsb2NraW5nIGNvbW1pdHMgdGhpcyBpc24n dCBhIHByb2JsZW0sIGJlY2F1c2UgZXZlcnlvbmUgZWxzZSB3aWxsCmp1c3QgYmxvY2sgdW50aWwg YWxsIHRoZSBDUlRDIGFyZSByZWNvbmZpZ3VyZWQuIE9ubHkgdGhpbmcgdXNlcnNwYWNlCmNhbiBu b3RpY2UgaXMgdGhlIGRyb3BwZWQgZnJhbWVzIHdpdGhvdXQgYW55IHJlYXNvbiBmb3Igd2h5IGZy YW1lcyBnb3QKZHJvcHBlZC4KCkNvbnNlbnN1cyBpcyB0aGF0IHdlIG5lZWQgbmV3IHVhcGkgdG8g aGFuZGxlIHRoaXMgcHJvcGVybHksIGJ1dCBubyBvbmUKaGFzIGFueSBpZGVhIHdoYXQgZXhhY3Rs eSB0aGUgbmV3IHVhcGkgc2hvdWxkIGxvb2sgbGlrZS4gQXMgYSBzdG9wLWdhcApwbHVnIHRoaXMg cHJvYmxlbSBieSBkZW1vdGluZyBub25ibG9ja2luZyBjb21taXRzIHdoaWNoIG1pZ2h0IGNhdXNl Cmlzc3VlcyBieSBpbmNsdWRpbmcgQ1JUQ3Mgbm90IGluIHRoZSBvcmlnaW5hbCByZXF1ZXN0IHRv IGJsb2NraW5nCmNvbW1pdHMuCgp2MjogQWRkIGNvbW1lbnRzIGFuZCBhIFdBUk5fT04gdG8gZW5m b3JjZSB0aGlzIG9ubHkgd2hlbiBhbGxvd2VkIC0gd2UKZG9uJ3Qgd2FudCB0byBzaWxlbnRseSBj b252ZXJ0IHBhZ2UgZmxpcHMgaW50byBibG9ja2luZyBwbGFuZSB1cGRhdGVzCmp1c3QgYmVjYXVz ZSB0aGUgZHJpdmVyIGlzIGJ1Z2d5LgoKUmVmZXJlbmNlczogaHR0cHM6Ly9saXN0cy5mcmVlZGVz a3RvcC5vcmcvYXJjaGl2ZXMvZHJpLWRldmVsLzIwMTgtSnVseS8xODIyODEuaHRtbApCdWd6aWxs YTogaHR0cHM6Ly9naXRsYWIuZnJlZWRlc2t0b3Aub3JnL3dheWxhbmQvd2VzdG9uL2lzc3Vlcy8y NCNub3RlXzk1NjgKQ2M6IERhbmllbCBTdG9uZSA8ZGFuaWVsQGZvb2lzaGJhci5vcmc+CkNjOiBQ ZWtrYSBQYWFsYW5lbiA8cGVra2EucGFhbGFuZW5AY29sbGFib3JhLmNvLnVrPgpDYzogc3RhYmxl QHZnZXIua2VybmVsLm9yZwpTaWduZWQtb2ZmLWJ5OiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwudmV0 dGVyQGludGVsLmNvbT4KLS0tCiBkcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pYy5jIHwgMzQgKysr KysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tLQogMSBmaWxlIGNoYW5nZWQsIDMxIGluc2Vy dGlvbnMoKyksIDMgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2Ry bV9hdG9taWMuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljLmMKaW5kZXggZDVjZWZiMWNi MmEyLi4wNTg1MTJmMTQ3NzIgMTAwNjQ0Ci0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWlj LmMKKysrIGIvZHJpdmVycy9ncHUvZHJtL2RybV9hdG9taWMuYwpAQCAtMjAxOCwxNSArMjAxOCw0 MyBAQCBFWFBPUlRfU1lNQk9MKGRybV9hdG9taWNfY29tbWl0KTsKIGludCBkcm1fYXRvbWljX25v bmJsb2NraW5nX2NvbW1pdChzdHJ1Y3QgZHJtX2F0b21pY19zdGF0ZSAqc3RhdGUpCiB7CiAJc3Ry dWN0IGRybV9tb2RlX2NvbmZpZyAqY29uZmlnID0gJnN0YXRlLT5kZXYtPm1vZGVfY29uZmlnOwot CWludCByZXQ7CisJdW5zaWduZWQgcmVxdWVzdGVkX2NydGMgPSAwOworCXVuc2lnbmVkIGFmZmVj dGVkX2NydGMgPSAwOworCXN0cnVjdCBkcm1fY3J0YyAqY3J0YzsKKwlzdHJ1Y3QgZHJtX2NydGNf c3RhdGUgKmNydGNfc3RhdGU7CisJYm9vbCBub25ibG9ja2luZyA9IHRydWU7CisJaW50IHJldCwg aTsKKworCS8qCisJICogRm9yIGNvbW1pdHMgdGhhdCBhbGxvdyBtb2Rlc2V0cyBkcml2ZXJzIGNh biBhZGQgb3RoZXIgQ1JUQ3MgdG8gdGhlCisJICogYXRvbWljIGNvbW1pdCwgZS5nLiB3aGVuIHRo ZXkgbmVlZCB0byByZWFsbG9jYXRlIGdsb2JhbCByZXNvdXJjZXMuCisJICoKKwkgKiBCdXQgd2hl biB1c2Vyc3BhY2UgYWxzbyByZXF1ZXN0cyBhIG5vbmJsb2NraW5nIGNvbW1pdCB0aGVuIHVzZXJz cGFjZQorCSAqIGNhbm5vdCBrbm93IHRoYXQgdGhlIGNvbW1pdCBhZmZlY3RzIG90aGVyIENSVENz LCB3aGljaCBjYW4gcmVzdWx0IGluCisJICogc3B1cmlvdXMgRUJVU1kgZmFpbHVyZXMuIFVudGls IHdlIGhhdmUgYmV0dGVyIHVhcGkgcGx1ZyB0aGlzIGJ5CisJICogZGVtb3Rpbmcgc3VjaCBjb21t aXRzIHRvIGJsb2NraW5nIG1vZGUuCisJICovCisJZm9yX2VhY2hfbmV3X2NydGNfaW5fc3RhdGUo c3RhdGUsIGNydGMsIGNydGNfc3RhdGUsIGkpCisJCXJlcXVlc3RlZF9jcnRjIHw9IGRybV9jcnRj X21hc2soY3J0Yyk7CiAKIAlyZXQgPSBkcm1fYXRvbWljX2NoZWNrX29ubHkoc3RhdGUpOwogCWlm IChyZXQpCiAJCXJldHVybiByZXQ7CiAKLQlEUk1fREVCVUdfQVRPTUlDKCJjb21taXR0aW5nICVw IG5vbmJsb2NraW5nXG4iLCBzdGF0ZSk7CisJZm9yX2VhY2hfbmV3X2NydGNfaW5fc3RhdGUoc3Rh dGUsIGNydGMsIGNydGNfc3RhdGUsIGkpCisJCWFmZmVjdGVkX2NydGMgfD0gZHJtX2NydGNfbWFz ayhjcnRjKTsKKworCWlmIChhZmZlY3RlZF9jcnRjICE9IHJlcXVlc3RlZF9jcnRjKSB7CisJCS8q IGFkZGluZyBvdGhlciBDUlRDIGlzIG9ubHkgYWxsb3dlZCBmb3IgbW9kZXNldCBjb21taXRzICov CisJCVdBUk5fT04oc3RhdGUtPmFsbG93X21vZGVzZXQpOworCisJCURSTV9ERUJVR19BVE9NSUMo ImRlbW90aW5nICVwIHRvIGJsb2NraW5nIG1vZGUgdG8gYXZvaWQgRUJVU1lcbiIsIHN0YXRlKTsK KwkJbm9uYmxvY2tpbmcgPSBmYWxzZTsKKwl9IGVsc2UgeworCQlEUk1fREVCVUdfQVRPTUlDKCJj b21taXR0aW5nICVwIG5vbmJsb2NraW5nXG4iLCBzdGF0ZSk7CisJfQogCi0JcmV0dXJuIGNvbmZp Zy0+ZnVuY3MtPmF0b21pY19jb21taXQoc3RhdGUtPmRldiwgc3RhdGUsIHRydWUpOworCXJldHVy biBjb25maWctPmZ1bmNzLT5hdG9taWNfY29tbWl0KHN0YXRlLT5kZXYsIHN0YXRlLCBub25ibG9j a2luZyk7CiB9CiBFWFBPUlRfU1lNQk9MKGRybV9hdG9taWNfbm9uYmxvY2tpbmdfY29tbWl0KTsK IAotLSAKMi4xOC4wCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2 ZWwK