Re: [Mesa-dev] ANDROID: eglCreateImageKHR missing modifiers
On 08/22/2018 07:56 PM, Emil Velikov wrote: Hi all, Pardon for dropping in so late. I've seems to have missed this. On 24 July 2018 at 14:24, Martin Fuzzey wrote: Hi Thomasz, thanks for your reply On 21/07/18 04:27, Tomasz Figa wrote: As you noticed, this adds back the dependency on gralloc handle structure. Moreover, it actually adds a dependency on the handle having a binary format compatible with gralloc_gbm_handle_t. This is not something that we can do in upstream Mesa, since there are more platforms running gralloc implementations other than gbm_gralloc. Looking at it a bit more is it really that bad? Note that there are at least three different implementations - the "original" drm_gralloc, gbm_gralloc (props to RobH) and an another one used in CrOS (or was it Android?) Chrome OS and Android Celadon use gralloc implemented in minigbm library. I think proposal from Tomasz is the best one, implement API between Mesa and gralloc to query modifiers. This makes it possible to use multiple gralloc implementations. The compile time dependency is on gralloc_handle.h which is actually part of libdrm not a specific gralloc implementation. Does mesa need to compile with old versions of libdrm? 0r is the structure going to be removed from libdrm? I think you missed gbm in gralloc_*gbm*_handle_t. The header gralloc_handle.h was added to libdrm by Robert Foss, explicitly to aim and standardise things. Unfortunately, I don't have a very good solution for this. In Chrome OS we just don't support modifiers in EGL/GLES on Android. Obviously it would be a good idea to have some way to query gralloc for the modifier, but I don't see Android providing any generic way to do it. The least evil I can think of as a makeshift for now could be an optional gralloc0 perform call that returns the modifier for given buffer. An advantage of a new perform call would be that other gralloc implementations could be extended to support it without changing their handle structure but is that really likely? I did not keep track of the gralloc_handle_t discussion, but Rob (cc'd) should have some info. From vague memory the standardized handle had means to track modifiers amongst others. That is obviously somewhat orthogonal to Android making it use of it ;-) HTH Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] ANDROID: eglCreateImageKHR missing modifiers
Hi all, Pardon for dropping in so late. I've seems to have missed this. On 24 July 2018 at 14:24, Martin Fuzzey wrote: > Hi Thomasz, > > thanks for your reply > > On 21/07/18 04:27, Tomasz Figa wrote: >> >> >> As you noticed, this adds back the dependency on gralloc handle >> structure. Moreover, it actually adds a dependency on the handle >> having a binary format compatible with gralloc_gbm_handle_t. This is >> not something that we can do in upstream Mesa, since there are more >> platforms running gralloc implementations other than gbm_gralloc. > > > Looking at it a bit more is it really that bad? > Note that there are at least three different implementations - the "original" drm_gralloc, gbm_gralloc (props to RobH) and an another one used in CrOS (or was it Android?) > The compile time dependency is on gralloc_handle.h which is actually part of > libdrm not a specific gralloc implementation. > Does mesa need to compile with old versions of libdrm? > 0r is the structure going to be removed from libdrm? > I think you missed gbm in gralloc_*gbm*_handle_t. The header gralloc_handle.h was added to libdrm by Robert Foss, explicitly to aim and standardise things. >> >> Unfortunately, I don't have a very good solution for this. In Chrome >> OS we just don't support modifiers in EGL/GLES on Android. Obviously >> it would be a good idea to have some way to query gralloc for the >> modifier, but I don't see Android providing any generic way to do it. >> >> The least evil I can think of as a makeshift for now could be an >> optional gralloc0 perform call that returns the modifier for given >> buffer. > > > An advantage of a new perform call would be that other gralloc > implementations could be extended to support it without changing their > handle structure but is that really likely? > I did not keep track of the gralloc_handle_t discussion, but Rob (cc'd) should have some info. From vague memory the standardized handle had means to track modifiers amongst others. That is obviously somewhat orthogonal to Android making it use of it ;-) HTH Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] ANDROID: eglCreateImageKHR missing modifiers
On Tue, Jul 24, 2018 at 10:25 PM Martin Fuzzey
wrote:
>
> Hi Thomasz,
>
> thanks for your reply
>
> On 21/07/18 04:27, Tomasz Figa wrote:
> >
> > As you noticed, this adds back the dependency on gralloc handle
> > structure. Moreover, it actually adds a dependency on the handle
> > having a binary format compatible with gralloc_gbm_handle_t. This is
> > not something that we can do in upstream Mesa, since there are more
> > platforms running gralloc implementations other than gbm_gralloc.
>
> Looking at it a bit more is it really that bad?
>
> The compile time dependency is on gralloc_handle.h which is actually
> part of libdrm not a specific gralloc implementation.
> Does mesa need to compile with old versions of libdrm?
> 0r is the structure going to be removed from libdrm?
>
> As you say there is also the runtime dependency on the binary format of
> the handle structure *but* checking the magic will ensure that if we run
> with a different implementation of gralloc which doesn't have the same
> magic we will gracefully fall back to not supporting modifiers, just as
> we would with a new optional gralloc perform call.
> We should probably also check the numFds, numInts field in the
> native_handle_t header to ensure that we can safely access the magic
> field to protect against any other gralloc implementations having a tiny
> handle format.
>
Alright, if we ensure that we can safely check for the right handle
format at runtime then I don't have anything against it.
Obviously it would be much better if we could have a generic way to
handle this, but given the Android way of doing things it doesn't
sound likely.
Best regards,
Tomasz
>
> >>
> >> --- a/src/egl/drivers/dri2/platform_android.c
> >> +++ b/src/egl/drivers/dri2/platform_android.c
> >> @@ -43,6 +43,8 @@
> >>#include "gralloc_drm.h"
> >>#endif /* HAVE_DRM_GRALLOC */
> >>
> >> +#include
> >> +
> >>#define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1))
> >>
> >>struct droid_yuv_format {
> >> @@ -801,6 +803,9 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp,
> >> _EGLContext *ctx,
> >> struct ANativeWindowBuffer *buf, int
> >> fd)
> >>{
> >> unsigned int pitch;
> >> + struct gralloc_gbm_handle_t *grh;
> >> + uint64_t modifier = 0;
> >> + bool have_modifier = false;
> >>
> >> if (is_yuv(buf->format)) {
> >> _EGLImage *image;
> >> @@ -829,17 +834,34 @@ droid_create_image_from_prime_fd(_EGLDisplay
> >> *disp, _EGLContext *ctx,
> >> return NULL;
> >> }
> >>
> >> - const EGLint attr_list[] = {
> >> + grh = (struct gralloc_gbm_handle_t *)buf->handle;
> >> + if (grh->magic == GRALLOC_HANDLE_MAGIC) {
> >> + modifier = grh->modifier;
> >> + have_modifier = true;
> >> + }
> >> +
> >> + EGLint attr_list[] = {
> >> EGL_WIDTH, buf->width,
> >> EGL_HEIGHT, buf->height,
> >> EGL_LINUX_DRM_FOURCC_EXT, fourcc,
> >> EGL_DMA_BUF_PLANE0_FD_EXT, fd,
> >> EGL_DMA_BUF_PLANE0_PITCH_EXT, pitch,
> >> EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
> >> + EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT, modifier & 0x,
> >> + EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT, modifier >> 32,
> >> EGL_NONE, 0
> >> };
> >>
> >> - return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list);
> >> + if (!have_modifier) {
> >> + for (int i=0; i < ARRAY_SIZE(attr_list); i+=2) {
> >> + if (attr_list[i] == EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT) {
> >> +attr_list[i] = EGL_NONE;
> >> +break;
> >> + }
> >> + }
> >> + }
> >> +
> >> + return dri2_create_image_dma_buf(disp, ctx, NULL, (const EGLint
> >> *)attr_list);
> >>}
> >>
> >> What is the preferred way of fixing this?
> > Unfortunately, I don't have a very good solution for this. In Chrome
> > OS we just don't support modifiers in EGL/GLES on Android. Obviously
> > it would be a good idea to have some way to query gralloc for the
> > modifier, but I don't see Android providing any generic way to do it.
> >
> > The least evil I can think of as a makeshift for now could be an
> > optional gralloc0 perform call that returns the modifier for given
> > buffer.
>
> An advantage of a new perform call would be that other gralloc
> implementations could be extended to support it without changing their
> handle structure but is that really likely?
>
> Regards,
>
> Martin
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] ANDROID: eglCreateImageKHR missing modifiers
Hi Thomasz,
thanks for your reply
On 21/07/18 04:27, Tomasz Figa wrote:
As you noticed, this adds back the dependency on gralloc handle
structure. Moreover, it actually adds a dependency on the handle
having a binary format compatible with gralloc_gbm_handle_t. This is
not something that we can do in upstream Mesa, since there are more
platforms running gralloc implementations other than gbm_gralloc.
Looking at it a bit more is it really that bad?
The compile time dependency is on gralloc_handle.h which is actually
part of libdrm not a specific gralloc implementation.
Does mesa need to compile with old versions of libdrm?
0r is the structure going to be removed from libdrm?
As you say there is also the runtime dependency on the binary format of
the handle structure *but* checking the magic will ensure that if we run
with a different implementation of gralloc which doesn't have the same
magic we will gracefully fall back to not supporting modifiers, just as
we would with a new optional gralloc perform call.
We should probably also check the numFds, numInts field in the
native_handle_t header to ensure that we can safely access the magic
field to protect against any other gralloc implementations having a tiny
handle format.
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -43,6 +43,8 @@
#include "gralloc_drm.h"
#endif /* HAVE_DRM_GRALLOC */
+#include
+
#define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1))
struct droid_yuv_format {
@@ -801,6 +803,9 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp,
_EGLContext *ctx,
struct ANativeWindowBuffer *buf, int fd)
{
unsigned int pitch;
+ struct gralloc_gbm_handle_t *grh;
+ uint64_t modifier = 0;
+ bool have_modifier = false;
if (is_yuv(buf->format)) {
_EGLImage *image;
@@ -829,17 +834,34 @@ droid_create_image_from_prime_fd(_EGLDisplay
*disp, _EGLContext *ctx,
return NULL;
}
- const EGLint attr_list[] = {
+ grh = (struct gralloc_gbm_handle_t *)buf->handle;
+ if (grh->magic == GRALLOC_HANDLE_MAGIC) {
+ modifier = grh->modifier;
+ have_modifier = true;
+ }
+
+ EGLint attr_list[] = {
EGL_WIDTH, buf->width,
EGL_HEIGHT, buf->height,
EGL_LINUX_DRM_FOURCC_EXT, fourcc,
EGL_DMA_BUF_PLANE0_FD_EXT, fd,
EGL_DMA_BUF_PLANE0_PITCH_EXT, pitch,
EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
+ EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT, modifier & 0x,
+ EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT, modifier >> 32,
EGL_NONE, 0
};
- return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list);
+ if (!have_modifier) {
+ for (int i=0; i < ARRAY_SIZE(attr_list); i+=2) {
+ if (attr_list[i] == EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT) {
+attr_list[i] = EGL_NONE;
+break;
+ }
+ }
+ }
+
+ return dri2_create_image_dma_buf(disp, ctx, NULL, (const EGLint
*)attr_list);
}
What is the preferred way of fixing this?
Unfortunately, I don't have a very good solution for this. In Chrome
OS we just don't support modifiers in EGL/GLES on Android. Obviously
it would be a good idea to have some way to query gralloc for the
modifier, but I don't see Android providing any generic way to do it.
The least evil I can think of as a makeshift for now could be an
optional gralloc0 perform call that returns the modifier for given
buffer.
An advantage of a new perform call would be that other gralloc
implementations could be extended to support it without changing their
handle structure but is that really likely?
Regards,
Martin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] ANDROID: eglCreateImageKHR missing modifiers
Hi Martin,
On Sat, Jul 21, 2018 at 1:01 AM Martin Fuzzey
wrote:
>
> Hi,
>
> I am testing mesa / etnaviv / gbm_gralloc under Android 8.1 on i.MX6
>
> I discovered the screen capture function was not working (it was
> producing empty buffers).
>
> The reason for this seems to be that eglCreateImageKHR() with
> EGL_NATIVE_BUFFER_ANDROID does not import the modifier information.
>
> I fixed it with the patch below, but I don't think this is the right way
> as it is adding back a dependency on the gralloc_handle structure.
As you noticed, this adds back the dependency on gralloc handle
structure. Moreover, it actually adds a dependency on the handle
having a binary format compatible with gralloc_gbm_handle_t. This is
not something that we can do in upstream Mesa, since there are more
platforms running gralloc implementations other than gbm_gralloc.
>
>
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -43,6 +43,8 @@
> #include "gralloc_drm.h"
> #endif /* HAVE_DRM_GRALLOC */
>
> +#include
> +
> #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1))
>
> struct droid_yuv_format {
> @@ -801,6 +803,9 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp,
> _EGLContext *ctx,
>struct ANativeWindowBuffer *buf, int fd)
> {
> unsigned int pitch;
> + struct gralloc_gbm_handle_t *grh;
> + uint64_t modifier = 0;
> + bool have_modifier = false;
>
> if (is_yuv(buf->format)) {
> _EGLImage *image;
> @@ -829,17 +834,34 @@ droid_create_image_from_prime_fd(_EGLDisplay
> *disp, _EGLContext *ctx,
> return NULL;
> }
>
> - const EGLint attr_list[] = {
> + grh = (struct gralloc_gbm_handle_t *)buf->handle;
> + if (grh->magic == GRALLOC_HANDLE_MAGIC) {
> + modifier = grh->modifier;
> + have_modifier = true;
> + }
> +
> + EGLint attr_list[] = {
> EGL_WIDTH, buf->width,
> EGL_HEIGHT, buf->height,
> EGL_LINUX_DRM_FOURCC_EXT, fourcc,
> EGL_DMA_BUF_PLANE0_FD_EXT, fd,
> EGL_DMA_BUF_PLANE0_PITCH_EXT, pitch,
> EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
> + EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT, modifier & 0x,
> + EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT, modifier >> 32,
> EGL_NONE, 0
> };
>
> - return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list);
> + if (!have_modifier) {
> + for (int i=0; i < ARRAY_SIZE(attr_list); i+=2) {
> + if (attr_list[i] == EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT) {
> +attr_list[i] = EGL_NONE;
> +break;
> + }
> + }
> + }
> +
> + return dri2_create_image_dma_buf(disp, ctx, NULL, (const EGLint
> *)attr_list);
> }
>
> What is the preferred way of fixing this?
Unfortunately, I don't have a very good solution for this. In Chrome
OS we just don't support modifiers in EGL/GLES on Android. Obviously
it would be a good idea to have some way to query gralloc for the
modifier, but I don't see Android providing any generic way to do it.
The least evil I can think of as a makeshift for now could be an
optional gralloc0 perform call that returns the modifier for given
buffer.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] ANDROID: eglCreateImageKHR missing modifiers
Hi,
I am testing mesa / etnaviv / gbm_gralloc under Android 8.1 on i.MX6
I discovered the screen capture function was not working (it was
producing empty buffers).
The reason for this seems to be that eglCreateImageKHR() with
EGL_NATIVE_BUFFER_ANDROID does not import the modifier information.
I fixed it with the patch below, but I don't think this is the right way
as it is adding back a dependency on the gralloc_handle structure.
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -43,6 +43,8 @@
#include "gralloc_drm.h"
#endif /* HAVE_DRM_GRALLOC */
+#include
+
#define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1))
struct droid_yuv_format {
@@ -801,6 +803,9 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp,
_EGLContext *ctx,
struct ANativeWindowBuffer *buf, int fd)
{
unsigned int pitch;
+ struct gralloc_gbm_handle_t *grh;
+ uint64_t modifier = 0;
+ bool have_modifier = false;
if (is_yuv(buf->format)) {
_EGLImage *image;
@@ -829,17 +834,34 @@ droid_create_image_from_prime_fd(_EGLDisplay
*disp, _EGLContext *ctx,
return NULL;
}
- const EGLint attr_list[] = {
+ grh = (struct gralloc_gbm_handle_t *)buf->handle;
+ if (grh->magic == GRALLOC_HANDLE_MAGIC) {
+ modifier = grh->modifier;
+ have_modifier = true;
+ }
+
+ EGLint attr_list[] = {
EGL_WIDTH, buf->width,
EGL_HEIGHT, buf->height,
EGL_LINUX_DRM_FOURCC_EXT, fourcc,
EGL_DMA_BUF_PLANE0_FD_EXT, fd,
EGL_DMA_BUF_PLANE0_PITCH_EXT, pitch,
EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
+ EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT, modifier & 0x,
+ EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT, modifier >> 32,
EGL_NONE, 0
};
- return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list);
+ if (!have_modifier) {
+ for (int i=0; i < ARRAY_SIZE(attr_list); i+=2) {
+ if (attr_list[i] == EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT) {
+ attr_list[i] = EGL_NONE;
+ break;
+ }
+ }
+ }
+
+ return dri2_create_image_dma_buf(disp, ctx, NULL, (const EGLint
*)attr_list);
}
What is the preferred way of fixing this?
Regards,
Martin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
