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.g...@intel.com>
> ---
>  .../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

Reply via email to