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

[Gurchetan Singh]

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.


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171213/6d6bb4fb/attachment.html>