All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
@ 2021-09-22 14:57 ` Matthew Brost
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2021-09-22 14:57 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: jani.nikula, tvrtko.ursulin, lucas.demarchi

Rather than stealing bits from i915_sw_fence function pointer use
seperate fields for function pointer and flags. If using two different
fields, the 4 byte alignment for the i915_sw_fence function pointer can
also be dropped.

v2:
 (CI)
  - Set new function field rather than flags in __i915_sw_fence_init

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
 drivers/gpu/drm/i915/i915_request.c           |  4 ++--
 drivers/gpu/drm/i915/i915_sw_fence.c          | 12 +++++------
 drivers/gpu/drm/i915/i915_sw_fence.h          | 21 +++++++++----------
 drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
 .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
 drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  4 ++--
 8 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a7ca38613f89..6d5bb55ffc82 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10323,7 +10323,7 @@ static void intel_atomic_commit_work(struct work_struct *work)
 	intel_atomic_commit_tail(state);
 }
 
-static int __i915_sw_fence_call
+static int
 intel_atomic_commit_ready(struct i915_sw_fence *fence,
 			  enum i915_sw_fence_notify notify)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c2ab0e22db0a..df5fec5c3da8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -800,7 +800,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
 	free_engines(engines);
 }
 
-static int __i915_sw_fence_call
+static int
 engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_gem_engines *engines =
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index ce446716d092..945d3025a0b6 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, int error)
 	intel_context_cancel_request(rq->context, rq);
 }
 
-static int __i915_sw_fence_call
+static int
 submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *request =
@@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 	return NOTIFY_DONE;
 }
 
-static int __i915_sw_fence_call
+static int
 semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *rq = container_of(fence, typeof(*rq), semaphore);
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..1c080dd1f718 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -34,7 +34,7 @@ enum {
 
 static void *i915_sw_fence_debug_hint(void *addr)
 {
-	return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK);
+	return (void *)(((struct i915_sw_fence *)addr)->fn);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -126,10 +126,7 @@ static inline void debug_fence_assert(struct i915_sw_fence *fence)
 static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
 				  enum i915_sw_fence_notify state)
 {
-	i915_sw_fence_notify_t fn;
-
-	fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
-	return fn(fence, state);
+	return fence->fn(fence, state);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -242,10 +239,11 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  const char *name,
 			  struct lock_class_key *key)
 {
-	BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
+	BUG_ON(!fn);
 
 	__init_waitqueue_head(&fence->wait, name, key);
-	fence->flags = (unsigned long)fn;
+	fence->fn = fn;
+	fence->flags = 0;
 
 	i915_sw_fence_reinit(fence);
 }
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h
index 30a863353ee6..70ba1789aa89 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.h
+++ b/drivers/gpu/drm/i915/i915_sw_fence.h
@@ -17,26 +17,25 @@
 
 struct completion;
 struct dma_resv;
+struct i915_sw_fence;
+
+enum i915_sw_fence_notify {
+	FENCE_COMPLETE,
+	FENCE_FREE
+};
+
+typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
+				      enum i915_sw_fence_notify state);
 
 struct i915_sw_fence {
 	wait_queue_head_t wait;
+	i915_sw_fence_notify_t fn;
 	unsigned long flags;
 	atomic_t pending;
 	int error;
 };
 
 #define I915_SW_FENCE_CHECKED_BIT	0 /* used internally for DAG checking */
-#define I915_SW_FENCE_PRIVATE_BIT	1 /* available for use by owner */
-#define I915_SW_FENCE_MASK		(~3)
-
-enum i915_sw_fence_notify {
-	FENCE_COMPLETE,
-	FENCE_FREE
-};
-
-typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
-				      enum i915_sw_fence_notify state);
-#define __i915_sw_fence_call __aligned(4)
 
 void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  i915_sw_fence_notify_t fn,
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
index 5b33ef23d54c..d2e56b387993 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
@@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
 	dma_fence_put(&f->dma);
 }
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
index cbf45d85cbff..daa985e5a19b 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -28,7 +28,7 @@
 
 #include "../i915_selftest.h"
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	switch (state) {
diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
index 080b90b63d16..eb59a41bdb79 100644
--- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
@@ -26,7 +26,7 @@
 
 /* Small library of different fence types useful for writing tests */
 
-static int __i915_sw_fence_call
+static int
 nop_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	return NOTIFY_DONE;
@@ -89,7 +89,7 @@ struct heap_fence {
 	};
 };
 
-static int __i915_sw_fence_call
+static int
 heap_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct heap_fence *h = container_of(fence, typeof(*h), fence);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
@ 2021-09-22 14:57 ` Matthew Brost
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2021-09-22 14:57 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: jani.nikula, tvrtko.ursulin, lucas.demarchi

Rather than stealing bits from i915_sw_fence function pointer use
seperate fields for function pointer and flags. If using two different
fields, the 4 byte alignment for the i915_sw_fence function pointer can
also be dropped.

v2:
 (CI)
  - Set new function field rather than flags in __i915_sw_fence_init

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
 drivers/gpu/drm/i915/i915_request.c           |  4 ++--
 drivers/gpu/drm/i915/i915_sw_fence.c          | 12 +++++------
 drivers/gpu/drm/i915/i915_sw_fence.h          | 21 +++++++++----------
 drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
 .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
 drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  4 ++--
 8 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a7ca38613f89..6d5bb55ffc82 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10323,7 +10323,7 @@ static void intel_atomic_commit_work(struct work_struct *work)
 	intel_atomic_commit_tail(state);
 }
 
-static int __i915_sw_fence_call
+static int
 intel_atomic_commit_ready(struct i915_sw_fence *fence,
 			  enum i915_sw_fence_notify notify)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c2ab0e22db0a..df5fec5c3da8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -800,7 +800,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
 	free_engines(engines);
 }
 
-static int __i915_sw_fence_call
+static int
 engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_gem_engines *engines =
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index ce446716d092..945d3025a0b6 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, int error)
 	intel_context_cancel_request(rq->context, rq);
 }
 
-static int __i915_sw_fence_call
+static int
 submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *request =
@@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 	return NOTIFY_DONE;
 }
 
-static int __i915_sw_fence_call
+static int
 semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *rq = container_of(fence, typeof(*rq), semaphore);
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..1c080dd1f718 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -34,7 +34,7 @@ enum {
 
 static void *i915_sw_fence_debug_hint(void *addr)
 {
-	return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK);
+	return (void *)(((struct i915_sw_fence *)addr)->fn);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -126,10 +126,7 @@ static inline void debug_fence_assert(struct i915_sw_fence *fence)
 static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
 				  enum i915_sw_fence_notify state)
 {
-	i915_sw_fence_notify_t fn;
-
-	fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
-	return fn(fence, state);
+	return fence->fn(fence, state);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -242,10 +239,11 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  const char *name,
 			  struct lock_class_key *key)
 {
-	BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
+	BUG_ON(!fn);
 
 	__init_waitqueue_head(&fence->wait, name, key);
-	fence->flags = (unsigned long)fn;
+	fence->fn = fn;
+	fence->flags = 0;
 
 	i915_sw_fence_reinit(fence);
 }
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h
index 30a863353ee6..70ba1789aa89 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.h
+++ b/drivers/gpu/drm/i915/i915_sw_fence.h
@@ -17,26 +17,25 @@
 
 struct completion;
 struct dma_resv;
+struct i915_sw_fence;
+
+enum i915_sw_fence_notify {
+	FENCE_COMPLETE,
+	FENCE_FREE
+};
+
+typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
+				      enum i915_sw_fence_notify state);
 
 struct i915_sw_fence {
 	wait_queue_head_t wait;
+	i915_sw_fence_notify_t fn;
 	unsigned long flags;
 	atomic_t pending;
 	int error;
 };
 
 #define I915_SW_FENCE_CHECKED_BIT	0 /* used internally for DAG checking */
-#define I915_SW_FENCE_PRIVATE_BIT	1 /* available for use by owner */
-#define I915_SW_FENCE_MASK		(~3)
-
-enum i915_sw_fence_notify {
-	FENCE_COMPLETE,
-	FENCE_FREE
-};
-
-typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
-				      enum i915_sw_fence_notify state);
-#define __i915_sw_fence_call __aligned(4)
 
 void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  i915_sw_fence_notify_t fn,
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
index 5b33ef23d54c..d2e56b387993 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
@@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
 	dma_fence_put(&f->dma);
 }
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
index cbf45d85cbff..daa985e5a19b 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -28,7 +28,7 @@
 
 #include "../i915_selftest.h"
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	switch (state) {
diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
index 080b90b63d16..eb59a41bdb79 100644
--- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
@@ -26,7 +26,7 @@
 
 /* Small library of different fence types useful for writing tests */
 
-static int __i915_sw_fence_call
+static int
 nop_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	return NOTIFY_DONE;
@@ -89,7 +89,7 @@ struct heap_fence {
 	};
 };
 
-static int __i915_sw_fence_call
+static int
 heap_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct heap_fence *h = container_of(fence, typeof(*h), fence);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
  2021-09-22 14:57 ` [Intel-gfx] " Matthew Brost
  (?)
@ 2021-09-22 15:21 ` Tvrtko Ursulin
  2021-09-22 15:25   ` Tvrtko Ursulin
  -1 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2021-09-22 15:21 UTC (permalink / raw)
  To: Matthew Brost, dri-devel, intel-gfx
  Cc: jani.nikula, tvrtko.ursulin, lucas.demarchi


On 22/09/2021 15:57, Matthew Brost wrote:
> Rather than stealing bits from i915_sw_fence function pointer use
> seperate fields for function pointer and flags. If using two different
> fields, the 4 byte alignment for the i915_sw_fence function pointer can
> also be dropped.
> 
> v2:
>   (CI)
>    - Set new function field rather than flags in __i915_sw_fence_init
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
>   drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
>   drivers/gpu/drm/i915/i915_request.c           |  4 ++--
>   drivers/gpu/drm/i915/i915_sw_fence.c          | 12 +++++------
>   drivers/gpu/drm/i915/i915_sw_fence.h          | 21 +++++++++----------
>   drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
>   .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
>   drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  4 ++--
>   8 files changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a7ca38613f89..6d5bb55ffc82 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10323,7 +10323,7 @@ static void intel_atomic_commit_work(struct work_struct *work)
>   	intel_atomic_commit_tail(state);
>   }
>   
> -static int __i915_sw_fence_call
> +static int
>   intel_atomic_commit_ready(struct i915_sw_fence *fence,
>   			  enum i915_sw_fence_notify notify)
>   {
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index c2ab0e22db0a..df5fec5c3da8 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -800,7 +800,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
>   	free_engines(engines);
>   }
>   
> -static int __i915_sw_fence_call
> +static int
>   engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>   {
>   	struct i915_gem_engines *engines =
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index ce446716d092..945d3025a0b6 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, int error)
>   	intel_context_cancel_request(rq->context, rq);
>   }
>   
> -static int __i915_sw_fence_call
> +static int
>   submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>   {
>   	struct i915_request *request =
> @@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>   	return NOTIFY_DONE;
>   }
>   
> -static int __i915_sw_fence_call
> +static int
>   semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>   {
>   	struct i915_request *rq = container_of(fence, typeof(*rq), semaphore);
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> index c589a681da77..1c080dd1f718 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> @@ -34,7 +34,7 @@ enum {
>   
>   static void *i915_sw_fence_debug_hint(void *addr)
>   {
> -	return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK);
> +	return (void *)(((struct i915_sw_fence *)addr)->fn);
>   }
>   
>   #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
> @@ -126,10 +126,7 @@ static inline void debug_fence_assert(struct i915_sw_fence *fence)
>   static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
>   				  enum i915_sw_fence_notify state)
>   {
> -	i915_sw_fence_notify_t fn;
> -
> -	fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
> -	return fn(fence, state);
> +	return fence->fn(fence, state);
>   }
>   
>   #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
> @@ -242,10 +239,11 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
>   			  const char *name,
>   			  struct lock_class_key *key)
>   {
> -	BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
> +	BUG_ON(!fn);
>   
>   	__init_waitqueue_head(&fence->wait, name, key);
> -	fence->flags = (unsigned long)fn;
> +	fence->fn = fn;
> +	fence->flags = 0;
>   
>   	i915_sw_fence_reinit(fence);
>   }
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h
> index 30a863353ee6..70ba1789aa89 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.h
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.h
> @@ -17,26 +17,25 @@
>   
>   struct completion;
>   struct dma_resv;
> +struct i915_sw_fence;
> +
> +enum i915_sw_fence_notify {
> +	FENCE_COMPLETE,
> +	FENCE_FREE
> +};
> +
> +typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
> +				      enum i915_sw_fence_notify state);
>   
>   struct i915_sw_fence {
>   	wait_queue_head_t wait;
> +	i915_sw_fence_notify_t fn;
>   	unsigned long flags;

Looks good to me. I'd just make the flags narrower now that they can be, 
and put them down..

>   	atomic_t pending;

.. here as unsigned int and so we save 4 bytes, maybe.

In fact, unless CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is on, which it won't 
be on release builds, I don't think anything uses flags any more. So you 
could even omit the flags in that case. Might be cumbersome so can leave 
for later, but would sure be nice not to waste space if we can avoid it.

Also please double check if i915_sw_fence_reinit() will be fine and 
won't bug on since fence->flags is always zero now I think.

Regards,

Tvrtko

>   	int error;
>   };
>   
>   #define I915_SW_FENCE_CHECKED_BIT	0 /* used internally for DAG checking */
> -#define I915_SW_FENCE_PRIVATE_BIT	1 /* available for use by owner */
> -#define I915_SW_FENCE_MASK		(~3)
> -
> -enum i915_sw_fence_notify {
> -	FENCE_COMPLETE,
> -	FENCE_FREE
> -};
> -
> -typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
> -				      enum i915_sw_fence_notify state);
> -#define __i915_sw_fence_call __aligned(4)
>   
>   void __i915_sw_fence_init(struct i915_sw_fence *fence,
>   			  i915_sw_fence_notify_t fn,
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
> index 5b33ef23d54c..d2e56b387993 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
> @@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
>   	dma_fence_put(&f->dma);
>   }
>   
> -static int __i915_sw_fence_call
> +static int
>   fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>   {
>   	struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
> diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> index cbf45d85cbff..daa985e5a19b 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> @@ -28,7 +28,7 @@
>   
>   #include "../i915_selftest.h"
>   
> -static int __i915_sw_fence_call
> +static int
>   fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>   {
>   	switch (state) {
> diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> index 080b90b63d16..eb59a41bdb79 100644
> --- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> +++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> @@ -26,7 +26,7 @@
>   
>   /* Small library of different fence types useful for writing tests */
>   
> -static int __i915_sw_fence_call
> +static int
>   nop_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>   {
>   	return NOTIFY_DONE;
> @@ -89,7 +89,7 @@ struct heap_fence {
>   	};
>   };
>   
> -static int __i915_sw_fence_call
> +static int
>   heap_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>   {
>   	struct heap_fence *h = container_of(fence, typeof(*h), fence);
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
  2021-09-22 15:21 ` Tvrtko Ursulin
@ 2021-09-22 15:25   ` Tvrtko Ursulin
  2021-09-22 15:40     ` Matthew Brost
  0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2021-09-22 15:25 UTC (permalink / raw)
  To: Matthew Brost, dri-devel, intel-gfx
  Cc: jani.nikula, tvrtko.ursulin, lucas.demarchi


On 22/09/2021 16:21, Tvrtko Ursulin wrote:
> 
> On 22/09/2021 15:57, Matthew Brost wrote:
>> Rather than stealing bits from i915_sw_fence function pointer use
>> seperate fields for function pointer and flags. If using two different
>> fields, the 4 byte alignment for the i915_sw_fence function pointer can
>> also be dropped.
>>
>> v2:
>>   (CI)
>>    - Set new function field rather than flags in __i915_sw_fence_init
>>
>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
>>   drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
>>   drivers/gpu/drm/i915/i915_request.c           |  4 ++--
>>   drivers/gpu/drm/i915/i915_sw_fence.c          | 12 +++++------
>>   drivers/gpu/drm/i915/i915_sw_fence.h          | 21 +++++++++----------
>>   drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
>>   .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
>>   drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  4 ++--
>>   8 files changed, 23 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
>> b/drivers/gpu/drm/i915/display/intel_display.c
>> index a7ca38613f89..6d5bb55ffc82 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -10323,7 +10323,7 @@ static void intel_atomic_commit_work(struct 
>> work_struct *work)
>>       intel_atomic_commit_tail(state);
>>   }
>> -static int __i915_sw_fence_call
>> +static int
>>   intel_atomic_commit_ready(struct i915_sw_fence *fence,
>>                 enum i915_sw_fence_notify notify)
>>   {
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
>> b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> index c2ab0e22db0a..df5fec5c3da8 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> @@ -800,7 +800,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
>>       free_engines(engines);
>>   }
>> -static int __i915_sw_fence_call
>> +static int
>>   engines_notify(struct i915_sw_fence *fence, enum 
>> i915_sw_fence_notify state)
>>   {
>>       struct i915_gem_engines *engines =
>> diff --git a/drivers/gpu/drm/i915/i915_request.c 
>> b/drivers/gpu/drm/i915/i915_request.c
>> index ce446716d092..945d3025a0b6 100644
>> --- a/drivers/gpu/drm/i915/i915_request.c
>> +++ b/drivers/gpu/drm/i915/i915_request.c
>> @@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, 
>> int error)
>>       intel_context_cancel_request(rq->context, rq);
>>   }
>> -static int __i915_sw_fence_call
>> +static int
>>   submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify 
>> state)
>>   {
>>       struct i915_request *request =
>> @@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum 
>> i915_sw_fence_notify state)
>>       return NOTIFY_DONE;
>>   }
>> -static int __i915_sw_fence_call
>> +static int
>>   semaphore_notify(struct i915_sw_fence *fence, enum 
>> i915_sw_fence_notify state)
>>   {
>>       struct i915_request *rq = container_of(fence, typeof(*rq), 
>> semaphore);
>> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c 
>> b/drivers/gpu/drm/i915/i915_sw_fence.c
>> index c589a681da77..1c080dd1f718 100644
>> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
>> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
>> @@ -34,7 +34,7 @@ enum {
>>   static void *i915_sw_fence_debug_hint(void *addr)
>>   {
>> -    return (void *)(((struct i915_sw_fence *)addr)->flags & 
>> I915_SW_FENCE_MASK);
>> +    return (void *)(((struct i915_sw_fence *)addr)->fn);
>>   }
>>   #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
>> @@ -126,10 +126,7 @@ static inline void debug_fence_assert(struct 
>> i915_sw_fence *fence)
>>   static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
>>                     enum i915_sw_fence_notify state)
>>   {
>> -    i915_sw_fence_notify_t fn;
>> -
>> -    fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
>> -    return fn(fence, state);
>> +    return fence->fn(fence, state);
>>   }
>>   #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
>> @@ -242,10 +239,11 @@ void __i915_sw_fence_init(struct i915_sw_fence 
>> *fence,
>>                 const char *name,
>>                 struct lock_class_key *key)
>>   {
>> -    BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
>> +    BUG_ON(!fn);
>>       __init_waitqueue_head(&fence->wait, name, key);
>> -    fence->flags = (unsigned long)fn;
>> +    fence->fn = fn;
>> +    fence->flags = 0;
>>       i915_sw_fence_reinit(fence);
>>   }
>> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h 
>> b/drivers/gpu/drm/i915/i915_sw_fence.h
>> index 30a863353ee6..70ba1789aa89 100644
>> --- a/drivers/gpu/drm/i915/i915_sw_fence.h
>> +++ b/drivers/gpu/drm/i915/i915_sw_fence.h
>> @@ -17,26 +17,25 @@
>>   struct completion;
>>   struct dma_resv;
>> +struct i915_sw_fence;
>> +
>> +enum i915_sw_fence_notify {
>> +    FENCE_COMPLETE,
>> +    FENCE_FREE
>> +};
>> +
>> +typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
>> +                      enum i915_sw_fence_notify state);
>>   struct i915_sw_fence {
>>       wait_queue_head_t wait;
>> +    i915_sw_fence_notify_t fn;
>>       unsigned long flags;
> 
> Looks good to me. I'd just make the flags narrower now that they can be, 
> and put them down..
> 
>>       atomic_t pending;
> 
> .. here as unsigned int and so we save 4 bytes, maybe.

No this won't work due test_and_set_bit needs a long, oh well.

> 
> In fact, unless CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is on, which it won't 
> be on release builds, I don't think anything uses flags any more. So you 
> could even omit the flags in that case. Might be cumbersome so can leave 
> for later, but would sure be nice not to waste space if we can avoid it.
> 
> Also please double check if i915_sw_fence_reinit() will be fine and 
> won't bug on since fence->flags is always zero now I think.

But these two are worth checking out.

Regards,

Tvrtko

> 
> Regards,
> 
> Tvrtko
> 
>>       int error;
>>   };
>>   #define I915_SW_FENCE_CHECKED_BIT    0 /* used internally for DAG 
>> checking */
>> -#define I915_SW_FENCE_PRIVATE_BIT    1 /* available for use by owner */
>> -#define I915_SW_FENCE_MASK        (~3)
>> -
>> -enum i915_sw_fence_notify {
>> -    FENCE_COMPLETE,
>> -    FENCE_FREE
>> -};
>> -
>> -typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
>> -                      enum i915_sw_fence_notify state);
>> -#define __i915_sw_fence_call __aligned(4)
>>   void __i915_sw_fence_init(struct i915_sw_fence *fence,
>>                 i915_sw_fence_notify_t fn,
>> diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c 
>> b/drivers/gpu/drm/i915/i915_sw_fence_work.c
>> index 5b33ef23d54c..d2e56b387993 100644
>> --- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
>> +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
>> @@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
>>       dma_fence_put(&f->dma);
>>   }
>> -static int __i915_sw_fence_call
>> +static int
>>   fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify 
>> state)
>>   {
>>       struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
>> diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c 
>> b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
>> index cbf45d85cbff..daa985e5a19b 100644
>> --- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
>> +++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
>> @@ -28,7 +28,7 @@
>>   #include "../i915_selftest.h"
>> -static int __i915_sw_fence_call
>> +static int
>>   fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify 
>> state)
>>   {
>>       switch (state) {
>> diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c 
>> b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
>> index 080b90b63d16..eb59a41bdb79 100644
>> --- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
>> +++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
>> @@ -26,7 +26,7 @@
>>   /* Small library of different fence types useful for writing tests */
>> -static int __i915_sw_fence_call
>> +static int
>>   nop_fence_notify(struct i915_sw_fence *fence, enum 
>> i915_sw_fence_notify state)
>>   {
>>       return NOTIFY_DONE;
>> @@ -89,7 +89,7 @@ struct heap_fence {
>>       };
>>   };
>> -static int __i915_sw_fence_call
>> +static int
>>   heap_fence_notify(struct i915_sw_fence *fence, enum 
>> i915_sw_fence_notify state)
>>   {
>>       struct heap_fence *h = container_of(fence, typeof(*h), fence);
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
  2021-09-22 15:25   ` Tvrtko Ursulin
@ 2021-09-22 15:40     ` Matthew Brost
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2021-09-22 15:40 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: dri-devel, intel-gfx, jani.nikula, tvrtko.ursulin, lucas.demarchi

On Wed, Sep 22, 2021 at 04:25:04PM +0100, Tvrtko Ursulin wrote:
> 
> On 22/09/2021 16:21, Tvrtko Ursulin wrote:
> > 
> > On 22/09/2021 15:57, Matthew Brost wrote:
> > > Rather than stealing bits from i915_sw_fence function pointer use
> > > seperate fields for function pointer and flags. If using two different
> > > fields, the 4 byte alignment for the i915_sw_fence function pointer can
> > > also be dropped.
> > > 
> > > v2:
> > >   (CI)
> > >    - Set new function field rather than flags in __i915_sw_fence_init
> > > 
> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
> > >   drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
> > >   drivers/gpu/drm/i915/i915_request.c           |  4 ++--
> > >   drivers/gpu/drm/i915/i915_sw_fence.c          | 12 +++++------
> > >   drivers/gpu/drm/i915/i915_sw_fence.h          | 21 +++++++++----------
> > >   drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
> > >   .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
> > >   drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  4 ++--
> > >   8 files changed, 23 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index a7ca38613f89..6d5bb55ffc82 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -10323,7 +10323,7 @@ static void intel_atomic_commit_work(struct
> > > work_struct *work)
> > >       intel_atomic_commit_tail(state);
> > >   }
> > > -static int __i915_sw_fence_call
> > > +static int
> > >   intel_atomic_commit_ready(struct i915_sw_fence *fence,
> > >                 enum i915_sw_fence_notify notify)
> > >   {
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > index c2ab0e22db0a..df5fec5c3da8 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > @@ -800,7 +800,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
> > >       free_engines(engines);
> > >   }
> > > -static int __i915_sw_fence_call
> > > +static int
> > >   engines_notify(struct i915_sw_fence *fence, enum
> > > i915_sw_fence_notify state)
> > >   {
> > >       struct i915_gem_engines *engines =
> > > diff --git a/drivers/gpu/drm/i915/i915_request.c
> > > b/drivers/gpu/drm/i915/i915_request.c
> > > index ce446716d092..945d3025a0b6 100644
> > > --- a/drivers/gpu/drm/i915/i915_request.c
> > > +++ b/drivers/gpu/drm/i915/i915_request.c
> > > @@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request
> > > *rq, int error)
> > >       intel_context_cancel_request(rq->context, rq);
> > >   }
> > > -static int __i915_sw_fence_call
> > > +static int
> > >   submit_notify(struct i915_sw_fence *fence, enum
> > > i915_sw_fence_notify state)
> > >   {
> > >       struct i915_request *request =
> > > @@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum
> > > i915_sw_fence_notify state)
> > >       return NOTIFY_DONE;
> > >   }
> > > -static int __i915_sw_fence_call
> > > +static int
> > >   semaphore_notify(struct i915_sw_fence *fence, enum
> > > i915_sw_fence_notify state)
> > >   {
> > >       struct i915_request *rq = container_of(fence, typeof(*rq),
> > > semaphore);
> > > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
> > > b/drivers/gpu/drm/i915/i915_sw_fence.c
> > > index c589a681da77..1c080dd1f718 100644
> > > --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> > > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> > > @@ -34,7 +34,7 @@ enum {
> > >   static void *i915_sw_fence_debug_hint(void *addr)
> > >   {
> > > -    return (void *)(((struct i915_sw_fence *)addr)->flags &
> > > I915_SW_FENCE_MASK);
> > > +    return (void *)(((struct i915_sw_fence *)addr)->fn);
> > >   }
> > >   #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
> > > @@ -126,10 +126,7 @@ static inline void debug_fence_assert(struct
> > > i915_sw_fence *fence)
> > >   static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
> > >                     enum i915_sw_fence_notify state)
> > >   {
> > > -    i915_sw_fence_notify_t fn;
> > > -
> > > -    fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
> > > -    return fn(fence, state);
> > > +    return fence->fn(fence, state);
> > >   }
> > >   #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
> > > @@ -242,10 +239,11 @@ void __i915_sw_fence_init(struct i915_sw_fence
> > > *fence,
> > >                 const char *name,
> > >                 struct lock_class_key *key)
> > >   {
> > > -    BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
> > > +    BUG_ON(!fn);
> > >       __init_waitqueue_head(&fence->wait, name, key);
> > > -    fence->flags = (unsigned long)fn;
> > > +    fence->fn = fn;
> > > +    fence->flags = 0;
> > >       i915_sw_fence_reinit(fence);
> > >   }
> > > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h
> > > b/drivers/gpu/drm/i915/i915_sw_fence.h
> > > index 30a863353ee6..70ba1789aa89 100644
> > > --- a/drivers/gpu/drm/i915/i915_sw_fence.h
> > > +++ b/drivers/gpu/drm/i915/i915_sw_fence.h
> > > @@ -17,26 +17,25 @@
> > >   struct completion;
> > >   struct dma_resv;
> > > +struct i915_sw_fence;
> > > +
> > > +enum i915_sw_fence_notify {
> > > +    FENCE_COMPLETE,
> > > +    FENCE_FREE
> > > +};
> > > +
> > > +typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
> > > +                      enum i915_sw_fence_notify state);
> > >   struct i915_sw_fence {
> > >       wait_queue_head_t wait;
> > > +    i915_sw_fence_notify_t fn;
> > >       unsigned long flags;
> > 
> > Looks good to me. I'd just make the flags narrower now that they can be,
> > and put them down..
> > 
> > >       atomic_t pending;
> > 
> > .. here as unsigned int and so we save 4 bytes, maybe.
> 
> No this won't work due test_and_set_bit needs a long, oh well.
> 
> > 
> > In fact, unless CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is on, which it won't
> > be on release builds, I don't think anything uses flags any more. So you
> > could even omit the flags in that case. Might be cumbersome so can leave
> > for later, but would sure be nice not to waste space if we can avoid it.
> > 
> > Also please double check if i915_sw_fence_reinit() will be fine and
> > won't bug on since fence->flags is always zero now I think.
> 

Yes, the BUG_ON(!fence->flags) is going to blow up, good catch. Will
skip this revisions testing and repost with this fixed. 

If we add a few 'ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG' we indeed can
only include fence->flags if CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is
defined. Will include in next rev.

Matt

> But these two are worth checking out.
> 
> Regards,
> 
> Tvrtko
> 
> > 
> > Regards,
> > 
> > Tvrtko
> > 
> > >       int error;
> > >   };
> > >   #define I915_SW_FENCE_CHECKED_BIT    0 /* used internally for DAG
> > > checking */
> > > -#define I915_SW_FENCE_PRIVATE_BIT    1 /* available for use by owner */
> > > -#define I915_SW_FENCE_MASK        (~3)
> > > -
> > > -enum i915_sw_fence_notify {
> > > -    FENCE_COMPLETE,
> > > -    FENCE_FREE
> > > -};
> > > -
> > > -typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
> > > -                      enum i915_sw_fence_notify state);
> > > -#define __i915_sw_fence_call __aligned(4)
> > >   void __i915_sw_fence_init(struct i915_sw_fence *fence,
> > >                 i915_sw_fence_notify_t fn,
> > > diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c
> > > b/drivers/gpu/drm/i915/i915_sw_fence_work.c
> > > index 5b33ef23d54c..d2e56b387993 100644
> > > --- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
> > > +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
> > > @@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
> > >       dma_fence_put(&f->dma);
> > >   }
> > > -static int __i915_sw_fence_call
> > > +static int
> > >   fence_notify(struct i915_sw_fence *fence, enum
> > > i915_sw_fence_notify state)
> > >   {
> > >       struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
> > > diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> > > b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> > > index cbf45d85cbff..daa985e5a19b 100644
> > > --- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> > > +++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> > > @@ -28,7 +28,7 @@
> > >   #include "../i915_selftest.h"
> > > -static int __i915_sw_fence_call
> > > +static int
> > >   fence_notify(struct i915_sw_fence *fence, enum
> > > i915_sw_fence_notify state)
> > >   {
> > >       switch (state) {
> > > diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> > > b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> > > index 080b90b63d16..eb59a41bdb79 100644
> > > --- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> > > +++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> > > @@ -26,7 +26,7 @@
> > >   /* Small library of different fence types useful for writing tests */
> > > -static int __i915_sw_fence_call
> > > +static int
> > >   nop_fence_notify(struct i915_sw_fence *fence, enum
> > > i915_sw_fence_notify state)
> > >   {
> > >       return NOTIFY_DONE;
> > > @@ -89,7 +89,7 @@ struct heap_fence {
> > >       };
> > >   };
> > > -static int __i915_sw_fence_call
> > > +static int
> > >   heap_fence_notify(struct i915_sw_fence *fence, enum
> > > i915_sw_fence_notify state)
> > >   {
> > >       struct heap_fence *h = container_of(fence, typeof(*h), fence);
> > > 

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
@ 2021-11-16 19:49 Matthew Brost
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2021-11-16 19:49 UTC (permalink / raw)
  To: intel-gfx, dri-devel

Rather than stealing bits from i915_sw_fence function pointer use
seperate fields for function pointer and flags. If using two different
fields, the 4 byte alignment for the i915_sw_fence function pointer can
also be dropped.

v2:
 (CI)
  - Set new function field rather than flags in __i915_sw_fence_init
v3:
 (Tvrtko)
  - Remove BUG_ON(!fence->flags) in reinit as that will now blow up
  - Only define fence->flags if CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is
    defined
v4:
  - Rebase, resend for CI

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
 drivers/gpu/drm/i915/gt/intel_context.c       |  2 +-
 drivers/gpu/drm/i915/i915_request.c           |  4 +--
 drivers/gpu/drm/i915/i915_sw_fence.c          | 28 +++++++++++--------
 drivers/gpu/drm/i915/i915_sw_fence.h          | 23 +++++++--------
 drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
 .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
 drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  8 +++---
 9 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 0ceee8ac6671..6636ac9e9ff8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8791,7 +8791,7 @@ static void intel_atomic_commit_work(struct work_struct *work)
 	intel_atomic_commit_tail(state);
 }
 
-static int __i915_sw_fence_call
+static int
 intel_atomic_commit_ready(struct i915_sw_fence *fence,
 			  enum i915_sw_fence_notify notify)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index ebd775cb1661..347dab952e90 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1001,7 +1001,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
 	free_engines(engines);
 }
 
-static int __i915_sw_fence_call
+static int
 engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_gem_engines *engines =
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 5634d14052bc..331ed688ceab 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -364,7 +364,7 @@ static int __intel_context_active(struct i915_active *active)
 	return 0;
 }
 
-static int __i915_sw_fence_call
+static int
 sw_fence_dummy_notify(struct i915_sw_fence *sf,
 		      enum i915_sw_fence_notify state)
 {
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 623273aca09e..08dda8889f4a 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, int error)
 	intel_context_cancel_request(rq->context, rq);
 }
 
-static int __i915_sw_fence_call
+static int
 submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *request =
@@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 	return NOTIFY_DONE;
 }
 
-static int __i915_sw_fence_call
+static int
 semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *rq = container_of(fence, typeof(*rq), semaphore);
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..f10d31818ecc 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -18,7 +18,9 @@
 #define I915_SW_FENCE_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
 #endif
 
+#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG
 static DEFINE_SPINLOCK(i915_sw_fence_lock);
+#endif
 
 #define WQ_FLAG_BITS \
 	BITS_PER_TYPE(typeof_member(struct wait_queue_entry, flags))
@@ -34,7 +36,7 @@ enum {
 
 static void *i915_sw_fence_debug_hint(void *addr)
 {
-	return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK);
+	return (void *)(((struct i915_sw_fence *)addr)->fn);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -126,10 +128,7 @@ static inline void debug_fence_assert(struct i915_sw_fence *fence)
 static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
 				  enum i915_sw_fence_notify state)
 {
-	i915_sw_fence_notify_t fn;
-
-	fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
-	return fn(fence, state);
+	return fence->fn(fence, state);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -242,10 +241,13 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  const char *name,
 			  struct lock_class_key *key)
 {
-	BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
+	BUG_ON(!fn);
 
 	__init_waitqueue_head(&fence->wait, name, key);
-	fence->flags = (unsigned long)fn;
+	fence->fn = fn;
+#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG
+	fence->flags = 0;
+#endif
 
 	i915_sw_fence_reinit(fence);
 }
@@ -257,7 +259,6 @@ void i915_sw_fence_reinit(struct i915_sw_fence *fence)
 	atomic_set(&fence->pending, 1);
 	fence->error = 0;
 
-	I915_SW_FENCE_BUG_ON(!fence->flags);
 	I915_SW_FENCE_BUG_ON(!list_empty(&fence->wait.head));
 }
 
@@ -279,6 +280,7 @@ static int i915_sw_fence_wake(wait_queue_entry_t *wq, unsigned mode, int flags,
 	return 0;
 }
 
+#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG
 static bool __i915_sw_fence_check_if_after(struct i915_sw_fence *fence,
 				    const struct i915_sw_fence * const signaler)
 {
@@ -322,9 +324,6 @@ static bool i915_sw_fence_check_if_after(struct i915_sw_fence *fence,
 	unsigned long flags;
 	bool err;
 
-	if (!IS_ENABLED(CONFIG_DRM_I915_SW_FENCE_CHECK_DAG))
-		return false;
-
 	spin_lock_irqsave(&i915_sw_fence_lock, flags);
 	err = __i915_sw_fence_check_if_after(fence, signaler);
 	__i915_sw_fence_clear_checked_bit(fence);
@@ -332,6 +331,13 @@ static bool i915_sw_fence_check_if_after(struct i915_sw_fence *fence,
 
 	return err;
 }
+#else
+static bool i915_sw_fence_check_if_after(struct i915_sw_fence *fence,
+					 const struct i915_sw_fence * const signaler)
+{
+	return false;
+}
+#endif
 
 static int __i915_sw_fence_await_sw_fence(struct i915_sw_fence *fence,
 					  struct i915_sw_fence *signaler,
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h
index 30a863353ee6..a7c603bc1b01 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.h
+++ b/drivers/gpu/drm/i915/i915_sw_fence.h
@@ -17,26 +17,27 @@
 
 struct completion;
 struct dma_resv;
+struct i915_sw_fence;
+
+enum i915_sw_fence_notify {
+	FENCE_COMPLETE,
+	FENCE_FREE
+};
+
+typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
+				      enum i915_sw_fence_notify state);
 
 struct i915_sw_fence {
 	wait_queue_head_t wait;
+	i915_sw_fence_notify_t fn;
+#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG
 	unsigned long flags;
+#endif
 	atomic_t pending;
 	int error;
 };
 
 #define I915_SW_FENCE_CHECKED_BIT	0 /* used internally for DAG checking */
-#define I915_SW_FENCE_PRIVATE_BIT	1 /* available for use by owner */
-#define I915_SW_FENCE_MASK		(~3)
-
-enum i915_sw_fence_notify {
-	FENCE_COMPLETE,
-	FENCE_FREE
-};
-
-typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
-				      enum i915_sw_fence_notify state);
-#define __i915_sw_fence_call __aligned(4)
 
 void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  i915_sw_fence_notify_t fn,
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
index 5b33ef23d54c..d2e56b387993 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
@@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
 	dma_fence_put(&f->dma);
 }
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
index cbf45d85cbff..daa985e5a19b 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -28,7 +28,7 @@
 
 #include "../i915_selftest.h"
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	switch (state) {
diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
index 080b90b63d16..bf2752cc1e0b 100644
--- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
@@ -26,7 +26,7 @@
 
 /* Small library of different fence types useful for writing tests */
 
-static int __i915_sw_fence_call
+static int
 nop_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	return NOTIFY_DONE;
@@ -41,12 +41,12 @@ void __onstack_fence_init(struct i915_sw_fence *fence,
 	__init_waitqueue_head(&fence->wait, name, key);
 	atomic_set(&fence->pending, 1);
 	fence->error = 0;
-	fence->flags = (unsigned long)nop_fence_notify;
+	fence->fn = nop_fence_notify;
 }
 
 void onstack_fence_fini(struct i915_sw_fence *fence)
 {
-	if (!fence->flags)
+	if (!fence->fn)
 		return;
 
 	i915_sw_fence_commit(fence);
@@ -89,7 +89,7 @@ struct heap_fence {
 	};
 };
 
-static int __i915_sw_fence_call
+static int
 heap_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct heap_fence *h = container_of(fence, typeof(*h), fence);
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
  2021-09-22 15:47 Matthew Brost
@ 2021-11-11 20:18   ` Teres Alexis, Alan Previn
  0 siblings, 0 replies; 11+ messages in thread
From: Teres Alexis, Alan Previn @ 2021-11-11 20:18 UTC (permalink / raw)
  To: dri-devel, Brost, Matthew, intel-gfx; +Cc: De Marchi, Lucas, Ursulin, Tvrtko

Hey Matt, apologies for the delay, went thru all the code, LGTM.

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

P.S. - As a side note, would be interesting to replay the original reason behind the overloading of the
func ptr bits to begin with... to see what the initial intention was.


...alan

On Wed, 2021-09-22 at 08:47 -0700, Matthew Brost wrote:
> Rather than stealing bits from i915_sw_fence function pointer use
> seperate fields for function pointer and flags. If using two different
> fields, the 4 byte alignment for the i915_sw_fence function pointer can
> also be dropped.
> 
> v2:
>  (CI)
>   - Set new function field rather than flags in __i915_sw_fence_init
> v3:
>  (Tvrtko)
>   - Remove BUG_ON(!fence->flags) in reinit as that will now blow up
>   - Only define fence->flags if CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is
>     defined
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
>  drivers/gpu/drm/i915/i915_request.c           |  4 +--
>  drivers/gpu/drm/i915/i915_sw_fence.c          | 28 +++++++++++--------
>  drivers/gpu/drm/i915/i915_sw_fence.h          | 23 +++++++--------
>  drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
>  .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
>  drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  8 +++---
>  8 files changed, 39 insertions(+), 32 deletions(-)
> 
>  
> -- 
> 2.32.0
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
@ 2021-11-11 20:18   ` Teres Alexis, Alan Previn
  0 siblings, 0 replies; 11+ messages in thread
From: Teres Alexis, Alan Previn @ 2021-11-11 20:18 UTC (permalink / raw)
  To: dri-devel, Brost, Matthew, intel-gfx; +Cc: De Marchi, Lucas

Hey Matt, apologies for the delay, went thru all the code, LGTM.

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

P.S. - As a side note, would be interesting to replay the original reason behind the overloading of the
func ptr bits to begin with... to see what the initial intention was.


...alan

On Wed, 2021-09-22 at 08:47 -0700, Matthew Brost wrote:
> Rather than stealing bits from i915_sw_fence function pointer use
> seperate fields for function pointer and flags. If using two different
> fields, the 4 byte alignment for the i915_sw_fence function pointer can
> also be dropped.
> 
> v2:
>  (CI)
>   - Set new function field rather than flags in __i915_sw_fence_init
> v3:
>  (Tvrtko)
>   - Remove BUG_ON(!fence->flags) in reinit as that will now blow up
>   - Only define fence->flags if CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is
>     defined
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
>  drivers/gpu/drm/i915/i915_request.c           |  4 +--
>  drivers/gpu/drm/i915/i915_sw_fence.c          | 28 +++++++++++--------
>  drivers/gpu/drm/i915/i915_sw_fence.h          | 23 +++++++--------
>  drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
>  .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
>  drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  8 +++---
>  8 files changed, 39 insertions(+), 32 deletions(-)
> 
>  
> -- 
> 2.32.0
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
@ 2021-09-22 15:47 Matthew Brost
  2021-11-11 20:18   ` Teres Alexis, Alan Previn
  0 siblings, 1 reply; 11+ messages in thread
From: Matthew Brost @ 2021-09-22 15:47 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: jani.nikula, tvrtko.ursulin, lucas.demarchi

Rather than stealing bits from i915_sw_fence function pointer use
seperate fields for function pointer and flags. If using two different
fields, the 4 byte alignment for the i915_sw_fence function pointer can
also be dropped.

v2:
 (CI)
  - Set new function field rather than flags in __i915_sw_fence_init
v3:
 (Tvrtko)
  - Remove BUG_ON(!fence->flags) in reinit as that will now blow up
  - Only define fence->flags if CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is
    defined

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
 drivers/gpu/drm/i915/i915_request.c           |  4 +--
 drivers/gpu/drm/i915/i915_sw_fence.c          | 28 +++++++++++--------
 drivers/gpu/drm/i915/i915_sw_fence.h          | 23 +++++++--------
 drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
 .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
 drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  8 +++---
 8 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a7ca38613f89..6d5bb55ffc82 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10323,7 +10323,7 @@ static void intel_atomic_commit_work(struct work_struct *work)
 	intel_atomic_commit_tail(state);
 }
 
-static int __i915_sw_fence_call
+static int
 intel_atomic_commit_ready(struct i915_sw_fence *fence,
 			  enum i915_sw_fence_notify notify)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c2ab0e22db0a..df5fec5c3da8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -800,7 +800,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
 	free_engines(engines);
 }
 
-static int __i915_sw_fence_call
+static int
 engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_gem_engines *engines =
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index ce446716d092..945d3025a0b6 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, int error)
 	intel_context_cancel_request(rq->context, rq);
 }
 
-static int __i915_sw_fence_call
+static int
 submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *request =
@@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 	return NOTIFY_DONE;
 }
 
-static int __i915_sw_fence_call
+static int
 semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *rq = container_of(fence, typeof(*rq), semaphore);
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..f10d31818ecc 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -18,7 +18,9 @@
 #define I915_SW_FENCE_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
 #endif
 
+#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG
 static DEFINE_SPINLOCK(i915_sw_fence_lock);
+#endif
 
 #define WQ_FLAG_BITS \
 	BITS_PER_TYPE(typeof_member(struct wait_queue_entry, flags))
@@ -34,7 +36,7 @@ enum {
 
 static void *i915_sw_fence_debug_hint(void *addr)
 {
-	return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK);
+	return (void *)(((struct i915_sw_fence *)addr)->fn);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -126,10 +128,7 @@ static inline void debug_fence_assert(struct i915_sw_fence *fence)
 static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
 				  enum i915_sw_fence_notify state)
 {
-	i915_sw_fence_notify_t fn;
-
-	fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
-	return fn(fence, state);
+	return fence->fn(fence, state);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -242,10 +241,13 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  const char *name,
 			  struct lock_class_key *key)
 {
-	BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
+	BUG_ON(!fn);
 
 	__init_waitqueue_head(&fence->wait, name, key);
-	fence->flags = (unsigned long)fn;
+	fence->fn = fn;
+#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG
+	fence->flags = 0;
+#endif
 
 	i915_sw_fence_reinit(fence);
 }
@@ -257,7 +259,6 @@ void i915_sw_fence_reinit(struct i915_sw_fence *fence)
 	atomic_set(&fence->pending, 1);
 	fence->error = 0;
 
-	I915_SW_FENCE_BUG_ON(!fence->flags);
 	I915_SW_FENCE_BUG_ON(!list_empty(&fence->wait.head));
 }
 
@@ -279,6 +280,7 @@ static int i915_sw_fence_wake(wait_queue_entry_t *wq, unsigned mode, int flags,
 	return 0;
 }
 
+#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG
 static bool __i915_sw_fence_check_if_after(struct i915_sw_fence *fence,
 				    const struct i915_sw_fence * const signaler)
 {
@@ -322,9 +324,6 @@ static bool i915_sw_fence_check_if_after(struct i915_sw_fence *fence,
 	unsigned long flags;
 	bool err;
 
-	if (!IS_ENABLED(CONFIG_DRM_I915_SW_FENCE_CHECK_DAG))
-		return false;
-
 	spin_lock_irqsave(&i915_sw_fence_lock, flags);
 	err = __i915_sw_fence_check_if_after(fence, signaler);
 	__i915_sw_fence_clear_checked_bit(fence);
@@ -332,6 +331,13 @@ static bool i915_sw_fence_check_if_after(struct i915_sw_fence *fence,
 
 	return err;
 }
+#else
+static bool i915_sw_fence_check_if_after(struct i915_sw_fence *fence,
+					 const struct i915_sw_fence * const signaler)
+{
+	return false;
+}
+#endif
 
 static int __i915_sw_fence_await_sw_fence(struct i915_sw_fence *fence,
 					  struct i915_sw_fence *signaler,
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h
index 30a863353ee6..a7c603bc1b01 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.h
+++ b/drivers/gpu/drm/i915/i915_sw_fence.h
@@ -17,26 +17,27 @@
 
 struct completion;
 struct dma_resv;
+struct i915_sw_fence;
+
+enum i915_sw_fence_notify {
+	FENCE_COMPLETE,
+	FENCE_FREE
+};
+
+typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
+				      enum i915_sw_fence_notify state);
 
 struct i915_sw_fence {
 	wait_queue_head_t wait;
+	i915_sw_fence_notify_t fn;
+#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG
 	unsigned long flags;
+#endif
 	atomic_t pending;
 	int error;
 };
 
 #define I915_SW_FENCE_CHECKED_BIT	0 /* used internally for DAG checking */
-#define I915_SW_FENCE_PRIVATE_BIT	1 /* available for use by owner */
-#define I915_SW_FENCE_MASK		(~3)
-
-enum i915_sw_fence_notify {
-	FENCE_COMPLETE,
-	FENCE_FREE
-};
-
-typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
-				      enum i915_sw_fence_notify state);
-#define __i915_sw_fence_call __aligned(4)
 
 void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  i915_sw_fence_notify_t fn,
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
index 5b33ef23d54c..d2e56b387993 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
@@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
 	dma_fence_put(&f->dma);
 }
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
index cbf45d85cbff..daa985e5a19b 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -28,7 +28,7 @@
 
 #include "../i915_selftest.h"
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	switch (state) {
diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
index 080b90b63d16..bf2752cc1e0b 100644
--- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
@@ -26,7 +26,7 @@
 
 /* Small library of different fence types useful for writing tests */
 
-static int __i915_sw_fence_call
+static int
 nop_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	return NOTIFY_DONE;
@@ -41,12 +41,12 @@ void __onstack_fence_init(struct i915_sw_fence *fence,
 	__init_waitqueue_head(&fence->wait, name, key);
 	atomic_set(&fence->pending, 1);
 	fence->error = 0;
-	fence->flags = (unsigned long)nop_fence_notify;
+	fence->fn = nop_fence_notify;
 }
 
 void onstack_fence_fini(struct i915_sw_fence *fence)
 {
-	if (!fence->flags)
+	if (!fence->fn)
 		return;
 
 	i915_sw_fence_commit(fence);
@@ -89,7 +89,7 @@ struct heap_fence {
 	};
 };
 
-static int __i915_sw_fence_call
+static int
 heap_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct heap_fence *h = container_of(fence, typeof(*h), fence);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
  2021-09-22  2:13 Matthew Brost
@ 2021-09-22 11:12 ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2021-09-22 11:12 UTC (permalink / raw)
  To: Matthew Brost, intel-gfx, dri-devel; +Cc: lucas.demarchi, tvrtko.ursulin

On Tue, 21 Sep 2021, Matthew Brost <matthew.brost@intel.com> wrote:
> Rather than stealing bits from i915_sw_fence function pointer use
> seperate fields for function pointer and flags. If using two different
> fields, the 4 byte alignment for the i915_sw_fence function pointer can
> also be dropped.

Yes, please, thank you.

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


BR,
Jani.

>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
>  drivers/gpu/drm/i915/i915_request.c           |  4 ++--
>  drivers/gpu/drm/i915/i915_sw_fence.c          |  9 +++-----
>  drivers/gpu/drm/i915/i915_sw_fence.h          | 21 +++++++++----------
>  drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
>  .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
>  drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  4 ++--
>  8 files changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a7ca38613f89..6d5bb55ffc82 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10323,7 +10323,7 @@ static void intel_atomic_commit_work(struct work_struct *work)
>  	intel_atomic_commit_tail(state);
>  }
>  
> -static int __i915_sw_fence_call
> +static int
>  intel_atomic_commit_ready(struct i915_sw_fence *fence,
>  			  enum i915_sw_fence_notify notify)
>  {
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index c2ab0e22db0a..df5fec5c3da8 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -800,7 +800,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
>  	free_engines(engines);
>  }
>  
> -static int __i915_sw_fence_call
> +static int
>  engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  {
>  	struct i915_gem_engines *engines =
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index ce446716d092..945d3025a0b6 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, int error)
>  	intel_context_cancel_request(rq->context, rq);
>  }
>  
> -static int __i915_sw_fence_call
> +static int
>  submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  {
>  	struct i915_request *request =
> @@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  	return NOTIFY_DONE;
>  }
>  
> -static int __i915_sw_fence_call
> +static int
>  semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  {
>  	struct i915_request *rq = container_of(fence, typeof(*rq), semaphore);
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> index c589a681da77..5cf101cf06d2 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> @@ -34,7 +34,7 @@ enum {
>  
>  static void *i915_sw_fence_debug_hint(void *addr)
>  {
> -	return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK);
> +	return (void *)(((struct i915_sw_fence *)addr)->fn);
>  }
>  
>  #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
> @@ -126,10 +126,7 @@ static inline void debug_fence_assert(struct i915_sw_fence *fence)
>  static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
>  				  enum i915_sw_fence_notify state)
>  {
> -	i915_sw_fence_notify_t fn;
> -
> -	fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
> -	return fn(fence, state);
> +	return fence->fn(fence, state);
>  }
>  
>  #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
> @@ -242,7 +239,7 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
>  			  const char *name,
>  			  struct lock_class_key *key)
>  {
> -	BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
> +	BUG_ON(!fn);
>  
>  	__init_waitqueue_head(&fence->wait, name, key);
>  	fence->flags = (unsigned long)fn;
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h
> index 30a863353ee6..70ba1789aa89 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.h
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.h
> @@ -17,26 +17,25 @@
>  
>  struct completion;
>  struct dma_resv;
> +struct i915_sw_fence;
> +
> +enum i915_sw_fence_notify {
> +	FENCE_COMPLETE,
> +	FENCE_FREE
> +};
> +
> +typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
> +				      enum i915_sw_fence_notify state);
>  
>  struct i915_sw_fence {
>  	wait_queue_head_t wait;
> +	i915_sw_fence_notify_t fn;
>  	unsigned long flags;
>  	atomic_t pending;
>  	int error;
>  };
>  
>  #define I915_SW_FENCE_CHECKED_BIT	0 /* used internally for DAG checking */
> -#define I915_SW_FENCE_PRIVATE_BIT	1 /* available for use by owner */
> -#define I915_SW_FENCE_MASK		(~3)
> -
> -enum i915_sw_fence_notify {
> -	FENCE_COMPLETE,
> -	FENCE_FREE
> -};
> -
> -typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
> -				      enum i915_sw_fence_notify state);
> -#define __i915_sw_fence_call __aligned(4)
>  
>  void __i915_sw_fence_init(struct i915_sw_fence *fence,
>  			  i915_sw_fence_notify_t fn,
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
> index 5b33ef23d54c..d2e56b387993 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
> @@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
>  	dma_fence_put(&f->dma);
>  }
>  
> -static int __i915_sw_fence_call
> +static int
>  fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  {
>  	struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
> diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> index cbf45d85cbff..daa985e5a19b 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
> @@ -28,7 +28,7 @@
>  
>  #include "../i915_selftest.h"
>  
> -static int __i915_sw_fence_call
> +static int
>  fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  {
>  	switch (state) {
> diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> index 080b90b63d16..eb59a41bdb79 100644
> --- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> +++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
> @@ -26,7 +26,7 @@
>  
>  /* Small library of different fence types useful for writing tests */
>  
> -static int __i915_sw_fence_call
> +static int
>  nop_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  {
>  	return NOTIFY_DONE;
> @@ -89,7 +89,7 @@ struct heap_fence {
>  	};
>  };
>  
> -static int __i915_sw_fence_call
> +static int
>  heap_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>  {
>  	struct heap_fence *h = container_of(fence, typeof(*h), fence);

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
@ 2021-09-22  2:13 Matthew Brost
  2021-09-22 11:12 ` Jani Nikula
  0 siblings, 1 reply; 11+ messages in thread
From: Matthew Brost @ 2021-09-22  2:13 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: jani.nikula, lucas.demarchi, tvrtko.ursulin

Rather than stealing bits from i915_sw_fence function pointer use
seperate fields for function pointer and flags. If using two different
fields, the 4 byte alignment for the i915_sw_fence function pointer can
also be dropped.

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
 drivers/gpu/drm/i915/i915_request.c           |  4 ++--
 drivers/gpu/drm/i915/i915_sw_fence.c          |  9 +++-----
 drivers/gpu/drm/i915/i915_sw_fence.h          | 21 +++++++++----------
 drivers/gpu/drm/i915/i915_sw_fence_work.c     |  2 +-
 .../gpu/drm/i915/selftests/i915_sw_fence.c    |  2 +-
 drivers/gpu/drm/i915/selftests/lib_sw_fence.c |  4 ++--
 8 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a7ca38613f89..6d5bb55ffc82 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10323,7 +10323,7 @@ static void intel_atomic_commit_work(struct work_struct *work)
 	intel_atomic_commit_tail(state);
 }
 
-static int __i915_sw_fence_call
+static int
 intel_atomic_commit_ready(struct i915_sw_fence *fence,
 			  enum i915_sw_fence_notify notify)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c2ab0e22db0a..df5fec5c3da8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -800,7 +800,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
 	free_engines(engines);
 }
 
-static int __i915_sw_fence_call
+static int
 engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_gem_engines *engines =
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index ce446716d092..945d3025a0b6 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, int error)
 	intel_context_cancel_request(rq->context, rq);
 }
 
-static int __i915_sw_fence_call
+static int
 submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *request =
@@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 	return NOTIFY_DONE;
 }
 
-static int __i915_sw_fence_call
+static int
 semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct i915_request *rq = container_of(fence, typeof(*rq), semaphore);
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..5cf101cf06d2 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -34,7 +34,7 @@ enum {
 
 static void *i915_sw_fence_debug_hint(void *addr)
 {
-	return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK);
+	return (void *)(((struct i915_sw_fence *)addr)->fn);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -126,10 +126,7 @@ static inline void debug_fence_assert(struct i915_sw_fence *fence)
 static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
 				  enum i915_sw_fence_notify state)
 {
-	i915_sw_fence_notify_t fn;
-
-	fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK);
-	return fn(fence, state);
+	return fence->fn(fence, state);
 }
 
 #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
@@ -242,7 +239,7 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  const char *name,
 			  struct lock_class_key *key)
 {
-	BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
+	BUG_ON(!fn);
 
 	__init_waitqueue_head(&fence->wait, name, key);
 	fence->flags = (unsigned long)fn;
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h
index 30a863353ee6..70ba1789aa89 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.h
+++ b/drivers/gpu/drm/i915/i915_sw_fence.h
@@ -17,26 +17,25 @@
 
 struct completion;
 struct dma_resv;
+struct i915_sw_fence;
+
+enum i915_sw_fence_notify {
+	FENCE_COMPLETE,
+	FENCE_FREE
+};
+
+typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
+				      enum i915_sw_fence_notify state);
 
 struct i915_sw_fence {
 	wait_queue_head_t wait;
+	i915_sw_fence_notify_t fn;
 	unsigned long flags;
 	atomic_t pending;
 	int error;
 };
 
 #define I915_SW_FENCE_CHECKED_BIT	0 /* used internally for DAG checking */
-#define I915_SW_FENCE_PRIVATE_BIT	1 /* available for use by owner */
-#define I915_SW_FENCE_MASK		(~3)
-
-enum i915_sw_fence_notify {
-	FENCE_COMPLETE,
-	FENCE_FREE
-};
-
-typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
-				      enum i915_sw_fence_notify state);
-#define __i915_sw_fence_call __aligned(4)
 
 void __i915_sw_fence_init(struct i915_sw_fence *fence,
 			  i915_sw_fence_notify_t fn,
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
index 5b33ef23d54c..d2e56b387993 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
@@ -23,7 +23,7 @@ static void fence_work(struct work_struct *work)
 	dma_fence_put(&f->dma);
 }
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
index cbf45d85cbff..daa985e5a19b 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -28,7 +28,7 @@
 
 #include "../i915_selftest.h"
 
-static int __i915_sw_fence_call
+static int
 fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	switch (state) {
diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
index 080b90b63d16..eb59a41bdb79 100644
--- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
@@ -26,7 +26,7 @@
 
 /* Small library of different fence types useful for writing tests */
 
-static int __i915_sw_fence_call
+static int
 nop_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	return NOTIFY_DONE;
@@ -89,7 +89,7 @@ struct heap_fence {
 	};
 };
 
-static int __i915_sw_fence_call
+static int
 heap_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
 	struct heap_fence *h = container_of(fence, typeof(*h), fence);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-11-16 19:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 14:57 [PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer Matthew Brost
2021-09-22 14:57 ` [Intel-gfx] " Matthew Brost
2021-09-22 15:21 ` Tvrtko Ursulin
2021-09-22 15:25   ` Tvrtko Ursulin
2021-09-22 15:40     ` Matthew Brost
  -- strict thread matches above, loose matches on Subject: below --
2021-11-16 19:49 Matthew Brost
2021-09-22 15:47 Matthew Brost
2021-11-11 20:18 ` Teres Alexis, Alan Previn
2021-11-11 20:18   ` Teres Alexis, Alan Previn
2021-09-22  2:13 Matthew Brost
2021-09-22 11:12 ` Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.