[Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
From: Tomasz Figa
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_, with the latter being recognized
by lock_ycbcr failing.
Reviewed-on: https://chromium-review.googlesource.com/566793
Signed-off-by: Tomasz Figa
Reviewed-by: Chad Versace
Signed-off-by: Robert Foss
---
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_:
+ case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
+ /*
+ * HACK: Hardcode this to RGBX_ as per cros_gralloc hack.
+ * TODO: Remove this once b/32077885 is fixed.
+ */
case HAL_PIXEL_FORMAT_RGBX_:
case HAL_PIXEL_FORMAT_BGRA_:
bpp = 4;
@@ -112,6 +121,11 @@ static int get_fourcc(int native)
case HAL_PIXEL_FORMAT_RGB_565: return __DRI_IMAGE_FOURCC_RGB565;
case HAL_PIXEL_FORMAT_BGRA_: return __DRI_IMAGE_FOURCC_ARGB;
case HAL_PIXEL_FORMAT_RGBA_: return __DRI_IMAGE_FOURCC_ABGR;
+ case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
+ /*
+ * HACK: Hardcode this to RGBX_ as per cros_gralloc hack.
+ * TODO: Remove this once b/32077885 is fixed.
+ */
case HAL_PIXEL_FORMAT_RGBX_: return __DRI_IMAGE_FOURCC_XBGR;
default:
_eglLog(_EGL_WARNING, "unsupported native buffer format 0x%x", native);
@@ -125,6 +139,11 @@ static int get_format(int format)
case HAL_PIXEL_FORMAT_BGRA_: return __DRI_IMAGE_FORMAT_ARGB;
case HAL_PIXEL_FORMAT_RGB_565: return __DRI_IMAGE_FORMAT_RGB565;
case HAL_PIXEL_FORMAT_RGBA_: return __DRI_IMAGE_FORMAT_ABGR;
+ case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
+ /*
+ * HACK: Hardcode this to RGBX_ as per cros_gralloc hack.
+ * TODO: Revert this once b/32077885 is fixed.
+ */
case HAL_PIXEL_FORMAT_RGBX_: return __DRI_IMAGE_FORMAT_XBGR;
default:
_eglLog(_EGL_WARNING, "unsupported native buffer format 0x%x", format);
@@ -678,6 +697,10 @@ droid_create_image_from_prime_fd_yuv(_EGLDisplay *disp,
_EGLContext *ctx,
ret = dri2_dpy->gralloc->lock_ycbcr(dri2_dpy->gralloc, buf->handle,
0, 0, 0, 0, 0, &ycbcr);
if (ret) {
+ /* HACK: See droid_create_image_from_prime_fd() and b/32077885. */
+ if (buf->format == HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED)
+ return NULL;
+
_eglLog(_EGL_WARNING, "gralloc->lock_ycbcr failed: %d", ret);
return NULL;
}
@@ -757,8 +780,20 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp,
_EGLContext *ctx,
{
unsigned int pitch;
- if (is_yuv(buf->format))
- return droid_create_image_from_prime_fd_yuv(disp, ctx, buf, fd);
+ if (is_yuv(buf->format)) {
+ _EGLImage *image;
+
+ image = droid_create_image_from_prime_fd_yuv(disp, ctx, buf, fd);
+ /*
+ * HACK: b/32077885
+ * 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_, with the latter being
recognized
+ * by lock_ycbcr failing.
+ */
+ if (image || buf->format != HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED)
+ return image;
+ }
const int fourcc = get_fourcc(buf->format);
if (fourcc == -1) {
--
2.14.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss wrote:
> Hey,
>
> On Tue, 2017-11-28 at 11:49 +, Emil Velikov wrote:
>> On 28 November 2017 at 10:45, Tapani Pälli
>> wrote:
>> > Hi;
>> >
>> >
>> > On 11/27/2017 04:14 PM, Robert Foss wrote:
>> > >
>> > > From: Tomasz Figa
>> > >
>> > > 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_, with the latter being
>> > > recognized
>> > > by lock_ycbcr failing.
>> > >
>> > > Reviewed-on: https://chromium-review.googlesource.com/566793
>> > >
>> > > Signed-off-by: Tomasz Figa
>> > > Reviewed-by: Chad Versace
>> > > Signed-off-by: Robert Foss
>> > > ---
>> > > 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.
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?
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?
(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.)
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On 11/30/2017 06:13 AM, Tomasz Figa wrote:
On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss wrote:
Hey,
On Tue, 2017-11-28 at 11:49 +, Emil Velikov wrote:
On 28 November 2017 at 10:45, Tapani Pälli
wrote:
Hi;
On 11/27/2017 04:14 PM, Robert Foss wrote:
From: Tomasz Figa
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_, with the latter being
recognized
by lock_ycbcr failing.
Reviewed-on: https://chromium-review.googlesource.com/566793
Signed-off-by: Tomasz Figa
Reviewed-by: Chad Versace
Signed-off-by: Robert Foss
---
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.
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?
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.
// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Thu, Nov 30, 2017 at 12:11 AM, Tapani Pälli wrote:
>
>
> On 11/30/2017 06:13 AM, Tomasz Figa wrote:
>>
>> On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss
>> wrote:
>>>
>>> Hey,
>>>
>>> On Tue, 2017-11-28 at 11:49 +, Emil Velikov wrote:
On 28 November 2017 at 10:45, Tapani Pälli
wrote:
>
> Hi;
>
>
> On 11/27/2017 04:14 PM, Robert Foss wrote:
>>
>>
>> From: Tomasz Figa
>>
>> 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_, with the latter being
>> recognized
>> by lock_ycbcr failing.
>>
>> Reviewed-on: https://chromium-review.googlesource.com/566793
>>
>> Signed-off-by: Tomasz Figa
>> Reviewed-by: Chad Versace
>> Signed-off-by: Robert Foss
>> ---
>>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.
>>
>>
>> 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 w
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Thu, 2017-11-30 at 11:14 -0600, Rob Herring wrote:
> On Thu, Nov 30, 2017 at 12:11 AM, Tapani Pälli m> wrote:
> >
> >
> > On 11/30/2017 06:13 AM, Tomasz Figa wrote:
> > >
> > > On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss > > ra.com>
> > > wrote:
> > > >
> > > > Hey,
> > > >
> > > > On Tue, 2017-11-28 at 11:49 +, Emil Velikov wrote:
> > > > >
> > > > > On 28 November 2017 at 10:45, Tapani Pälli > > > > l.com>
> > > > > wrote:
> > > > > >
> > > > > > Hi;
> > > > > >
> > > > > >
> > > > > > On 11/27/2017 04:14 PM, Robert Foss wrote:
> > > > > > >
> > > > > > >
> > > > > > > From: Tomasz Figa
> > > > > > >
> > > > > > > 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_, with the latter
> > > > > > > being
> > > > > > > recognized
> > > > > > > by lock_ycbcr failing.
> > > > > > >
> > > > > > > Reviewed-on: https://chromium-review.googlesource.com/566
> > > > > > > 793
> > > > > > >
> > > > > > > Signed-off-by: Tomasz Figa
> > > > > > > Reviewed-by: Chad Versace
> > > > > > > Signed-off-by: Robert Foss
> > > > > > > ---
> > > > > > >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 ;-)
> > > >
> > > >
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Fri, Dec 1, 2017 at 7:30 AM, Robert Foss 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 > m> wrote:
>> >
>> >
>> > On 11/30/2017 06:13 AM, Tomasz Figa wrote:
>> > >
>> > > On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss > > > ra.com>
>> > > wrote:
>> > > >
>> > > > Hey,
>> > > >
>> > > > On Tue, 2017-11-28 at 11:49 +, Emil Velikov wrote:
>> > > > >
>> > > > > On 28 November 2017 at 10:45, Tapani Pälli > > > > > l.com>
>> > > > > wrote:
>> > > > > >
>> > > > > > Hi;
>> > > > > >
>> > > > > >
>> > > > > > On 11/27/2017 04:14 PM, Robert Foss wrote:
>> > > > > > >
>> > > > > > >
>> > > > > > > From: Tomasz Figa
>> > > > > > >
>> > > > > > > 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_, with the latter
>> > > > > > > being
>> > > > > > > recognized
>> > > > > > > by lock_ycbcr failing.
>> > > > > > >
>> > > > > > > Reviewed-on: https://chromium-review.googlesource.com/566
>> > > > > > > 793
>> > > > > > >
>> > > > > > > Signed-off-by: Tomasz Figa
>> > > > > > > Reviewed-by: Chad Versace
>> > > > > > > Signed-off-by: Robert Foss
>> > > > > > > ---
>> > > > > > >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 perfectl
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Fri, Dec 1, 2017 at 11:20 PM, Rob Herring wrote:
> On Fri, Dec 1, 2017 at 7:30 AM, Robert Foss 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 >> m> wrote:
>>> >
>>> >
>>> > On 11/30/2017 06:13 AM, Tomasz Figa wrote:
>>> > >
>>> > > On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss >> > > ra.com>
>>> > > wrote:
>>> > > >
>>> > > > Hey,
>>> > > >
>>> > > > On Tue, 2017-11-28 at 11:49 +, Emil Velikov wrote:
>>> > > > >
>>> > > > > On 28 November 2017 at 10:45, Tapani Pälli >> > > > > l.com>
>>> > > > > wrote:
>>> > > > > >
>>> > > > > > Hi;
>>> > > > > >
>>> > > > > >
>>> > > > > > On 11/27/2017 04:14 PM, Robert Foss wrote:
>>> > > > > > >
>>> > > > > > >
>>> > > > > > > From: Tomasz Figa
>>> > > > > > >
>>> > > > > > > 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_, with the latter
>>> > > > > > > being
>>> > > > > > > recognized
>>> > > > > > > by lock_ycbcr failing.
>>> > > > > > >
>>> > > > > > > Reviewed-on: https://chromium-review.googlesource.com/566
>>> > > > > > > 793
>>> > > > > > >
>>> > > > > > > Signed-off-by: Tomasz Figa
>>> > > > > > > Reviewed-by: Chad Versace
>>> > > > > > > Signed-off-by: Robert Foss
>>> > > > > > > ---
>>> > > > > > >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
>>> > > > > mak
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Fri, Dec 1, 2017 at 8:44 AM, Tomasz Figa wrote: > On Fri, Dec 1, 2017 at 11:20 PM, Rob Herring wrote: >> On Fri, Dec 1, 2017 at 7:30 AM, Robert Foss >> 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 >>> m> wrote: > > > On 11/30/2017 06:13 AM, Tomasz Figa wrote: > > > > On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss >>> > > 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. For cases where a user has different components, then they have to copy the struct. > 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. Rob ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Sat, Dec 2, 2017 at 4:43 AM, Rob Herring wrote: > On Fri, Dec 1, 2017 at 8:44 AM, Tomasz Figa wrote: >> On Fri, Dec 1, 2017 at 11:20 PM, Rob Herring wrote: >>> On Fri, Dec 1, 2017 at 7:30 AM, Robert Foss >>> 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 m> wrote: > > > > > > On 11/30/2017 06:13 AM, Tomasz Figa wrote: > > > > > > On Thu, Nov 30, 2017 at 3:43 AM, Robert Foss > > 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 ___
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Tue, 2017-12-05 at 18:22 +0900, Tomasz Figa wrote: > On Sat, Dec 2, 2017 at 4:43 AM, Rob Herring wrote: > > On Fri, Dec 1, 2017 at 8:44 AM, Tomasz Figa > > wrote: > > > On Fri, Dec 1, 2017 at 11:20 PM, Rob Herring > > > wrote: > > > > On Fri, Dec 1, 2017 at 7:30 AM, Robert Foss > > > 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 > > > > > 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 > > > > > > > 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 > > li
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Tue, Dec 5, 2017 at 11:01 AM, Robert Foss wrote: > On Tue, 2017-12-05 at 18:22 +0900, Tomasz Figa wrote: >> On Sat, Dec 2, 2017 at 4:43 AM, Rob Herring wrote: >> > On Fri, Dec 1, 2017 at 8:44 AM, Tomasz Figa >> > wrote: >> > > On Fri, Dec 1, 2017 at 11:20 PM, Rob Herring >> > > wrote: >> > > > On Fri, Dec 1, 2017 at 7:30 AM, Robert Foss > > > > 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 > > > > > > 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 > > > > > > > > ss@collabo >> > > > > > > > ra.com> >> > > > > > > > wrote: >> > >> > [...] >> > > 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. My plan is similar to what I was thinking in the move to hwc. Move the struct as is to libdrm and get the dependencies (gbm_gralloc, drm_gralloc?, drm_hwc, mesa) switched over to use it. Then start modifying the struct contents. Here's my list: - versioning of the handle - remove .name (GEM handles. only dmabuf support) - better way to do buffer registration tracking (than a pid and ptr)? - multi-planar - switch format to fourcc (or add the fourcc format) - switch to accessor functions or library calls Rob ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Mon, Nov 27, 2017 at 8:14 AM, Robert Foss wrote:
> From: Tomasz Figa
>
> 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_, with the latter being recognized
> by lock_ycbcr failing.
>
> Reviewed-on: https://chromium-review.googlesource.com/566793
>
> Signed-off-by: Tomasz Figa
> Reviewed-by: Chad Versace
> Signed-off-by: Robert Foss
> ---
> 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_:
> + case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
> + /*
> + * HACK: Hardcode this to RGBX_ as per cros_gralloc hack.
Does this work with other grallocs?
> + * TODO: Remove this once b/32077885 is fixed.
Is that a public bug? A full url would be better if so.
Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
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 om> wrote:
> > From: Tomasz Figa
> >
> > 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_, with the latter being
> > recognized
> > by lock_ycbcr failing.
> >
> > Reviewed-on: https://chromium-review.googlesource.com/566793
> >
> > Signed-off-by: Tomasz Figa
> > Reviewed-by: Chad Versace
> > Signed-off-by: Robert Foss
> > ---
> > 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_:
> > + case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
> > + /*
> > + * HACK: Hardcode this to RGBX_ 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_.
>
> > + * 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.
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
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
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 > om> wrote:
> > > From: Tomasz Figa
> > >
> > > 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_, with the latter being
> > > recognized
> > > by lock_ycbcr failing.
> > >
> > > Reviewed-on: https://chromium-review.googlesource.com/566793
> > >
> > > Signed-off-by: Tomasz Figa
> > > Reviewed-by: Chad Versace
> > > Signed-off-by: Robert Foss
> > > ---
> > > 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_:
> > > + case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
> > > + /*
> > > + * HACK: Hardcode this to RGBX_ 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_.
>
> >
> > > + * 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.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
Hi;
On 11/27/2017 04:14 PM, Robert Foss wrote:
From: Tomasz Figa
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_, with the latter being recognized
by lock_ycbcr failing.
Reviewed-on: https://chromium-review.googlesource.com/566793
Signed-off-by: Tomasz Figa
Reviewed-by: Chad Versace
Signed-off-by: Robert Foss
---
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/deb323eafa321c725805a702ed19cb4983346b60
https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853561ef93c6c4e86c4c1a
Do you think this approach would work with Chromium as well?
};
static int
@@ -90,6 +94,11 @@ get_format_bpp(int native)
switch (native) {
case HAL_PIXEL_FORMAT_RGBA_:
+ case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
+ /*
+ * HACK: Hardcode this to RGBX_ as per cros_gralloc hack.
+ * TODO: Remove this once b/32077885 is fixed.
+ */
case HAL_PIXEL_FORMAT_RGBX_:
case HAL_PIXEL_FORMAT_BGRA_:
bpp = 4;
@@ -112,6 +121,11 @@ static int get_fourcc(int native)
case HAL_PIXEL_FORMAT_RGB_565: return __DRI_IMAGE_FOURCC_RGB565;
case HAL_PIXEL_FORMAT_BGRA_: return __DRI_IMAGE_FOURCC_ARGB;
case HAL_PIXEL_FORMAT_RGBA_: return __DRI_IMAGE_FOURCC_ABGR;
+ case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
+ /*
+ * HACK: Hardcode this to RGBX_ as per cros_gralloc hack.
+ * TODO: Remove this once b/32077885 is fixed.
+ */
case HAL_PIXEL_FORMAT_RGBX_: return __DRI_IMAGE_FOURCC_XBGR;
default:
_eglLog(_EGL_WARNING, "unsupported native buffer format 0x%x", native);
@@ -125,6 +139,11 @@ static int get_format(int format)
case HAL_PIXEL_FORMAT_BGRA_: return __DRI_IMAGE_FORMAT_ARGB;
case HAL_PIXEL_FORMAT_RGB_565: return __DRI_IMAGE_FORMAT_RGB565;
case HAL_PIXEL_FORMAT_RGBA_: return __DRI_IMAGE_FORMAT_ABGR;
+ case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
+ /*
+ * HACK: Hardcode this to RGBX_ as per cros_gralloc hack.
+ * TODO: Revert this once b/32077885 is fixed.
+ */
case HAL_PIXEL_FORMAT_RGBX_: return __DRI_IMAGE_FORMAT_XBGR;
default:
_eglLog(_EGL_WARNING, "unsupported native buffer format 0x%x", format);
@@ -678,6 +697,10 @@ droid_create_image_from_prime_fd_yuv(_EGLDisplay *disp,
_EGLContext *ctx,
ret = dri2_dpy->gralloc->lock_ycbcr(dri2_dpy->gralloc, buf->handle,
0, 0, 0, 0, 0, &ycbcr);
if (ret) {
+ /* HACK: See droid_create_image_from_prime_fd() and b/32077885. */
+ if (buf->format == HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED)
+ return NULL;
+
_eglLog(_EGL_WARNING, "gralloc->lock_ycbcr failed: %d", ret);
return NULL;
}
@@ -757,8 +780,20 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp,
_EGLContext *ctx,
{
unsigned int pitch;
- if (is_yuv(buf->format))
- return droid_create_image_from_prime_fd_yuv(disp, ctx, buf, fd);
+ if (is_yuv(buf->format)) {
+ _EGLImage *image;
+
+ image = droid_create_image_from_prime_fd_yuv(disp, ctx, buf, fd);
+ /*
+ * HACK: b/32077885
+ * 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_, with the latter being
recognized
+ * by lock_ycbcr failing.
+ */
+ if (image || buf->format != HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED)
+ return image;
+ }
const int fourcc = get_fourcc(buf->format);
if (fourcc == -1) {
_
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On 28 November 2017 at 10:45, Tapani Pälli wrote:
> Hi;
>
>
> On 11/27/2017 04:14 PM, Robert Foss wrote:
>>
>> From: Tomasz Figa
>>
>> 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_, with the latter being recognized
>> by lock_ycbcr failing.
>>
>> Reviewed-on: https://chromium-review.googlesource.com/566793
>>
>> Signed-off-by: Tomasz Figa
>> Reviewed-by: Chad Versace
>> Signed-off-by: Robert Foss
>> ---
>> 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/deb323eafa321c725805a702ed19cb4983346b60
>
> https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853561ef93c6c4e86c4c1a
>
> 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.
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 ;-)
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Tue, Nov 28, 2017 at 8:49 PM, Emil Velikov wrote:
> On 28 November 2017 at 10:45, Tapani Pälli wrote:
>> Hi;
>>
>>
>> On 11/27/2017 04:14 PM, Robert Foss wrote:
>>>
>>> From: Tomasz Figa
>>>
>>> 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_, with the latter being recognized
>>> by lock_ycbcr failing.
>>>
>>> Reviewed-on: https://chromium-review.googlesource.com/566793
>>>
>>> Signed-off-by: Tomasz Figa
>>> Reviewed-by: Chad Versace
>>> Signed-off-by: Robert Foss
>>> ---
>>> 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/deb323eafa321c725805a702ed19cb4983346b60
>>
>> https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853561ef93c6c4e86c4c1a
>>
>> 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.
>
> 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 ;-)
Looks very similar to my earlier internal proposal and should work for
us to, but need to test to make sure.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Tue, Nov 28, 2017 at 7:12 PM, Eric Engestrom
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 > > om> wrote:
>> > > From: Tomasz Figa
>> > >
>> > > 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_, with the latter being
>> > > recognized
>> > > by lock_ycbcr failing.
>> > >
>> > > Reviewed-on: https://chromium-review.googlesource.com/566793
>> > >
>> > > Signed-off-by: Tomasz Figa
>> > > Reviewed-by: Chad Versace
>> > > Signed-off-by: Robert Foss
>> > > ---
>> > > 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_:
>> > > + case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
>> > > + /*
>> > > + * HACK: Hardcode this to RGBX_ 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_.
>>
>> >
>> > > + * 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_ 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Tuesday, 2017-11-28 21:59:52 +0900, Tomasz Figa wrote:
> On Tue, Nov 28, 2017 at 7:12 PM, Eric Engestrom
> 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 >> > om> wrote:
> >> > > From: Tomasz Figa
> >> > >
> >> > > 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_, with the latter being
> >> > > recognized
> >> > > by lock_ycbcr failing.
> >> > >
> >> > > Reviewed-on: https://chromium-review.googlesource.com/566793
> >> > >
> >> > > Signed-off-by: Tomasz Figa
> >> > > Reviewed-by: Chad Versace
> >> > > Signed-off-by: Robert Foss
> >> > > ---
> >> > > 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_:
> >> > > + case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:
> >> > > + /*
> >> > > + * HACK: Hardcode this to RGBX_ 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_.
> >>
> >> >
> >> > > + * 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.
I didn't mean access to the bug should be open, but that the actual URL
should be there (instead of the bug ID) so that people can have a look,
which is useful for those who do have access, and which lets those who
don't have access see that right away instead of searching where the bug
with that ID is located :)
> Let me summarize the problem
> myself, since that would be the only useful information for this patch
> anyway.
Indeed; thanks for the explanation. As I'm assuming you were going to
do, this would be useful in the commit message.
>
> 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_ 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, beca
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Tue, Nov 28, 2017 at 5:49 AM, Emil Velikov wrote:
> On 28 November 2017 at 10:45, Tapani Pälli wrote:
>> Hi;
>>
>>
>> On 11/27/2017 04:14 PM, Robert Foss wrote:
[...]
>>> + /* 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/deb323eafa321c725805a702ed19cb4983346b60
>>
>> https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853561ef93c6c4e86c4c1a
>>
>> 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.
>
> 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 ;-)
gralloc1 has already come and gone. The interface is now (in O+)
IAllocator and IMapper aka gralloc 2.0. There is neither perform nor
getFormat(). Seemed to me gralloc1 was moving in the right direction,
but I guess making cross process calls to retrieve buffer metadata was
not a good design. Other than standardizing the native_handle_t
fields, I'm not sure how one would solve this in a gralloc2 world.
Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Tue, Nov 28, 2017 at 11:27 PM, Rob Herring wrote:
> On Tue, Nov 28, 2017 at 5:49 AM, Emil Velikov
> wrote:
>> On 28 November 2017 at 10:45, Tapani Pälli wrote:
>>> Hi;
>>>
>>>
>>> On 11/27/2017 04:14 PM, Robert Foss wrote:
>
> [...]
>
+ /* 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/deb323eafa321c725805a702ed19cb4983346b60
>>>
>>> https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853561ef93c6c4e86c4c1a
>>>
>>> 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.
>>
>> 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 ;-)
>
> gralloc1 has already come and gone. The interface is now (in O+)
> IAllocator and IMapper aka gralloc 2.0.
Oh, that was a new one for me. Do you have any idea about real world
adoption of this interface? We're still using gralloc0 in Chrome OS
Android.
> There is neither perform nor
> getFormat(). Seemed to me gralloc1 was moving in the right direction,
> but I guess making cross process calls to retrieve buffer metadata was
> not a good design. Other than standardizing the native_handle_t
> fields, I'm not sure how one would solve this in a gralloc2 world.
Well, the ideal solution would be to extend IMapper in next version of
the interface to have a method that gives us the data we need.
If not, one could still do some hacks like a library exporting some
semi-standard functions which take a handle as an argument and provide
the data we need as a result.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On Tue, Nov 28, 2017 at 8:42 AM, Tomasz Figa wrote:
> On Tue, Nov 28, 2017 at 11:27 PM, Rob Herring wrote:
>> On Tue, Nov 28, 2017 at 5:49 AM, Emil Velikov
>> wrote:
>>> On 28 November 2017 at 10:45, Tapani Pälli wrote:
Hi;
On 11/27/2017 04:14 PM, Robert Foss wrote:
>>
>> [...]
>>
> + /* 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/deb323eafa321c725805a702ed19cb4983346b60
https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853561ef93c6c4e86c4c1a
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.
>>>
>>> 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 ;-)
>>
>> gralloc1 has already come and gone. The interface is now (in O+)
>> IAllocator and IMapper aka gralloc 2.0.
>
> Oh, that was a new one for me. Do you have any idea about real world
> adoption of this interface? We're still using gralloc0 in Chrome OS
> Android.
Pretty sure everyone else is too and will be until something forces
them to move. Maybe P will be stricter and not allow pass-thru
implementations (though I don't see how you could ever binderize
mmap).
>> There is neither perform nor
>> getFormat(). Seemed to me gralloc1 was moving in the right direction,
>> but I guess making cross process calls to retrieve buffer metadata was
>> not a good design. Other than standardizing the native_handle_t
>> fields, I'm not sure how one would solve this in a gralloc2 world.
>
> Well, the ideal solution would be to extend IMapper in next version of
> the interface to have a method that gives us the data we need.
Vendors can extend interfaces. AIUI, the rule is the HALs have to work
with stock AOSP without the extensions.
> If not, one could still do some hacks like a library exporting some
> semi-standard functions which take a handle as an argument and provide
> the data we need as a result.
You can still have private interfaces amongst vendor components. We
can also just standardize how we sub-class the native_handle. That was
my intention with moving it from gralloc to drm_hwc. We can debate
whether we expose the struct or helper functions.
Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On 11/28/2017 07:38 PM, Rob Herring wrote:
On Tue, Nov 28, 2017 at 8:42 AM, Tomasz Figa wrote:
On Tue, Nov 28, 2017 at 11:27 PM, Rob Herring wrote:
On Tue, Nov 28, 2017 at 5:49 AM, Emil Velikov wrote:
On 28 November 2017 at 10:45, Tapani Pälli wrote:
Hi;
On 11/27/2017 04:14 PM, Robert Foss wrote:
[...]
+ /* 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/deb323eafa321c725805a702ed19cb4983346b60
https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853561ef93c6c4e86c4c1a
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.
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 ;-)
gralloc1 has already come and gone. The interface is now (in O+)
IAllocator and IMapper aka gralloc 2.0.
I heard about those but havent seen the actual API so I thought this
will just be the same old gralloc but via binder.
Oh, that was a new one for me. Do you have any idea about real world
adoption of this interface? We're still using gralloc0 in Chrome OS
Android.
Pretty sure everyone else is too and will be until something forces
them to move. Maybe P will be stricter and not allow pass-thru
implementations (though I don't see how you could ever binderize
mmap).
There is neither perform nor
getFormat(). Seemed to me gralloc1 was moving in the right direction,
but I guess making cross process calls to retrieve buffer metadata was
not a good design. Other than standardizing the native_handle_t
fields, I'm not sure how one would solve this in a gralloc2 world.
Well, the ideal solution would be to extend IMapper in next version of
the interface to have a method that gives us the data we need.
Vendors can extend interfaces. AIUI, the rule is the HALs have to work
with stock AOSP without the extensions.
If default API is not enough IMO would be good to define some set of
functions that are required when using Mesa driver.
If not, one could still do some hacks like a library exporting some
semi-standard functions which take a handle as an argument and provide
the data we need as a result.
You can still have private interfaces amongst vendor components. We
can also just standardize how we sub-class the native_handle. That was
my intention with moving it from gralloc to drm_hwc. We can debate
whether we expose the struct or helper functions.
Rob
// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
Hey,
On Tue, 2017-11-28 at 11:49 +, Emil Velikov wrote:
> On 28 November 2017 at 10:45, Tapani Pälli
> wrote:
> > Hi;
> >
> >
> > On 11/27/2017 04:14 PM, Robert Foss wrote:
> > >
> > > From: Tomasz Figa
> > >
> > > 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_, with the latter being
> > > recognized
> > > by lock_ycbcr failing.
> > >
> > > Reviewed-on: https://chromium-review.googlesource.com/566793
> > >
> > > Signed-off-by: Tomasz Figa
> > > Reviewed-by: Chad Versace
> > > Signed-off-by: Robert Foss
> > > ---
> > > 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.
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
Re: [Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED
On 29.11.2017 20:43, Robert Foss wrote:
Hey,
On Tue, 2017-11-28 at 11:49 +, Emil Velikov wrote:
On 28 November 2017 at 10:45, Tapani Pälli
wrote:
Hi;
On 11/27/2017 04:14 PM, Robert Foss wrote:
From: Tomasz Figa
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_, with the latter being
recognized
by lock_ycbcr failing.
Reviewed-on: https://chromium-review.googlesource.com/566793
Signed-off-by: Tomasz Figa
Reviewed-by: Chad Versace
Signed-off-by: Robert Foss
---
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.
Android-IA has one here:
https://github.com/intel/minigbm/tree/master/cros_gralloc
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.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
