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

[Tomasz Figa]

On Tue, Nov 28, 2017 at 7:12 PM, Eric Engestrom
<eric.engestrom at imgtec.com> wrote:
> On Monday, 2017-11-27 22:00:43 +0100, Robert Foss wrote:
>> Hey Rob,
>>
>> On Mon, 2017-11-27 at 13:42 -0600, Rob Herring wrote:
>> > On Mon, Nov 27, 2017 at 8:14 AM, Robert Foss <robert.foss at collabora.c
>> > om> 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 },
>> > >  };
>> > >
>> > >  static int
>> > > @@ -90,6 +94,11 @@ get_format_bpp(int native)
>> > >
>> > >     switch (native) {
>> > >     case HAL_PIXEL_FORMAT_RGBA_8888:
>> > > +   case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
>> > > +      /*
>> > > +       * HACK: Hardcode this to RGBX_8888 as per cros_gralloc
>> > > hack.
>> >
>> > Does this work with other grallocs?
>>
>> Not necessarily, it is implementation specific.
>> The implementation specific part is lock_ycbcr() failing for non YUV
>> formats, which allows the application to disambiguate between YCbCr
>> 4:2:0 or RGBX_8888.
>>
>> >
>> > > +       * TODO: Remove this once b/32077885 is fixed.
>> >
>> > Is that a public bug? A full url would be better if so.
>>
>> Unfortunately it isn't, I maintained the link since it is better than
>> nothing. And to be clear, I don't have access to that URL either at
>> this moment.
>
> Don't have access either, but I think the URL is
> https://issuetracker.google.com/32077885
>
> I just asked #dri-devel if someone has access and can confirm, so we can
> have the URL instead.

I'm afraid we can't open access to this bug, because it mentions
various things not released publicly yet. Let me summarize the problem
myself, since that would be the only useful information for this patch
anyway.

The HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED format is an opaque format
used currently by Android frameworks for virtual display and camera
use cases (when camera HAL v3 API is used). Buffers allocated for this
format are not supposed to be accessed by CPU and are expected to be
used only inside vendor-provided components, such as camera HAL,
hwcomposer HAL, EGL/GLES, etc.

What exactly the format means is up to gralloc implementation and
chosen format must satisfy usage flags given at allocation time. In
case of virtual display, the buffer must be renderable (for EGL/GLES)
and displayable (by hwcomposer HAL or SurfaceFlinger GLES fallback,
i.e. might be used as a texture by EGL/GLES). For camera use case, the
camera must be able to capture frames to the buffer and displayable.

Initially Chromium OS used to deal with this format with a very simple
hack - hardcoding to RGBX_8888 in all the parts of the stack. It
worked fine with our HAL v1-based camera, because it used legacy
formats. After we switched some platforms to HALv3, the format became
ambiguous, because the cameras typically produce some kind of YUV
pictures, which typically is not renderable by GPUs. That's why we
needed to allow our gralloc choose the real format depending on usage
flags and make other users use some interface to query it.

Best regards,
Tomasz