| Message ID | 20200402130842.918696-3-gary.bisson@boundarydevices.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | imx: fix gpu graphics support | expand |
Hello Gary, As usual, thanks for your work maintaining good support for i.MX platforms in Buildroot. One question below. On Thu, 2 Apr 2020 15:08:42 +0200 Gary Bisson <gary.bisson@boundarydevices.com> wrote: > ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb) > -define IMX_GPU_VIV_FIXUP_FB_HEADERS > +define IMX_GPU_VIV_FIXUP_HEADERS > $(SED) '39i\ > - #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) \n\ > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > #define EGL_API_FB \n\ > #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > endef > @@ -77,6 +77,12 @@ ifeq ($(IMX_GPU_VIV_LIB_TARGET),wl) > define IMX_GPU_VIV_FIXUP_PKGCONFIG > ln -sf egl_wayland.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc > endef > +define IMX_GPU_VIV_FIXUP_HEADERS > + $(SED) '39i\ > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > + #define WL_EGL_PLATFORM\n\ > + #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > +endef > endif Can we use a proper patch instead of these horrible SED calls ? Best regards, Thomas
Hi Thomas, On Thu, Apr 02, 2020 at 04:25:43PM +0200, Thomas Petazzoni wrote: > Hello Gary, > > As usual, thanks for your work maintaining good support for i.MX > platforms in Buildroot. One question below. No problem. > On Thu, 2 Apr 2020 15:08:42 +0200 > Gary Bisson <gary.bisson@boundarydevices.com> wrote: > > > ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb) > > -define IMX_GPU_VIV_FIXUP_FB_HEADERS > > +define IMX_GPU_VIV_FIXUP_HEADERS > > $(SED) '39i\ > > - #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) \n\ > > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > > #define EGL_API_FB \n\ > > #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > > endef > > @@ -77,6 +77,12 @@ ifeq ($(IMX_GPU_VIV_LIB_TARGET),wl) > > define IMX_GPU_VIV_FIXUP_PKGCONFIG > > ln -sf egl_wayland.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc > > endef > > +define IMX_GPU_VIV_FIXUP_HEADERS > > + $(SED) '39i\ > > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > > + #define WL_EGL_PLATFORM\n\ > > + #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > > +endef > > endif > > Can we use a proper patch instead of these horrible SED calls ? I know it isn't an elegant solution and I am open to discussion. However modifying eglplatform.h still seems like the less intrusive solution. Do you have anything in mind? What Yocto does is to patch packages that break by appending CFLAGS [1][2], I'd like to avoid such approach. Especially since in my case it happened building standard Weston package... It works in their case because they use a Weston fork (along with a libdrm fork, and wayland-protocols fork). Regards, Gary [1] https://github.com/Freescale/meta-freescale/blob/zeus/recipes-graphics/libsdl2/libsdl2_%25.bbappend [2] https://github.com/Freescale/meta-freescale/blob/zeus/recipes-graphics/libepoxy/libepoxy_1.5.%25.bbappend
Hi Thomas, Gentle ping on this, let me know if you see another approach that would be better, I'm open to suggestions. I'd love to have Weston support for i.MX8M* fixed for next release and my other series [1] depends on this patch. Then I am preparing patches to have VPU support using GStreamer-imx V2 for i.MX8M* as well. Regards, Gary [1] http://patchwork.ozlabs.org/project/buildroot/list/?series=184556 On Thu, Apr 02, 2020 at 05:10:26PM +0200, Gary Bisson wrote: > Hi Thomas, > > On Thu, Apr 02, 2020 at 04:25:43PM +0200, Thomas Petazzoni wrote: > > Hello Gary, > > > > As usual, thanks for your work maintaining good support for i.MX > > platforms in Buildroot. One question below. > > No problem. > > > On Thu, 2 Apr 2020 15:08:42 +0200 > > Gary Bisson <gary.bisson@boundarydevices.com> wrote: > > > > > ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb) > > > -define IMX_GPU_VIV_FIXUP_FB_HEADERS > > > +define IMX_GPU_VIV_FIXUP_HEADERS > > > $(SED) '39i\ > > > - #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) \n\ > > > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > > > #define EGL_API_FB \n\ > > > #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > > > endef > > > @@ -77,6 +77,12 @@ ifeq ($(IMX_GPU_VIV_LIB_TARGET),wl) > > > define IMX_GPU_VIV_FIXUP_PKGCONFIG > > > ln -sf egl_wayland.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc > > > endef > > > +define IMX_GPU_VIV_FIXUP_HEADERS > > > + $(SED) '39i\ > > > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > > > + #define WL_EGL_PLATFORM\n\ > > > + #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > > > +endef > > > endif > > > > Can we use a proper patch instead of these horrible SED calls ? > > I know it isn't an elegant solution and I am open to discussion. > However modifying eglplatform.h still seems like the less intrusive > solution. > > Do you have anything in mind? > > What Yocto does is to patch packages that break by appending CFLAGS > [1][2], I'd like to avoid such approach. > > Especially since in my case it happened building standard Weston > package... It works in their case because they use a Weston fork (along > with a libdrm fork, and wayland-protocols fork). > > Regards, > Gary > > [1] https://github.com/Freescale/meta-freescale/blob/zeus/recipes-graphics/libsdl2/libsdl2_%25.bbappend > [2] https://github.com/Freescale/meta-freescale/blob/zeus/recipes-graphics/libepoxy/libepoxy_1.5.%25.bbappend
Hi Gary, Hi Thomas, Am Mi., 8. Juli 2020 um 14:20 Uhr schrieb Gary Bisson <gary.bisson@boundarydevices.com>: > > Hi Thomas, > > Gentle ping on this, let me know if you see another approach that would > be better, I'm open to suggestions. > > I'd love to have Weston support for i.MX8M* fixed for next release and > my other series [1] depends on this patch. > > Then I am preparing patches to have VPU support using GStreamer-imx V2 > for i.MX8M* as well. Any news on that? I also would like to see weston support for imx8m in buildroot. @Gary: did you try to fix that upstream?
Hi Heiko, On Tue, Oct 13, 2020 at 08:30:50PM +0200, Heiko Thiery wrote: > Hi Gary, Hi Thomas, > > Am Mi., 8. Juli 2020 um 14:20 Uhr schrieb Gary Bisson > <gary.bisson@boundarydevices.com>: > > > > Hi Thomas, > > > > Gentle ping on this, let me know if you see another approach that would > > be better, I'm open to suggestions. > > > > I'd love to have Weston support for i.MX8M* fixed for next release and > > my other series [1] depends on this patch. > > > > Then I am preparing patches to have VPU support using GStreamer-imx V2 > > for i.MX8M* as well. > > Any news on that? I also would like to see weston support for imx8m in > buildroot. No, a Tested-by and/or Reviewed-by would definitely help the patch getting further. Have you tried it? > @Gary: did you try to fix that upstream? What upstream? There's none to my knowledge. As mentioned in the commit log, even NXP doesn't have a real solution for this. Regards, Gary
Hi Gary, Am Di., 13. Okt. 2020 um 22:07 Uhr schrieb Gary Bisson <gary.bisson@boundarydevices.com>: > > Hi Heiko, > > On Tue, Oct 13, 2020 at 08:30:50PM +0200, Heiko Thiery wrote: > > Hi Gary, Hi Thomas, > > > > Am Mi., 8. Juli 2020 um 14:20 Uhr schrieb Gary Bisson > > <gary.bisson@boundarydevices.com>: > > > > > > Hi Thomas, > > > > > > Gentle ping on this, let me know if you see another approach that would > > > be better, I'm open to suggestions. > > > > > > I'd love to have Weston support for i.MX8M* fixed for next release and > > > my other series [1] depends on this patch. > > > > > > Then I am preparing patches to have VPU support using GStreamer-imx V2 > > > for i.MX8M* as well. > > > > Any news on that? I also would like to see weston support for imx8m in > > buildroot. > > No, a Tested-by and/or Reviewed-by would definitely help the patch > getting further. Have you tried it? I'm in progress and hopefully have a running system in the next few days. If I get a running version I will do so. > > > @Gary: did you try to fix that upstream? > > What upstream? There's none to my knowledge. As mentioned in the commit > log, even NXP doesn't have a real solution for this. I have to admit I'm just at the beginning of that topic and have no experience yet ;-/ > > Regards, > Gary
Hi Gary, Am Do., 2. Apr. 2020 um 15:09 Uhr schrieb Gary Bisson <gary.bisson@boundarydevices.com>: > > Just like Jerome had to do with EGL_API_FB back in the days, in order to > compile with Vivante header it is necessary to have some defines setup > as the default behavior of eglplatform.h is to include Xlib.h. > > So this patch sets WL_EGL_PLATFORM when the Wayland backend is used. > This option is actually declared in egl.pc but not all packages use > pkg-config properly. > > Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> > --- > Fixes the following build issue (building weston): > In file included from host/aarch64-buildroot-linux-gnu/sysroot/usr/include/EGL/egl.h:39, > from ../libweston/renderer-gl/gl-renderer.h:36, > from ../libweston/backend-drm/drm-gbm.c:42: > host/aarch64-buildroot-linux-gnu/sysroot/usr/include/EGL/eglplatform.h:144:10: fatal error: X11/Xlib.h: No such file or directory > 144 | #include <X11/Xlib.h> > --- > package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk > index 646d4e3673..cd9a9339b4 100644 > --- a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk > +++ b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk > @@ -59,9 +59,9 @@ define IMX_GPU_VIV_BUILD_CMDS > endef > > ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb) > -define IMX_GPU_VIV_FIXUP_FB_HEADERS > +define IMX_GPU_VIV_FIXUP_HEADERS > $(SED) '39i\ > - #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) \n\ > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > #define EGL_API_FB \n\ > #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > endef > @@ -77,6 +77,12 @@ ifeq ($(IMX_GPU_VIV_LIB_TARGET),wl) > define IMX_GPU_VIV_FIXUP_PKGCONFIG > ln -sf egl_wayland.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc > endef > +define IMX_GPU_VIV_FIXUP_HEADERS > + $(SED) '39i\ > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > + #define WL_EGL_PLATFORM\n\ > + #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > +endef > endif > > ifeq ($(IMX_GPU_VIV_LIB_TARGET),x11) > @@ -89,7 +95,7 @@ endif > > define IMX_GPU_VIV_INSTALL_STAGING_CMDS > cp -r $(@D)/gpu-core/usr/* $(STAGING_DIR)/usr > - $(IMX_GPU_VIV_FIXUP_FB_HEADERS) > + $(IMX_GPU_VIV_FIXUP_HEADERS) > $(IMX_GPU_VIV_FIXUP_PKGCONFIG) > for lib in egl gbm glesv1_cm glesv2 vg; do \ > $(INSTALL) -m 0644 -D \ > -- > 2.25.1 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hi Thomas, Am Do., 2. Apr. 2020 um 16:25 Uhr schrieb Thomas Petazzoni <thomas.petazzoni@bootlin.com>: > > Hello Gary, > > As usual, thanks for your work maintaining good support for i.MX > platforms in Buildroot. One question below. > > On Thu, 2 Apr 2020 15:08:42 +0200 > Gary Bisson <gary.bisson@boundarydevices.com> wrote: > > > ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb) > > -define IMX_GPU_VIV_FIXUP_FB_HEADERS > > +define IMX_GPU_VIV_FIXUP_HEADERS > > $(SED) '39i\ > > - #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) \n\ > > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > > #define EGL_API_FB \n\ > > #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > > endef > > @@ -77,6 +77,12 @@ ifeq ($(IMX_GPU_VIV_LIB_TARGET),wl) > > define IMX_GPU_VIV_FIXUP_PKGCONFIG > > ln -sf egl_wayland.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc > > endef > > +define IMX_GPU_VIV_FIXUP_HEADERS > > + $(SED) '39i\ > > + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ > > + #define WL_EGL_PLATFORM\n\ > > + #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h > > +endef > > endif > > Can we use a proper patch instead of these horrible SED calls ? What do you mean with a proper patch? These fixups will modify a header file of weston. How could we do a patch of weston here?
Gary, Heiko, Thomas, All, On 2020-10-13 20:30 +0200, Heiko Thiery spake thusly: > Am Mi., 8. Juli 2020 um 14:20 Uhr schrieb Gary Bisson > <gary.bisson@boundarydevices.com>: > > Gentle ping on this, let me know if you see another approach that would > > be better, I'm open to suggestions. > > > > I'd love to have Weston support for i.MX8M* fixed for next release and > > my other series [1] depends on this patch. > > > > Then I am preparing patches to have VPU support using GStreamer-imx V2 > > for i.MX8M* as well. > > Any news on that? I also would like to see weston support for imx8m in > buildroot. So, we've discussed and investigated this with Heiko on IRC tonight, and we came to the conclusion that the weston build failure is a weston bug. Indeed, weston *is* using pkg-config to find EGL: Run-time dependency egl found: YES 8.0 Weston is indeed using the proper CFLAGS from egl.pc to build parts of its files; for example, bulding libweston/backend-wayland/wayland.c is using the proper -DWL_EGL_PLATFORM. However, the CFLAGS from egl.pc are not used when building gdbm-drm.c, when it should. Indeed, drm-gbm.c includes gl-renderer.h which includes egl.h which include eglpltform.h, so it seems totally expected that drm-gbm.c does use the CFLAGS frpm egl.pc. In my opinion, this build failre you report in your original patch is in fact a weston bug. Probably, patch like this could help: From Yann E. MORIN <yann.morin.1998@free.fr> libweston/backend/drm: might need EGL gbm-drm.c includes gl-renderer.h. When EGL is enabled, that in turns includes egl.h. As such, dependencies for drm should include EGL if it is available. This condition is modelled after a similar one in libweston/meson.build Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> Reported-by: Heiko Thiery <heiko.thiery@gmail.com> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> Cc: Refik Tuzakli <tuzakli.refik@gmail.com> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> diff --git a/libweston/backend-drm/meson.build b/libweston/backend-drm/meson.build index 484c2702..67648a09 100644 --- a/libweston/backend-drm/meson.build +++ b/libweston/backend-drm/meson.build @@ -53,6 +53,10 @@ if get_option('renderer-gl') config_h.set('HAVE_GBM_FD_IMPORT', '1') endif deps_drm += dep_gbm + deps_egl = dependency('egl', required: false) + if dep_egl.found() + deps_drm += dep_egl + endif srcs_drm += 'drm-gbm.c' config_h.set('BUILD_DRM_GBM', '1') endif The build succeeded for me with that patch, at least; I'll let you check it is valid, and maybe send it upstream Regards, Yann E. MORIN.
Gary, Heiko, Thomas, All, On 2020-10-20 23:20 +0200, Yann E. MORIN spake thusly: > On 2020-10-13 20:30 +0200, Heiko Thiery spake thusly: > > Am Mi., 8. Juli 2020 um 14:20 Uhr schrieb Gary Bisson > > <gary.bisson@boundarydevices.com>: > > > Gentle ping on this, let me know if you see another approach that would > > > be better, I'm open to suggestions. > So, we've discussed and investigated this with Heiko on IRC tonight, and > we came to the conclusion that the weston build failure is a weston bug. [--SNIP--] I forgot to say: As a consequence, I marked this patch as "changes requested" in patchwork. Regards, Yann E. MORIN.
diff --git a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk index 646d4e3673..cd9a9339b4 100644 --- a/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk +++ b/package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk @@ -59,9 +59,9 @@ define IMX_GPU_VIV_BUILD_CMDS endef ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb) -define IMX_GPU_VIV_FIXUP_FB_HEADERS +define IMX_GPU_VIV_FIXUP_HEADERS $(SED) '39i\ - #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) \n\ + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ #define EGL_API_FB \n\ #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h endef @@ -77,6 +77,12 @@ ifeq ($(IMX_GPU_VIV_LIB_TARGET),wl) define IMX_GPU_VIV_FIXUP_PKGCONFIG ln -sf egl_wayland.pc $(@D)/gpu-core/usr/lib/pkgconfig/egl.pc endef +define IMX_GPU_VIV_FIXUP_HEADERS + $(SED) '39i\ + #if !defined(EGL_API_X11) && !defined(EGL_API_DFB) && !defined(EGL_API_FB) && !defined(WL_EGL_PLATFORM) \n\ + #define WL_EGL_PLATFORM\n\ + #endif' $(STAGING_DIR)/usr/include/EGL/eglplatform.h +endef endif ifeq ($(IMX_GPU_VIV_LIB_TARGET),x11) @@ -89,7 +95,7 @@ endif define IMX_GPU_VIV_INSTALL_STAGING_CMDS cp -r $(@D)/gpu-core/usr/* $(STAGING_DIR)/usr - $(IMX_GPU_VIV_FIXUP_FB_HEADERS) + $(IMX_GPU_VIV_FIXUP_HEADERS) $(IMX_GPU_VIV_FIXUP_PKGCONFIG) for lib in egl gbm glesv1_cm glesv2 vg; do \ $(INSTALL) -m 0644 -D \
Just like Jerome had to do with EGL_API_FB back in the days, in order to compile with Vivante header it is necessary to have some defines setup as the default behavior of eglplatform.h is to include Xlib.h. So this patch sets WL_EGL_PLATFORM when the Wayland backend is used. This option is actually declared in egl.pc but not all packages use pkg-config properly. Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com> --- Fixes the following build issue (building weston): In file included from host/aarch64-buildroot-linux-gnu/sysroot/usr/include/EGL/egl.h:39, from ../libweston/renderer-gl/gl-renderer.h:36, from ../libweston/backend-drm/drm-gbm.c:42: host/aarch64-buildroot-linux-gnu/sysroot/usr/include/EGL/eglplatform.h:144:10: fatal error: X11/Xlib.h: No such file or directory 144 | #include <X11/Xlib.h> --- package/freescale-imx/imx-gpu-viv/imx-gpu-viv.mk | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)