Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
On 10/07/18 14:52, Pekka Paalanen wrote: > On Tue, 3 Jul 2018 16:46:29 +0100 > Emil Velikov wrote: > >> Hi Emilio, >> >> On 2 July 2018 at 16:22, Emilio Pozuelo Monfort wrote: >>> Signed-off-by: Emilio Pozuelo Monfort >>> --- >>> I tried a build with --disable-egl as I didn't have the headers >>> installed, and it broke here. The EGL usage here seemed optional so I >>> did that, but I didn't run-test the result. If it would make more sense >>> to disable the client if EGL support is disabled I can do that. >>> >>> clients/simple-dmabuf-drm.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >> Fairly orthogonal question, aimed at wayland devs: >> >> Why does this "simple" app has per-device code instead of using >> gbm_bo_map/unmap? >> API has been around for 2 years and every half recent Mesa driver has >> support for it. Only the really old ones do not radeon (r100), r200, >> nouveau_vieux and i915. > > Hi Emil, > > I've had the same question before myself, but now that you ask it > again, I don't remember the answer. > > This client was originally written in 2014, and I think at that time > there were no generic APIs to do what it needed to do. Not sure if > there were other reasons as well. I have a semi-working patch for this. I will send it soon to ask for comments. Emilio ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
On Tue, 3 Jul 2018 16:46:29 +0100 Emil Velikov wrote: > Hi Emilio, > > On 2 July 2018 at 16:22, Emilio Pozuelo Monfort wrote: > > Signed-off-by: Emilio Pozuelo Monfort > > --- > > I tried a build with --disable-egl as I didn't have the headers > > installed, and it broke here. The EGL usage here seemed optional so I > > did that, but I didn't run-test the result. If it would make more sense > > to disable the client if EGL support is disabled I can do that. > > > > clients/simple-dmabuf-drm.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > Fairly orthogonal question, aimed at wayland devs: > > Why does this "simple" app has per-device code instead of using > gbm_bo_map/unmap? > API has been around for 2 years and every half recent Mesa driver has > support for it. Only the really old ones do not radeon (r100), r200, > nouveau_vieux and i915. Hi Emil, I've had the same question before myself, but now that you ask it again, I don't remember the answer. This client was originally written in 2014, and I think at that time there were no generic APIs to do what it needed to do. Not sure if there were other reasons as well. Thanks, pq pgpUcB2fYlAzy.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
Hi Emilio, On 2 July 2018 at 16:22, Emilio Pozuelo Monfort wrote: > Signed-off-by: Emilio Pozuelo Monfort > --- > I tried a build with --disable-egl as I didn't have the headers > installed, and it broke here. The EGL usage here seemed optional so I > did that, but I didn't run-test the result. If it would make more sense > to disable the client if EGL support is disabled I can do that. > > clients/simple-dmabuf-drm.c | 2 ++ > 1 file changed, 2 insertions(+) > Fairly orthogonal question, aimed at wayland devs: Why does this "simple" app has per-device code instead of using gbm_bo_map/unmap? API has been around for 2 years and every half recent Mesa driver has support for it. Only the really old ones do not radeon (r100), r200, nouveau_vieux and i915. Thanks Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
On Tue, 3 Jul 2018 12:35:30 +0200 Emilio Pozuelo Monfort wrote: > On 03/07/18 11:00, Pekka Paalanen wrote: > > On Mon, 2 Jul 2018 17:22:30 +0200 > > Emilio Pozuelo Monfort wrote: > > > >> Signed-off-by: Emilio Pozuelo Monfort > >> --- > >> I tried a build with --disable-egl as I didn't have the headers > >> installed, and it broke here. The EGL usage here seemed optional so I > >> did that, but I didn't run-test the result. If it would make more sense > >> to disable the client if EGL support is disabled I can do that. > >> > >> clients/simple-dmabuf-drm.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c > >> index fcab30e5..1ae3a2dd 100644 > >> --- a/clients/simple-dmabuf-drm.c > >> +++ b/clients/simple-dmabuf-drm.c > >> @@ -863,6 +863,7 @@ create_display(int opts, int format) > >>display->req_dmabuf_immediate = opts & OPT_IMMEDIATE; > >>display->req_dmabuf_modifiers = (format == DRM_FORMAT_NV12); > >> > >> +#if ENABLE_EGL > >>/* > >> * hard code format if the platform egl doesn't support format > >> * querying / advertising. > >> @@ -871,6 +872,7 @@ create_display(int opts, int format) > >>if (extensions && !weston_check_egl_extension(extensions, > >>"EGL_EXT_image_dma_buf_import_modifiers")) > >>display->xrgb_format_found = 1; > >> +#endif > >> > >>display->registry = wl_display_get_registry(display->display); > >>wl_registry_add_listener(display->registry, > > > > Hi, > > > > that's very strange. This program does not use EGL or even GBM, that's > > a completely dead hunk of code you're ifdeffing there as I can see. > > Would be better to just remove it completely, and make sure the build > > does not link libEGL or GBM. Include for shared/platform.h should be > > useless too. > > It's not dead code, it's a fallback mechanism in case the EGL platform doesn't > support EGL_EXT_image_dma_buf_import_modifiers. The problem is that > configure.ac > checks for EGL for simple-dmabuf-drm-client, but the client is not disabled if > one builds with --disable-egl. Perhaps I should do that instead. After all, > disable-egl disables the gl-renderer, which is needed for zwp_linux_dmabuf, > which is needed by simple-dmabuf-drm-client. What I meant was that nothing calls eglGetDisplay or equivalent. It may be inspecting the EGL Client extensions, but it won't do anything with that information. The Wayland extension advertises modifier support completely independently. In fact, if the compositor did not advertise xrgb through zwp_linux_dmabuf, the flag for it should never be set. I believe you'd be fixing a bug by simply deleting that code. No, two bugs: the flag and the build. :-) Thanks, pq pgpdBq43kgINd.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
On 03/07/18 11:00, Pekka Paalanen wrote: > On Mon, 2 Jul 2018 17:22:30 +0200 > Emilio Pozuelo Monfort wrote: > >> Signed-off-by: Emilio Pozuelo Monfort >> --- >> I tried a build with --disable-egl as I didn't have the headers >> installed, and it broke here. The EGL usage here seemed optional so I >> did that, but I didn't run-test the result. If it would make more sense >> to disable the client if EGL support is disabled I can do that. >> >> clients/simple-dmabuf-drm.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c >> index fcab30e5..1ae3a2dd 100644 >> --- a/clients/simple-dmabuf-drm.c >> +++ b/clients/simple-dmabuf-drm.c >> @@ -863,6 +863,7 @@ create_display(int opts, int format) >> display->req_dmabuf_immediate = opts & OPT_IMMEDIATE; >> display->req_dmabuf_modifiers = (format == DRM_FORMAT_NV12); >> >> +#if ENABLE_EGL >> /* >> * hard code format if the platform egl doesn't support format >> * querying / advertising. >> @@ -871,6 +872,7 @@ create_display(int opts, int format) >> if (extensions && !weston_check_egl_extension(extensions, >> "EGL_EXT_image_dma_buf_import_modifiers")) >> display->xrgb_format_found = 1; >> +#endif >> >> display->registry = wl_display_get_registry(display->display); >> wl_registry_add_listener(display->registry, > > Hi, > > that's very strange. This program does not use EGL or even GBM, that's > a completely dead hunk of code you're ifdeffing there as I can see. > Would be better to just remove it completely, and make sure the build > does not link libEGL or GBM. Include for shared/platform.h should be > useless too. It's not dead code, it's a fallback mechanism in case the EGL platform doesn't support EGL_EXT_image_dma_buf_import_modifiers. The problem is that configure.ac checks for EGL for simple-dmabuf-drm-client, but the client is not disabled if one builds with --disable-egl. Perhaps I should do that instead. After all, disable-egl disables the gl-renderer, which is needed for zwp_linux_dmabuf, which is needed by simple-dmabuf-drm-client. Thoughts? Emilio ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
On Mon, 2 Jul 2018 17:22:30 +0200 Emilio Pozuelo Monfort wrote: > Signed-off-by: Emilio Pozuelo Monfort > --- > I tried a build with --disable-egl as I didn't have the headers > installed, and it broke here. The EGL usage here seemed optional so I > did that, but I didn't run-test the result. If it would make more sense > to disable the client if EGL support is disabled I can do that. > > clients/simple-dmabuf-drm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c > index fcab30e5..1ae3a2dd 100644 > --- a/clients/simple-dmabuf-drm.c > +++ b/clients/simple-dmabuf-drm.c > @@ -863,6 +863,7 @@ create_display(int opts, int format) > display->req_dmabuf_immediate = opts & OPT_IMMEDIATE; > display->req_dmabuf_modifiers = (format == DRM_FORMAT_NV12); > > +#if ENABLE_EGL > /* >* hard code format if the platform egl doesn't support format >* querying / advertising. > @@ -871,6 +872,7 @@ create_display(int opts, int format) > if (extensions && !weston_check_egl_extension(extensions, > "EGL_EXT_image_dma_buf_import_modifiers")) > display->xrgb_format_found = 1; > +#endif > > display->registry = wl_display_get_registry(display->display); > wl_registry_add_listener(display->registry, Hi, that's very strange. This program does not use EGL or even GBM, that's a completely dead hunk of code you're ifdeffing there as I can see. Would be better to just remove it completely, and make sure the build does not link libEGL or GBM. Include for shared/platform.h should be useless too. Thanks, pq pgp_UUw9OSWrO.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
Signed-off-by: Emilio Pozuelo Monfort --- I tried a build with --disable-egl as I didn't have the headers installed, and it broke here. The EGL usage here seemed optional so I did that, but I didn't run-test the result. If it would make more sense to disable the client if EGL support is disabled I can do that. clients/simple-dmabuf-drm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c index fcab30e5..1ae3a2dd 100644 --- a/clients/simple-dmabuf-drm.c +++ b/clients/simple-dmabuf-drm.c @@ -863,6 +863,7 @@ create_display(int opts, int format) display->req_dmabuf_immediate = opts & OPT_IMMEDIATE; display->req_dmabuf_modifiers = (format == DRM_FORMAT_NV12); +#if ENABLE_EGL /* * hard code format if the platform egl doesn't support format * querying / advertising. @@ -871,6 +872,7 @@ create_display(int opts, int format) if (extensions && !weston_check_egl_extension(extensions, "EGL_EXT_image_dma_buf_import_modifiers")) display->xrgb_format_found = 1; +#endif display->registry = wl_display_get_registry(display->display); wl_registry_add_listener(display->registry, -- 2.18.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
