| Message ID | 20200818191130.2888592-1-christian@paral.in |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/1] package/rpi-userland: add support for aarch64 build | expand |
Christian, All, On 2020-08-18 12:11 -0700, Christian Stewart spake thusly: > Tested on Pi4 Model B (aarch64). There was previous proposal for this: http://lists.busybox.net/pipermail/buildroot/2020-June/285000.html http://lists.busybox.net/pipermail/buildroot/2020-January/271588.html where it wsa noticed and pointed out that not all libraries were installed for tghe 64-bit variants, and that parts of the 64-bit support even git reverted upstream: https://github.com/raspberrypi/userland/commit/f97b1af1b3e653f9da2c1a3643479bfd469e3b74 How does that patch address these issues? Regards, Yann E. MORIN. > Signed-off-by: Christian Stewart <christian@paral.in> > --- > package/rpi-userland/Config.in | 4 ++-- > package/rpi-userland/rpi-userland.mk | 4 ++++ > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/package/rpi-userland/Config.in b/package/rpi-userland/Config.in > index 342faf26e3..81f3588822 100644 > --- a/package/rpi-userland/Config.in > +++ b/package/rpi-userland/Config.in > @@ -1,6 +1,6 @@ > config BR2_PACKAGE_RPI_USERLAND > bool "rpi-userland" > - depends on BR2_arm > + depends on BR2_arm || BR2_aarch64 > depends on BR2_INSTALL_LIBSTDCPP > depends on BR2_TOOLCHAIN_HAS_THREADS > depends on !BR2_STATIC_LIBS > @@ -40,6 +40,6 @@ config BR2_PACKAGE_RPI_USERLAND_HELLO > endif > > comment "rpi-userland needs a toolchain w/ C++, threads, dynamic library" > - depends on BR2_arm > + depends on BR2_arm || BR2_aarch64 > depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS || \ > BR2_STATIC_LIBS > diff --git a/package/rpi-userland/rpi-userland.mk b/package/rpi-userland/rpi-userland.mk > index 9edc92344e..ec5e26f140 100644 > --- a/package/rpi-userland/rpi-userland.mk > +++ b/package/rpi-userland/rpi-userland.mk > @@ -13,6 +13,10 @@ RPI_USERLAND_CONF_OPTS = -DVMCS_INSTALL_PREFIX=/usr > > RPI_USERLAND_PROVIDES = libegl libgles libopenmax libopenvg > > +ifeq ($(BR2_aarch64),y) > +RPI_USERLAND_CONF_OPTS += -DARM64=ON > +endif > + > ifeq ($(BR2_PACKAGE_RPI_USERLAND_HELLO),y) > > RPI_USERLAND_CONF_OPTS += -DALL_APPS=ON > -- > 2.28.0 >
Hi Yann, On Tue, Aug 18, 2020 at 12:33 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > On 2020-08-18 12:11 -0700, Christian Stewart spake thusly: > > Tested on Pi4 Model B (aarch64). > > There was previous proposal for this: > > http://lists.busybox.net/pipermail/buildroot/2020-June/285000.html > http://lists.busybox.net/pipermail/buildroot/2020-January/271588.html > > where it wsa noticed and pointed out that not all libraries were > installed for tghe 64-bit variants, and that parts of the 64-bit > support even git reverted upstream: > > https://github.com/raspberrypi/userland/commit/f97b1af1b3e653f9da2c1a3643479bfd469e3b74 > > How does that patch address these issues? I don't think this is relevant anymore. In the userland commit you link just above, they disable some things when targeting ARM64. We set ARM64 correctly in this patch with -DARM64=ON if BR2_aarch64, so this addresses everything correctly. Best, Christian
Christian., All, On 2020-08-18 12:48 -0700, Christian Stewart spake thusly: > On Tue, Aug 18, 2020 at 12:33 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > On 2020-08-18 12:11 -0700, Christian Stewart spake thusly: > > > Tested on Pi4 Model B (aarch64). > > > > There was previous proposal for this: > > > > http://lists.busybox.net/pipermail/buildroot/2020-June/285000.html > > http://lists.busybox.net/pipermail/buildroot/2020-January/271588.html > > > > where it wsa noticed and pointed out that not all libraries were > > installed for tghe 64-bit variants, and that parts of the 64-bit > > support even git reverted upstream: > > > > https://github.com/raspberrypi/userland/commit/f97b1af1b3e653f9da2c1a3643479bfd469e3b74 > > > > How does that patch address these issues? > > I don't think this is relevant anymore. > > In the userland commit you link just above, they disable some things > when targeting ARM64. We set ARM64 correctly in this patch with > -DARM64=ON if BR2_aarch64, so this addresses everything correctly. So, this would, amongst others, not build: - MMAL, so no low-level programming, - interface/khronos, so no OpenGL EGL/GLES, no OpenVG, - middleware/openmaxil, so no OpenMAX Unless I'm missing something non-obvious, I do not see how that can be irrelevant... Regards, Yann E. MORIN.
Hi Yann, On Tue, Aug 18, 2020 at 1:16 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > In the userland commit you link just above, they disable some things > > when targeting ARM64. We set ARM64 correctly in this patch with > > -DARM64=ON if BR2_aarch64, so this addresses everything correctly. > > So, this would, amongst others, not build: > > - MMAL, so no low-level programming, > > - interface/khronos, so no OpenGL EGL/GLES, no OpenVG, > > - middleware/openmaxil, so no OpenMAX > > Unless I'm missing something non-obvious, I do not see how that can be > irrelevant... Of course, not having everything is not irrelevant. However, this is something we expect the rpi-userland developers to address, not the Buildroot developers, correct? As far as I can tell, adding this patch will install the tools that the rpi developers currently have deemed compatible with aarch64. This is vcgencmd and others. This basically would add early support for aarch64 to rpi-userland in Buildroot. Although not everything is there, installing the package with the parts that are compatible with that arch makes sense, right? As an example: I was writing a package which adds support for the argon one fan controller for pi 4. It added a dependency for rpi-userland for the vcgencmd command. I wrote this patch so that vcgencmd and the package will be installed correctly, and also so that my package which depends on RPI_USERLAND, could also build and be installed. Everything works as expected. Other packages that depend on the arm-only binaries should already have BR2_arm dependency anyway, correct? Best regards, Christian Stewart
Christian, All, On 2020-08-19 13:54 -0700, Christian Stewart spake thusly: > On Tue, Aug 18, 2020 at 1:16 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > > In the userland commit you link just above, they disable some things > > > when targeting ARM64. We set ARM64 correctly in this patch with > > > -DARM64=ON if BR2_aarch64, so this addresses everything correctly. > > > > So, this would, amongst others, not build: > > > > - MMAL, so no low-level programming, > > > > - interface/khronos, so no OpenGL EGL/GLES, no OpenVG, > > > > - middleware/openmaxil, so no OpenMAX > > > > Unless I'm missing something non-obvious, I do not see how that can be > > irrelevant... > > Of course, not having everything is not irrelevant. However, this is > something we expect the rpi-userland developers to address, not the > Buildroot developers, correct? Right, but the package in Buildroot still declares providing libegl, libgels, vg, and max, which is clearly wrong in this case. With just your patch, the package would still advertise providing those, but they would not be built. Any pacjage that "just" needs a GL/GLES provider (and has ptherwise no requirement on the arcitecture) would fail to build. That is what was pointed out in the first review (which I already pointed to): http://lists.busybox.net/pipermail/buildroot/2020-January/271588.html ... which already had a proposal patch to fix the provider side of the package, and also identifies all the delta between what is installed for arm and for aarch64. > As far as I can tell, adding this patch will install the tools that > the rpi developers currently have deemed compatible with aarch64. This > is vcgencmd and others. This basically would add early support for > aarch64 to rpi-userland in Buildroot. Although not everything is > there, installing the package with the parts that are compatible with > that arch makes sense, right? No, see above about the GL and stuff provider. > As an example: I was writing a package which adds support for the > argon one fan controller for pi 4. It added a dependency for > rpi-userland for the vcgencmd command. I wrote this patch so that > vcgencmd and the package will be installed correctly, and also so that > my package which depends on RPI_USERLAND, could also build and be > installed. Everything works as expected. > > Other packages that depend on the arm-only binaries should already > have BR2_arm dependency anyway, correct? Not necessarily: packages that need a EGL/GLES provider could be enabled, and they have no idea about arm or aarch64 (or even other archs). Regards, Yann E. MORIN. > Best regards, > Christian Stewart
Hi Yann, On Wed, Aug 19, 2020 at 2:42 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > On 2020-08-19 13:54 -0700, Christian Stewart spake thusly: > > On Tue, Aug 18, 2020 at 1:16 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > Of course, not having everything is not irrelevant. However, this is > > something we expect the rpi-userland developers to address, not the > > Buildroot developers, correct? > > Right, but the package in Buildroot still declares providing libegl, > libgels, vg, and max, which is clearly wrong in this case. > > With just your patch, the package would still advertise providing those, > but they would not be built. Any pacjage that "just" needs a GL/GLES > provider (and has ptherwise no requirement on the arcitecture) would > fail to build. > > That is what was pointed out in the first review (which I already > pointed to): > > http://lists.busybox.net/pipermail/buildroot/2020-January/271588.html > > ... which already had a proposal patch to fix the provider side of the > package, and also identifies all the delta between what is installed for > arm and for aarch64. Ok, thanks for the clarification, this makes sense. I like the approach of making those flags for GL/GLES support conditional on BR2_arm, what was the status on that? Just need someone to test + format into a series? Best regards, Christian
diff --git a/package/rpi-userland/Config.in b/package/rpi-userland/Config.in index 342faf26e3..81f3588822 100644 --- a/package/rpi-userland/Config.in +++ b/package/rpi-userland/Config.in @@ -1,6 +1,6 @@ config BR2_PACKAGE_RPI_USERLAND bool "rpi-userland" - depends on BR2_arm + depends on BR2_arm || BR2_aarch64 depends on BR2_INSTALL_LIBSTDCPP depends on BR2_TOOLCHAIN_HAS_THREADS depends on !BR2_STATIC_LIBS @@ -40,6 +40,6 @@ config BR2_PACKAGE_RPI_USERLAND_HELLO endif comment "rpi-userland needs a toolchain w/ C++, threads, dynamic library" - depends on BR2_arm + depends on BR2_arm || BR2_aarch64 depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS || \ BR2_STATIC_LIBS diff --git a/package/rpi-userland/rpi-userland.mk b/package/rpi-userland/rpi-userland.mk index 9edc92344e..ec5e26f140 100644 --- a/package/rpi-userland/rpi-userland.mk +++ b/package/rpi-userland/rpi-userland.mk @@ -13,6 +13,10 @@ RPI_USERLAND_CONF_OPTS = -DVMCS_INSTALL_PREFIX=/usr RPI_USERLAND_PROVIDES = libegl libgles libopenmax libopenvg +ifeq ($(BR2_aarch64),y) +RPI_USERLAND_CONF_OPTS += -DARM64=ON +endif + ifeq ($(BR2_PACKAGE_RPI_USERLAND_HELLO),y) RPI_USERLAND_CONF_OPTS += -DALL_APPS=ON
Tested on Pi4 Model B (aarch64). Signed-off-by: Christian Stewart <christian@paral.in> --- package/rpi-userland/Config.in | 4 ++-- package/rpi-userland/rpi-userland.mk | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-)