[Mesa-dev] [PATCH v3 08/22] egl/tizen: add support of dri2_loader (v2)

[Emil Velikov]

On 4 October 2017 at 15:28, Rob Herring <robh at kernel.org> wrote:
> On Wed, Oct 4, 2017 at 1:50 AM, Gwan-gyeong Mun <elongbug at gmail.com> wrote:
>> It adds support of dri2_loader to egl dri2 tizen backend.
>>   - referenced a basic buffer flow and management implementation from android.
>>
>> And it implements a query buffer age extesion for tizen and turn on
>> swap_buffers_with_damage extension.
>>   - it add color buffer related member variables to dri_egl_surface for a
>>     management of color buffers.
>>
>> v2: Fixes from Emil's review:
>>    a) Remove a temporary variable and return directly on get_format_bpp()
>>    b) Remove unneeded compiler pragma
>>    c) Follow coding style
>>    d) Rename get_pitch() to get_stride() for using of consistent naming
>>    e) Remove mis-referencing from android implementation on treatment of buffer
>>       age.
>>       reference: https://lists.freedesktop.org/archives/mesa-dev/2017-June/158409.html
>>    f) Use dri2_egl_surface_free_outdated_buffers_and_update_size() helper
>>    g) Use dri2_egl_surface_record_buffers_and_update_back_buffer() helper
>>    h) Use add dri2_egl_surface_update_buffer_age() helper
>>    i) Use env_var_as_boolean for hw_accel variable on dri2_initialize_tizen()
>>    j) Remove getting of the device name and opening of the device node on dri2_initialize_tizen()
>>       And add duplicating of tbm_bufmgr_fd. As tbm_bufmgr_fd is managed by tbm_bufmgr,
>>       if mesa use this fd then we should duplicate it.
>>    k) Add comments why we can not drop the dri2 codepath on dri2_initialize_tizen()
>>       As some kernels ported for tizen don't support render node feature yet,
>>       currently we cannot drop the dri2 codepath.
>>
>> Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
>> ---
>>  src/egl/drivers/dri2/egl_dri2.h       |   9 ++
>>  src/egl/drivers/dri2/platform_tizen.c | 257 ++++++++++++++++++++++++++++++++--
>>  2 files changed, 252 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
>> index 6f9d936ca5..7d047bf5dd 100644
>> --- a/src/egl/drivers/dri2/egl_dri2.h
>> +++ b/src/egl/drivers/dri2/egl_dri2.h
>> @@ -340,6 +340,15 @@ struct dri2_egl_surface
>>     tpl_surface_t         *tpl_surface;
>>     tbm_surface_h          tbm_surface;
>>     tbm_format             tbm_format;
>> +
>> +   /* Used to record all the tbm_surface created by tpl_surface and their ages.
>> +    * Usually Tizen uses at most triple buffers in tpl_surface (tbm_surface_queue)
>> +    * so hardcode the number of color_buffers to 3.
>> +    */
>> +   struct {
>> +      tbm_surface_h       buffer;
>> +      int                 age;
>> +   } color_buffers[3], *back;
>
> dri2_egl_surface is quite the mess of ifdefery.
>
> So now we have 3 instances of color_buffer and *back. This struct
> really needs some refactoring to separate out the common and platform
> specific bits. I'm not saying it has to be done as part of this series
> though.
>
The helpers refactoring was meant to address a bit of it. Namely:

   struct {
     void *native_buffer; // aka wl_buffer//gbm_bo/ANativeWindowBuffer
     __DRIimage         *dri_image;
     /* for is_different_gpu case. NULL else */
     __DRIimage         *linear_copy;
     /* for swrast */
     void *data;
     int data_size;
     bool                locked; // can we reuse it for android/tizen ?
     int                 age;
  } color_buffers[4], *back, *current;

Side note: The 4 buffers thing is a bit strange. The Wayland commit
that introduces it is sparse on details why does so.

With above in mind there will be the odd cast in the platform_foo.c
but the helpers will be perfectly fine.
Aka there should be no new ifdef magic.

-Emil