[Buildroot] [RFC v2 1/1] ti-gfx: add new package
Spenser Gilliland
spenser at gillilanding.com
Wed Jun 26 17:52:52 UTC 2013
On Tue, 25 Jun 2013 22:31:25 +0200
Thomas Petazzoni <thomas.petazzoni at free-electrons.com> wrote:
Dear Thomas,
> Nice, thanks for your work on this. Having support for OpenGL on
> OMAP3, OMAP4 and AM335x is definitely one of the most important part
> of the GSoC.
I completely agree. I'm working very diligently on this.
>
> > Additional Info:
> > I've been using the 3.9.6-x3 tag of the kernel at
> > https://github.com/RobertCNelson/stable-kernel by use of the
> > LINUX_OVERRIDE_SRCDIR option.
>
> Are there some reports of the ti-gfx stuff working on such recent
> kernels? Have you looked at the Yocto meta-ti layer at
> http://git.yoctoproject.org/cgit/cgit.cgi/meta-ti/ ? If I look there,
> they apparently don't have anything more recent than 3.0 or 3.1.
Yes, the RobertCNelson kernel has builds up to 3.9.6. I am using this
with fairly good success. I've been using the meta-ti layer
extensively to understand the process of installing the SGX drivers.
Many things refer to a 3.8 kernel from ti. However, this kernel was
unbootable on the Beagle-XM. This may be a more apporiate option for
the Beagle Black though.
> Also, are you still trying on the Beagle-XM, or have you switched to
> the BeagleBone Black.
The Beagle-XM is the current target. From the start, I was trying to
read the revision number on the SGX core using devmem2. However for
some unknown reason, devmem2 is not working properly on the
Beagle-XM. When I finally tried the devmem from busybox, the revision
was correctly read. Thus, I'm thinking that there is a bug in devmem2
which is causing the problem. See patch here
https://github.com/openembedded/meta-oe/blob/master/meta-oe/recipes-support/devmem2/devmem2/devmem2-fixups-2.patch
> >
> > You must use a soft-float toolchain (ie Code Sourcery) as the
> > binaries provided by the Graphics SDK are soft-float. (hard float
> > is on the TODO list)
>
> Note that I've recently added the Arago toolchain as an external
> toolchain in Buildroot. I don't remember if it's a -mfloat-abi=hard or
> -mfloat-abi=softfp toolchain, though.
I'll run some tests with the Arago toolchain and determine if it's
appropriate to use for this SGX stuff.
> > + select BR2_LINUX_KERNEL
>
> We typically never "select" the kernel, but instead depend on it, and
> have a comment if the kernel is not enabled. This is because we can't
> enable the kernel in the back of the user: the user has to be aware
> that (s)he should go in the kernel menu, and configure the
> version/configuration.
Will fix.
> > +choice
> > + prompt "Target"
> > + default BR2_PACKAGE_TI_GFX_ES3
> > + help
> > + Select the SOC for which you would like to install
> > drivers. Please
> > + use the chart at
> > +
> > http://processors.wiki.ti.com/index.php/OMAP35x_Graphics_SDK_Getting_Started_Guide
> > + +config BR2_PACKAGE_TI_GFX_ES3
> > + bool "es3.x"
>
> That's a detail, but maybe it would be worth doing:
>
> bool "es3.x (OMAP35xx, AM35xx)"
>
> so that the user doesn't have to go in the help text of each option to
> find the right value to use.
Will fix.
> > +comment "requires an external eglibc/glibc based toolchain"
> > + depends on !(BR2_TOOLCHAIN_EXTERNAL_GLIBC ||
> > BR2_TOOLCHAIN_CTNG_eglibc || BR2_TOOLCHAIN_CTNG_glibc)
>
> the toolchain is not necessarily external, it may be using the
> crosstool-ng backend. So just "requires an eglibc/glibc based
> toolchain" is enough.
Will fix.
> > diff --git a/package/ti-gfx/ti-gfx-km_install_modules.patch
> > b/package/ti-gfx/ti-gfx-km_install_modules.patch new file mode
> > 100644 index 0000000..63bfd19
> > --- /dev/null
> > +++ b/package/ti-gfx/ti-gfx-km_install_modules.patch
>
> Missing description + SoB.
Will fix.
>
> > @@ -0,0 +1,14 @@
> > +Index: ti-gfx-4_09_00_01/GFX_Linux_KM/Makefile
> > +===================================================================
> > +--- ti-gfx-4_09_00_01.orig/GFX_Linux_KM/Makefile 2013-03-07
> > 11:00:11.000000000 -0600 ++++
> > ti-gfx-4_09_00_01/GFX_Linux_KM/Makefile 2013-05-23
> > 01:36:29.356676281 -0500 +@@ -479,6 +479,9 @@
> > + all:
> > + $(MAKE) -C $(KERNELDIR) M=`pwd` $*
> > +
> > ++install:
> > ++ $(MAKE) -C $(KERNELDIR) M=`pwd` modules_install
>
> You could directly do this modules_install from the Buildroot .mk
> file, but ok, why not.
Will fix. This was mentioned in the a previous review but I haven't
fixed it yet.
> > --- /dev/null
> > +++ b/package/ti-gfx/ti-gfx-newclkapi.patch
> > @@ -0,0 +1,62 @@
>
> Missing description + SoB.
>
> > +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-18 11:03:06.606245728 -0500 ++++
> > ti-gfx-4_09_00_01/GFX_Linux_KM/services4/system/omap3630/sysutils_linux.c
> > 2013-06-18 11:11:17.908972042 -0500 +@@ -166,11 +166,30 @@
> > + }
> > +
> > + PVR_DPF((PVR_DBG_MESSAGE, "EnableSGXClocks: Enabling SGX
> > Clocks")); +-
> > ++#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,2,0)
> > ++ res=clk_prepare(psSysSpecData->psSGX_FCK);
> > ++ if (res < 0)
> > ++ {
> > ++ PVR_DPF((PVR_DBG_ERROR, "EnableSGXClocks:
> > Couldn't enable SGX functional clock (%d)", res));
> > ++ clk_unprepare(psSysSpecData->psSGX_FCK);
> > ++ return PVRSRV_ERROR_UNABLE_TO_ENABLE_CLOCK;
> > ++ } ++#endif
> > + res=clk_enable(psSysSpecData->psSGX_FCK);
>
>
> One option would have been to replace clk_enable() by
> clk_prepare_enable(), that does both the prepare and enable. But that
> call has been introduced in 3.3 only, so for 3.2, you would still need
> to call clk_prepare().
>
> Or maybe you can do something like:
>
> #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
>
> this way, you can simply the rest of the patch by just doing
> s/clk_enable/clk_prepare_enable/. Ditto for disable/unprepare, of
> course.
Good idea. This sounds easier to port forward. Will fix.
>
> > +@@ -247,8 +271,9 @@
> > + PVR_DPF((PVR_DBG_MESSAGE, "DisableSGXClocks: Disabling
> > SGX Clocks"));
> > +
> > + clk_disable(psSysSpecData->psSGX_FCK);
> > +-
> > ++ clk_unprepare(psSysSpecData->psSGX_FCK);
> > + clk_disable(psSysSpecData->psSGX_ICK);
> > ++ clk_unprepare(psSysSpecData->psSGX_ICK);
>
> Missing kernel version conditionals here.
Oops, will fix with method above.
>
> > +TI_GFX_BIN_PATH = gfx_$(TI_GFX_DEBUG_LIB)_es$(TI_GFX_OMAPES)
> > +
> > +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
> > +
> > +TI_GFX_MAKE_CMD = cd $(@D)/GFX_Linux_KM && \
> > + $(MAKE) $(LINUX_MAKE_FLAGS) \
> > + BUILD=$(TI_GFX_DEBUG_KM) \
> > + TI_PLATFORM=$(TI_GFX_PLATFORM) \
> > + OMAPES=$(TI_GFX_OMAPES) \
> > + SUPPORT_XORG=0 \
> > + KERNELDIR=$(LINUX_DIR)
>
> This is a rather unusual way of doing this. We prefer something like:
>
> TI_GFX_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)
>
> And then do:
>
> define TI_GFX_BUILD_CMDS
> $(MAKE) $(TI_GFX_MAKE_OPTS) -C $(@D)/GFX_Linux_KM
> endef
Will fix. This is much cleaner.
> Also, I'm surprised you're not passing $(TARGET_CONFIGURE_OPTS) to
> ensure the right compiler is used, etc.
This information is included in LINUX_MAKE_FLAGS.
> > +define TI_GFX_BUILD_CMDS
> > + ( $(TI_GFX_MAKE_CMD) all )
> > +endef
>
> Parenthesis are not needed.
Will fix.
> > +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 +endef
> > +
> > +TI_GFX_TARGET_BIN = \
> > + pvrsrvctl \
> > +
> > +ifeq ($(BR2_PACKAGE_TI_GFX_DEBUG),y)
> > +TI_GFX_TARGET_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_IMGPV = "1.9.2188537"
> > +
> > +define TI_GFX_INSTALL_TARGET_CMDS
> > + ( $(TI_GFX_MAKE_CMD) install ) || \
> > + echo "Your kernel configuration must include
> > FB_DA8XX"
> > + for file in $(TI_GFX_TARGET_BIN); do \
> > + $(INSTALL) -D -m 0755
> > $(@D)/$(TI_GFX_BIN_PATH)/$$file $(TARGET_DIR)/usr/bin/$$file; \
> > + done
> > + for sofile in $$(find $(@D)/$(TI_GFX_BIN_PATH) -name
> > "lib*Open*.so") $$(find $(@D)/$(TI_GFX_BIN_PATH) -name
> > "lib*srv*.so") $$(find $(@D)/$(TI_GFX_BIN_PATH) -name "lib*gl*.so")
> > $$(find $(@D)/$(TI_GFX_BIN_PATH) -name "libpvr*.so") $$(find
> > $(@D)/$(TI_GFX_BIN_PATH) -name "lib*GL*.so") $$(find
> > $(@D)/$(TI_GFX_BIN_PATH) -name "libusc.so"); do \
>
> I guess this could maybe be refactored, but we can see that later once
> the whole thing works.
Yes, it's kinda ugly. I'm still working out the issues. Once I know
exactly what must be installed, I can start to clean this up.
> > + if [ "$$(readlink -n $${sofile})" = "" ] ; then \
> > + sobase=$$(basename $${sofile}); \
> > + $(INSTALL) -D -m 0755 $$sofile
> > $(TARGET_DIR)/usr/lib/$${sobase}.$(TI_GFX_IMGPV); \
> > + ln -sf $${sobase}.$(TI_GFX_IMGPV)
> > $(TARGET_DIR)/usr/lib/$${sobase}; \
> > + ln -sf $${sobase}.$(TI_GFX_IMGPV)
> > $(TARGET_DIR)/usr/lib/$${sobase}$$(echo $(TI_GFX_IMGPV) | awk -F.
> > '{print "." $$1}'); \
> > + ln -sf $${sobase}.$(TI_GFX_IMGPV)
> > $(TARGET_DIR)/usr/lib/$${sobase}$$(echo $(TI_GFX_IMGPV) | awk -F.
> > '{print "." $$1 "." $$2}'); \
> > + fi; \
> > + done
> > +endef
> > +
> > +define TI_GFX_CLEAN_CMDS
> > + ( $(TI_GFX_MAKE_CMD) clean )
> > +endef
>
More information about the buildroot
mailing list