Hey Tomasz,
On 01/24/2018 11:04 AM, Tomasz Figa wrote:
Hi Robert,
On Wed, Jan 17, 2018 at 2:36 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 gralloc implementations to communicate.
Robert Foss (7):
android: Move gralloc handle struct to libdrm
android: Add version variable to gralloc_handle_t
android: Mark gralloc_handle_t magic variable as const
android: Remove member name from gralloc_handle_t
android: Change gralloc_handle_t format from Android format to fourcc
android: Change gralloc_handle_t members to be fixed width
android: Add accessor functions for gralloc_handle_t variables
Again, thanks for working on this.
I looked through the series and it seems to be much different from
what I imagined when writing my previous reply. I must have
misunderstood your proposal back then.
Ah, glad we caught it before v2 then :)
Generally, current series doesn't solve Chromium OS main concern of
locking down the handle struct. Even though accessors are added, they
are implemented in libdrm and refer to the exact handle layout as per
the handle struct defined by libdrm.
So solving the problems of multiple projects is the goal, so reconsidering is
probably they way forward.
What I had in my mind, would be creating a secondary struct,
consisting only of callbacks, which would be filled in by particular
gralloc implementation running in the system with its accessors. This
would completely eliminate any dependencies on the handle struct
itself from consumers of gralloc buffers.
So just to sketch out the solution, it would look something like this?
struct gralloc_handle_t {
uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t plane);
uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
...
} gralloc_funcs_t;
struct gralloc_handle_t {
native_handle_t base;
/* api variables */
const int magic; /* differentiate between allocator impls */
const int version; /* api version */
gralloc_funcs_t funcs;
...
} gralloc_handle_t;
For reasons of backwards compatability gralloc_handle_t should probably
contain whatever gbm_gralloc_handle_t contains now too.
Since we're going to version this struct, we can always drop extraneous
variables later.
Since we'll be able to drop variables, we could add more variables to support
the cros minigbm variables of even the intel minigbm ones.
This would be a bit high churn, but probably ease adoption.
Additionally the gralloc buffer registering mechanism doesn't exist in any of
the gralloc implementations, so being able to start out with something that
works on all platforms would be nice.
Rob.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
