[Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers

[Marek Olšák]

On Tue, Apr 23, 2019 at 4:49 PM Kenneth Graunke <kenneth at whitecape.org>
wrote:

> On Monday, April 22, 2019 6:29:43 PM PDT Marek Olšák wrote:
> > From: Marek Olšák <marek.olsak at amd.com>
> >
> > for better CPU-GPU parallelism
> > ---
> >  src/gallium/state_trackers/dri/dri_drawable.c | 20 +++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/dri/dri_drawable.c
> b/src/gallium/state_trackers/dri/dri_drawable.c
> > index 26bfdbecc53..c1de3bed9dd 100644
> > --- a/src/gallium/state_trackers/dri/dri_drawable.c
> > +++ b/src/gallium/state_trackers/dri/dri_drawable.c
> > @@ -555,33 +555,33 @@ dri_flush(__DRIcontext *cPriv,
> >         *
> >         * This pulls a fence off the throttling queue and waits for it
> if the
> >         * number of fences on the throttling queue has reached the
> desired
> >         * number.
> >         *
> >         * Then flushes to insert a fence at the current rendering
> position, and
> >         * pushes that fence on the queue. This requires that the
> st_context_iface
> >         * flush method returns a fence even if there are no commands to
> flush.
> >         */
> >        struct pipe_screen *screen = drawable->screen->base.screen;
> > -      struct pipe_fence_handle *fence;
> > +      struct pipe_fence_handle *oldest_fence, *new_fence = NULL;
> >
> > -      fence = swap_fences_pop_front(drawable);
> > -      if (fence) {
> > -         (void) screen->fence_finish(screen, NULL, fence,
> PIPE_TIMEOUT_INFINITE);
> > -         screen->fence_reference(screen, &fence, NULL);
> > -      }
> > +      st->flush(st, flush_flags, &new_fence);
> >
> > -      st->flush(st, flush_flags, &fence);
> > +      oldest_fence = swap_fences_pop_front(drawable);
> > +      if (oldest_fence) {
> > +         screen->fence_finish(screen, NULL, oldest_fence,
> PIPE_TIMEOUT_INFINITE);
> > +         screen->fence_reference(screen, &oldest_fence, NULL);
> > +      }
> >
> > -      if (fence) {
> > -         swap_fences_push_back(drawable, fence);
> > -         screen->fence_reference(screen, &fence, NULL);
> > +      if (new_fence) {
> > +         swap_fences_push_back(drawable, new_fence);
> > +         screen->fence_reference(screen, &new_fence, NULL);
> >        }
> >     }
> >     else if (flags & (__DRI2_FLUSH_DRAWABLE | __DRI2_FLUSH_CONTEXT)) {
> >        st->flush(st, flush_flags, NULL);
> >     }
> >
> >     if (drawable) {
> >        drawable->flushing = FALSE;
> >     }
>
> It seems like this will let us submit one more batch before throttling,
> which is a little like increasing the throttle value from 2 to 3...but
> not exactly.  I'm not sure I have an opinion on which is better...
>

"Wait then flush" adds latency (input lag) to the unflushed frame.
"Flush then wait" doesn't add latency.

Where we should go from here is a separate issue. Both 1 and 0 would work
here, but 0 currently disables the throttling.

Marek



>
> The rest of the series is:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
> It looks like iris ends up with PIPE_CAP_MAX_FRAMES_IN_FLIGHT == 0 at
> the end of the series.  It was 2 before I botched the DRM_CONF config
> when adding driconf XML support, so I'd probably like it to be that way
> again.  Feel free to just fix that in your patch that introduces the
> cap, or I can push a patch after your series lands.
>
> Thanks for cleaning this up, it's nice to have one fewer place to
> configure this sort of stuff.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190423/4b5a08ce/attachment-0001.html>