[Intel-gfx] [PATCH] drm/i915/guc: Don't read SOFT_SCRATCH(15) on MMIO error

[Michal Wajdeczko]

On Tue, 29 May 2018 17:17:02 +0200, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2018-05-29 16:10:44)
>> On Tue, 29 May 2018 16:54:12 +0200, Chris Wilson
>> <chris at chris-wilson.co.uk> wrote:
>>
>> > Quoting Michal Wajdeczko (2018-05-28 18:16:18)
>> >> SOFT_SCRATCH(15) is used by GuC for sending MMIO GuC events to host  
>> and
>> >> those events are now handled by  
>> intel_guc_to_host_event_handler_mmio().
>> >>
>> >> We should not try to read it on MMIO action error as 1) we may be  
>> using
>> >> different set of registers for GuC MMIO communication, and 2) GuC may
>> >> use CTB mechanism for sending events to host.
>> >
>> > Ok.
>> >
>> >> While here, upgrade error message to DRM_ERROR.
>> >
>> > Does the error help? What do you want to convey to the user? For error
>> > handling, we want to propagate the result back anyway for the caller  
>> has
>> > to decide what to do next.
>>
>> We are propagating error code to the caller, but since any error from  
>> the
>> GuC is unexpected, we should rather always log it and don't rely on the
>> caller or drm debug for that. Note that in case of CTB we also log  
>> received
>> errors using DRM_ERROR (see intel_guc_send_ct).
>
> But whose error? Ours or the hw? We expect hw errors, or should ;)

well, it can be any i915/FW/HW - hard to tell without other full logs..

>
> But mostly from the pov of the message, is this the right information to
> flag as the error or does the caller have better context?

Only caller can easily provide additional info related for failed command
(such as index/address that was rejected by FW) that could help diagnose
the problem, but in case FW/HW errors it does not matter.

At this point, we can only identify request/action ID that has failed.
But that's better than nothing.

/Michal