[Intel-gfx] [PATCH] drm/i915/guc: Fix request re-submission after reset

[Tvrtko Ursulin]

On 09/03/2017 08:42, Oscar Mateo wrote:
> On 03/09/2017 02:05 AM, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> In order to ensure no missed interrupts we must first re-direct
>> the interrupts to GuC, and only then re-submit the requests to
>> be replayed after a GPU reset. Otherwise context switch can fire
>> before GuC has been set up to receive it triggering more hangs.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
>> Cc: Oscar Mateo <oscar.mateo at intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_guc_submission.c | 58
>> +++++++++++++++++++++++++++---
>>   drivers/gpu/drm/i915/intel_guc_loader.c    | 47
>> ------------------------
>>   2 files changed, 54 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index beb38e30d0e9..5b8ec0fab563 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -936,6 +936,52 @@ static void guc_reset_wq(struct i915_guc_client
>> *client)
>>       client->wq_tail = 0;
>>   }
>>
> <SNIP>
>>   int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
>>   {
>>       struct intel_guc *guc = &dev_priv->guc;
>> @@ -953,13 +999,17 @@ int i915_guc_submission_enable(struct
>> drm_i915_private *dev_priv)
>>         /* Take over from manual control of ELSP (execlists) */
>>       for_each_engine(engine, dev_priv, id) {
>> -        const int wqi_size = sizeof(struct guc_wq_item);
>> -        struct drm_i915_gem_request *rq;
>> -
>>           engine->submit_request = i915_guc_submit;
>>           engine->schedule = NULL;
>> +    }
>> +
>> +    guc_interrupts_capture(dev_priv);
>> +
>> +    /* Replay the current set of previously submitted requests */
>> +    for_each_engine(engine, dev_priv, id) {
>> +        const int wqi_size = sizeof(struct guc_wq_item);
>> +        struct drm_i915_gem_request *rq;
>>
> Don't you want to move the guc_interrupts_release to
> i915_guc_submission_disable as well?

I can't spot anything broken in that path. We never go in that direction 
with the live submission so why do you think it is needed?

Regards,

Tvrtko