Hi, On Tue, 18 Dec 2018 at 17:59, Lucas Stach <[email protected]> wrote: > Am Dienstag, den 18.12.2018, 17:43 +0000 schrieb Emil Velikov: > > > On Tue, 18 Dec 2018 at 11:16, Lucas Stach <[email protected]> wrote: > > > if (dri2_surf->back == NULL) > > > dri2_surf->back = &dri2_surf->color_buffers[i]; > > > - else if (dri2_surf->back->dri_image == NULL) > > > + else if (dri2_surf->back->dri_image == NULL && > > > dri2_surf->color_buffers[i].dri_image) > > > dri2_surf->back = &dri2_surf->color_buffers[i]; > > > + age = dri2_surf->back->age; > > > } > > > > > > > AFAICT this is the wayland equivalent of > > 4f1d27a406478d405eac6f9894ccc46a80034adb > > Where the exact same logic/commit message applies. > > No it isn't. It's exactly what it says in the commit log. It's keeping > the tripple buffer around for a bit, even if we don't strictly need it > when the client is currently doing double buffering.
Right - the crucial part in Derek's GBM commit was removing the 'break' and adding the extra conditional on age. Derek's patch stabilises the age of buffers handed back to the user, by always returning the oldest available buffer. That slightly pessimises rendering if there is a 'free' buffer in the queue: if four buffers are allocated, then we will always return a buffer from three flips ago, maybe meaning more rendering work. It means that, barring the client holding on to one buffer for unexpectedly long, the age of the oldest buffer in the queue will never be greater than the queue depth. This patch instead relies on unbalanced ages, where older buffers in the queue are allowed to age far beyond the queue depth if not used during normal rendering. > When things are on the edge between double buffering being enough and > sometimes a third buffer being needed to avoid stalling we would > otherwise bounce rapidly between allocating and disposing the third > buffer. > > The DRM platform has no such optimization and just keeps the third > buffer around forever. This patch keeps the optimization in the Wayland > platform, but adds a bit of hysteresis before disposing the buffer when > going from tripple to double buffering to see if things are settling on > double buffering. Ideally we'd have globally optimal behaviour for both platforms, but that doesn't really seem doable for now. I think this is a good balance though. There will only be one GBM user at a time, so having that allocate excessive buffers doesn't seem too bad, and the penalty for doing so is your entire system stuttering as the compositor becomes blocked. Given the general stability of compositors, if they need a larger queue depth at some point, they are likely to need it again in the near future. Conversely, there may be a great many Wayland clients, and these clients may bounce between overlay and GPU composition. Given that, it seems reasonable to opportunistically free up buffers, to make sure we have enough memory available across the system. > > 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. Yeah, having this #defined with a comment above it would be nice. With that, this patch is: Reviewed-by: Daniel Stone <[email protected]> Cheers, Daniel _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
