[Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED

[Robert Foss]

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 at intel.com>
> wrote:
> > Hi;
> > 
> > 
> > On 11/27/2017 04:14 PM, Robert Foss wrote:
> > > 
> > > From: Tomasz Figa <tfiga at 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/566793
> > > 
> > > Signed-off-by: Tomasz Figa <tfiga at chromium.org>
> > > Reviewed-by: Chad Versace <chadversary at chromium.org>
> > > Signed-off-by: Robert Foss <robert.foss at 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/deb323eafa321c725805a
> > 702ed19cb4983346b60
> > 
> > https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853
> > 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.

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171129/7685a87d/attachment.sig>