[Intel-gfx] [RFC v2 0/5] Waitboost drm syncobj waits

[Rob Clark]

On Thu, Feb 16, 2023 at 10:20 AM Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
>
> On Tue, Feb 14, 2023 at 11:14:00AM -0800, Rob Clark wrote:
> > On Fri, Feb 10, 2023 at 5:07 AM Tvrtko Ursulin
> > <tvrtko.ursulin at linux.intel.com> wrote:
> > >
> > > From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > >
> > > In i915 we have this concept of "wait boosting" where we give a priority boost
> > > for instance to fences which are actively waited upon from userspace. This has
> > > it's pros and cons and can certainly be discussed at lenght. However fact is
> > > some workloads really like it.
> > >
> > > Problem is that with the arrival of drm syncobj and a new userspace waiting
> > > entry point it added, the waitboost mechanism was bypassed. Hence I cooked up
> > > this mini series really (really) quickly to see if some discussion can be had.
> > >
> > > It adds a concept of "wait count" to dma fence, which is incremented for every
> > > explicit dma_fence_enable_sw_signaling and dma_fence_add_wait_callback (like
> > > dma_fence_add_callback but from explicit/userspace wait paths).
> >
> > I was thinking about a similar thing, but in the context of dma_fence
> > (or rather sync_file) fd poll()ing.  How does the kernel differentiate
> > between "housekeeping" poll()ers that don't want to trigger boost but
> > simply know when to do cleanup, and waiters who are waiting with some
> > urgency.  I think we could use EPOLLPRI for this purpose.
> >
> > Not sure how that translates to waits via the syncobj.  But I think we
> > want to let userspace give some hint about urgent vs housekeeping
> > waits.
>
> Should the hint be on the waits, or should the hints be on the executed
> context?

I think it should be on the wait, because different waits may be for
different purposes.  Ideally this could be exposed at the app API
level, but I guess first step is to expose it to userspace.

BR,
-R

> In the end we need some way to quickly ramp-up the frequency to avoid
> the execution bubbles.
>
> waitboost is trying to guess that, but in some cases it guess wrong
> and waste power.
>
> btw, this is something that other drivers might need:
>
> https://gitlab.freedesktop.org/drm/amd/-/issues/1500#note_825883
> Cc: Alex Deucher <alexander.deucher at amd.com>
>
> >
> > Also, on a related topic: https://lwn.net/Articles/868468/
> >
> > BR,
> > -R
> >
> > > Individual drivers can then inspect this via dma_fence_wait_count() and decide
> > > to wait boost the waits on such fences.
> > >
> > > Again, quickly put together and smoke tested only - no guarantees whatsoever and
> > > I will rely on interested parties to test and report if it even works or how
> > > well.
> > >
> > > v2:
> > >  * Small fixups based on CI feedback:
> > >     * Handle decrement correctly for already signalled case while adding callback.
> > >     * Remove i915 assert which was making sure struct i915_request does not grow.
> > >  * Split out the i915 patch into three separate functional changes.
> > >
> > > Tvrtko Ursulin (5):
> > >   dma-fence: Track explicit waiters
> > >   drm/syncobj: Mark syncobj waits as external waiters
> > >   drm/i915: Waitboost external waits
> > >   drm/i915: Mark waits as explicit
> > >   drm/i915: Wait boost requests waited upon by others
> > >
> > >  drivers/dma-buf/dma-fence.c               | 102 ++++++++++++++++------
> > >  drivers/gpu/drm/drm_syncobj.c             |   6 +-
> > >  drivers/gpu/drm/i915/gt/intel_engine_pm.c |   1 -
> > >  drivers/gpu/drm/i915/i915_request.c       |  13 ++-
> > >  include/linux/dma-fence.h                 |  14 +++
> > >  5 files changed, 101 insertions(+), 35 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >