Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-29 Thread Gurchetan Singh
Thanks for the review, Emil.  If we want to use swrast we'll probably need
to implement a new swrast version of the image extension.  We can't reuse
the functions from dri2.c, even for version 1 of the extension
(createImageFromName, createImageFromRenderbuffer etc), since they require
definitions from dri_driver.h (like struct winsys_handle).

However, we would like to get around the the constant re-implementation and
code movement, and use kms_swrast if possible.  Can you outline some the
challenges and possible solutions to using kms_swrast with platform_x11?
We could invest the time in making it work if it is achievable ...

On Wed, Jun 28, 2017 at 8:24 AM, Emil Velikov 
wrote:

> On 9 June 2017 at 01:28, gurchetansi...@chromium.org
>  wrote:
> > From: Gurchetan Singh 
> >
> > Otherwise, this extension is not visible to the EGL user
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_
> dri2.c
> > index 7175e827c9..9e845e99e3 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -429,6 +429,7 @@ static const struct dri2_extension_match
> swrast_driver_extensions[] = {
> >
> >  static const struct dri2_extension_match swrast_core_extensions[] = {
> > { __DRI_TEX_BUFFER, 2, offsetof(struct dri2_egl_display, tex_buffer)
> },
> > +   { __DRI_IMAGE, 1, offsetof(struct dri2_egl_display, image) },
> Considering the entry points used, check the exact _DRI_IMAGE version
> needed (see dri_interface.h) and update accordingly.
>
> Also move the line to optional_core_extensions, since by using
> swrast_core_extensions you will fail to load older swrast_dri.so which
> are otherwise perfectly capable of working (albeit w/o said EGL
> extensions).
>
> Before you ask - yes, extension is present in dri2_core_extensions, so
> doing a second check is sub-optimal/strange.
> Do add a comment clearing any ambiguities.
>
> With that said - I'm back to addressing all the issues I've saw in libEGL
> ;-)
>
> Thanks
> Emil
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-28 Thread Emil Velikov
On 9 June 2017 at 01:28, gurchetansi...@chromium.org
 wrote:
> From: Gurchetan Singh 
>
> Otherwise, this extension is not visible to the EGL user
> ---
>  src/egl/drivers/dri2/egl_dri2.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 7175e827c9..9e845e99e3 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -429,6 +429,7 @@ static const struct dri2_extension_match 
> swrast_driver_extensions[] = {
>
>  static const struct dri2_extension_match swrast_core_extensions[] = {
> { __DRI_TEX_BUFFER, 2, offsetof(struct dri2_egl_display, tex_buffer) },
> +   { __DRI_IMAGE, 1, offsetof(struct dri2_egl_display, image) },
Considering the entry points used, check the exact _DRI_IMAGE version
needed (see dri_interface.h) and update accordingly.

Also move the line to optional_core_extensions, since by using
swrast_core_extensions you will fail to load older swrast_dri.so which
are otherwise perfectly capable of working (albeit w/o said EGL
extensions).

Before you ask - yes, extension is present in dri2_core_extensions, so
doing a second check is sub-optimal/strange.
Do add a comment clearing any ambiguities.

With that said - I'm back to addressing all the issues I've saw in libEGL ;-)

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-28 Thread Emil Velikov
On 28 June 2017 at 11:17, Emil Velikov  wrote:
> Hi Gurchetan,
>
> Pardon for the delay.
>
> On 22 June 2017 at 00:40, Gurchetan Singh  wrote:
>> Emil,
>>
>> If I understand you correctly, you're proposing to add the ability to use
>> the kms_swrast driver in platform_x11.c (the host is a standard Ubuntu box
>> for the emulator use case, not CrOS) alongside swrast.
>>
>> In that case, we would need to:
>>
>> 1) Have a dri2_initialize_x11_kms_swrast function that's called when some
>> environment variable is set instead of dri2_initialize_x11_swrast.
> There is the obvious caveat that LIBGL_ALWAYS_SOFTWARE should be set.
>
> Other than that - we can try kms_swrast and fallback to swrast.
> If the former is very incomplete relative to the latter then we might
> consider adding another envvar.
>
>> 2) dri2_initialize_x11_kms_swrast would need access to the host card fd
>> (dri_kms_init_screen requires this) and call dri2_load_driver instead of
>> dri2_load_driver_swrast .
> Indeed - in the GBM case, we get that from the user. In here, we
> _should_ be able to use the existing one passed from X. See below for
> more.
>
>> 3) Use dri2_loader_extensions instead of swrast_loader_extensions,
>> dri2_x11_display_vtbl instead dri2_x11_swrast_display_vtbl etc.
>>
> Correct again.
>
>> I'm having trouble getting this to work, and I was wondering if what I'm
>> trying to do is what you want.  Attached is the patch I'm trying (it
>> compiles, but will crash your display).
>>
> Please keep patches a) inline and b) functional changes separate from
> code movement.
> Fishing for ~5 diff in a 100 line patch isn't fun :-\
>
> Re the issue:
>  - opening card0 without authenticating is a serious no-go
>  - at the same time we shouldn't need the open - reuse the fd, as
> mentioned above.
>  - if above does not work use the renderD for the given fd ->
> open(drmGetRenderDeviceNameFromFd(fd)...)
>
Upon second thought - I'm not too sure if my original suggestion may
not work without some serious work.

Which seemingly is not required for EGL_KHR_gl_image - lucky me got
mislead by more missing checks out our codebase :-\

Some background:
The __DRI_IMAGE extension adds the following
.createImageFromName
.createImageFromRenderbuffer
.destroyImage
.createImage
.queryImage
.dupImage
.validateUsage
.createImageFromNames
.fromPlanar
.createImageFromTexture
.createImageFromFds
.createImageFromDmaBufs
.blitImage
.getCapabilities
.mapImage
.unmapImage


Of which createImageFromTexture and createImageFromRenderbuffer seems
to be required for  KHR_gl_texture_*_image and
KHR_gl_renderbuffer_image respectively (see dri2_create_image_khr).


Although overall implementation-wise _DRI_IMAGE is as follows:

For software gallium drivers:
- MESA_drm_image, MESA_image_dma_buf_export are not possible since they require
a) fd set to -1, and required for the drmGetCap() ioctl
b) .queryImage is not implemented
resource_get_handle -> {softpipe,llvmpipe} displaytarget_get_handle ->
sw/dri - NOOP, sw/kms-dri fully implemented
c) .createImageFrom{Name,Fds,DmaBuf,DmaBufs2}
resource_from_handle -> {softpipe,llvmpipe} displaytarget_from_handle
-> sw/dri - NOOP, sw/kms-dri fully implemented

Overall st/dri:
- KHR_gl_renderbuffer_image although advertised does not work
a) .createImageFromRenderbuffer is a stub (see
dri2_create_image_from_renderbuffer) :-\


Tl;Dr; your initial approach may work with some minor changes -
comments coming shortly.
There's handful of bugs in Mesa we need to address as well.

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-28 Thread Emil Velikov
Hi Gurchetan,

Pardon for the delay.

On 22 June 2017 at 00:40, Gurchetan Singh  wrote:
> Emil,
>
> If I understand you correctly, you're proposing to add the ability to use
> the kms_swrast driver in platform_x11.c (the host is a standard Ubuntu box
> for the emulator use case, not CrOS) alongside swrast.
>
> In that case, we would need to:
>
> 1) Have a dri2_initialize_x11_kms_swrast function that's called when some
> environment variable is set instead of dri2_initialize_x11_swrast.
There is the obvious caveat that LIBGL_ALWAYS_SOFTWARE should be set.

Other than that - we can try kms_swrast and fallback to swrast.
If the former is very incomplete relative to the latter then we might
consider adding another envvar.

> 2) dri2_initialize_x11_kms_swrast would need access to the host card fd
> (dri_kms_init_screen requires this) and call dri2_load_driver instead of
> dri2_load_driver_swrast .
Indeed - in the GBM case, we get that from the user. In here, we
_should_ be able to use the existing one passed from X. See below for
more.

> 3) Use dri2_loader_extensions instead of swrast_loader_extensions,
> dri2_x11_display_vtbl instead dri2_x11_swrast_display_vtbl etc.
>
Correct again.

> I'm having trouble getting this to work, and I was wondering if what I'm
> trying to do is what you want.  Attached is the patch I'm trying (it
> compiles, but will crash your display).
>
Please keep patches a) inline and b) functional changes separate from
code movement.
Fishing for ~5 diff in a 100 line patch isn't fun :-\

Re the issue:
 - opening card0 without authenticating is a serious no-go
 - at the same time we shouldn't need the open - reuse the fd, as
mentioned above.
 - if above does not work use the renderD for the given fd ->
open(drmGetRenderDeviceNameFromFd(fd)...)


> Regarding the issues with the emulator, I filed a bug based your comments
> and the emulator team has started looking at it (see
> https://android-review.googlesource.com/#/c/418541/).
>
Great, glad to hear that it is getting resolved.

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-26 Thread Gurchetan Singh
Ping...

On Wed, Jun 21, 2017 at 4:40 PM, Gurchetan Singh <
gurchetansi...@chromium.org> wrote:

> Emil,
>
> If I understand you correctly, you're proposing to add the ability to use
> the kms_swrast driver in platform_x11.c (the host is a standard Ubuntu box
> for the emulator use case, not CrOS) alongside swrast.
>
> In that case, we would need to:
>
> 1) Have a dri2_initialize_x11_kms_swrast function that's called when some
> environment variable is set instead of dri2_initialize_x11_swrast.
> 2) dri2_initialize_x11_kms_swrast would need access to the host card fd
> (dri_kms_init_screen requires this) and call dri2_load_driver instead of
> dri2_load_driver_swrast .
> 3) Use dri2_loader_extensions instead of swrast_loader_extensions,
> dri2_x11_display_vtbl instead dri2_x11_swrast_display_vtbl etc.
>
> I'm having trouble getting this to work, and I was wondering if what I'm
> trying to do is what you want.  Attached is the patch I'm trying (it
> compiles, but will crash your display).
>
> Regarding the issues with the emulator, I filed a bug based your comments
> and the emulator team has started looking at it (see
> https://android-review.googlesource.com/#/c/418541/).
>
>
> On Tue, Jun 20, 2017 at 1:19 AM, Emil Velikov 
> wrote:
>
>> On 19 June 2017 at 20:46, Chad Versace  wrote:
>> > On Thu 15 Jun 2017, Gurchetan Singh wrote:
>> >> Emil, would you be fine with leaving the image extension in dri2.c but
>> still
>> >> adding it as a drisw extension?  That solution would look like:
>> >>
>> >> [1]https://patchwork.freedesktop.org/patch/154807/
>> >
>> > Observations:
>> > - src/gallium/state_trackers/dri/dri2.c:dri2ImageExtension
>> advertises v15 of __DRI_IMAGE.
>> > - egl_dri2.c requires only v1 of __DRI_IMAGE. Maybe a higher version
>> >   is required in practive, but the egl_dri2.c code checks only for
>> v1.
>> >
>> > Questions:
>> > 1. All functions implemented in dri2.c:dri2ImageExtensions, do they
>> >under swrast? Honest question, because I'm no expert on
>> >gallium.
>> >
>> > If question #1 is true, then I see no problem with your latest plan. But
>> > maybe Emil does.
>> >
>> > If question #1 is false, it should be straightforward to implement in
>> > drisw.c the small subset of __DRI_IMAGE functions required for v1.
>>
>> While I haven't checked how much [or well] DRI_IMAGE works with
>> swrast, there's no need to actually add it there.
>> An alternative is to add kms_swrast support for EGL like we already do
>> for GBM, as mentioned earlier [1].
>>
>> Gents, keep in mind that:
>>  - one cannot pull DRM specifics (dri2.c) code within drisw.c, and
>>  - DRI_IMAGE pulls DRM specifics, hence adding it into drisw.c is
>> again a no-go :-\
>>
>> FWIW the above architectural split applies for classic drivers as
>> well. swrast_dri.so simply cannot depend on anything DRM related.
>>
>> -Emil
>>
>> [1] https://lists.freedesktop.org/archives/mesa-dev/2017-June/159519.html
>>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-21 Thread Gurchetan Singh
Emil,

If I understand you correctly, you're proposing to add the ability to use
the kms_swrast driver in platform_x11.c (the host is a standard Ubuntu box
for the emulator use case, not CrOS) alongside swrast.

In that case, we would need to:

1) Have a dri2_initialize_x11_kms_swrast function that's called when some
environment variable is set instead of dri2_initialize_x11_swrast.
2) dri2_initialize_x11_kms_swrast would need access to the host card fd
(dri_kms_init_screen requires this) and call dri2_load_driver instead of
dri2_load_driver_swrast .
3) Use dri2_loader_extensions instead of swrast_loader_extensions,
dri2_x11_display_vtbl instead dri2_x11_swrast_display_vtbl etc.

I'm having trouble getting this to work, and I was wondering if what I'm
trying to do is what you want.  Attached is the patch I'm trying (it
compiles, but will crash your display).

Regarding the issues with the emulator, I filed a bug based your comments
and the emulator team has started looking at it (see
https://android-review.googlesource.com/#/c/418541/).


On Tue, Jun 20, 2017 at 1:19 AM, Emil Velikov 
wrote:

> On 19 June 2017 at 20:46, Chad Versace  wrote:
> > On Thu 15 Jun 2017, Gurchetan Singh wrote:
> >> Emil, would you be fine with leaving the image extension in dri2.c but
> still
> >> adding it as a drisw extension?  That solution would look like:
> >>
> >> [1]https://patchwork.freedesktop.org/patch/154807/
> >
> > Observations:
> > - src/gallium/state_trackers/dri/dri2.c:dri2ImageExtension
> advertises v15 of __DRI_IMAGE.
> > - egl_dri2.c requires only v1 of __DRI_IMAGE. Maybe a higher version
> >   is required in practive, but the egl_dri2.c code checks only for
> v1.
> >
> > Questions:
> > 1. All functions implemented in dri2.c:dri2ImageExtensions, do they
> >under swrast? Honest question, because I'm no expert on
> >gallium.
> >
> > If question #1 is true, then I see no problem with your latest plan. But
> > maybe Emil does.
> >
> > If question #1 is false, it should be straightforward to implement in
> > drisw.c the small subset of __DRI_IMAGE functions required for v1.
>
> While I haven't checked how much [or well] DRI_IMAGE works with
> swrast, there's no need to actually add it there.
> An alternative is to add kms_swrast support for EGL like we already do
> for GBM, as mentioned earlier [1].
>
> Gents, keep in mind that:
>  - one cannot pull DRM specifics (dri2.c) code within drisw.c, and
>  - DRI_IMAGE pulls DRM specifics, hence adding it into drisw.c is
> again a no-go :-\
>
> FWIW the above architectural split applies for classic drivers as
> well. swrast_dri.so simply cannot depend on anything DRM related.
>
> -Emil
>
> [1] https://lists.freedesktop.org/archives/mesa-dev/2017-June/159519.html
>
From cf984192dba114a91630f5d9fb6cf46061e64d68 Mon Sep 17 00:00:00 2001
From: Gurchetan Singh 
Date: Mon, 19 Jun 2017 16:09:09 -0700
Subject: [PATCH] egl/dri2: Add kms_swrast in platform_x11

---
 src/egl/drivers/dri2/platform_x11.c | 100 +++-
 1 file changed, 53 insertions(+), 47 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
index 95e560a32a..19e4b0c5ca 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1224,52 +1224,6 @@ disconnect:
return _eglError(EGL_BAD_ALLOC, msg);
 }
 
-static EGLBoolean
-dri2_initialize_x11_swrast(_EGLDriver *drv, _EGLDisplay *disp)
-{
-   struct dri2_egl_display *dri2_dpy;
-
-   dri2_dpy = calloc(1, sizeof *dri2_dpy);
-   if (!dri2_dpy)
-  return _eglError(EGL_BAD_ALLOC, "eglInitialize");
-
-   dri2_dpy->fd = -1;
-   if (!dri2_get_xcb_connection(drv, disp, dri2_dpy))
-  goto cleanup;
-
-   /*
-* Every hardware driver_name is set using strdup. Doing the same in
-* here will allow is to simply free the memory at dri2_terminate().
-*/
-   dri2_dpy->driver_name = strdup("swrast");
-   if (!dri2_load_driver_swrast(disp))
-  goto cleanup;
-
-   dri2_dpy->loader_extensions = swrast_loader_extensions;
-
-   if (!dri2_create_screen(disp))
-  goto cleanup;
-
-   if (!dri2_setup_extensions(disp))
-  goto cleanup;
-
-   dri2_setup_screen(disp);
-
-   if (!dri2_x11_add_configs_for_visuals(dri2_dpy, disp, true))
-  goto cleanup;
-
-   /* Fill vtbl last to prevent accidentally calling virtual function during
-* initialization.
-*/
-   dri2_dpy->vtbl = _x11_swrast_display_vtbl;
-
-   return EGL_TRUE;
-
- cleanup:
-   dri2_display_destroy(disp);
-   return EGL_FALSE;
-}
-
 static void
 dri2_x11_setup_swap_interval(struct dri2_egl_display *dri2_dpy)
 {
@@ -1422,6 +1376,58 @@ static const __DRIextension *dri2_loader_extensions[] = {
NULL,
 };
 
+static EGLBoolean
+dri2_initialize_x11_kms_swrast(_EGLDriver *drv, _EGLDisplay *disp)
+{
+   struct dri2_egl_display *dri2_dpy;
+
+   dri2_dpy = calloc(1, sizeof *dri2_dpy);

Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-20 Thread Emil Velikov
On 19 June 2017 at 20:46, Chad Versace  wrote:
> On Thu 15 Jun 2017, Gurchetan Singh wrote:
>> Emil, would you be fine with leaving the image extension in dri2.c but still
>> adding it as a drisw extension?  That solution would look like:
>>
>> [1]https://patchwork.freedesktop.org/patch/154807/
>
> Observations:
> - src/gallium/state_trackers/dri/dri2.c:dri2ImageExtension advertises v15 
> of __DRI_IMAGE.
> - egl_dri2.c requires only v1 of __DRI_IMAGE. Maybe a higher version
>   is required in practive, but the egl_dri2.c code checks only for v1.
>
> Questions:
> 1. All functions implemented in dri2.c:dri2ImageExtensions, do they
>under swrast? Honest question, because I'm no expert on
>gallium.
>
> If question #1 is true, then I see no problem with your latest plan. But
> maybe Emil does.
>
> If question #1 is false, it should be straightforward to implement in
> drisw.c the small subset of __DRI_IMAGE functions required for v1.

While I haven't checked how much [or well] DRI_IMAGE works with
swrast, there's no need to actually add it there.
An alternative is to add kms_swrast support for EGL like we already do
for GBM, as mentioned earlier [1].

Gents, keep in mind that:
 - one cannot pull DRM specifics (dri2.c) code within drisw.c, and
 - DRI_IMAGE pulls DRM specifics, hence adding it into drisw.c is
again a no-go :-\

FWIW the above architectural split applies for classic drivers as
well. swrast_dri.so simply cannot depend on anything DRM related.

-Emil

[1] https://lists.freedesktop.org/archives/mesa-dev/2017-June/159519.html
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-19 Thread Chad Versace
On Thu 15 Jun 2017, Gurchetan Singh wrote:
> Emil, would you be fine with leaving the image extension in dri2.c but still
> adding it as a drisw extension?  That solution would look like:
> 
> [1]https://patchwork.freedesktop.org/patch/154807/  

Observations:
- src/gallium/state_trackers/dri/dri2.c:dri2ImageExtension advertises v15 
of __DRI_IMAGE.
- egl_dri2.c requires only v1 of __DRI_IMAGE. Maybe a higher version
  is required in practive, but the egl_dri2.c code checks only for v1.

Questions:
1. All functions implemented in dri2.c:dri2ImageExtensions, do they
   under swrast? Honest question, because I'm no expert on
   gallium.
   
If question #1 is true, then I see no problem with your latest plan. But
maybe Emil does.

If question #1 is false, it should be straightforward to implement in
drisw.c the small subset of __DRI_IMAGE functions required for v1.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-15 Thread Emil Velikov
On 9 June 2017 at 16:40, Gurchetan Singh  wrote:
> Actually, these are the only patches that are required.  We're trying to run
> the Android Studio emulator using the host's GLES implementation.  The
> emulator uses the image extension in that case:
>
> https://android.googlesource.com/platform/sdk/+/emu-2.4-release/emulator/opengl/host/libs/libOpenglRender/FrameBuffer.cpp
> https://android.googlesource.com/platform/sdk/+/emu-2.4-release/emulator/opengl/host/libs/libOpenglRender/ColorBuffer.cpp
>
> It does only use a subset of the extension, but nothing hardware specific.
>
True, there's nothing HW specific, yet it does DRM specifics ;-)

From a quick look, all one needs to do is add kms_swrast support in
EGL analogous to GBM [1].
On the cool side, I've spotted a few (too many) bugs that we want to
address. I can tackle the Mesa ones [2], but please give the
emulator/qemu ones a try [3].

Thanks
Emil

[1] git show 3b176c441b7ddc5f7d2f891da3f76cf3c1814ce1  -- src/gbm/

[2] Mesa side
 - dozen or so missed checks for EGL_KHR_gl_texture_*_image extension caps

[3] The emulator code - seems to be moved to qemu/distrib/android, so
I'll omit the ones in the original sdk codebase ;-)
- query for ES1 extensions and assume that the ES2 context will have
the same ones
- error out if no ES1 configs are found - we don't care about those
since only ES2 are used
In general it seems like someone started ripping out ES1 support, but
did not quite complete it, pulling some ES2 bits by mistake. Having a
grep for s_gles1 and working up might be a good idea.

- has_eglimage_texture_2d can never be false, yet
ColorBuffer::create() takes it as an argument. In the unlikely case
that the extension is missing, function does a some setup, only to
_not_ create any images as needed later on.
- using strstr for checking extension presence is flawed.
For example: It will give false positives for OES_EGL_image_external
and OES_EGL_image_external_essl3 when one is interested in
OES_EGL_image. Disclaimer: I haven't checked if the alternative
extensions provide the required functionality.
- EGL_KHR_gl_renderbuffer_image is unused, yet checked
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-15 Thread Gurchetan Singh
Emil, would you be fine with leaving the image extension in dri2.c but
still adding it as a drisw extension?  That solution would look like:

https://patchwork.freedesktop.org/patch/154807/

On Fri, Jun 9, 2017 at 8:40 AM, Gurchetan Singh  wrote:

> Actually, these are the only patches that are required.  We're trying to
> run the Android Studio emulator using the host's GLES implementation.  The
> emulator uses the image extension in that case:
>
> https://android.googlesource.com/platform/sdk/+/emu-2.4-rele
> ase/emulator/opengl/host/libs/libOpenglRender/FrameBuffer.cpp
> https://android.googlesource.com/platform/sdk/+/emu-2.4-rele
> ase/emulator/opengl/host/libs/libOpenglRender/ColorBuffer.cpp
>
> It does only use a subset of the extension, but nothing hardware specific.
>
>
> On Fri, Jun 9, 2017 at 4:17 AM, Emil Velikov 
> wrote:
>
>> Hi Gurchetan,
>>
>> On 9 June 2017 at 01:28, gurchetansi...@chromium.org
>>  wrote:
>> > From: Gurchetan Singh 
>> >
>> > Otherwise, this extension is not visible to the EGL user
>> > ---
>> >  src/egl/drivers/dri2/egl_dri2.c | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/src/egl/drivers/dri2/egl_dri2.c
>> b/src/egl/drivers/dri2/egl_dri2.c
>> > index 7175e827c9..9e845e99e3 100644
>> > --- a/src/egl/drivers/dri2/egl_dri2.c
>> > +++ b/src/egl/drivers/dri2/egl_dri2.c
>> > @@ -429,6 +429,7 @@ static const struct dri2_extension_match
>> swrast_driver_extensions[] = {
>> >
>> >  static const struct dri2_extension_match swrast_core_extensions[] = {
>> > { __DRI_TEX_BUFFER, 2, offsetof(struct dri2_egl_display,
>> tex_buffer) },
>> > +   { __DRI_IMAGE, 1, offsetof(struct dri2_egl_display, image) },
>> IIRC the current codebase will not use it even, we expose it. Correct?
>> Wild guess here is that you guys have some extra patches around, like
>> say using vgem? Is there a public repo with the lot?
>>
>> On the st/dri side (earlier patches) - one should be able to build
>> st/dri without any hardware specific knowledge and/or files.
>> Currently that's done by isolating all the DRM specifics in dri2.c.
>> Not sure if breaking that, architectural split imho, is a good idea.
>>
>> -Emil
>>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-09 Thread Gurchetan Singh
Actually, these are the only patches that are required.  We're trying to
run the Android Studio emulator using the host's GLES implementation.  The
emulator uses the image extension in that case:

https://android.googlesource.com/platform/sdk/+/emu-2.4-
release/emulator/opengl/host/libs/libOpenglRender/FrameBuffer.cpp
https://android.googlesource.com/platform/sdk/+/emu-2.4-
release/emulator/opengl/host/libs/libOpenglRender/ColorBuffer.cpp

It does only use a subset of the extension, but nothing hardware specific.


On Fri, Jun 9, 2017 at 4:17 AM, Emil Velikov 
wrote:

> Hi Gurchetan,
>
> On 9 June 2017 at 01:28, gurchetansi...@chromium.org
>  wrote:
> > From: Gurchetan Singh 
> >
> > Otherwise, this extension is not visible to the EGL user
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_
> dri2.c
> > index 7175e827c9..9e845e99e3 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -429,6 +429,7 @@ static const struct dri2_extension_match
> swrast_driver_extensions[] = {
> >
> >  static const struct dri2_extension_match swrast_core_extensions[] = {
> > { __DRI_TEX_BUFFER, 2, offsetof(struct dri2_egl_display, tex_buffer)
> },
> > +   { __DRI_IMAGE, 1, offsetof(struct dri2_egl_display, image) },
> IIRC the current codebase will not use it even, we expose it. Correct?
> Wild guess here is that you guys have some extra patches around, like
> say using vgem? Is there a public repo with the lot?
>
> On the st/dri side (earlier patches) - one should be able to build
> st/dri without any hardware specific knowledge and/or files.
> Currently that's done by isolating all the DRM specifics in dri2.c.
> Not sure if breaking that, architectural split imho, is a good idea.
>
> -Emil
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

2017-06-09 Thread Emil Velikov
Hi Gurchetan,

On 9 June 2017 at 01:28, gurchetansi...@chromium.org
 wrote:
> From: Gurchetan Singh 
>
> Otherwise, this extension is not visible to the EGL user
> ---
>  src/egl/drivers/dri2/egl_dri2.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 7175e827c9..9e845e99e3 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -429,6 +429,7 @@ static const struct dri2_extension_match 
> swrast_driver_extensions[] = {
>
>  static const struct dri2_extension_match swrast_core_extensions[] = {
> { __DRI_TEX_BUFFER, 2, offsetof(struct dri2_egl_display, tex_buffer) },
> +   { __DRI_IMAGE, 1, offsetof(struct dri2_egl_display, image) },
IIRC the current codebase will not use it even, we expose it. Correct?
Wild guess here is that you guys have some extra patches around, like
say using vgem? Is there a public repo with the lot?

On the st/dri side (earlier patches) - one should be able to build
st/dri without any hardware specific knowledge and/or files.
Currently that's done by isolating all the DRM specifics in dri2.c.
Not sure if breaking that, architectural split imho, is a good idea.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev