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

[Emil Velikov]

On 25 November 2016 at 16:57, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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.
>
Skimming through patchwork and seems like this hasn't landed yet.

Derek does any of the above make sense ? Do we want to port this for
platform_wayland, or should we try porting the wayland approach here,
other ?

-Emil