[Buildroot] [PATCH] Add package gpu-viv-bin-mx6q to the freescale-imx directory
Arnout Vandecappelle
arnout at mind.be
Sun May 12 18:43:49 UTC 2013
On 11/05/13 12:20, Thomas Petazzoni wrote:
> Dear Henk Fijnvandraat,
>
> Arnout made some good comments, I have a few others below.
>
> On Thu, 9 May 2013 21:53:32 +0200, Henk Fijnvandraat wrote:
[snip]
>> +# The archive is a shell-self-extractor of a bzipped tar. It happens
>> +# to extract in the correct directory (gpu-viv-bin-mx6q-x.y.z)
>> +# The --force makes sure it doesn't fail if the source dir already exists.
>> +# The --auto-accept skips the license check - not needed for us
>> +# because we have legal-info.
>> +define GPU_VIV_BIN_MX6Q_EXTRACT_CMDS
>> + cd $(BUILD_DIR); \
>> + sh $(DL_DIR)/$(GPU_VIV_BIN_MX6Q_SOURCE) --force --auto-accept
>> +endef
>
> I think we generally prefer to have the entire command executed in a
> sub-shell, i.e:
>
> (cd $(BUILD_DIR); \
> sh $(DL_DIR)/$(GPU_VIV_BIN_MX6Q_SOURCE) --force --auto-accept)
Begging to differ here... I don't see much point of having three levels
of shells. make already starts a shell, then we start another shell to
run the extractor script. The third level added by the () looks pretty
redundant to me.
> Also, what guarantees us that it will actually be extracted to the
> right subfilter in $(BUILD_DIR) ?
Well, that's mentioned in the comment above :-). If it would be
extracted into a different directory, the only option would be to rename
it - but you still need to know the name of the directory in which it is
extracted by the extractor script.
>> +define GPU_VIV_BIN_MX6Q_INSTALL_STAGING_CMDS
>> + cp -r $(@D)/usr/* $(STAGING_DIR)/usr
>> +endef
>> +
>> +ifdef $(BR2_PACKAGE_GPU_VIV_BIN_IMX6Q_EXAMPLES)
>> +define GPU_VIV_BIN_MX6Q_INSTALL_EXAMPLES
>> + cp -r $(@D)/opt/* $(TARGET_DIR)/opt
>> +endef
>> +endif
>
> Maybe usr/share/gpu-viv-imx6q/examples/ is a better location than opt/ ?
Agreed, except that normally the share/ directory contains
platform-independent things like man pages, while the examples are all
binaries.
> Also, the installation steps should be after the build steps. It
> doesn't change anything from a functional point of view, but it's the
> way we write all packages, just because it is more logical.
>
>> +ifeq ($(BR2_PACKAGE_XORG7),y)
>> +GPU_VIV_BIN_MX6Q_LIB_TARGET = x11
>> +else
>> +# DirectFB is not supported (wrong version)
>> +GPU_VIV_BIN_MX6Q_LIB_TARGET = fb
>> +endif
>
> Hum, so if X.org is not selected, this package installs something that
> doesn't work? Seems strange.
Comment could be better: directfb (called dfb by this package) doesn't
work, but fb (= linux framebuffer without windowing layer) works.
> Also, I see that this installs EGL libraries, I thought EGL was here to
> allow the creation of OpenGL contexts when there is no windowing system
> such as X.org. For example, with the Rasberry Pi libraries, you can
> have EGL+OpenGL and Qt running on top of it, without DirectFB or X.org.
> Isn't that also possible here?
>
> Are you sure that the "fb" target actually requires DirectFB ?
>
>> +# Instead of building, we fix up the inconsistencies that exist
>> +# in the upstream archive here.
>> +# Make sure these commands are idempotent.
>> +define GPU_VIV_BIN_MX6Q_BUILD_CMDS
>> + $(SED) 's/defined(LINUX)/defined(__linux__)/g' $(@D)/usr/include/*/*.h
>> + for lib in EGL GAL VIVANTE; do \
>> + ln -sf lib$${lib}-$(GPU_VIV_BIN_MX6Q_LIB_TARGET).so \
>> + $(@D)/usr/lib/lib$${lib}.so; \
>> + done
>> + ln -sf libGL.so.1.2 $(@D)/usr/lib/libGL.so.1
>> + ln -sf libGL.so.1.2 $(@D)/usr/lib/libGL.so
>> +endef
>> +
>> +# On the target, remove the unused libraries.
>> +# Note that this is _required_, else ldconfig may create symlinks
>> +# to the wrong library
>> +define GPU_VIV_BIN_MX6Q_INSTALL_TARGET_CMDS
>> + $(GPU_VIV_BIN_MX6Q_INSTALL_EXAMPLES)
>> + cp -a $(@D)/usr/lib $(TARGET_DIR)/usr
>> + for lib in EGL GAL VIVANTE; do \
>> + for f in $(TARGET_DIR)/usr/lib/lib$${lib}-*.so; do \
>> + case $$f in \
>> + *-$(GPU_VIV_BIN_MX6Q_LIB_TARGET).so) : ;; \
>> + *) $(RM) $$f ;; \
>> + esac; \
>> + done; \
>> + done
>
> Isn't it either to only copy the relevant libraries instead of copying
> everything, and then remove everything that's not useful?
There are 16 libraries to install, so it makes sense to use a wildcard.
And I couldn't find a glob pattern that excludes the -x11 and -dfb files.
So removing them was easier.
[In case it wasn't clear: Henk and I collaborated on this patch]
Regards,
Arnout
>
> Thanks!
>
> Thomas
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
More information about the buildroot
mailing list