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

[Robert Foss]

+Kalyan Kondapally

On Wed, 2017-12-13 at 15:02 -0800, Gurchetan Singh 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:
> 
> a) We really don't have a need for fields like data_owner, void
> *data, etc.  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.
> 
> 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.
> 
> 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. 
> 

To me that is pretty persuasive, however I maintain 0 of these
implementations, so I'd like to get some input from Rob Herring
(gbm_gralloc) and Kalyan Kondapally (intel-minigbm) too.

The previous related discussion can be found here:
https://patchwork.freedesktop.org/patch/190406/


Rob.

> 
> On Wed, Dec 13, 2017 at 9:30 AM, Robert Foss <robert.foss at collabora.c
> om> 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/1
> > 78199.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
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171215/0bdd94f3/attachment.sig>