All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] drm: Make modeset locking easier
@ 2021-07-15 18:49 ` Ville Syrjala
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While staring at some DRM_MODESET_LOCK_ALL_{BEGIN,END}() conversions
I decided I don't really like what those macros do.

The main problems that I see:
- the magic goto inside limits their usefulness to baically
  a single statement, unless you're willing to look inside and
  find out the name of the magic label
- can't help at all in the "we don't want to lock everything"
  cases, which are quite numerous
- not at all obvious that there's a loop in there

So I figured I'd try to come up with something more universally useful.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>

Ville Syrjälä (4):
  drm: Introduce drm_modeset_lock_ctx_retry()
  drm: Introduce drm_modeset_lock_all_ctx_retry()
  drm/i915: Extract intel_crtc_initial_commit()
  drm/i915: Use drm_modeset_lock_ctx_retry() & co.

 drivers/gpu/drm/drm_modeset_lock.c            |  44 ++++
 drivers/gpu/drm/i915/display/g4x_dp.c         |  17 +-
 drivers/gpu/drm/i915/display/intel_audio.c    |  17 +-
 drivers/gpu/drm/i915/display/intel_ddi.c      |  16 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 192 ++++++++----------
 .../drm/i915/display/intel_display_debugfs.c  |  45 ++--
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  38 ++--
 include/drm/drm_modeset_lock.h                |  26 +++
 8 files changed, 188 insertions(+), 207 deletions(-)

-- 
2.31.1


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

* [Intel-gfx] [PATCH 0/4] drm: Make modeset locking easier
@ 2021-07-15 18:49 ` Ville Syrjala
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While staring at some DRM_MODESET_LOCK_ALL_{BEGIN,END}() conversions
I decided I don't really like what those macros do.

The main problems that I see:
- the magic goto inside limits their usefulness to baically
  a single statement, unless you're willing to look inside and
  find out the name of the magic label
- can't help at all in the "we don't want to lock everything"
  cases, which are quite numerous
- not at all obvious that there's a loop in there

So I figured I'd try to come up with something more universally useful.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>

Ville Syrjälä (4):
  drm: Introduce drm_modeset_lock_ctx_retry()
  drm: Introduce drm_modeset_lock_all_ctx_retry()
  drm/i915: Extract intel_crtc_initial_commit()
  drm/i915: Use drm_modeset_lock_ctx_retry() & co.

 drivers/gpu/drm/drm_modeset_lock.c            |  44 ++++
 drivers/gpu/drm/i915/display/g4x_dp.c         |  17 +-
 drivers/gpu/drm/i915/display/intel_audio.c    |  17 +-
 drivers/gpu/drm/i915/display/intel_ddi.c      |  16 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 192 ++++++++----------
 .../drm/i915/display/intel_display_debugfs.c  |  45 ++--
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  38 ++--
 include/drm/drm_modeset_lock.h                |  26 +++
 8 files changed, 188 insertions(+), 207 deletions(-)

-- 
2.31.1

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

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

* [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
@ 2021-07-15 18:49   ` Ville Syrjala
  -1 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Quite a few places are hand rolling the modeset lock backoff dance.
Let's suck that into a helper macro that is easier to use without
forgetting some steps.

The main downside is probably that the implementation of
drm_with_modeset_lock_ctx() is a bit harder to read than a hand
rolled version on account of being split across three functions,
but the actual code using it ends up being much simpler.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
 include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index fcfe1a03c4a1..083df96632e8 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
 	return 0;
 }
 EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
+
+void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
+			     struct drm_atomic_state *state,
+			     unsigned int flags, int *ret)
+{
+	drm_modeset_acquire_init(ctx, flags);
+
+	if (state)
+		state->acquire_ctx = ctx;
+
+	*ret = -EDEADLK;
+}
+EXPORT_SYMBOL(_drm_modeset_lock_begin);
+
+bool _drm_modeset_lock_loop(int *ret)
+{
+	if (*ret == -EDEADLK) {
+		*ret = 0;
+		return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(_drm_modeset_lock_loop);
+
+void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
+			   struct drm_atomic_state *state,
+			   int *ret)
+{
+	if (*ret == -EDEADLK) {
+		if (state)
+			drm_atomic_state_clear(state);
+
+		*ret = drm_modeset_backoff(ctx);
+		if (*ret == 0) {
+			*ret = -EDEADLK;
+			return;
+		}
+	}
+
+	drm_modeset_drop_locks(ctx);
+	drm_modeset_acquire_fini(ctx);
+}
+EXPORT_SYMBOL(_drm_modeset_lock_end);
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index aafd07388eb7..5eaad2533de5 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -26,6 +26,7 @@
 
 #include <linux/ww_mutex.h>
 
+struct drm_atomic_state;
 struct drm_modeset_lock;
 
 /**
@@ -203,4 +204,23 @@ modeset_lock_fail:							\
 	if (!drm_drv_uses_atomic_modeset(dev))				\
 		mutex_unlock(&dev->mode_config.mutex);
 
+void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
+			     struct drm_atomic_state *state,
+			     unsigned int flags,
+			     int *ret);
+bool _drm_modeset_lock_loop(int *ret);
+void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
+			   struct drm_atomic_state *state,
+			   int *ret);
+
+/*
+ * Note that one must always use "continue" rather than
+ * "break" or "return" to handle errors within the
+ * drm_modeset_lock_ctx_retry() block.
+ */
+#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret)))
+
 #endif /* DRM_MODESET_LOCK_H_ */
-- 
2.31.1


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

* [Intel-gfx] [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
@ 2021-07-15 18:49   ` Ville Syrjala
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Quite a few places are hand rolling the modeset lock backoff dance.
Let's suck that into a helper macro that is easier to use without
forgetting some steps.

The main downside is probably that the implementation of
drm_with_modeset_lock_ctx() is a bit harder to read than a hand
rolled version on account of being split across three functions,
but the actual code using it ends up being much simpler.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
 include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index fcfe1a03c4a1..083df96632e8 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
 	return 0;
 }
 EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
+
+void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
+			     struct drm_atomic_state *state,
+			     unsigned int flags, int *ret)
+{
+	drm_modeset_acquire_init(ctx, flags);
+
+	if (state)
+		state->acquire_ctx = ctx;
+
+	*ret = -EDEADLK;
+}
+EXPORT_SYMBOL(_drm_modeset_lock_begin);
+
+bool _drm_modeset_lock_loop(int *ret)
+{
+	if (*ret == -EDEADLK) {
+		*ret = 0;
+		return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(_drm_modeset_lock_loop);
+
+void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
+			   struct drm_atomic_state *state,
+			   int *ret)
+{
+	if (*ret == -EDEADLK) {
+		if (state)
+			drm_atomic_state_clear(state);
+
+		*ret = drm_modeset_backoff(ctx);
+		if (*ret == 0) {
+			*ret = -EDEADLK;
+			return;
+		}
+	}
+
+	drm_modeset_drop_locks(ctx);
+	drm_modeset_acquire_fini(ctx);
+}
+EXPORT_SYMBOL(_drm_modeset_lock_end);
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index aafd07388eb7..5eaad2533de5 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -26,6 +26,7 @@
 
 #include <linux/ww_mutex.h>
 
+struct drm_atomic_state;
 struct drm_modeset_lock;
 
 /**
@@ -203,4 +204,23 @@ modeset_lock_fail:							\
 	if (!drm_drv_uses_atomic_modeset(dev))				\
 		mutex_unlock(&dev->mode_config.mutex);
 
+void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
+			     struct drm_atomic_state *state,
+			     unsigned int flags,
+			     int *ret);
+bool _drm_modeset_lock_loop(int *ret);
+void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
+			   struct drm_atomic_state *state,
+			   int *ret);
+
+/*
+ * Note that one must always use "continue" rather than
+ * "break" or "return" to handle errors within the
+ * drm_modeset_lock_ctx_retry() block.
+ */
+#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret)))
+
 #endif /* DRM_MODESET_LOCK_H_ */
-- 
2.31.1

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

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

* [PATCH 2/4] drm: Introduce drm_modeset_lock_all_ctx_retry()
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
@ 2021-07-15 18:49   ` Ville Syrjala
  -1 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Layer drm_modeset_lock_all_ctx_retry() on top of
drm_modeset_lock_ctx_retry() to make the fairly common
"let's lock everything" pattern nicer.

Currently we have DRM_MODESET_LOCK_ALL_{BEGIN,END}() for this
but I don't really like it due to the magic gotos within,
which makes it hard to use if you want to do multiple steps
between the BEGING/END. One would either have to know the
name of the magic label, or always wrap the whole thing into
a function so only the single call appears between the BEGIN/END.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drm_modeset_lock.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 5eaad2533de5..2e2548680aaa 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -223,4 +223,10 @@ void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
 	     _drm_modeset_lock_loop(&(ret)); \
 	     _drm_modeset_lock_end((ctx), (state), &(ret)))
 
+#define drm_modeset_lock_all_ctx_retry(dev, ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret))) \
+		for_each_if(((ret) = drm_modeset_lock_all_ctx((dev), (ctx))) == 0)
+
 #endif /* DRM_MODESET_LOCK_H_ */
-- 
2.31.1


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

* [Intel-gfx] [PATCH 2/4] drm: Introduce drm_modeset_lock_all_ctx_retry()
@ 2021-07-15 18:49   ` Ville Syrjala
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Layer drm_modeset_lock_all_ctx_retry() on top of
drm_modeset_lock_ctx_retry() to make the fairly common
"let's lock everything" pattern nicer.

Currently we have DRM_MODESET_LOCK_ALL_{BEGIN,END}() for this
but I don't really like it due to the magic gotos within,
which makes it hard to use if you want to do multiple steps
between the BEGING/END. One would either have to know the
name of the magic label, or always wrap the whole thing into
a function so only the single call appears between the BEGIN/END.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drm_modeset_lock.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 5eaad2533de5..2e2548680aaa 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -223,4 +223,10 @@ void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
 	     _drm_modeset_lock_loop(&(ret)); \
 	     _drm_modeset_lock_end((ctx), (state), &(ret)))
 
+#define drm_modeset_lock_all_ctx_retry(dev, ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret))) \
+		for_each_if(((ret) = drm_modeset_lock_all_ctx((dev), (ctx))) == 0)
+
 #endif /* DRM_MODESET_LOCK_H_ */
-- 
2.31.1

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

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

* [PATCH 3/4] drm/i915: Extract intel_crtc_initial_commit()
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
@ 2021-07-15 18:49   ` Ville Syrjala
  -1 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract intel_crtc_initial_commit() from intel_initial_commit().
Should make subsequent changes a bit less convoluted.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 96 +++++++++++---------
 1 file changed, 52 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 65ddb6ca16e6..3718399c4c2f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12129,12 +12129,60 @@ static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv)
 	drm_dbg(&dev_priv->drm, "FDI PLL freq=%d\n", dev_priv->fdi_pll_freq);
 }
 
+static int intel_crtc_initial_commit(struct intel_atomic_state *state,
+				     struct intel_crtc *crtc)
+{
+	struct intel_crtc_state *crtc_state;
+	struct intel_encoder *encoder;
+	int ret;
+
+	crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	if (!crtc_state->hw.active)
+		return 0;
+
+	/*
+	 * We've not yet detected sink capabilities
+	 * (audio,infoframes,etc.) and thus we don't want to
+	 * force a full state recomputation yet. We want that to
+	 * happen only for the first real commit from userspace.
+	 * So preserve the inherited flag for the time being.
+	 */
+	crtc_state->inherited = true;
+
+	ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
+	if (ret)
+		return ret;
+
+	/*
+	 * FIXME hack to force a LUT update to avoid the
+	 * plane update forcing the pipe gamma on without
+	 * having a proper LUT loaded. Remove once we
+	 * have readout for pipe gamma enable.
+	 */
+	crtc_state->uapi.color_mgmt_changed = true;
+
+	for_each_intel_encoder_mask(state->base.dev, encoder, crtc_state->uapi.encoder_mask) {
+		if (encoder->initial_fastset_check &&
+		    !encoder->initial_fastset_check(encoder, crtc_state)) {
+			ret = drm_atomic_add_affected_connectors(&state->base,
+								 &crtc->base);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int intel_initial_commit(struct drm_device *dev)
 {
-	struct drm_atomic_state *state = NULL;
 	struct drm_modeset_acquire_ctx ctx;
+	struct drm_atomic_state *state;
 	struct intel_crtc *crtc;
-	int ret = 0;
+	int ret;
 
 	state = drm_atomic_state_alloc(dev);
 	if (!state)
@@ -12146,49 +12194,9 @@ static int intel_initial_commit(struct drm_device *dev)
 	state->acquire_ctx = &ctx;
 
 	for_each_intel_crtc(dev, crtc) {
-		struct intel_crtc_state *crtc_state =
-			intel_atomic_get_crtc_state(state, crtc);
-
-		if (IS_ERR(crtc_state)) {
-			ret = PTR_ERR(crtc_state);
+		ret = intel_crtc_initial_commit(to_intel_atomic_state(state), crtc);
+		if (ret)
 			goto out;
-		}
-
-		if (crtc_state->hw.active) {
-			struct intel_encoder *encoder;
-
-			/*
-			 * We've not yet detected sink capabilities
-			 * (audio,infoframes,etc.) and thus we don't want to
-			 * force a full state recomputation yet. We want that to
-			 * happen only for the first real commit from userspace.
-			 * So preserve the inherited flag for the time being.
-			 */
-			crtc_state->inherited = true;
-
-			ret = drm_atomic_add_affected_planes(state, &crtc->base);
-			if (ret)
-				goto out;
-
-			/*
-			 * FIXME hack to force a LUT update to avoid the
-			 * plane update forcing the pipe gamma on without
-			 * having a proper LUT loaded. Remove once we
-			 * have readout for pipe gamma enable.
-			 */
-			crtc_state->uapi.color_mgmt_changed = true;
-
-			for_each_intel_encoder_mask(dev, encoder,
-						    crtc_state->uapi.encoder_mask) {
-				if (encoder->initial_fastset_check &&
-				    !encoder->initial_fastset_check(encoder, crtc_state)) {
-					ret = drm_atomic_add_affected_connectors(state,
-										 &crtc->base);
-					if (ret)
-						goto out;
-				}
-			}
-		}
 	}
 
 	ret = drm_atomic_commit(state);
-- 
2.31.1


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

* [Intel-gfx] [PATCH 3/4] drm/i915: Extract intel_crtc_initial_commit()
@ 2021-07-15 18:49   ` Ville Syrjala
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract intel_crtc_initial_commit() from intel_initial_commit().
Should make subsequent changes a bit less convoluted.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 96 +++++++++++---------
 1 file changed, 52 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 65ddb6ca16e6..3718399c4c2f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12129,12 +12129,60 @@ static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv)
 	drm_dbg(&dev_priv->drm, "FDI PLL freq=%d\n", dev_priv->fdi_pll_freq);
 }
 
+static int intel_crtc_initial_commit(struct intel_atomic_state *state,
+				     struct intel_crtc *crtc)
+{
+	struct intel_crtc_state *crtc_state;
+	struct intel_encoder *encoder;
+	int ret;
+
+	crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	if (!crtc_state->hw.active)
+		return 0;
+
+	/*
+	 * We've not yet detected sink capabilities
+	 * (audio,infoframes,etc.) and thus we don't want to
+	 * force a full state recomputation yet. We want that to
+	 * happen only for the first real commit from userspace.
+	 * So preserve the inherited flag for the time being.
+	 */
+	crtc_state->inherited = true;
+
+	ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
+	if (ret)
+		return ret;
+
+	/*
+	 * FIXME hack to force a LUT update to avoid the
+	 * plane update forcing the pipe gamma on without
+	 * having a proper LUT loaded. Remove once we
+	 * have readout for pipe gamma enable.
+	 */
+	crtc_state->uapi.color_mgmt_changed = true;
+
+	for_each_intel_encoder_mask(state->base.dev, encoder, crtc_state->uapi.encoder_mask) {
+		if (encoder->initial_fastset_check &&
+		    !encoder->initial_fastset_check(encoder, crtc_state)) {
+			ret = drm_atomic_add_affected_connectors(&state->base,
+								 &crtc->base);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int intel_initial_commit(struct drm_device *dev)
 {
-	struct drm_atomic_state *state = NULL;
 	struct drm_modeset_acquire_ctx ctx;
+	struct drm_atomic_state *state;
 	struct intel_crtc *crtc;
-	int ret = 0;
+	int ret;
 
 	state = drm_atomic_state_alloc(dev);
 	if (!state)
@@ -12146,49 +12194,9 @@ static int intel_initial_commit(struct drm_device *dev)
 	state->acquire_ctx = &ctx;
 
 	for_each_intel_crtc(dev, crtc) {
-		struct intel_crtc_state *crtc_state =
-			intel_atomic_get_crtc_state(state, crtc);
-
-		if (IS_ERR(crtc_state)) {
-			ret = PTR_ERR(crtc_state);
+		ret = intel_crtc_initial_commit(to_intel_atomic_state(state), crtc);
+		if (ret)
 			goto out;
-		}
-
-		if (crtc_state->hw.active) {
-			struct intel_encoder *encoder;
-
-			/*
-			 * We've not yet detected sink capabilities
-			 * (audio,infoframes,etc.) and thus we don't want to
-			 * force a full state recomputation yet. We want that to
-			 * happen only for the first real commit from userspace.
-			 * So preserve the inherited flag for the time being.
-			 */
-			crtc_state->inherited = true;
-
-			ret = drm_atomic_add_affected_planes(state, &crtc->base);
-			if (ret)
-				goto out;
-
-			/*
-			 * FIXME hack to force a LUT update to avoid the
-			 * plane update forcing the pipe gamma on without
-			 * having a proper LUT loaded. Remove once we
-			 * have readout for pipe gamma enable.
-			 */
-			crtc_state->uapi.color_mgmt_changed = true;
-
-			for_each_intel_encoder_mask(dev, encoder,
-						    crtc_state->uapi.encoder_mask) {
-				if (encoder->initial_fastset_check &&
-				    !encoder->initial_fastset_check(encoder, crtc_state)) {
-					ret = drm_atomic_add_affected_connectors(state,
-										 &crtc->base);
-					if (ret)
-						goto out;
-				}
-			}
-		}
 	}
 
 	ret = drm_atomic_commit(state);
-- 
2.31.1

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

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

* [PATCH 4/4] drm/i915: Use drm_modeset_lock_ctx_retry() & co.
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
@ 2021-07-15 18:49   ` Ville Syrjala
  -1 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We have the modeset lock dance hand rolled in quite a few places.
Use drm_modeset_lock_ctx_retry() and drm_modeset_lock_all_ctx_retry()
to simplify our lives.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/g4x_dp.c         |  17 +--
 drivers/gpu/drm/i915/display/intel_audio.c    |  17 +--
 drivers/gpu/drm/i915/display/intel_ddi.c      |  16 +--
 drivers/gpu/drm/i915/display/intel_display.c  | 102 ++++++------------
 .../drm/i915/display/intel_display_debugfs.c  |  45 +++-----
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  38 +++----
 6 files changed, 69 insertions(+), 166 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
index de0f358184aa..68e7f4233fa9 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -1167,23 +1167,10 @@ intel_dp_hotplug(struct intel_encoder *encoder,
 
 	state = intel_encoder_hotplug(encoder, connector);
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	for (;;) {
+	drm_modeset_lock_ctx_retry(&ctx, NULL, 0, ret)
 		ret = intel_dp_retrain_link(encoder, &ctx);
 
-		if (ret == -EDEADLK) {
-			drm_modeset_backoff(&ctx);
-			continue;
-		}
-
-		break;
-	}
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-	drm_WARN(encoder->base.dev, ret,
-		 "Acquiring modeset locks failed with %i\n", ret);
+	drm_WARN_ON(encoder->base.dev, ret);
 
 	/*
 	 * Keeping it consistent with intel_ddi_hotplug() and
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 5f4f316b3ab5..a3b6b00e6633 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -969,28 +969,17 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
 	if (!crtc)
 		return;
 
-	drm_modeset_acquire_init(&ctx, 0);
 	state = drm_atomic_state_alloc(&dev_priv->drm);
 	if (drm_WARN_ON(&dev_priv->drm, !state))
 		return;
 
-	state->acquire_ctx = &ctx;
-
-retry:
-	ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state), crtc,
-					   enable);
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(&ctx);
-		goto retry;
-	}
+	drm_modeset_lock_ctx_retry(&ctx, state, 0, ret)
+		ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state),
+						   crtc, enable);
 
 	drm_WARN_ON(&dev_priv->drm, ret);
 
 	drm_atomic_state_put(state);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
 }
 
 static unsigned long i915_audio_component_get_power(struct device *kdev)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 26a3aa73fcc4..9aa7369d8dbe 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4210,26 +4210,14 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
 
 	state = intel_encoder_hotplug(encoder, connector);
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	for (;;) {
+	drm_modeset_lock_ctx_retry(&ctx, NULL, 0, ret) {
 		if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
 			ret = intel_hdmi_reset_link(encoder, &ctx);
 		else
 			ret = intel_dp_retrain_link(encoder, &ctx);
-
-		if (ret == -EDEADLK) {
-			drm_modeset_backoff(&ctx);
-			continue;
-		}
-
-		break;
 	}
 
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-	drm_WARN(encoder->base.dev, ret,
-		 "Acquiring modeset locks failed with %i\n", ret);
+	drm_WARN_ON(encoder->base.dev, ret);
 
 	/*
 	 * Unpowered type-c dongles can take some time to boot and be
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3718399c4c2f..6f5bc56d68e0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12057,40 +12057,30 @@ static void sanitize_watermarks(struct drm_i915_private *dev_priv)
 
 	intel_state = to_intel_atomic_state(state);
 
-	drm_modeset_acquire_init(&ctx, 0);
+	drm_modeset_lock_ctx_retry(&ctx, state, 0, ret) {
+		/*
+		 * Hardware readout is the only time we don't want to calculate
+		 * intermediate watermarks (since we don't trust the current
+		 * watermarks).
+		 */
+		if (!HAS_GMCH(dev_priv))
+			intel_state->skip_intermediate_wm = true;
 
-retry:
-	state->acquire_ctx = &ctx;
+		ret = sanitize_watermarks_add_affected(state);
+		if (ret)
+			continue;
 
-	/*
-	 * Hardware readout is the only time we don't want to calculate
-	 * intermediate watermarks (since we don't trust the current
-	 * watermarks).
-	 */
-	if (!HAS_GMCH(dev_priv))
-		intel_state->skip_intermediate_wm = true;
+		ret = intel_atomic_check(&dev_priv->drm, state);
+		if (ret)
+			continue;
 
-	ret = sanitize_watermarks_add_affected(state);
-	if (ret)
-		goto fail;
+		/* Write calculated watermark values back */
+		for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i) {
+			crtc_state->wm.need_postvbl_update = true;
+			dev_priv->display.optimize_watermarks(intel_state, crtc);
 
-	ret = intel_atomic_check(&dev_priv->drm, state);
-	if (ret)
-		goto fail;
-
-	/* Write calculated watermark values back */
-	for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i) {
-		crtc_state->wm.need_postvbl_update = true;
-		dev_priv->display.optimize_watermarks(intel_state, crtc);
-
-		to_intel_crtc_state(crtc->base.state)->wm = crtc_state->wm;
-	}
-
-fail:
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(&ctx);
-		goto retry;
+			to_intel_crtc_state(crtc->base.state)->wm = crtc_state->wm;
+		}
 	}
 
 	/*
@@ -12108,9 +12098,6 @@ static void sanitize_watermarks(struct drm_i915_private *dev_priv)
 		 "Could not determine valid watermarks for inherited state\n");
 
 	drm_atomic_state_put(state);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
 }
 
 static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv)
@@ -12181,38 +12168,28 @@ static int intel_initial_commit(struct drm_device *dev)
 {
 	struct drm_modeset_acquire_ctx ctx;
 	struct drm_atomic_state *state;
-	struct intel_crtc *crtc;
 	int ret;
 
 	state = drm_atomic_state_alloc(dev);
 	if (!state)
 		return -ENOMEM;
 
-	drm_modeset_acquire_init(&ctx, 0);
+	drm_modeset_lock_ctx_retry(&ctx, state, 0, ret) {
+		struct intel_crtc *crtc;
 
-retry:
-	state->acquire_ctx = &ctx;
-
-	for_each_intel_crtc(dev, crtc) {
-		ret = intel_crtc_initial_commit(to_intel_atomic_state(state), crtc);
+		for_each_intel_crtc(dev, crtc) {
+			ret = intel_crtc_initial_commit(to_intel_atomic_state(state), crtc);
+			if (ret)
+				break;
+		}
 		if (ret)
-			goto out;
-	}
+			continue;
 
-	ret = drm_atomic_commit(state);
-
-out:
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(&ctx);
-		goto retry;
+		ret = drm_atomic_commit(state);
 	}
 
 	drm_atomic_state_put(state);
 
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-
 	return ret;
 }
 
@@ -13323,25 +13300,14 @@ void intel_display_resume(struct drm_device *dev)
 		return;
 
 	dev_priv->modeset_restore_state = NULL;
-	if (state)
-		state->acquire_ctx = &ctx;
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	while (1) {
-		ret = drm_modeset_lock_all_ctx(dev, &ctx);
-		if (ret != -EDEADLK)
-			break;
-
-		drm_modeset_backoff(&ctx);
-	}
-
-	if (!ret)
+	drm_modeset_lock_all_ctx_retry(dev, &ctx, state, 0, ret) {
 		ret = __intel_display_resume(dev, state, &ctx);
+		if (ret)
+			continue;
 
-	intel_enable_ipc(dev_priv);
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
+		intel_enable_ipc(dev_priv);
+	}
 
 	if (ret)
 		drm_err(&dev_priv->drm,
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 65832c4d962f..e523a0ec6534 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -2294,44 +2294,32 @@ static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
 {
 	struct drm_connector *connector = m->private;
 	struct drm_device *dev = connector->dev;
-	struct drm_crtc *crtc;
-	struct intel_dp *intel_dp;
 	struct drm_modeset_acquire_ctx ctx;
-	struct intel_crtc_state *crtc_state = NULL;
-	int ret = 0;
-	bool try_again = false;
+	int ret;
 
-	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
+	drm_modeset_lock_ctx_retry(&ctx, NULL, DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret) {
+		struct intel_crtc_state *crtc_state;
+		struct intel_dp *intel_dp;
+		struct drm_crtc *crtc;
 
-	do {
-		try_again = false;
 		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
 				       &ctx);
-		if (ret) {
-			if (ret == -EDEADLK && !drm_modeset_backoff(&ctx)) {
-				try_again = true;
-				continue;
-			}
-			break;
-		}
+		if (ret)
+			continue;
+
 		crtc = connector->state->crtc;
 		if (connector->status != connector_status_connected || !crtc) {
 			ret = -ENODEV;
-			break;
+			continue;
 		}
+
 		ret = drm_modeset_lock(&crtc->mutex, &ctx);
-		if (ret == -EDEADLK) {
-			ret = drm_modeset_backoff(&ctx);
-			if (!ret) {
-				try_again = true;
-				continue;
-			}
-			break;
-		} else if (ret) {
-			break;
-		}
+		if (ret)
+			continue;
+
 		intel_dp = intel_attached_dp(to_intel_connector(connector));
 		crtc_state = to_intel_crtc_state(crtc->state);
+
 		seq_printf(m, "DSC_Enabled: %s\n",
 			   yesno(crtc_state->dsc.compression_enable));
 		seq_printf(m, "DSC_Sink_Support: %s\n",
@@ -2341,10 +2329,7 @@ static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
 		if (!intel_dp_is_edp(intel_dp))
 			seq_printf(m, "FEC_Sink_Support: %s\n",
 				   yesno(drm_dp_sink_supports_fec(intel_dp->fec_capable)));
-	} while (try_again);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
+	}
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
index 8ac263f471be..89435de4ff58 100644
--- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
@@ -293,46 +293,34 @@ intel_crtc_crc_setup_workarounds(struct intel_crtc *crtc, bool enable)
 	struct drm_modeset_acquire_ctx ctx;
 	int ret;
 
-	drm_modeset_acquire_init(&ctx, 0);
-
 	state = drm_atomic_state_alloc(&dev_priv->drm);
 	if (!state) {
 		ret = -ENOMEM;
 		goto unlock;
 	}
 
-	state->acquire_ctx = &ctx;
+	drm_modeset_lock_ctx_retry(&ctx, state, 0, ret) {
+		pipe_config = intel_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(pipe_config)) {
+			ret = PTR_ERR(pipe_config);
+			continue;
+		}
 
-retry:
-	pipe_config = intel_atomic_get_crtc_state(state, crtc);
-	if (IS_ERR(pipe_config)) {
-		ret = PTR_ERR(pipe_config);
-		goto put_state;
-	}
+		pipe_config->uapi.mode_changed = pipe_config->has_psr;
+		pipe_config->crc_enabled = enable;
 
-	pipe_config->uapi.mode_changed = pipe_config->has_psr;
-	pipe_config->crc_enabled = enable;
+		if (IS_HASWELL(dev_priv) &&
+		    pipe_config->hw.active && crtc->pipe == PIPE_A &&
+		    pipe_config->cpu_transcoder == TRANSCODER_EDP)
+			pipe_config->uapi.mode_changed = true;
 
-	if (IS_HASWELL(dev_priv) &&
-	    pipe_config->hw.active && crtc->pipe == PIPE_A &&
-	    pipe_config->cpu_transcoder == TRANSCODER_EDP)
-		pipe_config->uapi.mode_changed = true;
-
-	ret = drm_atomic_commit(state);
-
-put_state:
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(&ctx);
-		goto retry;
+		ret = drm_atomic_commit(state);
 	}
 
 	drm_atomic_state_put(state);
 unlock:
 	drm_WARN(&dev_priv->drm, ret,
 		 "Toggling workaround to %i returns %i\n", enable, ret);
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
 }
 
 static int ivb_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
-- 
2.31.1


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

* [Intel-gfx] [PATCH 4/4] drm/i915: Use drm_modeset_lock_ctx_retry() & co.
@ 2021-07-15 18:49   ` Ville Syrjala
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2021-07-15 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sean Paul, dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We have the modeset lock dance hand rolled in quite a few places.
Use drm_modeset_lock_ctx_retry() and drm_modeset_lock_all_ctx_retry()
to simplify our lives.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/g4x_dp.c         |  17 +--
 drivers/gpu/drm/i915/display/intel_audio.c    |  17 +--
 drivers/gpu/drm/i915/display/intel_ddi.c      |  16 +--
 drivers/gpu/drm/i915/display/intel_display.c  | 102 ++++++------------
 .../drm/i915/display/intel_display_debugfs.c  |  45 +++-----
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  38 +++----
 6 files changed, 69 insertions(+), 166 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
index de0f358184aa..68e7f4233fa9 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -1167,23 +1167,10 @@ intel_dp_hotplug(struct intel_encoder *encoder,
 
 	state = intel_encoder_hotplug(encoder, connector);
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	for (;;) {
+	drm_modeset_lock_ctx_retry(&ctx, NULL, 0, ret)
 		ret = intel_dp_retrain_link(encoder, &ctx);
 
-		if (ret == -EDEADLK) {
-			drm_modeset_backoff(&ctx);
-			continue;
-		}
-
-		break;
-	}
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-	drm_WARN(encoder->base.dev, ret,
-		 "Acquiring modeset locks failed with %i\n", ret);
+	drm_WARN_ON(encoder->base.dev, ret);
 
 	/*
 	 * Keeping it consistent with intel_ddi_hotplug() and
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 5f4f316b3ab5..a3b6b00e6633 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -969,28 +969,17 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
 	if (!crtc)
 		return;
 
-	drm_modeset_acquire_init(&ctx, 0);
 	state = drm_atomic_state_alloc(&dev_priv->drm);
 	if (drm_WARN_ON(&dev_priv->drm, !state))
 		return;
 
-	state->acquire_ctx = &ctx;
-
-retry:
-	ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state), crtc,
-					   enable);
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(&ctx);
-		goto retry;
-	}
+	drm_modeset_lock_ctx_retry(&ctx, state, 0, ret)
+		ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state),
+						   crtc, enable);
 
 	drm_WARN_ON(&dev_priv->drm, ret);
 
 	drm_atomic_state_put(state);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
 }
 
 static unsigned long i915_audio_component_get_power(struct device *kdev)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 26a3aa73fcc4..9aa7369d8dbe 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4210,26 +4210,14 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
 
 	state = intel_encoder_hotplug(encoder, connector);
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	for (;;) {
+	drm_modeset_lock_ctx_retry(&ctx, NULL, 0, ret) {
 		if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
 			ret = intel_hdmi_reset_link(encoder, &ctx);
 		else
 			ret = intel_dp_retrain_link(encoder, &ctx);
-
-		if (ret == -EDEADLK) {
-			drm_modeset_backoff(&ctx);
-			continue;
-		}
-
-		break;
 	}
 
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-	drm_WARN(encoder->base.dev, ret,
-		 "Acquiring modeset locks failed with %i\n", ret);
+	drm_WARN_ON(encoder->base.dev, ret);
 
 	/*
 	 * Unpowered type-c dongles can take some time to boot and be
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3718399c4c2f..6f5bc56d68e0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12057,40 +12057,30 @@ static void sanitize_watermarks(struct drm_i915_private *dev_priv)
 
 	intel_state = to_intel_atomic_state(state);
 
-	drm_modeset_acquire_init(&ctx, 0);
+	drm_modeset_lock_ctx_retry(&ctx, state, 0, ret) {
+		/*
+		 * Hardware readout is the only time we don't want to calculate
+		 * intermediate watermarks (since we don't trust the current
+		 * watermarks).
+		 */
+		if (!HAS_GMCH(dev_priv))
+			intel_state->skip_intermediate_wm = true;
 
-retry:
-	state->acquire_ctx = &ctx;
+		ret = sanitize_watermarks_add_affected(state);
+		if (ret)
+			continue;
 
-	/*
-	 * Hardware readout is the only time we don't want to calculate
-	 * intermediate watermarks (since we don't trust the current
-	 * watermarks).
-	 */
-	if (!HAS_GMCH(dev_priv))
-		intel_state->skip_intermediate_wm = true;
+		ret = intel_atomic_check(&dev_priv->drm, state);
+		if (ret)
+			continue;
 
-	ret = sanitize_watermarks_add_affected(state);
-	if (ret)
-		goto fail;
+		/* Write calculated watermark values back */
+		for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i) {
+			crtc_state->wm.need_postvbl_update = true;
+			dev_priv->display.optimize_watermarks(intel_state, crtc);
 
-	ret = intel_atomic_check(&dev_priv->drm, state);
-	if (ret)
-		goto fail;
-
-	/* Write calculated watermark values back */
-	for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i) {
-		crtc_state->wm.need_postvbl_update = true;
-		dev_priv->display.optimize_watermarks(intel_state, crtc);
-
-		to_intel_crtc_state(crtc->base.state)->wm = crtc_state->wm;
-	}
-
-fail:
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(&ctx);
-		goto retry;
+			to_intel_crtc_state(crtc->base.state)->wm = crtc_state->wm;
+		}
 	}
 
 	/*
@@ -12108,9 +12098,6 @@ static void sanitize_watermarks(struct drm_i915_private *dev_priv)
 		 "Could not determine valid watermarks for inherited state\n");
 
 	drm_atomic_state_put(state);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
 }
 
 static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv)
@@ -12181,38 +12168,28 @@ static int intel_initial_commit(struct drm_device *dev)
 {
 	struct drm_modeset_acquire_ctx ctx;
 	struct drm_atomic_state *state;
-	struct intel_crtc *crtc;
 	int ret;
 
 	state = drm_atomic_state_alloc(dev);
 	if (!state)
 		return -ENOMEM;
 
-	drm_modeset_acquire_init(&ctx, 0);
+	drm_modeset_lock_ctx_retry(&ctx, state, 0, ret) {
+		struct intel_crtc *crtc;
 
-retry:
-	state->acquire_ctx = &ctx;
-
-	for_each_intel_crtc(dev, crtc) {
-		ret = intel_crtc_initial_commit(to_intel_atomic_state(state), crtc);
+		for_each_intel_crtc(dev, crtc) {
+			ret = intel_crtc_initial_commit(to_intel_atomic_state(state), crtc);
+			if (ret)
+				break;
+		}
 		if (ret)
-			goto out;
-	}
+			continue;
 
-	ret = drm_atomic_commit(state);
-
-out:
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(&ctx);
-		goto retry;
+		ret = drm_atomic_commit(state);
 	}
 
 	drm_atomic_state_put(state);
 
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-
 	return ret;
 }
 
@@ -13323,25 +13300,14 @@ void intel_display_resume(struct drm_device *dev)
 		return;
 
 	dev_priv->modeset_restore_state = NULL;
-	if (state)
-		state->acquire_ctx = &ctx;
 
-	drm_modeset_acquire_init(&ctx, 0);
-
-	while (1) {
-		ret = drm_modeset_lock_all_ctx(dev, &ctx);
-		if (ret != -EDEADLK)
-			break;
-
-		drm_modeset_backoff(&ctx);
-	}
-
-	if (!ret)
+	drm_modeset_lock_all_ctx_retry(dev, &ctx, state, 0, ret) {
 		ret = __intel_display_resume(dev, state, &ctx);
+		if (ret)
+			continue;
 
-	intel_enable_ipc(dev_priv);
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
+		intel_enable_ipc(dev_priv);
+	}
 
 	if (ret)
 		drm_err(&dev_priv->drm,
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 65832c4d962f..e523a0ec6534 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -2294,44 +2294,32 @@ static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
 {
 	struct drm_connector *connector = m->private;
 	struct drm_device *dev = connector->dev;
-	struct drm_crtc *crtc;
-	struct intel_dp *intel_dp;
 	struct drm_modeset_acquire_ctx ctx;
-	struct intel_crtc_state *crtc_state = NULL;
-	int ret = 0;
-	bool try_again = false;
+	int ret;
 
-	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
+	drm_modeset_lock_ctx_retry(&ctx, NULL, DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret) {
+		struct intel_crtc_state *crtc_state;
+		struct intel_dp *intel_dp;
+		struct drm_crtc *crtc;
 
-	do {
-		try_again = false;
 		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
 				       &ctx);
-		if (ret) {
-			if (ret == -EDEADLK && !drm_modeset_backoff(&ctx)) {
-				try_again = true;
-				continue;
-			}
-			break;
-		}
+		if (ret)
+			continue;
+
 		crtc = connector->state->crtc;
 		if (connector->status != connector_status_connected || !crtc) {
 			ret = -ENODEV;
-			break;
+			continue;
 		}
+
 		ret = drm_modeset_lock(&crtc->mutex, &ctx);
-		if (ret == -EDEADLK) {
-			ret = drm_modeset_backoff(&ctx);
-			if (!ret) {
-				try_again = true;
-				continue;
-			}
-			break;
-		} else if (ret) {
-			break;
-		}
+		if (ret)
+			continue;
+
 		intel_dp = intel_attached_dp(to_intel_connector(connector));
 		crtc_state = to_intel_crtc_state(crtc->state);
+
 		seq_printf(m, "DSC_Enabled: %s\n",
 			   yesno(crtc_state->dsc.compression_enable));
 		seq_printf(m, "DSC_Sink_Support: %s\n",
@@ -2341,10 +2329,7 @@ static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
 		if (!intel_dp_is_edp(intel_dp))
 			seq_printf(m, "FEC_Sink_Support: %s\n",
 				   yesno(drm_dp_sink_supports_fec(intel_dp->fec_capable)));
-	} while (try_again);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
+	}
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
index 8ac263f471be..89435de4ff58 100644
--- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c
@@ -293,46 +293,34 @@ intel_crtc_crc_setup_workarounds(struct intel_crtc *crtc, bool enable)
 	struct drm_modeset_acquire_ctx ctx;
 	int ret;
 
-	drm_modeset_acquire_init(&ctx, 0);
-
 	state = drm_atomic_state_alloc(&dev_priv->drm);
 	if (!state) {
 		ret = -ENOMEM;
 		goto unlock;
 	}
 
-	state->acquire_ctx = &ctx;
+	drm_modeset_lock_ctx_retry(&ctx, state, 0, ret) {
+		pipe_config = intel_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(pipe_config)) {
+			ret = PTR_ERR(pipe_config);
+			continue;
+		}
 
-retry:
-	pipe_config = intel_atomic_get_crtc_state(state, crtc);
-	if (IS_ERR(pipe_config)) {
-		ret = PTR_ERR(pipe_config);
-		goto put_state;
-	}
+		pipe_config->uapi.mode_changed = pipe_config->has_psr;
+		pipe_config->crc_enabled = enable;
 
-	pipe_config->uapi.mode_changed = pipe_config->has_psr;
-	pipe_config->crc_enabled = enable;
+		if (IS_HASWELL(dev_priv) &&
+		    pipe_config->hw.active && crtc->pipe == PIPE_A &&
+		    pipe_config->cpu_transcoder == TRANSCODER_EDP)
+			pipe_config->uapi.mode_changed = true;
 
-	if (IS_HASWELL(dev_priv) &&
-	    pipe_config->hw.active && crtc->pipe == PIPE_A &&
-	    pipe_config->cpu_transcoder == TRANSCODER_EDP)
-		pipe_config->uapi.mode_changed = true;
-
-	ret = drm_atomic_commit(state);
-
-put_state:
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(&ctx);
-		goto retry;
+		ret = drm_atomic_commit(state);
 	}
 
 	drm_atomic_state_put(state);
 unlock:
 	drm_WARN(&dev_priv->drm, ret,
 		 "Toggling workaround to %i returns %i\n", enable, ret);
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
 }
 
 static int ivb_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
-- 
2.31.1

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

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Make modeset locking easier
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
                   ` (4 preceding siblings ...)
  (?)
@ 2021-07-16 22:21 ` Patchwork
  -1 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2021-07-16 22:21 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm: Make modeset locking easier
URL   : https://patchwork.freedesktop.org/series/92606/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b1e190e95142 drm: Introduce drm_modeset_lock_ctx_retry()
-:104: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'ctx' - possible side-effects?
#104: FILE: include/drm/drm_modeset_lock.h:221:
+#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret)))

-:104: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'state' - possible side-effects?
#104: FILE: include/drm/drm_modeset_lock.h:221:
+#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret)))

-:104: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'ret' - possible side-effects?
#104: FILE: include/drm/drm_modeset_lock.h:221:
+#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret)))

total: 0 errors, 0 warnings, 3 checks, 77 lines checked
03c1e97e22ca drm: Introduce drm_modeset_lock_all_ctx_retry()
-:32: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'ctx' - possible side-effects?
#32: FILE: include/drm/drm_modeset_lock.h:226:
+#define drm_modeset_lock_all_ctx_retry(dev, ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret))) \
+		for_each_if(((ret) = drm_modeset_lock_all_ctx((dev), (ctx))) == 0)

-:32: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'state' - possible side-effects?
#32: FILE: include/drm/drm_modeset_lock.h:226:
+#define drm_modeset_lock_all_ctx_retry(dev, ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret))) \
+		for_each_if(((ret) = drm_modeset_lock_all_ctx((dev), (ctx))) == 0)

-:32: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'ret' - possible side-effects?
#32: FILE: include/drm/drm_modeset_lock.h:226:
+#define drm_modeset_lock_all_ctx_retry(dev, ctx, state, flags, ret) \
+	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+	     _drm_modeset_lock_loop(&(ret)); \
+	     _drm_modeset_lock_end((ctx), (state), &(ret))) \
+		for_each_if(((ret) = drm_modeset_lock_all_ctx((dev), (ctx))) == 0)

total: 0 errors, 0 warnings, 3 checks, 10 lines checked
0ddd911343c8 drm/i915: Extract intel_crtc_initial_commit()
a7ff291d8e28 drm/i915: Use drm_modeset_lock_ctx_retry() & co.


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

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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: Make modeset locking easier
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
                   ` (5 preceding siblings ...)
  (?)
@ 2021-07-16 22:24 ` Patchwork
  -1 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2021-07-16 22:24 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm: Make modeset locking easier
URL   : https://patchwork.freedesktop.org/series/92606/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgv_sriovmsg.h:316:49: error: static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1345:25: error: incompatible types in comparison expression (different address spaces):
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1345:25:    struct dma_fence *
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1345:25:    struct dma_fence [noderef] __rcu *
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1346:17: error: incompatible types in comparison expression (different address spaces):
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1346:17:    struct dma_fence *
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1346:17:    struct dma_fence [noderef] __rcu *
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1405:17: error: incompatible types in comparison expression (different address spaces):
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1405:17:    struct dma_fence *
+drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:1405:17:    struct dma_fence [noderef] __rcu *
+drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:310:16: error: incompatible types in comparison expression (different type sizes):
+drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:310:16:    unsigned long *
+drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:310:16:    unsigned long long *
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:275:25: error: incompatible types in comparison expression (different address spaces):
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:275:25:    struct dma_fence *
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:275:25:    struct dma_fence [noderef] __rcu *
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:276:17: error: incompatible types in comparison expression (different address spaces):
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:276:17:    struct dma_fence *
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:276:17:    struct dma_fence [noderef] __rcu *
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:330:17: error: incompatible types in comparison expression (different address spaces):
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:330:17:    struct dma_fence *
+drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:330:17:    struct dma_fence [noderef] __rcu *
+drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h:90:56: error: marked inline, but without a definition
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
+drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:312:49: error:


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

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

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm: Make modeset locking easier
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
                   ` (6 preceding siblings ...)
  (?)
@ 2021-07-16 22:27 ` Patchwork
  -1 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2021-07-16 22:27 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm: Make modeset locking easier
URL   : https://patchwork.freedesktop.org/series/92606/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'jump_whitelist' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'shadow_map' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'batch_map' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Function parameter or member 'trampoline' not described in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'jump_whitelist' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'shadow_map' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'batch_map' description in 'intel_engine_cmd_parser'


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

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm: Make modeset locking easier
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
                   ` (7 preceding siblings ...)
  (?)
@ 2021-07-16 22:50 ` Patchwork
  -1 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2021-07-16 22:50 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx


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

== Series Details ==

Series: drm: Make modeset locking easier
URL   : https://patchwork.freedesktop.org/series/92606/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10346 -> Patchwork_20632
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@semaphore:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][1] ([fdo#109271]) +27 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/fi-bdw-5557u/igt@amdgpu/amd_basic@semaphore.html

  * igt@core_hotunplug@unbind-rebind:
    - fi-bdw-5557u:       NOTRUN -> [WARN][2] ([i915#3718])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/fi-bdw-5557u/igt@core_hotunplug@unbind-rebind.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-guc:         [PASS][3] -> [FAIL][4] ([i915#2203] / [i915#579])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-kefka:       [PASS][5] -> [INCOMPLETE][6] ([i915#2782] / [i915#2940])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/fi-bsw-kefka/igt@i915_selftest@live@execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/fi-bsw-kefka/igt@i915_selftest@live@execlists.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7500u:       [PASS][7] -> [DMESG-WARN][8] ([i915#2868])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][9] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/fi-bdw-5557u/igt@kms_chamelium@dp-crc-fast.html

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> [FAIL][10] ([fdo#109271] / [i915#1436])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/fi-bsw-kefka/igt@runner@aborted.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#2782]: https://gitlab.freedesktop.org/drm/intel/issues/2782
  [i915#2868]: https://gitlab.freedesktop.org/drm/intel/issues/2868
  [i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
  [i915#3718]: https://gitlab.freedesktop.org/drm/intel/issues/3718
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579


Participating hosts (41 -> 35)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-bdw-samus fi-tgl-y bat-jsl-1 


Build changes
-------------

  * Linux: CI_DRM_10346 -> Patchwork_20632

  CI-20190529: 20190529
  CI_DRM_10346: 6c4e3c031a995e641cc0d9563d21043415fb8d12 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6144: bc65ee9ee6593716306448c9fb82c77f284f2148 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20632: a7ff291d8e28efcf43245a527c62e692a75aa48c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a7ff291d8e28 drm/i915: Use drm_modeset_lock_ctx_retry() & co.
0ddd911343c8 drm/i915: Extract intel_crtc_initial_commit()
03c1e97e22ca drm: Introduce drm_modeset_lock_all_ctx_retry()
b1e190e95142 drm: Introduce drm_modeset_lock_ctx_retry()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/index.html

[-- Attachment #1.2: Type: text/html, Size: 4952 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] 27+ messages in thread

* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm: Make modeset locking easier
  2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
                   ` (8 preceding siblings ...)
  (?)
@ 2021-07-17  7:20 ` Patchwork
  -1 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2021-07-17  7:20 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx


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

== Series Details ==

Series: drm: Make modeset locking easier
URL   : https://patchwork.freedesktop.org/series/92606/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10346_full -> Patchwork_20632_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_20632_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_20632_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_20632_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gen9_exec_parse@bb-start-far:
    - shard-iclb:         NOTRUN -> [SKIP][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@gen9_exec_parse@bb-start-far.html

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-skl:          [PASS][2] -> [FAIL][3] +9 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-skl10/igt@i915_pm_backlight@fade_with_suspend.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl5/igt@i915_pm_backlight@fade_with_suspend.html
    - shard-iclb:         NOTRUN -> [FAIL][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@i915_pm_backlight@fade_with_suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-apl:          NOTRUN -> [FAIL][5] +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl3/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-vga1:
    - shard-snb:          NOTRUN -> [FAIL][6] +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-snb2/igt@kms_flip@flip-vs-suspend-interruptible@a-vga1.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-iclb:         [PASS][7] -> [FAIL][8] +20 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-kbl:          [PASS][9] -> [FAIL][10] +11 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-kbl3/igt@kms_hdr@bpc-switch-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl6/igt@kms_hdr@bpc-switch-suspend.html
    - shard-apl:          [PASS][11] -> [FAIL][12] +6 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-apl2/igt@kms_hdr@bpc-switch-suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl7/igt@kms_hdr@bpc-switch-suspend.html
    - shard-skl:          NOTRUN -> [FAIL][13]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes:
    - shard-glk:          [PASS][14] -> [FAIL][15] +12 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-glk1/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-glk5/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-snb:          [PASS][16] -> [FAIL][17] +5 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-snb2/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-snb5/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html

  
#### Warnings ####

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][18] ([i915#180]) -> [FAIL][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-snb:          [SKIP][20] ([fdo#109271]) -> [FAIL][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-snb6/igt@kms_fbcon_fbt@fbc-suspend.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-snb6/igt@kms_fbcon_fbt@fbc-suspend.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_eio@hibernate:
    - {shard-rkl}:        [PASS][22] -> [INCOMPLETE][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-rkl-1/igt@gem_eio@hibernate.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-rkl-6/igt@gem_eio@hibernate.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - {shard-rkl}:        [SKIP][24] ([fdo#112022]) -> [FAIL][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-rkl-1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-rkl-6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - {shard-rkl}:        [SKIP][26] ([i915#1845]) -> [FAIL][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-rkl-2/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-rkl-6/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  * igt@runner@aborted:
    - {shard-rkl}:        ([FAIL][28], [FAIL][29], [FAIL][30]) ([i915#2029] / [i915#3002]) -> ([FAIL][31], [FAIL][32], [FAIL][33], [FAIL][34]) ([i915#3002])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-rkl-5/igt@runner@aborted.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-rkl-5/igt@runner@aborted.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-rkl-1/igt@runner@aborted.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-rkl-1/igt@runner@aborted.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-rkl-6/igt@runner@aborted.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-rkl-1/igt@runner@aborted.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-rkl-5/igt@runner@aborted.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@legacy-engines-mixed:
    - shard-snb:          NOTRUN -> [SKIP][35] ([fdo#109271] / [i915#1099]) +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-snb5/igt@gem_ctx_persistence@legacy-engines-mixed.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-tglb:         NOTRUN -> [FAIL][36] ([i915#2842]) +4 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-tglb:         [PASS][37] -> [FAIL][38] ([i915#2842]) +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-tglb5/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb5/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace@vcs1:
    - shard-kbl:          [PASS][39] -> [FAIL][40] ([i915#2842])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-kbl4/igt@gem_exec_fair@basic-pace@vcs1.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl1/igt@gem_exec_fair@basic-pace@vcs1.html

  * igt@gem_huc_copy@huc-copy:
    - shard-apl:          NOTRUN -> [SKIP][41] ([fdo#109271] / [i915#2190])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl3/igt@gem_huc_copy@huc-copy.html

  * igt@gem_pread@exhaustion:
    - shard-apl:          NOTRUN -> [WARN][42] ([i915#2658]) +1 similar issue
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl3/igt@gem_pread@exhaustion.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-snb:          NOTRUN -> [WARN][43] ([i915#2658])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-snb2/igt@gem_pwrite@basic-exhaustion.html
    - shard-kbl:          NOTRUN -> [WARN][44] ([i915#2658])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl7/igt@gem_pwrite@basic-exhaustion.html

  * igt@gem_render_copy@y-tiled-mc-ccs-to-yf-tiled-ccs:
    - shard-iclb:         NOTRUN -> [SKIP][45] ([i915#768])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@gem_render_copy@y-tiled-mc-ccs-to-yf-tiled-ccs.html

  * igt@gem_workarounds@basic-read-context:
    - shard-snb:          [PASS][46] -> [TIMEOUT][47] ([i915#2808])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-snb7/igt@gem_workarounds@basic-read-context.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-snb6/igt@gem_workarounds@basic-read-context.html

  * igt@gen7_exec_parse@chained-batch:
    - shard-iclb:         NOTRUN -> [SKIP][48] ([fdo#109289]) +1 similar issue
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@gen7_exec_parse@chained-batch.html

  * igt@gen9_exec_parse@batch-invalid-length:
    - shard-snb:          NOTRUN -> [SKIP][49] ([fdo#109271]) +229 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-snb2/igt@gen9_exec_parse@batch-invalid-length.html

  * igt@i915_pm_lpsp@kms-lpsp:
    - shard-skl:          NOTRUN -> [SKIP][50] ([fdo#109271]) +112 similar issues
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@i915_pm_lpsp@kms-lpsp.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - shard-iclb:         NOTRUN -> [SKIP][51] ([fdo#110892])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@i915_pm_rpm@modeset-non-lpsp-stress.html

  * igt@kms_async_flips@alternate-sync-async-flip:
    - shard-skl:          [PASS][52] -> [FAIL][53] ([i915#2521])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-skl2/igt@kms_async_flips@alternate-sync-async-flip.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl10/igt@kms_async_flips@alternate-sync-async-flip.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-async-flip:
    - shard-skl:          NOTRUN -> [FAIL][54] ([i915#3722])
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-async-flip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-apl:          NOTRUN -> [SKIP][55] ([fdo#109271] / [i915#3777]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl7/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-hflip:
    - shard-skl:          NOTRUN -> [SKIP][56] ([fdo#109271] / [i915#3777])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-hflip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-kbl:          NOTRUN -> [SKIP][57] ([fdo#109271] / [i915#3777])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl7/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0:
    - shard-iclb:         NOTRUN -> [SKIP][58] ([fdo#110723]) +1 similar issue
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0.html

  * igt@kms_ccs@pipe-b-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc:
    - shard-skl:          NOTRUN -> [FAIL][59] ([i915#3678])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@kms_ccs@pipe-b-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-b-crc-primary-basic-yf_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][60] ([i915#3689]) +5 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@kms_ccs@pipe-b-crc-primary-basic-yf_tiled_ccs.html

  * igt@kms_chamelium@dp-hpd-storm-disable:
    - shard-tglb:         NOTRUN -> [SKIP][61] ([fdo#109284] / [fdo#111827]) +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@kms_chamelium@dp-hpd-storm-disable.html

  * igt@kms_chamelium@dp-mode-timings:
    - shard-apl:          NOTRUN -> [SKIP][62] ([fdo#109271] / [fdo#111827]) +24 similar issues
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl3/igt@kms_chamelium@dp-mode-timings.html

  * igt@kms_chamelium@hdmi-aspect-ratio:
    - shard-kbl:          NOTRUN -> [SKIP][63] ([fdo#109271] / [fdo#111827]) +1 similar issue
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl7/igt@kms_chamelium@hdmi-aspect-ratio.html

  * igt@kms_chamelium@vga-edid-read:
    - shard-iclb:         NOTRUN -> [SKIP][64] ([fdo#109284] / [fdo#111827]) +1 similar issue
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_color@pipe-d-ctm-negative:
    - shard-iclb:         NOTRUN -> [SKIP][65] ([fdo#109278] / [i915#1149]) +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@kms_color@pipe-d-ctm-negative.html

  * igt@kms_color_chamelium@pipe-invalid-ctm-matrix-sizes:
    - shard-skl:          NOTRUN -> [SKIP][66] ([fdo#109271] / [fdo#111827]) +11 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@kms_color_chamelium@pipe-invalid-ctm-matrix-sizes.html
    - shard-snb:          NOTRUN -> [SKIP][67] ([fdo#109271] / [fdo#111827]) +14 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-snb2/igt@kms_color_chamelium@pipe-invalid-ctm-matrix-sizes.html

  * igt@kms_cursor_crc@pipe-a-cursor-32x32-sliding:
    - shard-tglb:         NOTRUN -> [SKIP][68] ([i915#3319]) +2 similar issues
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@kms_cursor_crc@pipe-a-cursor-32x32-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-32x32-rapid-movement:
    - shard-glk:          NOTRUN -> [SKIP][69] ([fdo#109271]) +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-glk5/igt@kms_cursor_crc@pipe-c-cursor-32x32-rapid-movement.html

  * igt@kms_cursor_crc@pipe-d-cursor-512x170-offscreen:
    - shard-tglb:         NOTRUN -> [SKIP][70] ([fdo#109279] / [i915#3359]) +1 similar issue
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb1/igt@kms_cursor_crc@pipe-d-cursor-512x170-offscreen.html

  * igt@kms_cursor_crc@pipe-d-cursor-dpms:
    - shard-iclb:         NOTRUN -> [SKIP][71] ([fdo#109278]) +11 similar issues
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@kms_cursor_crc@pipe-d-cursor-dpms.html

  * igt@kms_cursor_legacy@cursorb-vs-flipb-atomic-transitions:
    - shard-iclb:         NOTRUN -> [SKIP][72] ([fdo#109274] / [fdo#109278])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@kms_cursor_legacy@cursorb-vs-flipb-atomic-transitions.html

  * igt@kms_cursor_legacy@pipe-d-torture-bo:
    - shard-apl:          NOTRUN -> [SKIP][73] ([fdo#109271] / [i915#533]) +4 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl2/igt@kms_cursor_legacy@pipe-d-torture-bo.html

  * igt@kms_dither@fb-8bpc-vs-panel-8bpc@edp-1-pipe-a:
    - shard-iclb:         [PASS][74] -> [SKIP][75] ([i915#3788])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-iclb8/igt@kms_dither@fb-8bpc-vs-panel-8bpc@edp-1-pipe-a.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb2/igt@kms_dither@fb-8bpc-vs-panel-8bpc@edp-1-pipe-a.html

  * igt@kms_dp_tiled_display@basic-test-pattern:
    - shard-tglb:         NOTRUN -> [SKIP][76] ([i915#426])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@kms_dp_tiled_display@basic-test-pattern.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-iclb:         [PASS][77] -> [FAIL][78] ([i915#64]) +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-iclb4/igt@kms_fbcon_fbt@fbc-suspend.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb1/igt@kms_fbcon_fbt@fbc-suspend.html
    - shard-glk:          [PASS][79] -> [FAIL][80] ([i915#64])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-glk5/igt@kms_fbcon_fbt@fbc-suspend.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-glk9/igt@kms_fbcon_fbt@fbc-suspend.html
    - shard-tglb:         [PASS][81] -> [FAIL][82] ([i915#2411] / [i915#64]) +1 similar issue
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-tglb6/igt@kms_fbcon_fbt@fbc-suspend.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb5/igt@kms_fbcon_fbt@fbc-suspend.html
    - shard-kbl:          [PASS][83] -> [FAIL][84] ([i915#64])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-kbl3/igt@kms_fbcon_fbt@fbc-suspend.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl1/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [PASS][85] -> [FAIL][86] ([i915#64])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-skl10/igt@kms_fbcon_fbt@psr-suspend.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl5/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_flip@2x-flip-vs-modeset-vs-hang:
    - shard-iclb:         NOTRUN -> [SKIP][87] ([fdo#109274])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb7/igt@kms_flip@2x-flip-vs-modeset-vs-hang.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-dp1:
    - shard-kbl:          [PASS][88] -> [FAIL][89] ([i915#79])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-kbl7/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-dp1.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl7/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-dp1.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1:
    - shard-skl:          NOTRUN -> [FAIL][90] ([i915#79])
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [PASS][91] -> [FAIL][92] ([i915#275]) +5 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_flip@flip-vs-suspend@a-dp1:
    - shard-apl:          [PASS][93] -> [FAIL][94] ([i915#275])
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-apl3/igt@kms_flip@flip-vs-suspend@a-dp1.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl6/igt@kms_flip@flip-vs-suspend@a-dp1.html

  * igt@kms_flip@flip-vs-suspend@a-edp1:
    - shard-skl:          [PASS][95] -> [FAIL][96] ([i915#275]) +4 similar issues
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-skl4/igt@kms_flip@flip-vs-suspend@a-edp1.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl5/igt@kms_flip@flip-vs-suspend@a-edp1.html
    - shard-tglb:         [PASS][97] -> [FAIL][98] ([i915#2411] / [i915#275]) +7 similar issues
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-tglb7/igt@kms_flip@flip-vs-suspend@a-edp1.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb6/igt@kms_flip@flip-vs-suspend@a-edp1.html

  * igt@kms_flip@flip-vs-suspend@a-hdmi-a1:
    - shard-glk:          [PASS][99] -> [FAIL][100] ([i915#275]) +17 similar issues
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-glk1/igt@kms_flip@flip-vs-suspend@a-hdmi-a1.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-glk5/igt@kms_flip@flip-vs-suspend@a-hdmi-a1.html

  * igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilercccs:
    - shard-apl:          NOTRUN -> [SKIP][101] ([fdo#109271] / [i915#2672])
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl3/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilercccs.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-blt:
    - shard-kbl:          NOTRUN -> [SKIP][102] ([fdo#109271]) +27 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-onoff:
    - shard-iclb:         NOTRUN -> [SKIP][103] ([fdo#109280]) +3 similar issues
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-onoff.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-blt:
    - shard-tglb:         NOTRUN -> [SKIP][104] ([fdo#111825]) +4 similar issues
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-blt.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-skl:          NOTRUN -> [SKIP][105] ([fdo#109271] / [i915#533]) +1 similar issue
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          NOTRUN -> [FAIL][106] ([fdo#108145] / [i915#265]) +2 similar issues
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> [FAIL][107] ([fdo#108145] / [i915#265]) +2 similar issues
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][108] -> [FAIL][109] ([fdo#108145] / [i915#265]) +1 similar issue
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb:
    - shard-apl:          NOTRUN -> [FAIL][110] ([i915#265]) +1 similar issue
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl3/igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb.html

  * igt@kms_plane_lowres@pipe-c-tiling-none:
    - shard-tglb:         NOTRUN -> [SKIP][111] ([i915#3536])
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@kms_plane_lowres@pipe-c-tiling-none.html

  * igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-c-scaler-with-clipping-clamping:
    - shard-apl:          NOTRUN -> [SKIP][112] ([fdo#109271] / [i915#2733])
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl1/igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-c-scaler-with-clipping-clamping.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2:
    - shard-apl:          NOTRUN -> [SKIP][113] ([fdo#109271] / [i915#658]) +5 similar issues
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl3/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-0:
    - shard-skl:          NOTRUN -> [SKIP][114] ([fdo#109271] / [i915#658]) +1 similar issue
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@kms_psr2_sf@plane-move-sf-dmg-area-0.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-2:
    - shard-kbl:          NOTRUN -> [SKIP][115] ([fdo#109271] / [i915#658])
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-kbl7/igt@kms_psr2_sf@plane-move-sf-dmg-area-2.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][116] -> [SKIP][117] ([fdo#109642] / [fdo#111068] / [i915#658])
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb7/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-iclb:         NOTRUN -> [SKIP][118] ([fdo#109441])
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@kms_psr@psr2_cursor_mmap_gtt.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][119] -> [SKIP][120] ([fdo#109441]) +2 similar issues
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb7/igt@kms_psr@psr2_primary_page_flip.html
    - shard-tglb:         NOTRUN -> [FAIL][121] ([i915#132] / [i915#3467])
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_psr@suspend:
    - shard-skl:          [PASS][122] -> [FAIL][123] ([i915#132])
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-skl10/igt@kms_psr@suspend.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl5/igt@kms_psr@suspend.html
    - shard-tglb:         [PASS][124] -> [FAIL][125] ([i915#132] / [i915#2411])
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-tglb7/igt@kms_psr@suspend.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb1/igt@kms_psr@suspend.html
    - shard-iclb:         [PASS][126] -> [FAIL][127] ([i915#132])
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-iclb7/igt@kms_psr@suspend.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb6/igt@kms_psr@suspend.html

  * igt@kms_sysfs_edid_timing:
    - shard-apl:          NOTRUN -> [FAIL][128] ([IGT#2])
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl6/igt@kms_sysfs_edid_timing.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-tglb:         [PASS][129] -> [FAIL][130] ([i915#2411]) +18 similar issues
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-tglb1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@kms_vrr@flip-basic:
    - shard-iclb:         NOTRUN -> [SKIP][131] ([fdo#109502])
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@kms_vrr@flip-basic.html

  * igt@kms_writeback@writeback-check-output:
    - shard-apl:          NOTRUN -> [SKIP][132] ([fdo#109271] / [i915#2437]) +2 similar issues
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl3/igt@kms_writeback@writeback-check-output.html

  * igt@kms_writeback@writeback-invalid-parameters:
    - shard-tglb:         NOTRUN -> [SKIP][133] ([i915#2437])
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@kms_writeback@writeback-invalid-parameters.html

  * igt@nouveau_crc@pipe-b-source-rg:
    - shard-iclb:         NOTRUN -> [SKIP][134] ([i915#2530])
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-iclb3/igt@nouveau_crc@pipe-b-source-rg.html

  * igt@nouveau_crc@pipe-d-source-outp-inactive:
    - shard-tglb:         NOTRUN -> [SKIP][135] ([i915#2530])
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb7/igt@nouveau_crc@pipe-d-source-outp-inactive.html

  * igt@prime_nv_api@i915_nv_reimport_twice_check_flink_name:
    - shard-apl:          NOTRUN -> [SKIP][136] ([fdo#109271]) +312 similar issues
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl8/igt@prime_nv_api@i915_nv_reimport_twice_check_flink_name.html

  * igt@prime_nv_test@i915_import_gtt_mmap:
    - shard-tglb:         NOTRUN -> [SKIP][137] ([fdo#109291]) +1 similar issue
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-tglb1/igt@prime_nv_test@i915_import_gtt_mmap.html

  * igt@sysfs_clients@pidname:
    - shard-skl:          NOTRUN -> [SKIP][138] ([fdo#109271] / [i915#2994])
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-skl3/igt@sysfs_clients@pidname.html

  * igt@sysfs_clients@recycle-many:
    - shard-apl:          NOTRUN -> [SKIP][139] ([fdo#109271] / [i915#2994]) +2 similar issues
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/shard-apl8/igt@sysfs_clients@recycle-many.html

  
#### Possible fixes ####

  * igt@fbdev@nullptr:
    - {shard-rkl}:        [SKIP][140] ([i915#2582]) -> [PASS][141]
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10346/shard-rkl-2/igt@fbdev@nullptr.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20632/index.html

[-- Attachment #1.2: Type: text/html, Size: 33518 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] 27+ messages in thread

* Re: [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
  2021-07-15 18:49   ` [Intel-gfx] " Ville Syrjala
@ 2021-07-20 13:44     ` Daniel Vetter
  -1 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2021-07-20 13:44 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx, Sean Paul, dri-devel

On Thu, Jul 15, 2021 at 09:49:51PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Quite a few places are hand rolling the modeset lock backoff dance.
> Let's suck that into a helper macro that is easier to use without
> forgetting some steps.
> 
> The main downside is probably that the implementation of
> drm_with_modeset_lock_ctx() is a bit harder to read than a hand
> rolled version on account of being split across three functions,
> but the actual code using it ends up being much simpler.
> 
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
>  include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> index fcfe1a03c4a1..083df96632e8 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
> +
> +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> +			     struct drm_atomic_state *state,
> +			     unsigned int flags, int *ret)
> +{
> +	drm_modeset_acquire_init(ctx, flags);
> +
> +	if (state)
> +		state->acquire_ctx = ctx;
> +
> +	*ret = -EDEADLK;
> +}
> +EXPORT_SYMBOL(_drm_modeset_lock_begin);
> +
> +bool _drm_modeset_lock_loop(int *ret)
> +{
> +	if (*ret == -EDEADLK) {
> +		*ret = 0;
> +		return true;
> +	}
> +
> +	return false;
> +}
> +EXPORT_SYMBOL(_drm_modeset_lock_loop);
> +
> +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> +			   struct drm_atomic_state *state,
> +			   int *ret)
> +{
> +	if (*ret == -EDEADLK) {
> +		if (state)
> +			drm_atomic_state_clear(state);
> +
> +		*ret = drm_modeset_backoff(ctx);
> +		if (*ret == 0) {
> +			*ret = -EDEADLK;
> +			return;
> +		}
> +	}
> +
> +	drm_modeset_drop_locks(ctx);
> +	drm_modeset_acquire_fini(ctx);
> +}
> +EXPORT_SYMBOL(_drm_modeset_lock_end);
> diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> index aafd07388eb7..5eaad2533de5 100644
> --- a/include/drm/drm_modeset_lock.h
> +++ b/include/drm/drm_modeset_lock.h
> @@ -26,6 +26,7 @@
>  
>  #include <linux/ww_mutex.h>
>  
> +struct drm_atomic_state;
>  struct drm_modeset_lock;
>  
>  /**
> @@ -203,4 +204,23 @@ modeset_lock_fail:							\
>  	if (!drm_drv_uses_atomic_modeset(dev))				\
>  		mutex_unlock(&dev->mode_config.mutex);
>  
> +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> +			     struct drm_atomic_state *state,
> +			     unsigned int flags,
> +			     int *ret);
> +bool _drm_modeset_lock_loop(int *ret);
> +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> +			   struct drm_atomic_state *state,
> +			   int *ret);
> +
> +/*
> + * Note that one must always use "continue" rather than
> + * "break" or "return" to handle errors within the
> + * drm_modeset_lock_ctx_retry() block.

I'm not sold on loop macros with these kind of restrictions, C just isn't
a great language for these. That's why e.g. drm_connector_iter doesn't
give you a macro, but only the begin/next/end function calls explicitly.

Yes the macro we have is also not nice, but at least it's a screaming
macro since it's all uppercase, so options are all a bit sucky. Which
leads me to think we have a bit a https://xkcd.com/927/ situation going
on.

I think minimally we should have one way to do this.
-Daniel

> + */
> +#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \
> +	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
> +	     _drm_modeset_lock_loop(&(ret)); \
> +	     _drm_modeset_lock_end((ctx), (state), &(ret)))
> +
>  #endif /* DRM_MODESET_LOCK_H_ */
> -- 
> 2.31.1
> 

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

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

* Re: [Intel-gfx] [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
@ 2021-07-20 13:44     ` Daniel Vetter
  0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2021-07-20 13:44 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx, Sean Paul, dri-devel

On Thu, Jul 15, 2021 at 09:49:51PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Quite a few places are hand rolling the modeset lock backoff dance.
> Let's suck that into a helper macro that is easier to use without
> forgetting some steps.
> 
> The main downside is probably that the implementation of
> drm_with_modeset_lock_ctx() is a bit harder to read than a hand
> rolled version on account of being split across three functions,
> but the actual code using it ends up being much simpler.
> 
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
>  include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> index fcfe1a03c4a1..083df96632e8 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
> +
> +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> +			     struct drm_atomic_state *state,
> +			     unsigned int flags, int *ret)
> +{
> +	drm_modeset_acquire_init(ctx, flags);
> +
> +	if (state)
> +		state->acquire_ctx = ctx;
> +
> +	*ret = -EDEADLK;
> +}
> +EXPORT_SYMBOL(_drm_modeset_lock_begin);
> +
> +bool _drm_modeset_lock_loop(int *ret)
> +{
> +	if (*ret == -EDEADLK) {
> +		*ret = 0;
> +		return true;
> +	}
> +
> +	return false;
> +}
> +EXPORT_SYMBOL(_drm_modeset_lock_loop);
> +
> +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> +			   struct drm_atomic_state *state,
> +			   int *ret)
> +{
> +	if (*ret == -EDEADLK) {
> +		if (state)
> +			drm_atomic_state_clear(state);
> +
> +		*ret = drm_modeset_backoff(ctx);
> +		if (*ret == 0) {
> +			*ret = -EDEADLK;
> +			return;
> +		}
> +	}
> +
> +	drm_modeset_drop_locks(ctx);
> +	drm_modeset_acquire_fini(ctx);
> +}
> +EXPORT_SYMBOL(_drm_modeset_lock_end);
> diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> index aafd07388eb7..5eaad2533de5 100644
> --- a/include/drm/drm_modeset_lock.h
> +++ b/include/drm/drm_modeset_lock.h
> @@ -26,6 +26,7 @@
>  
>  #include <linux/ww_mutex.h>
>  
> +struct drm_atomic_state;
>  struct drm_modeset_lock;
>  
>  /**
> @@ -203,4 +204,23 @@ modeset_lock_fail:							\
>  	if (!drm_drv_uses_atomic_modeset(dev))				\
>  		mutex_unlock(&dev->mode_config.mutex);
>  
> +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> +			     struct drm_atomic_state *state,
> +			     unsigned int flags,
> +			     int *ret);
> +bool _drm_modeset_lock_loop(int *ret);
> +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> +			   struct drm_atomic_state *state,
> +			   int *ret);
> +
> +/*
> + * Note that one must always use "continue" rather than
> + * "break" or "return" to handle errors within the
> + * drm_modeset_lock_ctx_retry() block.

I'm not sold on loop macros with these kind of restrictions, C just isn't
a great language for these. That's why e.g. drm_connector_iter doesn't
give you a macro, but only the begin/next/end function calls explicitly.

Yes the macro we have is also not nice, but at least it's a screaming
macro since it's all uppercase, so options are all a bit sucky. Which
leads me to think we have a bit a https://xkcd.com/927/ situation going
on.

I think minimally we should have one way to do this.
-Daniel

> + */
> +#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \
> +	for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
> +	     _drm_modeset_lock_loop(&(ret)); \
> +	     _drm_modeset_lock_end((ctx), (state), &(ret)))
> +
>  #endif /* DRM_MODESET_LOCK_H_ */
> -- 
> 2.31.1
> 

-- 
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] 27+ messages in thread

* Re: [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
  2021-07-20 13:44     ` [Intel-gfx] " Daniel Vetter
@ 2021-10-04 11:15       ` Ville Syrjälä
  -1 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2021-10-04 11:15 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel, Sean Paul, Fernando Ramos

On Tue, Jul 20, 2021 at 03:44:49PM +0200, Daniel Vetter wrote:
> On Thu, Jul 15, 2021 at 09:49:51PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Quite a few places are hand rolling the modeset lock backoff dance.
> > Let's suck that into a helper macro that is easier to use without
> > forgetting some steps.
> > 
> > The main downside is probably that the implementation of
> > drm_with_modeset_lock_ctx() is a bit harder to read than a hand
> > rolled version on account of being split across three functions,
> > but the actual code using it ends up being much simpler.
> > 
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
> >  include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
> >  2 files changed, 64 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> > index fcfe1a03c4a1..083df96632e8 100644
> > --- a/drivers/gpu/drm/drm_modeset_lock.c
> > +++ b/drivers/gpu/drm/drm_modeset_lock.c
> > @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
> > +
> > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > +			     struct drm_atomic_state *state,
> > +			     unsigned int flags, int *ret)
> > +{
> > +	drm_modeset_acquire_init(ctx, flags);
> > +
> > +	if (state)
> > +		state->acquire_ctx = ctx;
> > +
> > +	*ret = -EDEADLK;
> > +}
> > +EXPORT_SYMBOL(_drm_modeset_lock_begin);
> > +
> > +bool _drm_modeset_lock_loop(int *ret)
> > +{
> > +	if (*ret == -EDEADLK) {
> > +		*ret = 0;
> > +		return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +EXPORT_SYMBOL(_drm_modeset_lock_loop);
> > +
> > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > +			   struct drm_atomic_state *state,
> > +			   int *ret)
> > +{
> > +	if (*ret == -EDEADLK) {
> > +		if (state)
> > +			drm_atomic_state_clear(state);
> > +
> > +		*ret = drm_modeset_backoff(ctx);
> > +		if (*ret == 0) {
> > +			*ret = -EDEADLK;
> > +			return;
> > +		}
> > +	}
> > +
> > +	drm_modeset_drop_locks(ctx);
> > +	drm_modeset_acquire_fini(ctx);
> > +}
> > +EXPORT_SYMBOL(_drm_modeset_lock_end);
> > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> > index aafd07388eb7..5eaad2533de5 100644
> > --- a/include/drm/drm_modeset_lock.h
> > +++ b/include/drm/drm_modeset_lock.h
> > @@ -26,6 +26,7 @@
> >  
> >  #include <linux/ww_mutex.h>
> >  
> > +struct drm_atomic_state;
> >  struct drm_modeset_lock;
> >  
> >  /**
> > @@ -203,4 +204,23 @@ modeset_lock_fail:							\
> >  	if (!drm_drv_uses_atomic_modeset(dev))				\
> >  		mutex_unlock(&dev->mode_config.mutex);
> >  
> > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > +			     struct drm_atomic_state *state,
> > +			     unsigned int flags,
> > +			     int *ret);
> > +bool _drm_modeset_lock_loop(int *ret);
> > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > +			   struct drm_atomic_state *state,
> > +			   int *ret);
> > +
> > +/*
> > + * Note that one must always use "continue" rather than
> > + * "break" or "return" to handle errors within the
> > + * drm_modeset_lock_ctx_retry() block.
> 
> I'm not sold on loop macros with these kind of restrictions, C just isn't
> a great language for these. That's why e.g. drm_connector_iter doesn't
> give you a macro, but only the begin/next/end function calls explicitly.

We already use this pattern extensively in i915. Gem ww ctx has one,
power domains/pps/etc. use a similar things. It makes the code pretty nice,
with the slight caveat that an accidental 'break' can ruin your day. But
so can an accidental return with other constructs (and we even had that
happen a few times with the connector iterators), so not a dealbreaker
IMO.

So if we don't want this drm wide I guess I can propose this just for
i915 since it fits in perfectly there.

> 
> Yes the macro we have is also not nice, but at least it's a screaming
> macro since it's all uppercase, so options are all a bit sucky. Which
> leads me to think we have a bit a https://xkcd.com/927/ situation going
> on.
> 
> I think minimally we should have one way to do this.

Well, there is no one way atm. All you can do is hand roll all the
boilerplate (and likely get it slightly wrong) if you don't want
lock_all.

The current macros only help with lock_all, and IMO the hidden gotos
are even uglier than a hidden for loop. Fernando already hit a case
where he couldn't use the macros twice due to conflicting goto
labels. With this for loop thing I think it would have just worked(tm).

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
@ 2021-10-04 11:15       ` Ville Syrjälä
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2021-10-04 11:15 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel, Sean Paul, Fernando Ramos

On Tue, Jul 20, 2021 at 03:44:49PM +0200, Daniel Vetter wrote:
> On Thu, Jul 15, 2021 at 09:49:51PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Quite a few places are hand rolling the modeset lock backoff dance.
> > Let's suck that into a helper macro that is easier to use without
> > forgetting some steps.
> > 
> > The main downside is probably that the implementation of
> > drm_with_modeset_lock_ctx() is a bit harder to read than a hand
> > rolled version on account of being split across three functions,
> > but the actual code using it ends up being much simpler.
> > 
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
> >  include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
> >  2 files changed, 64 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> > index fcfe1a03c4a1..083df96632e8 100644
> > --- a/drivers/gpu/drm/drm_modeset_lock.c
> > +++ b/drivers/gpu/drm/drm_modeset_lock.c
> > @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
> > +
> > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > +			     struct drm_atomic_state *state,
> > +			     unsigned int flags, int *ret)
> > +{
> > +	drm_modeset_acquire_init(ctx, flags);
> > +
> > +	if (state)
> > +		state->acquire_ctx = ctx;
> > +
> > +	*ret = -EDEADLK;
> > +}
> > +EXPORT_SYMBOL(_drm_modeset_lock_begin);
> > +
> > +bool _drm_modeset_lock_loop(int *ret)
> > +{
> > +	if (*ret == -EDEADLK) {
> > +		*ret = 0;
> > +		return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +EXPORT_SYMBOL(_drm_modeset_lock_loop);
> > +
> > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > +			   struct drm_atomic_state *state,
> > +			   int *ret)
> > +{
> > +	if (*ret == -EDEADLK) {
> > +		if (state)
> > +			drm_atomic_state_clear(state);
> > +
> > +		*ret = drm_modeset_backoff(ctx);
> > +		if (*ret == 0) {
> > +			*ret = -EDEADLK;
> > +			return;
> > +		}
> > +	}
> > +
> > +	drm_modeset_drop_locks(ctx);
> > +	drm_modeset_acquire_fini(ctx);
> > +}
> > +EXPORT_SYMBOL(_drm_modeset_lock_end);
> > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> > index aafd07388eb7..5eaad2533de5 100644
> > --- a/include/drm/drm_modeset_lock.h
> > +++ b/include/drm/drm_modeset_lock.h
> > @@ -26,6 +26,7 @@
> >  
> >  #include <linux/ww_mutex.h>
> >  
> > +struct drm_atomic_state;
> >  struct drm_modeset_lock;
> >  
> >  /**
> > @@ -203,4 +204,23 @@ modeset_lock_fail:							\
> >  	if (!drm_drv_uses_atomic_modeset(dev))				\
> >  		mutex_unlock(&dev->mode_config.mutex);
> >  
> > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > +			     struct drm_atomic_state *state,
> > +			     unsigned int flags,
> > +			     int *ret);
> > +bool _drm_modeset_lock_loop(int *ret);
> > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > +			   struct drm_atomic_state *state,
> > +			   int *ret);
> > +
> > +/*
> > + * Note that one must always use "continue" rather than
> > + * "break" or "return" to handle errors within the
> > + * drm_modeset_lock_ctx_retry() block.
> 
> I'm not sold on loop macros with these kind of restrictions, C just isn't
> a great language for these. That's why e.g. drm_connector_iter doesn't
> give you a macro, but only the begin/next/end function calls explicitly.

We already use this pattern extensively in i915. Gem ww ctx has one,
power domains/pps/etc. use a similar things. It makes the code pretty nice,
with the slight caveat that an accidental 'break' can ruin your day. But
so can an accidental return with other constructs (and we even had that
happen a few times with the connector iterators), so not a dealbreaker
IMO.

So if we don't want this drm wide I guess I can propose this just for
i915 since it fits in perfectly there.

> 
> Yes the macro we have is also not nice, but at least it's a screaming
> macro since it's all uppercase, so options are all a bit sucky. Which
> leads me to think we have a bit a https://xkcd.com/927/ situation going
> on.
> 
> I think minimally we should have one way to do this.

Well, there is no one way atm. All you can do is hand roll all the
boilerplate (and likely get it slightly wrong) if you don't want
lock_all.

The current macros only help with lock_all, and IMO the hidden gotos
are even uglier than a hidden for loop. Fernando already hit a case
where he couldn't use the macros twice due to conflicting goto
labels. With this for loop thing I think it would have just worked(tm).

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
  2021-10-04 11:15       ` [Intel-gfx] " Ville Syrjälä
@ 2021-10-13 11:59         ` Daniel Vetter
  -1 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2021-10-13 11:59 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Daniel Vetter, intel-gfx, dri-devel, Sean Paul, Fernando Ramos

On Mon, Oct 04, 2021 at 02:15:51PM +0300, Ville Syrjälä wrote:
> On Tue, Jul 20, 2021 at 03:44:49PM +0200, Daniel Vetter wrote:
> > On Thu, Jul 15, 2021 at 09:49:51PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Quite a few places are hand rolling the modeset lock backoff dance.
> > > Let's suck that into a helper macro that is easier to use without
> > > forgetting some steps.
> > > 
> > > The main downside is probably that the implementation of
> > > drm_with_modeset_lock_ctx() is a bit harder to read than a hand
> > > rolled version on account of being split across three functions,
> > > but the actual code using it ends up being much simpler.
> > > 
> > > Cc: Sean Paul <seanpaul@chromium.org>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
> > >  include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
> > >  2 files changed, 64 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> > > index fcfe1a03c4a1..083df96632e8 100644
> > > --- a/drivers/gpu/drm/drm_modeset_lock.c
> > > +++ b/drivers/gpu/drm/drm_modeset_lock.c
> > > @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
> > >  	return 0;
> > >  }
> > >  EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
> > > +
> > > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > > +			     struct drm_atomic_state *state,
> > > +			     unsigned int flags, int *ret)
> > > +{
> > > +	drm_modeset_acquire_init(ctx, flags);
> > > +
> > > +	if (state)
> > > +		state->acquire_ctx = ctx;
> > > +
> > > +	*ret = -EDEADLK;
> > > +}
> > > +EXPORT_SYMBOL(_drm_modeset_lock_begin);
> > > +
> > > +bool _drm_modeset_lock_loop(int *ret)
> > > +{
> > > +	if (*ret == -EDEADLK) {
> > > +		*ret = 0;
> > > +		return true;
> > > +	}
> > > +
> > > +	return false;
> > > +}
> > > +EXPORT_SYMBOL(_drm_modeset_lock_loop);
> > > +
> > > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > > +			   struct drm_atomic_state *state,
> > > +			   int *ret)
> > > +{
> > > +	if (*ret == -EDEADLK) {
> > > +		if (state)
> > > +			drm_atomic_state_clear(state);
> > > +
> > > +		*ret = drm_modeset_backoff(ctx);
> > > +		if (*ret == 0) {
> > > +			*ret = -EDEADLK;
> > > +			return;
> > > +		}
> > > +	}
> > > +
> > > +	drm_modeset_drop_locks(ctx);
> > > +	drm_modeset_acquire_fini(ctx);
> > > +}
> > > +EXPORT_SYMBOL(_drm_modeset_lock_end);
> > > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> > > index aafd07388eb7..5eaad2533de5 100644
> > > --- a/include/drm/drm_modeset_lock.h
> > > +++ b/include/drm/drm_modeset_lock.h
> > > @@ -26,6 +26,7 @@
> > >  
> > >  #include <linux/ww_mutex.h>
> > >  
> > > +struct drm_atomic_state;
> > >  struct drm_modeset_lock;
> > >  
> > >  /**
> > > @@ -203,4 +204,23 @@ modeset_lock_fail:							\
> > >  	if (!drm_drv_uses_atomic_modeset(dev))				\
> > >  		mutex_unlock(&dev->mode_config.mutex);
> > >  
> > > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > > +			     struct drm_atomic_state *state,
> > > +			     unsigned int flags,
> > > +			     int *ret);
> > > +bool _drm_modeset_lock_loop(int *ret);
> > > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > > +			   struct drm_atomic_state *state,
> > > +			   int *ret);
> > > +
> > > +/*
> > > + * Note that one must always use "continue" rather than
> > > + * "break" or "return" to handle errors within the
> > > + * drm_modeset_lock_ctx_retry() block.
> > 
> > I'm not sold on loop macros with these kind of restrictions, C just isn't
> > a great language for these. That's why e.g. drm_connector_iter doesn't
> > give you a macro, but only the begin/next/end function calls explicitly.
> 
> We already use this pattern extensively in i915. Gem ww ctx has one,
> power domains/pps/etc. use a similar things. It makes the code pretty nice,
> with the slight caveat that an accidental 'break' can ruin your day. But
> so can an accidental return with other constructs (and we even had that
> happen a few times with the connector iterators), so not a dealbreaker
> IMO.
> 
> So if we don't want this drm wide I guess I can propose this just for
> i915 since it fits in perfectly there.

Well I don't like them for i915 either.

And yes C is dangerous, but also C is verbose. I think one lesson from igt
is that too many magic block constructs are bad, it's just not how C
works. Definitely not in the kernel, where "oops I got it wrong because it
was too clever" is bad.

> > Yes the macro we have is also not nice, but at least it's a screaming
> > macro since it's all uppercase, so options are all a bit sucky. Which
> > leads me to think we have a bit a https://xkcd.com/927/ situation going
> > on.
> > 
> > I think minimally we should have one way to do this.
> 
> Well, there is no one way atm. All you can do is hand roll all the
> boilerplate (and likely get it slightly wrong) if you don't want
> lock_all.
> 
> The current macros only help with lock_all, and IMO the hidden gotos
> are even uglier than a hidden for loop. Fernando already hit a case
> where he couldn't use the macros twice due to conflicting goto
> labels. With this for loop thing I think it would have just worked(tm).

I'm totally ok with repainting the shed, I just don't want some 80s
multicolor flash show.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [Intel-gfx] [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
@ 2021-10-13 11:59         ` Daniel Vetter
  0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2021-10-13 11:59 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Daniel Vetter, intel-gfx, dri-devel, Sean Paul, Fernando Ramos

On Mon, Oct 04, 2021 at 02:15:51PM +0300, Ville Syrjälä wrote:
> On Tue, Jul 20, 2021 at 03:44:49PM +0200, Daniel Vetter wrote:
> > On Thu, Jul 15, 2021 at 09:49:51PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Quite a few places are hand rolling the modeset lock backoff dance.
> > > Let's suck that into a helper macro that is easier to use without
> > > forgetting some steps.
> > > 
> > > The main downside is probably that the implementation of
> > > drm_with_modeset_lock_ctx() is a bit harder to read than a hand
> > > rolled version on account of being split across three functions,
> > > but the actual code using it ends up being much simpler.
> > > 
> > > Cc: Sean Paul <seanpaul@chromium.org>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
> > >  include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
> > >  2 files changed, 64 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> > > index fcfe1a03c4a1..083df96632e8 100644
> > > --- a/drivers/gpu/drm/drm_modeset_lock.c
> > > +++ b/drivers/gpu/drm/drm_modeset_lock.c
> > > @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
> > >  	return 0;
> > >  }
> > >  EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
> > > +
> > > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > > +			     struct drm_atomic_state *state,
> > > +			     unsigned int flags, int *ret)
> > > +{
> > > +	drm_modeset_acquire_init(ctx, flags);
> > > +
> > > +	if (state)
> > > +		state->acquire_ctx = ctx;
> > > +
> > > +	*ret = -EDEADLK;
> > > +}
> > > +EXPORT_SYMBOL(_drm_modeset_lock_begin);
> > > +
> > > +bool _drm_modeset_lock_loop(int *ret)
> > > +{
> > > +	if (*ret == -EDEADLK) {
> > > +		*ret = 0;
> > > +		return true;
> > > +	}
> > > +
> > > +	return false;
> > > +}
> > > +EXPORT_SYMBOL(_drm_modeset_lock_loop);
> > > +
> > > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > > +			   struct drm_atomic_state *state,
> > > +			   int *ret)
> > > +{
> > > +	if (*ret == -EDEADLK) {
> > > +		if (state)
> > > +			drm_atomic_state_clear(state);
> > > +
> > > +		*ret = drm_modeset_backoff(ctx);
> > > +		if (*ret == 0) {
> > > +			*ret = -EDEADLK;
> > > +			return;
> > > +		}
> > > +	}
> > > +
> > > +	drm_modeset_drop_locks(ctx);
> > > +	drm_modeset_acquire_fini(ctx);
> > > +}
> > > +EXPORT_SYMBOL(_drm_modeset_lock_end);
> > > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> > > index aafd07388eb7..5eaad2533de5 100644
> > > --- a/include/drm/drm_modeset_lock.h
> > > +++ b/include/drm/drm_modeset_lock.h
> > > @@ -26,6 +26,7 @@
> > >  
> > >  #include <linux/ww_mutex.h>
> > >  
> > > +struct drm_atomic_state;
> > >  struct drm_modeset_lock;
> > >  
> > >  /**
> > > @@ -203,4 +204,23 @@ modeset_lock_fail:							\
> > >  	if (!drm_drv_uses_atomic_modeset(dev))				\
> > >  		mutex_unlock(&dev->mode_config.mutex);
> > >  
> > > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > > +			     struct drm_atomic_state *state,
> > > +			     unsigned int flags,
> > > +			     int *ret);
> > > +bool _drm_modeset_lock_loop(int *ret);
> > > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > > +			   struct drm_atomic_state *state,
> > > +			   int *ret);
> > > +
> > > +/*
> > > + * Note that one must always use "continue" rather than
> > > + * "break" or "return" to handle errors within the
> > > + * drm_modeset_lock_ctx_retry() block.
> > 
> > I'm not sold on loop macros with these kind of restrictions, C just isn't
> > a great language for these. That's why e.g. drm_connector_iter doesn't
> > give you a macro, but only the begin/next/end function calls explicitly.
> 
> We already use this pattern extensively in i915. Gem ww ctx has one,
> power domains/pps/etc. use a similar things. It makes the code pretty nice,
> with the slight caveat that an accidental 'break' can ruin your day. But
> so can an accidental return with other constructs (and we even had that
> happen a few times with the connector iterators), so not a dealbreaker
> IMO.
> 
> So if we don't want this drm wide I guess I can propose this just for
> i915 since it fits in perfectly there.

Well I don't like them for i915 either.

And yes C is dangerous, but also C is verbose. I think one lesson from igt
is that too many magic block constructs are bad, it's just not how C
works. Definitely not in the kernel, where "oops I got it wrong because it
was too clever" is bad.

> > Yes the macro we have is also not nice, but at least it's a screaming
> > macro since it's all uppercase, so options are all a bit sucky. Which
> > leads me to think we have a bit a https://xkcd.com/927/ situation going
> > on.
> > 
> > I think minimally we should have one way to do this.
> 
> Well, there is no one way atm. All you can do is hand roll all the
> boilerplate (and likely get it slightly wrong) if you don't want
> lock_all.
> 
> The current macros only help with lock_all, and IMO the hidden gotos
> are even uglier than a hidden for loop. Fernando already hit a case
> where he couldn't use the macros twice due to conflicting goto
> labels. With this for loop thing I think it would have just worked(tm).

I'm totally ok with repainting the shed, I just don't want some 80s
multicolor flash show.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
  2021-10-13 11:59         ` [Intel-gfx] " Daniel Vetter
@ 2021-10-13 12:06           ` Ville Syrjälä
  -1 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2021-10-13 12:06 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel, Sean Paul, Fernando Ramos

On Wed, Oct 13, 2021 at 01:59:47PM +0200, Daniel Vetter wrote:
> On Mon, Oct 04, 2021 at 02:15:51PM +0300, Ville Syrjälä wrote:
> > On Tue, Jul 20, 2021 at 03:44:49PM +0200, Daniel Vetter wrote:
> > > On Thu, Jul 15, 2021 at 09:49:51PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Quite a few places are hand rolling the modeset lock backoff dance.
> > > > Let's suck that into a helper macro that is easier to use without
> > > > forgetting some steps.
> > > > 
> > > > The main downside is probably that the implementation of
> > > > drm_with_modeset_lock_ctx() is a bit harder to read than a hand
> > > > rolled version on account of being split across three functions,
> > > > but the actual code using it ends up being much simpler.
> > > > 
> > > > Cc: Sean Paul <seanpaul@chromium.org>
> > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
> > > >  include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
> > > >  2 files changed, 64 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> > > > index fcfe1a03c4a1..083df96632e8 100644
> > > > --- a/drivers/gpu/drm/drm_modeset_lock.c
> > > > +++ b/drivers/gpu/drm/drm_modeset_lock.c
> > > > @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
> > > >  	return 0;
> > > >  }
> > > >  EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
> > > > +
> > > > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > > > +			     struct drm_atomic_state *state,
> > > > +			     unsigned int flags, int *ret)
> > > > +{
> > > > +	drm_modeset_acquire_init(ctx, flags);
> > > > +
> > > > +	if (state)
> > > > +		state->acquire_ctx = ctx;
> > > > +
> > > > +	*ret = -EDEADLK;
> > > > +}
> > > > +EXPORT_SYMBOL(_drm_modeset_lock_begin);
> > > > +
> > > > +bool _drm_modeset_lock_loop(int *ret)
> > > > +{
> > > > +	if (*ret == -EDEADLK) {
> > > > +		*ret = 0;
> > > > +		return true;
> > > > +	}
> > > > +
> > > > +	return false;
> > > > +}
> > > > +EXPORT_SYMBOL(_drm_modeset_lock_loop);
> > > > +
> > > > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > > > +			   struct drm_atomic_state *state,
> > > > +			   int *ret)
> > > > +{
> > > > +	if (*ret == -EDEADLK) {
> > > > +		if (state)
> > > > +			drm_atomic_state_clear(state);
> > > > +
> > > > +		*ret = drm_modeset_backoff(ctx);
> > > > +		if (*ret == 0) {
> > > > +			*ret = -EDEADLK;
> > > > +			return;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	drm_modeset_drop_locks(ctx);
> > > > +	drm_modeset_acquire_fini(ctx);
> > > > +}
> > > > +EXPORT_SYMBOL(_drm_modeset_lock_end);
> > > > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> > > > index aafd07388eb7..5eaad2533de5 100644
> > > > --- a/include/drm/drm_modeset_lock.h
> > > > +++ b/include/drm/drm_modeset_lock.h
> > > > @@ -26,6 +26,7 @@
> > > >  
> > > >  #include <linux/ww_mutex.h>
> > > >  
> > > > +struct drm_atomic_state;
> > > >  struct drm_modeset_lock;
> > > >  
> > > >  /**
> > > > @@ -203,4 +204,23 @@ modeset_lock_fail:							\
> > > >  	if (!drm_drv_uses_atomic_modeset(dev))				\
> > > >  		mutex_unlock(&dev->mode_config.mutex);
> > > >  
> > > > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > > > +			     struct drm_atomic_state *state,
> > > > +			     unsigned int flags,
> > > > +			     int *ret);
> > > > +bool _drm_modeset_lock_loop(int *ret);
> > > > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > > > +			   struct drm_atomic_state *state,
> > > > +			   int *ret);
> > > > +
> > > > +/*
> > > > + * Note that one must always use "continue" rather than
> > > > + * "break" or "return" to handle errors within the
> > > > + * drm_modeset_lock_ctx_retry() block.
> > > 
> > > I'm not sold on loop macros with these kind of restrictions, C just isn't
> > > a great language for these. That's why e.g. drm_connector_iter doesn't
> > > give you a macro, but only the begin/next/end function calls explicitly.
> > 
> > We already use this pattern extensively in i915. Gem ww ctx has one,
> > power domains/pps/etc. use a similar things. It makes the code pretty nice,
> > with the slight caveat that an accidental 'break' can ruin your day. But
> > so can an accidental return with other constructs (and we even had that
> > happen a few times with the connector iterators), so not a dealbreaker
> > IMO.
> > 
> > So if we don't want this drm wide I guess I can propose this just for
> > i915 since it fits in perfectly there.
> 
> Well I don't like them for i915 either.

I think you're a bit alone in that. Most people seem pretty happy
with this style since it makes the code very readable.

> 
> And yes C is dangerous, but also C is verbose. I think one lesson from igt
> is that too many magic block constructs are bad, it's just not how C
> works. Definitely not in the kernel, where "oops I got it wrong because it
> was too clever" is bad.
> 
> > > Yes the macro we have is also not nice, but at least it's a screaming
> > > macro since it's all uppercase, so options are all a bit sucky. Which
> > > leads me to think we have a bit a https://xkcd.com/927/ situation going
> > > on.
> > > 
> > > I think minimally we should have one way to do this.
> > 
> > Well, there is no one way atm. All you can do is hand roll all the
> > boilerplate (and likely get it slightly wrong) if you don't want
> > lock_all.
> > 
> > The current macros only help with lock_all, and IMO the hidden gotos
> > are even uglier than a hidden for loop. Fernando already hit a case
> > where he couldn't use the macros twice due to conflicting goto
> > labels. With this for loop thing I think it would have just worked(tm).
> 
> I'm totally ok with repainting the shed, I just don't want some 80s
> multicolor flash show.

You have a better idea in mind?

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
@ 2021-10-13 12:06           ` Ville Syrjälä
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2021-10-13 12:06 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel, Sean Paul, Fernando Ramos

On Wed, Oct 13, 2021 at 01:59:47PM +0200, Daniel Vetter wrote:
> On Mon, Oct 04, 2021 at 02:15:51PM +0300, Ville Syrjälä wrote:
> > On Tue, Jul 20, 2021 at 03:44:49PM +0200, Daniel Vetter wrote:
> > > On Thu, Jul 15, 2021 at 09:49:51PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Quite a few places are hand rolling the modeset lock backoff dance.
> > > > Let's suck that into a helper macro that is easier to use without
> > > > forgetting some steps.
> > > > 
> > > > The main downside is probably that the implementation of
> > > > drm_with_modeset_lock_ctx() is a bit harder to read than a hand
> > > > rolled version on account of being split across three functions,
> > > > but the actual code using it ends up being much simpler.
> > > > 
> > > > Cc: Sean Paul <seanpaul@chromium.org>
> > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_modeset_lock.c | 44 ++++++++++++++++++++++++++++++
> > > >  include/drm/drm_modeset_lock.h     | 20 ++++++++++++++
> > > >  2 files changed, 64 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> > > > index fcfe1a03c4a1..083df96632e8 100644
> > > > --- a/drivers/gpu/drm/drm_modeset_lock.c
> > > > +++ b/drivers/gpu/drm/drm_modeset_lock.c
> > > > @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
> > > >  	return 0;
> > > >  }
> > > >  EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
> > > > +
> > > > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > > > +			     struct drm_atomic_state *state,
> > > > +			     unsigned int flags, int *ret)
> > > > +{
> > > > +	drm_modeset_acquire_init(ctx, flags);
> > > > +
> > > > +	if (state)
> > > > +		state->acquire_ctx = ctx;
> > > > +
> > > > +	*ret = -EDEADLK;
> > > > +}
> > > > +EXPORT_SYMBOL(_drm_modeset_lock_begin);
> > > > +
> > > > +bool _drm_modeset_lock_loop(int *ret)
> > > > +{
> > > > +	if (*ret == -EDEADLK) {
> > > > +		*ret = 0;
> > > > +		return true;
> > > > +	}
> > > > +
> > > > +	return false;
> > > > +}
> > > > +EXPORT_SYMBOL(_drm_modeset_lock_loop);
> > > > +
> > > > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > > > +			   struct drm_atomic_state *state,
> > > > +			   int *ret)
> > > > +{
> > > > +	if (*ret == -EDEADLK) {
> > > > +		if (state)
> > > > +			drm_atomic_state_clear(state);
> > > > +
> > > > +		*ret = drm_modeset_backoff(ctx);
> > > > +		if (*ret == 0) {
> > > > +			*ret = -EDEADLK;
> > > > +			return;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	drm_modeset_drop_locks(ctx);
> > > > +	drm_modeset_acquire_fini(ctx);
> > > > +}
> > > > +EXPORT_SYMBOL(_drm_modeset_lock_end);
> > > > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
> > > > index aafd07388eb7..5eaad2533de5 100644
> > > > --- a/include/drm/drm_modeset_lock.h
> > > > +++ b/include/drm/drm_modeset_lock.h
> > > > @@ -26,6 +26,7 @@
> > > >  
> > > >  #include <linux/ww_mutex.h>
> > > >  
> > > > +struct drm_atomic_state;
> > > >  struct drm_modeset_lock;
> > > >  
> > > >  /**
> > > > @@ -203,4 +204,23 @@ modeset_lock_fail:							\
> > > >  	if (!drm_drv_uses_atomic_modeset(dev))				\
> > > >  		mutex_unlock(&dev->mode_config.mutex);
> > > >  
> > > > +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
> > > > +			     struct drm_atomic_state *state,
> > > > +			     unsigned int flags,
> > > > +			     int *ret);
> > > > +bool _drm_modeset_lock_loop(int *ret);
> > > > +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
> > > > +			   struct drm_atomic_state *state,
> > > > +			   int *ret);
> > > > +
> > > > +/*
> > > > + * Note that one must always use "continue" rather than
> > > > + * "break" or "return" to handle errors within the
> > > > + * drm_modeset_lock_ctx_retry() block.
> > > 
> > > I'm not sold on loop macros with these kind of restrictions, C just isn't
> > > a great language for these. That's why e.g. drm_connector_iter doesn't
> > > give you a macro, but only the begin/next/end function calls explicitly.
> > 
> > We already use this pattern extensively in i915. Gem ww ctx has one,
> > power domains/pps/etc. use a similar things. It makes the code pretty nice,
> > with the slight caveat that an accidental 'break' can ruin your day. But
> > so can an accidental return with other constructs (and we even had that
> > happen a few times with the connector iterators), so not a dealbreaker
> > IMO.
> > 
> > So if we don't want this drm wide I guess I can propose this just for
> > i915 since it fits in perfectly there.
> 
> Well I don't like them for i915 either.

I think you're a bit alone in that. Most people seem pretty happy
with this style since it makes the code very readable.

> 
> And yes C is dangerous, but also C is verbose. I think one lesson from igt
> is that too many magic block constructs are bad, it's just not how C
> works. Definitely not in the kernel, where "oops I got it wrong because it
> was too clever" is bad.
> 
> > > Yes the macro we have is also not nice, but at least it's a screaming
> > > macro since it's all uppercase, so options are all a bit sucky. Which
> > > leads me to think we have a bit a https://xkcd.com/927/ situation going
> > > on.
> > > 
> > > I think minimally we should have one way to do this.
> > 
> > Well, there is no one way atm. All you can do is hand roll all the
> > boilerplate (and likely get it slightly wrong) if you don't want
> > lock_all.
> > 
> > The current macros only help with lock_all, and IMO the hidden gotos
> > are even uglier than a hidden for loop. Fernando already hit a case
> > where he couldn't use the macros twice due to conflicting goto
> > labels. With this for loop thing I think it would have just worked(tm).
> 
> I'm totally ok with repainting the shed, I just don't want some 80s
> multicolor flash show.

You have a better idea in mind?

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
  2021-10-13 12:06           ` [Intel-gfx] " Ville Syrjälä
@ 2021-10-13 21:00             ` Fernando Ramos
  -1 siblings, 0 replies; 27+ messages in thread
From: Fernando Ramos @ 2021-10-13 21:00 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx, dri-devel, Sean Paul

On 21/10/13 03:06PM, Ville Syrjälä wrote:
> > And yes C is dangerous, but also C is verbose. I think one lesson from igt
> > is that too many magic block constructs are bad, it's just not how C
> > works. Definitely not in the kernel, where "oops I got it wrong because it
> > was too clever" is bad.
> > 
> > > > Yes the macro we have is also not nice, but at least it's a screaming
> > > > macro since it's all uppercase, so options are all a bit sucky. Which
> > > > leads me to think we have a bit a https://xkcd.com/927/ situation going
> > > > on.
> > > > 
> > > > I think minimally we should have one way to do this.
> > > 
> > > Well, there is no one way atm. All you can do is hand roll all the
> > > boilerplate (and likely get it slightly wrong) if you don't want
> > > lock_all.
> > > 
> > > The current macros only help with lock_all, and IMO the hidden gotos
> > > are even uglier than a hidden for loop. Fernando already hit a case
> > > where he couldn't use the macros twice due to conflicting goto
> > > labels. With this for loop thing I think it would have just worked(tm).
> > 
> > I'm totally ok with repainting the shed, I just don't want some 80s
> > multicolor flash show.
> 
> You have a better idea in mind?

Sorry, I completely forgot this discussion was going on and I just published V4
of my patch set here:

    https://lore.kernel.org/dri-devel/20211013204846.90026-1-greenfoo@u92.eu/

Please, feel free to let me know (ideally, as a reply to the corresponding i915
patch from that set) if you rather me not to modify i915 files for now.

Thanks.

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

* Re: [Intel-gfx] [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
@ 2021-10-13 21:00             ` Fernando Ramos
  0 siblings, 0 replies; 27+ messages in thread
From: Fernando Ramos @ 2021-10-13 21:00 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx, dri-devel, Sean Paul

On 21/10/13 03:06PM, Ville Syrjälä wrote:
> > And yes C is dangerous, but also C is verbose. I think one lesson from igt
> > is that too many magic block constructs are bad, it's just not how C
> > works. Definitely not in the kernel, where "oops I got it wrong because it
> > was too clever" is bad.
> > 
> > > > Yes the macro we have is also not nice, but at least it's a screaming
> > > > macro since it's all uppercase, so options are all a bit sucky. Which
> > > > leads me to think we have a bit a https://xkcd.com/927/ situation going
> > > > on.
> > > > 
> > > > I think minimally we should have one way to do this.
> > > 
> > > Well, there is no one way atm. All you can do is hand roll all the
> > > boilerplate (and likely get it slightly wrong) if you don't want
> > > lock_all.
> > > 
> > > The current macros only help with lock_all, and IMO the hidden gotos
> > > are even uglier than a hidden for loop. Fernando already hit a case
> > > where he couldn't use the macros twice due to conflicting goto
> > > labels. With this for loop thing I think it would have just worked(tm).
> > 
> > I'm totally ok with repainting the shed, I just don't want some 80s
> > multicolor flash show.
> 
> You have a better idea in mind?

Sorry, I completely forgot this discussion was going on and I just published V4
of my patch set here:

    https://lore.kernel.org/dri-devel/20211013204846.90026-1-greenfoo@u92.eu/

Please, feel free to let me know (ideally, as a reply to the corresponding i915
patch from that set) if you rather me not to modify i915 files for now.

Thanks.

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

* Re: [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
  2021-10-13 21:00             ` [Intel-gfx] " Fernando Ramos
@ 2021-10-14 13:23               ` Daniel Vetter
  -1 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2021-10-14 13:23 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: Ville Syrjälä, Daniel Vetter, intel-gfx, dri-devel, Sean Paul

On Wed, Oct 13, 2021 at 11:00:35PM +0200, Fernando Ramos wrote:
> On 21/10/13 03:06PM, Ville Syrjälä wrote:
> > > And yes C is dangerous, but also C is verbose. I think one lesson from igt
> > > is that too many magic block constructs are bad, it's just not how C
> > > works. Definitely not in the kernel, where "oops I got it wrong because it
> > > was too clever" is bad.
> > > 
> > > > > Yes the macro we have is also not nice, but at least it's a screaming
> > > > > macro since it's all uppercase, so options are all a bit sucky. Which
> > > > > leads me to think we have a bit a https://xkcd.com/927/ situation going
> > > > > on.
> > > > > 
> > > > > I think minimally we should have one way to do this.
> > > > 
> > > > Well, there is no one way atm. All you can do is hand roll all the
> > > > boilerplate (and likely get it slightly wrong) if you don't want
> > > > lock_all.
> > > > 
> > > > The current macros only help with lock_all, and IMO the hidden gotos
> > > > are even uglier than a hidden for loop. Fernando already hit a case
> > > > where he couldn't use the macros twice due to conflicting goto
> > > > labels. With this for loop thing I think it would have just worked(tm).
> > > 
> > > I'm totally ok with repainting the shed, I just don't want some 80s
> > > multicolor flash show.
> > 
> > You have a better idea in mind?
> 
> Sorry, I completely forgot this discussion was going on and I just published V4
> of my patch set here:
> 
>     https://lore.kernel.org/dri-devel/20211013204846.90026-1-greenfoo@u92.eu/
> 
> Please, feel free to let me know (ideally, as a reply to the corresponding i915
> patch from that set) if you rather me not to modify i915 files for now.

My request is that we only have one way of doing this drm_modeset lock
retry business. So either this one here proposed by Ville, or the one Sean
Paul merged.

I honestly don't care which color we pick, as long as it's consistent
across the board. Please all you, figure this out.

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

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

* Re: [Intel-gfx] [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
@ 2021-10-14 13:23               ` Daniel Vetter
  0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2021-10-14 13:23 UTC (permalink / raw)
  To: Fernando Ramos
  Cc: Ville Syrjälä, Daniel Vetter, intel-gfx, dri-devel, Sean Paul

On Wed, Oct 13, 2021 at 11:00:35PM +0200, Fernando Ramos wrote:
> On 21/10/13 03:06PM, Ville Syrjälä wrote:
> > > And yes C is dangerous, but also C is verbose. I think one lesson from igt
> > > is that too many magic block constructs are bad, it's just not how C
> > > works. Definitely not in the kernel, where "oops I got it wrong because it
> > > was too clever" is bad.
> > > 
> > > > > Yes the macro we have is also not nice, but at least it's a screaming
> > > > > macro since it's all uppercase, so options are all a bit sucky. Which
> > > > > leads me to think we have a bit a https://xkcd.com/927/ situation going
> > > > > on.
> > > > > 
> > > > > I think minimally we should have one way to do this.
> > > > 
> > > > Well, there is no one way atm. All you can do is hand roll all the
> > > > boilerplate (and likely get it slightly wrong) if you don't want
> > > > lock_all.
> > > > 
> > > > The current macros only help with lock_all, and IMO the hidden gotos
> > > > are even uglier than a hidden for loop. Fernando already hit a case
> > > > where he couldn't use the macros twice due to conflicting goto
> > > > labels. With this for loop thing I think it would have just worked(tm).
> > > 
> > > I'm totally ok with repainting the shed, I just don't want some 80s
> > > multicolor flash show.
> > 
> > You have a better idea in mind?
> 
> Sorry, I completely forgot this discussion was going on and I just published V4
> of my patch set here:
> 
>     https://lore.kernel.org/dri-devel/20211013204846.90026-1-greenfoo@u92.eu/
> 
> Please, feel free to let me know (ideally, as a reply to the corresponding i915
> patch from that set) if you rather me not to modify i915 files for now.

My request is that we only have one way of doing this drm_modeset lock
retry business. So either this one here proposed by Ville, or the one Sean
Paul merged.

I honestly don't care which color we pick, as long as it's consistent
across the board. Please all you, figure this out.

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

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

end of thread, other threads:[~2021-10-14 13:23 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 18:49 [PATCH 0/4] drm: Make modeset locking easier Ville Syrjala
2021-07-15 18:49 ` [Intel-gfx] " Ville Syrjala
2021-07-15 18:49 ` [PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry() Ville Syrjala
2021-07-15 18:49   ` [Intel-gfx] " Ville Syrjala
2021-07-20 13:44   ` Daniel Vetter
2021-07-20 13:44     ` [Intel-gfx] " Daniel Vetter
2021-10-04 11:15     ` Ville Syrjälä
2021-10-04 11:15       ` [Intel-gfx] " Ville Syrjälä
2021-10-13 11:59       ` Daniel Vetter
2021-10-13 11:59         ` [Intel-gfx] " Daniel Vetter
2021-10-13 12:06         ` Ville Syrjälä
2021-10-13 12:06           ` [Intel-gfx] " Ville Syrjälä
2021-10-13 21:00           ` Fernando Ramos
2021-10-13 21:00             ` [Intel-gfx] " Fernando Ramos
2021-10-14 13:23             ` Daniel Vetter
2021-10-14 13:23               ` [Intel-gfx] " Daniel Vetter
2021-07-15 18:49 ` [PATCH 2/4] drm: Introduce drm_modeset_lock_all_ctx_retry() Ville Syrjala
2021-07-15 18:49   ` [Intel-gfx] " Ville Syrjala
2021-07-15 18:49 ` [PATCH 3/4] drm/i915: Extract intel_crtc_initial_commit() Ville Syrjala
2021-07-15 18:49   ` [Intel-gfx] " Ville Syrjala
2021-07-15 18:49 ` [PATCH 4/4] drm/i915: Use drm_modeset_lock_ctx_retry() & co Ville Syrjala
2021-07-15 18:49   ` [Intel-gfx] " Ville Syrjala
2021-07-16 22:21 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Make modeset locking easier Patchwork
2021-07-16 22:24 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-07-16 22:27 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2021-07-16 22:50 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-07-17  7:20 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork

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.