| Message ID | 20200122124154.483444-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gt: Include a tell-tale for engine parking | expand |
On 22/01/2020 12:41, Chris Wilson wrote: > We have two trace messages that rely on the function name for > distinction. However, if gcc inlines the function, the two traces end up > with the same function name and are indistinguishable. Add a different > message to each to clarify which one we hit, i.e. which phase of engine > parking we are processing. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > index ea90ab3e396e..b6cf284e3a2d 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > @@ -112,7 +112,7 @@ __queue_and_release_pm(struct i915_request *rq, > { > struct intel_gt_timelines *timelines = &engine->gt->timelines; > > - ENGINE_TRACE(engine, "\n"); > + ENGINE_TRACE(engine, "parking\n"); > > /* > * We have to serialise all potential retirement paths with our > @@ -249,7 +249,7 @@ static int __engine_park(struct intel_wakeref *wf) > if (!switch_to_kernel_context(engine)) > return -EBUSY; > > - ENGINE_TRACE(engine, "\n"); > + ENGINE_TRACE(engine, "parked\n"); > > call_idle_barriers(engine); /* cleanup after wedging */ > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
Chris Wilson <chris@chris-wilson.co.uk> writes: > We have two trace messages that rely on the function name for > distinction. However, if gcc inlines the function, the two traces end up > with the same function name and are indistinguishable. Add a different > message to each to clarify which one we hit, i.e. which phase of engine > parking we are processing. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > index ea90ab3e396e..b6cf284e3a2d 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > @@ -112,7 +112,7 @@ __queue_and_release_pm(struct i915_request *rq, > { > struct intel_gt_timelines *timelines = &engine->gt->timelines; > > - ENGINE_TRACE(engine, "\n"); > + ENGINE_TRACE(engine, "parking\n"); > > /* > * We have to serialise all potential retirement paths with our > @@ -249,7 +249,7 @@ static int __engine_park(struct intel_wakeref *wf) > if (!switch_to_kernel_context(engine)) > return -EBUSY; > > - ENGINE_TRACE(engine, "\n"); > + ENGINE_TRACE(engine, "parked\n"); Reading the functions, the exact spots are a mystery for me still as of why in these exact lines. Like the 'parked' would mean it is parked already, which it seems not to. However, what comes to the commit message and to immediate problem and fixing it, Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > call_idle_barriers(engine); /* cleanup after wedging */ > > -- > 2.25.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2020-01-22 14:39:19) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > We have two trace messages that rely on the function name for > > distinction. However, if gcc inlines the function, the two traces end up > > with the same function name and are indistinguishable. Add a different > > message to each to clarify which one we hit, i.e. which phase of engine > > parking we are processing. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > index ea90ab3e396e..b6cf284e3a2d 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > @@ -112,7 +112,7 @@ __queue_and_release_pm(struct i915_request *rq, > > { > > struct intel_gt_timelines *timelines = &engine->gt->timelines; > > > > - ENGINE_TRACE(engine, "\n"); > > + ENGINE_TRACE(engine, "parking\n"); > > > > /* > > * We have to serialise all potential retirement paths with our > > @@ -249,7 +249,7 @@ static int __engine_park(struct intel_wakeref *wf) > > if (!switch_to_kernel_context(engine)) > > return -EBUSY; > > > > - ENGINE_TRACE(engine, "\n"); > > + ENGINE_TRACE(engine, "parked\n"); > > Reading the functions, the exact spots are a mystery for me still > as of why in these exact lines. Like the 'parked' would mean it > is parked already, which it seems not to. True, but 'park' was too similar to 'parking', whereas 'parked' at least conveyed that 'parking' was over, which was the important concept that I wanted clarity over in the debug traces. They are mere labels, if you have a better idea, ... :) -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index ea90ab3e396e..b6cf284e3a2d 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -112,7 +112,7 @@ __queue_and_release_pm(struct i915_request *rq, { struct intel_gt_timelines *timelines = &engine->gt->timelines; - ENGINE_TRACE(engine, "\n"); + ENGINE_TRACE(engine, "parking\n"); /* * We have to serialise all potential retirement paths with our @@ -249,7 +249,7 @@ static int __engine_park(struct intel_wakeref *wf) if (!switch_to_kernel_context(engine)) return -EBUSY; - ENGINE_TRACE(engine, "\n"); + ENGINE_TRACE(engine, "parked\n"); call_idle_barriers(engine); /* cleanup after wedging */
We have two trace messages that rely on the function name for distinction. However, if gcc inlines the function, the two traces end up with the same function name and are indistinguishable. Add a different message to each to clarify which one we hit, i.e. which phase of engine parking we are processing. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)