ASoC: SOF: ipc4: Add support for split firmware releases #5270

ujfalusi · 2024-12-13T14:25:13Z

A split SOF release consists of a base firmware and two libraries:
<fw_filename>-openmodules.ri for processing (audio) modules
<fw_filename>-debug.ri for debug and developer modules

To handle this new release model add infrastructure to try to load the two
library after boot optionally.

This approach will allow flexibility on handling platforms in sof-bin with
single or split configuration:
single release: base firmware only
split release: base firmware + openmodules + debug

The files for the split firmware are located at the firmware directory.

The documentation update is: thesofproject/sof-docs#505

lyakh · 2024-12-16T07:25:01Z

sound/soc/sof/ipc4-loader.c

+
+		kfree(lib_filename);
+		if (ret)
+			break;


why break? If any of the libraries fail loading you don't attempt the rest? In this case it kind of makes sense - if there's no "main" module bundle, there's probably no "debug" either, but isn't the general concept - here's a list of optional libraries, we try to load whatever is available?

The sof_ipc4_load_library() will only return error if there were real error for the loading, the bundle not present is not an error, but if the bundle loading fails then that is an error, something is not right and we will abort the probing altogethe.

We should continue if a bundle fails, this way we still get base FW (and it may have some built-in modules) and a registered sound card. Yes we tell the user that we could not load the library and audio may not fully work as expected.

We will fail to probe the card as the modules form the failed library are missing and the topology will not probe.

OK, if we continue then the car will not come up, but the debugfs for SOF is going the be there to provide further clues.

ujfalusi · 2024-12-16T16:42:24Z

Changes since v1:

follow the sof-docs update [1] and look for the base libraries in the same directory as the firmware

[1] thesofproject/sof-docs#506

ujfalusi · 2024-12-17T10:16:40Z

Changes since v2:

Documentation as inline comment for the sof_ipc4_load_library_bundles() function
fix a memory leak (after kasprintf) and added missing NULL check after kzalloc

ujfalusi · 2024-12-17T14:03:40Z

Changes since v3:

Print warning on library loading error. The topology loading will likely fail and no card will be present, but the SOF debugfs is going to be available.

ranj063 · 2024-12-17T16:25:17Z

sound/soc/sof/ipc4-loader.c

+		ret = firmware_request_nowarn(&fw_lib->sof_fw.fw, lib_filename,
+					      sdev->dev);
+		if (ret < 0) {
+			dev_dbg(sdev->dev, "Library file '%s' is not present\n",


@ujfalusi with the current releases where the openmodule/debug binaries are absent, will we always see this debug message? I think this will too much of a distraction. How about we check the presence of these files before requesting to load the libraries?

Or not print here at all if does not exist...

ujfalusi · 2024-12-18T08:05:55Z

Changes since v4:

Drop the debug print if the optional library is not found in filesystem
Change terms to single and split firmware release (two types of the modular firmware).

ranj063 · 2024-12-18T17:31:55Z

sound/soc/sof/ipc4-loader.c

+ * sof_ipc4_load_library_bundles - loads the library parts of a modular firmware
+ * @sdev: SOF device
+ *
+ * With IPC4 the firmware can be monolithic or modular release.


@ujfalusi you want to remove the monolithic reference in this PR too?

yes, I wanted to... ;)

ujfalusi · 2024-12-19T09:43:42Z

Changes since v5:

replace references to modular to split firmware
change sof_ipc4_load_library_bundles() to sof_ipc4_complete_split_release() to better align with the functionality

bardliao · 2024-12-23T03:15:21Z

SOFCI TEST

bardliao · 2024-12-26T08:39:52Z

SOFCI TEST

lyakh

works with thesofproject/sof#9702

ujfalusi · 2025-01-14T10:32:32Z

@ranj063, can you check the patch again?

lyakh · 2025-01-30T11:54:57Z

sound/soc/sof/ipc4-loader.c

+	if (!lib_name_base)
+		return -ENOMEM;
+
+	strscpy(lib_name_base, fw_filename, sizeof(lib_name_base));


can you check the size please?

which size?

there's only one size - the last argument of strscpy(). It's a size of a pointer, is that what is needed here?

valid, it did worked on most platform with sof-123.ri pattern ;)

it's because on 64-bit systems sizeof(char *) == 8

lyakh · 2025-01-30T11:56:34Z

sound/soc/sof/ipc4-loader.c

 	if (unlikely(ret))
 		goto release;

-	kfree(fw_filename);
-
+	(*lib_id)++;


what sense does it make incrementing this if the caller then ignores it?

it is in the for loop, the lib_id is incremented for the next call?

I think there were a reason why I did this, but cannot recall, I guess the increment can be done in a caller function.
Likely I had a different implementation which I reworked to arrive to this one..

ah, right, it's a loop in sof_ipc4_complete_split_release(), I only looked at sof_ipc4_load_library_by_uuid() where it's just a single call. Ok, it's a bit confusing there, that a value is incremented but then ignored, but I see that it's actually ok. If you make a change, maybe just increment the lib_id in the loop instead of in the function, then you also don't need to change an int to a pointer at multiple locations in the function body

lyakh

2 comments seem too suspicious

A split SOF release consists of a base firmware and two libraries: <fw_filename>-openmodules.ri for processing (audio) modules <fw_filename>-debug.ri for debug and developer modules To handle this new release model add infrastructure to try to load the two library after boot optionally. This approach will allow flexibility on handling platforms in sof-bin with single or split configuration: single release: base firmware only split release: base firmware + openmodules + debug The files for the split firmware are located at the firmware directory. Signed-off-by: Peter Ujfalusi <[email protected]>

lyakh · 2025-01-30T12:57:51Z

sound/soc/sof/ipc4-loader.c

+	if (!lib_name_base)
+		return -ENOMEM;
+
+	strscpy(lib_name_base, fw_filename, sizeof(lib_name_base));


it's because on 64-bit systems sizeof(char *) == 8

ujfalusi · 2025-01-30T13:01:18Z

Changes since v6:

Correct strscpy() size parameter and add comments to explain what is going on
There is no real need to pass the lib_id as pointer to sof_ipc4_load_library(), it can be incremented in sof_ipc4_complete_split_release() if needed.

ujfalusi · 2025-01-30T13:02:12Z

@lyakh, thank you for pointing the issues out, the strscpy() was a really bad from me :(

ujfalusi requested a review from lyakh December 13, 2024 14:25

ujfalusi requested review from lgirdwood, plbossart, ranj063, kv2019i, dbaluta and bardliao as code owners December 13, 2024 14:25

lyakh reviewed Dec 16, 2024

View reviewed changes

ujfalusi force-pushed the peter/sof/pr/ipc4-modular-sof-01 branch from 36acd21 to 1664690 Compare December 16, 2024 16:41

ujfalusi force-pushed the peter/sof/pr/ipc4-modular-sof-01 branch from 1664690 to d09062a Compare December 17, 2024 10:16

ujfalusi force-pushed the peter/sof/pr/ipc4-modular-sof-01 branch from d09062a to 2096214 Compare December 17, 2024 14:02

ranj063 reviewed Dec 17, 2024

View reviewed changes

ujfalusi force-pushed the peter/sof/pr/ipc4-modular-sof-01 branch from 2096214 to 8760089 Compare December 18, 2024 08:04

ujfalusi changed the title ~~ASoC: SOF: ipc4: Add support for modular firmware releases~~ ASoC: SOF: ipc4: Add support for split firmware releases Dec 18, 2024

ranj063 reviewed Dec 18, 2024

View reviewed changes

ujfalusi force-pushed the peter/sof/pr/ipc4-modular-sof-01 branch from 8760089 to 8bc8b0c Compare December 19, 2024 09:42

lyakh mentioned this pull request Jan 6, 2025

LLEXT: build libraries thesofproject/sof#9702

Merged

lyakh approved these changes Jan 7, 2025

View reviewed changes

lgirdwood previously approved these changes Jan 8, 2025

View reviewed changes

ujfalusi requested a review from ranj063 January 9, 2025 09:55

bardliao previously approved these changes Jan 14, 2025

View reviewed changes

kv2019i previously approved these changes Jan 30, 2025

View reviewed changes

lyakh reviewed Jan 30, 2025

View reviewed changes

lyakh requested changes Jan 30, 2025

View reviewed changes

ujfalusi dismissed stale reviews from kv2019i, bardliao, and lgirdwood via 1f641c5 January 30, 2025 12:58

ujfalusi force-pushed the peter/sof/pr/ipc4-modular-sof-01 branch from 8bc8b0c to 1f641c5 Compare January 30, 2025 12:58

lyakh approved these changes Jan 30, 2025

View reviewed changes

kv2019i approved these changes Jan 30, 2025

View reviewed changes

ranj063 approved these changes Jan 30, 2025

View reviewed changes

ranj063 merged commit 1475854 into thesofproject:topic/sof-dev Jan 30, 2025
12 of 14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ASoC: SOF: ipc4: Add support for split firmware releases #5270

ASoC: SOF: ipc4: Add support for split firmware releases #5270

ujfalusi commented Dec 13, 2024 •

edited

Loading

lyakh Dec 16, 2024

ujfalusi Dec 16, 2024

lgirdwood Dec 16, 2024

ujfalusi Dec 17, 2024

ujfalusi Dec 17, 2024

ujfalusi commented Dec 16, 2024

ujfalusi commented Dec 17, 2024

ujfalusi commented Dec 17, 2024

ranj063 Dec 17, 2024

ujfalusi Dec 18, 2024

ujfalusi commented Dec 18, 2024

ranj063 Dec 18, 2024

ujfalusi Dec 19, 2024

ujfalusi commented Dec 19, 2024

bardliao commented Dec 23, 2024

bardliao commented Dec 26, 2024

lyakh left a comment

ujfalusi commented Jan 14, 2025

lyakh Jan 30, 2025

ujfalusi Jan 30, 2025

lyakh Jan 30, 2025

ujfalusi Jan 30, 2025

lyakh Jan 30, 2025

lyakh Jan 30, 2025

ujfalusi Jan 30, 2025

ujfalusi Jan 30, 2025

lyakh Jan 30, 2025

lyakh left a comment

lyakh Jan 30, 2025

ujfalusi commented Jan 30, 2025

ujfalusi commented Jan 30, 2025

ASoC: SOF: ipc4: Add support for split firmware releases #5270

ASoC: SOF: ipc4: Add support for split firmware releases #5270

Conversation

ujfalusi commented Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi commented Dec 16, 2024

ujfalusi commented Dec 17, 2024

ujfalusi commented Dec 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi commented Dec 19, 2024

bardliao commented Dec 23, 2024

bardliao commented Dec 26, 2024

lyakh left a comment

Choose a reason for hiding this comment

ujfalusi commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lyakh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi commented Jan 30, 2025

ujfalusi commented Jan 30, 2025

ujfalusi commented Dec 13, 2024 •

edited

Loading