[Intel-gfx] [PATCH 1/2] drm/i915/guc: Fix sleep under spinlock during reset

[Michel Thierry]

On 27/04/17 11:20, Tvrtko Ursulin wrote:
>
> On 27/04/2017 19:14, Michel Thierry wrote:
>> On 12/04/17 09:22, Michel Thierry wrote:
>>> On 12/04/17 08:58, Chris Wilson wrote:
>>>> On Wed, Apr 12, 2017 at 04:48:42PM +0100, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>>
>>>>> Looks like intel_guc_reset had the ability to sleep under the
>>>>> uncore spinlock since forever but it wasn't detected until the
>>>>> recent changes annotated the wait for register with might_sleep.
>>>>>
>>>>> I have fixed it by removing holding of the uncore spinlock over
>>>>> the call to gen6_hw_domain_reset, since I do not see that is
>>>>> really needed. But there is always a possibility I am missing
>>>>> some nasty detail so please double check.
>>>>
>>>> Afaik, no we are not using the uncore.lock here to serialise resets so
>>>> yes we should be safe in dropping it.
>>>>
>>>> Will the guc be coming under the same hw semaphore as gen8 per-engine
>>>> resets?
>>>
>>> A bit unrelated, but should intel_guc_reset be intel_reset_guc instead?
>>> Here we're trying to reset the microcontroller, not asking guc to do a
>>> reset.
>>
>> Ping?
>>
>> Anyone unlucky enough to be using GuC submission should be seeing this
>> warning when the firmware has to be reloaded (for example after any
>> i-g-t hang test).
>>
>> I still think the function should be renamed to _reset_guc though, since
>> it's the hw reseting the guc, not the other way around.
>>
>> Acked-by: Michel Thierry <michel.thierry at intel.com>
>
> Thanks! Now just exercise restrain in suggesting bikesheds and if
> someone can provide an r-b we could merge this. ;) (To be read as - lets
> leave the renaming for a follow up work since this fix is not to blame
> for the objectionable name.)
>
> Regards,
>

_Invoking GuC experts_

Agreed, and since I'm the one that will tell the guc to perform a reset, 
I can include the bikeshed in my patches.