Heya,
On 12/22/17 1:09 PM, Tomasz Figa wrote:
On Fri, Dec 22, 2017 at 10:09 AM, Gurchetan Singh
<gurchetansi...@chromium.org> wrote:
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 <r...@kernel.org> wrote:
On Wed, Dec 13, 2017 at 5:02 PM, Gurchetan Singh
<gurchetansi...@chromium.org> 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.
So I had a look into implementing this, and using function pointers is
problematic due to this struct being passed between processes which would
prevent mesa calling a function assigned in gbm_gralloc for example.
It could be used to provide runtime support for multiple gralloc
implementations, but that seems to about the only advantage to adding FPs.
Am I missing a good usecase for FPs? If not I think the added complexity/cruft
is more problematic than good.
Any thoughts?
Rob.
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.
I think there might be also d). Define a standard struct in libdrm
headers and add a custom call to gralloc that would fill in such
struct for given buffer. This would keep the libdrm handle independent
of gralloc's internal handle.
P.S. Top-posting is bad.
Best regards,
Tomasz
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
