[Buildroot] [RFC v3 1/1] ti-gfx: add new package
Thomas Petazzoni
thomas.petazzoni at free-electrons.com
Fri Jun 28 16:26:06 UTC 2013
Dear Spenser Gilliland,
On Thu, 27 Jun 2013 17:56:06 -0500, Spenser Gilliland wrote:
> diff --git a/package/ti-gfx/Config.in b/package/ti-gfx/Config.in
> new file mode 100644
> index 0000000..bef8526
> --- /dev/null
> +++ b/package/ti-gfx/Config.in
> @@ -0,0 +1,64 @@
> +config BR2_PACKAGE_TI_GFX
> + bool "ti-gfx"
> + select BR2_PACKAGE_HAS_OPENGL_EGL
> + select BR2_PACKAGE_HAS_OPENGL_ES
> + depends on BR2_LINUX_KERNEL && (BR2_TOOLCHAIN_EXTERNAL_GLIBC || \
> + BR2_TOOLCHAIN_CTNG_eglibc || BR2_TOOLCHAIN_CTNG_glibc)
> + help
> + Graphics libraries for TI boards.
> +
> + http://downloads.ti.com/dsps/dsps_public_sw/gfxsdk/
> +
> +if BR2_PACKAGE_TI_GFX
> +
> +config BR2_PACKAGE_TI_GFX_DEBUG
> + bool "enable debug support"
> + help
> + Turn on debugging in kernel module and install libraries built with
> + debugging enabled
and also various tools.
> +config BR2_PACKAGE_TI_GFX_DEMOS
> + bool "install demos"
> + help
> + install the OGLES2ChameleonMan and OGLES2MagicLantern demos
> +
> +config BR2_PACKAGE_TI_GFX_HARD_FLOAT
> + bool "use hard float binaries"
> + help
> + Install hard float binaries (required if using a hard float toolchain)
I don't think this should be a sub-options of the package. I believe we
need to add an additional eabihf ABI next to the existing oabi
(deprecated) and eabi. But that's a larger work. But that's too much
work to do right now, so maybe a sub-option is a good intermediate
solution.
> diff --git a/package/ti-gfx/esrev.sh b/package/ti-gfx/esrev.sh
> new file mode 100644
> index 0000000..4733ea9
> --- /dev/null
> +++ b/package/ti-gfx/esrev.sh
> @@ -0,0 +1,59 @@
> +#!/bin/sh
> +
> +# Debug script to determine proper ES revision for the current board. The
> +# pvrsrvkm module must be insmoded before attempting to get the es rev.
Is there a real reason to keep this within the Buildroot sources? The
Config.in options are pretty clear on which ES revision to choose for
which SoC, no?
> diff --git a/package/ti-gfx/ti-gfx-newclkapi.patch b/package/ti-gfx/ti-gfx-newclkapi.patch
> new file mode 100644
> index 0000000..eb4c301
> --- /dev/null
> +++ b/package/ti-gfx/ti-gfx-newclkapi.patch
> @@ -0,0 +1,72 @@
> +This patch adjusts the omap3630 portion of the powervr driver to use the new
> +clk kernel api.
> +
> +Signed-off-by: Spenser Gilliland <spenser at gillilanding.com>
> +----
> +Index: ti-gfx-4_09_00_01/GFX_Linux_KM/services4/system/omap3630/sysutils_linux.c
> +===================================================================
> +--- ti-gfx-4_09_00_01.orig/GFX_Linux_KM/services4/system/omap3630/sysutils_linux.c 2013-06-27 14:57:02.687204341 -0500
> ++++ ti-gfx-4_09_00_01/GFX_Linux_KM/services4/system/omap3630/sysutils_linux.c 2013-06-27 15:04:44.103212764 -0500
> +@@ -153,6 +153,28 @@
> + psTimingInfo->ui32ActivePowManLatencyms = SYS_SGX_ACTIVE_POWER_LATENCY_MS;
> + }
> +
> ++#if LINUX_VERSION_CODE < KERNEL_VERSION(3,2,0)
> ++int clk_prepare_enable(struct clk *clk)
> ++{
> ++ return clk_enable(clk);
> ++}
> ++#elif LINUX_VERSION_CODE < KERNEL_VERSION(3,3,0)
> ++int clk_prepare_enable(struct clk *clk)
> ++{
> ++ res = clk_prepare(clk);
> ++ if (ret < 0)
> ++ return res;
> ++
> ++ res = clk_enable(clk);
> ++ if (res < 0) {
> ++ clk_unprepare(clk);
> ++ return res;
> ++ }
> ++
> ++ return 0;
> ++}
> ++#endif
> ++
> + PVRSRV_ERROR EnableSGXClocks(SYS_DATA *psSysData)
> + {
> + #if !defined(NO_HARDWARE)
> +@@ -167,14 +189,14 @@
> +
> + PVR_DPF((PVR_DBG_MESSAGE, "EnableSGXClocks: Enabling SGX Clocks"));
> +
> +- res=clk_enable(psSysSpecData->psSGX_FCK);
> ++ res=clk_prepare_enable(psSysSpecData->psSGX_FCK);
> + if (res < 0)
> + {
> + PVR_DPF((PVR_DBG_ERROR, "EnableSGXClocks: Couldn't enable SGX functional clock (%d)", res));
> + return PVRSRV_ERROR_UNABLE_TO_ENABLE_CLOCK;
> + }
> +
> +- res=clk_enable(psSysSpecData->psSGX_ICK);
> ++ res=clk_prepare_enable(psSysSpecData->psSGX_ICK);
> + if (res < 0)
> + {
> + PVR_DPF((PVR_DBG_ERROR, "EnableSGXClocks: Couldn't enable SGX interface clock (%d)", res));
> +@@ -374,14 +396,14 @@
> + rate = clk_get_rate(psSysSpecData->psGPT11_FCK);
> + PVR_TRACE(("GPTIMER11 clock is %dMHz", HZ_TO_MHZ(rate)));
> +
> +- res = clk_enable(psSysSpecData->psGPT11_FCK);
> ++ res = clk_prepare_enable(psSysSpecData->psGPT11_FCK);
> + if (res < 0)
> + {
> + PVR_DPF((PVR_DBG_ERROR, "EnableSystemClocks: Couldn't enable GPTIMER11 functional clock (%d)", res));
> + goto ExitError;
> + }
> +
> +- res = clk_enable(psSysSpecData->psGPT11_ICK);
> ++ res = clk_prepare_enable(psSysSpecData->psGPT11_ICK);
> + if (res < 0)
> + {
> + PVR_DPF((PVR_DBG_ERROR, "EnableSystemClocks: Couldn't enable GPTIMER11 interface clock (%d)", res));
You're not handling clk_disable/clk_unprepare.
> diff --git a/package/ti-gfx/ti-gfx.mk b/package/ti-gfx/ti-gfx.mk
> new file mode 100644
> index 0000000..b7b8d94
> --- /dev/null
> +++ b/package/ti-gfx/ti-gfx.mk
> @@ -0,0 +1,163 @@
> +###############################################################################
> +#
> +# ti-gfx
> +#
> +###############################################################################
> +
> +TI_GFX_VERSION = 4_09_00_01
> +TI_GFX_IMGPV = 1.9.2188537
This one would probably need a comment, and maybe a better name, like
TI_GFX_SO_VERSION or something like that.
> +ifeq ($(BR2_PACKAGE_TI_GFX_HARD_FLOAT),y)
> +TI_GFX_SOURCE = Graphics_SDK_setuplinux_$(TI_GFX_VERSION)_hardfp_minimal_demos.bin
> +else
> +TI_GFX_SOURCE = Graphics_SDK_setuplinux_$(TI_GFX_VERSION)_minimal_demos.bin
> +endif
> +
> +TI_GFX_SITE = http://software-dl.ti.com/dsps/dsps_public_sw/sdo_sb/targetcontent/gfxsdk/$(TI_GFX_VERSION)/exports/
> +TI_GFX_LICENSE = Technology / Software Publicly Available
> +TI_GFX_LICENSE_FILES = TSPA.txt
> +TI_GFX_INSTALL_STAGING = YES
> +
> +TI_GFX_DEPENDENCIES = linux
> +
> +ifeq ($(BR2_PACKAGE_TI_GFX_ES3),y)
> +TI_GFX_OMAPES = 3.x
> +TI_GFX_PLATFORM = omap3
> +endif
> +ifeq ($(BR2_PACKAGE_TI_GFX_ES5),y)
> +TI_GFX_OMAPES = 5.x
> +TI_GFX_PLATFORM = omap3630
> +endif
> +ifeq ($(BR2_PACKAGE_TI_GFX_ES6),y)
> +TI_GFX_OMPAES = 6.x
> +TI_GFX_PLATFORM = ti81xx
> +endif
> +ifeq ($(BR2_PACKAGE_TI_GFX_ES8),y)
> +TI_GFX_OMAPES = 8.x
> +TI_GFX_PLATFORM = ti335x
> +endif
> +
> +ifeq ($(BR2_PACKAGE_TI_GFX_DEBUG),y)
> +TI_GFX_DEBUG_LIB = dbg
> +TI_GFX_DEBUG_KM = debug
> +else
> +TI_GFX_DEBUG_LIB = rel
> +TI_GFX_DEBUG_KM = release
> +endif
> +
> +TI_GFX_BIN_PATH = gfx_$(TI_GFX_DEBUG_LIB)_es$(TI_GFX_OMAPES)
> +
> +TI_GFX_KM_MAKE_OPTS = \
> + $(LINUX_MAKE_FLAGS) \
> + BUILD=$(TI_GFX_DEBUG_KM) \
> + TI_PLATFORM=$(TI_GFX_PLATFORM) \
> + OMAPES=$(TI_GFX_OMAPES) \
> + SUPPORT_XORG=0 \
> + KERNELDIR=$(LINUX_DIR)
> +
> +TI_GFX_DEMO_MAKE_OPTS = \
> + PLATFORM=LinuxARMV7 \
> + Common=1 \
> + X11BUILD=0
> +
> +# The only required binary is pvrsrvctl all others are optional
> +TI_GFX_BIN = pvrsrvctl
> +
> +ifeq ($(BR2_PACKAGE_TI_GFX_DEBUG),y)
> +TI_GFX_BIN += \
> + eglinfo ews_server ews_server_es2 ews_test_gles1 ews_test_gles2 \
> + ews_test_swrender gles1test1 gles2test1 pvr2d_test services_test \
> + sgx_blit_test sgx_clipblit_test sgx_flip_test sgx_init_test \
> + sgx_render_flip_test
> +endif
> +
> +TI_GFX_LIB = \
> + libEGL.so libews.so libGLES_CM.so libGLESv2.so libglslcompiler.so \
> + libIMGegl.so libpvr2d.so libpvrEWS_WSEGL.so libpvrPVR2D_BLITWSEGL.so \
> + libpvrPVR2D_FLIPWSEGL.so libpvrPVR2D_FRONTWSEGL.so \
> + libpvrPVR2D_LINUXFBWSEGL.so libpvrPVRScopeServices.so \
> + libsrv_init.so libsrv_um.so libusc.so
> +
> +TI_GFX_DEMOS = ChameleonMan MagicLantern
> +TI_GFX_DEMOS_LOC = GFX_Linux_SDK/OGLES2/SDKPackage/Demos
> +TI_GFX_DEMOS_MAKE_LOC = OGLES2/Build/LinuxGeneric
> +TI_GFX_DEMOS_BIN_LOC = OGLES2/Build/LinuxARMV7/ReleaseRaw/
> +
> +define TI_GFX_EXTRACT_CMDS
> + $(RM) -rf $(TI_GFX_DIR)
> + chmod +x $(DL_DIR)/$(TI_GFX_SOURCE)
> + printf "Y\nY\n qY\n\n" | $(DL_DIR)/$(TI_GFX_SOURCE) \
> + --prefix $(@D) \
> + --mode console
> +endef
> +
> +define TI_GFX_BUILD_KM_CMDS
> + $(MAKE) $(TI_GFX_KM_MAKE_OPTS) -C $(@D)/GFX_Linux_KM all
> +endef
Any reason not to build everything, i.e "make all" at the root? This
automatically builds the demos and so on.
> +define TI_GFX_BUILD_DEMO_CMDS
> + $(foreach demo, $(TI_GFX_DEMOS), \
> + cd $(@D)/$(TI_GFX_DEMOS_LOC)/$(demo)/$(TI_GFX_DEMOS_MAKE_LOC); \
> + $(TARGET_MAKE_ENV) $(MAKE) $(TI_GFX_DEMO_MAKE_OPTS);)
> +endef
So you could get rid of this, I believe.
> +define TI_GFX_BUILD_CMDS
> + $(call TI_GFX_BUILD_KM_CMDS)
> + $(if $(BR2_PACKAGE_TI_GFX_DEMOS),$(call TI_GFX_BUILD_DEMO_CMDS))
> +endef
And simplify this one.
BTW, there's no need to do a $(call ...) here. Just use the variable.
> +define TI_GFX_INSTALL_STAGING_CMDS
> + for incdir in EGL EWS GLES2 KHR; do \
> + $(INSTALL) -d $(STAGING_DIR)/usr/include/$$incdir; \
> + $(INSTALL) -D -m 0644 $(@D)/include/OGLES2/$$incdir/*.h \
> + $(STAGING_DIR)/usr/include/$$incdir; \
> + done
> + $(INSTALL) -D -m 0755 $(@D)/$(TI_GFX_BIN_PATH)/*.so $(STAGING_DIR)/usr/lib
Why not use TI_GFX_LIB here, for consistency?
> +endef
> +
> +define TI_GFX_INSTALL_KM_CMDS
> + $(MAKE) $(TI_GFX_KM_MAKE_OPTS) -C $(@D)/GFX_Linux_KM install
> +endef
> +
> +define TI_GFX_INSTALL_BINS_CMDS
> + $(foreach bin,$(TI_GFX_BIN),
> + $(INSTALL) -D -m 0755 $(@D)/$(TI_GFX_BIN_PATH)/$(bin) \
> + $(TARGET_DIR)/usr/bin/$(bin))
> +endef
> +
> +define TI_GFX_INSTALL_LIBS_CMDS
> + # create symlinks and install libraries. these libraries do not have a
> + # SONAME defined; therefore, they will not be automatically renamed and
> + # must be renamed manually.
Even if they had a SONAME, why would they be automatically renamed?
> + $(foreach lib,$(TI_GFX_LIBS),
> + $(INSTALL) -D -m 0755 $(@D)/$(TI_GFX_BIN_PATH)/$(lib) \
> + $(TARGET_DIR)/usr/lib/$(lib).$(TI_GFX_IMGPV); \
> + ln -sf $(lib).$(TI_GFX_IMGPV) \
> + $(TARGET_DIR)/usr/lib/$(lib)$$(echo $(TI_GFX_IMGPV) \
> + | awk -F. '{print "." $$1} ); \
> + ln -sf $(lib).$(TI_GFX_IMGPV) \
> + $(TARGET_DIR)/usr/lib/$(lib)$$(echo $(TI_GFX_IMGPV) \
> + | awk -F. '{print "." $$1 "." $$2} );)
I am not sure we need two symbolic links per library, just one would be
enough, so something like:
$(foreach lib,$(TI_GFX_LIBS),
$(INSTALL) -D -m 0755 $(@D)/$(TI_GFX_BIN_PAT)/$(lib) \
$(TARGET_DIR)/usr/lib/$(lib).$(TI_GFX_IMGPV) ; \
ln -sf $(lib).$(TI_GFX_IMGPV) \
$(TARGET_DIR)/usr/lib/$(lib))
and that's it.
> + # libs use the following file for configuration.
> + $(install) -D -m 0644 package/ti-gfx/powervr.ini $(TARGET_DIR)/etc/powervr.ini
$(INSTALL)
> +endef
> +
> +define TI_GFX_INSTALL_DEMOS_CMDS
> + $(foreach demo,$(TI_GFX_DEMOS),
> + $(INSTALL) -D -m 0755 \
> + $(@D)/$(TI_GFX_DEMOS_LOC)/$$demo/$(TI_GFX_DEMOS_BIN_LOC)/OGLES2$${demo} \
> + $(TARGET_DIR)/usr/bin/OGLES2$${demo};)
Aren't the .pod files needed for execution? I don't know, I haven't
tested to run the demos without them.
> +endef
> +
> +define TI_GFX_INSTALL_INIT_SYSV
> + $(INSTALL) -D -m 0755 package/ti-gfx/ti-gfx.sysv /etc/init.d/S80ti-gfx
> +endef
You should use <pkg>_INSTALL_INIT_SYSV.
> +define TI_GFX_INSTALL_TARGET_CMDS
> + $(call TI_GFX_INSTALL_KM_CMDS)
> + $(call TI_GFX_INSTALL_BINS_CMDS)
> + $(call TI_GFX_INSTALL_LIBS_CMDS)
> + $(if $(BR2_PACKAGE_TI_GFX_DEMOS),$(call TI_GFX_INSTALL_DEMOS_CMDS))
No need to use $(call ...), just use the variable directly.
> +endef
> +
> +$(eval $(generic-package))
> diff --git a/package/ti-gfx/ti-gfx.sysv b/package/ti-gfx/ti-gfx.sysv
Should be named S80ti-gfx. We generally name the scripts in
package/<foo>/ with the filename they will have on the target, so that
if you're looking for the source of a script, you can do a "find .
-name 'S80ti-gfx'" and find it in the Buildroot sources.
> new file mode 100644
> index 0000000..372750f
> --- /dev/null
> +++ b/package/ti-gfx/ti-gfx.sysv
> @@ -0,0 +1,45 @@
> +#!/bin/sh
> +
> +BITSPERPIXEL="$(fbset | grep geom | awk '{print $6}')"
> +YRES="$(fbset | grep geom | awk '{print $3}')"
> +
> +if [ "$1" = "" ]; then
> + echo PVR-INIT: Please use start, stop, or restart.
> + exit 1
> +fi
> +
> +if [ "$1" = "stop" -o "$1" = "restart" ]; then
> + echo Stopping PVR
> + rmmod bufferclass_ti
> + rmmod omaplfb
> + rmmod pvrsrvkm
> +fi
> +
> +if [ "$1" = "stop" ]; then
> + exit 0
> +fi
> +
> +# Set RGBA ordering to something the drivers like
> +if [ "$BITSPERPIXEL" = "32" ] ; then
> + fbset -rgba 8/16,8/8,8/0,8/24
> +fi
> +
> +# Try to enable triple buffering when there's enough VRAM
> +fbset -vyres $(expr $YRES \* 3)
> +
> +modprobe omaplfb
> +modprobe bufferclass_ti
> +
> +pvr_maj=`grep "pvrsrvkm$" /proc/devices | cut -b1,2,3`
> +bc_maj=`grep "bc" /proc/devices | cut -b1,2,3`
> +
> +if [ -e /dev/pvrsrvkm ] ; then
> + rm -f /dev/pvrsrvkm
> +fi
> +
> +mknod /dev/pvrsrvkm c $pvr_maj 0
> +chmod 666 /dev/pvrsrvkm
> +
> +if ! /usr/bin/pvrsrvctl --start --no-module; then
> + echo "powervr: unable to start server"
> +fi
Why not make this look more usual, with a case for start, stop, restart
and so on?
I think we're almost good. I believe the next revision you can remove
the "RFC" tag and use "PATCH" instead. We want to get this merged now.
Thanks for the good work!
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
More information about the buildroot
mailing list