torizon / meta-toradex-torizon

Torizon OS OpenEmbedded Distro Layer
MIT License
5 stars 9 forks source link

torizon-core-common: add Alsa UCM configuration #58

Closed EdTheBearded closed 2 months ago

EdTheBearded commented 2 months ago

Due to BSP changes, ALSA configuration files were reworked into a new recipe.

Removing old config: https://git.toradex.com/cgit/meta-toradex-nxp.git/commit/?h=kirkstone-6.x.y&id=54721dc86ece6b41d88f5e9de4262b575efc5393 https://git.toradex.com/cgit/meta-toradex-ti.git/commit/?h=kirkstone-6.x.y&id=d5d48dc1cc1ab54b3a498a0d5ea19646cab381ee

Adding new UCM config: https://git.toradex.com/cgit/meta-toradex-bsp-common.git/commit/?h=kirkstone-6.x.y&id=5183100ea2bb3fe29702f96657e05f4e905433b5

jsrc27 commented 2 months ago

From what I can understand this new alsa-ucm-conf-tdx recipe is only providing configs for the verdin boards right? So optimally we should only be installing this when building for those machines right?

Though I see the BSP installs this recipe unconditionally in their reference images so maybe it's not a big deal: https://git.toradex.com/cgit/meta-toradex-demos.git/commit/?h=kirkstone-6.x.y&id=d7a89bf699d730e3f6081c4da12346d78f6fd8b6

Your thoughts?

EdTheBearded commented 2 months ago

From what I can understand this new alsa-ucm-conf-tdx recipe is only providing configs for the verdin boards right? So optimally we should only be installing this when building for those machines right?

Though I see the BSP installs this recipe unconditionally in their reference images so maybe it's not a big deal: https://git.toradex.com/cgit/meta-toradex-demos.git/commit/?h=kirkstone-6.x.y&id=d7a89bf699d730e3f6081c4da12346d78f6fd8b6

Your thoughts?

Makes sense. I've updated the commit.

rborn-tx commented 2 months ago

From what I can understand this new alsa-ucm-conf-tdx recipe is only providing configs for the verdin boards right? So optimally we should only be installing this when building for those machines right?

Even though I agree this would be optimal, it seems to me this should be a concern for the alsa-ucm-conf-tdx recipe. Otherwise, on every update of the recipe we'd need to re-evaluate our decision on only including the package for verdins and this would probably make us miss it when the BSP adds configs for other machines; well, unless we know there won't be configs for other machines...

P.S.: @EdTheBearded Sorry, I'm commenting this after you updated the PR.

EdTheBearded commented 2 months ago

From what I can understand this new alsa-ucm-conf-tdx recipe is only providing configs for the verdin boards right? So optimally we should only be installing this when building for those machines right?

Even though I agree this would be optimal, it seems to me this should be a concern for the alsa-ucm-conf-tdx recipe. Otherwise, on every update of the recipe we'd need to re-evaluate our decision on only including the package for verdins and this would probably make us miss it when the BSP adds configs for other machines; well, unless we know there won't be configs for other machines...

P.S.: @EdTheBearded Sorry, I'm commenting this after you updated the PR.

It's ok. I've changed it back as well. As you said, this isn't a recipe specific for Verdin (although the installation of the configs should), and if they add more devices in the future, we're already covered.

leograba commented 2 months ago

Tagging @hiagofranco who worked on the ALSA stuff as far as I know, maybe he can help provide some background if needed.

hiagofranco commented 2 months ago

Hi all,

this was done only to Verdin for now because this was the modules being affected by the read-only-rootfs breaking Yocto builds because of different codecs between Dahlia and Dev boards.

> cat recipes-bsp/alsa-state/alsa-state.bbappend

FILESEXTRAPATHS:prepend := "${THISDIR}/alsa-state/:"

# make this machine specific, as we have different codecs with different settings
PACKAGE_ARCH:tdx = "${MACHINE_ARCH}"
> ls recipes-bsp/alsa-state/alsa-state
 apalis-imx6   apalis-imx8   colibri-imx6   colibri-imx7   colibri-imx7-emmc

The alsa bbappend is still there under meta-toradex-nxp layer and it will make Apalis and Colibri work for now. In the future, we will also remove these files and add UCM files for all boards, which you will get for free.

This is current on our backlog and being tracked under ELB-5712. Please feel free to add any comments or any suggestions, or any reccomendations for me, thanks!

jsrc27 commented 2 months ago

@hiagofranco Thank you for clarifying! Guess we'll go ahead with the merge then.