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: > > > > > > > > Hey, > > > > > > > > On Tue, 2017-11-28 at 11:49 +0000, Emil Velikov wrote: > > > > > > > > > > On 28 November 2017 at 10:45, Tapani Pälli <tapani.palli@inte > > > > > l.com> > > > > > wrote: > > > > > > > > > > > > Hi; > > > > > > > > > > > > > > > > > > On 11/27/2017 04:14 PM, Robert Foss wrote: > > > > > > > > > > > > > > > > > > > > > From: Tomasz Figa <tf...@chromium.org> > > > > > > > > > > > > > > There is no API available to properly query the > > > > > > > IMPLEMENTATION_DEFINED > > > > > > > format. As a workaround we rely here on gralloc > > > > > > > allocating either > > > > > > > an arbitrary YCbCr 4:2:0 or RGBX_8888, with the latter > > > > > > > being > > > > > > > recognized > > > > > > > by lock_ycbcr failing. > > > > > > > > > > > > > > Reviewed-on: https://chromium-review.googlesource.com/566 > > > > > > > 793 > > > > > > > > > > > > > > Signed-off-by: Tomasz Figa <tf...@chromium.org> > > > > > > > Reviewed-by: Chad Versace <chadvers...@chromium.org> > > > > > > > Signed-off-by: Robert Foss <robert.f...@collabora.com> > > > > > > > --- > > > > > > > src/egl/drivers/dri2/platform_android.c | 39 > > > > > > > +++++++++++++++++++++++++++++++-- > > > > > > > 1 file changed, 37 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > diff --git a/src/egl/drivers/dri2/platform_android.c > > > > > > > b/src/egl/drivers/dri2/platform_android.c > > > > > > > index 63223e9a69..ae914d79c1 100644 > > > > > > > --- a/src/egl/drivers/dri2/platform_android.c > > > > > > > +++ b/src/egl/drivers/dri2/platform_android.c > > > > > > > @@ -59,6 +59,10 @@ static const struct droid_yuv_format > > > > > > > droid_yuv_formats[] = { > > > > > > > { HAL_PIXEL_FORMAT_YCbCr_420_888, 0, 1, > > > > > > > __DRI_IMAGE_FOURCC_YUV420 > > > > > > > }, > > > > > > > { HAL_PIXEL_FORMAT_YCbCr_420_888, 1, 1, > > > > > > > __DRI_IMAGE_FOURCC_YVU420 > > > > > > > }, > > > > > > > { HAL_PIXEL_FORMAT_YV12, 1, 1, > > > > > > > __DRI_IMAGE_FOURCC_YVU420 > > > > > > > }, > > > > > > > + /* HACK: See droid_create_image_from_prime_fd() and > > > > > > > b/32077885. */ > > > > > > > + { HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED, 0, 2, > > > > > > > __DRI_IMAGE_FOURCC_NV12 }, > > > > > > > + { HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED, 0, 1, > > > > > > > __DRI_IMAGE_FOURCC_YUV420 }, > > > > > > > + { HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED, 1, 1, > > > > > > > __DRI_IMAGE_FOURCC_YVU420 }, > > > > > > > > > > > > > > > > > > > > > > > > One alternative way would be to ask gralloc about these > > > > > > formats. On > > > > > > gralloc0 > > > > > > this would need a perform() hook and gralloc1 has > > > > > > getFormat(). This > > > > > > is how > > > > > > it is done currently on Android-IA, see following commits: > > > > > > > > > > > > https://github.com/intel/external-mesa/commit/deb323eafa321 > > > > > > c725805a > > > > > > 702ed19cb4983346b60 > > > > > > > > > > > > https://github.com/intel/external-mesa/commit/7cc01beaf540e > > > > > > 29862853 > > > > > > 561ef93c6c4e86c4c1a > > > > > > > > > > > > Do you think this approach would work with Chromium as > > > > > > well? > > > > > > > > > > > > > > > > i think the Android-IA approach looks good, although it > > > > > depends on > > > > > local gralloc0 changes. With gralloc1 on the horizon, I don't > > > > > know > > > > > how > > > > > much sense it makes to extend the predecessor. > > > > > AFAICT the patch should not cause any issues and it's nicely > > > > > documented. > > > > > > > > > > > > I had a look at the chromiumos/minigbm implementation, and it > > > > does not > > > > contain a gralloc1 implementation as far as I can see. I assume > > > > that it > > > > is available somewhere, but maybe not on a public branch. > > > > > > > > Would it be possible to make the minigbm gralloc1 impl. public? > > > > That > > > > way I could submit a patch mirroring what intel/minigbm does. > > > > > > > > If you fine folks as at Google prefer to roll it yourselves, > > > > just give > > > > me a poke. > > > > > > > > > There is no gralloc1 implementation for ChromiumOS minigbm and > > > AFAIK > > > we don't have any plans of adding one. AFAICT there is nothing we > > > would gain with it over gralloc0. > > > > > > > > > > > Those are the two options I'm seeing. > > > > > > > > As for gralloc0 support, would it be needed? > > > > > > > > > > > > > > Perhaps someone from the Google/CrOS team can assist in > > > > > making the > > > > > bug > > > > > public, although even then it might be better to focus on a > > > > > 'perfect' > > > > > gralloc1? > > > > > > > > > > IMHO the patch looks perfectly reasonable and we could merge > > > > > it even, > > > > > if we were to switch to gralloc1 in the not too distant > > > > > future ;-) > > > > > > > > > > > > Maybe doing both is reasonable. > > > > > > > > > I believe there isn't much adoption of gralloc1 in the wild. > > > Android-IA is the first I saw (might have missed something, > > > though). > > > Tapani, what was the reason for switching to gralloc1? > > > > > > Main reason was that we thought this is something Android will be > > moving in > > to (and deprecating gralloc0). But now if it's gone, it does not > > make sense > > to support it. > > > > > Could we just support gralloc0 for now in Mesa, make sure the > > > next > > > generation IAllocator/IMapper stuff suites our needs and switch > > > to it > > > later when it happens? > > For CTS/VTS compliance, AIUI, you may have to switch based on the > release a device is shipping on. > > > Yes, this sounds good to me. > > > > > (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? > > 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. On a seperate note, my immidiate concern is making the Android platform more functional, and in my mind the ^^ patch is minimally invasive and won't alter other usecases, but simply make mesa more predictable. I would like to make the bug IDs into full URLs though, but with that change in place, is the patch acceptable? 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