<div dir="ltr">Hi Robert,<div><br></div><div>Thanks for looking into this! We need to decide if we want:</div><div><br></div><div>(1) A common struct that implementations can subclass, i.e:<br></div><div><br></div><div>struct blah_gralloc_handle {</div><div> alloc_handle_t alloc_handle;</div><div> int x, y, z;</div><div> ....</div><div>} </div><div><br></div><div>(2) An accessor library that vendors can implement, i.e:</div><div><br></div><div>struct drmAndroidHandleInfo { </div><div> uint32_t (*get_fourcc)(buffer_handle_t handle); </div><div> uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);</div><div> uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);</div><div><div> uint64_t (*get_modifier)(buffer_handle_<wbr>t handle);</div><div>};</div><div><br></div><div>From my perspective as someone who has to maintain the minigbm gralloc implementation, (2) is preferable since:</div><div><br></div><div>a) We really don't have a need for fields like data_owner, <span style="font-size:12.8px">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.</span></div><div><br></div><div>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<wbr>. Using <span style="font-size:12.8px">__attribute__((aligned(<wbr>xx))) is less portable than maintaining a POD struct.</span></div></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">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. </span></div><div><span style="font-size:12.8px"><br></span></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 13, 2017 at 9:30 AM, Robert Foss <span dir="ltr"><<a href="mailto:robert.foss@collabora.com" target="_blank">robert.foss@collabora.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This series moves {gbm,drm,cros}_gralloc_handle_<wbr>t struct to libdrm,<br>
since at least 4 implementations exist, and share a lot of contents.<br>
The idea is to keep the common stuff defined in one place, and libdrm<br>
is the common codebase to all of these platforms.<br>
<br>
Additionally, having this struct defined in libdrm will make it<br>
easier for mesa and grallocs to communicate.<br>
<br>
Curretly missing is:<br>
- Planar formats<br>
- Get/Set functions<br>
<br>
<br>
Planar formats<br>
--------------<br>
Support for planar formats is needed, but has not been added<br>
yet, mostly since this was not already implemented in {gbm,drm}_gralloc<br>
and the fact the having at least initial backwards compatability would<br>
be nice. Anonymous unions can of course be used later on to provide<br>
backwards compatability if so desired.<br>
<br>
<br>
Get/Set functions<br>
-----------------<br>
During the previous discussion[1] one suggestion was to add accessor<br>
functions. In this RFC I've only provided a alloc_handle_create()<br>
function.<br>
<br>
The Get/Set functions have not been added yet, I was hoping for some<br>
conclusive arguments for them being adeded.<br>
<br>
Lastly it was suggested by Rob Herring that having a fourcc<->android<br>
pixel format conversion function would be useful.<br>
<br>
<br>
[1] <a href="https://lists.freedesktop.org/archives/mesa-dev/2017-November/178199.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>archives/mesa-dev/2017-<wbr>November/178199.html</a><br>
<br>
Robert Foss (5):<br>
android: Move gralloc handle struct to libdrm<br>
android: Add version variable to alloc_handle_t<br>
android: Mark alloc_handle_t magic variable as const<br>
android: Remove member name from alloc_handle_t<br>
android: Change alloc_handle_t format from Android format to fourcc<br>
<br>
Android.mk | 8 +++-<br>
Makefile.sources | 3 ++<br>
android/alloc_handle.h | 87 ++++++++++++++++++++++++++++++<wbr>++++++++++++++<br>
android/gralloc_drm_handle.h | 1 +<br>
4 files changed, 97 insertions(+), 2 deletions(-)<br>
create mode 100644 android/alloc_handle.h<br>
create mode 120000 android/gralloc_drm_handle.h<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
2.14.1<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div>