Re: [Buildroot] [PATCH v9 09/11] package/ti-rogue-km: new package

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Andreas, All,

On 2023-06-22 11:02 -0500, Andreas Dannenberg via buildroot spake thusly:
> This package adds the kernel module for the Rogue graphics
> accelerator of the following Texas Instuments SoCs: AM62,
> J721E, J721S2, and J784S4.
> 
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
[--SNIP--]
> diff --git a/package/ti-rogue-km/Config.in b/package/ti-rogue-km/Config.in
> new file mode 100644
> index 0000000000..b1565929d5
> --- /dev/null
> +++ b/package/ti-rogue-km/Config.in
> @@ -0,0 +1,28 @@
> +comment "ti-rogue-km needs a Linux kernel to be built"
> +	depends on BR2_aarch64
> +	depends on !BR2_LINUX_KERNEL
> +
> +config BR2_PACKAGE_TI_ROGUE_KM
> +	bool "ti-rogue-km"
> +	depends on BR2_LINUX_KERNEL && BR2_aarch64
> +	select BR2_LINUX_NEEDS_MODULES
> +	help
> +	  Kernel modules for TI SoCs with Rogue GPU.
> +	  This package supports AM62, J721E, J721S2, and J784S4
> +	  SoCs only.
> +
> +	  Note: it needs a TI specific kernel to build properly.
> +
> +	  https://git.ti.com/cgit/graphics/ti-img-rogue-driver
> +
> +if BR2_PACKAGE_TI_ROGUE_KM
> +
> +config BR2_TARGET_TI_ROGUE_KM_TARGET_PRODUCT
> +	string "KM driver target product"
> +	default "am62_linux"
> +	help
> +	  Select target product to be used for KM driver. This needs to
> +	  match one of the SoC-specific folders within the build/linux
> +	  folder of the ti-img-rogue-driver Git repository.

If you go with the choice option I suggested in my review of patch 2,
then this is going to be trivial to implement without asking the user:

    config BR2_TARGET_TI_ROGUE_KM_TARGET_PRODUCT
        string
        default "am62_linux" if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62X
        default 

> +endif
> diff --git a/package/ti-rogue-km/ti-rogue-km.hash b/package/ti-rogue-km/ti-rogue-km.hash
> new file mode 100644
> index 0000000000..4a08efbb98
> --- /dev/null
> +++ b/package/ti-rogue-km/ti-rogue-km.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  ef50124b3e79fb1d28d3a2b4135032505abbb5c6b337b24a9bed7223d521dcc1  ti-rogue-km-1dd6291a5cad4f2b909fc2a14bd717a3bc5f0bb2-br1.tar.gz
> +sha256  c878f4e43e468329d3dcf4db09054e94472bfeee9f8ab2d69a122b978e9f773e  README
> diff --git a/package/ti-rogue-km/ti-rogue-km.mk b/package/ti-rogue-km/ti-rogue-km.mk
> new file mode 100644
> index 0000000000..7b0a595c71
> --- /dev/null
> +++ b/package/ti-rogue-km/ti-rogue-km.mk
> @@ -0,0 +1,37 @@
> +################################################################################
> +#
> +# ti-rogue-km
> +#
> +################################################################################
> +
> +# This corresponds to SDK 08.06.00
> +TI_ROGUE_KM_VERSION = 1dd6291a5cad4f2b909fc2a14bd717a3bc5f0bb2
> +TI_ROGUE_KM_SITE = https://git.ti.com/git/graphics/ti-img-rogue-driver.git
> +TI_ROGUE_KM_SITE_METHOD = git
> +TI_ROGUE_KM_LICENSE = MIT or GPL-2.0
> +TI_ROGUE_KM_LICENSE_FILES = README
> +
> +TI_ROGUE_KM_DEPENDENCIES = linux
> +
> +PVR_BUILD = "release"
> +PVR_WS = "wayland"
> +
> +TI_ROGUE_KM_MAKE_OPTS = \
> +	$(LINUX_MAKE_FLAGS) \
> +	KERNELDIR=$(LINUX_DIR) \
> +	BUILD=$(PVR_BUILD) \
> +	PVR_BUILD_DIR=$(BR2_TARGET_TI_ROGUE_KM_TARGET_PRODUCT) \
> +	WINDOW_SYSTEM=$(PVR_WS)
> +
> +define TI_ROGUE_KM_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(TI_ROGUE_KM_MAKE_OPTS)
> +endef
> +
> +define TI_ROGUE_KM_INSTALL_TARGET_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) -C $(LINUX_DIR) \
> +		M=$(@D)/binary_$(BR2_TARGET_TI_ROGUE_KM_TARGET_PRODUCT)_$(PVR_WS)_$(PVR_BUILD)/target_aarch64/kbuild \
> +		INSTALL_MOD_PATH=$(TARGET_DIR) \
> +		modules_install
> +endef

Can't we use the kernel-module infra instead?
    https://buildroot.org/downloads/manual/manual.html#_infrastructure_for_packages_building_kernel_modules

i.e. something like:

    TI_ROGUE_KM_MODULE_MAKE_OPTS = \
        PVR_BUILD_DIR=$(BR2_TARGET_TI_ROGUE_KM_TARGET_PRODUCT) \
        WINDOW_SYSTEM=wayland

I'm a bit concerned about the hoops the Makefile goes through to end up
with a path like:
    (@D)/binary_$(BR2_TARGET_TI_ROGUE_KM_TARGET_PRODUCT)_$(PVR_WS)_$(PVR_BUILD)/target_aarch64/kbuild

so maybe it is more complex than that... In any case, it would be good
to explain in the commit log why we can't use our kernel-module infra.

Regards,
Yann E. MORIN.

> +$(eval $(generic-package))
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot