[Mesa-dev] [PATCH] egl/wayland: break double/tripple buffering feedback loops

[Pekka Paalanen]

On Tue, 18 Dec 2018 18:59:10 +0100
Lucas Stach <l.stach at pengutronix.de> wrote:

> Am Dienstag, den 18.12.2018, 17:43 +0000 schrieb Emil Velikov:
> > > On Tue, 18 Dec 2018 at 11:16, Lucas Stach <l.stach at pengutronix.de> wrote:
> > > 
> > > Currently we dispose any unneeded color buffers immediately if we detect that
> > > there are more unlocked buffers than we need. This can lead to feedback loops
> > > between the compositor and the application causing rapid toggling between
> > > double and tripple buffering. Scenario: 2 buffers already qeued to the
> > > compositor, egl/wayland allocates a new back buffer to avoid trottling,
> > > slowing down the frame, this allows the compositor to catch up and unlock
> > > both buffers, then EGL detects that there are more buffers than currently
> > > need, freeing the buffer, restartig the loop shortly after.
> > > 
> > > To avoid wasting CPU time on rapidly freeing and reallocating color buffers
> > > break those feedback loops by letting the unneeded buffers sit around for a
> > > short while before disposing them.
> > >   
> > > > > Signed-off-by: Lucas Stach <l.stach at pengutronix.de>  
> > > ---
> > >  src/egl/drivers/dri2/platform_wayland.c | 13 +++++++++----
> > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
> > > index 34e09d7ec167..3fa08c1639d1 100644
> > > --- a/src/egl/drivers/dri2/platform_wayland.c
> > > +++ b/src/egl/drivers/dri2/platform_wayland.c

...

> > >        if (dri2_surf->back)
> > > @@ -615,10 +617,13 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
> > > 
> > >     /* If we have an extra unlocked buffer at this point, we had to do triple
> > >      * buffering for a while, but now can go back to just double buffering.
> > > -    * That means we can free any unlocked buffer now. */
> > > +    * That means we can free any unlocked buffer now. To avoid toggling between
> > > +    * going back to double buffering and needing to allocate third buffer too
> > > +    * fast we let the unneeded buffer sit around for a short while. */
> > >     for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> > >        if (!dri2_surf->color_buffers[i].locked &&
> > > -          dri2_surf->color_buffers[i].wl_buffer) {
> > > +          dri2_surf->color_buffers[i].wl_buffer &&
> > > +          dri2_surf->color_buffers[i].age > 18) {  
> > 
> > The age check here seems strange - both number used and it's relation
> > to double/triple buffering.
> > Have you considered tracking/checking how many buffers we have?  
> 
> A hysteresis value of 18 is just something that worked well in
> practice. It didn't appear to defer the buffer destruction for too long
>  while keeping the feedback loop well under control.

Hi,

it would be really nice if there was a code comment explaining where
the magic 18 comes from. There is no way to deduce it from the code
alone.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190115/a8bacf6a/attachment.sig>