[Intel-gfx] [CI 2/2] drm/i915/gt: Warn more clearly if the context state is still pinned

[Tvrtko Ursulin]

On 23/04/2020 16:17, Chris Wilson wrote:
> Quoting Chris Wilson (2020-04-23 16:16:09)
>> Quoting Tvrtko Ursulin (2020-04-23 15:47:58)
>>>
>>> On 23/04/2020 09:59, Chris Wilson wrote:
>>>> When recording the default context state, we submit an ordinary context
>>>> and then steal the context image for our defaults. To be able to steal
>>>> the state, we must have total ownership of the context. During CI we
>>>> want to make this error extremely obvious, as otherwise we will fail the
>>>> user's module load.
>>>>
>>>> References: https://gitlab.freedesktop.org/drm/intel/-/issues/1763
>>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>> ---
>>>>    drivers/gpu/drm/i915/gt/intel_gt.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
>>>> index 1c99cc72305a..379eb39e7979 100644
>>>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>>>> @@ -472,6 +472,7 @@ static int __engines_record_defaults(struct intel_gt *gt)
>>>>    
>>>>                /* We want to be able to unbind the state from the GGTT */
>>>>                GEM_BUG_ON(intel_context_is_pinned(rq->context));
>>>> +             GEM_BUG_ON(i915_vma_is_pinned(state));
>>>
>>> Not sure - context->state ownership is in the context pinned status and
>>> then there is the unbind below which will fail if vma is pinned. Which
>>> sounds better than a bug on. Is it difficult to figure out what is
>>> failing in practice? A debug message on the unbind failure path might be
>>> more friendly in that case.
>>
>> I was able to recognise the failure :) I thought this might be more
>> explicit.
> 
> One thing in the GEM_BUG_ON()'s favour is that in the unbind we do have
> one more wait, which gives a small saving grace and we have fewer
> EAGAINs than BUG_ONs. So the BUG_ON were more sensitive and so useful
> for testing.

Okay.. :)

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

Regards,

Tvrtko