Re: [Intel-xe] [PATCH] drm/xe/display: Do not use i915 frontbuffer tracking implementation

Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> · Mon, 6 Mar 2023 21:23:50 +0100

Hey,

On 2023-03-06 16:23, Souza, Jose wrote:
On Mon, 2023-03-06 at 15:16 +0100, Maarten Lankhorst wrote:
As a fallback if we decide not to merge the frontbuffer tracking, allow
i915 to keep its own implementation, and do the right thing in Xe.

The frontbuffer tracking for Xe is still done per-fb, while i915 can
keep doing the weird intel_frontbuffer + i915_active thing without
blocking Xe.
Please also disable PSR and FBC with this or at least add a way for users to disable those features.
Without frontbuffer tracker those two features will break in some cases.

FBC and PSR work completely as expected. I don't remove frontbuffer 
tracking; I only remove the GEM parts.

Explicit invalidation using pageflip or CPU rendering + DirtyFB continue 
to work, as I validated on my laptop with FBC.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
---
  .../gpu/drm/i915/display/intel_display_types.h  | 12 ++++++++++++
  drivers/gpu/drm/i915/display/intel_drrs.c       |  1 +
  drivers/gpu/drm/i915/display/intel_fb.c         |  8 +++++---
  drivers/gpu/drm/i915/display/intel_fbdev.c      |  2 +-
  .../gpu/drm/i915/display/intel_frontbuffer.c    | 17 +++++++++++++----
  .../gpu/drm/i915/display/intel_frontbuffer.h    | 12 ++++++++++--
  drivers/gpu/drm/i915/display/intel_psr.c        |  1 +
  .../gpu/drm/i915/display/skl_universal_plane.c  |  2 ++
  8 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index f2918bb07107..a4a57aa24422 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -133,7 +133,11 @@ struct intel_fb_view {
  
  struct intel_framebuffer {
  	struct drm_framebuffer base;
+#ifdef I915
  	struct intel_frontbuffer *frontbuffer;
+#else
+	atomic_t bits;
+#endif
  
  	/* Params to remap the FB pages and program the plane registers in each view. */
  	struct intel_fb_view normal_view;
@@ -2074,10 +2078,18 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *plane_
  #endif
  }
  
+#ifdef I915
  static inline struct intel_frontbuffer *
  to_intel_frontbuffer(struct drm_framebuffer *fb)
  {
  	return fb ? to_intel_framebuffer(fb)->frontbuffer : NULL;
  }
+#else
+static inline struct intel_framebuffer *
+to_intel_frontbuffer(struct drm_framebuffer *fb)
+{
+	return fb ? to_intel_framebuffer(fb) : NULL;
+}
+#endif
  
  #endif /*  __INTEL_DISPLAY_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c
index 5b9e44443814..3503d112387d 100644
--- a/drivers/gpu/drm/i915/display/intel_drrs.c
+++ b/drivers/gpu/drm/i915/display/intel_drrs.c
@@ -9,6 +9,7 @@
  #include "intel_de.h"
  #include "intel_display_types.h"
  #include "intel_drrs.h"
+#include "intel_frontbuffer.h"
  #include "intel_panel.h"
  
  /**
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
index 8c357a4098f6..e67c71f9b29d 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -1846,6 +1846,8 @@ static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
  #ifdef I915
  	if (intel_fb_uses_dpt(fb))
  		intel_dpt_destroy(intel_fb->dpt_vm);
+
+	intel_frontbuffer_put(intel_fb->frontbuffer);
  #else
  	if (intel_fb_obj(fb)->flags & XE_BO_CREATE_PINNED_BIT) {
  		struct xe_bo *bo = intel_fb_obj(fb);
@@ -1857,8 +1859,6 @@ static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
  	}
  #endif
  
-	intel_frontbuffer_put(intel_fb->frontbuffer);
-
  	kfree(intel_fb);
  }
  
@@ -1966,9 +1966,9 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
  		obj->flags |= XE_BO_SCANOUT_BIT;
  	}
  	ttm_bo_unreserve(&obj->ttm);
-#endif
  
  	atomic_set(&intel_fb->bits, 0);
+#endif
  
  	if (!drm_any_plane_has_format(&dev_priv->drm,
  				      mode_cmd->pixel_format,
@@ -2085,7 +2085,9 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
  	return 0;
  
  err:
+#ifdef I915
  	intel_frontbuffer_put(intel_fb->frontbuffer);
+#endif
  	return ret;
  }
  
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 75d8029185f0..2682b26b511f 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -82,7 +82,7 @@ struct intel_fbdev {
  
  static struct intel_frontbuffer *to_frontbuffer(struct intel_fbdev *ifbdev)
  {
-	return ifbdev->fb->frontbuffer;
+	return to_intel_frontbuffer(&ifbdev->fb->base);
  }
  
  static void intel_fbdev_invalidate(struct intel_fbdev *ifbdev)
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 17a7aa8b28c2..64fe73d2ac4d 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -163,11 +163,17 @@ void intel_frontbuffer_flip(struct drm_i915_private *i915,
  	frontbuffer_flush(i915, frontbuffer_bits, ORIGIN_FLIP);
  }
  
+#ifdef I915
+#define intel_front_obj(front) ((front)->obj)
+#else
+#define intel_front_obj(front) (front)
+#endif
+
  void __intel_fb_invalidate(struct intel_frontbuffer *front,
  			   enum fb_op_origin origin,
  			   unsigned int frontbuffer_bits)
  {
-	struct drm_i915_private *i915 = to_i915(front->obj->base.dev);
+	struct drm_i915_private *i915 = to_i915(intel_front_obj(front)->base.dev);
  
  	if (origin == ORIGIN_CS) {
  		spin_lock(&i915->display.fb_tracking.lock);
@@ -188,7 +194,7 @@ void __intel_fb_flush(struct intel_frontbuffer *front,
  		      enum fb_op_origin origin,
  		      unsigned int frontbuffer_bits)
  {
-	struct drm_i915_private *i915 = to_i915(front->obj->base.dev);
+	struct drm_i915_private *i915 = to_i915(intel_front_obj(front)->base.dev);
  
  	if (origin == ORIGIN_CS) {
  		spin_lock(&i915->display.fb_tracking.lock);
@@ -202,6 +208,7 @@ void __intel_fb_flush(struct intel_frontbuffer *front,
  		frontbuffer_flush(i915, frontbuffer_bits, origin);
  }
  
+#ifdef I915
  static int frontbuffer_active(struct i915_active *ref)
  {
  	struct intel_frontbuffer *front =
@@ -289,6 +296,8 @@ void intel_frontbuffer_put(struct intel_frontbuffer *front)
  		      &to_i915(front->obj->base.dev)->display.fb_tracking.lock);
  }
  
+#endif
+
  /**
   * intel_frontbuffer_track - update frontbuffer tracking
   * @old: current buffer for the frontbuffer slots
@@ -315,13 +324,13 @@ void intel_frontbuffer_track(struct intel_frontbuffer *old,
  	BUILD_BUG_ON(I915_MAX_PLANES > INTEL_FRONTBUFFER_BITS_PER_PIPE);
  
  	if (old) {
-		drm_WARN_ON(old->obj->base.dev,
+		drm_WARN_ON(intel_front_obj(old)->base.dev,
  			    !(atomic_read(&old->bits) & frontbuffer_bits));
  		atomic_andnot(frontbuffer_bits, &old->bits);
  	}
  
  	if (new) {
-		drm_WARN_ON(new->obj->base.dev,
+		drm_WARN_ON(intel_front_obj(new)->base.dev,
  			    atomic_read(&new->bits) & frontbuffer_bits);
  		atomic_or(frontbuffer_bits, &new->bits);
  	}
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
index 3c474ed937fb..a79e5ca419ec 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
@@ -28,8 +28,6 @@
  #include <linux/bits.h>
  #include <linux/kref.h>
  
-#include "gem/i915_gem_object_types.h"
-#include "i915_active_types.h"
  
  struct drm_i915_private;
  
@@ -41,6 +39,10 @@ enum fb_op_origin {
  	ORIGIN_CURSOR_UPDATE,
  };
  
+#ifdef I915
+#include "gem/i915_gem_object_types.h"
+#include "i915_active_types.h"
+
  struct intel_frontbuffer {
  	struct kref ref;
  	atomic_t bits;
@@ -48,6 +50,9 @@ struct intel_frontbuffer {
  	struct drm_i915_gem_object *obj;
  	struct rcu_head rcu;
  };
+#else
+#define intel_frontbuffer intel_framebuffer
+#endif
  
  /*
   * Frontbuffer tracking bits. Set in obj->frontbuffer_bits while a gem bo is
@@ -73,6 +78,7 @@ void intel_frontbuffer_flip_complete(struct drm_i915_private *i915,
  void intel_frontbuffer_flip(struct drm_i915_private *i915,
  			    unsigned frontbuffer_bits);
  
+#ifdef I915
  void intel_frontbuffer_put(struct intel_frontbuffer *front);
  
  static inline struct intel_frontbuffer *
@@ -105,6 +111,8 @@ __intel_frontbuffer_get(const struct drm_i915_gem_object *obj)
  struct intel_frontbuffer *
  intel_frontbuffer_get(struct drm_i915_gem_object *obj);
  
+#endif
+
  void __intel_fb_invalidate(struct intel_frontbuffer *front,
  			   enum fb_op_origin origin,
  			   unsigned int frontbuffer_bits);
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 9820e5fdd087..bc998b526d88 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -33,6 +33,7 @@
  #include "intel_de.h"
  #include "intel_display_types.h"
  #include "intel_dp_aux.h"
+#include "intel_frontbuffer.h"
  #include "intel_hdmi.h"
  #include "intel_psr.h"
  #include "intel_snps_phy.h"
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 38f70f27ff1b..86d022e195c7 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -16,11 +16,13 @@
  #include "intel_display_types.h"
  #include "intel_fb.h"
  #include "intel_fbc.h"
+#include "intel_frontbuffer.h"
  #include "intel_psr.h"
  #include "intel_sprite.h"
  #include "skl_scaler.h"
  #include "skl_universal_plane.h"
  #include "skl_watermark.h"
+
  #ifdef I915
  #include "pxp/intel_pxp.h"
  #else






