[Intel-gfx] [PATCH v2 1/3] drm/i915/display/fbc: Make fences a nice-to-have for GEN11+

* [Intel-gfx] [PATCH v2 1/3] drm/i915/display/fbc: Make fences a nice-to-have for GEN11+
@ 2020-02-06  2:08 José Roberto de Souza
  2020-02-06  2:08 ` [Intel-gfx] [PATCH v2 2/3] drm/i915/display: Do not write in removed FBC fence registers José Roberto de Souza
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: José Roberto de Souza @ 2020-02-06  2:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Dhinakaran Pandiyan

dGFX have local memory so it do not have aperture and do not support
CPU fences but even for iGFX it have a small number of fences.

As replacement for fences to track frontbuffer modifications by CPU
we have a software tracking that is already in used by FBC and PSR.
PSR don't support fences so it shows that this tracking is reliable.

So lets make fences a nice-to-have to activate FBC for GEN11+, this
will allow us to enable FBC for dGFXs and iGFXs even when there is no
available fence.

We do not set fences to rotated planes but FBC only have restrictions
against 16bpp, so adding it here.

Also adding a new check for the tiling format, fences are only set
to X and Y tiled planes but again FBC don't have any restrictions
against tiling so adding linear as supported as well, other formats
should be added after tested but IGT only supports drawing in thse
3 formats.

intel_fbc_hw_tracking_covers_screen() maybe can also have the same
treatment as fences but BSpec is not clear if the size limitation is
for hardware tracking or general use of FBC and I don't have a 5K
display to test it, so keeping as is for safety.

v2:
- Added tiling and pixel format rotation checks
- Changed the GEN version not requiring fences to 11 from 9, DDX
needs some changes but it don't have support for GEN11+

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 42 ++++++++++++++++++++----
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index ddf8d3bb7a7d..3a9e41e93ebf 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -585,7 +585,7 @@ static bool stride_is_valid(struct drm_i915_private *dev_priv,
 }
 
 static bool pixel_format_is_valid(struct drm_i915_private *dev_priv,
-				  u32 pixel_format)
+				  u32 pixel_format, unsigned int rotation)
 {
 	switch (pixel_format) {
 	case DRM_FORMAT_XRGB8888:
@@ -599,6 +599,9 @@ static bool pixel_format_is_valid(struct drm_i915_private *dev_priv,
 		/* WaFbcOnly1to1Ratio:ctg */
 		if (IS_G4X(dev_priv))
 			return false;
+		if ((rotation & (DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270)) &&
+		    INTEL_GEN(dev_priv) >= 9)
+			return false;
 		return true;
 	default:
 		return false;
@@ -639,6 +642,18 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
 	return effective_w <= max_w && effective_h <= max_h;
 }
 
+static bool tiling_is_valid(uint64_t modifier)
+{
+	switch (modifier) {
+	case DRM_FORMAT_MOD_LINEAR:
+	case I915_FORMAT_MOD_X_TILED:
+	case I915_FORMAT_MOD_Y_TILED:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 					 const struct intel_crtc_state *crtc_state,
 					 const struct intel_plane_state *plane_state)
@@ -672,6 +687,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 
 	cache->fb.format = fb->format;
 	cache->fb.stride = fb->pitches[0];
+	cache->fb.modifier = fb->modifier;
 
 	drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
 		    !plane_state->vma->fence);
@@ -720,23 +736,34 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 		return false;
 	}
 
-	/* The use of a CPU fence is mandatory in order to detect writes
-	 * by the CPU to the scanout and trigger updates to the FBC.
+	/* The use of a CPU fence is one of two ways to detect writes by the
+	 * CPU to the scanout and trigger updates to the FBC.
+	 *
+	 * The other method is by software tracking(see
+	 * intel_fbc_invalidate/flush()), it will manually notify FBC and nuke
+	 * the current compressed buffer and recompress it.
 	 *
 	 * Note that is possible for a tiled surface to be unmappable (and
-	 * so have no fence associated with it) due to aperture constaints
+	 * so have no fence associated with it) due to aperture constraints
 	 * at the time of pinning.
 	 *
 	 * FIXME with 90/270 degree rotation we should use the fence on
 	 * the normal GTT view (the rotated view doesn't even have a
 	 * fence). Would need changes to the FBC fence Y offset as well.
-	 * For now this will effecively disable FBC with 90/270 degree
+	 * For now this will effectively disable FBC with 90/270 degree
 	 * rotation.
 	 */
-	if (cache->fence_id < 0) {
+	if (INTEL_GEN(dev_priv) < 11 && cache->fence_id < 0) {
 		fbc->no_fbc_reason = "framebuffer not tiled or fenced";
 		return false;
 	}
+
+	/* Only check tiling for platforms that fence is not mandatory */
+	if (INTEL_GEN(dev_priv) >= 11 && !tiling_is_valid(cache->fb.modifier)) {
+		fbc->no_fbc_reason = "tiling unsupported";
+		return false;
+	}
+
 	if (INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv) &&
 	    cache->plane.rotation != DRM_MODE_ROTATE_0) {
 		fbc->no_fbc_reason = "rotation unsupported";
@@ -748,7 +775,8 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 		return false;
 	}
 
-	if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
+	if (!pixel_format_is_valid(dev_priv, cache->fb.format->format,
+				   cache->plane.rotation)) {
 		fbc->no_fbc_reason = "pixel format is invalid";
 		return false;
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3452926d7b77..a481a0454e69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -413,6 +413,7 @@ struct intel_fbc {
 		struct {
 			const struct drm_format_info *format;
 			unsigned int stride;
+			uint64_t modifier;
 		} fb;
 		u16 gen9_wa_cfb_stride;
 		s8 fence_id;
-- 
2.25.0

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

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