On 21 December 2017 at 18:14, Rob Herring <r...@kernel.org> wrote:
> On Wed, Dec 13, 2017 at 5:02 PM, Gurchetan Singh
> <gurchetansi...@chromium.org> wrote:
>> Hi Robert,
>>
>> Thanks for looking into this! We need to decide if we want:
>>
>> (1) A common struct that implementations can subclass, i.e:
>>
>> struct blah_gralloc_handle {
>> alloc_handle_t alloc_handle;
>> int x, y, z;
>> ....
>> }
>>
>> (2) An accessor library that vendors can implement, i.e:
>>
>> struct drmAndroidHandleInfo {
>> uint32_t (*get_fourcc)(buffer_handle_t handle);
>> uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
>> uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
>> uint64_t (*get_modifier)(buffer_handle_t handle);
>> };
>>
>> From my perspective as someone who has to maintain the minigbm gralloc
>> implementation, (2) is preferable since:
>
> Yeah, I'd prefer not to encourage 1 as the default.
>
>> a) We really don't have a need for fields like data_owner, void *data, etc.
>
> We should be able to get rid of this. It's just for tracking imports.
>
>> Also, minigbm puts per plane fds, strides, offsets into the handle.
>> Separating the information for the first plane (for the alloc_handle_t) and
>> then rest of the planes would be annoying.
>
> The plan is to add those to alloc_handle_t.
>
>> b) we can avoid the struct within a struct that happens when we subclass,
>> since alignment/padding issues often pop up during
>> serialization/de-serialization. Using __attribute__((aligned(xx))) is less
>> portable than maintaining a POD struct.
>
> Yes. Even just between 32 and 64 bit it's problematic.
>
Mostly a drive-by comment:
Iff alignment/padding is the major concern one could use something
like the wayland-egl ABI checks [1] and enforcing extra constrains.
Although in reality even without those, a similar test would be
strongly recommended. It will help greatly with versioning and ABI
stability.
Couple of important suggestions, wrt documentation:
- is the version field bidirectional (aka Vulkan style) or not
- how do things work, as various users support different version
- the macro defined in the header is the current _defined_ version of
the interface and _not_ the one _implemented_ by A/B
Thanks
Emil
[1]
https://cgit.freedesktop.org/mesa/mesa/tree/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
