[Intel-gfx] [PATCH 2/6] drm/i915: Initialise basic fence before acquiring seqno

[Tvrtko Ursulin]

On 03/02/2020 10:58, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-02-03 10:55:52)
>> On 03/02/2020 09:41, Chris Wilson wrote:
>>> Inside the intel_timeline_get_seqno(), we currently track the retirement
>>> of the old cachelines by listening to the new request. This requires
>>> that the new request is ready to be used and so requires a minimum bit
>>> of initialisation prior to getting the new seqno.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>> Cc: Matthew Auld <matthew.auld at intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_request.c | 21 ++++++++++++++-------
>>>    1 file changed, 14 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
>>> index 78a5f5d3c070..f56b046a32de 100644
>>> --- a/drivers/gpu/drm/i915/i915_request.c
>>> +++ b/drivers/gpu/drm/i915/i915_request.c
>>> @@ -595,6 +595,8 @@ static void __i915_request_ctor(void *arg)
>>>        i915_sw_fence_init(&rq->submit, submit_notify);
>>>        i915_sw_fence_init(&rq->semaphore, semaphore_notify);
>>>    
>>> +     dma_fence_init(&rq->fence, &i915_fence_ops, &rq->lock, 0, 0);
>>> +
>>>        rq->file_priv = NULL;
>>>        rq->capture_list = NULL;
>>>    
>>> @@ -653,25 +655,30 @@ __i915_request_create(struct intel_context *ce, gfp_t gfp)
>>>                }
>>>        }
>>>    
>>> -     ret = intel_timeline_get_seqno(tl, rq, &seqno);
>>> -     if (ret)
>>> -             goto err_free;
>>> -
>>>        rq->i915 = ce->engine->i915;
>>>        rq->context = ce;
>>>        rq->engine = ce->engine;
>>>        rq->ring = ce->ring;
>>>        rq->execution_mask = ce->engine->mask;
>>>    
>>> +     kref_init(&rq->fence.refcount);
>>> +     rq->fence.flags = 0;
>>> +     rq->fence.error = 0;
>>> +     INIT_LIST_HEAD(&rq->fence.cb_list);
>>
>> One of these fields the previous user leaves in a non-empty state?
> 
> Yes, we don't reset cb_list after signaling, error and flags hold
> residuals, but refcount is known to be 0!

Okay,

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko