On Tue, 2017-12-05 at 18:22 +0900, Tomasz Figa wrote: > On Sat, Dec 2, 2017 at 4:43 AM, Rob Herring <r...@kernel.org> wrote: > > On Fri, Dec 1, 2017 at 8:44 AM, Tomasz Figa <tf...@chromium.org> > > wrote: > > > On Fri, Dec 1, 2017 at 11:20 PM, Rob Herring <r...@kernel.org> > > > wrote: > > > > On Fri, Dec 1, 2017 at 7:30 AM, Robert Foss <robert.foss@collab > > > > ora.com> wrote: > > > > > On Thu, 2017-11-30 at 11:14 -0600, Rob Herring wrote: > > > > > > On Thu, Nov 30, 2017 at 12:11 AM, Tapani Pälli <tapani.pall > > > > > > i...@intel.co > > > > > > m> wrote: > > > > > > > > > > > > > > > > > > > > > On 11/30/2017 06:13 AM, Tomasz Figa wrote: > > > > > > > > > > > > > > > > On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss <robert.fo > > > > > > > > ss@collabo > > > > > > > > ra.com> > > > > > > > > wrote: > > > > [...] > > > > > > > > > > (As a side note, I had an idea to create a new > > > > > > > > interface, > > > > > > > > standardized > > > > > > > > by Mesa, let's say libdri_android, completely free of > > > > > > > > any > > > > > > > > gralloc-internals. It would have to be exposed > > > > > > > > additionally by > > > > > > > > any > > > > > > > > Android that intends to run Mesa. Given the need to > > > > > > > > deal with 3 > > > > > > > > different gralloc versions already, it could be > > > > > > > > something easier > > > > > > > > to > > > > > > > > manage.) > > > > > > > > > > > > > > > > > > > > > Makes sense, it is a bit messy and we have bit too much > > > > > > > patches on > > > > > > > our tree > > > > > > > because of these differences. > > > > > > > > > > > > Seems overly complicated to me. The information needed is > > > > > > within the > > > > > > ints in the native_handle in most/all implementations. I > > > > > > don't think > > > > > > there's another way to globally store dmabuf metadata > > > > > > unless you have > > > > > > a custom interface in your DRM driver. So standardizing to > > > > > > a common > > > > > > library implies a common handle struct here. I think the > > > > > > options are: > > > > > > > > > > > > - common struct definition (native_handle_t + dmabuf fd(s) > > > > > > + width, > > > > > > height, stride, format, usage, etc.) > > > > > > - common struct plus inline accessor functions > > > > > > - common opaque struct plus accessor library > > > > > > > > > > So these common parts would be much like what currently > > > > > exists in > > > > > minigbm/cros_gralloc_handle.h and > > > > > gbm_gralloc/gralloc_drm_handle.h > > > > > then, but extended with the above suggestions? > > > > > > > > Yes, but which part do you think needs to be extended? > > > > > > > > As we discussed on IRC, I think perhaps we just need to change > > > > the > > > > handle format field in gralloc_drm_handle.h to use fourcc (aka > > > > DRM and > > > > GBM formats) instead of the Android format. I think all the > > > > users just > > > > end up converting it to their own internal format anyway. > > > > > > We keep the handle opaque for consumers and even minigbm > > > dereferences > > > it only when creating/registering the buffer, further using the > > > handle > > > pointer only as a key to internal bookkeeping map. > > > > What you say implies that you don't need any metadata in the > > handle, > > but you do have pretty much all the same data. Whether you > > > > > Relying on the struct itself is not only error prone, as there is > > > no > > > way to check if the struct on gralloc implementation side matches > > > what > > > we expect, but also makes it difficult to change the handle > > > struct at > > > our convenience. > > > > How does a library solve this? > > > > Everything in Android gets built together and the handle pretty > > much > > has to stay the same across components in any implementation I've > > seen. Maybe someday that will change and we'll need versioning and > > backwards compatibility, but for now that's unnecessary complexity. > > We'd have to get to a single, well controlled and defined handle > > first > > anyway before we start versioning. > > > > Anyone is still free to change things downstream however they want. > > We're only talking about what does mainline/upstream do. > > > > > > > > Also, I don't think whatever is standardized should live in > > > > > > Mesa. > > > > > > There's a need to support drm_hwcomposer (which has the > > > > > > same > > > > > > dependencies as mesa) with non-Mesa GL implementations > > > > > > (yes, vendor > > > > > > binaries). > > > > > > > > > > Excluding Mesa and the composer leaves us with the allocator > > > > > or > > > > > creating a new library. > > > > > I would assume that creating a new library is the worse > > > > > option. > > > > > > > > Why excluding the composer? If we have to pick, I'd put it > > > > there or > > > > perhaps libdrm? > > > > > > There is neither a single central composer nor libdrm is used on > > > every > > > system... (The latter is not even used by Intel driver in Mesa > > > anymore.) > > > > I think you are confusing libdrm_intel which was removed with > > libdrm > > (the ioctl wrappers) which is still a dependency. I don't think > > there > > is any plan to remove libdrm completely. > > Okay, my bad. libdrm is used for opening and manipulating DRI device > nodes after all. > > > > > For cases where a user has different components, then they have to > > copy the struct. > > Yeah, I guess that's not much different from what we're doing in > Chromium OS with proprietary vendor components already... > > > > > > However I fully agree that there are other upstream components > > > (e.g. > > > drm_hwcomposer), which would benefit from it and nobody wants to > > > include Mesa in the build just for one header. Should we have a > > > separate freedesktop project for it? > > > > I'm still going to say libdrm. If that's really a problem, then we > > can > > split it out later. > > At least for Chromium OS, libdrm would work fine indeed.
Excellent, so with the question of where (libdrm) covered, that leaves us with what. Currently this is what what the XXX_handle structs look like: gbm_gralloc: https://github.com/robherring/gbm_gralloc/blob/master/gral loc_drm_handle.h#L36 minigbm: https://chromium.googlesource.com/chromiumos/platform/minigbm/ +/master/cros_gralloc/cros_gralloc_handle.h#20 intel-minigbm: https://github.com/intel/minigbm/blob/master/cros_grallo c/cros_gralloc_handle.h#L20 A lot seems to be shared between the 3, gbm_gralloc seems to have modifier support, but lack multi-planar YUV support. DRV_MAX_PLANES sounds to be a bit of a misnomer, as (I think) it refers to YUV planes and not planes supported by the driver/hw. I would assume that all shared variables would be available in the libdrm-struct, as well the ones planar YUV support. As for the non-obvious ones, maybe the should be listed so that we can dig into if they are really needed, implementation specific or unused. Rob.
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
