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

[Tvrtko Ursulin]

On 16/08/16 10:39, Goel, Akash wrote:
>
>
> 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);

I get it now, the manual flush was the bit which I was not seeing.

Regards,

Tvrtko