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.f...@collabora.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.pa...@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.foss@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. Best regards, Tomasz _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
