[Mesa-dev] [PATCH 1/2] loader/dri3: Use client local back to front blit in copySubBuffer if available

[Thomas Hellstrom]

Hi!

On 09/05/2017 07:36 AM, Axel Davy wrote:
> On 04/09/2017 14:27, Thomas Hellstrom wrote:
>> The copySubBuffer functionality always attempted a server side blit from
>> back to fake front if a fake front was present, and we weren't 
>> displaying
>> on a remote GPU.
>>
>> Now that we always have local blit capability on modern drivers, first
>> attempt a local blit, and only if that fails, try the server blit.
>>
>> Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
>> ---
>>   src/loader/loader_dri3_helper.c | 16 +++++++---------
>>   1 file changed, 7 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/loader/loader_dri3_helper.c 
>> b/src/loader/loader_dri3_helper.c
>> index e3120f5..c0a6e0d 100644
>> --- a/src/loader/loader_dri3_helper.c
>> +++ b/src/loader/loader_dri3_helper.c
>> @@ -635,14 +635,6 @@ loader_dri3_copy_sub_buffer(struct 
>> loader_dri3_drawable *draw,
>>                                       back->image,
>>                                       0, 0, back->width, back->height,
>>                                       0, 0, __BLIT_FLAG_FLUSH);
>> -      /* We use blit_image to update our fake front,
>> -       */
>> -      if (draw->have_fake_front)
>> -         (void) loader_dri3_blit_image(draw,
>> - dri3_fake_front_buffer(draw)->image,
>> -                                       back->image,
>> -                                       x, y, width, height,
>> -                                       x, y, __BLIT_FLAG_FLUSH);
>>      }
> Why removing that part ? It's a bit easier to read when the 
> is_different_gpu path is separated to the normal path here.

It's less code. We're avoiding duplicating the same code in two paths.

>
>> loader_dri3_swapbuffer_barrier(draw);
>> @@ -656,7 +648,13 @@ loader_dri3_copy_sub_buffer(struct 
>> loader_dri3_drawable *draw,
>>      /* Refresh the fake front (if present) after we just damaged the 
>> real
>>       * front.
>>       */
>> -   if (draw->have_fake_front && !draw->is_different_gpu) {
>> +   if (draw->have_fake_front &&
>> +       !loader_dri3_blit_image(draw,
>> + dri3_fake_front_buffer(draw)->image,
>> +                               back->image,
>> +                               x, y, width, height,
>> +                               x, y, __BLIT_FLAG_FLUSH) &&
>> +       !draw->is_different_gpu) {
>>         dri3_fence_reset(draw->conn, dri3_fake_front_buffer(draw));
>>         dri3_copy_area(draw->conn,
>>                        back->pixmap,
>
> In the case of is_different_gpu, reverting to using a server copy if 
> loader_dri3_blit_image fails doesn't seem a good choice, because what 
> the server sees is the linear copy of the buffers. You'd have to add a 
> blit from the linear copy to the tiled buffer (which won't work if 
> local blit is not available).

The server copy is never performed if is_different_gpu is true. In the 
"is_different_gpu" path, the functionality is not changed, except the 
back-to-fake-front blit is processed *after* the linear-to-real-front 
blit has been scheduled.

Since, in the is_different_gpu case, dri3 refuses to start unless we 
have local blit capability, a failure of loader_dri3_blit_image means 
that we have failed a context creation and in that case, skipping the 
back-to-fake-front blit seems reasonable to me.

Thanks,
/Thomas

>
>
> Yours,
>
>
> Axel Davy
>