From patchwork Thu Jul 5 10:21:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10508543 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B4E41603D7 for ; Thu, 5 Jul 2018 10:21:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A482828EBB for ; Thu, 5 Jul 2018 10:21:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 992D028EC1; Thu, 5 Jul 2018 10:21:33 +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=unavailable 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 4C77C28EBB for ; Thu, 5 Jul 2018 10:21:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 35ABD6ECD8; Thu, 5 Jul 2018 10:21:30 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org 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 A6D596E184 for ; Thu, 5 Jul 2018 10:21:28 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id w14-v6so5968868eds.6 for ; Thu, 05 Jul 2018 03:21:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=wuClBbIXy2co/NnHGM4CF7n2DNCOcOlGwxTqvMFV4vQ=; b=Itl1UD2XhY+qft9mz28o7Bk9evz1XcE5wbnIFADtsMi6+tQh1JvYzKnhNUZuDCOk6J b1PjbzJDfxd1DJ3JWZrsLFP6d/3lEAwHUCe1RFETQMimWBlU5sxe/CadLmw7zihHLP/2 WnIqpUfCBqWF0BhB8AvuhbtDbhf9ThHbglVvD1Su9wqJdhQf1uUvzOncwH6wuMaCai2Y G4edKfsHJWi8f5sw/LkMVgaSD3ku5L2YS7E3NMiunS8pjmQLyVMvRV0iXoZTdE/XlpEd uB4hV8LjKsnkN2QojCSWujMnEY2p7u1E8PUdzkVlbUUT7ZLETRECMI06W1NwjGQ+eqNa joWA== X-Gm-Message-State: APt69E34XuJpBmrPTKcn+iHegLApYm2zv3gluI/ioSKs/j/9wIU9UgV7 MXMhdj6AsggClnW2uhvUrng++A== X-Google-Smtp-Source: AAOMgpdFESavD1Zul6zd/x5Jo8e7FPdjF4uo7UQLG1R2wJTMbS0k5GbOeK9I/dFYgIfPrGl9QCm0eQ== X-Received: by 2002:a50:9f8a:: with SMTP id c10-v6mr6263575edf.17.1530786087332; Thu, 05 Jul 2018 03:21:27 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:5628:0:496f:7dc5:66d7:a057]) by smtp.gmail.com with ESMTPSA id c15-v6sm2660251eds.19.2018.07.05.03.21.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 05 Jul 2018 03:21:26 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Thu, 5 Jul 2018 12:21:21 +0200 Message-Id: <20180705102121.5091-1-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180705101043.4883-1-daniel.vetter@ffwll.ch> References: <20180705101043.4883-1-daniel.vetter@ffwll.ch> Subject: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Vetter , Intel Graphics Development , stable@vger.kernel.org, Pekka Paalanen , Daniel Vetter MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP 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. v3: Fix inverted WARN_ON (Pekka). 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 Reviewed-by: Daniel Stone --- 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..3c46a4ef7898 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);