[Mesa-dev] [RFC libdrm 0/5] Move alloc_handle_t from gralloc impls.

[Rob Herring]

On Wed, Dec 13, 2017 at 5:02 PM, Gurchetan Singh
<gurchetansingh at 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.

> c) IMO creating the handle should be left to the gralloc implementation.
> Having accessor functions clearly defines what we need from libdrm -- to
> make up for shortcomings of the gralloc API for DRM/KMS use cases.
>
>
> On Wed, Dec 13, 2017 at 9:30 AM, Robert Foss <robert.foss at collabora.com>
> wrote:
>>
>> This series moves {gbm,drm,cros}_gralloc_handle_t struct to libdrm,
>> since at least 4 implementations exist, and share a lot of contents.
>> The idea is to keep the common stuff defined in one place, and libdrm
>> is the common codebase to all of these platforms.
>>
>> Additionally, having this struct defined in libdrm will make it
>> easier for mesa and grallocs to communicate.
>>
>> Curretly missing is:
>>  - Planar formats
>>  - Get/Set functions
>>
>>
>> Planar formats
>> --------------
>> Support for planar formats is needed, but has not been added
>> yet, mostly since this was not already implemented in {gbm,drm}_gralloc
>> and the fact the having at least initial backwards compatability would
>> be nice. Anonymous unions can of course be used later on to provide
>> backwards compatability if so desired.
>>
>>
>> Get/Set functions
>> -----------------
>> During the previous discussion[1] one suggestion was to add accessor
>> functions. In this RFC I've only provided a alloc_handle_create()
>> function.
>>
>> The Get/Set functions have not been added yet, I was hoping for some
>> conclusive arguments for them being adeded.
>>
>> Lastly it was suggested by Rob Herring that having a fourcc<->android
>> pixel format conversion function would be useful.
>>
>>
>> [1]
>> https://lists.freedesktop.org/archives/mesa-dev/2017-November/178199.html
>>
>> Robert Foss (5):
>>   android: Move gralloc handle struct to libdrm
>>   android: Add version variable to alloc_handle_t
>>   android: Mark alloc_handle_t magic variable as const
>>   android: Remove member name from alloc_handle_t
>>   android: Change alloc_handle_t format from Android format to fourcc
>>
>>  Android.mk                   |  8 +++-
>>  Makefile.sources             |  3 ++
>>  android/alloc_handle.h       | 87
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  android/gralloc_drm_handle.h |  1 +
>>  4 files changed, 97 insertions(+), 2 deletions(-)
>>  create mode 100644 android/alloc_handle.h
>>  create mode 120000 android/gralloc_drm_handle.h
>>
>> --
>> 2.14.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>