[Intel-gfx] [PATCH] drm/i914/guc: Fix resume on platforms w/o GuC submission but enabled

[Daniele Ceraolo Spurio]

On 10/28/19 11:17 AM, Hiatt, Don wrote:
>> From: Ceraolo Spurio, Daniele <daniele.ceraolospurio at intel.com>
>> Sent: Monday, October 28, 2019 9:44 AM
>> To: Hiatt, Don <don.hiatt at intel.com>; intel-gfx at lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i914/guc: Fix resume on platforms w/o GuC
>> submission but enabled
>>
>>
>>
>> On 10/24/19 9:29 AM, don.hiatt at intel.com wrote:
>>> From: Don Hiatt <don.hiatt at intel.com>
>>>
>>> Check to see if GuC submission is enabled before requesting the
>>> EXIT_S_STATE action.
>>>
>>
>> You're only skipping the resume, but does it make any sense to do the
>> suspend action if we're not going to call the resume one? Does guc do
>> anything in the suspend action that we still require? I thought it only
>> saved the submission status, which we don't care about if guc submission
>> is disabled.
>>
>> Daniele
>>
> 
> Hi Daniele,
> 
> I tried skipping the suspend all together but then the HuC gets timeouts
> waiting for the GuC to acknowledge the authentication request which leads to a
> wedged GPU. ☹
> 

Do we know why? if we skip the suspend/resume H2G and reload the blobs 
after resetting the HW it should look like a clean boot from the HW 
perspective, so the fact that HuC auth times out feels weird and might 
hide other issues. I asked one of the guc devs and he also thinks this 
is not expected behavior. Can you dig a bit more?

Thanks,
Daniele

> BTW, I made a typo in the patch, should be 'drm/i915' not '914', I'll fix that
> up.
> 
> Thanks,
> 
> don
> 
> 
>>> On some platforms (e.g. KBL) that do not support GuC submission, but
>>> the user enabled the GuC communication (e.g for HuC authentication)
>>> calling the GuC EXIT_S_STATE action results in lose of ability to
>>> enter RC6. Guard against this by only requesting the GuC action on
>>> platforms that support GuC submission.
>>>
>>> I've verfied that intel_guc_resume() only gets called when driver
>>> is loaded with: guc_enable={1,2,3}, all other cases (no args,
>>> guc_enable={0,-1} the intel_guc_resume() is not called.
>>>
>>> Signed-off-by: Don Hiatt <don.hiatt at intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gt/uc/intel_guc.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> index 37f7bcbf7dac..33318ed135c0 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> @@ -565,7 +565,10 @@ int intel_guc_resume(struct intel_guc *guc)
>>>    		GUC_POWER_D0,
>>>    	};
>>>
>>> -	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>>> +	if (guc->submission_supported)
>>> +		return intel_guc_send(guc, action, ARRAY_SIZE(action));
>>> +
>>> +	return 0;
>>>    }
>>>
>>>    /**
>>>