Message-ID: <17f7e755-fce2-b7cf-dd6f-0a0dec618bba@amd.com>
Date:   Fri, 21 May 2021 09:43:59 +0200
From:   Christian König <christian.koenig@....com>
To:     Daniel Vetter <daniel@...ll.ch>
Cc:     Christian König <ckoenig.leichtzumerken@...il.com>,
        Rob Clark <robdclark@...il.com>,
        Rob Clark <robdclark@...omium.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        "moderated list:DMA BUFFER SHARING FRAMEWORK" 
        <linaro-mm-sig@...ts.linaro.org>,
        freedreno <freedreno@...ts.freedesktop.org>,
        "open list:DMA BUFFER SHARING FRAMEWORK" 
        <linux-media@...r.kernel.org>
Subject: Re: [Linaro-mm-sig] [RFC 1/3] dma-fence: Add boost fence op

Am 20.05.21 um 19:08 schrieb Daniel Vetter:
> [SNIP]
>> AH! So we are basically telling the fence backend that we have just
>> missed an event we waited for.
>>
>> So what we want to know is how long the frontend wanted to wait instead
>> of how long the backend took for rendering.
> tbh I'm not sure the timestamp matters at all. What we do in i915 is
> boost quite aggressively, and then let the usual clock tuning wittle
> it down if we overshot. Plus soom cool-down to prevent
> abuse/continuous boosting. I think we also differentiate between
> display boost and userspace waits.

I was not thinking about time stamps here, but more like which 
information we need at which place.

> On the display side we also wait until the vblank has passed we aimed
> for (atm always the next, we don't have target_frame support like
> amdgpu), to avoid boosting when there's no point.
>
>>> So boosting right when you've missed your frame (not what Rob implements
>>> currently, but fixable) is the right semantics.
>>>
>>> The other issue is that for cpu waits, we want to differentiate from fence
>>> waits that userspace does intentially (e.g. wait ioctl) and waits that
>>> random other things are doing within the kernel to keep track of progress.
>>>
>>> For the former we know that userspace is stuck waiting for the gpu, and we
>>> probably want to boost. For the latter we most definitely do _not_ want to
>>> boost.
>>>
>>> Otoh I do agree with you that the current api is a bit awkward, so perhaps
>>> we do need a dma_fence_userspace_wait wrapper which boosts automatically
>>> after a bit. And similarly perhaps a drm_vblank_dma_fence_wait, where you
>>> give it a vblank target, and if the fence isn't signalled by then, we kick
>>> it real hard.
>> Yeah, something like an use case driven API would be nice to have.
>>
>> For this particular case I suggest that we somehow extend the enable
>> signaling callback.
>>
>>> But otherwise yes this is absolutely a thing that matters a ton. If you
>>> look at Matt Brost's scheduler rfc, there's also a line item in there
>>> about adding this kind of boosting to drm/scheduler.
>> BTW: I still can't see this in my inbox.
> You've replied already:
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20210518235830.133834-1-matthew.brost%40intel.com%2F&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7Ce4f3688b832842c4236e08d91bb1e148%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637571273080820910%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=uk3Gs%2FW42BDqMuMJtujcAH5GvN8mOlDnmywK8x1I%2F0k%3D&amp;reserved=0

Yeah, but doesn't that also require some changes to the DRM scheduler?

I was expecting that this is a bit more than just two patches.

Christian.

>
> It's just the big picture plan of what areas we're all trying to
> tackle with some why, so that everyone knows what's coming in the next
> half year at least. Probably longer until this is all sorted. I think
> Matt has some poc hacked-up pile, but nothing really to show.
> -Daniel
>
>> Do you have a link?
>>
>> Christian.
>>
>>> -Daniel
>>>
>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> BR,
>>>>> -R
>>>>>
>>>>>> Thanks,
>>>>>> Christian.
>>>>>>
>>>>>>> BR,
>>>>>>> -R
>>>>>>>
>>>>>>>> Christian.
>>>>>>>>
>>>>>>>> Am 19.05.21 um 20:38 schrieb Rob Clark:
>>>>>>>>> From: Rob Clark <robdclark@...omium.org>
>>>>>>>>>
>>>>>>>>> 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@...omium.org>
>>>>>>>>> ---
>>>>>>>>>       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);
>>>>>>>>>
>>>>> _______________________________________________
>>>>> Linaro-mm-sig mailing list
>>>>> Linaro-mm-sig@...ts.linaro.org
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.linaro.org%2Fmailman%2Flistinfo%2Flinaro-mm-sig&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7Ce4f3688b832842c4236e08d91bb1e148%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637571273080820910%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=lOOKD4J4h7byys2ifx0Ibn5vVr9gwZGGGsgrNmaymc4%3D&amp;reserved=0
>

Powered by blists - more mailing lists