On Fri, May 21, 2021 at 09:43:59AM +0200, Christian König wrote: > 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&data=04%7C01%7Cchristian.koenig%40amd.com%7Ce4f3688b832842c4236e08d91bb1e148%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637571273080820910%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=uk3Gs%2FW42BDqMuMJtujcAH5GvN8mOlDnmywK8x1I%2F0k%3D&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. It's just the rfc document, per the new rfc process: https://dri.freedesktop.org/docs/drm/gpu/rfc/ It's rather obviously not any piece of code in there, but just meant to check rough direction before we go rewrite the entire i915 execbuf frontend. -Daniel > > 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@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); > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > Linaro-mm-sig mailing list > > > > > > Linaro-mm-sig@xxxxxxxxxxxxxxxx > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.linaro.org%2Fmailman%2Flistinfo%2Flinaro-mm-sig&data=04%7C01%7Cchristian.koenig%40amd.com%7Ce4f3688b832842c4236e08d91bb1e148%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637571273080820910%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=lOOKD4J4h7byys2ifx0Ibn5vVr9gwZGGGsgrNmaymc4%3D&reserved=0 > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
