All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2018-07-05 10:10 ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2018-07-05 10:10 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Stone,
	Pekka Paalanen, stable, Daniel Vetter

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 <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2018-07-05 10:10 ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2018-07-05 10:10 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, stable,
	Pekka Paalanen, Daniel Vetter

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 <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2018-07-05 10:10 ` Daniel Vetter
  (?)
@ 2018-07-05 10:21 ` Daniel Vetter
  2020-01-31  7:34     ` Daniel Stone
  -1 siblings, 1 reply; 54+ messages in thread
From: Daniel Vetter @ 2018-07-05 10:21 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Stone,
	Pekka Paalanen, stable, Daniel Vetter

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 <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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);
 
-- 
2.18.0

^ permalink raw reply	[flat|nested] 54+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2018-07-05 10:10 ` Daniel Vetter
  (?)
  (?)
@ 2018-07-05 10:25 ` Patchwork
  -1 siblings, 0 replies; 54+ messages in thread
From: Patchwork @ 2018-07-05 10:25 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: drm: avoid spurious EBUSY due to nonblocking atomic modesets
URL   : https://patchwork.freedesktop.org/series/45968/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ae0e8aaf3ac4 drm: avoid spurious EBUSY due to nonblocking atomic modesets
-:33: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#33: 
References: https://lists.freedesktop.org/archives/dri-devel/2018-July/182281.html

-:49: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#49: FILE: drivers/gpu/drm/drm_atomic.c:2021:
+	unsigned requested_crtc = 0;

-:50: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#50: FILE: drivers/gpu/drm/drm_atomic.c:2022:
+	unsigned affected_crtc = 0;

total: 0 errors, 3 warnings, 0 checks, 46 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* ✓ Fi.CI.BAT: success for drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2018-07-05 10:10 ` Daniel Vetter
                   ` (2 preceding siblings ...)
  (?)
@ 2018-07-05 10:41 ` Patchwork
  -1 siblings, 0 replies; 54+ messages in thread
From: Patchwork @ 2018-07-05 10:41 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: drm: avoid spurious EBUSY due to nonblocking atomic modesets
URL   : https://patchwork.freedesktop.org/series/45968/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4430 -> Patchwork_9534 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45968/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9534 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927


== Participating hosts (47 -> 41) ==

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-skl-iommu 


== Build changes ==

    * Linux: CI_DRM_4430 -> Patchwork_9534

  CI_DRM_4430: 6b02112a5dacd9e464c08c6d54f762bafd24937f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4537: 5a160e9e1fe19c67e58e9c298303cb94c96aeb7d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9534: ae0e8aaf3ac4f64bb159033b7400bb9eeaefa7bf @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ae0e8aaf3ac4 drm: avoid spurious EBUSY due to nonblocking atomic modesets

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9534/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm: avoid spurious EBUSY due to nonblocking atomic modesets (rev2)
  2018-07-05 10:10 ` Daniel Vetter
                   ` (3 preceding siblings ...)
  (?)
@ 2018-07-05 10:45 ` Patchwork
  -1 siblings, 0 replies; 54+ messages in thread
From: Patchwork @ 2018-07-05 10:45 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: drm: avoid spurious EBUSY due to nonblocking atomic modesets (rev2)
URL   : https://patchwork.freedesktop.org/series/45968/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
25d0f62b1aec drm: avoid spurious EBUSY due to nonblocking atomic modesets
-:35: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#35: 
References: https://lists.freedesktop.org/archives/dri-devel/2018-July/182281.html

-:51: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#51: FILE: drivers/gpu/drm/drm_atomic.c:2021:
+	unsigned requested_crtc = 0;

-:52: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#52: FILE: drivers/gpu/drm/drm_atomic.c:2022:
+	unsigned affected_crtc = 0;

total: 0 errors, 3 warnings, 0 checks, 46 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* ✓ Fi.CI.BAT: success for drm: avoid spurious EBUSY due to nonblocking atomic modesets (rev2)
  2018-07-05 10:10 ` Daniel Vetter
                   ` (4 preceding siblings ...)
  (?)
@ 2018-07-05 11:01 ` Patchwork
  -1 siblings, 0 replies; 54+ messages in thread
From: Patchwork @ 2018-07-05 11:01 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: drm: avoid spurious EBUSY due to nonblocking atomic modesets (rev2)
URL   : https://patchwork.freedesktop.org/series/45968/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4430 -> Patchwork_9535 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45968/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9535 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-dsi:         PASS -> FAIL (fdo#100368)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927


== Participating hosts (47 -> 42) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4430 -> Patchwork_9535

  CI_DRM_4430: 6b02112a5dacd9e464c08c6d54f762bafd24937f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4537: 5a160e9e1fe19c67e58e9c298303cb94c96aeb7d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9535: 25d0f62b1aec31059dce206cb9b821c58f7a0fc3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

25d0f62b1aec drm: avoid spurious EBUSY due to nonblocking atomic modesets

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9535/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* ✗ Fi.CI.IGT: failure for drm: avoid spurious EBUSY due to nonblocking atomic modesets (rev2)
  2018-07-05 10:10 ` Daniel Vetter
                   ` (5 preceding siblings ...)
  (?)
@ 2018-07-05 12:23 ` Patchwork
  -1 siblings, 0 replies; 54+ messages in thread
From: Patchwork @ 2018-07-05 12:23 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: drm: avoid spurious EBUSY due to nonblocking atomic modesets (rev2)
URL   : https://patchwork.freedesktop.org/series/45968/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4430_full -> Patchwork_9535_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9535_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9535_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9535_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_busy@basic-modeset-a:
      shard-glk:          PASS -> FAIL +3

    igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
      shard-apl:          PASS -> FAIL +4
      shard-hsw:          PASS -> FAIL +1

    igt@kms_cursor_legacy@nonblocking-modeset-vs-cursor-atomic:
      shard-kbl:          PASS -> FAIL +4

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          NOTRUN -> FAIL

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS +1

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_9535_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-bsd:
      shard-glk:          NOTRUN -> FAIL (fdo#103158) +1

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          NOTRUN -> FAIL (fdo#103822)

    igt@kms_setmode@basic:
      shard-glk:          NOTRUN -> FAIL (fdo#99912)
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@perf_pmu@other-read-4:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
      shard-hsw:          FAIL (fdo#103355) -> PASS

    
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4430 -> Patchwork_9535

  CI_DRM_4430: 6b02112a5dacd9e464c08c6d54f762bafd24937f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4537: 5a160e9e1fe19c67e58e9c298303cb94c96aeb7d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9535: 25d0f62b1aec31059dce206cb9b821c58f7a0fc3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9535/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2018-07-05 10:21 ` Daniel Vetter
  2020-01-31  7:34     ` Daniel Stone
@ 2020-01-31  7:34     ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-01-31  7:34 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Pekka Paalanen,
	stable, Daniel Vetter

On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.

Thanks for writing this up Daniel, and for reminding me about it some
time later as well ...

Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-01-31  7:34     ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-01-31  7:34 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.

Thanks for writing this up Daniel, and for reminding me about it some
time later as well ...

Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-01-31  7:34     ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-01-31  7:34 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.

Thanks for writing this up Daniel, and for reminding me about it some
time later as well ...

Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-01-31  7:34     ` Daniel Stone
  (?)
@ 2020-09-22 13:36       ` Marius Vlad
  -1 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-22 13:36 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Daniel Vetter, Pekka Paalanen, Intel Graphics Development,
	stable, DRI Development, Daniel Vetter

[-- Attachment #1: Type: text/plain, Size: 1944 bytes --]

On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
Gentle ping. I've tried out Linus's master tree and, and like Pekka,
I've noticed this isn't integrated/added.

Noticed this is fixing (also) DPMS when multiple outputs are in use.
Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
the warning quite often.

> 
> Thanks for writing this up Daniel, and for reminding me about it some
> time later as well ...
> 
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> 
> Cheers,
> Daniel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 13:36       ` Marius Vlad
  0 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-22 13:36 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	stable, Pekka Paalanen, Daniel Vetter


[-- Attachment #1.1: Type: text/plain, Size: 1944 bytes --]

On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
Gentle ping. I've tried out Linus's master tree and, and like Pekka,
I've noticed this isn't integrated/added.

Noticed this is fixing (also) DPMS when multiple outputs are in use.
Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
the warning quite often.

> 
> Thanks for writing this up Daniel, and for reminding me about it some
> time later as well ...
> 
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> 
> Cheers,
> Daniel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 13:36       ` Marius Vlad
  0 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-22 13:36 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	stable, Pekka Paalanen, Daniel Vetter


[-- Attachment #1.1: Type: text/plain, Size: 1944 bytes --]

On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
Gentle ping. I've tried out Linus's master tree and, and like Pekka,
I've noticed this isn't integrated/added.

Noticed this is fixing (also) DPMS when multiple outputs are in use.
Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
the warning quite often.

> 
> Thanks for writing this up Daniel, and for reminding me about it some
> time later as well ...
> 
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> 
> Cheers,
> Daniel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-09-22 13:36       ` Marius Vlad
  (?)
@ 2020-09-22 14:04         ` Daniel Vetter
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-22 14:04 UTC (permalink / raw)
  To: Marius Vlad
  Cc: Daniel Stone, Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> I've noticed this isn't integrated/added.

Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.

> Noticed this is fixing (also) DPMS when multiple outputs are in use.
> Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> the warning quite often.

This would be a driver bug I think. That really shouldn't happen for
normal page flips.
-Daniel

> >
> > Thanks for writing this up Daniel, and for reminding me about it some
> > time later as well ...
> >
> > Reviewed-by: Daniel Stone <daniels@collabora.com>
> >
> > Cheers,
> > Daniel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 14:04         ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-22 14:04 UTC (permalink / raw)
  To: Marius Vlad
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen

On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> I've noticed this isn't integrated/added.

Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.

> Noticed this is fixing (also) DPMS when multiple outputs are in use.
> Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> the warning quite often.

This would be a driver bug I think. That really shouldn't happen for
normal page flips.
-Daniel

> >
> > Thanks for writing this up Daniel, and for reminding me about it some
> > time later as well ...
> >
> > Reviewed-by: Daniel Stone <daniels@collabora.com>
> >
> > Cheers,
> > Daniel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 14:04         ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-22 14:04 UTC (permalink / raw)
  To: Marius Vlad
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen

On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> I've noticed this isn't integrated/added.

Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.

> Noticed this is fixing (also) DPMS when multiple outputs are in use.
> Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> the warning quite often.

This would be a driver bug I think. That really shouldn't happen for
normal page flips.
-Daniel

> >
> > Thanks for writing this up Daniel, and for reminding me about it some
> > time later as well ...
> >
> > Reviewed-by: Daniel Stone <daniels@collabora.com>
> >
> > Cheers,
> > Daniel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-09-22 14:04         ` Daniel Vetter
  (?)
@ 2020-09-22 14:14           ` Daniel Stone
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-09-22 14:14 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Marius Vlad, Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

On Tue, 22 Sep 2020 at 15:04, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > I've noticed this isn't integrated/added.
>
> Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.

This really, really, really, bites.

I think we need a guarantee that this never happens if ALLOW_MODESET
is always used in blocking mode, plus in future a cap we can use to
detect that we won't be getting spurious EBUSY events.

I really don't want to ever paper over this, because it's one of the
clearest indications that userspace has its timing/signalling wrong.

Cheers,
Daniel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 14:14           ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-09-22 14:14 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen, Marius Vlad

On Tue, 22 Sep 2020 at 15:04, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > I've noticed this isn't integrated/added.
>
> Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.

This really, really, really, bites.

I think we need a guarantee that this never happens if ALLOW_MODESET
is always used in blocking mode, plus in future a cap we can use to
detect that we won't be getting spurious EBUSY events.

I really don't want to ever paper over this, because it's one of the
clearest indications that userspace has its timing/signalling wrong.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 14:14           ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-09-22 14:14 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen, Marius Vlad

On Tue, 22 Sep 2020 at 15:04, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > I've noticed this isn't integrated/added.
>
> Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.

This really, really, really, bites.

I think we need a guarantee that this never happens if ALLOW_MODESET
is always used in blocking mode, plus in future a cap we can use to
detect that we won't be getting spurious EBUSY events.

I really don't want to ever paper over this, because it's one of the
clearest indications that userspace has its timing/signalling wrong.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-09-22 14:14           ` Daniel Stone
  (?)
@ 2020-09-22 16:01             ` Daniel Vetter
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-22 16:01 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Marius Vlad, Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

On Tue, Sep 22, 2020 at 4:14 PM Daniel Stone <daniel@fooishbar.org> wrote:
>
> On Tue, 22 Sep 2020 at 15:04, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > I've noticed this isn't integrated/added.
> >
> > Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.
>
> This really, really, really, bites.
>
> I think we need a guarantee that this never happens if ALLOW_MODESET
> is always used in blocking mode, plus in future a cap we can use to
> detect that we won't be getting spurious EBUSY events.
>
> I really don't want to ever paper over this, because it's one of the
> clearest indications that userspace has its timing/signalling wrong.

Ok so the hang-up last time around iirc was that I broke igt by making
a few things more synchronous. Let's hope I'm not also breaking stuff
with the WARN_ON ...

New plan:
- make this patch here only document existing behaviour and enforce it
with the WARN_ON
- new uapi would be behind a flag or something, with userspace and
everything hanging off it.

Thoughts?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 16:01             ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-22 16:01 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen, Marius Vlad

On Tue, Sep 22, 2020 at 4:14 PM Daniel Stone <daniel@fooishbar.org> wrote:
>
> On Tue, 22 Sep 2020 at 15:04, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > I've noticed this isn't integrated/added.
> >
> > Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.
>
> This really, really, really, bites.
>
> I think we need a guarantee that this never happens if ALLOW_MODESET
> is always used in blocking mode, plus in future a cap we can use to
> detect that we won't be getting spurious EBUSY events.
>
> I really don't want to ever paper over this, because it's one of the
> clearest indications that userspace has its timing/signalling wrong.

Ok so the hang-up last time around iirc was that I broke igt by making
a few things more synchronous. Let's hope I'm not also breaking stuff
with the WARN_ON ...

New plan:
- make this patch here only document existing behaviour and enforce it
with the WARN_ON
- new uapi would be behind a flag or something, with userspace and
everything hanging off it.

Thoughts?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 16:01             ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-22 16:01 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen, Marius Vlad

On Tue, Sep 22, 2020 at 4:14 PM Daniel Stone <daniel@fooishbar.org> wrote:
>
> On Tue, 22 Sep 2020 at 15:04, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > I've noticed this isn't integrated/added.
> >
> > Defacto the uapi we have now is that userspace needs to ignore "spurious" EBUSY.
>
> This really, really, really, bites.
>
> I think we need a guarantee that this never happens if ALLOW_MODESET
> is always used in blocking mode, plus in future a cap we can use to
> detect that we won't be getting spurious EBUSY events.
>
> I really don't want to ever paper over this, because it's one of the
> clearest indications that userspace has its timing/signalling wrong.

Ok so the hang-up last time around iirc was that I broke igt by making
a few things more synchronous. Let's hope I'm not also breaking stuff
with the WARN_ON ...

New plan:
- make this patch here only document existing behaviour and enforce it
with the WARN_ON
- new uapi would be behind a flag or something, with userspace and
everything hanging off it.

Thoughts?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-09-22 16:01             ` Daniel Vetter
  (?)
@ 2020-09-22 19:02               ` Daniel Stone
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-09-22 19:02 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Marius Vlad, Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

Hi,

On Tue, 22 Sep 2020 at 17:02, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Tue, Sep 22, 2020 at 4:14 PM Daniel Stone <daniel@fooishbar.org> wrote:
> > I think we need a guarantee that this never happens if ALLOW_MODESET
> > is always used in blocking mode, plus in future a cap we can use to
> > detect that we won't be getting spurious EBUSY events.
> >
> > I really don't want to ever paper over this, because it's one of the
> > clearest indications that userspace has its timing/signalling wrong.
>
> Ok so the hang-up last time around iirc was that I broke igt by making
> a few things more synchronous. Let's hope I'm not also breaking stuff
> with the WARN_ON ...
>
> New plan:
> - make this patch here only document existing behaviour and enforce it
> with the WARN_ON
> - new uapi would be behind a flag or something, with userspace and
> everything hanging off it.
>
> Thoughts?

What do you mean by 'new uapi'? The proposal that the kernel
communicates back which object IDs have been added to the state behind
your back? That it's been made automatically blocking? Something else?

Cheers,
Daniel (the other one)

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 19:02               ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-09-22 19:02 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen, Marius Vlad

Hi,

On Tue, 22 Sep 2020 at 17:02, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Tue, Sep 22, 2020 at 4:14 PM Daniel Stone <daniel@fooishbar.org> wrote:
> > I think we need a guarantee that this never happens if ALLOW_MODESET
> > is always used in blocking mode, plus in future a cap we can use to
> > detect that we won't be getting spurious EBUSY events.
> >
> > I really don't want to ever paper over this, because it's one of the
> > clearest indications that userspace has its timing/signalling wrong.
>
> Ok so the hang-up last time around iirc was that I broke igt by making
> a few things more synchronous. Let's hope I'm not also breaking stuff
> with the WARN_ON ...
>
> New plan:
> - make this patch here only document existing behaviour and enforce it
> with the WARN_ON
> - new uapi would be behind a flag or something, with userspace and
> everything hanging off it.
>
> Thoughts?

What do you mean by 'new uapi'? The proposal that the kernel
communicates back which object IDs have been added to the state behind
your back? That it's been made automatically blocking? Something else?

Cheers,
Daniel (the other one)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-22 19:02               ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-09-22 19:02 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen, Marius Vlad

Hi,

On Tue, 22 Sep 2020 at 17:02, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Tue, Sep 22, 2020 at 4:14 PM Daniel Stone <daniel@fooishbar.org> wrote:
> > I think we need a guarantee that this never happens if ALLOW_MODESET
> > is always used in blocking mode, plus in future a cap we can use to
> > detect that we won't be getting spurious EBUSY events.
> >
> > I really don't want to ever paper over this, because it's one of the
> > clearest indications that userspace has its timing/signalling wrong.
>
> Ok so the hang-up last time around iirc was that I broke igt by making
> a few things more synchronous. Let's hope I'm not also breaking stuff
> with the WARN_ON ...
>
> New plan:
> - make this patch here only document existing behaviour and enforce it
> with the WARN_ON
> - new uapi would be behind a flag or something, with userspace and
> everything hanging off it.
>
> Thoughts?

What do you mean by 'new uapi'? The proposal that the kernel
communicates back which object IDs have been added to the state behind
your back? That it's been made automatically blocking? Something else?

Cheers,
Daniel (the other one)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-09-22 13:36       ` Marius Vlad
  (?)
@ 2020-09-23 10:58         ` Daniel Vetter
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-23 10:58 UTC (permalink / raw)
  To: Marius Vlad
  Cc: Daniel Stone, Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> I've noticed this isn't integrated/added.
>
> Noticed this is fixing (also) DPMS when multiple outputs are in use.
> Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> the warning quite often.

On which driver/chip does this happen?
-Daniel

>
> >
> > Thanks for writing this up Daniel, and for reminding me about it some
> > time later as well ...
> >
> > Reviewed-by: Daniel Stone <daniels@collabora.com>
> >
> > Cheers,
> > Daniel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-23 10:58         ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-23 10:58 UTC (permalink / raw)
  To: Marius Vlad
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen

On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> I've noticed this isn't integrated/added.
>
> Noticed this is fixing (also) DPMS when multiple outputs are in use.
> Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> the warning quite often.

On which driver/chip does this happen?
-Daniel

>
> >
> > Thanks for writing this up Daniel, and for reminding me about it some
> > time later as well ...
> >
> > Reviewed-by: Daniel Stone <daniels@collabora.com>
> >
> > Cheers,
> > Daniel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-23 10:58         ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-23 10:58 UTC (permalink / raw)
  To: Marius Vlad
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen

On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> I've noticed this isn't integrated/added.
>
> Noticed this is fixing (also) DPMS when multiple outputs are in use.
> Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> the warning quite often.

On which driver/chip does this happen?
-Daniel

>
> >
> > Thanks for writing this up Daniel, and for reminding me about it some
> > time later as well ...
> >
> > Reviewed-by: Daniel Stone <daniels@collabora.com>
> >
> > Cheers,
> > Daniel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-09-23 10:58         ` Daniel Vetter
  (?)
@ 2020-09-23 11:14           ` Marius Vlad
  -1 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-23 11:14 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Stone, Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

[-- Attachment #1: Type: text/plain, Size: 2451 bytes --]

On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> >
> > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > I've noticed this isn't integrated/added.
> >
> > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > the warning quite often.
> 
> On which driver/chip does this happen?
I've tried it out on i915.
> -Daniel
> 
> >
> > >
> > > Thanks for writing this up Daniel, and for reminding me about it some
> > > time later as well ...
> > >
> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > >
> > > Cheers,
> > > Daniel
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-23 11:14           ` Marius Vlad
  0 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-23 11:14 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen


[-- Attachment #1.1: Type: text/plain, Size: 2451 bytes --]

On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> >
> > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > I've noticed this isn't integrated/added.
> >
> > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > the warning quite often.
> 
> On which driver/chip does this happen?
I've tried it out on i915.
> -Daniel
> 
> >
> > >
> > > Thanks for writing this up Daniel, and for reminding me about it some
> > > time later as well ...
> > >
> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > >
> > > Cheers,
> > > Daniel
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-23 11:14           ` Marius Vlad
  0 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-23 11:14 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen


[-- Attachment #1.1: Type: text/plain, Size: 2451 bytes --]

On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> >
> > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > I've noticed this isn't integrated/added.
> >
> > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > the warning quite often.
> 
> On which driver/chip does this happen?
I've tried it out on i915.
> -Daniel
> 
> >
> > >
> > > Thanks for writing this up Daniel, and for reminding me about it some
> > > time later as well ...
> > >
> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > >
> > > Cheers,
> > > Daniel
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-09-23 11:14           ` Marius Vlad
  (?)
@ 2020-09-23 11:16             ` Daniel Vetter
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-23 11:16 UTC (permalink / raw)
  To: Marius Vlad, Syrjala, Ville
  Cc: Daniel Stone, Pekka Paalanen, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter

On Wed, Sep 23, 2020 at 1:14 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > >
> > > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > I've noticed this isn't integrated/added.
> > >
> > > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > > the warning quite often.
> >
> > On which driver/chip does this happen?
> I've tried it out on i915.

lspci -nn please.

Also adding Ville, who has an idea where this can all go wrong. The
one he pointed out thus far is gen12+ only though.
-Daniel

> > -Daniel
> >
> > >
> > > >
> > > > Thanks for writing this up Daniel, and for reminding me about it some
> > > > time later as well ...
> > > >
> > > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > >
> > > > Cheers,
> > > > Daniel
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-23 11:16             ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-23 11:16 UTC (permalink / raw)
  To: Marius Vlad, Syrjala, Ville
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen

On Wed, Sep 23, 2020 at 1:14 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > >
> > > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > I've noticed this isn't integrated/added.
> > >
> > > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > > the warning quite often.
> >
> > On which driver/chip does this happen?
> I've tried it out on i915.

lspci -nn please.

Also adding Ville, who has an idea where this can all go wrong. The
one he pointed out thus far is gen12+ only though.
-Daniel

> > -Daniel
> >
> > >
> > > >
> > > > Thanks for writing this up Daniel, and for reminding me about it some
> > > > time later as well ...
> > > >
> > > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > >
> > > > Cheers,
> > > > Daniel
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-23 11:16             ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-09-23 11:16 UTC (permalink / raw)
  To: Marius Vlad, Syrjala, Ville
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen

On Wed, Sep 23, 2020 at 1:14 PM Marius Vlad <marius.vlad@collabora.com> wrote:
>
> On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > >
> > > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > I've noticed this isn't integrated/added.
> > >
> > > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > > the warning quite often.
> >
> > On which driver/chip does this happen?
> I've tried it out on i915.

lspci -nn please.

Also adding Ville, who has an idea where this can all go wrong. The
one he pointed out thus far is gen12+ only though.
-Daniel

> > -Daniel
> >
> > >
> > > >
> > > > Thanks for writing this up Daniel, and for reminding me about it some
> > > > time later as well ...
> > > >
> > > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > >
> > > > Cheers,
> > > > Daniel
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-09-23 11:16             ` Daniel Vetter
  (?)
@ 2020-09-23 11:31               ` Marius Vlad
  -1 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-23 11:31 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Syrjala, Ville, Daniel Stone, Pekka Paalanen,
	Intel Graphics Development, stable, DRI Development,
	Daniel Vetter

[-- Attachment #1: Type: text/plain, Size: 5278 bytes --]

On Wed, Sep 23, 2020 at 01:16:42PM +0200, Daniel Vetter wrote:
> On Wed, Sep 23, 2020 at 1:14 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> >
> > On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> > > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > > >
> > > > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > > I've noticed this isn't integrated/added.
> > > >
> > > > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > > > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > > > the warning quite often.
> > >
> > > On which driver/chip does this happen?
> > I've tried it out on i915.
> 
> lspci -nn please.
Sure,

$ lspci -nn
00:00.0 Host bridge [0600]: Intel Corporation Xeon E3-1200 v6/7th Gen Core Processor Host Bridge/DRAM Registers [8086:5914] (rev 08)
00:02.0 VGA compatible controller [0300]: Intel Corporation UHD Graphics 620 [8086:5917] (rev 07)
00:04.0 Signal processing controller [1180]: Intel Corporation Xeon E3-1200 v5/E3-1500 v5/6th Gen Core Processor Thermal Subsystem [8086:1903] (rev 08)
00:14.0 USB controller [0c03]: Intel Corporation Sunrise Point-LP USB 3.0 xHCI Controller [8086:9d2f] (rev 21)
00:14.2 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Thermal subsystem [8086:9d31] (rev 21)
00:15.0 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #0 [8086:9d60] (rev 21)
00:15.1 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #1 [8086:9d61] (rev 21)
00:16.0 Communication controller [0780]: Intel Corporation Sunrise Point-LP CSME HECI #1 [8086:9d3a] (rev 21)
00:1c.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #1 [8086:9d10] (rev f1)
00:1c.2 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #3 [8086:9d12] (rev f1)
00:1c.4 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #5 [8086:9d14] (rev f1)
00:1d.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #9 [8086:9d18] (rev f1)
00:1f.0 ISA bridge [0601]: Intel Corporation Sunrise Point LPC Controller/eSPI Controller [8086:9d4e] (rev 21)
00:1f.2 Memory controller [0580]: Intel Corporation Sunrise Point-LP PMC [8086:9d21] (rev 21)
00:1f.3 Audio device [0403]: Intel Corporation Sunrise Point-LP HD Audio [8086:9d71] (rev 21)
00:1f.4 SMBus [0c05]: Intel Corporation Sunrise Point-LP SMBus [8086:9d23] (rev 21)
01:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. RTS525A PCI Express Card Reader [10ec:525a] (rev 01)
02:00.0 Network controller [0280]: Qualcomm Atheros QCA6174 802.11ac Wireless Network Adapter [168c:003e] (rev 32)
6e:00.0 Non-Volatile memory controller [0108]: Toshiba Corporation Device [1179:0116]

(it's a xps laptop)

> 
> Also adding Ville, who has an idea where this can all go wrong. The
> one he pointed out thus far is gen12+ only though.
> -Daniel
> 
> > > -Daniel
> > >
> > > >
> > > > >
> > > > > Thanks for writing this up Daniel, and for reminding me about it some
> > > > > time later as well ...
> > > > >
> > > > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > > >
> > > > > Cheers,
> > > > > Daniel
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-23 11:31               ` Marius Vlad
  0 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-23 11:31 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen


[-- Attachment #1.1: Type: text/plain, Size: 5278 bytes --]

On Wed, Sep 23, 2020 at 01:16:42PM +0200, Daniel Vetter wrote:
> On Wed, Sep 23, 2020 at 1:14 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> >
> > On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> > > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > > >
> > > > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > > I've noticed this isn't integrated/added.
> > > >
> > > > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > > > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > > > the warning quite often.
> > >
> > > On which driver/chip does this happen?
> > I've tried it out on i915.
> 
> lspci -nn please.
Sure,

$ lspci -nn
00:00.0 Host bridge [0600]: Intel Corporation Xeon E3-1200 v6/7th Gen Core Processor Host Bridge/DRAM Registers [8086:5914] (rev 08)
00:02.0 VGA compatible controller [0300]: Intel Corporation UHD Graphics 620 [8086:5917] (rev 07)
00:04.0 Signal processing controller [1180]: Intel Corporation Xeon E3-1200 v5/E3-1500 v5/6th Gen Core Processor Thermal Subsystem [8086:1903] (rev 08)
00:14.0 USB controller [0c03]: Intel Corporation Sunrise Point-LP USB 3.0 xHCI Controller [8086:9d2f] (rev 21)
00:14.2 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Thermal subsystem [8086:9d31] (rev 21)
00:15.0 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #0 [8086:9d60] (rev 21)
00:15.1 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #1 [8086:9d61] (rev 21)
00:16.0 Communication controller [0780]: Intel Corporation Sunrise Point-LP CSME HECI #1 [8086:9d3a] (rev 21)
00:1c.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #1 [8086:9d10] (rev f1)
00:1c.2 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #3 [8086:9d12] (rev f1)
00:1c.4 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #5 [8086:9d14] (rev f1)
00:1d.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #9 [8086:9d18] (rev f1)
00:1f.0 ISA bridge [0601]: Intel Corporation Sunrise Point LPC Controller/eSPI Controller [8086:9d4e] (rev 21)
00:1f.2 Memory controller [0580]: Intel Corporation Sunrise Point-LP PMC [8086:9d21] (rev 21)
00:1f.3 Audio device [0403]: Intel Corporation Sunrise Point-LP HD Audio [8086:9d71] (rev 21)
00:1f.4 SMBus [0c05]: Intel Corporation Sunrise Point-LP SMBus [8086:9d23] (rev 21)
01:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. RTS525A PCI Express Card Reader [10ec:525a] (rev 01)
02:00.0 Network controller [0280]: Qualcomm Atheros QCA6174 802.11ac Wireless Network Adapter [168c:003e] (rev 32)
6e:00.0 Non-Volatile memory controller [0108]: Toshiba Corporation Device [1179:0116]

(it's a xps laptop)

> 
> Also adding Ville, who has an idea where this can all go wrong. The
> one he pointed out thus far is gen12+ only though.
> -Daniel
> 
> > > -Daniel
> > >
> > > >
> > > > >
> > > > > Thanks for writing this up Daniel, and for reminding me about it some
> > > > > time later as well ...
> > > > >
> > > > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > > >
> > > > > Cheers,
> > > > > Daniel
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-09-23 11:31               ` Marius Vlad
  0 siblings, 0 replies; 54+ messages in thread
From: Marius Vlad @ 2020-09-23 11:31 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, stable,
	Daniel Vetter, Pekka Paalanen


[-- Attachment #1.1: Type: text/plain, Size: 5278 bytes --]

On Wed, Sep 23, 2020 at 01:16:42PM +0200, Daniel Vetter wrote:
> On Wed, Sep 23, 2020 at 1:14 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> >
> > On Wed, Sep 23, 2020 at 12:58:30PM +0200, Daniel Vetter wrote:
> > > On Tue, Sep 22, 2020 at 3:36 PM Marius Vlad <marius.vlad@collabora.com> wrote:
> > > >
> > > > On Fri, Jan 31, 2020 at 07:34:00AM +0000, Daniel Stone wrote:
> > > > > On Thu, 5 Jul 2018 at 11:21, Daniel Vetter <daniel.vetter@ffwll.ch> 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.
> > > > Gentle ping. I've tried out Linus's master tree and, and like Pekka,
> > > > I've noticed this isn't integrated/added.
> > > >
> > > > Noticed this is fixing (also) DPMS when multiple outputs are in use.
> > > > Wondering if we can just use a _ONCE() variant instead of WARN_ON(). I'm seeing
> > > > the warning quite often.
> > >
> > > On which driver/chip does this happen?
> > I've tried it out on i915.
> 
> lspci -nn please.
Sure,

$ lspci -nn
00:00.0 Host bridge [0600]: Intel Corporation Xeon E3-1200 v6/7th Gen Core Processor Host Bridge/DRAM Registers [8086:5914] (rev 08)
00:02.0 VGA compatible controller [0300]: Intel Corporation UHD Graphics 620 [8086:5917] (rev 07)
00:04.0 Signal processing controller [1180]: Intel Corporation Xeon E3-1200 v5/E3-1500 v5/6th Gen Core Processor Thermal Subsystem [8086:1903] (rev 08)
00:14.0 USB controller [0c03]: Intel Corporation Sunrise Point-LP USB 3.0 xHCI Controller [8086:9d2f] (rev 21)
00:14.2 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Thermal subsystem [8086:9d31] (rev 21)
00:15.0 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #0 [8086:9d60] (rev 21)
00:15.1 Signal processing controller [1180]: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #1 [8086:9d61] (rev 21)
00:16.0 Communication controller [0780]: Intel Corporation Sunrise Point-LP CSME HECI #1 [8086:9d3a] (rev 21)
00:1c.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #1 [8086:9d10] (rev f1)
00:1c.2 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #3 [8086:9d12] (rev f1)
00:1c.4 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #5 [8086:9d14] (rev f1)
00:1d.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #9 [8086:9d18] (rev f1)
00:1f.0 ISA bridge [0601]: Intel Corporation Sunrise Point LPC Controller/eSPI Controller [8086:9d4e] (rev 21)
00:1f.2 Memory controller [0580]: Intel Corporation Sunrise Point-LP PMC [8086:9d21] (rev 21)
00:1f.3 Audio device [0403]: Intel Corporation Sunrise Point-LP HD Audio [8086:9d71] (rev 21)
00:1f.4 SMBus [0c05]: Intel Corporation Sunrise Point-LP SMBus [8086:9d23] (rev 21)
01:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. RTS525A PCI Express Card Reader [10ec:525a] (rev 01)
02:00.0 Network controller [0280]: Qualcomm Atheros QCA6174 802.11ac Wireless Network Adapter [168c:003e] (rev 32)
6e:00.0 Non-Volatile memory controller [0108]: Toshiba Corporation Device [1179:0116]

(it's a xps laptop)

> 
> Also adding Ville, who has an idea where this can all go wrong. The
> one he pointed out thus far is gen12+ only though.
> -Daniel
> 
> > > -Daniel
> > >
> > > >
> > > > >
> > > > > Thanks for writing this up Daniel, and for reminding me about it some
> > > > > time later as well ...
> > > > >
> > > > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > > >
> > > > > Cheers,
> > > > > Daniel
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-05-14  7:40           ` Daniel Stone
@ 2020-05-14 12:32             ` Daniel Vetter
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-05-14 12:32 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Daniel Vetter, DRI Development, Intel Graphics Development,
	Pekka Paalanen, stable, Daniel Stone, Ville Syrjälä,
	Daniel Vetter

On Thu, May 14, 2020 at 08:40:21AM +0100, Daniel Stone wrote:
> On Thu, 14 May 2020 at 08:25, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Thu, May 14, 2020 at 9:18 AM Daniel Stone <daniel@fooishbar.org> wrote:
> > > On Thu, 14 May 2020 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > I'd be very much in favour of putting the blocking down in the kernel
> > > at least until the kernel can give us a clear indication to tell us
> > > what's going on, and ideally which other resources need to be dragged
> > > in, in a way which is distinguishable from your compositor having
> > > broken synchronisation.
> >
> > We know, the patch already computes that ... So would be a matter of
> > exporting that to userspace. We have a mask of all additional crtc
> > that will get an event and will -EBUSY until that's done.
> 
> Yep, but unless and until that happens, could we please get this in?
> Given it would require uAPI changes, we'd need to modify all the
> compositors to work with the old path (random EBUSY) and the new path
> (predictable and obvious), so at least preserving the promise that
> per-CRTC updates are really independent would be good.

I haven't found the time to look at the intel-gfx-ci fail in igt nor
really think about that. Nor care enough to just hammer this ignoring ci,
since I didn't even get around to understand why the igt now fails If
someone else takes this over, happy to see it land.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-05-14 12:32             ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-05-14 12:32 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Daniel Stone, Daniel Vetter, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter, Pekka Paalanen

On Thu, May 14, 2020 at 08:40:21AM +0100, Daniel Stone wrote:
> On Thu, 14 May 2020 at 08:25, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Thu, May 14, 2020 at 9:18 AM Daniel Stone <daniel@fooishbar.org> wrote:
> > > On Thu, 14 May 2020 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > I'd be very much in favour of putting the blocking down in the kernel
> > > at least until the kernel can give us a clear indication to tell us
> > > what's going on, and ideally which other resources need to be dragged
> > > in, in a way which is distinguishable from your compositor having
> > > broken synchronisation.
> >
> > We know, the patch already computes that ... So would be a matter of
> > exporting that to userspace. We have a mask of all additional crtc
> > that will get an event and will -EBUSY until that's done.
> 
> Yep, but unless and until that happens, could we please get this in?
> Given it would require uAPI changes, we'd need to modify all the
> compositors to work with the old path (random EBUSY) and the new path
> (predictable and obvious), so at least preserving the promise that
> per-CRTC updates are really independent would be good.

I haven't found the time to look at the intel-gfx-ci fail in igt nor
really think about that. Nor care enough to just hammer this ignoring ci,
since I didn't even get around to understand why the igt now fails If
someone else takes this over, happy to see it land.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-05-14  7:25         ` Daniel Vetter
@ 2020-05-14  7:40           ` Daniel Stone
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-05-14  7:40 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Pekka Paalanen,
	stable, Daniel Stone, Ville Syrjälä,
	Daniel Vetter

On Thu, 14 May 2020 at 08:25, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, May 14, 2020 at 9:18 AM Daniel Stone <daniel@fooishbar.org> wrote:
> > On Thu, 14 May 2020 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > I'd be very much in favour of putting the blocking down in the kernel
> > at least until the kernel can give us a clear indication to tell us
> > what's going on, and ideally which other resources need to be dragged
> > in, in a way which is distinguishable from your compositor having
> > broken synchronisation.
>
> We know, the patch already computes that ... So would be a matter of
> exporting that to userspace. We have a mask of all additional crtc
> that will get an event and will -EBUSY until that's done.

Yep, but unless and until that happens, could we please get this in?
Given it would require uAPI changes, we'd need to modify all the
compositors to work with the old path (random EBUSY) and the new path
(predictable and obvious), so at least preserving the promise that
per-CRTC updates are really independent would be good.

Cheers,
Daniel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-05-14  7:40           ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-05-14  7:40 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Stone, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter, Pekka Paalanen

On Thu, 14 May 2020 at 08:25, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, May 14, 2020 at 9:18 AM Daniel Stone <daniel@fooishbar.org> wrote:
> > On Thu, 14 May 2020 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > I'd be very much in favour of putting the blocking down in the kernel
> > at least until the kernel can give us a clear indication to tell us
> > what's going on, and ideally which other resources need to be dragged
> > in, in a way which is distinguishable from your compositor having
> > broken synchronisation.
>
> We know, the patch already computes that ... So would be a matter of
> exporting that to userspace. We have a mask of all additional crtc
> that will get an event and will -EBUSY until that's done.

Yep, but unless and until that happens, could we please get this in?
Given it would require uAPI changes, we'd need to modify all the
compositors to work with the old path (random EBUSY) and the new path
(predictable and obvious), so at least preserving the promise that
per-CRTC updates are really independent would be good.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-05-14  7:16       ` Daniel Stone
@ 2020-05-14  7:25         ` Daniel Vetter
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-05-14  7:25 UTC (permalink / raw)
  To: Daniel Stone
  Cc: DRI Development, Intel Graphics Development, Pekka Paalanen,
	stable, Daniel Stone, Ville Syrjälä,
	Daniel Vetter

On Thu, May 14, 2020 at 9:18 AM Daniel Stone <daniel@fooishbar.org> wrote:
>
> Hi,
>
> On Thu, 14 May 2020 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > Did anything happen with this?
> >
> > Nope. There's an igt now that fails with this, and I'm not sure
> > whether changing the igt is the right idea or not.
> >
> > I'm kinda now thinking about changing this to instead document under
> > which exact situations you can get a spurious EBUSY, and enforcing
> > that in the code with some checks. Essentially only possible if you do
> > a ALLOW_MODESET | NONBLOCKING on the other crtc. And then tell
> > userspace you get to eat that. We've been shipping with this for so
> > long by now that's defacto the uapi anyway :-/
> >
> > Thoughts? Too horrible?
>
> I've been trying to avoid that, to be honest. Taking a random delay
> because the kernel needs to do global things is fine. But making
> userspace either do an expensive/complicated cross-CRTC
> synchronisation is less easy; for some compositors, that means
> reaching across threads to make sure all CRTCs are quiescent. Either
> that, or deferring your ALLOW_MODESET to somewhere else, like an idle
> handler, far away from where you were originally trying to do it,
> which wouldn't be pleasant. The other option is that we teach people
> to ignore EBUSY as random noise which can just sometimes happen and to
> try again (when? how often? and you still have cross-CRTC
> synchronisation issues), which doesn't scream compositor best practice
> to me.
>
> I'd be very much in favour of putting the blocking down in the kernel
> at least until the kernel can give us a clear indication to tell us
> what's going on, and ideally which other resources need to be dragged
> in, in a way which is distinguishable from your compositor having
> broken synchronisation.

We know, the patch already computes that ... So would be a matter of
exporting that to userspace. We have a mask of all additional crtc
that will get an event and will -EBUSY until that's done.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-05-14  7:25         ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-05-14  7:25 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Daniel Stone, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter, Pekka Paalanen

On Thu, May 14, 2020 at 9:18 AM Daniel Stone <daniel@fooishbar.org> wrote:
>
> Hi,
>
> On Thu, 14 May 2020 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > Did anything happen with this?
> >
> > Nope. There's an igt now that fails with this, and I'm not sure
> > whether changing the igt is the right idea or not.
> >
> > I'm kinda now thinking about changing this to instead document under
> > which exact situations you can get a spurious EBUSY, and enforcing
> > that in the code with some checks. Essentially only possible if you do
> > a ALLOW_MODESET | NONBLOCKING on the other crtc. And then tell
> > userspace you get to eat that. We've been shipping with this for so
> > long by now that's defacto the uapi anyway :-/
> >
> > Thoughts? Too horrible?
>
> I've been trying to avoid that, to be honest. Taking a random delay
> because the kernel needs to do global things is fine. But making
> userspace either do an expensive/complicated cross-CRTC
> synchronisation is less easy; for some compositors, that means
> reaching across threads to make sure all CRTCs are quiescent. Either
> that, or deferring your ALLOW_MODESET to somewhere else, like an idle
> handler, far away from where you were originally trying to do it,
> which wouldn't be pleasant. The other option is that we teach people
> to ignore EBUSY as random noise which can just sometimes happen and to
> try again (when? how often? and you still have cross-CRTC
> synchronisation issues), which doesn't scream compositor best practice
> to me.
>
> I'd be very much in favour of putting the blocking down in the kernel
> at least until the kernel can give us a clear indication to tell us
> what's going on, and ideally which other resources need to be dragged
> in, in a way which is distinguishable from your compositor having
> broken synchronisation.

We know, the patch already computes that ... So would be a matter of
exporting that to userspace. We have a mask of all additional crtc
that will get an event and will -EBUSY until that's done.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-05-14  7:08     ` Daniel Vetter
@ 2020-05-14  7:16       ` Daniel Stone
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-05-14  7:16 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Pekka Paalanen,
	stable, Daniel Stone, Ville Syrjälä,
	Daniel Vetter

Hi,

On Thu, 14 May 2020 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Did anything happen with this?
>
> Nope. There's an igt now that fails with this, and I'm not sure
> whether changing the igt is the right idea or not.
>
> I'm kinda now thinking about changing this to instead document under
> which exact situations you can get a spurious EBUSY, and enforcing
> that in the code with some checks. Essentially only possible if you do
> a ALLOW_MODESET | NONBLOCKING on the other crtc. And then tell
> userspace you get to eat that. We've been shipping with this for so
> long by now that's defacto the uapi anyway :-/
>
> Thoughts? Too horrible?

I've been trying to avoid that, to be honest. Taking a random delay
because the kernel needs to do global things is fine. But making
userspace either do an expensive/complicated cross-CRTC
synchronisation is less easy; for some compositors, that means
reaching across threads to make sure all CRTCs are quiescent. Either
that, or deferring your ALLOW_MODESET to somewhere else, like an idle
handler, far away from where you were originally trying to do it,
which wouldn't be pleasant. The other option is that we teach people
to ignore EBUSY as random noise which can just sometimes happen and to
try again (when? how often? and you still have cross-CRTC
synchronisation issues), which doesn't scream compositor best practice
to me.

I'd be very much in favour of putting the blocking down in the kernel
at least until the kernel can give us a clear indication to tell us
what's going on, and ideally which other resources need to be dragged
in, in a way which is distinguishable from your compositor having
broken synchronisation.

Cheers,
Daniel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-05-14  7:16       ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-05-14  7:16 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Stone, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter, Pekka Paalanen

Hi,

On Thu, 14 May 2020 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Did anything happen with this?
>
> Nope. There's an igt now that fails with this, and I'm not sure
> whether changing the igt is the right idea or not.
>
> I'm kinda now thinking about changing this to instead document under
> which exact situations you can get a spurious EBUSY, and enforcing
> that in the code with some checks. Essentially only possible if you do
> a ALLOW_MODESET | NONBLOCKING on the other crtc. And then tell
> userspace you get to eat that. We've been shipping with this for so
> long by now that's defacto the uapi anyway :-/
>
> Thoughts? Too horrible?

I've been trying to avoid that, to be honest. Taking a random delay
because the kernel needs to do global things is fine. But making
userspace either do an expensive/complicated cross-CRTC
synchronisation is less easy; for some compositors, that means
reaching across threads to make sure all CRTCs are quiescent. Either
that, or deferring your ALLOW_MODESET to somewhere else, like an idle
handler, far away from where you were originally trying to do it,
which wouldn't be pleasant. The other option is that we teach people
to ignore EBUSY as random noise which can just sometimes happen and to
try again (when? how often? and you still have cross-CRTC
synchronisation issues), which doesn't scream compositor best practice
to me.

I'd be very much in favour of putting the blocking down in the kernel
at least until the kernel can give us a clear indication to tell us
what's going on, and ideally which other resources need to be dragged
in, in a way which is distinguishable from your compositor having
broken synchronisation.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-05-14  6:40   ` Daniel Stone
@ 2020-05-14  7:08     ` Daniel Vetter
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-05-14  7:08 UTC (permalink / raw)
  To: Daniel Stone
  Cc: DRI Development, Intel Graphics Development, Pekka Paalanen,
	stable, Daniel Stone, Ville Syrjälä,
	Daniel Vetter

On Thu, May 14, 2020 at 8:42 AM Daniel Stone <daniel@fooishbar.org> wrote:
>
> On Wed, 8 Apr 2020 at 17:24, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Resending because last attempt failed CI and meanwhile the results are
> > lost :-/
>
> Did anything happen with this?

Nope. There's an igt now that fails with this, and I'm not sure
whether changing the igt is the right idea or not.

I'm kinda now thinking about changing this to instead document under
which exact situations you can get a spurious EBUSY, and enforcing
that in the code with some checks. Essentially only possible if you do
a ALLOW_MODESET | NONBLOCKING on the other crtc. And then tell
userspace you get to eat that. We've been shipping with this for so
long by now that's defacto the uapi anyway :-/

Thoughts? Too horrible?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-05-14  7:08     ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-05-14  7:08 UTC (permalink / raw)
  To: Daniel Stone
  Cc: Daniel Stone, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter, Pekka Paalanen

On Thu, May 14, 2020 at 8:42 AM Daniel Stone <daniel@fooishbar.org> wrote:
>
> On Wed, 8 Apr 2020 at 17:24, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Resending because last attempt failed CI and meanwhile the results are
> > lost :-/
>
> Did anything happen with this?

Nope. There's an igt now that fails with this, and I'm not sure
whether changing the igt is the right idea or not.

I'm kinda now thinking about changing this to instead document under
which exact situations you can get a spurious EBUSY, and enforcing
that in the code with some checks. Essentially only possible if you do
a ALLOW_MODESET | NONBLOCKING on the other crtc. And then tell
userspace you get to eat that. We've been shipping with this for so
long by now that's defacto the uapi anyway :-/

Thoughts? Too horrible?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
  2020-04-08 16:24 ` Daniel Vetter
@ 2020-05-14  6:40   ` Daniel Stone
  -1 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-05-14  6:40 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Pekka Paalanen,
	stable, Daniel Stone, Ville Syrjälä,
	Daniel Vetter

On Wed, 8 Apr 2020 at 17:24, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Resending because last attempt failed CI and meanwhile the results are
> lost :-/

Did anything happen with this?

Cheers,
Daniel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-05-14  6:40   ` Daniel Stone
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Stone @ 2020-05-14  6:40 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Stone, Intel Graphics Development, stable,
	DRI Development, Daniel Vetter, Pekka Paalanen

On Wed, 8 Apr 2020 at 17:24, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Resending because last attempt failed CI and meanwhile the results are
> lost :-/

Did anything happen with this?

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-04-08 16:24 ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-04-08 16:24 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Stone,
	Pekka Paalanen, stable, Daniel Stone, Ville Syrjälä,
	Daniel Vetter

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 <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: stable@vger.kernel.org
Reviewed-by: Daniel Stone <daniels@collabora.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
--
Resending because last attempt failed CI and meanwhile the results are
lost :-/
-Daniel
---
 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 965173fd0ac2..4f140ff6fb98 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);
+
+		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.25.1


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-04-08 16:24 ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-04-08 16:24 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Stone, Daniel Vetter, Intel Graphics Development, stable,
	Daniel Vetter, Pekka Paalanen

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 <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: stable@vger.kernel.org
Reviewed-by: Daniel Stone <daniels@collabora.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
--
Resending because last attempt failed CI and meanwhile the results are
lost :-/
-Daniel
---
 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 965173fd0ac2..4f140ff6fb98 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);
+
+		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.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-02-25 11:50 ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-02-25 11:50 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Stone,
	Pekka Paalanen, stable, Daniel Stone, Daniel Vetter

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 <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: stable@vger.kernel.org
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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);
+
+		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.24.1


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets
@ 2020-02-25 11:50 ` Daniel Vetter
  0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2020-02-25 11:50 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Stone, Daniel Vetter, Intel Graphics Development, stable,
	Daniel Vetter, Pekka Paalanen

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 <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: stable@vger.kernel.org
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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);
+
+		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.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2020-09-23 11:32 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-05 10:10 [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets Daniel Vetter
2018-07-05 10:10 ` Daniel Vetter
2018-07-05 10:21 ` Daniel Vetter
2020-01-31  7:34   ` Daniel Stone
2020-01-31  7:34     ` [Intel-gfx] " Daniel Stone
2020-01-31  7:34     ` Daniel Stone
2020-09-22 13:36     ` Marius Vlad
2020-09-22 13:36       ` [Intel-gfx] " Marius Vlad
2020-09-22 13:36       ` Marius Vlad
2020-09-22 14:04       ` Daniel Vetter
2020-09-22 14:04         ` [Intel-gfx] " Daniel Vetter
2020-09-22 14:04         ` Daniel Vetter
2020-09-22 14:14         ` Daniel Stone
2020-09-22 14:14           ` [Intel-gfx] " Daniel Stone
2020-09-22 14:14           ` Daniel Stone
2020-09-22 16:01           ` Daniel Vetter
2020-09-22 16:01             ` [Intel-gfx] " Daniel Vetter
2020-09-22 16:01             ` Daniel Vetter
2020-09-22 19:02             ` Daniel Stone
2020-09-22 19:02               ` [Intel-gfx] " Daniel Stone
2020-09-22 19:02               ` Daniel Stone
2020-09-23 10:58       ` Daniel Vetter
2020-09-23 10:58         ` [Intel-gfx] " Daniel Vetter
2020-09-23 10:58         ` Daniel Vetter
2020-09-23 11:14         ` Marius Vlad
2020-09-23 11:14           ` [Intel-gfx] " Marius Vlad
2020-09-23 11:14           ` Marius Vlad
2020-09-23 11:16           ` Daniel Vetter
2020-09-23 11:16             ` [Intel-gfx] " Daniel Vetter
2020-09-23 11:16             ` Daniel Vetter
2020-09-23 11:31             ` Marius Vlad
2020-09-23 11:31               ` [Intel-gfx] " Marius Vlad
2020-09-23 11:31               ` Marius Vlad
2018-07-05 10:25 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-07-05 10:41 ` ✓ Fi.CI.BAT: success " Patchwork
2018-07-05 10:45 ` ✗ Fi.CI.CHECKPATCH: warning for drm: avoid spurious EBUSY due to nonblocking atomic modesets (rev2) Patchwork
2018-07-05 11:01 ` ✓ Fi.CI.BAT: success " Patchwork
2018-07-05 12:23 ` ✗ Fi.CI.IGT: failure " Patchwork
2020-02-25 11:50 [PATCH] drm: avoid spurious EBUSY due to nonblocking atomic modesets Daniel Vetter
2020-02-25 11:50 ` Daniel Vetter
2020-04-08 16:24 Daniel Vetter
2020-04-08 16:24 ` Daniel Vetter
2020-05-14  6:40 ` Daniel Stone
2020-05-14  6:40   ` Daniel Stone
2020-05-14  7:08   ` Daniel Vetter
2020-05-14  7:08     ` Daniel Vetter
2020-05-14  7:16     ` Daniel Stone
2020-05-14  7:16       ` Daniel Stone
2020-05-14  7:25       ` Daniel Vetter
2020-05-14  7:25         ` Daniel Vetter
2020-05-14  7:40         ` Daniel Stone
2020-05-14  7:40           ` Daniel Stone
2020-05-14 12:32           ` Daniel Vetter
2020-05-14 12:32             ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.