[Intel-gfx] [PATCH 35/39] drm/i915: Encode fence specific waitqueue behaviour into the wait.flags

[Chris Wilson]

Quoting Thomas Hellström (Intel) (2020-09-03 10:50:45)
> 
> On 9/2/20 4:02 PM, Thomas Hellström (Intel) wrote:
> > Hi, Chris,
> >
> > On 8/26/20 3:28 PM, Chris Wilson wrote:
> >> Use the wait_queue_entry.flags to denote the special fence behaviour
> >> (flattening continuations along fence chains, and for propagating
> >> errors) rather than trying to detect ordinary waiters by their
> >> functions.
> >>
> >> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >> ---
> >>   drivers/gpu/drm/i915/i915_sw_fence.c | 25 +++++++++++++++----------
> >>   1 file changed, 15 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c 
> >> b/drivers/gpu/drm/i915/i915_sw_fence.c
> >> index 4cd2038cbe35..4e557d1c4644 100644
> >> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> >> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> >> @@ -18,10 +18,15 @@
> >>   #define I915_SW_FENCE_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
> >>   #endif
> >>   -#define I915_SW_FENCE_FLAG_ALLOC BIT(3) /* after WQ_FLAG_* for 
> >> safety */
> >> -
> >>   static DEFINE_SPINLOCK(i915_sw_fence_lock);
> >>   +#define WQ_FLAG_BITS \
> >> +    BITS_PER_TYPE(typeof_member(struct wait_queue_entry, flags))
> >> +
> >> +/* after WQ_FLAG_* for safety */
> >> +#define I915_SW_FENCE_FLAG_FENCE BIT(WQ_FLAG_BITS - 1)
> >> +#define I915_SW_FENCE_FLAG_ALLOC BIT(WQ_FLAG_BITS - 2)
> >
> > Not sure if sharing the flags field with the wait.c implementation is 
> > fully OK either. Is the @key parameter to the wake function useable? I 
> > mean rather than passing just a list head could we pass something like
> >
> > struct i915_sw_fence_key {
> >     bool no_recursion; /* Makes the wait function just put its entry 
> > on @continuation and return */
> >     int error;
> >     struct list_head continuation;
> > }

That would mean wait_event-esque routines do not work on a fence.

> internal wake function instead of the autoremove_wake_function remove 
> the fragility. Would that be possible?

autoremove is the function used by display for its wait_event loop over
multiple sources.
-Chris