[PATCH 0/2] drm/i915: Remove frontbuffer tracking from gem.

[PATCH 0/2] drm/i915: Remove frontbuffer tracking from gem.

[Maarten Lankhorst]

Frontbuffer tracking in gem is used in old drivers, but nowadays everyone
calls dirtyfb explicitly. Remove frontbuffer tracking from gem, and
isolate it to display only.

Maarten Lankhorst (2):
  drm/i915: Remove gem and overlay frontbuffer tracking
  drm/i915: Remove special frontbuffer type

 drivers/gpu/drm/i915/display/intel_cursor.c   |   6 +-
 drivers/gpu/drm/i915/display/intel_display.c  |   4 +-
 .../drm/i915/display/intel_display_types.h    |   8 +-
 drivers/gpu/drm/i915/display/intel_fb.c       |  11 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c    |   7 +-
 .../gpu/drm/i915/display/intel_frontbuffer.c  | 103 ++----------------
 .../gpu/drm/i915/display/intel_frontbuffer.h  |  65 ++---------
 drivers/gpu/drm/i915/display/intel_overlay.c  |  14 ---
 .../drm/i915/display/intel_plane_initial.c    |   3 +-
 drivers/gpu/drm/i915/display/intel_psr.c      |   1 +
 drivers/gpu/drm/i915/gem/i915_gem_clflush.c   |   4 -
 drivers/gpu/drm/i915/gem/i915_gem_domain.c    |   7 --
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |   2 -
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |  25 -----
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |  22 ----
 drivers/gpu/drm/i915/gem/i915_gem_phys.c      |   4 -
 drivers/gpu/drm/i915/i915_driver.c            |   1 +
 drivers/gpu/drm/i915/i915_drv.h               |   1 -
 drivers/gpu/drm/i915/i915_gem.c               |   8 --
 drivers/gpu/drm/i915/i915_gem_gtt.c           |   1 -
 drivers/gpu/drm/i915/i915_vma.c               |  12 --
 21 files changed, 35 insertions(+), 274 deletions(-)

-- 
2.34.1

[PATCH 1/2] drm/i915: Remove gem and overlay frontbuffer tracking

[Maarten Lankhorst]

Frontbuffer update handling should be done explicitly by using dirtyfb
calls only.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fb.c       |  1 +
 drivers/gpu/drm/i915/display/intel_overlay.c  | 14 -----------
 .../drm/i915/display/intel_plane_initial.c    |  1 +
 drivers/gpu/drm/i915/display/intel_psr.c      |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_clflush.c   |  4 ---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c    |  7 ------
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  2 --
 drivers/gpu/drm/i915/gem/i915_gem_object.c    | 25 -------------------
 drivers/gpu/drm/i915/gem/i915_gem_object.h    | 22 ----------------
 drivers/gpu/drm/i915/gem/i915_gem_phys.c      |  4 ---
 drivers/gpu/drm/i915/i915_drv.h               |  1 -
 drivers/gpu/drm/i915/i915_gem.c               |  6 -----
 drivers/gpu/drm/i915/i915_gem_gtt.c           |  1 -
 drivers/gpu/drm/i915/i915_vma.c               | 12 ---------
 14 files changed, 3 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
index b191915ab351..88d6189f9c1c 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -12,6 +12,7 @@
 #include "intel_display_types.h"
 #include "intel_dpt.h"
 #include "intel_fb.h"
+#include "intel_frontbuffer.h"
 
 #define check_array_bounds(i915, a, i) drm_WARN_ON(&(i915)->drm, (i) >= ARRAY_SIZE(a))
 
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index 79ed8bd04a07..23feb6e8d336 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -186,7 +186,6 @@ struct intel_overlay {
 	struct intel_crtc *crtc;
 	struct i915_vma *vma;
 	struct i915_vma *old_vma;
-	struct intel_frontbuffer *frontbuffer;
 	bool active;
 	bool pfit_active;
 	u32 pfit_vscale_ratio; /* shifted-point number, (1<<12) == 1.0 */
@@ -287,20 +286,9 @@ static void intel_overlay_flip_prepare(struct intel_overlay *overlay,
 				       struct i915_vma *vma)
 {
 	enum pipe pipe = overlay->crtc->pipe;
-	struct intel_frontbuffer *frontbuffer = NULL;
 
 	drm_WARN_ON(&overlay->i915->drm, overlay->old_vma);
 
-	if (vma)
-		frontbuffer = intel_frontbuffer_get(vma->obj);
-
-	intel_frontbuffer_track(overlay->frontbuffer, frontbuffer,
-				INTEL_FRONTBUFFER_OVERLAY(pipe));
-
-	if (overlay->frontbuffer)
-		intel_frontbuffer_put(overlay->frontbuffer);
-	overlay->frontbuffer = frontbuffer;
-
 	intel_frontbuffer_flip_prepare(overlay->i915,
 				       INTEL_FRONTBUFFER_OVERLAY(pipe));
 
@@ -810,8 +798,6 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 		goto out_pin_section;
 	}
 
-	i915_gem_object_flush_frontbuffer(new_bo, ORIGIN_DIRTYFB);
-
 	if (!overlay->active) {
 		const struct intel_crtc_state *crtc_state =
 			overlay->crtc->config;
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index d10f27d0b7b0..8ca6d2fab243 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -9,6 +9,7 @@
 #include "intel_display.h"
 #include "intel_display_types.h"
 #include "intel_fb.h"
+#include "intel_frontbuffer.h"
 #include "intel_plane_initial.h"
 
 static bool
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 98c3c8015a5c..a12ef0132697 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -32,6 +32,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/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
index 0512afdd20d8..427ff744b02a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
@@ -6,8 +6,6 @@
 
 #include <drm/drm_cache.h>
 
-#include "display/intel_frontbuffer.h"
-
 #include "i915_drv.h"
 #include "i915_gem_clflush.h"
 #include "i915_sw_fence_work.h"
@@ -22,8 +20,6 @@ static void __do_clflush(struct drm_i915_gem_object *obj)
 {
 	GEM_BUG_ON(!i915_gem_object_has_pages(obj));
 	drm_clflush_sg(obj->mm.pages);
-
-	i915_gem_object_flush_frontbuffer(obj, ORIGIN_CPU);
 }
 
 static void clflush_work(struct dma_fence_work *base)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 1674b0c5802b..667c76aefb52 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -4,7 +4,6 @@
  * Copyright © 2014-2016 Intel Corporation
  */
 
-#include "display/intel_frontbuffer.h"
 #include "gt/intel_gt.h"
 
 #include "i915_drv.h"
@@ -63,8 +62,6 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 				intel_gt_flush_ggtt_writes(vma->vm->gt);
 		}
 		spin_unlock(&obj->vma.lock);
-
-		i915_gem_object_flush_frontbuffer(obj, ORIGIN_CPU);
 		break;
 
 	case I915_GEM_DOMAIN_WC:
@@ -616,9 +613,6 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 out_unlock:
 	i915_gem_object_unlock(obj);
 
-	if (!err && write_domain)
-		i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU);
-
 out:
 	i915_gem_object_put(obj);
 	return err;
@@ -729,7 +723,6 @@ int i915_gem_object_prepare_write(struct drm_i915_gem_object *obj,
 	}
 
 out:
-	i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU);
 	obj->mm.dirty = true;
 	/* return with the pages pinned */
 	return 0;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index cd75b0ca2555..166bc7fc35bd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -11,8 +11,6 @@
 
 #include <drm/drm_syncobj.h>
 
-#include "display/intel_frontbuffer.h"
-
 #include "gem/i915_gem_ioctls.h"
 #include "gt/intel_context.h"
 #include "gt/intel_gpu_commands.h"
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 389e9f157ca5..65f1a8fcc401 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -27,7 +27,6 @@
 
 #include <drm/drm_cache.h>
 
-#include "display/intel_frontbuffer.h"
 #include "pxp/intel_pxp.h"
 
 #include "i915_drv.h"
@@ -386,30 +385,6 @@ static void i915_gem_free_object(struct drm_gem_object *gem_obj)
 		queue_work(i915->wq, &i915->mm.free_work);
 }
 
-void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj,
-					 enum fb_op_origin origin)
-{
-	struct intel_frontbuffer *front;
-
-	front = __intel_frontbuffer_get(obj);
-	if (front) {
-		intel_frontbuffer_flush(front, origin);
-		intel_frontbuffer_put(front);
-	}
-}
-
-void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj,
-					      enum fb_op_origin origin)
-{
-	struct intel_frontbuffer *front;
-
-	front = __intel_frontbuffer_get(obj);
-	if (front) {
-		intel_frontbuffer_invalidate(front, origin);
-		intel_frontbuffer_put(front);
-	}
-}
-
 static void
 i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 6f0a3ce35567..549a17e7a77b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -11,7 +11,6 @@
 #include <drm/drm_file.h>
 #include <drm/drm_device.h>
 
-#include "display/intel_frontbuffer.h"
 #include "intel_memory_region.h"
 #include "i915_gem_object_types.h"
 #include "i915_gem_gtt.h"
@@ -570,27 +569,6 @@ int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj,
 				  unsigned int flags,
 				  const struct i915_sched_attr *attr);
 
-void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj,
-					 enum fb_op_origin origin);
-void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj,
-					      enum fb_op_origin origin);
-
-static inline void
-i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj,
-				  enum fb_op_origin origin)
-{
-	if (unlikely(rcu_access_pointer(obj->frontbuffer)))
-		__i915_gem_object_flush_frontbuffer(obj, origin);
-}
-
-static inline void
-i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj,
-				       enum fb_op_origin origin)
-{
-	if (unlikely(rcu_access_pointer(obj->frontbuffer)))
-		__i915_gem_object_invalidate_frontbuffer(obj, origin);
-}
-
 int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size);
 
 bool i915_gem_object_is_shmem(const struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 0d0e46dae559..b768a210f29a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -156,15 +156,11 @@ int i915_gem_object_pwrite_phys(struct drm_i915_gem_object *obj,
 	 * We manually control the domain here and pretend that it
 	 * remains coherent i.e. in the GTT domain, like shmem_pwrite.
 	 */
-	i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU);
-
 	if (copy_from_user(vaddr, user_data, args->size))
 		return -EFAULT;
 
 	drm_clflush_virt_range(vaddr, args->size);
 	intel_gt_chipset_flush(to_gt(i915));
-
-	i915_gem_object_flush_frontbuffer(obj, ORIGIN_CPU);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4733c5a01da..4a5d42b1cf41 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -44,7 +44,6 @@
 #include "display/intel_dpll_mgr.h"
 #include "display/intel_dsb.h"
 #include "display/intel_fbc.h"
-#include "display/intel_frontbuffer.h"
 #include "display/intel_global_state.h"
 #include "display/intel_gmbus.h"
 #include "display/intel_opregion.h"
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 702e5b89be22..331f5d96c18f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -40,7 +40,6 @@
 #include <drm/drm_vma_manager.h>
 
 #include "display/intel_display.h"
-#include "display/intel_frontbuffer.h"
 
 #include "gem/i915_gem_clflush.h"
 #include "gem/i915_gem_context.h"
@@ -569,8 +568,6 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 		goto out_rpm;
 	}
 
-	i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU);
-
 	user_data = u64_to_user_ptr(args->data_ptr);
 	offset = args->offset;
 	remain = args->size;
@@ -613,7 +610,6 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 	}
 
 	intel_gt_flush_ggtt_writes(ggtt->vm.gt);
-	i915_gem_object_flush_frontbuffer(obj, ORIGIN_CPU);
 
 	i915_gem_gtt_cleanup(obj, &node, vma);
 out_rpm:
@@ -700,8 +696,6 @@ i915_gem_shmem_pwrite(struct drm_i915_gem_object *obj,
 		offset = 0;
 	}
 
-	i915_gem_object_flush_frontbuffer(obj, ORIGIN_CPU);
-
 	i915_gem_object_unpin_pages(obj);
 	return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 329ff75b80b9..416a6e931ac8 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -15,7 +15,6 @@
 #include <asm/set_memory.h>
 #include <asm/smp.h>
 
-#include "display/intel_frontbuffer.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_requests.h"
 
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 260371716490..df86baba1d8f 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -26,7 +26,6 @@
 #include <linux/dma-fence-array.h>
 #include <drm/drm_gem.h>
 
-#include "display/intel_frontbuffer.h"
 #include "gem/i915_gem_lmem.h"
 #include "gem/i915_gem_tiling.h"
 #include "gt/intel_engine.h"
@@ -1866,17 +1865,6 @@ int _i915_vma_move_to_active(struct i915_vma *vma,
 			return err;
 	}
 
-	if (flags & EXEC_OBJECT_WRITE) {
-		struct intel_frontbuffer *front;
-
-		front = __intel_frontbuffer_get(obj);
-		if (unlikely(front)) {
-			if (intel_frontbuffer_invalidate(front, ORIGIN_CS))
-				i915_active_add_request(&front->write, rq);
-			intel_frontbuffer_put(front);
-		}
-	}
-
 	if (fence) {
 		struct dma_fence *curr;
 		enum dma_resv_usage usage;
-- 
2.34.1

[PATCH 2/2] drm/i915: Remove special frontbuffer type

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c   |   6 +-
 drivers/gpu/drm/i915/display/intel_display.c  |   4 +-
 .../drm/i915/display/intel_display_types.h    |   8 +-
 drivers/gpu/drm/i915/display/intel_fb.c       |  10 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c    |   7 +-
 .../gpu/drm/i915/display/intel_frontbuffer.c  | 103 ++----------------
 .../gpu/drm/i915/display/intel_frontbuffer.h  |  65 ++---------
 .../drm/i915/display/intel_plane_initial.c    |   2 +-
 drivers/gpu/drm/i915/i915_driver.c            |   1 +
 drivers/gpu/drm/i915/i915_gem.c               |   2 -
 10 files changed, 32 insertions(+), 176 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 16ac560eab49..eaf46487e720 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -689,10 +689,10 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 	if (ret)
 		goto out_free;
 
-	intel_frontbuffer_flush(to_intel_frontbuffer(new_plane_state->hw.fb),
+	intel_frontbuffer_flush(to_intel_framebuffer(new_plane_state->hw.fb),
 				ORIGIN_CURSOR_UPDATE);
-	intel_frontbuffer_track(to_intel_frontbuffer(old_plane_state->hw.fb),
-				to_intel_frontbuffer(new_plane_state->hw.fb),
+	intel_frontbuffer_track(to_intel_framebuffer(old_plane_state->hw.fb),
+				to_intel_framebuffer(new_plane_state->hw.fb),
 				plane->frontbuffer_bit);
 
 	/* Swap plane state */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 533fff79aeda..ae71ac40de41 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7727,8 +7727,8 @@ static void intel_atomic_track_fbs(struct intel_atomic_state *state)
 
 	for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state,
 					     new_plane_state, i)
-		intel_frontbuffer_track(to_intel_frontbuffer(old_plane_state->hw.fb),
-					to_intel_frontbuffer(new_plane_state->hw.fb),
+		intel_frontbuffer_track(to_intel_framebuffer(old_plane_state->hw.fb),
+					to_intel_framebuffer(new_plane_state->hw.fb),
 					plane->frontbuffer_bit);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 0da9b208d56e..f0963de13abf 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -132,6 +132,8 @@ struct intel_fb_view {
 
 struct intel_framebuffer {
 	struct drm_framebuffer base;
+
+	atomic_t bits;
 	struct intel_frontbuffer *frontbuffer;
 
 	/* Params to remap the FB pages and program the plane registers in each view. */
@@ -2061,10 +2063,4 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *plane_
 	return i915_ggtt_offset(plane_state->ggtt_vma);
 }
 
-static inline struct intel_frontbuffer *
-to_intel_frontbuffer(struct drm_framebuffer *fb)
-{
-	return fb ? to_intel_framebuffer(fb)->frontbuffer : NULL;
-}
-
 #endif /*  __INTEL_DISPLAY_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
index 88d6189f9c1c..9c1a093ed9b1 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -1820,8 +1820,6 @@ static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
 	if (intel_fb_uses_dpt(fb))
 		intel_dpt_destroy(intel_fb->dpt_vm);
 
-	intel_frontbuffer_put(intel_fb->frontbuffer);
-
 	kfree(intel_fb);
 }
 
@@ -1850,7 +1848,7 @@ static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 
 	i915_gem_object_flush_if_display(obj);
-	intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
+	intel_frontbuffer_flush(to_intel_framebuffer(fb), ORIGIN_DIRTYFB);
 
 	return 0;
 }
@@ -1872,10 +1870,7 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 	int ret = -EINVAL;
 	int i;
 
-	intel_fb->frontbuffer = intel_frontbuffer_get(obj);
-	if (!intel_fb->frontbuffer)
-		return -ENOMEM;
-
+	atomic_set(&intel_fb->bits, 0);
 	i915_gem_object_lock(obj, NULL);
 	tiling = i915_gem_object_get_tiling(obj);
 	stride = i915_gem_object_get_stride(obj);
@@ -2011,7 +2006,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 	return 0;
 
 err:
-	intel_frontbuffer_put(intel_fb->frontbuffer);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 221336178991..002e850dcd62 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -67,14 +67,9 @@ struct intel_fbdev {
 	struct mutex hpd_lock;
 };
 
-static struct intel_frontbuffer *to_frontbuffer(struct intel_fbdev *ifbdev)
-{
-	return ifbdev->fb->frontbuffer;
-}
-
 static void intel_fbdev_invalidate(struct intel_fbdev *ifbdev)
 {
-	intel_frontbuffer_invalidate(to_frontbuffer(ifbdev), ORIGIN_CPU);
+	intel_frontbuffer_invalidate(ifbdev->fb, ORIGIN_CPU);
 }
 
 static int intel_fbdev_set_par(struct fb_info *info)
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 791248f812aa..8215faefcc1a 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -163,11 +163,11 @@ void intel_frontbuffer_flip(struct drm_i915_private *i915,
 	frontbuffer_flush(i915, frontbuffer_bits, ORIGIN_FLIP);
 }
 
-void __intel_fb_invalidate(struct intel_frontbuffer *front,
+void __intel_fb_invalidate(struct intel_framebuffer *fb,
 			   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(fb->base.dev);
 
 	if (origin == ORIGIN_CS) {
 		spin_lock(&i915->fb_tracking.lock);
@@ -184,11 +184,11 @@ void __intel_fb_invalidate(struct intel_frontbuffer *front,
 	intel_fbc_invalidate(i915, frontbuffer_bits, origin);
 }
 
-void __intel_fb_flush(struct intel_frontbuffer *front,
+void __intel_fb_flush(struct intel_framebuffer *fb,
 		      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(fb->base.dev);
 
 	if (origin == ORIGIN_CS) {
 		spin_lock(&i915->fb_tracking.lock);
@@ -202,93 +202,6 @@ void __intel_fb_flush(struct intel_frontbuffer *front,
 		frontbuffer_flush(i915, frontbuffer_bits, origin);
 }
 
-static int frontbuffer_active(struct i915_active *ref)
-{
-	struct intel_frontbuffer *front =
-		container_of(ref, typeof(*front), write);
-
-	kref_get(&front->ref);
-	return 0;
-}
-
-static void frontbuffer_retire(struct i915_active *ref)
-{
-	struct intel_frontbuffer *front =
-		container_of(ref, typeof(*front), write);
-
-	intel_frontbuffer_flush(front, ORIGIN_CS);
-	intel_frontbuffer_put(front);
-}
-
-static void frontbuffer_release(struct kref *ref)
-	__releases(&to_i915(front->obj->base.dev)->fb_tracking.lock)
-{
-	struct intel_frontbuffer *front =
-		container_of(ref, typeof(*front), ref);
-	struct drm_i915_gem_object *obj = front->obj;
-	struct i915_vma *vma;
-
-	drm_WARN_ON(obj->base.dev, atomic_read(&front->bits));
-
-	spin_lock(&obj->vma.lock);
-	for_each_ggtt_vma(vma, obj) {
-		i915_vma_clear_scanout(vma);
-		vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
-	}
-	spin_unlock(&obj->vma.lock);
-
-	RCU_INIT_POINTER(obj->frontbuffer, NULL);
-	spin_unlock(&to_i915(obj->base.dev)->fb_tracking.lock);
-
-	i915_active_fini(&front->write);
-
-	i915_gem_object_put(obj);
-	kfree_rcu(front, rcu);
-}
-
-struct intel_frontbuffer *
-intel_frontbuffer_get(struct drm_i915_gem_object *obj)
-{
-	struct drm_i915_private *i915 = to_i915(obj->base.dev);
-	struct intel_frontbuffer *front;
-
-	front = __intel_frontbuffer_get(obj);
-	if (front)
-		return front;
-
-	front = kmalloc(sizeof(*front), GFP_KERNEL);
-	if (!front)
-		return NULL;
-
-	front->obj = obj;
-	kref_init(&front->ref);
-	atomic_set(&front->bits, 0);
-	i915_active_init(&front->write,
-			 frontbuffer_active,
-			 frontbuffer_retire,
-			 I915_ACTIVE_RETIRE_SLEEPS);
-
-	spin_lock(&i915->fb_tracking.lock);
-	if (rcu_access_pointer(obj->frontbuffer)) {
-		kfree(front);
-		front = rcu_dereference_protected(obj->frontbuffer, true);
-		kref_get(&front->ref);
-	} else {
-		i915_gem_object_get(obj);
-		rcu_assign_pointer(obj->frontbuffer, front);
-	}
-	spin_unlock(&i915->fb_tracking.lock);
-
-	return front;
-}
-
-void intel_frontbuffer_put(struct intel_frontbuffer *front)
-{
-	kref_put_lock(&front->ref,
-		      frontbuffer_release,
-		      &to_i915(front->obj->base.dev)->fb_tracking.lock);
-}
-
 /**
  * intel_frontbuffer_track - update frontbuffer tracking
  * @old: current buffer for the frontbuffer slots
@@ -298,8 +211,8 @@ void intel_frontbuffer_put(struct intel_frontbuffer *front)
  * This updates the frontbuffer tracking bits @frontbuffer_bits by clearing them
  * from @old and setting them in @new. Both @old and @new can be NULL.
  */
-void intel_frontbuffer_track(struct intel_frontbuffer *old,
-			     struct intel_frontbuffer *new,
+void intel_frontbuffer_track(struct intel_framebuffer *old,
+			     struct intel_framebuffer *new,
 			     unsigned int frontbuffer_bits)
 {
 	/*
@@ -313,13 +226,13 @@ void intel_frontbuffer_track(struct intel_frontbuffer *old,
 		     BITS_PER_TYPE(atomic_t));
 
 	if (old) {
-		drm_WARN_ON(old->obj->base.dev,
+		drm_WARN_ON(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(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 ff0c37b079aa..c2e921c9efd9 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
@@ -28,7 +28,6 @@
 #include <linux/kref.h>
 
 #include "gem/i915_gem_object_types.h"
-#include "i915_active_types.h"
 
 struct drm_i915_private;
 
@@ -40,14 +39,6 @@ enum fb_op_origin {
 	ORIGIN_CURSOR_UPDATE,
 };
 
-struct intel_frontbuffer {
-	struct kref ref;
-	atomic_t bits;
-	struct i915_active write;
-	struct drm_i915_gem_object *obj;
-	struct rcu_head rcu;
-};
-
 void intel_frontbuffer_flip_prepare(struct drm_i915_private *i915,
 				    unsigned frontbuffer_bits);
 void intel_frontbuffer_flip_complete(struct drm_i915_private *i915,
@@ -55,39 +46,7 @@ void intel_frontbuffer_flip_complete(struct drm_i915_private *i915,
 void intel_frontbuffer_flip(struct drm_i915_private *i915,
 			    unsigned frontbuffer_bits);
 
-void intel_frontbuffer_put(struct intel_frontbuffer *front);
-
-static inline struct intel_frontbuffer *
-__intel_frontbuffer_get(const struct drm_i915_gem_object *obj)
-{
-	struct intel_frontbuffer *front;
-
-	if (likely(!rcu_access_pointer(obj->frontbuffer)))
-		return NULL;
-
-	rcu_read_lock();
-	do {
-		front = rcu_dereference(obj->frontbuffer);
-		if (!front)
-			break;
-
-		if (unlikely(!kref_get_unless_zero(&front->ref)))
-			continue;
-
-		if (likely(front == rcu_access_pointer(obj->frontbuffer)))
-			break;
-
-		intel_frontbuffer_put(front);
-	} while (1);
-	rcu_read_unlock();
-
-	return front;
-}
-
-struct intel_frontbuffer *
-intel_frontbuffer_get(struct drm_i915_gem_object *obj);
-
-void __intel_fb_invalidate(struct intel_frontbuffer *front,
+void __intel_fb_invalidate(struct intel_framebuffer *front,
 			   enum fb_op_origin origin,
 			   unsigned int frontbuffer_bits);
 
@@ -102,23 +61,23 @@ void __intel_fb_invalidate(struct intel_frontbuffer *front,
  * until the rendering completes or a flip on this frontbuffer plane is
  * scheduled.
  */
-static inline bool intel_frontbuffer_invalidate(struct intel_frontbuffer *front,
+static inline bool intel_frontbuffer_invalidate(struct intel_framebuffer *fb,
 						enum fb_op_origin origin)
 {
 	unsigned int frontbuffer_bits;
 
-	if (!front)
+	if (!fb)
 		return false;
 
-	frontbuffer_bits = atomic_read(&front->bits);
+	frontbuffer_bits = atomic_read(&fb->bits);
 	if (!frontbuffer_bits)
 		return false;
 
-	__intel_fb_invalidate(front, origin, frontbuffer_bits);
+	__intel_fb_invalidate(fb, origin, frontbuffer_bits);
 	return true;
 }
 
-void __intel_fb_flush(struct intel_frontbuffer *front,
+void __intel_fb_flush(struct intel_framebuffer *fb,
 		      enum fb_op_origin origin,
 		      unsigned int frontbuffer_bits);
 
@@ -130,23 +89,23 @@ void __intel_fb_flush(struct intel_frontbuffer *front,
  * This function gets called every time rendering on the given object has
  * completed and frontbuffer caching can be started again.
  */
-static inline void intel_frontbuffer_flush(struct intel_frontbuffer *front,
+static inline void intel_frontbuffer_flush(struct intel_framebuffer *fb,
 					   enum fb_op_origin origin)
 {
 	unsigned int frontbuffer_bits;
 
-	if (!front)
+	if (!fb)
 		return;
 
-	frontbuffer_bits = atomic_read(&front->bits);
+	frontbuffer_bits = atomic_read(&fb->bits);
 	if (!frontbuffer_bits)
 		return;
 
-	__intel_fb_flush(front, origin, frontbuffer_bits);
+	__intel_fb_flush(fb, origin, frontbuffer_bits);
 }
 
-void intel_frontbuffer_track(struct intel_frontbuffer *old,
-			     struct intel_frontbuffer *new,
+void intel_frontbuffer_track(struct intel_framebuffer *old,
+			     struct intel_framebuffer *new,
 			     unsigned int frontbuffer_bits);
 
 #endif /* __INTEL_FRONTBUFFER_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index 8ca6d2fab243..8b35cc1e50d7 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -281,7 +281,7 @@ intel_find_initial_plane_obj(struct intel_crtc *crtc,
 	plane_state->uapi.crtc = &crtc->base;
 	intel_plane_copy_uapi_to_hw_state(plane_state, plane_state, crtc);
 
-	atomic_or(plane->frontbuffer_bit, &to_intel_frontbuffer(fb)->bits);
+	atomic_or(plane->frontbuffer_bit, &to_intel_framebuffer(fb)->bits);
 }
 
 static void plane_config_fini(struct intel_initial_plane_config *plane_config)
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index deb8a8b76965..1985b8b0cc98 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -330,6 +330,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->gpu_error.lock);
+	spin_lock_init(&dev_priv->fb_tracking.lock);
 	mutex_init(&dev_priv->backlight_lock);
 
 	mutex_init(&dev_priv->sb_lock);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 331f5d96c18f..26c228ca4a1c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1206,8 +1206,6 @@ void i915_gem_init_early(struct drm_i915_private *dev_priv)
 {
 	i915_gem_init__mm(dev_priv);
 	i915_gem_init__contexts(dev_priv);
-
-	spin_lock_init(&dev_priv->fb_tracking.lock);
 }
 
 void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
-- 
2.34.1

Re: [Intel-gfx] [PATCH 0/2] drm/i915: Remove frontbuffer tracking from gem.

[Zanoni, Paulo R]

Hi

I was given a link to https://patchwork.freedesktop.org/series/111494/
but can't seem to find it on the mailing list, so I'll reply here.

On Thu, 2022-08-25 at 08:46 +0200, Maarten Lankhorst wrote:
> Frontbuffer tracking in gem is used in old drivers, but nowadays everyone
> calls dirtyfb explicitly. Remove frontbuffer tracking from gem, and
> isolate it to display only.

Ok, but then how can the Kernel know if the user space running is an
"old driver" or a new one? Assuming everybody is following the new
policy is at least risky.

(crazy idea: have an ioctl for user space to tell whether it knows how
to DirtyFB and another where it can even say it will never be touching
frontbuffers at all)

Also, when I wrote igt/kms_frontbuffer_tracking, it was agreed that
whatever was there was a representation of the "rules" of the
frontbfuffer writing contract/API. And I see on the link above that the
basic tests of kms_frontbuffer_tracking fail. If
kms_frontbuffer_tracking requires change due to i915.ko having changed
the frontbuffer writing rules, then we should do it, and possibly have
those rules written somewhere.

But then, having the Kernel change expectations like that is not
exactly what I learned we should be doing, because it's the recipe to
break user space.

I'm confused, please clarify us a little more. More sentences in the
commit messages would be absolutely great.

Thanks,
Paulo

> 
> Maarten Lankhorst (2):
>   drm/i915: Remove gem and overlay frontbuffer tracking
>   drm/i915: Remove special frontbuffer type
> 
>  drivers/gpu/drm/i915/display/intel_cursor.c   |   6 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |   4 +-
>  .../drm/i915/display/intel_display_types.h    |   8 +-
>  drivers/gpu/drm/i915/display/intel_fb.c       |  11 +-
>  drivers/gpu/drm/i915/display/intel_fbdev.c    |   7 +-
>  .../gpu/drm/i915/display/intel_frontbuffer.c  | 103 ++----------------
>  .../gpu/drm/i915/display/intel_frontbuffer.h  |  65 ++---------
>  drivers/gpu/drm/i915/display/intel_overlay.c  |  14 ---
>  .../drm/i915/display/intel_plane_initial.c    |   3 +-
>  drivers/gpu/drm/i915/display/intel_psr.c      |   1 +
>  drivers/gpu/drm/i915/gem/i915_gem_clflush.c   |   4 -
>  drivers/gpu/drm/i915/gem/i915_gem_domain.c    |   7 --
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |   2 -
>  drivers/gpu/drm/i915/gem/i915_gem_object.c    |  25 -----
>  drivers/gpu/drm/i915/gem/i915_gem_object.h    |  22 ----
>  drivers/gpu/drm/i915/gem/i915_gem_phys.c      |   4 -
>  drivers/gpu/drm/i915/i915_driver.c            |   1 +
>  drivers/gpu/drm/i915/i915_drv.h               |   1 -
>  drivers/gpu/drm/i915/i915_gem.c               |   8 --
>  drivers/gpu/drm/i915/i915_gem_gtt.c           |   1 -
>  drivers/gpu/drm/i915/i915_vma.c               |  12 --
>  21 files changed, 35 insertions(+), 274 deletions(-)
> 

Re: [Intel-gfx] [PATCH 0/2] drm/i915: Remove frontbuffer tracking from gem.

[Tvrtko Ursulin]

On 01/12/2022 22:03, Zanoni, Paulo R wrote:
> Hi
> 
> I was given a link to https://patchwork.freedesktop.org/series/111494/
> but can't seem to find it on the mailing list, so I'll reply here.
> 
> On Thu, 2022-08-25 at 08:46 +0200, Maarten Lankhorst wrote:
>> Frontbuffer tracking in gem is used in old drivers, but nowadays everyone
>> calls dirtyfb explicitly. Remove frontbuffer tracking from gem, and
>> isolate it to display only.
> 
> Ok, but then how can the Kernel know if the user space running is an
> "old driver" or a new one? Assuming everybody is following the new
> policy is at least risky.
> 
> (crazy idea: have an ioctl for user space to tell whether it knows how
> to DirtyFB and another where it can even say it will never be touching
> frontbuffers at all)
> 
> Also, when I wrote igt/kms_frontbuffer_tracking, it was agreed that
> whatever was there was a representation of the "rules" of the
> frontbfuffer writing contract/API. And I see on the link above that the
> basic tests of kms_frontbuffer_tracking fail. If
> kms_frontbuffer_tracking requires change due to i915.ko having changed
> the frontbuffer writing rules, then we should do it, and possibly have
> those rules written somewhere.
> 
> But then, having the Kernel change expectations like that is not
> exactly what I learned we should be doing, because it's the recipe to
> break user space.
> 
> I'm confused, please clarify us a little more. More sentences in the
> commit messages would be absolutely great.

Right, +1 - we need to be sure there aren't some binaries, running in a 
distro somewhere, or whatever, which rely on this and then we are not 
allowed to break them. As minimum at least we need an audit and versions 
to know how old libraries/programs we are talking about here ie. when 
did everyone stop relying on implicit tracking.

Regards,

Tvrtko

Re: [Intel-gfx] [PATCH 0/2] drm/i915: Remove frontbuffer tracking from gem.

[Rodrigo Vivi]

[-- Attachment #1: Type: text/plain, Size: 2392 bytes --]

On Fri, Dec 2, 2022 at 4:17 AM Tvrtko Ursulin <
tvrtko.ursulin@linux.intel.com> wrote:

For some reason I has missed this. Thanks Tvrtko for pointing this out.


> On 01/12/2022 22:03, Zanoni, Paulo R wrote:
> > Hi
> >
> > I was given a link to https://patchwork.freedesktop.org/series/111494/
> > but can't seem to find it on the mailing list, so I'll reply here.
> >
> > On Thu, 2022-08-25 at 08:46 +0200, Maarten Lankhorst wrote:
> >> Frontbuffer tracking in gem is used in old drivers, but nowadays
> everyone
> >> calls dirtyfb explicitly. Remove frontbuffer tracking from gem, and
> >> isolate it to display only.
> >
> > Ok, but then how can the Kernel know if the user space running is an
> > "old driver" or a new one? Assuming everybody is following the new
> > policy is at least risky.
>

Agree. This is a very old problem. That won't go away simply.


> >
> > (crazy idea: have an ioctl for user space to tell whether it knows how
> > to DirtyFB and another where it can even say it will never be touching
> > frontbuffers at all)
>

To be honest we had this "crazy" idea back there. But we thought that the
frontbuffer tracking would be easiest than new api...
We were wrong... and we probably need this now if we want to remove the
frontbuffer tracking.



> >
> > Also, when I wrote igt/kms_frontbuffer_tracking, it was agreed that
> > whatever was there was a representation of the "rules" of the
> > frontbfuffer writing contract/API. And I see on the link above that the
> > basic tests of kms_frontbuffer_tracking fail. If
> > kms_frontbuffer_tracking requires change due to i915.ko having changed
> > the frontbuffer writing rules, then we should do it, and possibly have
> > those rules written somewhere.
> >
> > But then, having the Kernel change expectations like that is not
> > exactly what I learned we should be doing, because it's the recipe to
> > break user space.
> >
> > I'm confused, please clarify us a little more. More sentences in the
> > commit messages would be absolutely great.
>
> Right, +1 - we need to be sure there aren't some binaries, running in a
> distro somewhere, or whatever, which rely on this and then we are not
> allowed to break them. As minimum at least we need an audit and versions
> to know how old libraries/programs we are talking about here ie. when
> did everyone stop relying on implicit tracking.
>
> Regards,
>
> Tvrtko
>

[-- Attachment #2: Type: text/html, Size: 3595 bytes --]