All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups
@ 2021-04-14  2:23 Ville Syrjala
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 1/8] drm/i915: Add frontbuffer tracking tracepoints Ville Syrjala
                   ` (11 more replies)
  0 siblings, 12 replies; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

The FBC code is a bit of mess. Start cleaning it up a bit.
The main thing here is throwing out tons of redundant state
from the fbc_state_cache and just checkng that stuff ahead of
time from the plane/crtc states.

Ville Syrjälä (8):
  drm/i915: Add frontbuffer tracking tracepoints
  drm/i915: Rewrite the FBC tiling check a bit
  drm/i915: Extract intel_fbc_update()
  drm/i915: Clear no_fbc_reason on activate
  drm/i915: Move the "recompress on activate" to a central place
  drm/i915: Nuke lots of crap from intel_fbc_state_cache
  drm/i915: No FBC+double wide pipe
  drm/i915: Pimp the FBC debugfs output

 drivers/gpu/drm/i915/display/intel_display.c  |  10 +-
 .../drm/i915/display/intel_display_debugfs.c  |  50 ++-
 .../drm/i915/display/intel_display_types.h    |   2 +-
 drivers/gpu/drm/i915/display/intel_fbc.c      | 424 +++++++++---------
 drivers/gpu/drm/i915/display/intel_fbc.h      |   5 +-
 .../gpu/drm/i915/display/intel_frontbuffer.c  |   5 +
 drivers/gpu/drm/i915/i915_drv.h               |  21 +-
 drivers/gpu/drm/i915/i915_trace.h             |  38 ++
 8 files changed, 305 insertions(+), 250 deletions(-)

-- 
2.26.3

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

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

* [Intel-gfx] [PATCH 1/8] drm/i915: Add frontbuffer tracking tracepoints
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
@ 2021-04-14  2:23 ` Ville Syrjala
  2021-04-21 13:01   ` Jani Nikula
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit Ville Syrjala
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

Add some tracpoints for frontbuffer tracking so we can
try to figure out what's going on.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_frontbuffer.c  |  5 +++
 drivers/gpu/drm/i915/i915_trace.h             | 38 +++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 6fc6965b6133..8161d49e78ba 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -58,6 +58,7 @@
 #include "display/intel_dp.h"
 
 #include "i915_drv.h"
+#include "i915_trace.h"
 #include "intel_display_types.h"
 #include "intel_fbc.h"
 #include "intel_frontbuffer.h"
@@ -87,6 +88,8 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
 	if (!frontbuffer_bits)
 		return;
 
+	trace_intel_frontbuffer_flush(frontbuffer_bits, origin);
+
 	might_sleep();
 	intel_edp_drrs_flush(i915, frontbuffer_bits);
 	intel_psr_flush(i915, frontbuffer_bits, origin);
@@ -173,6 +176,8 @@ void __intel_fb_invalidate(struct intel_frontbuffer *front,
 		spin_unlock(&i915->fb_tracking.lock);
 	}
 
+	trace_intel_frontbuffer_invalidate(frontbuffer_bits, origin);
+
 	might_sleep();
 	intel_psr_invalidate(i915, frontbuffer_bits, origin);
 	intel_edp_drrs_invalidate(i915, frontbuffer_bits);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index a4addcc64978..81f5e1721180 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -474,6 +474,44 @@ TRACE_EVENT(intel_pipe_update_end,
 		      __entry->scanline)
 );
 
+/* frontbuffer tracking */
+
+TRACE_EVENT(intel_frontbuffer_invalidate,
+	    TP_PROTO(unsigned int frontbuffer_bits, unsigned int origin),
+	    TP_ARGS(frontbuffer_bits, origin),
+
+	    TP_STRUCT__entry(
+			     __field(unsigned int, frontbuffer_bits)
+			     __field(unsigned int, origin)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->frontbuffer_bits = frontbuffer_bits;
+			   __entry->origin = origin;
+			   ),
+
+	    TP_printk("frontbuffer_bits=0x%08x, origin=%u",
+		      __entry->frontbuffer_bits, __entry->origin)
+);
+
+TRACE_EVENT(intel_frontbuffer_flush,
+	    TP_PROTO(unsigned int frontbuffer_bits, unsigned int origin),
+	    TP_ARGS(frontbuffer_bits, origin),
+
+	    TP_STRUCT__entry(
+			     __field(unsigned int, frontbuffer_bits)
+			     __field(unsigned int, origin)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->frontbuffer_bits = frontbuffer_bits;
+			   __entry->origin = origin;
+			   ),
+
+	    TP_printk("frontbuffer_bits=0x%08x, origin=%u",
+		      __entry->frontbuffer_bits, __entry->origin)
+);
+
 /* object tracking */
 
 TRACE_EVENT(i915_gem_object_create,
-- 
2.26.3

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

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

* [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 1/8] drm/i915: Add frontbuffer tracking tracepoints Ville Syrjala
@ 2021-04-14  2:23 ` Ville Syrjala
  2021-04-14 15:09   ` Jani Nikula
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 3/8] drm/i915: Extract intel_fbc_update() Ville Syrjala
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

Write the tiling check in a nicer form.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 04d9c7d22b04..178243a6d3a2 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -681,11 +681,9 @@ static bool tiling_is_valid(struct drm_i915_private *dev_priv,
 {
 	switch (modifier) {
 	case DRM_FORMAT_MOD_LINEAR:
-		if (DISPLAY_VER(dev_priv) >= 9)
-			return true;
-		return false;
-	case I915_FORMAT_MOD_X_TILED:
 	case I915_FORMAT_MOD_Y_TILED:
+		return DISPLAY_VER(dev_priv) >= 9;
+	case I915_FORMAT_MOD_X_TILED:
 		return true;
 	default:
 		return false;
-- 
2.26.3

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

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

* [Intel-gfx] [PATCH 3/8] drm/i915: Extract intel_fbc_update()
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 1/8] drm/i915: Add frontbuffer tracking tracepoints Ville Syrjala
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit Ville Syrjala
@ 2021-04-14  2:23 ` Ville Syrjala
  2021-04-21 13:00   ` Jani Nikula
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 4/8] drm/i915: Clear no_fbc_reason on activate Ville Syrjala
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

Pull the fbc enable vs. disable stuff into a small helper so
we don't have to have it pollute the higher level modeset code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  5 +---
 drivers/gpu/drm/i915/display/intel_fbc.c     | 26 ++++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_fbc.h     |  2 +-
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 411b46c012f8..a4b8fb5c20f0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9799,10 +9799,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
 			intel_encoders_update_pipe(state, crtc);
 	}
 
-	if (new_crtc_state->update_pipe && !new_crtc_state->enable_fbc)
-		intel_fbc_disable(crtc);
-	else
-		intel_fbc_enable(state, crtc);
+	intel_fbc_update(state, crtc);
 
 	/* Perform vblank evasion around commit operation */
 	intel_pipe_update_start(new_crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 178243a6d3a2..4968e79a6235 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1250,8 +1250,8 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
  * intel_fbc_enable multiple times for the same pipe without an
  * intel_fbc_disable in the middle, as long as it is deactivated.
  */
-void intel_fbc_enable(struct intel_atomic_state *state,
-		      struct intel_crtc *crtc)
+static void intel_fbc_enable(struct intel_atomic_state *state,
+			     struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
@@ -1324,6 +1324,28 @@ void intel_fbc_disable(struct intel_crtc *crtc)
 	mutex_unlock(&fbc->lock);
 }
 
+/**
+ * intel_fbc_update: enable/disable FBC on the CRTC
+ * @state: atomic state
+ * @crtc: the CRTC
+ *
+ * This function checks if the given CRTC was chosen for FBC, then enables it if
+ * possible. Notice that it doesn't activate FBC. It is valid to call
+ * intel_fbc_update multiple times for the same pipe without an
+ * intel_fbc_disable in the middle.
+ */
+void intel_fbc_update(struct intel_atomic_state *state,
+		      struct intel_crtc *crtc)
+{
+	const struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+
+	if (crtc_state->update_pipe && !crtc_state->enable_fbc)
+		intel_fbc_disable(crtc);
+	else
+		intel_fbc_enable(state, crtc);
+}
+
 /**
  * intel_fbc_global_disable - globally disable FBC
  * @dev_priv: i915 device instance
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h
index 6dc1edefe81b..b97d908738e6 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.h
+++ b/drivers/gpu/drm/i915/display/intel_fbc.h
@@ -24,7 +24,7 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state,
 void intel_fbc_post_update(struct intel_atomic_state *state,
 			   struct intel_crtc *crtc);
 void intel_fbc_init(struct drm_i915_private *dev_priv);
-void intel_fbc_enable(struct intel_atomic_state *state,
+void intel_fbc_update(struct intel_atomic_state *state,
 		      struct intel_crtc *crtc);
 void intel_fbc_disable(struct intel_crtc *crtc);
 void intel_fbc_global_disable(struct drm_i915_private *dev_priv);
-- 
2.26.3

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

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

* [Intel-gfx] [PATCH 4/8] drm/i915: Clear no_fbc_reason on activate
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (2 preceding siblings ...)
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 3/8] drm/i915: Extract intel_fbc_update() Ville Syrjala
@ 2021-04-14  2:23 ` Ville Syrjala
  2021-04-21 13:48   ` Jani Nikula
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 5/8] drm/i915: Move the "recompress on activate" to a central place Ville Syrjala
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

We try to set no_fbc_reason when FBC is not possible, let's
consistently clear when activating FBC.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 4968e79a6235..fb8c0872a2b7 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -411,6 +411,17 @@ bool intel_fbc_is_active(struct drm_i915_private *dev_priv)
 	return dev_priv->fbc.active;
 }
 
+static void intel_fbc_activate(struct drm_i915_private *dev_priv)
+{
+	struct intel_fbc *fbc = &dev_priv->fbc;
+
+	drm_WARN_ON(&dev_priv->drm, !mutex_is_locked(&fbc->lock));
+
+	intel_fbc_hw_activate(dev_priv);
+
+	fbc->no_fbc_reason = NULL;
+}
+
 static void intel_fbc_deactivate(struct drm_i915_private *dev_priv,
 				 const char *reason)
 {
@@ -1094,7 +1105,7 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc)
 		return;
 
 	if (!fbc->busy_bits)
-		intel_fbc_hw_activate(dev_priv);
+		intel_fbc_activate(dev_priv);
 	else
 		intel_fbc_deactivate(dev_priv, "frontbuffer write");
 }
-- 
2.26.3

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

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

* [Intel-gfx] [PATCH 5/8] drm/i915: Move the "recompress on activate" to a central place
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (3 preceding siblings ...)
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 4/8] drm/i915: Clear no_fbc_reason on activate Ville Syrjala
@ 2021-04-14  2:23 ` Ville Syrjala
  2021-04-21 13:17   ` Jani Nikula
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 6/8] drm/i915: Nuke lots of crap from intel_fbc_state_cache Ville Syrjala
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

On ILK+ we current do a nuke right after activating FBC. If my
memory isn't playing tricks on me this is actially required if
FBC didn't stay disabled for a full frame. In that case the
deactivate+reactivate may not invalidate the cfb. I'd have to
double chekc to be sure though.

So let's keep the nuke, and just extend it backwards to cover
all the platforms by doing it a bit higher up.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index fb8c0872a2b7..8165bdb6f921 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -212,16 +212,16 @@ static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
 /* This function forces a CFB recompression through the nuke operation. */
 static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
 {
-	struct intel_fbc *fbc = &dev_priv->fbc;
-
-	trace_intel_fbc_nuke(fbc->crtc);
-
 	intel_de_write(dev_priv, MSG_FBC_REND_STATE, FBC_REND_NUKE);
 	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
 }
 
 static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
 {
+	struct intel_fbc *fbc = &dev_priv->fbc;
+
+	trace_intel_fbc_nuke(fbc->crtc);
+
 	if (DISPLAY_VER(dev_priv) >= 6)
 		snb_fbc_recompress(dev_priv);
 	else if (DISPLAY_VER(dev_priv) >= 4)
@@ -274,8 +274,6 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 		       params->fence_y_offset);
 	/* enable it... */
 	intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
-
-	intel_fbc_recompress(dev_priv);
 }
 
 static void ilk_fbc_deactivate(struct drm_i915_private *dev_priv)
@@ -348,8 +346,6 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
 		dpfc_ctl |= FBC_CTL_FALSE_COLOR;
 
 	intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
-
-	intel_fbc_recompress(dev_priv);
 }
 
 static bool intel_fbc_hw_is_active(struct drm_i915_private *dev_priv)
@@ -418,6 +414,7 @@ static void intel_fbc_activate(struct drm_i915_private *dev_priv)
 	drm_WARN_ON(&dev_priv->drm, !mutex_is_locked(&fbc->lock));
 
 	intel_fbc_hw_activate(dev_priv);
+	intel_fbc_recompress(dev_priv);
 
 	fbc->no_fbc_reason = NULL;
 }
-- 
2.26.3

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

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

* [Intel-gfx] [PATCH 6/8] drm/i915: Nuke lots of crap from intel_fbc_state_cache
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (4 preceding siblings ...)
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 5/8] drm/i915: Move the "recompress on activate" to a central place Ville Syrjala
@ 2021-04-14  2:23 ` Ville Syrjala
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 7/8] drm/i915: No FBC+double wide pipe Ville Syrjala
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

There's no need to store all this stuff in intel_fbc_state_cache.
Just check it all against the plane/crtc states and store only
what we need. Probably more should get nuked still, but this
is a start.

So what we'll do is:
- each crtc will check its own state and update its local
  no_fbc_reason
- the per-crtc no_fbc_reason (if any) then gets propagated
  to the cache->no_fbc_reason while doing the actual update
- fbc->no_fbc_reason gets updated in the end with either
  the value from the cache or directly from frontbuffer
  tracing

It's still a bit messy, but should hopefuly get cleaned up
more in the future. At least now we can observe each pipe's
reasons for rejecting FBC now more consistently, and we don't
have so mcuh redundant state store all over the place.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |   5 +-
 .../drm/i915/display/intel_display_types.h    |   2 +-
 drivers/gpu/drm/i915/display/intel_fbc.c      | 363 ++++++++----------
 drivers/gpu/drm/i915/display/intel_fbc.h      |   3 +-
 drivers/gpu/drm/i915/i915_drv.h               |  21 +-
 5 files changed, 177 insertions(+), 217 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a4b8fb5c20f0..f1d71cb7952e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9575,7 +9575,6 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
-	intel_fbc_choose_crtc(dev_priv, state);
 	ret = calc_watermark_data(state);
 	if (ret)
 		goto fail;
@@ -9604,6 +9603,10 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	ret = intel_fbc_atomic_check(state);
+	if (ret)
+		goto fail;
+
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		if (new_crtc_state->uapi.async_flip) {
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index e2e707c4dff5..c08fae63bae5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1084,7 +1084,7 @@ struct intel_crtc_state {
 
 	bool crc_enabled;
 
-	bool enable_fbc;
+	const char *no_fbc_reason;
 
 	bool double_wide;
 
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 8165bdb6f921..a8565c58d1f1 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -43,6 +43,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "i915_vgpu.h"
+#include "intel_cdclk.h"
 #include "intel_display_types.h"
 #include "intel_fbc.h"
 #include "intel_frontbuffer.h"
@@ -588,9 +589,16 @@ void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv)
 	mutex_unlock(&fbc->lock);
 }
 
-static bool stride_is_valid(struct drm_i915_private *dev_priv,
-			    u64 modifier, unsigned int stride)
+static bool stride_is_valid(const struct intel_plane_state *plane_state)
 {
+	struct drm_i915_private *dev_priv = to_i915(plane_state->uapi.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->hw.fb;
+	unsigned int stride;
+
+	stride = plane_state->view.color_plane[0].stride;
+	if (drm_rotation_90_or_270(plane_state->hw.rotation))
+		stride *= fb->format->cpp[0];
+
 	/* This should have been caught earlier. */
 	if (drm_WARN_ON_ONCE(&dev_priv->drm, (stride & (64 - 1)) != 0))
 		return false;
@@ -607,7 +615,7 @@ static bool stride_is_valid(struct drm_i915_private *dev_priv,
 
 	/* Display WA #1105: skl,bxt,kbl,cfl,glk */
 	if (IS_DISPLAY_VER(dev_priv, 9) &&
-	    modifier == DRM_FORMAT_MOD_LINEAR && stride & 511)
+	    fb->modifier == DRM_FORMAT_MOD_LINEAR && stride & 511)
 		return false;
 
 	if (stride > 16384)
@@ -616,10 +624,12 @@ static bool stride_is_valid(struct drm_i915_private *dev_priv,
 	return true;
 }
 
-static bool pixel_format_is_valid(struct drm_i915_private *dev_priv,
-				  u32 pixel_format)
+static bool pixel_format_is_valid(const struct intel_plane_state *plane_state)
 {
-	switch (pixel_format) {
+	struct drm_i915_private *dev_priv = to_i915(plane_state->uapi.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->hw.fb;
+
+	switch (fb->format->format) {
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_XBGR8888:
 		return true;
@@ -637,10 +647,14 @@ static bool pixel_format_is_valid(struct drm_i915_private *dev_priv,
 	}
 }
 
-static bool rotation_is_valid(struct drm_i915_private *dev_priv,
-			      u32 pixel_format, unsigned int rotation)
+static bool rotation_is_valid(const struct intel_plane_state *plane_state)
 {
-	if (DISPLAY_VER(dev_priv) >= 9 && pixel_format == DRM_FORMAT_RGB565 &&
+	struct drm_i915_private *dev_priv = to_i915(plane_state->uapi.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->hw.fb;
+	unsigned int rotation = plane_state->hw.rotation;
+
+	if (DISPLAY_VER(dev_priv) >= 9 &&
+	    fb->format->format == DRM_FORMAT_RGB565 &&
 	    drm_rotation_90_or_270(rotation))
 		return false;
 	else if (DISPLAY_VER(dev_priv) <= 4 && !IS_G4X(dev_priv) &&
@@ -656,10 +670,10 @@ static bool rotation_is_valid(struct drm_i915_private *dev_priv,
  * the X and Y offset registers. That's why we include the src x/y offsets
  * instead of just looking at the plane size.
  */
-static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
+static bool
+intel_fbc_hw_tracking_covers_screen(const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_fbc *fbc = &dev_priv->fbc;
+	struct drm_i915_private *dev_priv = to_i915(plane_state->uapi.plane->dev);
 	unsigned int effective_w, effective_h, max_w, max_h;
 
 	if (DISPLAY_VER(dev_priv) >= 10) {
@@ -676,18 +690,20 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
 		max_h = 1536;
 	}
 
-	intel_fbc_get_plane_source_size(&fbc->state_cache, &effective_w,
-					&effective_h);
-	effective_w += fbc->state_cache.plane.adjusted_x;
-	effective_h += fbc->state_cache.plane.adjusted_y;
+	effective_w = plane_state->view.color_plane[0].x +
+		(drm_rect_width(&plane_state->uapi.src) >> 16);
+	effective_h = plane_state->view.color_plane[0].y +
+		(drm_rect_height(&plane_state->uapi.src) >> 16);
 
 	return effective_w <= max_w && effective_h <= max_h;
 }
 
-static bool tiling_is_valid(struct drm_i915_private *dev_priv,
-			    u64 modifier)
+static bool tiling_is_valid(const struct intel_plane_state *plane_state)
 {
-	switch (modifier) {
+	struct drm_i915_private *dev_priv = to_i915(plane_state->uapi.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->hw.fb;
+
+	switch (fb->modifier) {
 	case DRM_FORMAT_MOD_LINEAR:
 	case I915_FORMAT_MOD_Y_TILED:
 		return DISPLAY_VER(dev_priv) >= 9;
@@ -707,15 +723,10 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	struct intel_fbc_state_cache *cache = &fbc->state_cache;
 	struct drm_framebuffer *fb = plane_state->hw.fb;
 
-	cache->plane.visible = plane_state->uapi.visible;
-	if (!cache->plane.visible)
+	cache->no_fbc_reason = crtc_state->no_fbc_reason;
+	if (cache->no_fbc_reason)
 		return;
 
-	cache->crtc.mode_flags = crtc_state->hw.adjusted_mode.flags;
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		cache->crtc.hsw_bdw_pixel_rate = crtc_state->pixel_rate;
-
-	cache->plane.rotation = plane_state->hw.rotation;
 	/*
 	 * Src coordinates are already rotated by 270 degrees for
 	 * the 90/270 degree plane rotation cases (to match the
@@ -723,10 +734,6 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	 */
 	cache->plane.src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
 	cache->plane.src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
-	cache->plane.adjusted_x = plane_state->view.color_plane[0].x;
-	cache->plane.adjusted_y = plane_state->view.color_plane[0].y;
-
-	cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
 
 	cache->fb.format = fb->format;
 	cache->fb.modifier = fb->modifier;
@@ -749,8 +756,6 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 		cache->fence_id = plane_state->vma->fence->id;
 	else
 		cache->fence_id = -1;
-
-	cache->psr2_active = crtc_state->has_psr2;
 }
 
 static bool intel_fbc_cfb_size_changed(struct drm_i915_private *dev_priv)
@@ -784,6 +789,11 @@ static bool intel_fbc_can_enable(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc *fbc = &dev_priv->fbc;
 
+	if (!HAS_FBC(dev_priv)) {
+		fbc->no_fbc_reason = "unsupported by this chipset";
+		return false;
+	}
+
 	if (intel_vgpu_active(dev_priv)) {
 		fbc->no_fbc_reason = "VGPU is active";
 		return false;
@@ -802,6 +812,114 @@ static bool intel_fbc_can_enable(struct drm_i915_private *dev_priv)
 	return true;
 }
 
+static int intel_crtc_fbc_check(struct intel_atomic_state *state,
+				struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+	const struct intel_plane_state *plane_state =
+		intel_atomic_get_new_plane_state(state, plane);
+	const struct drm_framebuffer *fb;
+
+	if (!plane->has_fbc)
+		return 0;
+
+	if (!plane_state)
+		return 0;
+
+	fb = plane_state->hw.fb;
+
+	if (!plane_state->uapi.visible) {
+		crtc_state->no_fbc_reason = "plane not visible";
+		return 0;
+	}
+
+	if (crtc_state->hw.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
+		crtc_state->no_fbc_reason = "interlaced mode";
+		return 0;
+	}
+
+	if (!intel_fbc_hw_tracking_covers_screen(plane_state)) {
+		crtc_state->no_fbc_reason = "plane too large";
+		return 0;
+	}
+
+	if (!pixel_format_is_valid(plane_state)) {
+		crtc_state->no_fbc_reason = "incompatible pixel format";
+		return 0;
+	}
+
+	if (!tiling_is_valid(plane_state)) {
+		crtc_state->no_fbc_reason = "incompatible tiling";
+		return 0;
+	}
+
+	if (!rotation_is_valid(plane_state)) {
+		crtc_state->no_fbc_reason = "incompatible rotation";
+		return 0;
+	}
+
+	if (!stride_is_valid(plane_state)) {
+		crtc_state->no_fbc_reason = "incompatible stride";
+		return 0;
+	}
+
+	if (plane_state->hw.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&
+	    plane_state->hw.fb->format->has_alpha) {
+		crtc_state->no_fbc_reason = "alpha blending enabled";
+		return 0;
+	}
+
+	/*
+	 * Work around a problem on GEN9+ HW, where enabling FBC on a plane
+	 * having a Y offset that isn't divisible by 4 causes FIFO underrun
+	 * and screen flicker.
+	 */
+	if (DISPLAY_VER(dev_priv) >= 9 &&
+	    plane_state->view.color_plane[0].y & 3) {
+		crtc_state->no_fbc_reason = "plane start Y offset misaligned";
+		return 0;
+	}
+
+	/* Wa_22010751166: icl, ehl, tgl, dg1, rkl */
+	if (DISPLAY_VER(dev_priv) >= 11 &&
+	    (plane_state->view.color_plane[0].y + (drm_rect_height(&plane_state->uapi.src) >> 16)) & 3) {
+		crtc_state->no_fbc_reason = "plane end Y offset misaligned";
+		return 0;
+	}
+
+	/*
+	 * Tigerlake is not supporting FBC with PSR2.
+	 * Recommendation is to keep this combination disabled
+	 * Bspec: 50422 HSD: 14010260002
+	 */
+	if (IS_TIGERLAKE(dev_priv) && crtc_state->has_psr2) {
+		crtc_state->no_fbc_reason = "PSR2 enabled";
+		return 0;
+	}
+
+	/* WaFbcExceedCdClockThreshold:hsw,bdw */
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+		const struct intel_cdclk_state *cdclk_state;
+
+		cdclk_state = intel_atomic_get_cdclk_state(state);
+		if (IS_ERR(cdclk_state))
+			return PTR_ERR(cdclk_state);
+
+		if (crtc_state->pixel_rate >
+		    cdclk_state->logical.cdclk * 95 / 100) {
+			crtc_state->no_fbc_reason = "pixel rate too high";
+			return 0;
+		}
+	}
+
+	crtc_state->no_fbc_reason = NULL;
+
+	return 0;
+}
+
 static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -811,26 +929,8 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 	if (!intel_fbc_can_enable(dev_priv))
 		return false;
 
-	if (!cache->plane.visible) {
-		fbc->no_fbc_reason = "primary plane not visible";
-		return false;
-	}
-
-	/* We don't need to use a state cache here since this information is
-	 * global for all CRTC.
-	 */
-	if (fbc->underrun_detected) {
-		fbc->no_fbc_reason = "underrun detected";
-		return false;
-	}
-
-	if (cache->crtc.mode_flags & DRM_MODE_FLAG_INTERLACE) {
-		fbc->no_fbc_reason = "incompatible mode";
-		return false;
-	}
-
-	if (!intel_fbc_hw_tracking_covers_screen(crtc)) {
-		fbc->no_fbc_reason = "mode too large for compression";
+	if (cache->no_fbc_reason) {
+		fbc->no_fbc_reason = cache->no_fbc_reason;
 		return false;
 	}
 
@@ -852,41 +952,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 	 * rotation.
 	 */
 	if (DISPLAY_VER(dev_priv) < 9 && cache->fence_id < 0) {
-		fbc->no_fbc_reason = "framebuffer not tiled or fenced";
-		return false;
-	}
-
-	if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
-		fbc->no_fbc_reason = "pixel format is invalid";
-		return false;
-	}
-
-	if (!rotation_is_valid(dev_priv, cache->fb.format->format,
-			       cache->plane.rotation)) {
-		fbc->no_fbc_reason = "rotation unsupported";
-		return false;
-	}
-
-	if (!tiling_is_valid(dev_priv, cache->fb.modifier)) {
-		fbc->no_fbc_reason = "tiling unsupported";
-		return false;
-	}
-
-	if (!stride_is_valid(dev_priv, cache->fb.modifier, cache->fb.stride)) {
-		fbc->no_fbc_reason = "framebuffer stride not supported";
-		return false;
-	}
-
-	if (cache->plane.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&
-	    cache->fb.format->has_alpha) {
-		fbc->no_fbc_reason = "per-pixel alpha blending is incompatible with FBC";
-		return false;
-	}
-
-	/* WaFbcExceedCdClockThreshold:hsw,bdw */
-	if ((IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) &&
-	    cache->crtc.hsw_bdw_pixel_rate >= dev_priv->cdclk.hw.cdclk * 95 / 100) {
-		fbc->no_fbc_reason = "pixel rate is too big";
+		fbc->no_fbc_reason = "framebuffer not fenced";
 		return false;
 	}
 
@@ -905,34 +971,6 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 		return false;
 	}
 
-	/*
-	 * Work around a problem on GEN9+ HW, where enabling FBC on a plane
-	 * having a Y offset that isn't divisible by 4 causes FIFO underrun
-	 * and screen flicker.
-	 */
-	if (DISPLAY_VER(dev_priv) >= 9 &&
-	    (fbc->state_cache.plane.adjusted_y & 3)) {
-		fbc->no_fbc_reason = "plane Y offset is misaligned";
-		return false;
-	}
-
-	/* Wa_22010751166: icl, ehl, tgl, dg1, rkl */
-	if (DISPLAY_VER(dev_priv) >= 11 &&
-	    (cache->plane.src_h + cache->plane.adjusted_y) % 4) {
-		fbc->no_fbc_reason = "plane height + offset is non-modulo of 4";
-		return false;
-	}
-
-	/*
-	 * Tigerlake is not supporting FBC with PSR2.
-	 * Recommendation is to keep this combination disabled
-	 * Bspec: 50422 HSD: 14010260002
-	 */
-	if (fbc->state_cache.psr2_active && IS_TIGERLAKE(dev_priv)) {
-		fbc->no_fbc_reason = "not supported with PSR2";
-		return false;
-	}
-
 	return true;
 }
 
@@ -963,8 +1001,6 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
 	params->cfb_size = intel_fbc_calculate_cfb_size(dev_priv, cache);
 
 	params->gen9_wa_cfb_stride = cache->gen9_wa_cfb_stride;
-
-	params->plane_visible = cache->plane.visible;
 }
 
 static bool intel_fbc_can_flip_nuke(const struct intel_crtc_state *crtc_state)
@@ -978,7 +1014,7 @@ static bool intel_fbc_can_flip_nuke(const struct intel_crtc_state *crtc_state)
 	if (drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
 		return false;
 
-	if (!params->plane_visible)
+	if (fbc->no_fbc_reason)
 		return false;
 
 	if (!intel_fbc_can_activate(crtc))
@@ -1012,7 +1048,6 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state,
 		intel_atomic_get_new_plane_state(state, plane);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_fbc *fbc = &dev_priv->fbc;
-	const char *reason = "update pending";
 	bool need_vblank_wait = false;
 
 	if (!plane->has_fbc || !plane_state)
@@ -1027,7 +1062,7 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state,
 	fbc->flip_pending = true;
 
 	if (!intel_fbc_can_flip_nuke(crtc_state)) {
-		intel_fbc_deactivate(dev_priv, reason);
+		intel_fbc_deactivate(dev_priv, "update pending");
 
 		/*
 		 * Display WA #1198: glk+
@@ -1063,6 +1098,7 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state,
 static void __intel_fbc_disable(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc *fbc = &dev_priv->fbc;
+	struct intel_fbc_state_cache *cache = &fbc->state_cache;
 	struct intel_crtc *crtc = fbc->crtc;
 
 	drm_WARN_ON(&dev_priv->drm, !mutex_is_locked(&fbc->lock));
@@ -1074,6 +1110,7 @@ static void __intel_fbc_disable(struct drm_i915_private *dev_priv)
 
 	__intel_fbc_cleanup_cfb(dev_priv);
 
+	fbc->no_fbc_reason = cache->no_fbc_reason;
 	fbc->crtc = NULL;
 }
 
@@ -1089,13 +1126,6 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc)
 
 	fbc->flip_pending = false;
 
-	if (!dev_priv->params.enable_fbc) {
-		intel_fbc_deactivate(dev_priv, "disabled at runtime per module param");
-		__intel_fbc_disable(dev_priv);
-
-		return;
-	}
-
 	intel_fbc_get_reg_params(crtc, &fbc->params);
 
 	if (!intel_fbc_can_activate(crtc))
@@ -1189,75 +1219,23 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
 	mutex_unlock(&fbc->lock);
 }
 
-/**
- * intel_fbc_choose_crtc - select a CRTC to enable FBC on
- * @dev_priv: i915 device instance
- * @state: the atomic state structure
- *
- * This function looks at the proposed state for CRTCs and planes, then chooses
- * which pipe is going to have FBC by setting intel_crtc_state->enable_fbc to
- * true.
- *
- * Later, intel_fbc_enable is going to look for state->enable_fbc and then maybe
- * enable FBC for the chosen CRTC. If it does, it will set dev_priv->fbc.crtc.
- */
-void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
-			   struct intel_atomic_state *state)
+int intel_fbc_atomic_check(struct intel_atomic_state *state)
 {
-	struct intel_fbc *fbc = &dev_priv->fbc;
-	struct intel_plane *plane;
-	struct intel_plane_state *plane_state;
-	bool crtc_chosen = false;
+	struct intel_crtc_state *crtc_state;
+	struct intel_crtc *crtc;
 	int i;
 
-	mutex_lock(&fbc->lock);
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		int ret;
 
-	/* Does this atomic commit involve the CRTC currently tied to FBC? */
-	if (fbc->crtc &&
-	    !intel_atomic_get_new_crtc_state(state, fbc->crtc))
-		goto out;
-
-	if (!intel_fbc_can_enable(dev_priv))
-		goto out;
-
-	/* Simply choose the first CRTC that is compatible and has a visible
-	 * plane. We could go for fancier schemes such as checking the plane
-	 * size, but this would just affect the few platforms that don't tie FBC
-	 * to pipe or plane A. */
-	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
-		struct intel_crtc_state *crtc_state;
-		struct intel_crtc *crtc = to_intel_crtc(plane_state->hw.crtc);
-
-		if (!plane->has_fbc)
-			continue;
-
-		if (!plane_state->uapi.visible)
-			continue;
-
-		crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
-
-		crtc_state->enable_fbc = true;
-		crtc_chosen = true;
-		break;
+		ret = intel_crtc_fbc_check(state, crtc);
+		if (ret)
+			return ret;
 	}
 
-	if (!crtc_chosen)
-		fbc->no_fbc_reason = "no suitable CRTC for FBC";
-
-out:
-	mutex_unlock(&fbc->lock);
+	return 0;
 }
 
-/**
- * intel_fbc_enable: tries to enable FBC on the CRTC
- * @crtc: the CRTC
- * @state: corresponding &drm_crtc_state for @crtc
- *
- * This function checks if the given CRTC was chosen for FBC, then enables it if
- * possible. Notice that it doesn't activate FBC. It is valid to call
- * intel_fbc_enable multiple times for the same pipe without an
- * intel_fbc_disable in the middle, as long as it is deactivated.
- */
 static void intel_fbc_enable(struct intel_atomic_state *state,
 			     struct intel_crtc *crtc)
 {
@@ -1287,15 +1265,14 @@ static void intel_fbc_enable(struct intel_atomic_state *state,
 	drm_WARN_ON(&dev_priv->drm, fbc->active);
 
 	intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
-
-	/* FIXME crtc_state->enable_fbc lies :( */
-	if (!cache->plane.visible)
+	if (cache->no_fbc_reason) {
+		fbc->no_fbc_reason = cache->no_fbc_reason;
 		goto out;
+	}
 
 	if (intel_fbc_alloc_cfb(dev_priv,
 				intel_fbc_calculate_cfb_size(dev_priv, cache),
 				plane_state->hw.fb->format->cpp[0])) {
-		cache->plane.visible = false;
 		fbc->no_fbc_reason = "not enough stolen memory";
 		goto out;
 	}
@@ -1348,7 +1325,7 @@ void intel_fbc_update(struct intel_atomic_state *state,
 	const struct intel_crtc_state *crtc_state =
 		intel_atomic_get_new_crtc_state(state, crtc);
 
-	if (crtc_state->update_pipe && !crtc_state->enable_fbc)
+	if (crtc_state->update_pipe && crtc_state->no_fbc_reason)
 		intel_fbc_disable(crtc);
 	else
 		intel_fbc_enable(state, crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h
index b97d908738e6..c678b83f7509 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.h
+++ b/drivers/gpu/drm/i915/display/intel_fbc.h
@@ -16,8 +16,7 @@ struct intel_crtc;
 struct intel_crtc_state;
 struct intel_plane_state;
 
-void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
-			   struct intel_atomic_state *state);
+int intel_fbc_atomic_check(struct intel_atomic_state *state);
 bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
 bool intel_fbc_pre_update(struct intel_atomic_state *state,
 			  struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 69e43bf91a15..cbe880477ee1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -385,25 +385,8 @@ struct intel_fbc {
 	 */
 	struct intel_fbc_state_cache {
 		struct {
-			unsigned int mode_flags;
-			u32 hsw_bdw_pixel_rate;
-		} crtc;
-
-		struct {
-			unsigned int rotation;
 			int src_w;
 			int src_h;
-			bool visible;
-			/*
-			 * Display surface base address adjustement for
-			 * pageflips. Note that on gen4+ this only adjusts up
-			 * to a tile, offsets within a tile are handled in
-			 * the hw itself (with the TILEOFF register).
-			 */
-			int adjusted_x;
-			int adjusted_y;
-
-			u16 pixel_blend_mode;
 		} plane;
 
 		struct {
@@ -416,7 +399,7 @@ struct intel_fbc {
 		u16 gen9_wa_cfb_stride;
 		u16 interval;
 		s8 fence_id;
-		bool psr2_active;
+		const char *no_fbc_reason;
 	} state_cache;
 
 	/*
@@ -443,9 +426,7 @@ struct intel_fbc {
 		u16 gen9_wa_cfb_stride;
 		u16 interval;
 		s8 fence_id;
-		bool plane_visible;
 	} params;
-
 	const char *no_fbc_reason;
 };
 
-- 
2.26.3

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

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

* [Intel-gfx] [PATCH 7/8] drm/i915: No FBC+double wide pipe
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (5 preceding siblings ...)
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 6/8] drm/i915: Nuke lots of crap from intel_fbc_state_cache Ville Syrjala
@ 2021-04-14  2:23 ` Ville Syrjala
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 8/8] drm/i915: Pimp the FBC debugfs output Ville Syrjala
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

FBC and double wide pipe are mutually exclusive. Disable FBC when
we have to resort to double wide.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index a8565c58d1f1..48cddf70488f 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -841,6 +841,11 @@ static int intel_crtc_fbc_check(struct intel_atomic_state *state,
 		return 0;
 	}
 
+	if (crtc_state->double_wide) {
+		crtc_state->no_fbc_reason = "double wide pipe";
+		return 0;
+	}
+
 	if (!intel_fbc_hw_tracking_covers_screen(plane_state)) {
 		crtc_state->no_fbc_reason = "plane too large";
 		return 0;
-- 
2.26.3

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

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

* [Intel-gfx] [PATCH 8/8] drm/i915: Pimp the FBC debugfs output
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (6 preceding siblings ...)
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 7/8] drm/i915: No FBC+double wide pipe Ville Syrjala
@ 2021-04-14  2:23 ` Ville Syrjala
  2021-04-21 13:59   ` Jani Nikula
  2021-04-14  3:08 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: FBC cleanups Patchwork
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2021-04-14  2:23 UTC (permalink / raw)
  To: intel-gfx

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

Now that each pipe tracks its own no_fbc_reason we can print that
out in debugfs, and we can also show which pipe is currently
selected for FBC duty.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 50 +++++++++++++------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 183c414d554a..26317e66cb95 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -38,15 +38,36 @@ static int i915_frontbuffer_tracking(struct seq_file *m, void *unused)
 	return 0;
 }
 
+static bool i915_fbc_is_compressing(struct drm_i915_private *dev_priv)
+{
+	if (!intel_fbc_is_active(dev_priv))
+		return false;
+
+	if (DISPLAY_VER(dev_priv) >= 8)
+		return intel_de_read(dev_priv, IVB_FBC_STATUS2) & BDW_FBC_COMP_SEG_MASK;
+	else if (DISPLAY_VER(dev_priv) >= 7)
+		return intel_de_read(dev_priv, IVB_FBC_STATUS2) & IVB_FBC_COMP_SEG_MASK;
+	else if (DISPLAY_VER(dev_priv) >= 5)
+		return intel_de_read(dev_priv, ILK_DPFC_STATUS) & ILK_DPFC_COMP_SEG_MASK;
+	else if (IS_G4X(dev_priv))
+		return intel_de_read(dev_priv, DPFC_STATUS) & DPFC_COMP_SEG_MASK;
+	else
+		return intel_de_read(dev_priv, FBC_STATUS) &
+			(FBC_STAT_COMPRESSING | FBC_STAT_COMPRESSED);
+}
+
 static int i915_fbc_status(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	struct intel_fbc *fbc = &dev_priv->fbc;
+	struct intel_crtc *crtc;
 	intel_wakeref_t wakeref;
 
 	if (!HAS_FBC(dev_priv))
 		return -ENODEV;
 
+	drm_modeset_lock_all(&dev_priv->drm);
+
 	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
 	mutex_lock(&fbc->lock);
 
@@ -55,27 +76,28 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 	else
 		seq_printf(m, "FBC disabled: %s\n", fbc->no_fbc_reason);
 
-	if (intel_fbc_is_active(dev_priv)) {
-		u32 mask;
+	seq_printf(m, "Compressing: %s\n", yesno(i915_fbc_is_compressing(dev_priv)));
 
-		if (DISPLAY_VER(dev_priv) >= 8)
-			mask = intel_de_read(dev_priv, IVB_FBC_STATUS2) & BDW_FBC_COMP_SEG_MASK;
-		else if (DISPLAY_VER(dev_priv) >= 7)
-			mask = intel_de_read(dev_priv, IVB_FBC_STATUS2) & IVB_FBC_COMP_SEG_MASK;
-		else if (DISPLAY_VER(dev_priv) >= 5)
-			mask = intel_de_read(dev_priv, ILK_DPFC_STATUS) & ILK_DPFC_COMP_SEG_MASK;
-		else if (IS_G4X(dev_priv))
-			mask = intel_de_read(dev_priv, DPFC_STATUS) & DPFC_COMP_SEG_MASK;
-		else
-			mask = intel_de_read(dev_priv, FBC_STATUS) &
-				(FBC_STAT_COMPRESSING | FBC_STAT_COMPRESSED);
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+		const struct intel_crtc_state *crtc_state =
+			to_intel_crtc_state(crtc->base.state);
 
-		seq_printf(m, "Compressing: %s\n", yesno(mask));
+		if (!plane->has_fbc)
+			continue;
+
+		seq_printf(m, "%c [CRTC:%d:%s]/[PLANE:%d:%s]: %s\n",
+			   fbc->crtc == crtc ? '*' : ' ',
+			   crtc->base.base.id, crtc->base.name,
+			   plane->base.base.id, plane->base.name,
+			   crtc_state->no_fbc_reason ?: "FBC possible");
 	}
 
 	mutex_unlock(&fbc->lock);
 	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
 
+	drm_modeset_unlock_all(&dev_priv->drm);
+
 	return 0;
 }
 
-- 
2.26.3

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

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: FBC cleanups
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (7 preceding siblings ...)
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 8/8] drm/i915: Pimp the FBC debugfs output Ville Syrjala
@ 2021-04-14  3:08 ` Patchwork
  2021-04-14  3:10 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2021-04-14  3:08 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: FBC cleanups
URL   : https://patchwork.freedesktop.org/series/89036/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
d006a5bb82f9 drm/i915: Add frontbuffer tracking tracepoints
-:58: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#58: FILE: drivers/gpu/drm/i915/i915_trace.h:483:
+	    TP_STRUCT__entry(

-:63: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#63: FILE: drivers/gpu/drm/i915/i915_trace.h:488:
+	    TP_fast_assign(

-:76: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#76: FILE: drivers/gpu/drm/i915/i915_trace.h:501:
+	    TP_STRUCT__entry(

-:81: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#81: FILE: drivers/gpu/drm/i915/i915_trace.h:506:
+	    TP_fast_assign(

total: 0 errors, 0 warnings, 4 checks, 67 lines checked
576bdb61126e drm/i915: Rewrite the FBC tiling check a bit
2f5361a510d8 drm/i915: Extract intel_fbc_update()
9ef16c7d0055 drm/i915: Clear no_fbc_reason on activate
064c4919edc5 drm/i915: Move the "recompress on activate" to a central place
761fdc25c3fe drm/i915: Nuke lots of crap from intel_fbc_state_cache
-:326: WARNING:LONG_LINE: line length of 105 exceeds 100 columns
#326: FILE: drivers/gpu/drm/i915/display/intel_fbc.c:888:
+	    (plane_state->view.color_plane[0].y + (drm_rect_height(&plane_state->uapi.src) >> 16)) & 3) {

total: 0 errors, 1 warnings, 0 checks, 615 lines checked
86585b14aee8 drm/i915: No FBC+double wide pipe
b64f68cd1fa2 drm/i915: Pimp the FBC debugfs output


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

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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: FBC cleanups
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (8 preceding siblings ...)
  2021-04-14  3:08 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: FBC cleanups Patchwork
@ 2021-04-14  3:10 ` Patchwork
  2021-04-14  3:13 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
  2021-04-14  3:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  11 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2021-04-14  3:10 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: FBC cleanups
URL   : https://patchwork.freedesktop.org/series/89036/
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/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_reset.c:1329:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gt/intel_ring_submission.c:1203:24: warning: Using plain integer as NULL pointer
+drivers/gpu/drm/i915/gvt/mmio.c:295:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1434:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1488:15: warning: memset with byte count of 16777216
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


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

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

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915: FBC cleanups
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (9 preceding siblings ...)
  2021-04-14  3:10 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2021-04-14  3:13 ` Patchwork
  2021-04-14  3:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  11 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2021-04-14  3:13 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: FBC cleanups
URL   : https://patchwork.freedesktop.org/series/89036/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:102: warning: Function parameter or member 'ww' not described in 'i915_gem_shrink'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'jump_whitelist' not described in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'shadow_map' not described in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'batch_map' not described in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' 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] 21+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: FBC cleanups
  2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
                   ` (10 preceding siblings ...)
  2021-04-14  3:13 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
@ 2021-04-14  3:38 ` Patchwork
  11 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2021-04-14  3:38 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx


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

== Series Details ==

Series: drm/i915: FBC cleanups
URL   : https://patchwork.freedesktop.org/series/89036/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9966 -> Patchwork_19928
====================================================

Summary
-------

  **FAILURE**

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

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

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-icl-y:           NOTRUN -> [FAIL][1] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-y/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-icl-u2:          [PASS][2] -> [FAIL][3] +1 similar issue
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9966/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  
#### Suppressed ####

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

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - {fi-ehl-2}:         [PASS][4] -> [FAIL][5] +1 similar issue
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9966/fi-ehl-2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-ehl-2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@semaphore:
    - fi-icl-y:           NOTRUN -> [SKIP][6] ([fdo#109315]) +17 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-y/igt@amdgpu/amd_basic@semaphore.html

  * igt@gem_huc_copy@huc-copy:
    - fi-icl-y:           NOTRUN -> [SKIP][7] ([i915#2190])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-y/igt@gem_huc_copy@huc-copy.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-icl-y:           NOTRUN -> [SKIP][8] ([fdo#109284] / [fdo#111827]) +8 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-y/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-y:           NOTRUN -> [SKIP][9] ([fdo#109285])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-y/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-icl-y:           NOTRUN -> [SKIP][10] ([fdo#109278])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-y/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-icl-y:           NOTRUN -> [SKIP][11] ([fdo#110189]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-y/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-userptr:
    - fi-icl-y:           NOTRUN -> [SKIP][12] ([i915#3301])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-icl-y/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-bdw-5557u:       NOTRUN -> [FAIL][13] ([i915#1602] / [i915#2029])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-bdw-5557u/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - fi-tgl-y:           [DMESG-WARN][14] ([i915#1982] / [k.org#205379]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9966/fi-tgl-y/igt@i915_module_load@reload.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19928/fi-tgl-y/igt@i915_module_load@reload.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1602]: https://gitlab.freedesktop.org/drm/intel/issues/1602
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2029]: https://gitlab.freedesktop.org/drm/intel/issues/2029
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2629]: https://gitlab.freedesktop.org/drm/intel/issues/2629
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


Participating hosts (46 -> 42)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


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

  * Linux: CI_DRM_9966 -> Patchwork_19928

  CI-20190529: 20190529
  CI_DRM_9966: 0f7f5236775ef3b8bb2ed5ba456797850f0c4e93 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6064: 48d89e2c65c54883b0776930a884e6d3bcefb45b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19928: b64f68cd1fa2208314a4da5288eb80e3a2847c27 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

b64f68cd1fa2 drm/i915: Pimp the FBC debugfs output
86585b14aee8 drm/i915: No FBC+double wide pipe
761fdc25c3fe drm/i915: Nuke lots of crap from intel_fbc_state_cache
064c4919edc5 drm/i915: Move the "recompress on activate" to a central place
9ef16c7d0055 drm/i915: Clear no_fbc_reason on activate
2f5361a510d8 drm/i915: Extract intel_fbc_update()
576bdb61126e drm/i915: Rewrite the FBC tiling check a bit
d006a5bb82f9 drm/i915: Add frontbuffer tracking tracepoints

== Logs ==

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

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

* Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit Ville Syrjala
@ 2021-04-14 15:09   ` Jani Nikula
  2021-04-15 15:34     ` Ville Syrjälä
  0 siblings, 1 reply; 21+ messages in thread
From: Jani Nikula @ 2021-04-14 15:09 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Write the tiling check in a nicer form.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 04d9c7d22b04..178243a6d3a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -681,11 +681,9 @@ static bool tiling_is_valid(struct drm_i915_private *dev_priv,
>  {
>  	switch (modifier) {
>  	case DRM_FORMAT_MOD_LINEAR:
> -		if (DISPLAY_VER(dev_priv) >= 9)
> -			return true;
> -		return false;
> -	case I915_FORMAT_MOD_X_TILED:
>  	case I915_FORMAT_MOD_Y_TILED:
> +		return DISPLAY_VER(dev_priv) >= 9;

So this adds the version check on I915_FORMAT_MOD_Y_TILED which didn't
have it before?

BR,
Jani.


> +	case I915_FORMAT_MOD_X_TILED:
>  		return true;
>  	default:
>  		return false;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit
  2021-04-14 15:09   ` Jani Nikula
@ 2021-04-15 15:34     ` Ville Syrjälä
  2021-04-21 13:00       ` Jani Nikula
  0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2021-04-15 15:34 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, Apr 14, 2021 at 06:09:23PM +0300, Jani Nikula wrote:
> On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Write the tiling check in a nicer form.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 04d9c7d22b04..178243a6d3a2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -681,11 +681,9 @@ static bool tiling_is_valid(struct drm_i915_private *dev_priv,
> >  {
> >  	switch (modifier) {
> >  	case DRM_FORMAT_MOD_LINEAR:
> > -		if (DISPLAY_VER(dev_priv) >= 9)
> > -			return true;
> > -		return false;
> > -	case I915_FORMAT_MOD_X_TILED:
> >  	case I915_FORMAT_MOD_Y_TILED:
> > +		return DISPLAY_VER(dev_priv) >= 9;
> 
> So this adds the version check on I915_FORMAT_MOD_Y_TILED which didn't
> have it before?

Yeah, but Y tile scanout is gen9+ anyway. Should have pointed
that out in the commit msg I suppose.

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

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

* Re: [Intel-gfx] [PATCH 3/8] drm/i915: Extract intel_fbc_update()
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 3/8] drm/i915: Extract intel_fbc_update() Ville Syrjala
@ 2021-04-21 13:00   ` Jani Nikula
  0 siblings, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2021-04-21 13:00 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Pull the fbc enable vs. disable stuff into a small helper so
> we don't have to have it pollute the higher level modeset code.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  5 +---
>  drivers/gpu/drm/i915/display/intel_fbc.c     | 26 ++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_fbc.h     |  2 +-
>  3 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 411b46c012f8..a4b8fb5c20f0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9799,10 +9799,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>  			intel_encoders_update_pipe(state, crtc);
>  	}
>  
> -	if (new_crtc_state->update_pipe && !new_crtc_state->enable_fbc)
> -		intel_fbc_disable(crtc);
> -	else
> -		intel_fbc_enable(state, crtc);
> +	intel_fbc_update(state, crtc);
>  
>  	/* Perform vblank evasion around commit operation */
>  	intel_pipe_update_start(new_crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 178243a6d3a2..4968e79a6235 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1250,8 +1250,8 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
>   * intel_fbc_enable multiple times for the same pipe without an
>   * intel_fbc_disable in the middle, as long as it is deactivated.
>   */
> -void intel_fbc_enable(struct intel_atomic_state *state,
> -		      struct intel_crtc *crtc)
> +static void intel_fbc_enable(struct intel_atomic_state *state,
> +			     struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> @@ -1324,6 +1324,28 @@ void intel_fbc_disable(struct intel_crtc *crtc)
>  	mutex_unlock(&fbc->lock);
>  }
>  
> +/**
> + * intel_fbc_update: enable/disable FBC on the CRTC
> + * @state: atomic state
> + * @crtc: the CRTC
> + *
> + * This function checks if the given CRTC was chosen for FBC, then enables it if
> + * possible. Notice that it doesn't activate FBC. It is valid to call
> + * intel_fbc_update multiple times for the same pipe without an
> + * intel_fbc_disable in the middle.
> + */
> +void intel_fbc_update(struct intel_atomic_state *state,
> +		      struct intel_crtc *crtc)
> +{
> +	const struct intel_crtc_state *crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +
> +	if (crtc_state->update_pipe && !crtc_state->enable_fbc)
> +		intel_fbc_disable(crtc);
> +	else
> +		intel_fbc_enable(state, crtc);
> +}
> +
>  /**
>   * intel_fbc_global_disable - globally disable FBC
>   * @dev_priv: i915 device instance
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h
> index 6dc1edefe81b..b97d908738e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.h
> @@ -24,7 +24,7 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state,
>  void intel_fbc_post_update(struct intel_atomic_state *state,
>  			   struct intel_crtc *crtc);
>  void intel_fbc_init(struct drm_i915_private *dev_priv);
> -void intel_fbc_enable(struct intel_atomic_state *state,
> +void intel_fbc_update(struct intel_atomic_state *state,
>  		      struct intel_crtc *crtc);
>  void intel_fbc_disable(struct intel_crtc *crtc);
>  void intel_fbc_global_disable(struct drm_i915_private *dev_priv);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit
  2021-04-15 15:34     ` Ville Syrjälä
@ 2021-04-21 13:00       ` Jani Nikula
  0 siblings, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2021-04-21 13:00 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, 15 Apr 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Apr 14, 2021 at 06:09:23PM +0300, Jani Nikula wrote:
>> On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Write the tiling check in a nicer form.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++----
>> >  1 file changed, 2 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
>> > index 04d9c7d22b04..178243a6d3a2 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>> > @@ -681,11 +681,9 @@ static bool tiling_is_valid(struct drm_i915_private *dev_priv,
>> >  {
>> >  	switch (modifier) {
>> >  	case DRM_FORMAT_MOD_LINEAR:
>> > -		if (DISPLAY_VER(dev_priv) >= 9)
>> > -			return true;
>> > -		return false;
>> > -	case I915_FORMAT_MOD_X_TILED:
>> >  	case I915_FORMAT_MOD_Y_TILED:
>> > +		return DISPLAY_VER(dev_priv) >= 9;
>> 
>> So this adds the version check on I915_FORMAT_MOD_Y_TILED which didn't
>> have it before?
>
> Yeah, but Y tile scanout is gen9+ anyway. Should have pointed
> that out in the commit msg I suppose.

Right. With that added in the commit message (while applying is fine),

Reviewed-by: Jani Nikula <jani.nikula@intel.com>



-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 1/8] drm/i915: Add frontbuffer tracking tracepoints
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 1/8] drm/i915: Add frontbuffer tracking tracepoints Ville Syrjala
@ 2021-04-21 13:01   ` Jani Nikula
  0 siblings, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2021-04-21 13:01 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add some tracpoints for frontbuffer tracking so we can
> try to figure out what's going on.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  .../gpu/drm/i915/display/intel_frontbuffer.c  |  5 +++
>  drivers/gpu/drm/i915/i915_trace.h             | 38 +++++++++++++++++++
>  2 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> index 6fc6965b6133..8161d49e78ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> @@ -58,6 +58,7 @@
>  #include "display/intel_dp.h"
>  
>  #include "i915_drv.h"
> +#include "i915_trace.h"
>  #include "intel_display_types.h"
>  #include "intel_fbc.h"
>  #include "intel_frontbuffer.h"
> @@ -87,6 +88,8 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
>  	if (!frontbuffer_bits)
>  		return;
>  
> +	trace_intel_frontbuffer_flush(frontbuffer_bits, origin);
> +
>  	might_sleep();
>  	intel_edp_drrs_flush(i915, frontbuffer_bits);
>  	intel_psr_flush(i915, frontbuffer_bits, origin);
> @@ -173,6 +176,8 @@ void __intel_fb_invalidate(struct intel_frontbuffer *front,
>  		spin_unlock(&i915->fb_tracking.lock);
>  	}
>  
> +	trace_intel_frontbuffer_invalidate(frontbuffer_bits, origin);
> +
>  	might_sleep();
>  	intel_psr_invalidate(i915, frontbuffer_bits, origin);
>  	intel_edp_drrs_invalidate(i915, frontbuffer_bits);
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index a4addcc64978..81f5e1721180 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -474,6 +474,44 @@ TRACE_EVENT(intel_pipe_update_end,
>  		      __entry->scanline)
>  );
>  
> +/* frontbuffer tracking */
> +
> +TRACE_EVENT(intel_frontbuffer_invalidate,
> +	    TP_PROTO(unsigned int frontbuffer_bits, unsigned int origin),
> +	    TP_ARGS(frontbuffer_bits, origin),
> +
> +	    TP_STRUCT__entry(
> +			     __field(unsigned int, frontbuffer_bits)
> +			     __field(unsigned int, origin)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->frontbuffer_bits = frontbuffer_bits;
> +			   __entry->origin = origin;
> +			   ),
> +
> +	    TP_printk("frontbuffer_bits=0x%08x, origin=%u",
> +		      __entry->frontbuffer_bits, __entry->origin)
> +);
> +
> +TRACE_EVENT(intel_frontbuffer_flush,
> +	    TP_PROTO(unsigned int frontbuffer_bits, unsigned int origin),
> +	    TP_ARGS(frontbuffer_bits, origin),
> +
> +	    TP_STRUCT__entry(
> +			     __field(unsigned int, frontbuffer_bits)
> +			     __field(unsigned int, origin)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->frontbuffer_bits = frontbuffer_bits;
> +			   __entry->origin = origin;
> +			   ),
> +
> +	    TP_printk("frontbuffer_bits=0x%08x, origin=%u",
> +		      __entry->frontbuffer_bits, __entry->origin)
> +);
> +
>  /* object tracking */
>  
>  TRACE_EVENT(i915_gem_object_create,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 5/8] drm/i915: Move the "recompress on activate" to a central place
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 5/8] drm/i915: Move the "recompress on activate" to a central place Ville Syrjala
@ 2021-04-21 13:17   ` Jani Nikula
  0 siblings, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2021-04-21 13:17 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On ILK+ we current do a nuke right after activating FBC. If my
> memory isn't playing tricks on me this is actially required if
> FBC didn't stay disabled for a full frame. In that case the
> deactivate+reactivate may not invalidate the cfb. I'd have to
> double chekc to be sure though.
>
> So let's keep the nuke, and just extend it backwards to cover
> all the platforms by doing it a bit higher up.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Does what it says on the box, and the change overall seems logical.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index fb8c0872a2b7..8165bdb6f921 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -212,16 +212,16 @@ static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
>  /* This function forces a CFB recompression through the nuke operation. */
>  static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
>  {
> -	struct intel_fbc *fbc = &dev_priv->fbc;
> -
> -	trace_intel_fbc_nuke(fbc->crtc);
> -
>  	intel_de_write(dev_priv, MSG_FBC_REND_STATE, FBC_REND_NUKE);
>  	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
>  }
>  
>  static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
>  {
> +	struct intel_fbc *fbc = &dev_priv->fbc;
> +
> +	trace_intel_fbc_nuke(fbc->crtc);
> +
>  	if (DISPLAY_VER(dev_priv) >= 6)
>  		snb_fbc_recompress(dev_priv);
>  	else if (DISPLAY_VER(dev_priv) >= 4)
> @@ -274,8 +274,6 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  		       params->fence_y_offset);
>  	/* enable it... */
>  	intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> -
> -	intel_fbc_recompress(dev_priv);
>  }
>  
>  static void ilk_fbc_deactivate(struct drm_i915_private *dev_priv)
> @@ -348,8 +346,6 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  		dpfc_ctl |= FBC_CTL_FALSE_COLOR;
>  
>  	intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> -
> -	intel_fbc_recompress(dev_priv);
>  }
>  
>  static bool intel_fbc_hw_is_active(struct drm_i915_private *dev_priv)
> @@ -418,6 +414,7 @@ static void intel_fbc_activate(struct drm_i915_private *dev_priv)
>  	drm_WARN_ON(&dev_priv->drm, !mutex_is_locked(&fbc->lock));
>  
>  	intel_fbc_hw_activate(dev_priv);
> +	intel_fbc_recompress(dev_priv);
>  
>  	fbc->no_fbc_reason = NULL;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 4/8] drm/i915: Clear no_fbc_reason on activate
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 4/8] drm/i915: Clear no_fbc_reason on activate Ville Syrjala
@ 2021-04-21 13:48   ` Jani Nikula
  0 siblings, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2021-04-21 13:48 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We try to set no_fbc_reason when FBC is not possible, let's
> consistently clear when activating FBC.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 4968e79a6235..fb8c0872a2b7 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -411,6 +411,17 @@ bool intel_fbc_is_active(struct drm_i915_private *dev_priv)
>  	return dev_priv->fbc.active;
>  }
>  
> +static void intel_fbc_activate(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_fbc *fbc = &dev_priv->fbc;
> +
> +	drm_WARN_ON(&dev_priv->drm, !mutex_is_locked(&fbc->lock));
> +
> +	intel_fbc_hw_activate(dev_priv);
> +
> +	fbc->no_fbc_reason = NULL;
> +}
> +
>  static void intel_fbc_deactivate(struct drm_i915_private *dev_priv,
>  				 const char *reason)
>  {
> @@ -1094,7 +1105,7 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc)
>  		return;
>  
>  	if (!fbc->busy_bits)
> -		intel_fbc_hw_activate(dev_priv);
> +		intel_fbc_activate(dev_priv);
>  	else
>  		intel_fbc_deactivate(dev_priv, "frontbuffer write");
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 8/8] drm/i915: Pimp the FBC debugfs output
  2021-04-14  2:23 ` [Intel-gfx] [PATCH 8/8] drm/i915: Pimp the FBC debugfs output Ville Syrjala
@ 2021-04-21 13:59   ` Jani Nikula
  0 siblings, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2021-04-21 13:59 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Wed, 14 Apr 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that each pipe tracks its own no_fbc_reason we can print that
> out in debugfs, and we can also show which pipe is currently
> selected for FBC duty.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 50 +++++++++++++------
>  1 file changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 183c414d554a..26317e66cb95 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -38,15 +38,36 @@ static int i915_frontbuffer_tracking(struct seq_file *m, void *unused)
>  	return 0;
>  }
>  
> +static bool i915_fbc_is_compressing(struct drm_i915_private *dev_priv)
> +{
> +	if (!intel_fbc_is_active(dev_priv))
> +		return false;
> +
> +	if (DISPLAY_VER(dev_priv) >= 8)
> +		return intel_de_read(dev_priv, IVB_FBC_STATUS2) & BDW_FBC_COMP_SEG_MASK;
> +	else if (DISPLAY_VER(dev_priv) >= 7)
> +		return intel_de_read(dev_priv, IVB_FBC_STATUS2) & IVB_FBC_COMP_SEG_MASK;
> +	else if (DISPLAY_VER(dev_priv) >= 5)
> +		return intel_de_read(dev_priv, ILK_DPFC_STATUS) & ILK_DPFC_COMP_SEG_MASK;
> +	else if (IS_G4X(dev_priv))
> +		return intel_de_read(dev_priv, DPFC_STATUS) & DPFC_COMP_SEG_MASK;
> +	else
> +		return intel_de_read(dev_priv, FBC_STATUS) &
> +			(FBC_STAT_COMPRESSING | FBC_STAT_COMPRESSED);
> +}

I wouldn't mind moving debugfs helpers like this to files by feature,
e.g. intel_fbc.[ch] in this case. This one could just return bool like
here; in some cases I'm starting to think passing struct seq_file * and
having the helper print there would make sense.

BR,
Jani.


> +
>  static int i915_fbc_status(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>  	struct intel_fbc *fbc = &dev_priv->fbc;
> +	struct intel_crtc *crtc;
>  	intel_wakeref_t wakeref;
>  
>  	if (!HAS_FBC(dev_priv))
>  		return -ENODEV;
>  
> +	drm_modeset_lock_all(&dev_priv->drm);
> +
>  	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>  	mutex_lock(&fbc->lock);
>  
> @@ -55,27 +76,28 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
>  	else
>  		seq_printf(m, "FBC disabled: %s\n", fbc->no_fbc_reason);
>  
> -	if (intel_fbc_is_active(dev_priv)) {
> -		u32 mask;
> +	seq_printf(m, "Compressing: %s\n", yesno(i915_fbc_is_compressing(dev_priv)));
>  
> -		if (DISPLAY_VER(dev_priv) >= 8)
> -			mask = intel_de_read(dev_priv, IVB_FBC_STATUS2) & BDW_FBC_COMP_SEG_MASK;
> -		else if (DISPLAY_VER(dev_priv) >= 7)
> -			mask = intel_de_read(dev_priv, IVB_FBC_STATUS2) & IVB_FBC_COMP_SEG_MASK;
> -		else if (DISPLAY_VER(dev_priv) >= 5)
> -			mask = intel_de_read(dev_priv, ILK_DPFC_STATUS) & ILK_DPFC_COMP_SEG_MASK;
> -		else if (IS_G4X(dev_priv))
> -			mask = intel_de_read(dev_priv, DPFC_STATUS) & DPFC_COMP_SEG_MASK;
> -		else
> -			mask = intel_de_read(dev_priv, FBC_STATUS) &
> -				(FBC_STAT_COMPRESSING | FBC_STAT_COMPRESSED);
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> +		const struct intel_crtc_state *crtc_state =
> +			to_intel_crtc_state(crtc->base.state);
>  
> -		seq_printf(m, "Compressing: %s\n", yesno(mask));
> +		if (!plane->has_fbc)
> +			continue;
> +
> +		seq_printf(m, "%c [CRTC:%d:%s]/[PLANE:%d:%s]: %s\n",
> +			   fbc->crtc == crtc ? '*' : ' ',
> +			   crtc->base.base.id, crtc->base.name,
> +			   plane->base.base.id, plane->base.name,
> +			   crtc_state->no_fbc_reason ?: "FBC possible");
>  	}
>  
>  	mutex_unlock(&fbc->lock);
>  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>  
> +	drm_modeset_unlock_all(&dev_priv->drm);
> +
>  	return 0;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2021-04-21 14:00 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-14  2:23 [Intel-gfx] [PATCH 0/8] drm/i915: FBC cleanups Ville Syrjala
2021-04-14  2:23 ` [Intel-gfx] [PATCH 1/8] drm/i915: Add frontbuffer tracking tracepoints Ville Syrjala
2021-04-21 13:01   ` Jani Nikula
2021-04-14  2:23 ` [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit Ville Syrjala
2021-04-14 15:09   ` Jani Nikula
2021-04-15 15:34     ` Ville Syrjälä
2021-04-21 13:00       ` Jani Nikula
2021-04-14  2:23 ` [Intel-gfx] [PATCH 3/8] drm/i915: Extract intel_fbc_update() Ville Syrjala
2021-04-21 13:00   ` Jani Nikula
2021-04-14  2:23 ` [Intel-gfx] [PATCH 4/8] drm/i915: Clear no_fbc_reason on activate Ville Syrjala
2021-04-21 13:48   ` Jani Nikula
2021-04-14  2:23 ` [Intel-gfx] [PATCH 5/8] drm/i915: Move the "recompress on activate" to a central place Ville Syrjala
2021-04-21 13:17   ` Jani Nikula
2021-04-14  2:23 ` [Intel-gfx] [PATCH 6/8] drm/i915: Nuke lots of crap from intel_fbc_state_cache Ville Syrjala
2021-04-14  2:23 ` [Intel-gfx] [PATCH 7/8] drm/i915: No FBC+double wide pipe Ville Syrjala
2021-04-14  2:23 ` [Intel-gfx] [PATCH 8/8] drm/i915: Pimp the FBC debugfs output Ville Syrjala
2021-04-21 13:59   ` Jani Nikula
2021-04-14  3:08 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: FBC cleanups Patchwork
2021-04-14  3:10 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-04-14  3:13 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2021-04-14  3:38 ` [Intel-gfx] ✗ Fi.CI.BAT: 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.