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

[Thomas Hellström (Intel)]

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;
> }
>
> /Thomas
>
>
Actually, after doing some thinking, wouldn't just comparing against the 
internal wake function instead of the autoremove_wake_function remove 
the fragility. Would that be possible?

/Thomas