On Wed, 29 Oct 2025, Jani Nikula <[email protected]> wrote: > On Wed, 29 Oct 2025, Jani Nikula <[email protected]> wrote: >> On Thu, 16 Oct 2025, Ville Syrjala <[email protected]> wrote: >>> From: Ville Syrjälä <[email protected]> >>> >>> The current attempted split between xe/i915 vs. display >>> for intel_frontbuffer is a mess: >>> - the i915 rcu leaks through the interface to the display side >>> - the obj->frontbuffer write-side is now protected by a display >>> specific spinlock even though the actual obj->framebuffer >>> pointer lives in a i915 specific structure >>> - the kref is getting poked directly from both sides >>> - i915_active is still on the display side >>> >>> Clean up the mess by moving everything about the frontbuffer >>> lifetime management to the i915/xe side: >>> - the rcu usage is now completely contained in i915 >>> - frontbuffer_lock is moved into i915 >>> - kref is on the i915/xe side (xe needs the refcount as well >>> due to intel_frontbuffer_queue_flush()->intel_frontbuffer_ref()) >>> - the bo (and its refcounting) is no longer on the display side >>> - i915_active is contained in i915 >>> >>> I was pondering whether we could do this in some kind of smaller >>> steps, and perhaps we could, but it would probably have to start >>> with a bunch of reverts (which for sure won't go cleanly anymore). >>> So not convinced it's worth the hassle. >> >> It's a PITA to review, that's for sure. :p >> >> I'm not particularly fond of embedding struct intel_frontbuffer inside >> struct i915_frontbuffer and struct xe_frontbuffer, because it means i915 >> and xe will need to know the struct intel_frontbuffer definition. If we >> can't live with the embedding long term, we'll probably need opaque >> pointers back and forth. >> >> That said, I think the overall change here is net positive, and makes >> life much easier. We don't have to fix everything at once, so let's go >> with this. >> >> I didn't spot any obvious issues, but my confidence level with the >> review is super low. :( >> >> I guess the alternatives are to just go with that, trusting CI, or give >> me more time to review. I'm fine either way, as I can trust you to step >> up if it goes crashing down. ;) > > One approach is to send 1-8 first, get CI, get them merged, and then do > 9-10 separately, to get separate CI. Maybe? *shrug*
Any conclusions on this? Just merge the whole thing as-is rather than let it go stale...? BR, Jani. > >> >> BR, >> Jani. >> >> PS. I think patches 1-2 are fine with the acks alone. >> >>> >>> Acked-by: Jani Nikula <[email protected]> >>> Signed-off-by: Ville Syrjälä <[email protected]> >>> --- >>> drivers/gpu/drm/i915/Makefile | 1 + >>> drivers/gpu/drm/i915/display/intel_bo.c | 34 ++++-- >>> drivers/gpu/drm/i915/display/intel_bo.h | 6 +- >>> .../gpu/drm/i915/display/intel_display_core.h | 3 - >>> .../drm/i915/display/intel_display_driver.c | 1 - >>> .../gpu/drm/i915/display/intel_frontbuffer.c | 89 ++------------- >>> .../gpu/drm/i915/display/intel_frontbuffer.h | 13 +-- >>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 12 +- >>> .../i915/gem/i915_gem_object_frontbuffer.c | 103 ++++++++++++++++++ >>> .../i915/gem/i915_gem_object_frontbuffer.h | 48 +++----- >>> .../gpu/drm/i915/gem/i915_gem_object_types.h | 2 +- >>> drivers/gpu/drm/i915/i915_drv.h | 2 + >>> drivers/gpu/drm/i915/i915_gem.c | 2 + >>> drivers/gpu/drm/i915/i915_vma.c | 6 +- >>> .../gpu/drm/xe/compat-i915-headers/i915_vma.h | 2 - >>> drivers/gpu/drm/xe/display/intel_bo.c | 52 ++++++++- >>> 16 files changed, 227 insertions(+), 149 deletions(-) >>> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_object_frontbuffer.c >>> >>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile >>> index aa2f0fd95117..717e57205c85 100644 >>> --- a/drivers/gpu/drm/i915/Makefile >>> +++ b/drivers/gpu/drm/i915/Makefile >>> @@ -155,6 +155,7 @@ gem-y += \ >>> gem/i915_gem_lmem.o \ >>> gem/i915_gem_mman.o \ >>> gem/i915_gem_object.o \ >>> + gem/i915_gem_object_frontbuffer.o \ >>> gem/i915_gem_pages.o \ >>> gem/i915_gem_phys.o \ >>> gem/i915_gem_pm.o \ >>> diff --git a/drivers/gpu/drm/i915/display/intel_bo.c >>> b/drivers/gpu/drm/i915/display/intel_bo.c >>> index 2792aca7bc22..f3687eb63467 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_bo.c >>> +++ b/drivers/gpu/drm/i915/display/intel_bo.c >>> @@ -39,20 +39,40 @@ int intel_bo_read_from_page(struct drm_gem_object *obj, >>> u64 offset, void *dst, i >>> return i915_gem_object_read_from_page(to_intel_bo(obj), offset, dst, >>> size); >>> } >>> >>> -struct intel_frontbuffer *intel_bo_get_frontbuffer(struct drm_gem_object >>> *obj) >>> +struct intel_frontbuffer *intel_bo_frontbuffer_get(struct drm_gem_object >>> *_obj) >>> { >>> - return i915_gem_object_get_frontbuffer(to_intel_bo(obj)); >>> + struct drm_i915_gem_object *obj = to_intel_bo(_obj); >>> + struct i915_frontbuffer *front; >>> + >>> + front = i915_gem_object_frontbuffer_get(obj); >>> + if (!front) >>> + return NULL; >>> + >>> + return &front->base; >>> +} >>> + >>> +void intel_bo_frontbuffer_ref(struct intel_frontbuffer *_front) >>> +{ >>> + struct i915_frontbuffer *front = >>> + container_of(_front, typeof(*front), base); >>> + >>> + i915_gem_object_frontbuffer_ref(front); >>> } >>> >>> -struct intel_frontbuffer *intel_bo_set_frontbuffer(struct drm_gem_object >>> *obj, >>> - struct intel_frontbuffer >>> *front) >>> +void intel_bo_frontbuffer_put(struct intel_frontbuffer *_front) >>> { >>> - return i915_gem_object_set_frontbuffer(to_intel_bo(obj), front); >>> + struct i915_frontbuffer *front = >>> + container_of(_front, typeof(*front), base); >>> + >>> + return i915_gem_object_frontbuffer_put(front); >>> } >>> >>> -void intel_bo_frontbuffer_flush_for_display(struct intel_frontbuffer >>> *front) >>> +void intel_bo_frontbuffer_flush_for_display(struct intel_frontbuffer >>> *_front) >>> { >>> - i915_gem_object_flush_if_display(to_intel_bo(front->obj)); >>> + struct i915_frontbuffer *front = >>> + container_of(_front, typeof(*front), base); >>> + >>> + i915_gem_object_flush_if_display(front->obj); >>> } >>> >>> void intel_bo_describe(struct seq_file *m, struct drm_gem_object *obj) >>> diff --git a/drivers/gpu/drm/i915/display/intel_bo.h >>> b/drivers/gpu/drm/i915/display/intel_bo.h >>> index 08247bf36d40..fc05f680dc76 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_bo.h >>> +++ b/drivers/gpu/drm/i915/display/intel_bo.h >>> @@ -19,9 +19,9 @@ bool intel_bo_is_protected(struct drm_gem_object *obj); >>> int intel_bo_fb_mmap(struct drm_gem_object *obj, struct vm_area_struct >>> *vma); >>> int intel_bo_read_from_page(struct drm_gem_object *obj, u64 offset, void >>> *dst, int size); >>> >>> -struct intel_frontbuffer *intel_bo_get_frontbuffer(struct drm_gem_object >>> *obj); >>> -struct intel_frontbuffer *intel_bo_set_frontbuffer(struct drm_gem_object >>> *obj, >>> - struct intel_frontbuffer >>> *front); >>> +struct intel_frontbuffer *intel_bo_frontbuffer_get(struct drm_gem_object >>> *obj); >>> +void intel_bo_frontbuffer_ref(struct intel_frontbuffer *front); >>> +void intel_bo_frontbuffer_put(struct intel_frontbuffer *front); >>> void intel_bo_frontbuffer_flush_for_display(struct intel_frontbuffer >>> *front); >>> >>> void intel_bo_describe(struct seq_file *m, struct drm_gem_object *obj); >>> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h >>> b/drivers/gpu/drm/i915/display/intel_display_core.h >>> index 13576d07c999..34d578e2cc25 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_display_core.h >>> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h >>> @@ -141,9 +141,6 @@ struct intel_dpll_global { >>> }; >>> >>> struct intel_frontbuffer_tracking { >>> - /* protects obj->frontbuffer (write-side) */ >>> - spinlock_t frontbuffer_lock; >>> - >>> /* protects busy_bits */ >>> spinlock_t lock; >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c >>> b/drivers/gpu/drm/i915/display/intel_display_driver.c >>> index ac684f8c5d40..f84a0b26b7a6 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c >>> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c >>> @@ -184,7 +184,6 @@ void intel_display_driver_early_probe(struct >>> intel_display *display) >>> if (!HAS_DISPLAY(display)) >>> return; >>> >>> - spin_lock_init(&display->fb_tracking.frontbuffer_lock); >>> spin_lock_init(&display->fb_tracking.lock); >>> mutex_init(&display->backlight.lock); >>> mutex_init(&display->audio.mutex); >>> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c >>> b/drivers/gpu/drm/i915/display/intel_frontbuffer.c >>> index 5d627eac07bd..4761e116e442 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c >>> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c >>> @@ -57,8 +57,6 @@ >>> >>> #include <drm/drm_gem.h> >>> >>> -#include "i915_active.h" >>> -#include "i915_vma.h" >>> #include "intel_bo.h" >>> #include "intel_display_trace.h" >>> #include "intel_display_types.h" >>> @@ -167,7 +165,7 @@ void __intel_fb_flush(struct intel_frontbuffer *front, >>> >>> static void intel_frontbuffer_ref(struct intel_frontbuffer *front) >>> { >>> - kref_get(&front->ref); >>> + intel_bo_frontbuffer_ref(front); >>> } >>> >>> static void intel_frontbuffer_flush_work(struct work_struct *work) >>> @@ -196,89 +194,26 @@ void intel_frontbuffer_queue_flush(struct >>> intel_frontbuffer *front) >>> intel_frontbuffer_put(front); >>> } >>> >>> -static int frontbuffer_active(struct i915_active *ref) >>> +void intel_frontbuffer_init(struct intel_frontbuffer *front, struct >>> drm_device *drm) >>> { >>> - 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(&front->display->fb_tracking.frontbuffer_lock) >>> -{ >>> - struct intel_frontbuffer *ret, *front = >>> - container_of(ref, typeof(*front), ref); >>> - struct intel_display *display = front->display; >>> - struct drm_gem_object *obj = front->obj; >>> - >>> - drm_WARN_ON(display->drm, atomic_read(&front->bits)); >>> - >>> - i915_ggtt_clear_scanout(to_intel_bo(obj)); >>> - >>> - ret = intel_bo_set_frontbuffer(obj, NULL); >>> - drm_WARN_ON(display->drm, ret); >>> - spin_unlock(&display->fb_tracking.frontbuffer_lock); >>> - >>> - i915_active_fini(&front->write); >>> - >>> - drm_gem_object_put(obj); >>> - kfree_rcu(front, rcu); >>> -} >>> - >>> -struct intel_frontbuffer * >>> -intel_frontbuffer_get(struct drm_gem_object *obj) >>> -{ >>> - struct intel_display *display = to_intel_display(obj->dev); >>> - struct intel_frontbuffer *front, *cur; >>> - >>> - front = intel_bo_get_frontbuffer(obj); >>> - if (front) >>> - return front; >>> - >>> - front = kmalloc(sizeof(*front), GFP_KERNEL); >>> - if (!front) >>> - return NULL; >>> - >>> - drm_gem_object_get(obj); >>> - >>> - front->obj = obj; >>> - front->display = display; >>> - kref_init(&front->ref); >>> + front->display = to_intel_display(drm); >>> atomic_set(&front->bits, 0); >>> - i915_active_init(&front->write, >>> - frontbuffer_active, >>> - frontbuffer_retire, >>> - I915_ACTIVE_RETIRE_SLEEPS); >>> INIT_WORK(&front->flush_work, intel_frontbuffer_flush_work); >>> +} >>> >>> - spin_lock(&display->fb_tracking.frontbuffer_lock); >>> - cur = intel_bo_set_frontbuffer(obj, front); >>> - spin_unlock(&display->fb_tracking.frontbuffer_lock); >>> +void intel_frontbuffer_fini(struct intel_frontbuffer *front) >>> +{ >>> + drm_WARN_ON(front->display->drm, atomic_read(&front->bits)); >>> +} >>> >>> - if (cur != front) { >>> - drm_gem_object_put(obj); >>> - kfree(front); >>> - } >>> - >>> - return cur; >>> +struct intel_frontbuffer *intel_frontbuffer_get(struct drm_gem_object *obj) >>> +{ >>> + return intel_bo_frontbuffer_get(obj); >>> } >>> >>> void intel_frontbuffer_put(struct intel_frontbuffer *front) >>> { >>> - kref_put_lock(&front->ref, >>> - frontbuffer_release, >>> - &front->display->fb_tracking.frontbuffer_lock); >>> + intel_bo_frontbuffer_put(front); >>> } >>> >>> /** >>> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h >>> b/drivers/gpu/drm/i915/display/intel_frontbuffer.h >>> index ff2a6ac75a34..22677acb4c06 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h >>> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h >>> @@ -26,10 +26,9 @@ >>> >>> #include <linux/atomic.h> >>> #include <linux/bits.h> >>> -#include <linux/kref.h> >>> - >>> -#include "i915_active_types.h" >>> +#include <linux/workqueue_types.h> >>> >>> +struct drm_device; >>> struct drm_gem_object; >>> struct intel_display; >>> >>> @@ -42,13 +41,8 @@ enum fb_op_origin { >>> }; >>> >>> struct intel_frontbuffer { >>> - struct kref ref; >>> struct intel_display *display; >>> atomic_t bits; >>> - struct i915_active write; >>> - struct drm_gem_object *obj; >>> - struct rcu_head rcu; >>> - >>> struct work_struct flush_work; >>> }; >>> >>> @@ -141,4 +135,7 @@ void intel_frontbuffer_track(struct intel_frontbuffer >>> *old, >>> struct intel_frontbuffer *new, >>> unsigned int frontbuffer_bits); >>> >>> +void intel_frontbuffer_init(struct intel_frontbuffer *front, struct >>> drm_device *drm); >>> +void intel_frontbuffer_fini(struct intel_frontbuffer *front); >>> + >>> #endif /* __INTEL_FRONTBUFFER_H__ */ >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c >>> b/drivers/gpu/drm/i915/gem/i915_gem_object.c >>> index 478011e5ecb3..36680eddf88e 100644 >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c >>> @@ -476,24 +476,24 @@ static void i915_gem_free_object(struct >>> drm_gem_object *gem_obj) >>> void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, >>> enum fb_op_origin origin) >>> { >>> - struct intel_frontbuffer *front; >>> + struct i915_frontbuffer *front; >>> >>> front = i915_gem_object_get_frontbuffer(obj); >>> if (front) { >>> - intel_frontbuffer_flush(front, origin); >>> - intel_frontbuffer_put(front); >>> + intel_frontbuffer_flush(&front->base, origin); >>> + i915_gem_object_frontbuffer_put(front); >>> } >>> } >>> >>> void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object >>> *obj, >>> enum fb_op_origin origin) >>> { >>> - struct intel_frontbuffer *front; >>> + struct i915_frontbuffer *front; >>> >>> front = i915_gem_object_get_frontbuffer(obj); >>> if (front) { >>> - intel_frontbuffer_invalidate(front, origin); >>> - intel_frontbuffer_put(front); >>> + intel_frontbuffer_invalidate(&front->base, origin); >>> + i915_gem_object_frontbuffer_put(front); >>> } >>> } >>> >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_frontbuffer.c >>> b/drivers/gpu/drm/i915/gem/i915_gem_object_frontbuffer.c >>> new file mode 100644 >>> index 000000000000..7ef89613c025 >>> --- /dev/null >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_frontbuffer.c >>> @@ -0,0 +1,103 @@ >>> +// SPDX-License-Identifier: MIT >>> +/* Copyright © 2025 Intel Corporation */ >>> + >>> +#include "i915_drv.h" >>> +#include "i915_gem_object_frontbuffer.h" >>> + >>> +static int frontbuffer_active(struct i915_active *ref) >>> +{ >>> + struct i915_frontbuffer *front = >>> + container_of(ref, typeof(*front), write); >>> + >>> + kref_get(&front->ref); >>> + return 0; >>> +} >>> + >>> +static void frontbuffer_retire(struct i915_active *ref) >>> +{ >>> + struct i915_frontbuffer *front = >>> + container_of(ref, typeof(*front), write); >>> + >>> + intel_frontbuffer_flush(&front->base, ORIGIN_CS); >>> + i915_gem_object_frontbuffer_put(front); >>> +} >>> + >>> +struct i915_frontbuffer * >>> +i915_gem_object_frontbuffer_get(struct drm_i915_gem_object *obj) >>> +{ >>> + struct drm_i915_private *i915 = to_i915(obj->base.dev); >>> + struct i915_frontbuffer *front, *cur; >>> + >>> + front = i915_gem_object_get_frontbuffer(obj); >>> + if (front) >>> + return front; >>> + >>> + front = kmalloc(sizeof(*front), GFP_KERNEL); >>> + if (!front) >>> + return NULL; >>> + >>> + intel_frontbuffer_init(&front->base, &i915->drm); >>> + >>> + kref_init(&front->ref); >>> + i915_gem_object_get(obj); >>> + front->obj = obj; >>> + >>> + i915_active_init(&front->write, >>> + frontbuffer_active, >>> + frontbuffer_retire, >>> + I915_ACTIVE_RETIRE_SLEEPS); >>> + >>> + spin_lock(&i915->frontbuffer_lock); >>> + if (rcu_access_pointer(obj->frontbuffer)) { >>> + cur = rcu_dereference_protected(obj->frontbuffer, true); >>> + kref_get(&cur->ref); >>> + } else { >>> + cur = front; >>> + rcu_assign_pointer(obj->frontbuffer, front); >>> + } >>> + spin_unlock(&i915->frontbuffer_lock); >>> + >>> + if (cur != front) { >>> + i915_gem_object_put(obj); >>> + intel_frontbuffer_fini(&front->base); >>> + kfree(front); >>> + } >>> + >>> + return cur; >>> +} >>> + >>> +void i915_gem_object_frontbuffer_ref(struct i915_frontbuffer *front) >>> +{ >>> + kref_get(&front->ref); >>> +} >>> + >>> +static void frontbuffer_release(struct kref *ref) >>> + __releases(&i915->frontbuffer_lock) >>> +{ >>> + struct i915_frontbuffer *front = >>> + container_of(ref, typeof(*front), ref); >>> + struct drm_i915_gem_object *obj = front->obj; >>> + struct drm_i915_private *i915 = to_i915(obj->base.dev); >>> + >>> + i915_ggtt_clear_scanout(obj); >>> + >>> + RCU_INIT_POINTER(obj->frontbuffer, NULL); >>> + >>> + spin_unlock(&i915->frontbuffer_lock); >>> + >>> + i915_active_fini(&front->write); >>> + >>> + i915_gem_object_put(obj); >>> + >>> + intel_frontbuffer_fini(&front->base); >>> + >>> + kfree_rcu(front, rcu); >>> +} >>> + >>> +void i915_gem_object_frontbuffer_put(struct i915_frontbuffer *front) >>> +{ >>> + struct drm_i915_private *i915 = to_i915(front->obj->base.dev); >>> + >>> + kref_put_lock(&front->ref, frontbuffer_release, >>> + &i915->frontbuffer_lock); >>> +} >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_frontbuffer.h >>> b/drivers/gpu/drm/i915/gem/i915_gem_object_frontbuffer.h >>> index 1ec382c43aee..385f7e8049b8 100644 >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_frontbuffer.h >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_frontbuffer.h >>> @@ -12,6 +12,14 @@ >>> #include "display/intel_frontbuffer.h" >>> #include "i915_gem_object_types.h" >>> >>> +struct i915_frontbuffer { >>> + struct intel_frontbuffer base; >>> + struct drm_i915_gem_object *obj; >>> + struct i915_active write; >>> + struct rcu_head rcu; >>> + struct kref ref; >>> +}; >>> + >>> 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, >>> @@ -33,6 +41,10 @@ i915_gem_object_invalidate_frontbuffer(struct >>> drm_i915_gem_object *obj, >>> __i915_gem_object_invalidate_frontbuffer(obj, origin); >>> } >>> >>> +struct i915_frontbuffer *i915_gem_object_frontbuffer_get(struct >>> drm_i915_gem_object *obj); >>> +void i915_gem_object_frontbuffer_ref(struct i915_frontbuffer *front); >>> +void i915_gem_object_frontbuffer_put(struct i915_frontbuffer *front); >>> + >>> /** >>> * i915_gem_object_get_frontbuffer - Get the object's frontbuffer >>> * @obj: The object whose frontbuffer to get. >>> @@ -42,10 +54,10 @@ i915_gem_object_invalidate_frontbuffer(struct >>> drm_i915_gem_object *obj, >>> * >>> * Return: pointer to object's frontbuffer is such exists or NULL >>> */ >>> -static inline struct intel_frontbuffer * >>> +static inline struct i915_frontbuffer * >>> i915_gem_object_get_frontbuffer(const struct drm_i915_gem_object *obj) >>> { >>> - struct intel_frontbuffer *front; >>> + struct i915_frontbuffer *front; >>> >>> if (likely(!rcu_access_pointer(obj->frontbuffer))) >>> return NULL; >>> @@ -62,41 +74,11 @@ i915_gem_object_get_frontbuffer(const struct >>> drm_i915_gem_object *obj) >>> if (likely(front == rcu_access_pointer(obj->frontbuffer))) >>> break; >>> >>> - intel_frontbuffer_put(front); >>> + i915_gem_object_frontbuffer_put(front); >>> } while (1); >>> rcu_read_unlock(); >>> >>> return front; >>> } >>> >>> -/** >>> - * i915_gem_object_set_frontbuffer - Set the object's frontbuffer >>> - * @obj: The object whose frontbuffer to set. >>> - * @front: The frontbuffer to set >>> - * >>> - * Set object's frontbuffer pointer. If frontbuffer is already set for the >>> - * object keep it and return it's pointer to the caller. Please note that >>> RCU >>> - * mechanism is used to handle e.g. ongoing removal of frontbuffer >>> pointer. This >>> - * function is protected by i915->display->fb_tracking.frontbuffer_lock >>> - * >>> - * Return: pointer to frontbuffer which was set. >>> - */ >>> -static inline struct intel_frontbuffer * >>> -i915_gem_object_set_frontbuffer(struct drm_i915_gem_object *obj, >>> - struct intel_frontbuffer *front) >>> -{ >>> - struct intel_frontbuffer *cur = front; >>> - >>> - if (!front) { >>> - RCU_INIT_POINTER(obj->frontbuffer, NULL); >>> - } else if (rcu_access_pointer(obj->frontbuffer)) { >>> - cur = rcu_dereference_protected(obj->frontbuffer, true); >>> - kref_get(&cur->ref); >>> - } else { >>> - rcu_assign_pointer(obj->frontbuffer, front); >>> - } >>> - >>> - return cur; >>> -} >>> - >>> #endif >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h >>> b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h >>> index 64600aa8227f..465ce94aee76 100644 >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h >>> @@ -574,7 +574,7 @@ struct drm_i915_gem_object { >>> */ >>> u16 write_domain; >>> >>> - struct intel_frontbuffer __rcu *frontbuffer; >>> + struct i915_frontbuffer __rcu *frontbuffer; >>> >>> /** Current tiling stride for the object, if it's tiled. */ >>> unsigned int tiling_and_stride; >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>> b/drivers/gpu/drm/i915/i915_drv.h >>> index 95f9ddf22ce4..5381a934a671 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -311,6 +311,8 @@ struct drm_i915_private { >>> struct file *mmap_singleton; >>> } gem; >>> >>> + spinlock_t frontbuffer_lock; /* protects obj->frontbuffer (write-side) >>> */ >>> + >>> struct intel_pxp *pxp; >>> >>> struct i915_pmu pmu; >>> diff --git a/drivers/gpu/drm/i915/i915_gem.c >>> b/drivers/gpu/drm/i915/i915_gem.c >>> index e14a0c3db999..39b747c3e223 100644 >>> --- a/drivers/gpu/drm/i915/i915_gem.c >>> +++ b/drivers/gpu/drm/i915/i915_gem.c >>> @@ -1298,6 +1298,8 @@ 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->frontbuffer_lock); >>> } >>> >>> void i915_gem_cleanup_early(struct drm_i915_private *dev_priv) >>> diff --git a/drivers/gpu/drm/i915/i915_vma.c >>> b/drivers/gpu/drm/i915/i915_vma.c >>> index 25e97031d76e..cb36daaa101d 100644 >>> --- a/drivers/gpu/drm/i915/i915_vma.c >>> +++ b/drivers/gpu/drm/i915/i915_vma.c >>> @@ -1990,13 +1990,13 @@ int _i915_vma_move_to_active(struct i915_vma *vma, >>> } >>> >>> if (flags & EXEC_OBJECT_WRITE) { >>> - struct intel_frontbuffer *front; >>> + struct i915_frontbuffer *front; >>> >>> front = i915_gem_object_get_frontbuffer(obj); >>> if (unlikely(front)) { >>> - if (intel_frontbuffer_invalidate(front, ORIGIN_CS)) >>> + if (intel_frontbuffer_invalidate(&front->base, >>> ORIGIN_CS)) >>> i915_active_add_request(&front->write, rq); >>> - intel_frontbuffer_put(front); >>> + i915_gem_object_frontbuffer_put(front); >>> } >>> } >>> >>> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_vma.h >>> b/drivers/gpu/drm/xe/compat-i915-headers/i915_vma.h >>> index 4465c40f8134..b17e3bab23d5 100644 >>> --- a/drivers/gpu/drm/xe/compat-i915-headers/i915_vma.h >>> +++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_vma.h >>> @@ -26,8 +26,6 @@ struct i915_vma { >>> struct xe_ggtt_node *node; >>> }; >>> >>> -#define i915_ggtt_clear_scanout(bo) do { } while (0) >>> - >>> #define i915_vma_fence_id(vma) -1 >>> >>> static inline u32 i915_ggtt_offset(const struct i915_vma *vma) >>> diff --git a/drivers/gpu/drm/xe/display/intel_bo.c >>> b/drivers/gpu/drm/xe/display/intel_bo.c >>> index 2437c00a2d3e..bad2243b9114 100644 >>> --- a/drivers/gpu/drm/xe/display/intel_bo.c >>> +++ b/drivers/gpu/drm/xe/display/intel_bo.c >>> @@ -5,6 +5,7 @@ >>> >>> #include "xe_bo.h" >>> #include "intel_bo.h" >>> +#include "intel_frontbuffer.h" >>> >>> bool intel_bo_is_tiled(struct drm_gem_object *obj) >>> { >>> @@ -40,15 +41,56 @@ int intel_bo_read_from_page(struct drm_gem_object *obj, >>> u64 offset, void *dst, i >>> return xe_bo_read(bo, offset, dst, size); >>> } >>> >>> -struct intel_frontbuffer *intel_bo_get_frontbuffer(struct drm_gem_object >>> *obj) >>> +struct xe_frontbuffer { >>> + struct intel_frontbuffer base; >>> + struct drm_gem_object *obj; >>> + struct kref ref; >>> +}; >>> + >>> +struct intel_frontbuffer *intel_bo_frontbuffer_get(struct drm_gem_object >>> *obj) >>> { >>> - return NULL; >>> + struct xe_frontbuffer *front; >>> + >>> + front = kmalloc(sizeof(*front), GFP_KERNEL); >>> + if (!front) >>> + return NULL; >>> + >>> + intel_frontbuffer_init(&front->base, obj->dev); >>> + >>> + kref_init(&front->ref); >>> + >>> + drm_gem_object_get(obj); >>> + front->obj = obj; >>> + >>> + return &front->base; >>> } >>> >>> -struct intel_frontbuffer *intel_bo_set_frontbuffer(struct drm_gem_object >>> *obj, >>> - struct intel_frontbuffer >>> *front) >>> +void intel_bo_frontbuffer_ref(struct intel_frontbuffer *_front) >>> { >>> - return front; >>> + struct xe_frontbuffer *front = >>> + container_of(_front, typeof(*front), base); >>> + >>> + kref_get(&front->ref); >>> +} >>> + >>> +static void frontbuffer_release(struct kref *ref) >>> +{ >>> + struct xe_frontbuffer *front = >>> + container_of(ref, typeof(*front), ref); >>> + >>> + intel_frontbuffer_fini(&front->base); >>> + >>> + drm_gem_object_put(front->obj); >>> + >>> + kfree(front); >>> +} >>> + >>> +void intel_bo_frontbuffer_put(struct intel_frontbuffer *_front) >>> +{ >>> + struct xe_frontbuffer *front = >>> + container_of(_front, typeof(*front), base); >>> + >>> + kref_put(&front->ref, frontbuffer_release); >>> } >>> >>> void intel_bo_frontbuffer_flush_for_display(struct intel_frontbuffer >>> *front) -- Jani Nikula, Intel
