Re: [RFC 1/3] dma-fence: Add boost fence op

[Christian König]

Am 20.05.21 um 16:07 schrieb Rob Clark:
> On Wed, May 19, 2021 at 11:47 PM Christian König
> <christian.koenig@xxxxxxx> wrote:
> > Uff, that looks very hardware specific to me.
> Howso?  I'm not sure I agree.. and even if it was not useful for some
> hw, it should be useful for enough drivers (and harm no drivers), so I
> still think it is a good idea
>
> The fallback plan is to go the i915 route and stop using atomic
> helpers and do the same thing inside the driver, but that doesn't help
> any of the cases where you have a separate kms and gpu driver.

Yeah, that's certainly not something we want.

> > As far as I can see you can also implement completely inside the backend
> > by starting a timer on enable_signaling, don't you?
> Not really.. I mean, the fact that something waited on a fence could
> be a useful input signal to gpu freq governor, but it is entirely
> insufficient..
>
> If the cpu is spending a lot of time waiting on a fence, cpufreq will
> clock down so you spend less time waiting.  And no problem has been
> solved.  You absolutely need the concept of a missed deadline, and a
> timer doesn't give you that.

Ok then I probably don't understand the use case here.

What exactly do you try to solve?

Thanks,
Christian.

> BR,
> -R
>
> > Christian.
> >
> > Am 19.05.21 um 20:38 schrieb Rob Clark:
> > > From: Rob Clark <robdclark@xxxxxxxxxxxx>
> > >
> > > Add a way to hint to the fence signaler that a fence waiter has missed a
> > > deadline waiting on the fence.
> > >
> > > In some cases, missing a vblank can result in lower gpu utilization,
> > > when really we want to go in the opposite direction and boost gpu freq.
> > > The boost callback gives some feedback to the fence signaler that we
> > > are missing deadlines, so it can take this into account in it's freq/
> > > utilization calculations.
> > >
> > > Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> > > ---
> > >    include/linux/dma-fence.h | 26 ++++++++++++++++++++++++++
> > >    1 file changed, 26 insertions(+)
> > >
> > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> > > index 9f12efaaa93a..172702521acc 100644
> > > --- a/include/linux/dma-fence.h
> > > +++ b/include/linux/dma-fence.h
> > > @@ -231,6 +231,17 @@ struct dma_fence_ops {
> > >        signed long (*wait)(struct dma_fence *fence,
> > >                            bool intr, signed long timeout);
> > >
> > > +     /**
> > > +      * @boost:
> > > +      *
> > > +      * Optional callback, to indicate that a fence waiter missed a deadline.
> > > +      * This can serve as a signal that (if possible) whatever signals the
> > > +      * fence should boost it's clocks.
> > > +      *
> > > +      * This can be called in any context that can call dma_fence_wait().
> > > +      */
> > > +     void (*boost)(struct dma_fence *fence);
> > > +
> > >        /**
> > >         * @release:
> > >         *
> > > @@ -586,6 +597,21 @@ static inline signed long dma_fence_wait(struct dma_fence *fence, bool intr)
> > >        return ret < 0 ? ret : 0;
> > >    }
> > >
> > > +/**
> > > + * dma_fence_boost - hint from waiter that it missed a deadline
> > > + *
> > > + * @fence: the fence that caused the missed deadline
> > > + *
> > > + * This function gives a hint from a fence waiter that a deadline was
> > > + * missed, so that the fence signaler can factor this in to device
> > > + * power state decisions
> > > + */
> > > +static inline void dma_fence_boost(struct dma_fence *fence)
> > > +{
> > > +     if (fence->ops->boost)
> > > +             fence->ops->boost(fence);
> > > +}
> > > +
> > >    struct dma_fence *dma_fence_get_stub(void);
> > >    u64 dma_fence_context_alloc(unsigned num);