+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@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > >
signature.asc
Description: This is a digitally signed message part
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
