[Mesa-dev] [PATCH v2] gbm/drm: Pick the oldest available buffer in get_back_bo

[Emil Velikov]

On 23 November 2016 at 22:40, Derek Foreman <derekf at osg.samsung.com> wrote:
> Applications may query the back buffer age to efficiently perform
> partial updates. Generally the application will keep a fixed length
> damage history, and use this to calculate what needs to be redrawn
> based on the age of the back buffer it's about to render to.
>
> If presented with a buffer that has an age greater than the
> length of the damage history, the application will likely have
> to completely repaint the buffer.
>
> Our current buffer selection strategy is to pick the first available
> buffer without considering its age.  If an application frequently
> manages to fit within two buffers but occasionally requires a third,
> this extra buffer will almost always be old enough to fall outside
> of a reasonably long damage history, and require a full repaint.
>
> This patch changes the buffer selection behaviour to prefer the oldest
> available buffer.
>
> By selecting the oldest available buffer, the application will likely
> always be able to use its damage history, at a cost of having to
> perform slightly more work every frame.  This is an improvement if
> the cost of a full repaint is heavy, and the surface damage between
> frames is relatively small.
>
> It should be noted that since we don't currently trim our queue in
> any way, an application that briefly needs a large number of buffers
> will continue to receive older buffers than it would if it only ever
> needed two buffers.
>
> Reviewed-by: Daniel Stone <daniels at collabora.com>
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>
> The only changes are in the commit log, which hopefully better
> expresses the rationale for the change.
>
>  src/egl/drivers/dri2/platform_drm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
> index 2099314..f812ab5 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -215,13 +215,15 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
>     struct dri2_egl_display *dri2_dpy =
>        dri2_egl_display(dri2_surf->base.Resource.Display);
>     struct gbm_dri_surface *surf = dri2_surf->gbm_surf;
> +   int age = 0;
>     unsigned i;
>
>     if (dri2_surf->back == NULL) {
>        for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> -        if (!dri2_surf->color_buffers[i].locked) {
> +        if (!dri2_surf->color_buffers[i].locked &&
> +             dri2_surf->color_buffers[i].age >= age) {
>             dri2_surf->back = &dri2_surf->color_buffers[i];
> -           break;
> +           age = dri2_surf->color_buffers[i].age;
Don't we want to apply this in the platform_wayland as well ?
One thing that comes to mind is that wayland does purge the
old/unlocked buffers via update_buffers() thus the scenario mentioned
above is not so significant..

Looking at how close yet different drm and wayland codepaths are makes
me wonder if we have some subtle functionality differences or it's
mostly due to shortage of time/testing.

Thanks
Emil