| Message ID | 20230530061417.2384188-3-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Do not access i915_gem_object members from frontbuffer tracking | expand |
On Tue, 30 May 2023, Jouni Högander <jouni.hogander@intel.com> wrote: > Add getter/setter for i915_gem_object->frontbuffer and use it instead of > directly touching i915_gem_object->frontbuffer frontbuffer pointer. Before going into the details (which, at a glance, look fine) I think we need to talk about the potential performance impact. I've never seen any other reason for the static inlines here than avoiding a function call when possible. Are there any other reasons? Is that a useless micro-optimization or something that could have an impact? On what? BR, Jani. > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > .../gpu/drm/i915/display/intel_frontbuffer.c | 18 ++--- > .../gpu/drm/i915/display/intel_frontbuffer.h | 27 ------- > drivers/gpu/drm/i915/gem/i915_gem_object.c | 70 ++++++++++++++++++- > drivers/gpu/drm/i915/gem/i915_gem_object.h | 6 ++ > drivers/gpu/drm/i915/i915_vma.c | 2 +- > 5 files changed, 81 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c > index 3ce0436a0c7d..41ac65c98720 100644 > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c > @@ -237,7 +237,7 @@ static void frontbuffer_release(struct kref *ref) > } > spin_unlock(&obj->vma.lock); > > - RCU_INIT_POINTER(obj->frontbuffer, NULL); > + i915_gem_object_set_frontbuffer(obj, NULL); > spin_unlock(&intel_bo_to_i915(obj)->display.fb_tracking.lock); > > i915_active_fini(&front->write); > @@ -250,9 +250,9 @@ struct intel_frontbuffer * > intel_frontbuffer_get(struct drm_i915_gem_object *obj) > { > struct drm_i915_private *i915 = intel_bo_to_i915(obj); > - struct intel_frontbuffer *front; > + struct intel_frontbuffer *front, *front_ret; > > - front = __intel_frontbuffer_get(obj); > + front = i915_gem_object_get_frontbuffer(obj); > if (front) > return front; > > @@ -269,16 +269,10 @@ intel_frontbuffer_get(struct drm_i915_gem_object *obj) > I915_ACTIVE_RETIRE_SLEEPS); > > spin_lock(&i915->display.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); > - } > + front_ret = i915_gem_object_set_frontbuffer(obj, front); > spin_unlock(&i915->display.fb_tracking.lock); > - > + if (front_ret != front) > + kfree(front); > return front; > } > > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h > index 3c474ed937fb..eeccc847331d 100644 > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h > @@ -75,33 +75,6 @@ void intel_frontbuffer_flip(struct drm_i915_private *i915, > > 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); > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c > index 46a19b099ec8..6945e903e106 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c > @@ -463,7 +463,7 @@ void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, > { > struct intel_frontbuffer *front; > > - front = __intel_frontbuffer_get(obj); > + front = i915_gem_object_get_frontbuffer(obj); > if (front) { > intel_frontbuffer_flush(front, origin); > intel_frontbuffer_put(front); > @@ -475,7 +475,7 @@ void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj, > { > struct intel_frontbuffer *front; > > - front = __intel_frontbuffer_get(obj); > + front = i915_gem_object_get_frontbuffer(obj); > if (front) { > intel_frontbuffer_invalidate(front, origin); > intel_frontbuffer_put(front); > @@ -952,6 +952,72 @@ bool i915_gem_object_has_unknown_state(struct drm_i915_gem_object *obj) > return obj->mm.unknown_state; > } > > +/** > + * i915_gem_object_get_frontbuffer - Get the object's frontbuffer > + * @obj: The object whose frontbuffer to get. > + * > + * Get pointer to object's frontbuffer if such exists. Please note that RCU > + * mechanism is used to handle e.g. ongoing removal of frontbuffer pointer. > + * > + * Return: pointer to object's frontbuffer is such exists or NULL > + */ > +struct intel_frontbuffer * > +i915_gem_object_get_frontbuffer(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; > +} > + > +/** > + * 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. > + * > + * Return: pointer to frontbuffer which was set. > + */ > +struct intel_frontbuffer * > +i915_gem_object_set_frontbuffer(struct drm_i915_gem_object *obj, > + struct intel_frontbuffer *front) > +{ > + struct intel_frontbuffer *front_ret = front; > + > + if (!front) { > + RCU_INIT_POINTER(obj->frontbuffer, NULL); > + } else if (rcu_access_pointer(obj->frontbuffer)) { > + front_ret = rcu_dereference_protected(obj->frontbuffer, true); > + kref_get(&front_ret->ref); > + } else { > + drm_gem_object_get(&intel_bo_to_drm_bo(obj)); > + rcu_assign_pointer(obj->frontbuffer, front); > + } > + > + return front_ret; > +} > + > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) > #include "selftests/huge_gem_object.c" > #include "selftests/huge_pages.c" > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h > index 884a17275b3a..69c5fa91152a 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > @@ -891,4 +891,10 @@ static inline int i915_gem_object_userptr_validate(struct drm_i915_gem_object *o > > #endif > > +struct intel_frontbuffer * > +i915_gem_object_get_frontbuffer(const struct drm_i915_gem_object *obj); > +struct intel_frontbuffer * > +i915_gem_object_set_frontbuffer(struct drm_i915_gem_object *obj, > + struct intel_frontbuffer *front); > + > #endif > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c > index ffb425ba591c..c66ff2157f6a 100644 > --- a/drivers/gpu/drm/i915/i915_vma.c > +++ b/drivers/gpu/drm/i915/i915_vma.c > @@ -1908,7 +1908,7 @@ int _i915_vma_move_to_active(struct i915_vma *vma, > if (flags & EXEC_OBJECT_WRITE) { > struct intel_frontbuffer *front; > > - front = __intel_frontbuffer_get(obj); > + front = i915_gem_object_get_frontbuffer(obj); > if (unlikely(front)) { > if (intel_frontbuffer_invalidate(front, ORIGIN_CS)) > i915_active_add_request(&front->write, rq);
On Fri, 2023-06-02 at 19:50 +0300, Jani Nikula wrote: > On Tue, 30 May 2023, Jouni Högander <jouni.hogander@intel.com> wrote: > > Add getter/setter for i915_gem_object->frontbuffer and use it > > instead of > > directly touching i915_gem_object->frontbuffer frontbuffer pointer. > > Before going into the details (which, at a glance, look fine) I think > we > need to talk about the potential performance impact. I've never seen > any > other reason for the static inlines here than avoiding a function > call > when possible. Are there any other reasons? Is that a useless > micro-optimization or something that could have an impact? On what? I was thinking this as well. I couldn't figure out any other reason for this being static inline than optimization. Maybe safest option would still be just move it to i915_gem_object.h and have set_frontbuffer there as well? > > BR, > Jani. > > > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > --- > > .../gpu/drm/i915/display/intel_frontbuffer.c | 18 ++--- > > .../gpu/drm/i915/display/intel_frontbuffer.h | 27 ------- > > drivers/gpu/drm/i915/gem/i915_gem_object.c | 70 > > ++++++++++++++++++- > > drivers/gpu/drm/i915/gem/i915_gem_object.h | 6 ++ > > drivers/gpu/drm/i915/i915_vma.c | 2 +- > > 5 files changed, 81 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c > > b/drivers/gpu/drm/i915/display/intel_frontbuffer.c > > index 3ce0436a0c7d..41ac65c98720 100644 > > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c > > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c > > @@ -237,7 +237,7 @@ static void frontbuffer_release(struct kref > > *ref) > > } > > spin_unlock(&obj->vma.lock); > > > > - RCU_INIT_POINTER(obj->frontbuffer, NULL); > > + i915_gem_object_set_frontbuffer(obj, NULL); > > spin_unlock(&intel_bo_to_i915(obj)- > > >display.fb_tracking.lock); > > > > i915_active_fini(&front->write); > > @@ -250,9 +250,9 @@ struct intel_frontbuffer * > > intel_frontbuffer_get(struct drm_i915_gem_object *obj) > > { > > struct drm_i915_private *i915 = intel_bo_to_i915(obj); > > - struct intel_frontbuffer *front; > > + struct intel_frontbuffer *front, *front_ret; > > > > - front = __intel_frontbuffer_get(obj); > > + front = i915_gem_object_get_frontbuffer(obj); > > if (front) > > return front; > > > > @@ -269,16 +269,10 @@ intel_frontbuffer_get(struct > > drm_i915_gem_object *obj) > > I915_ACTIVE_RETIRE_SLEEPS); > > > > spin_lock(&i915->display.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); > > - } > > + front_ret = i915_gem_object_set_frontbuffer(obj, front); > > spin_unlock(&i915->display.fb_tracking.lock); > > - > > + if (front_ret != front) > > + kfree(front); > > return front; > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h > > b/drivers/gpu/drm/i915/display/intel_frontbuffer.h > > index 3c474ed937fb..eeccc847331d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h > > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h > > @@ -75,33 +75,6 @@ void intel_frontbuffer_flip(struct > > drm_i915_private *i915, > > > > 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); > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c > > b/drivers/gpu/drm/i915/gem/i915_gem_object.c > > index 46a19b099ec8..6945e903e106 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c > > @@ -463,7 +463,7 @@ void __i915_gem_object_flush_frontbuffer(struct > > drm_i915_gem_object *obj, > > { > > struct intel_frontbuffer *front; > > > > - front = __intel_frontbuffer_get(obj); > > + front = i915_gem_object_get_frontbuffer(obj); > > if (front) { > > intel_frontbuffer_flush(front, origin); > > intel_frontbuffer_put(front); > > @@ -475,7 +475,7 @@ void > > __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object > > *obj, > > { > > struct intel_frontbuffer *front; > > > > - front = __intel_frontbuffer_get(obj); > > + front = i915_gem_object_get_frontbuffer(obj); > > if (front) { > > intel_frontbuffer_invalidate(front, origin); > > intel_frontbuffer_put(front); > > @@ -952,6 +952,72 @@ bool i915_gem_object_has_unknown_state(struct > > drm_i915_gem_object *obj) > > return obj->mm.unknown_state; > > } > > > > +/** > > + * i915_gem_object_get_frontbuffer - Get the object's frontbuffer > > + * @obj: The object whose frontbuffer to get. > > + * > > + * Get pointer to object's frontbuffer if such exists. Please note > > that RCU > > + * mechanism is used to handle e.g. ongoing removal of frontbuffer > > pointer. > > + * > > + * Return: pointer to object's frontbuffer is such exists or NULL > > + */ > > +struct intel_frontbuffer * > > +i915_gem_object_get_frontbuffer(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; > > +} > > + > > +/** > > + * 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. > > + * > > + * Return: pointer to frontbuffer which was set. > > + */ > > +struct intel_frontbuffer * > > +i915_gem_object_set_frontbuffer(struct drm_i915_gem_object *obj, > > + struct intel_frontbuffer *front) > > +{ > > + struct intel_frontbuffer *front_ret = front; > > + > > + if (!front) { > > + RCU_INIT_POINTER(obj->frontbuffer, NULL); > > + } else if (rcu_access_pointer(obj->frontbuffer)) { > > + front_ret = rcu_dereference_protected(obj- > > >frontbuffer, true); > > + kref_get(&front_ret->ref); > > + } else { > > + drm_gem_object_get(&intel_bo_to_drm_bo(obj)); > > + rcu_assign_pointer(obj->frontbuffer, front); > > + } > > + > > + return front_ret; > > +} > > + > > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) > > #include "selftests/huge_gem_object.c" > > #include "selftests/huge_pages.c" > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h > > b/drivers/gpu/drm/i915/gem/i915_gem_object.h > > index 884a17275b3a..69c5fa91152a 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > > @@ -891,4 +891,10 @@ static inline int > > i915_gem_object_userptr_validate(struct drm_i915_gem_object *o > > > > #endif > > > > +struct intel_frontbuffer * > > +i915_gem_object_get_frontbuffer(const struct drm_i915_gem_object > > *obj); > > +struct intel_frontbuffer * > > +i915_gem_object_set_frontbuffer(struct drm_i915_gem_object *obj, > > + struct intel_frontbuffer *front); > > + > > #endif > > diff --git a/drivers/gpu/drm/i915/i915_vma.c > > b/drivers/gpu/drm/i915/i915_vma.c > > index ffb425ba591c..c66ff2157f6a 100644 > > --- a/drivers/gpu/drm/i915/i915_vma.c > > +++ b/drivers/gpu/drm/i915/i915_vma.c > > @@ -1908,7 +1908,7 @@ int _i915_vma_move_to_active(struct i915_vma > > *vma, > > if (flags & EXEC_OBJECT_WRITE) { > > struct intel_frontbuffer *front; > > > > - front = __intel_frontbuffer_get(obj); > > + front = i915_gem_object_get_frontbuffer(obj); > > if (unlikely(front)) { > > if (intel_frontbuffer_invalidate(front, > > ORIGIN_CS)) > > i915_active_add_request(&front- > > >write, rq); >
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c index 3ce0436a0c7d..41ac65c98720 100644 --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c @@ -237,7 +237,7 @@ static void frontbuffer_release(struct kref *ref) } spin_unlock(&obj->vma.lock); - RCU_INIT_POINTER(obj->frontbuffer, NULL); + i915_gem_object_set_frontbuffer(obj, NULL); spin_unlock(&intel_bo_to_i915(obj)->display.fb_tracking.lock); i915_active_fini(&front->write); @@ -250,9 +250,9 @@ struct intel_frontbuffer * intel_frontbuffer_get(struct drm_i915_gem_object *obj) { struct drm_i915_private *i915 = intel_bo_to_i915(obj); - struct intel_frontbuffer *front; + struct intel_frontbuffer *front, *front_ret; - front = __intel_frontbuffer_get(obj); + front = i915_gem_object_get_frontbuffer(obj); if (front) return front; @@ -269,16 +269,10 @@ intel_frontbuffer_get(struct drm_i915_gem_object *obj) I915_ACTIVE_RETIRE_SLEEPS); spin_lock(&i915->display.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); - } + front_ret = i915_gem_object_set_frontbuffer(obj, front); spin_unlock(&i915->display.fb_tracking.lock); - + if (front_ret != front) + kfree(front); return front; } diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h index 3c474ed937fb..eeccc847331d 100644 --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h @@ -75,33 +75,6 @@ void intel_frontbuffer_flip(struct drm_i915_private *i915, 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); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 46a19b099ec8..6945e903e106 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -463,7 +463,7 @@ void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, { struct intel_frontbuffer *front; - front = __intel_frontbuffer_get(obj); + front = i915_gem_object_get_frontbuffer(obj); if (front) { intel_frontbuffer_flush(front, origin); intel_frontbuffer_put(front); @@ -475,7 +475,7 @@ void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj, { struct intel_frontbuffer *front; - front = __intel_frontbuffer_get(obj); + front = i915_gem_object_get_frontbuffer(obj); if (front) { intel_frontbuffer_invalidate(front, origin); intel_frontbuffer_put(front); @@ -952,6 +952,72 @@ bool i915_gem_object_has_unknown_state(struct drm_i915_gem_object *obj) return obj->mm.unknown_state; } +/** + * i915_gem_object_get_frontbuffer - Get the object's frontbuffer + * @obj: The object whose frontbuffer to get. + * + * Get pointer to object's frontbuffer if such exists. Please note that RCU + * mechanism is used to handle e.g. ongoing removal of frontbuffer pointer. + * + * Return: pointer to object's frontbuffer is such exists or NULL + */ +struct intel_frontbuffer * +i915_gem_object_get_frontbuffer(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; +} + +/** + * 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. + * + * Return: pointer to frontbuffer which was set. + */ +struct intel_frontbuffer * +i915_gem_object_set_frontbuffer(struct drm_i915_gem_object *obj, + struct intel_frontbuffer *front) +{ + struct intel_frontbuffer *front_ret = front; + + if (!front) { + RCU_INIT_POINTER(obj->frontbuffer, NULL); + } else if (rcu_access_pointer(obj->frontbuffer)) { + front_ret = rcu_dereference_protected(obj->frontbuffer, true); + kref_get(&front_ret->ref); + } else { + drm_gem_object_get(&intel_bo_to_drm_bo(obj)); + rcu_assign_pointer(obj->frontbuffer, front); + } + + return front_ret; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/huge_gem_object.c" #include "selftests/huge_pages.c" diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 884a17275b3a..69c5fa91152a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -891,4 +891,10 @@ static inline int i915_gem_object_userptr_validate(struct drm_i915_gem_object *o #endif +struct intel_frontbuffer * +i915_gem_object_get_frontbuffer(const struct drm_i915_gem_object *obj); +struct intel_frontbuffer * +i915_gem_object_set_frontbuffer(struct drm_i915_gem_object *obj, + struct intel_frontbuffer *front); + #endif diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index ffb425ba591c..c66ff2157f6a 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -1908,7 +1908,7 @@ int _i915_vma_move_to_active(struct i915_vma *vma, if (flags & EXEC_OBJECT_WRITE) { struct intel_frontbuffer *front; - front = __intel_frontbuffer_get(obj); + front = i915_gem_object_get_frontbuffer(obj); if (unlikely(front)) { if (intel_frontbuffer_invalidate(front, ORIGIN_CS)) i915_active_add_request(&front->write, rq);
Add getter/setter for i915_gem_object->frontbuffer and use it instead of directly touching i915_gem_object->frontbuffer frontbuffer pointer. Signed-off-by: Jouni Högander <jouni.hogander@intel.com> --- .../gpu/drm/i915/display/intel_frontbuffer.c | 18 ++--- .../gpu/drm/i915/display/intel_frontbuffer.h | 27 ------- drivers/gpu/drm/i915/gem/i915_gem_object.c | 70 ++++++++++++++++++- drivers/gpu/drm/i915/gem/i915_gem_object.h | 6 ++ drivers/gpu/drm/i915/i915_vma.c | 2 +- 5 files changed, 81 insertions(+), 42 deletions(-)