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

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Thu Apr 27 18:20:48 UTC 2017


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,

Tvrtko


More information about the Intel-gfx mailing list