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

[Pekka Paalanen]

On Wed,  9 Nov 2016 14:57:22 -0600
Derek Foreman <derekf at osg.samsung.com> wrote:

> We find the oldest backbuffer we can, on the grounds that clients are
> only going to keep a fixed history queue, so this gives them the
> greatest chance of being able to use that queue via making sure
> the age is ~always less than the depth of the swapchain

Hi,

right, that is one side of the story.

On the other side, damage accumulates frame by frame. When you always
pick the oldest free buffer, you also maximize the accumulated damage
and need to repaint more than if you picked the youngest free buffer.

Furthermore, if you normally can do with cycling just two buffers, and
a hickup causes you to allocate up to four buffers and afterwards you
come back to the normal steady flow, you are now always cycling through
all the four buffers and repainting damage for age=4 instead of age=2.

If one was always picking the youngest free buffer, we could even have
heuristics to destroy buffers that have not been used for N swaps to
release some memory.

There is a trade-off between repainting a little more than necessary in
the normal case to mitigate the impact on hickups and making the normal
case optimized while hickups cause a heavy hit (full repaint plus
possibly even allocating a new buffer). However, in a proper compositor
I fail to see how such hickups would even occur - so you wouldn't need
to mitigate hickups but also using the oldest would not be any worse
since you never allocate extra buffers.

It would be nice to see more rationale in the commit message about why
picking the oldest is better than picking the youngest buffer. Age
greater than the length of the swapchain is only a trigger for full
repaint, just like a freshly allocated buffer is, except you skip the
cost of allocating.

My counter-proposal is probably worse, but I'd like to see an
explanation because it's not obvious.


Thanks,
pq

> Reviewed-by: Daniel Stone <daniels at collabora.com>
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>  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;
>  	 }
>        }
>     }

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