intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking
@ 2023-05-30  6:14 Jouni Högander
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 1/4] drm/i915: Add macros to get i915 device from i915_gem_object Jouni Högander
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Jouni Högander @ 2023-05-30  6:14 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Rodrigo Vivi

We are preparing for Xe driver. Binary objects will have differing
implementation in Xe driver. Due this we want to remove direct
accesses to i915_gem_object members and leave details to binary object
implementation.

v2: desribe i915_ggtt_clear_scanout function parameter

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Jouni Högander (4):
  drm/i915: Add macros to get i915 device from i915_gem_object
  drm/i915: Add getter/setter for i915_gem_object->frontbuffer
  drm/i915/display: Remove i915_gem_object_types.h from
    intel_frontbuffer.h
  drm/i915: Add function to clear scanout flag for vmas

 .../gpu/drm/i915/display/intel_frontbuffer.c  | 44 +++++-------
 .../gpu/drm/i915/display/intel_frontbuffer.h  | 28 --------
 drivers/gpu/drm/i915/gem/i915_gem_object.c    | 70 ++++++++++++++++++-
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |  6 ++
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  3 +
 drivers/gpu/drm/i915/i915_vma.c               | 22 +++++-
 drivers/gpu/drm/i915/i915_vma.h               |  2 +
 7 files changed, 116 insertions(+), 59 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH v2 1/4] drm/i915: Add macros to get i915 device from i915_gem_object
  2023-05-30  6:14 [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking Jouni Högander
@ 2023-05-30  6:14 ` Jouni Högander
  2023-06-02 16:46   ` Jani Nikula
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 2/4] drm/i915: Add getter/setter for i915_gem_object->frontbuffer Jouni Högander
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Jouni Högander @ 2023-05-30  6:14 UTC (permalink / raw)
  To: intel-gfx

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@intel.com>
---
 .../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)
+#define intel_bo_to_i915(bo) to_i915(intel_bo_to_drm_bo(bo).dev)
+
 static inline struct drm_i915_gem_object *
 to_intel_bo(struct drm_gem_object *gem)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH v2 2/4] drm/i915: Add getter/setter for i915_gem_object->frontbuffer
  2023-05-30  6:14 [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking Jouni Högander
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 1/4] drm/i915: Add macros to get i915 device from i915_gem_object Jouni Högander
@ 2023-05-30  6:14 ` Jouni Högander
  2023-06-02 16:50   ` Jani Nikula
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 3/4] drm/i915/display: Remove i915_gem_object_types.h from intel_frontbuffer.h Jouni Högander
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Jouni Högander @ 2023-05-30  6:14 UTC (permalink / raw)
  To: intel-gfx

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(-)

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);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH v2 3/4] drm/i915/display: Remove i915_gem_object_types.h from intel_frontbuffer.h
  2023-05-30  6:14 [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking Jouni Högander
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 1/4] drm/i915: Add macros to get i915 device from i915_gem_object Jouni Högander
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 2/4] drm/i915: Add getter/setter for i915_gem_object->frontbuffer Jouni Högander
@ 2023-05-30  6:14 ` Jouni Högander
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 4/4] drm/i915: Add function to clear scanout flag for vmas Jouni Högander
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Jouni Högander @ 2023-05-30  6:14 UTC (permalink / raw)
  To: intel-gfx

Now as we have removed all the references to internals of i915_gem_object
from the frontbuffer header we can also remove including
i915_gem_object_types.h.

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_frontbuffer.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
index eeccc847331d..72d89be3284b 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/bits.h>
 #include <linux/kref.h>
 
-#include "gem/i915_gem_object_types.h"
 #include "i915_active_types.h"
 
 struct drm_i915_private;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH v2 4/4] drm/i915: Add function to clear scanout flag for vmas
  2023-05-30  6:14 [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking Jouni Högander
                   ` (2 preceding siblings ...)
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 3/4] drm/i915/display: Remove i915_gem_object_types.h from intel_frontbuffer.h Jouni Högander
@ 2023-05-30  6:14 ` Jouni Högander
  2023-06-02 16:52   ` Jani Nikula
  2023-05-31  2:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Do not access i915_gem_object members from frontbuffer tracking (rev2) Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Jouni Högander @ 2023-05-30  6:14 UTC (permalink / raw)
  To: intel-gfx

Currently frontbuffer tracking code is directly iterating over object vmas
and clearing scanout flags for them. Add function to clear scanout flag for
vmas and use it from frontbuffer tracking code.

v2: describe function parameter.

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 .../gpu/drm/i915/display/intel_frontbuffer.c  |  8 +-------
 drivers/gpu/drm/i915/i915_vma.c               | 20 +++++++++++++++++++
 drivers/gpu/drm/i915/i915_vma.h               |  2 ++
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 41ac65c98720..29ac068b8fa5 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -226,16 +226,10 @@ static void frontbuffer_release(struct kref *ref)
 	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(&intel_bo_to_i915(obj)->drm, 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);
+	i915_ggtt_clear_scanout(obj);
 
 	i915_gem_object_set_frontbuffer(obj, NULL);
 	spin_unlock(&intel_bo_to_i915(obj)->display.fb_tracking.lock);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index c66ff2157f6a..c6be96206ee5 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1629,6 +1629,26 @@ int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
 	return err;
 }
 
+/**
+ * i915_ggtt_clear_scanout - Clear scanout flag for all objects ggtt vmas
+ * @obj: i915 GEM object
+ * This function clears scanout flags for objects ggtt vmas. These flags are set
+ * when object is pinned for display use and this function to clear them all is
+ * targeted to be called by frontbuffer tracking code when the frontbuffer is
+ * about to be released.
+ */
+void i915_ggtt_clear_scanout(struct drm_i915_gem_object *obj)
+{
+	struct i915_vma *vma;
+
+	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);
+}
+
 static void __vma_close(struct i915_vma *vma, struct intel_gt *gt)
 {
 	/*
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 9a9729205d5b..eaa310864370 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -435,6 +435,8 @@ static inline void i915_vma_clear_scanout(struct i915_vma *vma)
 	clear_bit(I915_VMA_SCANOUT_BIT, __i915_vma_flags(vma));
 }
 
+void i915_ggtt_clear_scanout(struct drm_i915_gem_object *obj);
+
 #define for_each_until(cond) if (cond) break; else
 
 /**
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Do not access i915_gem_object members from frontbuffer tracking (rev2)
  2023-05-30  6:14 [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking Jouni Högander
                   ` (3 preceding siblings ...)
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 4/4] drm/i915: Add function to clear scanout flag for vmas Jouni Högander
@ 2023-05-31  2:51 ` Patchwork
  2023-05-31  3:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2023-06-01  8:06 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2023-05-31  2:51 UTC (permalink / raw)
  To: Jouni Högander; +Cc: intel-gfx

== Series Details ==

Series: Do not access i915_gem_object members from frontbuffer tracking (rev2)
URL   : https://patchwork.freedesktop.org/series/118475/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for Do not access i915_gem_object members from frontbuffer tracking (rev2)
  2023-05-30  6:14 [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking Jouni Högander
                   ` (4 preceding siblings ...)
  2023-05-31  2:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Do not access i915_gem_object members from frontbuffer tracking (rev2) Patchwork
@ 2023-05-31  3:07 ` Patchwork
  2023-06-01  8:06 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2023-05-31  3:07 UTC (permalink / raw)
  To: Jouni Högander; +Cc: intel-gfx

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

== Series Details ==

Series: Do not access i915_gem_object members from frontbuffer tracking (rev2)
URL   : https://patchwork.freedesktop.org/series/118475/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13202 -> Patchwork_118475v2
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/index.html

Participating hosts (38 -> 35)
------------------------------

  Missing    (3): fi-kbl-soraka bat-adln-1 bat-mtlp-6 

Known issues
------------

  Here are the changes found in Patchwork_118475v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@requests:
    - bat-rpls-1:         [PASS][1] -> [ABORT][2] ([i915#7911] / [i915#7920] / [i915#7982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-rpls-1/igt@i915_selftest@live@requests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/bat-rpls-1/igt@i915_selftest@live@requests.html

  
#### Possible fixes ####

  * igt@core_auth@basic-auth:
    - {bat-adlp-11}:      [ABORT][3] ([i915#8011]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-adlp-11/igt@core_auth@basic-auth.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/bat-adlp-11/igt@core_auth@basic-auth.html

  * igt@i915_module_load@load:
    - {bat-adlp-11}:      [DMESG-WARN][5] ([i915#4423] / [i915#8189]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-adlp-11/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/bat-adlp-11/igt@i915_module_load@load.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-hsw-4770:        [SKIP][7] ([fdo#109271]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/fi-hsw-4770/igt@i915_pm_rpm@basic-pci-d3-state.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/fi-hsw-4770/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rpm@basic-rte:
    - fi-hsw-4770:        [FAIL][9] ([i915#7364]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/fi-hsw-4770/igt@i915_pm_rpm@basic-rte.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/fi-hsw-4770/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_selftest@live@gt_mocs:
    - {bat-mtlp-8}:       [DMESG-FAIL][11] ([i915#7059]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@hangcheck:
    - fi-skl-guc:         [DMESG-WARN][13] ([i915#8073]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/fi-skl-guc/igt@i915_selftest@live@hangcheck.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/fi-skl-guc/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@slpc:
    - {bat-mtlp-8}:       [DMESG-WARN][15] ([i915#6367]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/bat-mtlp-8/igt@i915_selftest@live@slpc.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/bat-mtlp-8/igt@i915_selftest@live@slpc.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
  [i915#6121]: https://gitlab.freedesktop.org/drm/intel/issues/6121
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6868]: https://gitlab.freedesktop.org/drm/intel/issues/6868
  [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
  [i915#7364]: https://gitlab.freedesktop.org/drm/intel/issues/7364
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
  [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
  [i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8073]: https://gitlab.freedesktop.org/drm/intel/issues/8073
  [i915#8189]: https://gitlab.freedesktop.org/drm/intel/issues/8189


Build changes
-------------

  * Linux: CI_DRM_13202 -> Patchwork_118475v2

  CI-20190529: 20190529
  CI_DRM_13202: cb4a9d17f1ae011ad60f6bf502b0c7216d6390d0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7311: c031030f39aff973330668a5a2f1593408da78ae @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_118475v2: cb4a9d17f1ae011ad60f6bf502b0c7216d6390d0 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

b5180f27a655 drm/i915: Add function to clear scanout flag for vmas
0156bf9d5c00 drm/i915/display: Remove i915_gem_object_types.h from intel_frontbuffer.h
598d90e57921 drm/i915: Add getter/setter for i915_gem_object->frontbuffer
d535f01c01c4 drm/i915: Add macros to get i915 device from i915_gem_object

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/index.html

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for Do not access i915_gem_object members from frontbuffer tracking (rev2)
  2023-05-30  6:14 [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking Jouni Högander
                   ` (5 preceding siblings ...)
  2023-05-31  3:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2023-06-01  8:06 ` Patchwork
  6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2023-06-01  8:06 UTC (permalink / raw)
  To: Jouni Högander; +Cc: intel-gfx

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

== Series Details ==

Series: Do not access i915_gem_object members from frontbuffer tracking (rev2)
URL   : https://patchwork.freedesktop.org/series/118475/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13202_full -> Patchwork_118475v2_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

New tests
---------

  New tests have been introduced between CI_DRM_13202_full and Patchwork_118475v2_full:

### New IGT tests (1) ###

  * igt@kms_flip@flip-vs-panning-vs-hang@b-hdmi-a1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  

Known issues
------------

  Here are the changes found in Patchwork_118475v2_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-deadline:
    - shard-glk:          [PASS][1] -> [FAIL][2] ([i915#2846])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-glk3/igt@gem_exec_fair@basic-deadline.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk6/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [PASS][3] -> [FAIL][4] ([i915#2842])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-apl1/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-apl7/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#2842])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-glk4/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk5/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_lmem_swapping@verify-ccs:
    - shard-glk:          NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#4613])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk8/igt@gem_lmem_swapping@verify-ccs.html

  * igt@i915_pm_rps@reset:
    - shard-snb:          [PASS][8] -> [INCOMPLETE][9] ([i915#7790])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-snb4/igt@i915_pm_rps@reset.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-snb2/igt@i915_pm_rps@reset.html

  * igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_rc_ccs:
    - shard-glk:          NOTRUN -> [SKIP][10] ([fdo#109271]) +16 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk8/igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_rc_ccs.html

  * igt@kms_ccs@pipe-b-bad-rotation-90-y_tiled_gen12_mc_ccs:
    - shard-glk:          NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#3886])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk8/igt@kms_ccs@pipe-b-bad-rotation-90-y_tiled_gen12_mc_ccs.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [PASS][12] -> [FAIL][13] ([i915#2346])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a2:
    - shard-glk:          [PASS][14] -> [FAIL][15] ([i915#79])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a2.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a2.html

  * igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-32bpp-4tiledg2rcccs-upscaling@pipe-a-valid-mode:
    - shard-glk:          NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#4579]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk3/igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-32bpp-4tiledg2rcccs-upscaling@pipe-a-valid-mode.html

  * igt@kms_plane_scaling@plane-upscale-with-modifiers-20x20@pipe-b-hdmi-a-1:
    - shard-snb:          NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#4579]) +16 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-snb1/igt@kms_plane_scaling@plane-upscale-with-modifiers-20x20@pipe-b-hdmi-a-1.html

  * igt@kms_plane_scaling@plane-upscale-with-modifiers-factor-0-25@pipe-a-vga-1:
    - shard-snb:          NOTRUN -> [SKIP][18] ([fdo#109271]) +19 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-snb7/igt@kms_plane_scaling@plane-upscale-with-modifiers-factor-0-25@pipe-a-vga-1.html

  * igt@kms_setmode@basic@pipe-a-hdmi-a-1:
    - shard-snb:          NOTRUN -> [FAIL][19] ([i915#5465]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-snb1/igt@kms_setmode@basic@pipe-a-hdmi-a-1.html

  * igt@perf@stress-open-close@0-rcs0:
    - shard-glk:          NOTRUN -> [ABORT][20] ([i915#5213] / [i915#7941])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk3/igt@perf@stress-open-close@0-rcs0.html

  
#### Possible fixes ####

  * igt@gem_barrier_race@remote-request@rcs0:
    - shard-glk:          [ABORT][21] ([i915#7461] / [i915#8211]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-glk1/igt@gem_barrier_race@remote-request@rcs0.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk8/igt@gem_barrier_race@remote-request@rcs0.html

  * igt@gem_lmem_swapping@smem-oom@lmem0:
    - {shard-dg1}:        [TIMEOUT][23] ([i915#5493]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-dg1-18/igt@gem_lmem_swapping@smem-oom@lmem0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-dg1-14/igt@gem_lmem_swapping@smem-oom@lmem0.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - {shard-dg1}:        [SKIP][25] ([i915#1937] / [i915#4579]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-dg1-17/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-dg1-19/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@i915_pm_rc6_residency@rc6-idle@rcs0:
    - {shard-dg1}:        [FAIL][27] ([i915#3591]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-dg1-19/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-dg1-18/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html

  * igt@i915_pm_rpm@dpms-mode-unset-lpsp:
    - {shard-rkl}:        [SKIP][29] ([i915#1397]) -> [PASS][30] +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-rkl-2/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-rkl-7/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip:
    - {shard-dg1}:        [FAIL][31] ([i915#3743] / [i915#7959]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-dg1-19/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-dg1-18/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [FAIL][33] ([i915#2346]) -> [PASS][34] +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-apl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-apl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@single-bo@pipe-b:
    - {shard-dg1}:        [INCOMPLETE][35] ([i915#8011] / [i915#8347]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-dg1-19/igt@kms_cursor_legacy@single-bo@pipe-b.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-dg1-18/igt@kms_cursor_legacy@single-bo@pipe-b.html

  * igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1:
    - shard-glk:          [FAIL][37] ([i915#79]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-glk9/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-glk3/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1.html

  * igt@kms_rotation_crc@primary-rotation-90:
    - {shard-rkl}:        [ABORT][39] ([i915#8311]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13202/shard-rkl-4/igt@kms_rotation_crc@primary-rotation-90.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/shard-rkl-4/igt@kms_rotation_crc@primary-rotation-90.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3743]: https://gitlab.freedesktop.org/drm/intel/issues/3743
  [i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5213]: https://gitlab.freedesktop.org/drm/intel/issues/5213
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5465]: https://gitlab.freedesktop.org/drm/intel/issues/5465
  [i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493
  [i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6755]: https://gitlab.freedesktop.org/drm/intel/issues/6755
  [i915#6786]: https://gitlab.freedesktop.org/drm/intel/issues/6786
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7790]: https://gitlab.freedesktop.org/drm/intel/issues/7790
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7941]: https://gitlab.freedesktop.org/drm/intel/issues/7941
  [i915#7959]: https://gitlab.freedesktop.org/drm/intel/issues/7959
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247
  [i915#8311]: https://gitlab.freedesktop.org/drm/intel/issues/8311
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
  [i915#8414]: https://gitlab.freedesktop.org/drm/intel/issues/8414
  [i915#8562]: https://gitlab.freedesktop.org/drm/intel/issues/8562


Build changes
-------------

  * Linux: CI_DRM_13202 -> Patchwork_118475v2

  CI-20190529: 20190529
  CI_DRM_13202: cb4a9d17f1ae011ad60f6bf502b0c7216d6390d0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7311: c031030f39aff973330668a5a2f1593408da78ae @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_118475v2: cb4a9d17f1ae011ad60f6bf502b0c7216d6390d0 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118475v2/index.html

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add macros to get i915 device from i915_gem_object
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 1/4] drm/i915: Add macros to get i915 device from i915_gem_object Jouni Högander
@ 2023-06-02 16:46   ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2023-06-02 16:46 UTC (permalink / raw)
  To: Jouni Högander, intel-gfx

On Tue, 30 May 2023, Jouni Högander <jouni.hogander@intel.com> 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@intel.com>
> ---
>  .../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@intel.com>


> +
>  static inline struct drm_i915_gem_object *
>  to_intel_bo(struct drm_gem_object *gem)
>  {

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: Add getter/setter for i915_gem_object->frontbuffer
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 2/4] drm/i915: Add getter/setter for i915_gem_object->frontbuffer Jouni Högander
@ 2023-06-02 16:50   ` Jani Nikula
  2023-06-05  8:43     ` Hogander, Jouni
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2023-06-02 16:50 UTC (permalink / raw)
  To: Jouni Högander, intel-gfx

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);

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v2 4/4] drm/i915: Add function to clear scanout flag for vmas
  2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 4/4] drm/i915: Add function to clear scanout flag for vmas Jouni Högander
@ 2023-06-02 16:52   ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2023-06-02 16:52 UTC (permalink / raw)
  To: Jouni Högander, intel-gfx

On Tue, 30 May 2023, Jouni Högander <jouni.hogander@intel.com> wrote:
> Currently frontbuffer tracking code is directly iterating over object vmas
> and clearing scanout flags for them. Add function to clear scanout flag for
> vmas and use it from frontbuffer tracking code.
>
> v2: describe function parameter.
>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>

I don't really have confidence on what the right placement of the
function should be, but it does what it says on the box.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  .../gpu/drm/i915/display/intel_frontbuffer.c  |  8 +-------
>  drivers/gpu/drm/i915/i915_vma.c               | 20 +++++++++++++++++++
>  drivers/gpu/drm/i915/i915_vma.h               |  2 ++
>  3 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> index 41ac65c98720..29ac068b8fa5 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> @@ -226,16 +226,10 @@ static void frontbuffer_release(struct kref *ref)
>  	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(&intel_bo_to_i915(obj)->drm, 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);
> +	i915_ggtt_clear_scanout(obj);
>  
>  	i915_gem_object_set_frontbuffer(obj, NULL);
>  	spin_unlock(&intel_bo_to_i915(obj)->display.fb_tracking.lock);
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index c66ff2157f6a..c6be96206ee5 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -1629,6 +1629,26 @@ int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
>  	return err;
>  }
>  
> +/**
> + * i915_ggtt_clear_scanout - Clear scanout flag for all objects ggtt vmas
> + * @obj: i915 GEM object
> + * This function clears scanout flags for objects ggtt vmas. These flags are set
> + * when object is pinned for display use and this function to clear them all is
> + * targeted to be called by frontbuffer tracking code when the frontbuffer is
> + * about to be released.
> + */
> +void i915_ggtt_clear_scanout(struct drm_i915_gem_object *obj)
> +{
> +	struct i915_vma *vma;
> +
> +	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);
> +}
> +
>  static void __vma_close(struct i915_vma *vma, struct intel_gt *gt)
>  {
>  	/*
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 9a9729205d5b..eaa310864370 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -435,6 +435,8 @@ static inline void i915_vma_clear_scanout(struct i915_vma *vma)
>  	clear_bit(I915_VMA_SCANOUT_BIT, __i915_vma_flags(vma));
>  }
>  
> +void i915_ggtt_clear_scanout(struct drm_i915_gem_object *obj);
> +
>  #define for_each_until(cond) if (cond) break; else
>  
>  /**

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: Add getter/setter for i915_gem_object->frontbuffer
  2023-06-02 16:50   ` Jani Nikula
@ 2023-06-05  8:43     ` Hogander, Jouni
  0 siblings, 0 replies; 12+ messages in thread
From: Hogander, Jouni @ 2023-06-05  8:43 UTC (permalink / raw)
  To: intel-gfx, jani.nikula

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);
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-06-05  8:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-30  6:14 [Intel-gfx] [PATCH v2 0/4] Do not access i915_gem_object members from frontbuffer tracking Jouni Högander
2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 1/4] drm/i915: Add macros to get i915 device from i915_gem_object Jouni Högander
2023-06-02 16:46   ` Jani Nikula
2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 2/4] drm/i915: Add getter/setter for i915_gem_object->frontbuffer Jouni Högander
2023-06-02 16:50   ` Jani Nikula
2023-06-05  8:43     ` Hogander, Jouni
2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 3/4] drm/i915/display: Remove i915_gem_object_types.h from intel_frontbuffer.h Jouni Högander
2023-05-30  6:14 ` [Intel-gfx] [PATCH v2 4/4] drm/i915: Add function to clear scanout flag for vmas Jouni Högander
2023-06-02 16:52   ` Jani Nikula
2023-05-31  2:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Do not access i915_gem_object members from frontbuffer tracking (rev2) Patchwork
2023-05-31  3:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-06-01  8:06 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).