[Intel-gfx] [PATCH 3/8] drm/i915/pmu: Expose a PMU interface for perf queries

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-09-26 13:28:03)
> 
> On 25/09/2017 18:37, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> > index 4eb1a76731b2..19b8b31afbbc 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_request.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> > @@ -477,6 +477,7 @@ void __i915_gem_request_submit(struct drm_i915_gem_request *request)
> >          engine->emit_breadcrumb(request,
> >                                  request->ring->vaddr + request->postfix);
> >   
> > +       atomic_dec(&request->engine->queued);
> >          spin_lock(&request->timeline->lock);
> >          list_move_tail(&request->link, &timeline->requests);
> >          spin_unlock(&request->timeline->lock);
> > @@ -525,6 +526,7 @@ void __i915_gem_request_unsubmit(struct drm_i915_gem_request *request)
> >          spin_lock(&timeline->lock);
> >          list_move(&request->link, &timeline->requests);
> >          spin_unlock(&timeline->lock);
> > +       atomic_inc(&request->engine->queued);
> >   
> >          /* We don't need to wake_up any waiters on request->execute, they
> >           * will get woken by any other event or us re-adding this request
> > @@ -556,6 +558,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
> >          switch (state) {
> >          case FENCE_COMPLETE:
> >                  trace_i915_gem_request_submit(request);
> > +               atomic_inc(&request->engine->queued);
> >                  request->engine->submit_request(request);
> 
> Or have the counter normal unsigned int under the engine timeline lock, 
> moving into the execlists_submit_request etc?
> 
> >                  break;
> >   
> > 
> > That excludes the requests actually in the hw queue, just those in the sw
> > queue. Or both.
> 
> This looks good to me, I was just reluctant to suggest new software 
> tracking. Happy to go with this and to set the metric meaning to this?

It's always a cost-benefit analysis. The cost is usually pretty clear;
the benefits less so! Though there's a hidden cost, being ABI we want to
make sure that it is meaningful for a large user base and remain
applicable for the next 5 years or so (without becoming a burden upon
ourselves).

In this case, we have a potential consumer that we think will be poc for
a good chunk of applications, and we are trying to find just the right
counter(s) that will satisfy it for the next few years. So the benefit
will be clear as well (or not and we can scrap it before committing
ourselves to it).
-Chris