[Intel-gfx] [PATCH 14/18] drm/i915: Forcefully flush GuC log buffer on reset

[Goel, Akash]

On 8/16/2016 2:55 PM, Tvrtko Ursulin wrote:
>
> On 16/08/16 06:25, Goel, Akash wrote:
>> On 8/15/2016 9:18 PM, Tvrtko Ursulin wrote:
>>> On 15/08/16 15:49, akash.goel at intel.com wrote:
>>>> From: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>>>>
>>>> Before capturing the GuC logs as a part of error state, there should
>>>> be a
>>>> force log buffer flush action sent to GuC before proceeding with GPU
>>>> reset
>>>> and re-initializing GUC. There could be some data in the log buffer
>>>> which
>>>> is yet to be captured and those logs would be particularly useful to
>>>> understand that why the GPU reset was initiated.
>>>>
>>>> v2:
>>>> - Avoid the wait via flush_work, to serialize against an ongoing log
>>>>    buffer flush, from the error state capture path. (Chris)
>>>
>>> Could you explain if the patch does anything now that the flush has been
>>> removed?
>>>
>> flush_work for the regular log buffer flush work item has been removed
>> but the forceful command is still sent to GuC.
>>
>>> In fact I don't even understand what it was doing before. :)
>>>
>> I am sorry for that.
>>
>>> If the idea is to send a flush command to GuC so it can raise an
>>> interrupt for a partially full buffer,
>> Yes exactly this is the idea.
>
> But then isn't the order wrong? Should it first send the flush command
> to the GuC and then wait for something maybe gets flushed?
As I tried to clarify in my last email that GuC firmware just ignores 
the forceful flush command received from Host, if it sees there is a 
pending request for regular log buffer flush, for which it hasn't 
received the ack.

So from the Host side, we need to first wait for the regular log buffer 
flush work item to finish execution, if any, and then send the forceful
flush command to GuC.

> I can see that it could be tricky since the timing is undefined, but I don't
Yes it is deinitely tricky with respect to the timing.

> understand where it currently actually processes that potential extra
> packets.
The extra left over logs are captured manually just after sending the 
forceful flush command to GuC.
	i915_guc_flush_logs(dev_priv, true);
	/* GuC would have updated log buffer by now, so capture it */
	i915_guc_capture_logs(dev_priv);
	
> Especially since it disabled interrupts before hand.
Had disabled the interrupt, out of paranoia, to avoid a situation of 
work item getting scheduled again (for a different buffer type) while we 
manually collect the extra logs.

Best regards
Akash

>
> Regards,
>
> Tvrtko
>