[PATCH v2] drm/i915/gt: Use ENGINE_TRACE for tracing.

[Tvrtko Ursulin]

On 25/10/2024 12:21, Andi Shyti wrote:
> On Fri, Oct 25, 2024 at 09:02:16AM +0100, Tvrtko Ursulin wrote:
>>
>> On 24/10/2024 19:58, Matt Roper wrote:
>>> On Thu, Oct 24, 2024 at 04:09:17PM +0530, Nitin Gote wrote:
>>>> There is ENGINE_TRACE() macro which introduce engine name
>>>> with GEM tracing in i915. So, it will be good to use ENGINE_TRACE()
>>>> over drm_err() drm_device based logging for engine debug log.
>>>
>>> Doesn't this just eliminate the logging completely if the driver is
>>> built without CONFIG_DRM_I915_TRACE_GEM?  That means that most users
>>> will probably get no dmesg output at all on failure now, which could
>>> make it harder for us to understand and debug user-reported bugs.
>>>
>>> Of course intel_ring_submission.c only gets used for the old
>>> pre-execlist platforms (HSW and older) so maybe there are few enough of
>>> those in active usage that we don't really get too many new bug reports
>>> anyway?
>>
>> Yeah, plus, the justification about engine name does not appear to hold,
>> since the old message was printing it already. So if there is something more
>> happening here under the covers please do explain it properly in the commit
>> message.
> 
> I'm sorry, but I already applied the patch.
> 
> I agree with it because most of the information provided are not
> really useful to the failure, but more to who is actually
> debugging the code.
> 
> Would it make sense to add a gt_err() after or before the
> ENGINE_TRACE() so that we have the error printing along with
> debug information?

I think so. Without that Matt's point that the patch makes harder to 
understand from the field bug report stands.

Regards,

Tvrtko