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? Matt > > v2: Bit more specific in commit description (Andi) > > Signed-off-by: Nitin Gote <nitin.r.gote@xxxxxxxxx> > --- > .../gpu/drm/i915/gt/intel_ring_submission.c | 20 +++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c > index 32f3b52a183a..74d6a2b703ac 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c > +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c > @@ -282,16 +282,16 @@ static int xcs_resume(struct intel_engine_cs *engine) > return 0; > > err: > - drm_err(&engine->i915->drm, > - "%s initialization failed; " > - "ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n", > - engine->name, > - ENGINE_READ(engine, RING_CTL), > - ENGINE_READ(engine, RING_CTL) & RING_VALID, > - ENGINE_READ(engine, RING_HEAD), ring->head, > - ENGINE_READ(engine, RING_TAIL), ring->tail, > - ENGINE_READ(engine, RING_START), > - i915_ggtt_offset(ring->vma)); > + ENGINE_TRACE(engine, > + "initialization failed; " > + "ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n", > + ENGINE_READ(engine, RING_CTL), > + ENGINE_READ(engine, RING_CTL) & RING_VALID, > + ENGINE_READ(engine, RING_HEAD), ring->head, > + ENGINE_READ(engine, RING_TAIL), ring->tail, > + ENGINE_READ(engine, RING_START), > + i915_ggtt_offset(ring->vma)); > + GEM_TRACE_DUMP(); > return -EIO; > } > > -- > 2.25.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation