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
