[Intel-gfx] [PATCH 00/12] drm/i915: FBC fixes

[Intel-gfx] [PATCH 00/12] drm/i915: FBC fixes

[Ville Syrjala]

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

A bunch of FBC fixes. The most important thing is fixing glk+linear,
but included a pile of stuff I had lying about for older platforms
as well.

Ville Syrjälä (12):
  drm/i915/fbc: Require linear fb stride to be multiple of 512 bytes on
    gen9/glk
  drm/i915/fbc: Use the correct plane stride
  drm/i915/fbc: Fix fence_y_offset handling
  drm/i915/fbc: Fix nuke for pre-snb platforms
  drm/i915/fbc: Enable fbc on i865
  drm/i915/fbc: Don't clear busy_bits for origin==GTT
  drm/i915/fbc: Allow FBC to recompress after a 3D workload on i85x/i865
  drm/i915/fbc: Parametrize FBC_CONTROL
  drm/i915/fbc: Store the fbc1 compression interval in the params
  drm/i915/fbc: Reduce fbc1 compression interval to 1 second
  drm/i915: Fix g4x fbc watermark enable
  drm/i915: Suppress spurious underruns on gen2

 drivers/gpu/drm/i915/display/intel_display.c |  18 +++-
 drivers/gpu/drm/i915/display/intel_display.h |   1 +
 drivers/gpu/drm/i915/display/intel_fbc.c     | 104 ++++++++++++-------
 drivers/gpu/drm/i915/i915_drv.h              |   8 +-
 drivers/gpu/drm/i915/i915_pci.c              |   1 +
 drivers/gpu/drm/i915/i915_reg.h              |  19 ++--
 drivers/gpu/drm/i915/intel_pm.c              |  43 +++++---
 7 files changed, 135 insertions(+), 59 deletions(-)

-- 
2.24.1

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

[Intel-gfx] [PATCH 01/12] drm/i915/fbc: Require linear fb stride to be multiple of 512 bytes on gen9/glk

[Ville Syrjala]

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

Display WA #1105 says that FBC requires PLANE_STRIDE to be a multiple
of 512 bytes on gen9 and glk.

This is definitely true for glk as certain tests (such as
igt/kms_big_fb/linear-16bpp-rotate-0) are now failing when the
display resolution results in a plane stride which is not a
multiple of 512 bytes.

Curiously I was not able to reproduce this on a KBL. First I
suspected that our use of the FBC override stride explain this,
but after trying to use the override stride on glk the test
still failed. I did try both the old CHICKEN_MISC_4 way and
the new FBC_CHICKEN way, neither had any effect on the result.

Anyways, we need this at least on glk. But let's trust the spec
and apply the w/a for all gen9 as well, despite being unable to
reproduce the problem.

Cc: José Roberto de Souza <jose.souza@intel.com>
Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for GEN9+")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index c6afa10e814c..7194f9bc62c5 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -564,7 +564,7 @@ void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv)
 }
 
 static bool stride_is_valid(struct drm_i915_private *dev_priv,
-			    unsigned int stride)
+			    u64 modifier, unsigned int stride)
 {
 	/* This should have been caught earlier. */
 	if (drm_WARN_ON_ONCE(&dev_priv->drm, (stride & (64 - 1)) != 0))
@@ -580,6 +580,11 @@ static bool stride_is_valid(struct drm_i915_private *dev_priv,
 	if (IS_GEN(dev_priv, 4) && !IS_G4X(dev_priv) && stride < 2048)
 		return false;
 
+	/* Display WA #1105: skl,bxt,kbl,cfl,glk */
+	if (IS_GEN(dev_priv, 9) &&
+	    modifier == DRM_FORMAT_MOD_LINEAR && stride & 511)
+		return false;
+
 	if (stride > 16384)
 		return false;
 
@@ -810,7 +815,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 		return false;
 	}
 
-	if (!stride_is_valid(dev_priv, cache->fb.stride)) {
+	if (!stride_is_valid(dev_priv, cache->fb.modifier, cache->fb.stride)) {
 		fbc->no_fbc_reason = "framebuffer stride not supported";
 		return false;
 	}
-- 
2.24.1

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

[Intel-gfx] [PATCH 02/12] drm/i915/fbc: Use the correct plane stride

[Ville Syrjala]

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

Consult the actual plane stride instead of the fb stride. The two
will disagree when we remap the gtt. The plane stride is what the
hw will be fed so that's what we should look at for the FBC
retrictions/cfb allocation.

Since we no longer require a fence we are going to attempt using
FBC with remapping, and so we should look at correct stride.

Cc: José Roberto de Souza <jose.souza@intel.com>
Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for GEN9+")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 7194f9bc62c5..192c5ff142ee 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -707,8 +707,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
 
 	cache->fb.format = fb->format;
-	cache->fb.stride = fb->pitches[0];
 	cache->fb.modifier = fb->modifier;
+	cache->fb.stride = plane_state->color_plane[0].stride;
 
 	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
 		    !plane_state->vma->fence);
-- 
2.24.1

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

[Intel-gfx] [PATCH 03/12] drm/i915/fbc: Fix fence_y_offset handling

[Ville Syrjala]

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

The current fence_y_offset calculation is broken. I think it more or
less used to do the right thing, but then I changed the plane code
to put the final x/y source offsets back into the src rectangle so
now it's just subtraacting the same value from itself. The code would
never have worked if we allowed the framebuffer to have a non-zero
offset.

Let's do this in a better way by just calculating the fence_y_offset
from the final plane surface offset. Note that we don't align the
plane surface address to fence rows so with horizontal panning there's
often a horizontal offset from the fence start to the surface address
as well. We have no way to tell the hardware about that so we just
ignore it. Based on some quick tests the invlidation still happens
correctly. I presume due to the invalidation nuking at least the full
line (or a segment of multiple lines).

Fixes: 54d4d719fa11 ("drm/i915: Overcome display engine stride limits via GTT remapping")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 11 +++++++
 drivers/gpu/drm/i915/display/intel_display.h |  1 +
 drivers/gpu/drm/i915/display/intel_fbc.c     | 32 ++++++--------------
 drivers/gpu/drm/i915/i915_drv.h              |  6 ++--
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6bb87965801e..e5fa49337883 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3822,6 +3822,17 @@ skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state,
 	return true;
 }
 
+unsigned int
+intel_plane_fence_y_offset(const struct intel_plane_state *plane_state)
+{
+	int x = 0, y = 0;
+
+	intel_plane_adjust_aligned_offset(&x, &y, plane_state, 0,
+					  plane_state->color_plane[0].offset, 0);
+
+	return y;
+}
+
 static int skl_check_main_surface(struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane_state->uapi.plane->dev);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index efb4da205ea2..3a06f72c9859 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -608,6 +608,7 @@ unsigned int i9xx_plane_max_stride(struct intel_plane *plane,
 				   u32 pixel_format, u64 modifier,
 				   unsigned int rotation);
 int bdw_get_pipemisc_bpp(struct intel_crtc *crtc);
+unsigned int intel_plane_fence_y_offset(const struct intel_plane_state *plane_state);
 
 struct intel_display_error_state *
 intel_display_capture_error_state(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 192c5ff142ee..613ab499d42e 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -47,19 +47,6 @@
 #include "intel_fbc.h"
 #include "intel_frontbuffer.h"
 
-/*
- * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
- * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
- * origin so the x and y offsets can actually fit the registers. As a
- * consequence, the fence doesn't really start exactly at the display plane
- * address we program because it starts at the real start of the buffer, so we
- * have to take this into consideration here.
- */
-static unsigned int get_crtc_fence_y_offset(struct intel_fbc *fbc)
-{
-	return fbc->state_cache.plane.y - fbc->state_cache.plane.adjusted_y;
-}
-
 /*
  * For SKL+, the plane source size used by the hardware is based on the value we
  * write to the PLANE_SIZE register. For BDW-, the hardware looks at the value
@@ -141,7 +128,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
 			fbc_ctl2 |= FBC_CTL_CPU_FENCE;
 		intel_de_write(dev_priv, FBC_CONTROL2, fbc_ctl2);
 		intel_de_write(dev_priv, FBC_FENCE_OFF,
-			       params->crtc.fence_y_offset);
+			       params->fence_y_offset);
 	}
 
 	/* enable it... */
@@ -175,7 +162,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
 	if (params->fence_id >= 0) {
 		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fence_id;
 		intel_de_write(dev_priv, DPFC_FENCE_YOFF,
-			       params->crtc.fence_y_offset);
+			       params->fence_y_offset);
 	} else {
 		intel_de_write(dev_priv, DPFC_FENCE_YOFF, 0);
 	}
@@ -243,7 +230,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 			intel_de_write(dev_priv, SNB_DPFC_CTL_SA,
 				       SNB_CPU_FENCE_ENABLE | params->fence_id);
 			intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET,
-				       params->crtc.fence_y_offset);
+				       params->fence_y_offset);
 		}
 	} else {
 		if (IS_GEN(dev_priv, 6)) {
@@ -253,7 +240,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 	}
 
 	intel_de_write(dev_priv, ILK_DPFC_FENCE_YOFF,
-		       params->crtc.fence_y_offset);
+		       params->fence_y_offset);
 	/* enable it... */
 	intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
@@ -320,7 +307,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
 		intel_de_write(dev_priv, SNB_DPFC_CTL_SA,
 			       SNB_CPU_FENCE_ENABLE | params->fence_id);
 		intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET,
-			       params->crtc.fence_y_offset);
+			       params->fence_y_offset);
 	} else if (dev_priv->ggtt.num_fences) {
 		intel_de_write(dev_priv, SNB_DPFC_CTL_SA, 0);
 		intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET, 0);
@@ -628,8 +615,8 @@ static bool rotation_is_valid(struct drm_i915_private *dev_priv,
 /*
  * For some reason, the hardware tracking starts looking at whatever we
  * programmed as the display plane base address register. It does not look at
- * the X and Y offset registers. That's why we look at the crtc->adjusted{x,y}
- * variables instead of just looking at the pipe/plane size.
+ * 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)
 {
@@ -702,7 +689,6 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	cache->plane.src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
 	cache->plane.adjusted_x = plane_state->color_plane[0].x;
 	cache->plane.adjusted_y = plane_state->color_plane[0].y;
-	cache->plane.y = plane_state->uapi.src.y1 >> 16;
 
 	cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
 
@@ -710,6 +696,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	cache->fb.modifier = fb->modifier;
 	cache->fb.stride = plane_state->color_plane[0].stride;
 
+	cache->fence_y_offset = intel_plane_fence_y_offset(plane_state);
+
 	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
 		    !plane_state->vma->fence);
 
@@ -880,10 +868,10 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
 	memset(params, 0, sizeof(*params));
 
 	params->fence_id = cache->fence_id;
+	params->fence_y_offset = cache->fence_y_offset;
 
 	params->crtc.pipe = crtc->pipe;
 	params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)->i9xx_plane;
-	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
 
 	params->fb.format = cache->fb.format;
 	params->fb.stride = cache->fb.stride;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b00f0845cbc3..a634fd2330c3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -408,8 +408,6 @@ struct intel_fbc {
 			int adjusted_x;
 			int adjusted_y;
 
-			int y;
-
 			u16 pixel_blend_mode;
 		} plane;
 
@@ -418,6 +416,8 @@ struct intel_fbc {
 			unsigned int stride;
 			u64 modifier;
 		} fb;
+
+		unsigned int fence_y_offset;
 		u16 gen9_wa_cfb_stride;
 		s8 fence_id;
 	} state_cache;
@@ -433,7 +433,6 @@ struct intel_fbc {
 		struct {
 			enum pipe pipe;
 			enum i9xx_plane_id i9xx_plane;
-			unsigned int fence_y_offset;
 		} crtc;
 
 		struct {
@@ -442,6 +441,7 @@ struct intel_fbc {
 		} fb;
 
 		int cfb_size;
+		unsigned int fence_y_offset;
 		u16 gen9_wa_cfb_stride;
 		s8 fence_id;
 		bool plane_visible;
-- 
2.24.1

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

[Intel-gfx] [PATCH 04/12] drm/i915/fbc: Fix nuke for pre-snb platforms

[Ville Syrjala]

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

The MSG_FBC_REND_STATE register only exists on snb+. For older
platforms (would also work for snb+) we can simply rewite DSPSURF
to trigger a flip nuke.

While generally RMW is considered harmful we'll use it here for
simplicity. And since FBC doesn't exist in i830 we don't have to
worry about the DSPSURF double buffering hardware fails present
on that platform.

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

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 613ab499d42e..983224e07eaf 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -188,8 +188,30 @@ static bool g4x_fbc_is_active(struct drm_i915_private *dev_priv)
 	return intel_de_read(dev_priv, DPFC_CONTROL) & DPFC_CTL_EN;
 }
 
+static void i8xx_fbc_recompress(struct drm_i915_private *dev_priv)
+{
+	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
+	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
+
+	spin_lock_irq(&dev_priv->uncore.lock);
+	intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
+			  intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
+	spin_unlock_irq(&dev_priv->uncore.lock);
+}
+
+static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
+{
+	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
+	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
+
+	spin_lock_irq(&dev_priv->uncore.lock);
+	intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
+			  intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
+	spin_unlock_irq(&dev_priv->uncore.lock);
+}
+
 /* This function forces a CFB recompression through the nuke operation. */
-static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
+static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc *fbc = &dev_priv->fbc;
 
@@ -199,6 +221,16 @@ static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
 	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
 }
 
+static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_GEN(dev_priv) >= 6)
+		snb_fbc_recompress(dev_priv);
+	else if (INTEL_GEN(dev_priv) >= 4)
+		i965_fbc_recompress(dev_priv);
+	else
+		i8xx_fbc_recompress(dev_priv);
+}
+
 static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
-- 
2.24.1

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

[Intel-gfx] [PATCH 05/12] drm/i915/fbc: Enable fbc on i865

[Ville Syrjala]

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

Unlike all the other pre-snb desktop platforms i865 actually
supports FBC. Let's enable it.

Quote from the spec:
"DevSDG provides the same Run-Length Encoded Frame Buffer
 Compression (RLEFBC) function as exists in DevMGM."

As i865 only has the one pipe we want to skip massaging the
plane<->pipe assignment aimed at getting FBC+LVDS working on
the mobile platforms.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e5fa49337883..a0d1057d75ee 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16186,7 +16186,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 * On gen2/3 only plane A can do FBC, but the panel fitter and LVDS
 	 * port is hooked to pipe B. Hence we want plane A feeding pipe B.
 	 */
-	if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4)
+	if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4 &&
+	    INTEL_NUM_PIPES(dev_priv) == 2)
 		plane->i9xx_plane = (enum i9xx_plane_id) !pipe;
 	else
 		plane->i9xx_plane = (enum i9xx_plane_id) pipe;
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 1faf9d6ec0a4..71afe7bc3d2d 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -217,6 +217,7 @@ static const struct intel_device_info i85x_info = {
 static const struct intel_device_info i865g_info = {
 	I845_FEATURES,
 	PLATFORM(INTEL_I865G),
+	.display.has_fbc = 1,
 };
 
 #define GEN3_FEATURES \
-- 
2.24.1

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

[Intel-gfx] [PATCH 06/12] drm/i915/fbc: Don't clear busy_bits for origin==GTT

[Ville Syrjala]

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

The hardware host tracking won't nuke the entire cfb (unless the
entire fb is written through the gtt) so don't clear the busy_bits
for gtt tracking.

Not that it really matters anymore since we've lost ORIGIN_GTT usage
everywhere.

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

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 983224e07eaf..56eeafa645de 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1107,11 +1107,19 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
 	if (!HAS_FBC(dev_priv))
 		return;
 
+	/*
+	 * GTT tracking does not nuke the entire cfb
+	 * so don't clear busy_bits set for some other
+	 * reason.
+	 */
+	if (origin == ORIGIN_GTT)
+		return;
+
 	mutex_lock(&fbc->lock);
 
 	fbc->busy_bits &= ~frontbuffer_bits;
 
-	if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
+	if (origin == ORIGIN_FLIP)
 		goto out;
 
 	if (!fbc->busy_bits && fbc->crtc &&
-- 
2.24.1

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

[Intel-gfx] [PATCH 07/12] drm/i915/fbc: Allow FBC to recompress after a 3D workload on i85x/i865

[Ville Syrjala]

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

Normally i85x/i865 3D activity will block FBC until a 2D blit
occurs. I suppose this was meant to avoid recompression while
3D activity is still going on but the frame hasn't yet been
presented. Unfortunately that also means that a page flipped
3D workload will permanently block FBC even if it only renders
a single frame and then does nothing.

Since we are using software render tracking anyway we might as
well flip the chicken bit so that 3D does not block FBC. This
will avoid the permament FBC blockage in the aforemention use
case, but thanks to the software tracking the compressor will
not disturb 3D rendering activity.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 96d9f8853343..088215025661 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2815,6 +2815,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define VLV_GU_CTL0	_MMIO(VLV_DISPLAY_BASE + 0x2030)
 #define VLV_GU_CTL1	_MMIO(VLV_DISPLAY_BASE + 0x2034)
 #define SCPD0		_MMIO(0x209c) /* 915+ only */
+#define  SCPD_FBC_IGNORE_3D			(1 << 6)
 #define  CSTATE_RENDER_CLOCK_GATE_DISABLE	(1 << 5)
 #define GEN2_IER	_MMIO(0x20a0)
 #define GEN2_IIR	_MMIO(0x20a4)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bfb180fe8047..1e99b35f007e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7369,6 +7369,16 @@ static void i85x_init_clock_gating(struct drm_i915_private *dev_priv)
 
 	I915_WRITE(MEM_MODE,
 		   _MASKED_BIT_ENABLE(MEM_DISPLAY_TRICKLE_FEED_DISABLE));
+
+	/*
+	 * Have FBC ignore 3D activity since we use software
+	 * render tracking, and otherwise a pure 3D workload
+	 * (even if it just renders a single frame and then does
+	 * abosultely nothing) would not allow FBC to recompress
+	 * until a 2D blit occurs.
+	 */
+	I915_WRITE(SCPD0,
+		   _MASKED_BIT_ENABLE(SCPD_FBC_IGNORE_3D));
 }
 
 static void i830_init_clock_gating(struct drm_i915_private *dev_priv)
-- 
2.24.1

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

[Intel-gfx] [PATCH 08/12] drm/i915/fbc: Parametrize FBC_CONTROL

[Ville Syrjala]

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

Parametrize the FBC_CONTROL bits for neater code.

Also add the one missing bit: "stop compression on modification".

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c |  8 ++++----
 drivers/gpu/drm/i915/i915_reg.h          | 18 +++++++++++-------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 56eeafa645de..dbef58af4b94 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -133,13 +133,13 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
 
 	/* enable it... */
 	fbc_ctl = intel_de_read(dev_priv, FBC_CONTROL);
-	fbc_ctl &= 0x3fff << FBC_CTL_INTERVAL_SHIFT;
+	fbc_ctl &= FBC_CTL_INTERVAL(0x3fff);
 	fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
 	if (IS_I945GM(dev_priv))
 		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
-	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
+	fbc_ctl |= FBC_CTL_STRIDE(cfb_pitch & 0xff);
 	if (params->fence_id >= 0)
-		fbc_ctl |= params->fence_id;
+		fbc_ctl |= FBC_CTL_FENCENO(params->fence_id);
 	intel_de_write(dev_priv, FBC_CONTROL, fbc_ctl);
 }
 
@@ -1452,7 +1452,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
 	/* This value was pulled out of someone's hat */
 	if (INTEL_GEN(dev_priv) <= 4 && !IS_GM45(dev_priv))
 		intel_de_write(dev_priv, FBC_CONTROL,
-		               500 << FBC_CTL_INTERVAL_SHIFT);
+			       FBC_CTL_INTERVAL(500));
 
 	/* We still don't have any sort of hardware state readout for FBC, so
 	 * deactivate it in case the BIOS activated it to make sure software
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 088215025661..e9fb64e8f28f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3194,13 +3194,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define FBC_CFB_BASE		_MMIO(0x3200) /* 4k page aligned */
 #define FBC_LL_BASE		_MMIO(0x3204) /* 4k page aligned */
 #define FBC_CONTROL		_MMIO(0x3208)
-#define   FBC_CTL_EN		(1 << 31)
-#define   FBC_CTL_PERIODIC	(1 << 30)
-#define   FBC_CTL_INTERVAL_SHIFT (16)
-#define   FBC_CTL_UNCOMPRESSIBLE (1 << 14)
-#define   FBC_CTL_C3_IDLE	(1 << 13)
-#define   FBC_CTL_STRIDE_SHIFT	(5)
-#define   FBC_CTL_FENCENO_SHIFT	(0)
+#define   FBC_CTL_EN		REG_BIT(31)
+#define   FBC_CTL_PERIODIC	REG_BIT(30)
+#define   FBC_CTL_INTERVAL_MASK	REG_GENMASK(29, 16)
+#define   FBC_CTL_INTERVAL(x)	REG_FIELD_PREP(FBC_CTL_INTERVAL_MASK, (x))
+#define   FBC_CTL_STOP_ON_MOD	REG_BIT(15)
+#define   FBC_CTL_UNCOMPRESSIBLE REG_BIT(14) /* i915+ */
+#define   FBC_CTL_C3_IDLE	REG_BIT(13) /* i945gm */
+#define   FBC_CTL_STRIDE_MASK	REG_GENMASK(12, 5)
+#define   FBC_CTL_STRIDE(x)	REG_FIELD_PREP(FBC_CTL_STRIDE_MASK, (x))
+#define   FBC_CTL_FENCENO_MASK	REG_GENMASK(3, 0)
+#define   FBC_CTL_FENCENO(x)	REG_FIELD_PREP(FBC_CTL_FENCENO_MASK, (x))
 #define FBC_COMMAND		_MMIO(0x320c)
 #define   FBC_CMD_COMPRESS	(1 << 0)
 #define FBC_STATUS		_MMIO(0x3210)
-- 
2.24.1

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

[Intel-gfx] [PATCH 09/12] drm/i915/fbc: Store the fbc1 compression interval in the params

[Ville Syrjala]

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

Avoid the FBC_CONTROL rmw and just store the fbc compression
interval in the params/

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

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index dbef58af4b94..b1eb6a2ecc43 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -132,8 +132,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
 	}
 
 	/* enable it... */
-	fbc_ctl = intel_de_read(dev_priv, FBC_CONTROL);
-	fbc_ctl &= FBC_CTL_INTERVAL(0x3fff);
+	fbc_ctl = FBC_CTL_INTERVAL(params->interval);
 	fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
 	if (IS_I945GM(dev_priv))
 		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
@@ -728,6 +727,9 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	cache->fb.modifier = fb->modifier;
 	cache->fb.stride = plane_state->color_plane[0].stride;
 
+	/* This value was pulled out of someone's hat */
+	cache->interval = 500;
+
 	cache->fence_y_offset = intel_plane_fence_y_offset(plane_state);
 
 	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
@@ -902,6 +904,8 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
 	params->fence_id = cache->fence_id;
 	params->fence_y_offset = cache->fence_y_offset;
 
+	params->interval = cache->interval;
+
 	params->crtc.pipe = crtc->pipe;
 	params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)->i9xx_plane;
 
@@ -1449,11 +1453,6 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
 		return;
 	}
 
-	/* This value was pulled out of someone's hat */
-	if (INTEL_GEN(dev_priv) <= 4 && !IS_GM45(dev_priv))
-		intel_de_write(dev_priv, FBC_CONTROL,
-			       FBC_CTL_INTERVAL(500));
-
 	/* We still don't have any sort of hardware state readout for FBC, so
 	 * deactivate it in case the BIOS activated it to make sure software
 	 * matches the hardware state. */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a634fd2330c3..bc66a7cb886b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -419,6 +419,7 @@ struct intel_fbc {
 
 		unsigned int fence_y_offset;
 		u16 gen9_wa_cfb_stride;
+		u16 interval;
 		s8 fence_id;
 	} state_cache;
 
@@ -443,6 +444,7 @@ struct intel_fbc {
 		int cfb_size;
 		unsigned int fence_y_offset;
 		u16 gen9_wa_cfb_stride;
+		u16 interval;
 		s8 fence_id;
 		bool plane_visible;
 	} params;
-- 
2.24.1

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

[Intel-gfx] [PATCH 10/12] drm/i915/fbc: Reduce fbc1 compression interval to 1 second

[Ville Syrjala]

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

The default fbc1 compression interval we use is 500 frames. That
translates to over 8 seconds typically. That's rather excessive
so let's drop it to 1 second.

The hardware will not attempt recompression unless at least one
line has been modified, so a shorter compression interval should
not cause extra bandwidth use in the purely idle scenario. Of
course in the mostly idle case we are possibly going to recompress
a bit more.

Should really try to find some kind of sweet spot to minimize
the energy usage...

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

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index b1eb6a2ecc43..6ee45d634cf6 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -727,8 +727,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	cache->fb.modifier = fb->modifier;
 	cache->fb.stride = plane_state->color_plane[0].stride;
 
-	/* This value was pulled out of someone's hat */
-	cache->interval = 500;
+	/* FBC1 compression interval: arbitrary choice of 1 second */
+	cache->interval = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode);
 
 	cache->fence_y_offset = intel_plane_fence_y_offset(plane_state);
 
-- 
2.24.1

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

[Intel-gfx] [PATCH 11/12] drm/i915: Fix g4x fbc watermark enable

[Ville Syrjala]

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

'level' here means the highest level we can't use, so when checking
the fbc watermarks we need a -1 to get at the last enabled level.

While at if refactor the code a bit to declutter
g4x_compute_pipe_wm().

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1e99b35f007e..1c92ebf64a34 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1344,6 +1344,23 @@ static void g4x_invalidate_wms(struct intel_crtc *crtc,
 	}
 }
 
+static bool g4x_compute_fbc_en(const struct g4x_wm_state *wm_state,
+			       int level)
+{
+	if (level < G4X_WM_LEVEL_SR)
+		return false;
+
+	if (level >= G4X_WM_LEVEL_SR &&
+	    wm_state->sr.fbc > g4x_fbc_fifo_size(G4X_WM_LEVEL_SR))
+		return false;
+
+	if (level >= G4X_WM_LEVEL_HPLL &&
+	    wm_state->hpll.fbc > g4x_fbc_fifo_size(G4X_WM_LEVEL_HPLL))
+		return false;
+
+	return true;
+}
+
 static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -1383,7 +1400,6 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 		wm_state->wm.plane[plane_id] = raw->plane[plane_id];
 
 	level = G4X_WM_LEVEL_SR;
-
 	if (!g4x_raw_crtc_wm_is_valid(crtc_state, level))
 		goto out;
 
@@ -1395,7 +1411,6 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 	wm_state->cxsr = num_active_planes == BIT(PLANE_PRIMARY);
 
 	level = G4X_WM_LEVEL_HPLL;
-
 	if (!g4x_raw_crtc_wm_is_valid(crtc_state, level))
 		goto out;
 
@@ -1418,17 +1433,11 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 	/*
 	 * Determine if the FBC watermark(s) can be used. IF
 	 * this isn't the case we prefer to disable the FBC
-	 ( watermark(s) rather than disable the SR/HPLL
-	 * level(s) entirely.
+	 * watermark(s) rather than disable the SR/HPLL
+	 * level(s) entirely. 'level-1' is the highest valid
+	 * level here.
 	 */
-	wm_state->fbc_en = level > G4X_WM_LEVEL_NORMAL;
-
-	if (level >= G4X_WM_LEVEL_SR &&
-	    wm_state->sr.fbc > g4x_fbc_fifo_size(G4X_WM_LEVEL_SR))
-		wm_state->fbc_en = false;
-	else if (level >= G4X_WM_LEVEL_HPLL &&
-		 wm_state->hpll.fbc > g4x_fbc_fifo_size(G4X_WM_LEVEL_HPLL))
-		wm_state->fbc_en = false;
+	wm_state->fbc_en = g4x_compute_fbc_en(wm_state, level - 1);
 
 	return 0;
 }
-- 
2.24.1

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

[Intel-gfx] [PATCH 12/12] drm/i915: Suppress spurious underruns on gen2

[Ville Syrjala]

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

Often we seem to detect an underrun right after modeset on gen2.
It seems to be a spurious detection (potentially the pipe is still
in a wonky state when we enable the planes). An extra vblank wait
seems to cure it.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a0d1057d75ee..f330054e64c5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7517,6 +7517,10 @@ static void i9xx_crtc_enable(struct intel_atomic_state *state,
 	intel_crtc_vblank_on(new_crtc_state);
 
 	intel_encoders_enable(state, crtc);
+
+	/* prevents spurious underruns */
+	if (IS_GEN(dev_priv, 2))
+		intel_wait_for_vblank(dev_priv, pipe);
 }
 
 static void i9xx_pfit_disable(const struct intel_crtc_state *old_crtc_state)
-- 
2.24.1

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: FBC fixes

[Patchwork]

== Series Details ==

Series: drm/i915: FBC fixes
URL   : https://patchwork.freedesktop.org/series/76714/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8390 -> Patchwork_17507
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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


Changes
-------

  No changes found


Participating hosts (48 -> 41)
------------------------------

  Missing    (7): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7500u fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8390 -> Patchwork_17507

  CI-20190529: 20190529
  CI_DRM_8390: 89473e10666c78c4df9e92c9caf03d7311c291cb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5614: d095827add11d4e8158b87683971ee659749d9a4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17507: e3315c131c6d7813355fbd64c818dcd7eefba6ea @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e3315c131c6d drm/i915: Suppress spurious underruns on gen2
04289a752655 drm/i915: Fix g4x fbc watermark enable
46ddb82a1501 drm/i915/fbc: Reduce fbc1 compression interval to 1 second
04304c504db1 drm/i915/fbc: Store the fbc1 compression interval in the params
d39ccd00f107 drm/i915/fbc: Parametrize FBC_CONTROL
f5b2ff08e428 drm/i915/fbc: Allow FBC to recompress after a 3D workload on i85x/i865
db225e51ab61 drm/i915/fbc: Don't clear busy_bits for origin==GTT
873203fee92b drm/i915/fbc: Enable fbc on i865
184da4f02cce drm/i915/fbc: Fix nuke for pre-snb platforms
b91927c97ffd drm/i915/fbc: Fix fence_y_offset handling
db723f635b7f drm/i915/fbc: Use the correct plane stride
1c91198476c8 drm/i915/fbc: Require linear fb stride to be multiple of 512 bytes on gen9/glk

== Logs ==

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

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: FBC fixes

[Patchwork]

== Series Details ==

Series: drm/i915: FBC fixes
URL   : https://patchwork.freedesktop.org/series/76714/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8390_full -> Patchwork_17507_full
====================================================

Summary
-------

  **FAILURE**

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_big_fb@y-tiled-32bpp-rotate-270:
    - shard-tglb:         [PASS][1] -> [INCOMPLETE][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-tglb1/igt@kms_big_fb@y-tiled-32bpp-rotate-270.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-tglb6/igt@kms_big_fb@y-tiled-32bpp-rotate-270.html
    - shard-iclb:         [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-iclb6/igt@kms_big_fb@y-tiled-32bpp-rotate-270.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-iclb5/igt@kms_big_fb@y-tiled-32bpp-rotate-270.html

  * igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
    - shard-kbl:          [PASS][5] -> [INCOMPLETE][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-kbl7/igt@kms_plane_scaling@pipe-a-scaler-with-rotation.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-kbl3/igt@kms_plane_scaling@pipe-a-scaler-with-rotation.html
    - shard-apl:          [PASS][7] -> [INCOMPLETE][8] +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-apl1/igt@kms_plane_scaling@pipe-a-scaler-with-rotation.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-apl4/igt@kms_plane_scaling@pipe-a-scaler-with-rotation.html

  * igt@runner@aborted:
    - shard-kbl:          NOTRUN -> ([FAIL][9], [FAIL][10]) ([i915#1611] / [i915#602])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-kbl3/igt@runner@aborted.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-kbl4/igt@runner@aborted.html
    - shard-iclb:         NOTRUN -> ([FAIL][11], [FAIL][12])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-iclb5/igt@runner@aborted.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-iclb2/igt@runner@aborted.html
    - shard-tglb:         NOTRUN -> ([FAIL][13], [FAIL][14])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-tglb6/igt@runner@aborted.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-tglb6/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@legacy-engines-mixed-process@vebox:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([i915#1528])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-skl8/igt@gem_ctx_persistence@legacy-engines-mixed-process@vebox.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-skl9/igt@gem_ctx_persistence@legacy-engines-mixed-process@vebox.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [PASS][17] -> [DMESG-WARN][18] ([i915#716])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-skl10/igt@gen9_exec_parse@allowed-single.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-skl1/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [PASS][19] -> [FAIL][20] ([i915#454])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-iclb3/igt@i915_pm_dc@dc6-psr.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-iclb6/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rps@waitboost:
    - shard-apl:          [PASS][21] -> [FAIL][22] ([i915#39])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-apl1/igt@i915_pm_rps@waitboost.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-apl4/igt@i915_pm_rps@waitboost.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][23] -> [DMESG-WARN][24] ([i915#180]) +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-apl4/igt@i915_suspend@sysfs-reader.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-apl7/igt@i915_suspend@sysfs-reader.html

  * igt@kms_big_fb@y-tiled-32bpp-rotate-270:
    - shard-glk:          [PASS][25] -> [INCOMPLETE][26] ([i915#58] / [k.org#198133]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-glk2/igt@kms_big_fb@y-tiled-32bpp-rotate-270.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-glk2/igt@kms_big_fb@y-tiled-32bpp-rotate-270.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x21-random:
    - shard-kbl:          [PASS][27] -> [FAIL][28] ([i915#54] / [i915#93] / [i915#95])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [PASS][29] -> [FAIL][30] ([i915#1188])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-skl10/igt@kms_hdr@bpc-switch.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-skl5/igt@kms_hdr@bpc-switch.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [PASS][31] -> [DMESG-WARN][32] ([i915#180]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-kbl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
    - shard-iclb:         [PASS][33] -> [INCOMPLETE][34] ([i915#249])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-iclb7/igt@kms_plane_scaling@pipe-a-scaler-with-rotation.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-iclb2/igt@kms_plane_scaling@pipe-a-scaler-with-rotation.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [PASS][35] -> [SKIP][36] ([fdo#109441]) +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-iclb1/igt@kms_psr@psr2_cursor_blt.html

  
#### Possible fixes ####

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [DMESG-WARN][37] ([i915#180]) -> [PASS][38] +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-apl4/igt@gem_workarounds@suspend-resume-context.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-apl7/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_big_fb@linear-16bpp-rotate-0:
    - shard-glk:          [FAIL][39] ([i915#1119]) -> [PASS][40] +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-glk6/igt@kms_big_fb@linear-16bpp-rotate-0.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-glk1/igt@kms_big_fb@linear-16bpp-rotate-0.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [FAIL][41] ([IGT#5] / [i915#697]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-skl9/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_draw_crc@draw-method-rgb565-render-untiled:
    - shard-glk:          [FAIL][43] ([i915#52] / [i915#54]) -> [PASS][44] +6 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-glk9/igt@kms_draw_crc@draw-method-rgb565-render-untiled.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-glk5/igt@kms_draw_crc@draw-method-rgb565-render-untiled.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled:
    - shard-kbl:          [FAIL][45] ([fdo#108145] / [i915#177] / [i915#52] / [i915#54] / [i915#93] / [i915#95]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-kbl2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-kbl2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
    - shard-apl:          [FAIL][47] ([fdo#108145] / [i915#52] / [i915#54] / [i915#95]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-apl2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-apl3/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html

  * {igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-hdmi-a1-hdmi-a2}:
    - shard-glk:          [FAIL][49] ([i915#79]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-glk9/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-hdmi-a1-hdmi-a2.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-glk5/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-hdmi-a1-hdmi-a2.html

  * {igt@kms_flip@flip-vs-suspend-interruptible@a-dp1}:
    - shard-kbl:          [DMESG-WARN][51] ([i915#180]) -> [PASS][52] +7 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-kbl4/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-kbl1/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * {igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1}:
    - shard-skl:          [FAIL][53] ([i915#34]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-skl7/igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-skl10/igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-skl:          [INCOMPLETE][55] ([i915#123] / [i915#69]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-skl8/igt@kms_frontbuffer_tracking@psr-suspend.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-skl2/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
    - shard-snb:          [SKIP][57] ([fdo#109271]) -> [PASS][58] +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-snb4/igt@kms_pipe_crc_basic@hang-read-crc-pipe-b.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-snb2/igt@kms_pipe_crc_basic@hang-read-crc-pipe-b.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][59] ([fdo#108145] / [i915#265]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-none:
    - shard-glk:          [FAIL][61] ([i915#899]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-glk8/igt@kms_plane_lowres@pipe-a-tiling-none.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-glk2/igt@kms_plane_lowres@pipe-a-tiling-none.html

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-iclb:         [SKIP][63] ([fdo#109441]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-iclb8/igt@kms_psr@psr2_cursor_mmap_gtt.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_gtt.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-psr:
    - shard-snb:          [INCOMPLETE][65] ([i915#82]) -> [SKIP][66] ([fdo#109271])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-snb6/igt@i915_pm_dc@dc6-psr.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-snb4/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-kbl:          [FAIL][67] ([i915#93] / [i915#95]) -> [DMESG-FAIL][68] ([i915#180] / [i915#95])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8390/shard-kbl2/igt@kms_fbcon_fbt@fbc-suspend.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17507/shard-kbl2/igt@kms_fbcon_fbt@fbc-suspend.html

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

  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
  [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1611]: https://gitlab.freedesktop.org/drm/intel/issues/1611
  [i915#177]: https://gitlab.freedesktop.org/drm/intel/issues/177
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#249]: https://gitlab.freedesktop.org/drm/intel/issues/249
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#39]: https://gitlab.freedesktop.org/drm/intel/issues/39
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [i915#602]: https://gitlab.freedesktop.org/drm/intel/issues/602
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#697]: https://gitlab.freedesktop.org/drm/intel/issues/697
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#899]: https://gitlab.freedesktop.org/drm/intel/issues/899
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8390 -> Patchwork_17507

  CI-20190529: 20190529
  CI_DRM_8390: 89473e10666c78c4df9e92c9caf03d7311c291cb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5614: d095827add11d4e8158b87683971ee659749d9a4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17507: e3315c131c6d7813355fbd64c818dcd7eefba6ea @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

[Intel-gfx] [PATCH v2 02/12] drm/i915/fbc: Use the correct plane stride

[Ville Syrjala]

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

Consult the actual plane stride instead of the fb stride. The two
will disagree when we remap the gtt. The plane stride is what the
hw will be fed so that's what we should look at for the FBC
retrictions/cfb allocation.

Since we no longer require a fence we are going to attempt using
FBC with remapping, and so we should look at correct stride.

With 90/270 degree rotation the plane stride is stored in units
of pixels, so we need to conver it to bytes for the purposes
of calculating the cfb stride. Not entirely sure if this matches
the hw behaviour though. Need to reverse engineer that at some
point...

We also need to reorder the pixel format check vs. stride check
to avoid triggering a spurious WARN(stride & 63) with cpp==1 and
plane stride==32.

v2: Try to deal with rotated stride and related WARN

Cc: José Roberto de Souza <jose.souza@intel.com>
Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for GEN9+")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 7194f9bc62c5..7f2b2382b813 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -707,9 +707,13 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
 
 	cache->fb.format = fb->format;
-	cache->fb.stride = fb->pitches[0];
 	cache->fb.modifier = fb->modifier;
 
+	/* FIXME is this correct? */
+	cache->fb.stride = plane_state->color_plane[0].stride;
+	if (drm_rotation_90_or_270(plane_state->hw.rotation))
+		cache->fb.stride *= fb->format->cpp[0];
+
 	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
 		    !plane_state->vma->fence);
 
@@ -804,6 +808,11 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 		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";
@@ -820,11 +829,6 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 		return false;
 	}
 
-	if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
-		fbc->no_fbc_reason = "pixel format is invalid";
-		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";
-- 
2.24.1

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: FBC fixes (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: FBC fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/76714/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8392 -> Patchwork_17517
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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


Changes
-------

  No changes found


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

  Missing    (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7500u fi-ctg-p8600 fi-byt-clapper 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8392 -> Patchwork_17517

  CI-20190529: 20190529
  CI_DRM_8392: c5ceaac881b4dc4eca6473abeb27342663c898d3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5614: d095827add11d4e8158b87683971ee659749d9a4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17517: 15396082f189f069234e45031d13ee4568e0e64f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

15396082f189 drm/i915: Suppress spurious underruns on gen2
10255531ded0 drm/i915: Fix g4x fbc watermark enable
9ea3c9303844 drm/i915/fbc: Reduce fbc1 compression interval to 1 second
039571106777 drm/i915/fbc: Store the fbc1 compression interval in the params
42638b346f43 drm/i915/fbc: Parametrize FBC_CONTROL
9b69c1713be1 drm/i915/fbc: Allow FBC to recompress after a 3D workload on i85x/i865
f684c1d99dc1 drm/i915/fbc: Don't clear busy_bits for origin==GTT
392e1957dcdd drm/i915/fbc: Enable fbc on i865
bfe3b237394d drm/i915/fbc: Fix nuke for pre-snb platforms
1f2ef6c459ff drm/i915/fbc: Fix fence_y_offset handling
39ee6ccb2c59 drm/i915/fbc: Use the correct plane stride
781df50da31f drm/i915/fbc: Require linear fb stride to be multiple of 512 bytes on gen9/glk

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: FBC fixes (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: FBC fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/76714/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8392_full -> Patchwork_17517_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@sysfs-reader:
    - shard-skl:          [PASS][1] -> [INCOMPLETE][2] ([i915#69])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-skl3/igt@i915_suspend@sysfs-reader.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-skl4/igt@i915_suspend@sysfs-reader.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x21-random:
    - shard-kbl:          [PASS][3] -> [FAIL][4] ([i915#54] / [i915#93] / [i915#95])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled:
    - shard-snb:          [PASS][5] -> [SKIP][6] ([fdo#109271]) +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-snb2/igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-snb6/igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-tglb:         [PASS][7] -> [SKIP][8] ([i915#668]) +9 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][9] -> [DMESG-WARN][10] ([i915#180]) +2 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([i915#173])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-iclb7/igt@kms_psr@no_drrs.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109441]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-iclb3/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [PASS][15] -> [FAIL][16] ([i915#31])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-kbl1/igt@kms_setmode@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-kbl4/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [PASS][17] -> [DMESG-WARN][18] ([i915#180]) +4 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-kbl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@engines-mixed-process@vcs0:
    - shard-skl:          [FAIL][19] ([i915#1528]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-skl5/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-skl7/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html

  * igt@gem_exec_params@invalid-bsd-ring:
    - shard-iclb:         [SKIP][21] ([fdo#109276]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-iclb6/igt@gem_exec_params@invalid-bsd-ring.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-iclb4/igt@gem_exec_params@invalid-bsd-ring.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-kbl:          [DMESG-WARN][23] ([i915#1436] / [i915#716]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-kbl3/igt@gen9_exec_parse@allowed-single.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-kbl3/igt@gen9_exec_parse@allowed-single.html

  * igt@kms_big_fb@linear-16bpp-rotate-0:
    - shard-glk:          [FAIL][25] ([i915#1119]) -> [PASS][26] +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-glk1/igt@kms_big_fb@linear-16bpp-rotate-0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-glk6/igt@kms_big_fb@linear-16bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][27] ([i915#165] / [i915#180]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [SKIP][29] ([fdo#109349]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-iclb5/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_draw_crc@draw-method-rgb565-render-untiled:
    - shard-glk:          [FAIL][31] ([i915#52] / [i915#54]) -> [PASS][32] +6 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-glk1/igt@kms_draw_crc@draw-method-rgb565-render-untiled.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-glk5/igt@kms_draw_crc@draw-method-rgb565-render-untiled.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled:
    - shard-kbl:          [FAIL][33] ([fdo#108145] / [i915#177] / [i915#52] / [i915#54] / [i915#93] / [i915#95]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-kbl7/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-kbl7/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
    - shard-apl:          [FAIL][35] ([fdo#108145] / [i915#52] / [i915#54] / [i915#95]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-apl6/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-apl6/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html

  * {igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1}:
    - shard-skl:          [FAIL][37] ([i915#46]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-skl2/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html

  * {igt@kms_flip@flip-vs-suspend-interruptible@a-edp1}:
    - shard-skl:          [INCOMPLETE][39] ([i915#198]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-skl9/igt@kms_flip@flip-vs-suspend-interruptible@a-edp1.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-skl3/igt@kms_flip@flip-vs-suspend-interruptible@a-edp1.html

  * {igt@kms_flip@flip-vs-suspend@b-dp1}:
    - shard-kbl:          [DMESG-WARN][41] ([i915#180]) -> [PASS][42] +3 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-kbl7/igt@kms_flip@flip-vs-suspend@b-dp1.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-kbl7/igt@kms_flip@flip-vs-suspend@b-dp1.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][43] ([i915#1188]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-skl4/igt@kms_hdr@bpc-switch-dpms.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-skl2/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-apl:          [DMESG-WARN][45] ([i915#180]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-apl2/igt@kms_hdr@bpc-switch-suspend.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-apl8/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-skl:          [FAIL][47] ([i915#1036]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-skl10/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-skl10/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_lowres@pipe-a-tiling-none:
    - shard-glk:          [FAIL][49] ([i915#899]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-glk5/igt@kms_plane_lowres@pipe-a-tiling-none.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-glk7/igt@kms_plane_lowres@pipe-a-tiling-none.html

  * igt@kms_psr@psr2_primary_blt:
    - shard-iclb:         [SKIP][51] ([fdo#109441]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-iclb8/igt@kms_psr@psr2_primary_blt.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-iclb2/igt@kms_psr@psr2_primary_blt.html

  * {igt@perf@polling-parameterized}:
    - shard-hsw:          [FAIL][53] ([i915#1542]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-hsw8/igt@perf@polling-parameterized.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-hsw5/igt@perf@polling-parameterized.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-psr:
    - shard-snb:          [INCOMPLETE][55] ([i915#82]) -> [SKIP][56] ([fdo#109271])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-snb5/igt@i915_pm_dc@dc6-psr.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-snb6/igt@i915_pm_dc@dc6-psr.html
    - shard-tglb:         [FAIL][57] ([i915#454]) -> [SKIP][58] ([i915#468])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8392/shard-tglb6/igt@i915_pm_dc@dc6-psr.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17517/shard-tglb2/igt@i915_pm_dc@dc6-psr.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#1036]: https://gitlab.freedesktop.org/drm/intel/issues/1036
  [i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
  [i915#173]: https://gitlab.freedesktop.org/drm/intel/issues/173
  [i915#177]: https://gitlab.freedesktop.org/drm/intel/issues/177
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#46]: https://gitlab.freedesktop.org/drm/intel/issues/46
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#899]: https://gitlab.freedesktop.org/drm/intel/issues/899
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8392 -> Patchwork_17517

  CI-20190529: 20190529
  CI_DRM_8392: c5ceaac881b4dc4eca6473abeb27342663c898d3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5614: d095827add11d4e8158b87683971ee659749d9a4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17517: 15396082f189f069234e45031d13ee4568e0e64f @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [Intel-gfx] [PATCH 01/12] drm/i915/fbc: Require linear fb stride to be multiple of 512 bytes on gen9/glk

[Matt Roper]

On Wed, Apr 29, 2020 at 01:10:23PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Display WA #1105 says that FBC requires PLANE_STRIDE to be a multiple
> of 512 bytes on gen9 and glk.
> 
> This is definitely true for glk as certain tests (such as
> igt/kms_big_fb/linear-16bpp-rotate-0) are now failing when the
> display resolution results in a plane stride which is not a
> multiple of 512 bytes.
> 
> Curiously I was not able to reproduce this on a KBL. First I
> suspected that our use of the FBC override stride explain this,
> but after trying to use the override stride on glk the test
> still failed. I did try both the old CHICKEN_MISC_4 way and
> the new FBC_CHICKEN way, neither had any effect on the result.
> 
> Anyways, we need this at least on glk. But let's trust the spec
> and apply the w/a for all gen9 as well, despite being unable to
> reproduce the problem.
> 
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for GEN9+")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index c6afa10e814c..7194f9bc62c5 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -564,7 +564,7 @@ void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv)
>  }
>  
>  static bool stride_is_valid(struct drm_i915_private *dev_priv,
> -			    unsigned int stride)
> +			    u64 modifier, unsigned int stride)
>  {
>  	/* This should have been caught earlier. */
>  	if (drm_WARN_ON_ONCE(&dev_priv->drm, (stride & (64 - 1)) != 0))
> @@ -580,6 +580,11 @@ static bool stride_is_valid(struct drm_i915_private *dev_priv,
>  	if (IS_GEN(dev_priv, 4) && !IS_G4X(dev_priv) && stride < 2048)
>  		return false;
>  
> +	/* Display WA #1105: skl,bxt,kbl,cfl,glk */
> +	if (IS_GEN(dev_priv, 9) &&
> +	    modifier == DRM_FORMAT_MOD_LINEAR && stride & 511)

Might be slightly more readable to use !IS_ALIGNED(stride, 512), but
either way,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> +		return false;
> +
>  	if (stride > 16384)
>  		return false;
>  
> @@ -810,7 +815,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  		return false;
>  	}
>  
> -	if (!stride_is_valid(dev_priv, cache->fb.stride)) {
> +	if (!stride_is_valid(dev_priv, cache->fb.modifier, cache->fb.stride)) {
>  		fbc->no_fbc_reason = "framebuffer stride not supported";
>  		return false;
>  	}
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 02/12] drm/i915/fbc: Use the correct plane stride

[Matt Roper]

On Wed, Apr 29, 2020 at 06:29:21PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Consult the actual plane stride instead of the fb stride. The two
> will disagree when we remap the gtt. The plane stride is what the
> hw will be fed so that's what we should look at for the FBC
> retrictions/cfb allocation.
> 
> Since we no longer require a fence we are going to attempt using
> FBC with remapping, and so we should look at correct stride.
> 
> With 90/270 degree rotation the plane stride is stored in units
> of pixels, so we need to conver it to bytes for the purposes
> of calculating the cfb stride. Not entirely sure if this matches
> the hw behaviour though. Need to reverse engineer that at some
> point...
> 
> We also need to reorder the pixel format check vs. stride check
> to avoid triggering a spurious WARN(stride & 63) with cpp==1 and
> plane stride==32.
> 
> v2: Try to deal with rotated stride and related WARN
> 
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for GEN9+")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 7194f9bc62c5..7f2b2382b813 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -707,9 +707,13 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  	cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
>  
>  	cache->fb.format = fb->format;
> -	cache->fb.stride = fb->pitches[0];
>  	cache->fb.modifier = fb->modifier;
>  
> +	/* FIXME is this correct? */
> +	cache->fb.stride = plane_state->color_plane[0].stride;

We still have a comment in intel_fbc_calculate_cfb_size() that indicates
that we need to use the framebuffer stride instead of the plane stride
(explicitly added in commit 850bfaab7120a).  The bspec (page 49227) uses
terminology "Stride of plane uncompressed surface" which sounds like
framebuffer size to me; I'm not sure if switching it to the plane's size
will cause problems if the plane is only scanning out a subregion of the
framebuffer?

If it really is safe to use the plane size instead of the framebuffer
size, then I think we at least need to remove or change that comment
too.


Matt

> +	if (drm_rotation_90_or_270(plane_state->hw.rotation))
> +		cache->fb.stride *= fb->format->cpp[0];
> +
>  	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
>  		    !plane_state->vma->fence);
>  
> @@ -804,6 +808,11 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  		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";
> @@ -820,11 +829,6 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  		return false;
>  	}
>  
> -	if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
> -		fbc->no_fbc_reason = "pixel format is invalid";
> -		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";
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/12] drm/i915/fbc: Fix fence_y_offset handling

[Matt Roper]

On Wed, Apr 29, 2020 at 01:10:25PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The current fence_y_offset calculation is broken. I think it more or
> less used to do the right thing, but then I changed the plane code
> to put the final x/y source offsets back into the src rectangle so
> now it's just subtraacting the same value from itself. The code would
> never have worked if we allowed the framebuffer to have a non-zero
> offset.
> 
> Let's do this in a better way by just calculating the fence_y_offset
> from the final plane surface offset. Note that we don't align the
> plane surface address to fence rows so with horizontal panning there's
> often a horizontal offset from the fence start to the surface address
> as well. We have no way to tell the hardware about that so we just
> ignore it. Based on some quick tests the invlidation still happens
> correctly. I presume due to the invalidation nuking at least the full
> line (or a segment of multiple lines).
> 
> Fixes: 54d4d719fa11 ("drm/i915: Overcome display engine stride limits via GTT remapping")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 11 +++++++
>  drivers/gpu/drm/i915/display/intel_display.h |  1 +
>  drivers/gpu/drm/i915/display/intel_fbc.c     | 32 ++++++--------------
>  drivers/gpu/drm/i915/i915_drv.h              |  6 ++--
>  4 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6bb87965801e..e5fa49337883 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3822,6 +3822,17 @@ skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state,
>  	return true;
>  }
>  
> +unsigned int
> +intel_plane_fence_y_offset(const struct intel_plane_state *plane_state)
> +{
> +	int x = 0, y = 0;
> +
> +	intel_plane_adjust_aligned_offset(&x, &y, plane_state, 0,
> +					  plane_state->color_plane[0].offset, 0);
> +
> +	return y;
> +}
> +
>  static int skl_check_main_surface(struct intel_plane_state *plane_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane_state->uapi.plane->dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index efb4da205ea2..3a06f72c9859 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -608,6 +608,7 @@ unsigned int i9xx_plane_max_stride(struct intel_plane *plane,
>  				   u32 pixel_format, u64 modifier,
>  				   unsigned int rotation);
>  int bdw_get_pipemisc_bpp(struct intel_crtc *crtc);
> +unsigned int intel_plane_fence_y_offset(const struct intel_plane_state *plane_state);
>  
>  struct intel_display_error_state *
>  intel_display_capture_error_state(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 192c5ff142ee..613ab499d42e 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -47,19 +47,6 @@
>  #include "intel_fbc.h"
>  #include "intel_frontbuffer.h"
>  
> -/*
> - * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
> - * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
> - * origin so the x and y offsets can actually fit the registers. As a
> - * consequence, the fence doesn't really start exactly at the display plane
> - * address we program because it starts at the real start of the buffer, so we
> - * have to take this into consideration here.
> - */
> -static unsigned int get_crtc_fence_y_offset(struct intel_fbc *fbc)
> -{
> -	return fbc->state_cache.plane.y - fbc->state_cache.plane.adjusted_y;
> -}
> -
>  /*
>   * For SKL+, the plane source size used by the hardware is based on the value we
>   * write to the PLANE_SIZE register. For BDW-, the hardware looks at the value
> @@ -141,7 +128,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
>  			fbc_ctl2 |= FBC_CTL_CPU_FENCE;
>  		intel_de_write(dev_priv, FBC_CONTROL2, fbc_ctl2);
>  		intel_de_write(dev_priv, FBC_FENCE_OFF,
> -			       params->crtc.fence_y_offset);
> +			       params->fence_y_offset);
>  	}
>  
>  	/* enable it... */
> @@ -175,7 +162,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
>  	if (params->fence_id >= 0) {
>  		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fence_id;
>  		intel_de_write(dev_priv, DPFC_FENCE_YOFF,
> -			       params->crtc.fence_y_offset);
> +			       params->fence_y_offset);
>  	} else {
>  		intel_de_write(dev_priv, DPFC_FENCE_YOFF, 0);
>  	}
> @@ -243,7 +230,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  			intel_de_write(dev_priv, SNB_DPFC_CTL_SA,
>  				       SNB_CPU_FENCE_ENABLE | params->fence_id);
>  			intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET,
> -				       params->crtc.fence_y_offset);
> +				       params->fence_y_offset);
>  		}
>  	} else {
>  		if (IS_GEN(dev_priv, 6)) {
> @@ -253,7 +240,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  	}
>  
>  	intel_de_write(dev_priv, ILK_DPFC_FENCE_YOFF,
> -		       params->crtc.fence_y_offset);
> +		       params->fence_y_offset);
>  	/* enable it... */
>  	intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
>  
> @@ -320,7 +307,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  		intel_de_write(dev_priv, SNB_DPFC_CTL_SA,
>  			       SNB_CPU_FENCE_ENABLE | params->fence_id);
>  		intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET,
> -			       params->crtc.fence_y_offset);
> +			       params->fence_y_offset);
>  	} else if (dev_priv->ggtt.num_fences) {
>  		intel_de_write(dev_priv, SNB_DPFC_CTL_SA, 0);
>  		intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET, 0);
> @@ -628,8 +615,8 @@ static bool rotation_is_valid(struct drm_i915_private *dev_priv,
>  /*
>   * For some reason, the hardware tracking starts looking at whatever we
>   * programmed as the display plane base address register. It does not look at
> - * the X and Y offset registers. That's why we look at the crtc->adjusted{x,y}
> - * variables instead of just looking at the pipe/plane size.
> + * 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)
>  {
> @@ -702,7 +689,6 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  	cache->plane.src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
>  	cache->plane.adjusted_x = plane_state->color_plane[0].x;
>  	cache->plane.adjusted_y = plane_state->color_plane[0].y;
> -	cache->plane.y = plane_state->uapi.src.y1 >> 16;
>  
>  	cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
>  
> @@ -710,6 +696,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  	cache->fb.modifier = fb->modifier;
>  	cache->fb.stride = plane_state->color_plane[0].stride;
>  
> +	cache->fence_y_offset = intel_plane_fence_y_offset(plane_state);
> +
>  	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
>  		    !plane_state->vma->fence);
>  
> @@ -880,10 +868,10 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
>  	memset(params, 0, sizeof(*params));
>  
>  	params->fence_id = cache->fence_id;
> +	params->fence_y_offset = cache->fence_y_offset;
>  
>  	params->crtc.pipe = crtc->pipe;
>  	params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)->i9xx_plane;
> -	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
>  
>  	params->fb.format = cache->fb.format;
>  	params->fb.stride = cache->fb.stride;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b00f0845cbc3..a634fd2330c3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -408,8 +408,6 @@ struct intel_fbc {
>  			int adjusted_x;
>  			int adjusted_y;
>  
> -			int y;
> -
>  			u16 pixel_blend_mode;
>  		} plane;
>  
> @@ -418,6 +416,8 @@ struct intel_fbc {
>  			unsigned int stride;
>  			u64 modifier;
>  		} fb;
> +
> +		unsigned int fence_y_offset;
>  		u16 gen9_wa_cfb_stride;
>  		s8 fence_id;
>  	} state_cache;
> @@ -433,7 +433,6 @@ struct intel_fbc {
>  		struct {
>  			enum pipe pipe;
>  			enum i9xx_plane_id i9xx_plane;
> -			unsigned int fence_y_offset;
>  		} crtc;
>  
>  		struct {
> @@ -442,6 +441,7 @@ struct intel_fbc {
>  		} fb;
>  
>  		int cfb_size;
> +		unsigned int fence_y_offset;
>  		u16 gen9_wa_cfb_stride;
>  		s8 fence_id;
>  		bool plane_visible;
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/12] drm/i915/fbc: Fix nuke for pre-snb platforms

[Matt Roper]

On Wed, Apr 29, 2020 at 01:10:26PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The MSG_FBC_REND_STATE register only exists on snb+. For older

I only find this register in the bspec for HSW+.  Is the spec incomplete
or am I looking in the wrong place?

It's a bit hard to review these changes for older platforms since there
doesn't really seem to be much FBC/DPFC documentation at all in the
bspec until we get to BDW and beyond.  The only explicit mention I can
find of nuke-on-flip for older platforms is a SNB-specific bit in
FBC_CTL that disables that behavior.  Do you have other documents that
clarify that this will indeed work farther back?


Matt

> platforms (would also work for snb+) we can simply rewite DSPSURF
> to trigger a flip nuke.
> 
> While generally RMW is considered harmful we'll use it here for
> simplicity. And since FBC doesn't exist in i830 we don't have to
> worry about the DSPSURF double buffering hardware fails present
> on that platform.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 34 +++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 613ab499d42e..983224e07eaf 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -188,8 +188,30 @@ static bool g4x_fbc_is_active(struct drm_i915_private *dev_priv)
>  	return intel_de_read(dev_priv, DPFC_CONTROL) & DPFC_CTL_EN;
>  }
>  
> +static void i8xx_fbc_recompress(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> +	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> +
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
> +			  intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
> +	spin_unlock_irq(&dev_priv->uncore.lock);
> +}
> +
> +static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> +	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> +
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
> +			  intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
> +	spin_unlock_irq(&dev_priv->uncore.lock);
> +}
> +
>  /* This function forces a CFB recompression through the nuke operation. */
> -static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> +static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_fbc *fbc = &dev_priv->fbc;
>  
> @@ -199,6 +221,16 @@ static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
>  	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
>  }
>  
> +static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> +{
> +	if (INTEL_GEN(dev_priv) >= 6)
> +		snb_fbc_recompress(dev_priv);
> +	else if (INTEL_GEN(dev_priv) >= 4)
> +		i965_fbc_recompress(dev_priv);
> +	else
> +		i8xx_fbc_recompress(dev_priv);
> +}
> +
>  static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 02/12] drm/i915/fbc: Use the correct plane stride

[Ville Syrjälä]

On Fri, May 01, 2020 at 05:16:13PM -0700, Matt Roper wrote:
> On Wed, Apr 29, 2020 at 06:29:21PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Consult the actual plane stride instead of the fb stride. The two
> > will disagree when we remap the gtt. The plane stride is what the
> > hw will be fed so that's what we should look at for the FBC
> > retrictions/cfb allocation.
> > 
> > Since we no longer require a fence we are going to attempt using
> > FBC with remapping, and so we should look at correct stride.
> > 
> > With 90/270 degree rotation the plane stride is stored in units
> > of pixels, so we need to conver it to bytes for the purposes
> > of calculating the cfb stride. Not entirely sure if this matches
> > the hw behaviour though. Need to reverse engineer that at some
> > point...
> > 
> > We also need to reorder the pixel format check vs. stride check
> > to avoid triggering a spurious WARN(stride & 63) with cpp==1 and
> > plane stride==32.
> > 
> > v2: Try to deal with rotated stride and related WARN
> > 
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for GEN9+")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 7194f9bc62c5..7f2b2382b813 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -707,9 +707,13 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
> >  	cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
> >  
> >  	cache->fb.format = fb->format;
> > -	cache->fb.stride = fb->pitches[0];
> >  	cache->fb.modifier = fb->modifier;
> >  
> > +	/* FIXME is this correct? */
> > +	cache->fb.stride = plane_state->color_plane[0].stride;
> 
> We still have a comment in intel_fbc_calculate_cfb_size() that indicates
> that we need to use the framebuffer stride instead of the plane stride
> (explicitly added in commit 850bfaab7120a).

That's not really what it's saying. full buffer stride == plane stride,
vs. active area == plane width

> The bspec (page 49227) uses
> terminology "Stride of plane uncompressed surface" which sounds like
> framebuffer size to me; I'm not sure if switching it to the plane's size
> will cause problems if the plane is only scanning out a subregion of the
> framebuffer?

There is no framebuffer stride as far as the hardware is concerned.
There is only plane width and plane stride.

> 
> If it really is safe to use the plane size instead of the framebuffer
> size, then I think we at least need to remove or change that comment
> too.
> 
> 
> Matt
> 
> > +	if (drm_rotation_90_or_270(plane_state->hw.rotation))
> > +		cache->fb.stride *= fb->format->cpp[0];
> > +
> >  	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
> >  		    !plane_state->vma->fence);
> >  
> > @@ -804,6 +808,11 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> >  		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";
> > @@ -820,11 +829,6 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> >  		return false;
> >  	}
> >  
> > -	if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
> > -		fbc->no_fbc_reason = "pixel format is invalid";
> > -		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";
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795

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

Re: [Intel-gfx] [PATCH 04/12] drm/i915/fbc: Fix nuke for pre-snb platforms

[Ville Syrjälä]

On Fri, May 01, 2020 at 06:18:18PM -0700, Matt Roper wrote:
> On Wed, Apr 29, 2020 at 01:10:26PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The MSG_FBC_REND_STATE register only exists on snb+. For older
> 
> I only find this register in the bspec for HSW+.  Is the spec incomplete
> or am I looking in the wrong place?

The docs are a bit of a mess around this area. IIRC this rcs nuke
workaround was documented for ivb+ (presumably due to ppgtt).
I thinka the bltter counterpart (part of the BCS_ECOSKPD dance)
was documented for SNB as well which implies the register is there
and working. Also the fact that the code works does confirm that.

We're not really following much of the documented stuff for 
FBC since we basically don't use the hardware tracking all.
So the value of the docs is mostly in finding the right bits
to cause nukes and turn off hw tracking as much as possible.

> 
> It's a bit hard to review these changes for older platforms since there
> doesn't really seem to be much FBC/DPFC documentation at all in the
> bspec until we get to BDW and beyond.  The only explicit mention I can
> find of nuke-on-flip for older platforms is a SNB-specific bit in
> FBC_CTL that disables that behavior.  Do you have other documents that
> clarify that this will indeed work farther back?

gen2-gen4 bspec has slightly better docs on FBC compared to more recent
platforms. Sadly I've never been able to find a way to trigger a nuke
explicitly, hence we resort to (ab)using a flip nuke.

> 
> 
> Matt
> 
> > platforms (would also work for snb+) we can simply rewite DSPSURF
> > to trigger a flip nuke.
> > 
> > While generally RMW is considered harmful we'll use it here for
> > simplicity. And since FBC doesn't exist in i830 we don't have to
> > worry about the DSPSURF double buffering hardware fails present
> > on that platform.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c | 34 +++++++++++++++++++++++-
> >  1 file changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 613ab499d42e..983224e07eaf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -188,8 +188,30 @@ static bool g4x_fbc_is_active(struct drm_i915_private *dev_priv)
> >  	return intel_de_read(dev_priv, DPFC_CONTROL) & DPFC_CTL_EN;
> >  }
> >  
> > +static void i8xx_fbc_recompress(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> > +	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> > +
> > +	spin_lock_irq(&dev_priv->uncore.lock);
> > +	intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
> > +			  intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
> > +	spin_unlock_irq(&dev_priv->uncore.lock);
> > +}
> > +
> > +static void i965_fbc_recompress(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> > +	enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> > +
> > +	spin_lock_irq(&dev_priv->uncore.lock);
> > +	intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
> > +			  intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
> > +	spin_unlock_irq(&dev_priv->uncore.lock);
> > +}
> > +
> >  /* This function forces a CFB recompression through the nuke operation. */
> > -static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> > +static void snb_fbc_recompress(struct drm_i915_private *dev_priv)
> >  {
> >  	struct intel_fbc *fbc = &dev_priv->fbc;
> >  
> > @@ -199,6 +221,16 @@ static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> >  	intel_de_posting_read(dev_priv, MSG_FBC_REND_STATE);
> >  }
> >  
> > +static void intel_fbc_recompress(struct drm_i915_private *dev_priv)
> > +{
> > +	if (INTEL_GEN(dev_priv) >= 6)
> > +		snb_fbc_recompress(dev_priv);
> > +	else if (INTEL_GEN(dev_priv) >= 4)
> > +		i965_fbc_recompress(dev_priv);
> > +	else
> > +		i8xx_fbc_recompress(dev_priv);
> > +}
> > +
> >  static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
> >  {
> >  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795

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

Re: [Intel-gfx] [PATCH 08/12] drm/i915/fbc: Parametrize FBC_CONTROL

[Souza, Jose]

On Wed, 2020-04-29 at 13:10 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Parametrize the FBC_CONTROL bits for neater code.
> 
> Also add the one missing bit: "stop compression on modification".
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c |  8 ++++----
>  drivers/gpu/drm/i915/i915_reg.h          | 18 +++++++++++-------
>  2 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 56eeafa645de..dbef58af4b94 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -133,13 +133,13 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
>  
>  	/* enable it... */
>  	fbc_ctl = intel_de_read(dev_priv, FBC_CONTROL);
> -	fbc_ctl &= 0x3fff << FBC_CTL_INTERVAL_SHIFT;
> +	fbc_ctl &= FBC_CTL_INTERVAL(0x3fff);
>  	fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
>  	if (IS_I945GM(dev_priv))
>  		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
> -	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
> +	fbc_ctl |= FBC_CTL_STRIDE(cfb_pitch & 0xff);
>  	if (params->fence_id >= 0)
> -		fbc_ctl |= params->fence_id;
> +		fbc_ctl |= FBC_CTL_FENCENO(params->fence_id);
>  	intel_de_write(dev_priv, FBC_CONTROL, fbc_ctl);
>  }
>  
> @@ -1452,7 +1452,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
>  	/* This value was pulled out of someone's hat */
>  	if (INTEL_GEN(dev_priv) <= 4 && !IS_GM45(dev_priv))
>  		intel_de_write(dev_priv, FBC_CONTROL,
> -		               500 << FBC_CTL_INTERVAL_SHIFT);
> +			       FBC_CTL_INTERVAL(500));
>  
>  	/* We still don't have any sort of hardware state readout for FBC, so
>  	 * deactivate it in case the BIOS activated it to make sure software
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 088215025661..e9fb64e8f28f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3194,13 +3194,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define FBC_CFB_BASE		_MMIO(0x3200) /* 4k page aligned */
>  #define FBC_LL_BASE		_MMIO(0x3204) /* 4k page aligned */
>  #define FBC_CONTROL		_MMIO(0x3208)
> -#define   FBC_CTL_EN		(1 << 31)
> -#define   FBC_CTL_PERIODIC	(1 << 30)
> -#define   FBC_CTL_INTERVAL_SHIFT (16)
> -#define   FBC_CTL_UNCOMPRESSIBLE (1 << 14)
> -#define   FBC_CTL_C3_IDLE	(1 << 13)
> -#define   FBC_CTL_STRIDE_SHIFT	(5)
> -#define   FBC_CTL_FENCENO_SHIFT	(0)
> +#define   FBC_CTL_EN		REG_BIT(31)
> +#define   FBC_CTL_PERIODIC	REG_BIT(30)
> +#define   FBC_CTL_INTERVAL_MASK	REG_GENMASK(29, 16)
> +#define   FBC_CTL_INTERVAL(x)	REG_FIELD_PREP(FBC_CTL_INTERVAL_MASK, (x))
> +#define   FBC_CTL_STOP_ON_MOD	REG_BIT(15)
> +#define   FBC_CTL_UNCOMPRESSIBLE REG_BIT(14) /* i915+ */
> +#define   FBC_CTL_C3_IDLE	REG_BIT(13) /* i945gm */
> +#define   FBC_CTL_STRIDE_MASK	REG_GENMASK(12, 5)
> +#define   FBC_CTL_STRIDE(x)	REG_FIELD_PREP(FBC_CTL_STRIDE_MASK, (x))
> +#define   FBC_CTL_FENCENO_MASK	REG_GENMASK(3, 0)
> +#define   FBC_CTL_FENCENO(x)	REG_FIELD_PREP(FBC_CTL_FENCENO_MASK, (x))
>  #define FBC_COMMAND		_MMIO(0x320c)
>  #define   FBC_CMD_COMPRESS	(1 << 0)
>  #define FBC_STATUS		_MMIO(0x3210)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/12] drm/i915/fbc: Store the fbc1 compression interval in the params

[Souza, Jose]

On Wed, 2020-04-29 at 13:10 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Avoid the FBC_CONTROL rmw and just store the fbc compression
> interval in the params/

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 13 ++++++-------
>  drivers/gpu/drm/i915/i915_drv.h          |  2 ++
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index dbef58af4b94..b1eb6a2ecc43 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -132,8 +132,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
>  	}
>  
>  	/* enable it... */
> -	fbc_ctl = intel_de_read(dev_priv, FBC_CONTROL);
> -	fbc_ctl &= FBC_CTL_INTERVAL(0x3fff);
> +	fbc_ctl = FBC_CTL_INTERVAL(params->interval);

CI results are good so no need to keep any bit that we don't touch set.

>  	fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
>  	if (IS_I945GM(dev_priv))
>  		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
> @@ -728,6 +727,9 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  	cache->fb.modifier = fb->modifier;
>  	cache->fb.stride = plane_state->color_plane[0].stride;
>  
> +	/* This value was pulled out of someone's hat */
> +	cache->interval = 500;
> +
>  	cache->fence_y_offset = intel_plane_fence_y_offset(plane_state);
>  
>  	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
> @@ -902,6 +904,8 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
>  	params->fence_id = cache->fence_id;
>  	params->fence_y_offset = cache->fence_y_offset;
>  
> +	params->interval = cache->interval;
> +
>  	params->crtc.pipe = crtc->pipe;
>  	params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)->i9xx_plane;
>  
> @@ -1449,11 +1453,6 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
>  		return;
>  	}
>  
> -	/* This value was pulled out of someone's hat */
> -	if (INTEL_GEN(dev_priv) <= 4 && !IS_GM45(dev_priv))
> -		intel_de_write(dev_priv, FBC_CONTROL,
> -			       FBC_CTL_INTERVAL(500));
> -
>  	/* We still don't have any sort of hardware state readout for FBC, so
>  	 * deactivate it in case the BIOS activated it to make sure software
>  	 * matches the hardware state. */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a634fd2330c3..bc66a7cb886b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -419,6 +419,7 @@ struct intel_fbc {
>  
>  		unsigned int fence_y_offset;
>  		u16 gen9_wa_cfb_stride;
> +		u16 interval;
>  		s8 fence_id;
>  	} state_cache;
>  
> @@ -443,6 +444,7 @@ struct intel_fbc {
>  		int cfb_size;
>  		unsigned int fence_y_offset;
>  		u16 gen9_wa_cfb_stride;
> +		u16 interval;
>  		s8 fence_id;
>  		bool plane_visible;
>  	} params;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/12] drm/i915/fbc: Reduce fbc1 compression interval to 1 second

[Souza, Jose]

On Wed, 2020-04-29 at 13:10 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The default fbc1 compression interval we use is 500 frames. That
> translates to over 8 seconds typically. That's rather excessive
> so let's drop it to 1 second.
> 
> The hardware will not attempt recompression unless at least one
> line has been modified, so a shorter compression interval should
> not cause extra bandwidth use in the purely idle scenario. Of
> course in the mostly idle case we are possibly going to recompress
> a bit more.
> 
> Should really try to find some kind of sweet spot to minimize
> the energy usage...

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index b1eb6a2ecc43..6ee45d634cf6 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -727,8 +727,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  	cache->fb.modifier = fb->modifier;
>  	cache->fb.stride = plane_state->color_plane[0].stride;
>  
> -	/* This value was pulled out of someone's hat */
> -	cache->interval = 500;
> +	/* FBC1 compression interval: arbitrary choice of 1 second */
> +	cache->interval = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode);
>  
>  	cache->fence_y_offset = intel_plane_fence_y_offset(plane_state);
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/12] drm/i915: Suppress spurious underruns on gen2

[Souza, Jose]

On Wed, 2020-04-29 at 13:10 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Often we seem to detect an underrun right after modeset on gen2.
> It seems to be a spurious detection (potentially the pipe is still
> in a wonky state when we enable the planes). An extra vblank wait
> seems to cure it.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a0d1057d75ee..f330054e64c5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7517,6 +7517,10 @@ static void i9xx_crtc_enable(struct intel_atomic_state *state,
>  	intel_crtc_vblank_on(new_crtc_state);
>  
>  	intel_encoders_enable(state, crtc);
> +
> +	/* prevents spurious underruns */
> +	if (IS_GEN(dev_priv, 2))
> +		intel_wait_for_vblank(dev_priv, pipe);
>  }
>  
>  static void i9xx_pfit_disable(const struct intel_crtc_state *old_crtc_state)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/12] drm/i915: Fix g4x fbc watermark enable

[Souza, Jose]

On Wed, 2020-04-29 at 13:10 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 'level' here means the highest level we can't use, so when checking
> the fbc watermarks we need a -1 to get at the last enabled level.
> 
> While at if refactor the code a bit to declutter
> g4x_compute_pipe_wm().
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1e99b35f007e..1c92ebf64a34 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1344,6 +1344,23 @@ static void g4x_invalidate_wms(struct intel_crtc *crtc,
>  	}
>  }
>  
> +static bool g4x_compute_fbc_en(const struct g4x_wm_state *wm_state,
> +			       int level)
> +{
> +	if (level < G4X_WM_LEVEL_SR)
> +		return false;
> +
> +	if (level >= G4X_WM_LEVEL_SR &&
> +	    wm_state->sr.fbc > g4x_fbc_fifo_size(G4X_WM_LEVEL_SR))
> +		return false;
> +
> +	if (level >= G4X_WM_LEVEL_HPLL &&
> +	    wm_state->hpll.fbc > g4x_fbc_fifo_size(G4X_WM_LEVEL_HPLL))
> +		return false;
> +
> +	return true;
> +}
> +
>  static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -1383,7 +1400,6 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
>  		wm_state->wm.plane[plane_id] = raw->plane[plane_id];
>  
>  	level = G4X_WM_LEVEL_SR;
> -
>  	if (!g4x_raw_crtc_wm_is_valid(crtc_state, level))
>  		goto out;
>  
> @@ -1395,7 +1411,6 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
>  	wm_state->cxsr = num_active_planes == BIT(PLANE_PRIMARY);
>  
>  	level = G4X_WM_LEVEL_HPLL;
> -
>  	if (!g4x_raw_crtc_wm_is_valid(crtc_state, level))
>  		goto out;
>  
> @@ -1418,17 +1433,11 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
>  	/*
>  	 * Determine if the FBC watermark(s) can be used. IF
>  	 * this isn't the case we prefer to disable the FBC
> -	 ( watermark(s) rather than disable the SR/HPLL
> -	 * level(s) entirely.
> +	 * watermark(s) rather than disable the SR/HPLL
> +	 * level(s) entirely. 'level-1' is the highest valid
> +	 * level here.
>  	 */
> -	wm_state->fbc_en = level > G4X_WM_LEVEL_NORMAL;
> -
> -	if (level >= G4X_WM_LEVEL_SR &&
> -	    wm_state->sr.fbc > g4x_fbc_fifo_size(G4X_WM_LEVEL_SR))
> -		wm_state->fbc_en = false;
> -	else if (level >= G4X_WM_LEVEL_HPLL &&
> -		 wm_state->hpll.fbc > g4x_fbc_fifo_size(G4X_WM_LEVEL_HPLL))
> -		wm_state->fbc_en = false;
> +	wm_state->fbc_en = g4x_compute_fbc_en(wm_state, level - 1);
>  
>  	return 0;
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/12] drm/i915/fbc: Don't clear busy_bits for origin==GTT

[Souza, Jose]

On Wed, 2020-04-29 at 13:10 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The hardware host tracking won't nuke the entire cfb (unless the
> entire fb is written through the gtt) so don't clear the busy_bits
> for gtt tracking.
> 
> Not that it really matters anymore since we've lost ORIGIN_GTT usage
> everywhere.

Maybe drop it then? But for now this change looks good.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 983224e07eaf..56eeafa645de 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1107,11 +1107,19 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>  	if (!HAS_FBC(dev_priv))
>  		return;
>  
> +	/*
> +	 * GTT tracking does not nuke the entire cfb
> +	 * so don't clear busy_bits set for some other
> +	 * reason.
> +	 */
> +	if (origin == ORIGIN_GTT)
> +		return;
> +
>  	mutex_lock(&fbc->lock);
>  
>  	fbc->busy_bits &= ~frontbuffer_bits;
>  
> -	if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
> +	if (origin == ORIGIN_FLIP)
>  		goto out;
>  
>  	if (!fbc->busy_bits && fbc->crtc &&
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx