So the plan is for alloc_handle_t to not be sub-classed by the implementations, but have all necessary information that an implementation would need?
If so, how do we reconcile the implementation specific information that is often in the handle: https://github.com/intel/minigbm/blob/master/cros_ gralloc/cros_gralloc_handle.h [consumer_usage, producer_usage, yuv_color_range, is_updated etc.] https://chromium.googlesource.com/chromiumos/platform/ minigbm/+/master/cros_gralloc/cros_gralloc_handle.h [use_flags, pixel_stride] In our case, removing our minigbm specific use flags from the handle would add complexity to our (*registerBuffer) path. On Thu, Dec 21, 2017 at 10:14 AM, Rob Herring <[email protected]> wrote: > On Wed, Dec 13, 2017 at 5:02 PM, Gurchetan Singh > <[email protected]> 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 <[email protected]> > > 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 > >> [email protected] > >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
