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? Thanks, Andi
