[Intel-gfx] [PATCH 1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[Tvrtko Ursulin]

On 20/06/2017 17:05, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-06-20 16:55:12)
>>
>> On 20/06/2017 13:43, Chris Wilson wrote:
>>> Since we may track unfenced access (GPU access to the vma that
>>> explicitly requires no fence), vma->last_fence may be set without any
>>
>> Is this the gen < 4 code path? There is no actual fence in this case?
> 
> Yes, this is for old hw that used a fence for GPU access, and in this
> case indeed there is no fence and we are tracking the access for when
> the fence is not wanted to ensure that don't install a fence before the
> GPU finished accessing the region.
> 
>>> attached fence (vma->fence) and so will not be flushed when we call
>>> i915_vma_put_fence(). Since we stopped doing a full retire of the
>>> activity trackers for unbind, we need to explicitly retire each tracker.
>>>
>>> Fixes: b0decaf75bd9 ("drm/i915: Track active vma requests")
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_vma.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>>> index 532c709febbd..1cfe137cdc32 100644
>>> --- a/drivers/gpu/drm/i915/i915_vma.c
>>> +++ b/drivers/gpu/drm/i915/i915_vma.c
>>> @@ -672,6 +672,11 @@ int i915_vma_unbind(struct i915_vma *vma)
>>>                                break;
>>>                }
>>>    
>>> +             if (!ret) {
>>> +                     ret = i915_gem_active_retire(&vma->last_fence,
>>> +                                                  &vma->vm->i915->drm.struct_mutex);
>>> +             }
>>> +
>>>                __i915_vma_unpin(vma);
>>>                if (ret)
>>>                        return ret;
>>>
>>
>> Looks safe anyway, but I'd like to understand how exactly it happens and
>> if it is still possible after patch 2/3.
> 
> For gen2/3, we still use last_fence. For others it was accidental and
> should be no more after patch 2. I considered add a GEM_BUG_ON to
> i915_move_to_active but didn't have a convenient i915 pointer, and I
> have grander plans :)

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

Regards,

Tvrtko