Re: [PATCH 7/8] drm/i915/pmu: Wire up engine busy stats to PMU

Rogozhkin, Dmitry V
2017-09-26T18:46:50Z

On Tue, 2017-09-26 at 13:32 +0100, Tvrtko Ursulin wrote:
> On 25/09/2017 18:48, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-09-25 16:15:42)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> We can use engine busy stats instead of the MMIO sampling timer
> >> for better efficiency.
> >>
> >> As minimum this saves period * num_engines / sec mmio reads,
> >> and in a better case, when only engine busy samplers are active,
> >> it enables us to not kick off the sampling timer at all.
> >
> > Or you could inspect port_isset(execlists.port).
> > You can avoid the mmio for this case also by just using HWSP. It's just
> > that I never enabled busy tracking in isolation, so I always ended up
> > using the mmio.
>
> This would be for execlists only. I could change the main patch to do
> this, you think it is worth it?

You know, I wonder why we limit this by execlists? Is that because
scheduler is working only for execlists and doesn't work for ringbuffers
on older HW? But consider the following. If we don't have a scheduler,
then we have FIFO queue and either hw semaphore or sw sync. For the
userspace applications real execution and wait are not actually
distinguishable: for him engine is busy, it either executes or it is
stalled and can't execute anything else, thus - it is busy.

From this perspective we can consider to extend what we do currently for
execlists to cover FIFO ringbuffers. How do you think? Other metrics
like SEMA or WAIT would be second level of details and will mean
distribution of BUSY time: we spent SEMA time on semaphore or WAIT time
on the wait and actively ran something BUSY-SEMA(WAIT) time.

By the way, with the BDW+ and execlists, is my understanding right that
we report WAIT metric and SEMA is always 0?

>
> > Stronger argument is that this method give more precise timing than
> > stochastic sampling.
> >
> >>
> >> v2: Rebase.
> >> v3:
> >> * Rebase, comments.
> >> * Leave engine busyness controls out of workers.
> >> v4: Checkpatch cleanup.
> >> v5: Added comment to pmu_needs_timer change.
> >> v6:
> >> * Rebase.
> >> * Fix style of some comments. (Chris Wilson)
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >
> > With a better explanation of why this is preferred,
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Thanks,
>
> Tvrtko
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx