On Tue, 30 May 2023, Jouni Högander <jouni.hogander@xxxxxxxxx> wrote:
> We want to stop touching directly i915_gem_object struct members in
> intel_frontbuffer code. As a part of this we add helper macro to get i915
> device from i915_gem_object.
>
> Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> ---
> .../gpu/drm/i915/display/intel_frontbuffer.c | 18 +++++++++---------
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 +++
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> index 17a7aa8b28c2..3ce0436a0c7d 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> @@ -167,7 +167,7 @@ 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 = intel_bo_to_i915(front->obj);
>
> if (origin == ORIGIN_CS) {
> spin_lock(&i915->display.fb_tracking.lock);
> @@ -188,7 +188,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 = intel_bo_to_i915(front->obj);
>
> if (origin == ORIGIN_CS) {
> spin_lock(&i915->display.fb_tracking.lock);
> @@ -221,14 +221,14 @@ static void frontbuffer_retire(struct i915_active *ref)
> }
>
> static void frontbuffer_release(struct kref *ref)
> - __releases(&to_i915(front->obj->base.dev)->display.fb_tracking.lock)
> + __releases(&intel_bo_to_i915(front->obj)->display.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));
> + drm_WARN_ON(&intel_bo_to_i915(obj)->drm, atomic_read(&front->bits));
>
> spin_lock(&obj->vma.lock);
> for_each_ggtt_vma(vma, obj) {
> @@ -238,7 +238,7 @@ static void frontbuffer_release(struct kref *ref)
> spin_unlock(&obj->vma.lock);
>
> RCU_INIT_POINTER(obj->frontbuffer, NULL);
> - spin_unlock(&to_i915(obj->base.dev)->display.fb_tracking.lock);
> + spin_unlock(&intel_bo_to_i915(obj)->display.fb_tracking.lock);
>
> i915_active_fini(&front->write);
>
> @@ -249,7 +249,7 @@ static void frontbuffer_release(struct kref *ref)
> struct intel_frontbuffer *
> intel_frontbuffer_get(struct drm_i915_gem_object *obj)
> {
> - struct drm_i915_private *i915 = to_i915(obj->base.dev);
> + struct drm_i915_private *i915 = intel_bo_to_i915(obj);
> struct intel_frontbuffer *front;
>
> front = __intel_frontbuffer_get(obj);
> @@ -286,7 +286,7 @@ void intel_frontbuffer_put(struct intel_frontbuffer *front)
> {
> kref_put_lock(&front->ref,
> frontbuffer_release,
> - &to_i915(front->obj->base.dev)->display.fb_tracking.lock);
> + &intel_bo_to_i915(front->obj)->display.fb_tracking.lock);
> }
>
> /**
> @@ -315,13 +315,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_bo_to_i915(old->obj)->drm,
> !(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_bo_to_i915(new->obj)->drm,
> atomic_read(&new->bits) & frontbuffer_bits);
> atomic_or(frontbuffer_bits, &new->bits);
> }
> 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 e72c57716bee..658bdac2a75f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -718,6 +718,9 @@ struct drm_i915_gem_object {
> };
> };
>
> +#define intel_bo_to_drm_bo(bo) ((bo)->base)
All of the foo_to_bar() things we have operate on and return pointers,
and I think this one should too.
I.e. (&(bo)->base)
> +#define intel_bo_to_i915(bo) to_i915(intel_bo_to_drm_bo(bo).dev)
And consequently this needs to have ->dev.
Other than that,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> +
> static inline struct drm_i915_gem_object *
> to_intel_bo(struct drm_gem_object *gem)
> {
--
Jani Nikula, Intel Open Source Graphics Center
