From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 761C5C35E01 for ; Tue, 25 Feb 2020 15:34:06 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 57A6221744 for ; Tue, 25 Feb 2020 15:34:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 57A6221744 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A7BCA6E8CA; Tue, 25 Feb 2020 15:34:05 +0000 (UTC) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id AB20E6E8CA; Tue, 25 Feb 2020 15:34:04 +0000 (UTC) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Feb 2020 07:34:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,484,1574150400"; d="scan'208";a="271349579" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by fmsmga002.fm.intel.com with SMTP; 25 Feb 2020 07:34:00 -0800 Received: by stinkbox (sSMTP sendmail emulation); Tue, 25 Feb 2020 17:34:00 +0200 Date: Tue, 25 Feb 2020 17:34:00 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Daniel Vetter Subject: Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets Message-ID: <20200225153400.GE13686@intel.com> References: <20200225115024.2386811-1-daniel.vetter@ffwll.ch> <20200225144814.GC13686@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Patchwork-Hint: comment User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Stone , Intel Graphics Development , stable , DRI Development , Daniel Vetter , Pekka Paalanen Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, Feb 25, 2020 at 04:09:26PM +0100, Daniel Vetter wrote: > On Tue, Feb 25, 2020 at 3:48 PM Ville Syrj=E4l=E4 > wrote: > > > > On Tue, Feb 25, 2020 at 12:50:24PM +0100, Daniel Vetter wrote: > > > 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-Jul= y/182281.html > > > Bugzilla: https://gitlab.freedesktop.org/wayland/weston/issues/24#not= e_9568 > > > Cc: Daniel Stone > > > Cc: Pekka Paalanen > > > Cc: stable@vger.kernel.org > > > Reviewed-by: Daniel Stone > > > 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_atomi= c.c > > > index 9ccfbf213d72..4c035abf98b8 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -1362,15 +1362,43 @@ EXPORT_SYMBOL(drm_atomic_commit); > > > int drm_atomic_nonblocking_commit(struct drm_atomic_state *state) > > > { > > > struct drm_mode_config *config =3D &state->dev->mode_config; > > > - int ret; > > > + unsigned requested_crtc =3D 0; > > > + unsigned affected_crtc =3D 0; > > > + struct drm_crtc *crtc; > > > + struct drm_crtc_state *crtc_state; > > > + bool nonblocking =3D 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 reso= urces. > > > + * > > > + * But when userspace also requests a nonblocking commit then u= serspace > > > + * cannot know that the commit affects other CRTCs, which can r= esult 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 |=3D drm_crtc_mask(crtc); > > > > > > ret =3D 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 |=3D drm_crtc_mask(crtc); > > > + > > > + if (affected_crtc !=3D requested_crtc) { > > > + /* adding other CRTC is only allowed for modeset commit= s */ > > > + WARN_ON(!state->allow_modeset); > > > > Not sure that's really true. What if the driver needs to eg. > > redistribute FIFO space or something between the pipes? Or do we > > expect drivers to now examine state->allow_modeset to figure out > > if they're allowed to do certain things? > = > Maybe we need more fine-grained flags here, but adding other states > (and blocking a commit flow) is exactly the uapi headaches this patch > tries to solve here. So if our driver currently adds crtc states to > reallocate fifo between pipes for an atomic flip then yes we're > breaking userspace. Well, everyone figured out by now that you get > random EBUSY and dropped frames for no apparent reason at all, and > work around it. But happy, they are not. I don't think we do this currently for the FIFO, but in theory we could. The one thing we might do currently is cdclk reprogramming, but that can only happen without a full modeset when there's only a single active pipe. So we shouldn't hit this right now. But that restriction is going to disappear in the future, at which point we may want to do this even with multiple active pipes. -- = Ville Syrj=E4l=E4 Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 65197C35E04 for ; Tue, 25 Feb 2020 15:34:08 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 43E9121744 for ; Tue, 25 Feb 2020 15:34:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 43E9121744 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7D6C06E8DC; Tue, 25 Feb 2020 15:34:06 +0000 (UTC) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id AB20E6E8CA; Tue, 25 Feb 2020 15:34:04 +0000 (UTC) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Feb 2020 07:34:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,484,1574150400"; d="scan'208";a="271349579" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by fmsmga002.fm.intel.com with SMTP; 25 Feb 2020 07:34:00 -0800 Received: by stinkbox (sSMTP sendmail emulation); Tue, 25 Feb 2020 17:34:00 +0200 Date: Tue, 25 Feb 2020 17:34:00 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Daniel Vetter Message-ID: <20200225153400.GE13686@intel.com> References: <20200225115024.2386811-1-daniel.vetter@ffwll.ch> <20200225144814.GC13686@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Patchwork-Hint: comment User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Stone , Intel Graphics Development , stable , DRI Development , Daniel Vetter , Pekka Paalanen Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, Feb 25, 2020 at 04:09:26PM +0100, Daniel Vetter wrote: > On Tue, Feb 25, 2020 at 3:48 PM Ville Syrj=E4l=E4 > wrote: > > > > On Tue, Feb 25, 2020 at 12:50:24PM +0100, Daniel Vetter wrote: > > > 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-Jul= y/182281.html > > > Bugzilla: https://gitlab.freedesktop.org/wayland/weston/issues/24#not= e_9568 > > > Cc: Daniel Stone > > > Cc: Pekka Paalanen > > > Cc: stable@vger.kernel.org > > > Reviewed-by: Daniel Stone > > > 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_atomi= c.c > > > index 9ccfbf213d72..4c035abf98b8 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -1362,15 +1362,43 @@ EXPORT_SYMBOL(drm_atomic_commit); > > > int drm_atomic_nonblocking_commit(struct drm_atomic_state *state) > > > { > > > struct drm_mode_config *config =3D &state->dev->mode_config; > > > - int ret; > > > + unsigned requested_crtc =3D 0; > > > + unsigned affected_crtc =3D 0; > > > + struct drm_crtc *crtc; > > > + struct drm_crtc_state *crtc_state; > > > + bool nonblocking =3D 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 reso= urces. > > > + * > > > + * But when userspace also requests a nonblocking commit then u= serspace > > > + * cannot know that the commit affects other CRTCs, which can r= esult 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 |=3D drm_crtc_mask(crtc); > > > > > > ret =3D 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 |=3D drm_crtc_mask(crtc); > > > + > > > + if (affected_crtc !=3D requested_crtc) { > > > + /* adding other CRTC is only allowed for modeset commit= s */ > > > + WARN_ON(!state->allow_modeset); > > > > Not sure that's really true. What if the driver needs to eg. > > redistribute FIFO space or something between the pipes? Or do we > > expect drivers to now examine state->allow_modeset to figure out > > if they're allowed to do certain things? > = > Maybe we need more fine-grained flags here, but adding other states > (and blocking a commit flow) is exactly the uapi headaches this patch > tries to solve here. So if our driver currently adds crtc states to > reallocate fifo between pipes for an atomic flip then yes we're > breaking userspace. Well, everyone figured out by now that you get > random EBUSY and dropped frames for no apparent reason at all, and > work around it. But happy, they are not. I don't think we do this currently for the FIFO, but in theory we could. The one thing we might do currently is cdclk reprogramming, but that can only happen without a full modeset when there's only a single active pipe. So we shouldn't hit this right now. But that restriction is going to disappear in the future, at which point we may want to do this even with multiple active pipes. -- = Ville Syrj=E4l=E4 Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BB13CC35E04 for ; Tue, 25 Feb 2020 15:34:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 98B5721744 for ; Tue, 25 Feb 2020 15:34:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731042AbgBYPeF (ORCPT ); Tue, 25 Feb 2020 10:34:05 -0500 Received: from mga18.intel.com ([134.134.136.126]:18134 "EHLO mga18.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730971AbgBYPeF (ORCPT ); Tue, 25 Feb 2020 10:34:05 -0500 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Feb 2020 07:34:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,484,1574150400"; d="scan'208";a="271349579" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by fmsmga002.fm.intel.com with SMTP; 25 Feb 2020 07:34:00 -0800 Received: by stinkbox (sSMTP sendmail emulation); Tue, 25 Feb 2020 17:34:00 +0200 Date: Tue, 25 Feb 2020 17:34:00 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Daniel Vetter Cc: DRI Development , Daniel Stone , Intel Graphics Development , stable , Daniel Vetter , Pekka Paalanen Subject: Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets Message-ID: <20200225153400.GE13686@intel.com> References: <20200225115024.2386811-1-daniel.vetter@ffwll.ch> <20200225144814.GC13686@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Patchwork-Hint: comment User-Agent: Mutt/1.10.1 (2018-07-13) Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org On Tue, Feb 25, 2020 at 04:09:26PM +0100, Daniel Vetter wrote: > On Tue, Feb 25, 2020 at 3:48 PM Ville Syrjälä > wrote: > > > > On Tue, Feb 25, 2020 at 12:50:24PM +0100, Daniel Vetter wrote: > > > 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 > > > Reviewed-by: Daniel Stone > > > 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 9ccfbf213d72..4c035abf98b8 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -1362,15 +1362,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); > > > > Not sure that's really true. What if the driver needs to eg. > > redistribute FIFO space or something between the pipes? Or do we > > expect drivers to now examine state->allow_modeset to figure out > > if they're allowed to do certain things? > > Maybe we need more fine-grained flags here, but adding other states > (and blocking a commit flow) is exactly the uapi headaches this patch > tries to solve here. So if our driver currently adds crtc states to > reallocate fifo between pipes for an atomic flip then yes we're > breaking userspace. Well, everyone figured out by now that you get > random EBUSY and dropped frames for no apparent reason at all, and > work around it. But happy, they are not. I don't think we do this currently for the FIFO, but in theory we could. The one thing we might do currently is cdclk reprogramming, but that can only happen without a full modeset when there's only a single active pipe. So we shouldn't hit this right now. But that restriction is going to disappear in the future, at which point we may want to do this even with multiple active pipes. -- Ville Syrjälä Intel