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.f...@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
