Re: [Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers
On Tue, Apr 23, 2019 at 7:30 PM Marek Olšák wrote:
> On Tue, Apr 23, 2019 at 4:49 PM Kenneth Graunke
> wrote:
>
>> On Monday, April 22, 2019 6:29:43 PM PDT Marek Olšák wrote:
>> > From: Marek Olšák
>> >
>> > 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, , NULL);
>> > - }
>> > + st->flush(st, flush_flags, _fence);
>> >
>> > - st->flush(st, flush_flags, );
>> > + oldest_fence = swap_fences_pop_front(drawable);
>> > + if (oldest_fence) {
>> > + screen->fence_finish(screen, NULL, oldest_fence,
>> PIPE_TIMEOUT_INFINITE);
>> > + screen->fence_reference(screen, _fence, NULL);
>> > + }
>> >
>> > - if (fence) {
>> > - swap_fences_push_back(drawable, fence);
>> > - screen->fence_reference(screen, , NULL);
>> > + if (new_fence) {
>> > + swap_fences_push_back(drawable, new_fence);
>> > + screen->fence_reference(screen, _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.
>
Actually, if it has to wait, there is already large input lag, so I guess
this patch doesn't change anything other than increasing the number. I'll
drop it.
Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers
On Tue, Apr 23, 2019 at 4:49 PM Kenneth Graunke
wrote:
> On Monday, April 22, 2019 6:29:43 PM PDT Marek Olšák wrote:
> > From: Marek Olšák
> >
> > 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, , NULL);
> > - }
> > + st->flush(st, flush_flags, _fence);
> >
> > - st->flush(st, flush_flags, );
> > + oldest_fence = swap_fences_pop_front(drawable);
> > + if (oldest_fence) {
> > + screen->fence_finish(screen, NULL, oldest_fence,
> PIPE_TIMEOUT_INFINITE);
> > + screen->fence_reference(screen, _fence, NULL);
> > + }
> >
> > - if (fence) {
> > - swap_fences_push_back(drawable, fence);
> > - screen->fence_reference(screen, , NULL);
> > + if (new_fence) {
> > + swap_fences_push_back(drawable, new_fence);
> > + screen->fence_reference(screen, _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
>
> 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.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers
On Monday, April 22, 2019 6:29:43 PM PDT Marek Olšák wrote:
> From: Marek Olšák
>
> 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, , NULL);
> - }
> + st->flush(st, flush_flags, _fence);
>
> - st->flush(st, flush_flags, );
> + oldest_fence = swap_fences_pop_front(drawable);
> + if (oldest_fence) {
> + screen->fence_finish(screen, NULL, oldest_fence,
> PIPE_TIMEOUT_INFINITE);
> + screen->fence_reference(screen, _fence, NULL);
> + }
>
> - if (fence) {
> - swap_fences_push_back(drawable, fence);
> - screen->fence_reference(screen, , NULL);
> + if (new_fence) {
> + swap_fences_push_back(drawable, new_fence);
> + screen->fence_reference(screen, _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...
The rest of the series is:
Reviewed-by: Kenneth Graunke
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.
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers
From: Marek Olšák
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, , NULL);
- }
+ st->flush(st, flush_flags, _fence);
- st->flush(st, flush_flags, );
+ oldest_fence = swap_fences_pop_front(drawable);
+ if (oldest_fence) {
+ screen->fence_finish(screen, NULL, oldest_fence,
PIPE_TIMEOUT_INFINITE);
+ screen->fence_reference(screen, _fence, NULL);
+ }
- if (fence) {
- swap_fences_push_back(drawable, fence);
- screen->fence_reference(screen, , NULL);
+ if (new_fence) {
+ swap_fences_push_back(drawable, new_fence);
+ screen->fence_reference(screen, _fence, NULL);
}
}
else if (flags & (__DRI2_FLUSH_DRAWABLE | __DRI2_FLUSH_CONTEXT)) {
st->flush(st, flush_flags, NULL);
}
if (drawable) {
drawable->flushing = FALSE;
}
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers
From: Marek Olšák
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, , NULL);
- }
+ st->flush(st, flush_flags, _fence);
- st->flush(st, flush_flags, );
+ oldest_fence = swap_fences_pop_front(drawable);
+ if (oldest_fence) {
+ screen->fence_finish(screen, NULL, oldest_fence,
PIPE_TIMEOUT_INFINITE);
+ screen->fence_reference(screen, _fence, NULL);
+ }
- if (fence) {
- swap_fences_push_back(drawable, fence);
- screen->fence_reference(screen, , NULL);
+ if (new_fence) {
+ swap_fences_push_back(drawable, new_fence);
+ screen->fence_reference(screen, _fence, NULL);
}
}
else if (flags & (__DRI2_FLUSH_DRAWABLE | __DRI2_FLUSH_CONTEXT)) {
st->flush(st, flush_flags, NULL);
}
if (drawable) {
drawable->flushing = FALSE;
}
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
