Feature request: ACPI NHLT generation for topology2

[juimonen]

I'd like to add ACPI NHLT generation into topology2 parsing.

Why: In certain future sof firmware versions this is needed for DAI configuration
Why to alsa-utils: adding it to kernel driver was internally rejected, also I can't link to topology2 parsing outside alsa-utils

Good thing: if you ever wondered how acpi nhlt blobs are created and what they contain in intel platforms -> this code is doing it.

Bad things: I can think many, but I'm trying to do this with least harm to every party (kernel driver, firmware)

Example of the code can be found here: thesofproject#4

So mainly asking for comments/suggestions from @perexg

@lrgirdwo @ranj063 FYI.

[perexg]

NHLT table is an Intel specific extension, isn't ? My understanding is that the hardware vendor should provide the right contents in BIOS/UEFI. For possible ACPI table override, we can use the standard linux's ACPI layer. I may miss something, so please, explain your goal.

If we decide to handle this table in the topology files, the topology should just handle a binary blob. The generator may be implemented as a standalone tool.

[juimonen]

@perexg hmm yeah kind of agree. Is there a possibility for me to link somehow to topology parsing, because then I wouldn't need to write my own parser for the topology (as alsa-utils is doing this)? the nhlt blobs are generated from dai values in tplg.conf...

[juimonen]

@perexg we've seen also erroneous bios nhlt blobs -> we would like to override those from topology, one way or another

[perexg]

@perexg hmm yeah kind of agree. Is there a possibility for me to link somehow to topology parsing, because then I wouldn't need to write my own parser for the topology (as alsa-utils is doing this)? the nhlt blobs are generated from dai values in tplg.conf...

See snd_tplg_decode() / snd_tplg_save() - you can get the raw contents back and grab the wanted values from the ALSA configuration tree. Try alsatplg -d file.tplg -o alsa-raw-tplg.conf.

@perexg we've seen also erroneous bios nhlt blobs -> we would like to override those from topology, one way or another

I see. I'm just not convinced to carry this override in the topology files.

[crojewsk]

Hmm.. if one encounters platform with broken NHLT table, somehow kernel would have to be notified about need for taking different topology (either entire one or part of it) e.g.: through DMI. Otherwise we end up in situation when the entire copy of 'correct' blobs is always part of topology whether or not NHLT from BIOS is ok.

If that's the case, we can always use standard ACPI-table upgrade/overwrite. Early 2020 I've added NHLT to list of standard ACPI tables, see: ACPICA: Add NHLT table signature
From that point onward you can overwrite/upgrade NHLT just like any other ACPI table. Documentation found in thesofproject:
https://github.com/thesofproject/acpi-scripts applies here too.

[juimonen]

@crojewsk I would really like to get rid of sending these blobs to firmware altogether :) but as I can't change the interface in certain versions of it, I would like to have a mechanism to alter the blobs in a easy way. Using the acpi struct here is just a means to carry the "collection" of intel private blobs to kernel -> don't want to invent the wheel again to pack these into some other container.

[lgirdwood]

@perexg to give you some background, we need to support the following uses cases.

1. Load NHLT from ACPI (Windows/Linux)
2. Load NHLT from topology (Chrome, NHLT in ACPI not supported on Chrome)
3. Override bad ACPI NHLT via updated topology file
4. Override bad ACPI NHLT via updated ACPI table.

The NHLT data and the topology data are tightly coupled and duplicate certain items. e.g. the I2S configuration is contained within both topology tuples and NHLT blob (topology tuples for drivers and NHLT I2S IP register values for FW) but is generated from the same topology source text.

The update here allows for the generation of NHLT data blobs based on the topology configuration for rapid community development and debug. i.e. you change the topology text, rebuild and then both driver and FW configurations are in sync.

[perexg]

Thanks for the clarification @lgirdwood . Isn't better to use just a module option (with the NHLT binary) to override the NHLT table in the sound driver on demand (udev rules may set the filename). I think that chrome already sets the firmware/topology files through udev. You can resolve 2) and 3) with it. I feel a bit that using topology for the ACPI tables is a bit misuse. The only benefit is that you have all-in-one file.

[lgirdwood]

@perexg yes for module option to override on regular Linux, we may also have some cases where a quirk is needed based on DMI. The NHLT data file still needs generated though and must align with the topology, but I think the PR also has an option to generate two files (NHLT blob and topology binary) from the same source data. @juimonen pls confirm.

Chrome is a different case though where there is no NHLT data in ACPI so it has to come from the a file, so it's definitely easier to deploy as a single binary file on Chrome (topology tuples + NHLT blob stitched together).

The kernel code for NHLT module option or NHLT + topology option should be the same albeit the data is loaded differently so no issue there, my only concern is synchronizing each NHLT blob file with a corresponding topology file as udev rules would need to be updated in parallel with topology (otherwise we would have non functional audio) ?

[crojewsk]

@juimonen could you elaborate on 'reinvent the wheel' part? I may simply be missing something.

[juimonen]

@juimonen could you elaborate on 'reinvent the wheel' part? I may simply be missing something.

I mean, I'm using the acpi nhlt here as a container to package the vendor blobs and sending them to kernel. In principle I could add these blobs to some other container or add them as individual data blobs to dais. As we have the nhlt header in kernel, why not to use the acpi thing as it can do this.

So I'm not trying to replace the BIOS blob here just for the sake of replacing it, just trying to get these vendor blobs to kernel from user space.

NHLT doesn't specify the vendor specific blobs, but we are creating them already in sof firmware code. So this is just combining the blob generation and sending the thing as ACPI container, as in certain firmware versions the IPC expects the blob to be sent outside. So in kernel I can re-use the code that you guys had in skl driver -> to dig out the vendor blob.

This is quite handy when trying different formats or combination of parameters for the firmware -> just change value in topology file, compile it and test. And you clearly see what values you are setting and where. I think it is easier than doing ACPI updates. So trying to maintain the sof flexibility with topology even though the fw ipc changes.

Sorry for the extended explanation, but at least me this is/was very useful in practice for developing.

[crojewsk]

@juimonen There is nothing to be sorry about. <3 for in-depth explanation, truly.

[perexg]

The other question is, why the parameters in the NHLT table should be encoded in the user space and decoded back in the kernel space as an ACPI table. The topology has already tokens, so you can just define extra tokens for the missing stuff and the acpi functions in kernel may just redirected to the code which gets this from the topology configuration directly instead from the ACPI table blob. You already noted that some information is doubled.

[juimonen]

@perexg unfortunately we have a "blob" interface in some firmware versions and I can't change the interface. The blob generation is not trivial.. for example the dmic code calculates filter coefficients and bit twiddles direct hw register values from topology tokens -> the resulting blob can be like 6kbytes.

1. Now my first idea was to add this to kernel driver, but that was rejected in our internal review.
   • a lot of code to driver, maybe also time critical as the calculation takes time
2. Ok, second idea is to add it to alsa-utils, so we can get the blob conditionally with 1 command.
   • this is a conditional sub "branch" of topology2 auto attr processing, so probably would not harm people
   • but I understand that there are other issues (intel specific, maintenance etc)
3. I could maybe add the blob by pre scanning the topology with some standalone tool, before calling alsatplg
   • just no nice way to tap into the parsing
   • sof developer should use multiple tools to get the blob to topology if needed

So kind of all options are bad in some sense. Not sure if I could call some sof tool from alsa-utils topology2 parsing to generate the blob if needed? not sure if it would create some kind of unwanted interdependency...

[lgirdwood]

@perexg the kernel doesn't decode most of the NHLT data as most is consumed directly by the firmware e.g. the I2S register programming values, DMIC register values, DMIC coefficients. The kernel consumes the IPC tuples that correspond to the examples, e.g. we have a driver tuple for I2S BCLK speed (to program the codec driver) that corresponds to a I2S register value in the NHLT blob (that is sent to FW to program the FW BCLK).

The kernel really walks the the containers within the NHLT data so it knows when and what to send to FW depending on the use case.

Ideally we would like to do option 2 as this means users don't have to install additional SOF packages to debug/update setting on new laptops, i.e. they can edit the topology text file, reload the driver modules and retest.

We can do 3 if needed, but this would mean having the tooling outside of ALSA and will give users an extra step when debugging new devices.