rimage: add ipc4 extended module config introduction #451

RanderWang · 2023-01-31T08:51:57Z

This is a introduction of ipc4 extended module config and how to add these information in config file xxx.toml for ipc4.

RanderWang · 2023-01-31T08:55:29Z

@cujomalainey this is the document for thesofproject/rimage#125

mmaka1 · 2023-02-02T12:36:48Z

developer_guides/rimage/ipc4_extended_module_config.rst

+| **uuid**: uuid of the module
+| **affinity_mask**: usable dsp core mask, bit N is for core N, 0x3 means core 0 and 1 can be used
+| **instance_count**: max module instance count can be active in fw
+| **domain_types**: two schedule domains: domain ll (0), and dp (1)


I'd assume these are indexes of bits in the mask since you may define a module that works in both domains? But you provided value of "0" in the example above which suggests these are mutually exclusive modes?

I find "domain_types LL DP" in real case.
Let's use index of bit. Thanks!

should give a bit more guidance here about which to choose for developers who may just building a standalone module are not SOF developers. LL and DP mean nothing to outsiders. To be honest I only know edf and ll, I have no clue what dp is.

@cujomalainey Some of this is referring to ongoing work, which understandably makes this hard to follow. For DP, the PR to add this concept to SOF was just submitted for review thesofproject/sof#7089

developer_guides/rimage/ipc4_extended_module_config.rst

RanderWang · 2023-02-03T05:32:06Z

updated comments for domain type

kv2019i · 2023-02-03T14:13:45Z

developer_guides/rimage/ipc4_extended_module_config.rst

+        esensing,
+        emax,
+        einvalid = emax
+   };


@mwasko @mmaka1 Can we add something about the semantics here? What should people adding new modules know about setting the "module_type"?

@kv2019i module_type enum was defined for the Host SW driver purpose. The idea was to have option to identify components of specific functionality like for e.g. wov, mux or any other listed above. That capability allow for more topology flexibility at Host SW level. For example, instead of hardcoding in topology that you need Module_1 with specific GUID you can point that you need Module_1 with specific functionality type. That allow to swap components of the same functionality without need to change the topology itself / modifying GUIDs.

The bottom line is that new modules module_type should be matched with one of the available on list or if necessary the new custom type should be added. I would also recommend to change the enum name to something like module_fuctionality_type.

Ack @mwasko . I think the "bottom line" part should be added directly in the docs to help module developers choose correctly.

lgirdwood

@mwasko good for you ?

plbossart

good start but lots of questions raised @RanderWang

developer_guides/rimage/ipc4_extended_module_config.rst

developer_guides/rimage/index.rst

developer_guides/rimage/ipc4_extended_module_config.rst

plbossart · 2023-02-06T17:52:38Z

developer_guides/rimage/ipc4_extended_module_config.rst

+***********************
+| **name**: name of the module
+| **uuid**: uuid of the module
+| **affinity_mask**: usable dsp core mask, bit N is for core N, 0x3 means core 0 and 1 can be used


what does 'usable' mean? is this restricting on which core a module can run, with the actual determination done on a per-instance basis?

developer_guides/rimage/ipc4_extended_module_config.rst

plbossart · 2023-02-06T17:57:29Z

developer_guides/rimage/ipc4_extended_module_config.rst

+  enum type {
+    pcm = 0,
+    mp3,
+    aac,


no. this should not be added, @mmaka1 mentioned this is not used by firmware.

@plbossart extended module config are used by driver, not by firmware. type is included in conf and I need to comments it

I am not going to question @mmaka1 's answer " The driver never used this one. And it was always hardcoded to ePcm. "

don't document things that are not useful or even used.

ack - lets remove

developer_guides/rimage/ipc4_extended_module_config.rst

lgirdwood · 2023-02-14T16:21:14Z

@plbossart updates good for you ?

developer_guides/rimage/ipc4_extended_module_config.rst

cujomalainey · 2023-02-14T20:27:50Z

developer_guides/rimage/ipc4_extended_module_config.rst

+| **uuid**: uuid of the module
+| **affinity_mask**: usable dsp core mask, bit N is for core N, 0x3 means core 0 and 1 can be used
+| **instance_count**: max module instance count can be active in fw
+| **domain_types**: two schedule domains: domain ll (0), and dp (1)


should give a bit more guidance here about which to choose for developers who may just building a standalone module are not SOF developers. LL and DP mean nothing to outsiders. To be honest I only know edf and ll, I have no clue what dp is.

developer_guides/rimage/ipc4_extended_module_config.rst

cujomalainey · 2023-02-14T20:36:05Z

developer_guides/rimage/ipc4_extended_module_config.rst

+.. code-block:: bash
+
+  pin = [dir, type, sample rates, sample size, sample container size, channel config]
+  For example, gain module pin = [0, 0, 0xfeef, 0xf, 0xf, 0x45ff], it supports input pcm


is there no better way to document/configure this? This seems like a heavy load on the user to (de)compile and humans are great sources of error

it needs to develop a new tool to make it easy, something like a ui tool to chose feature by clicking mouse.

I mean, just simply using enums, and discrete fields would be better than manual bitpacking

@cujomalainey sure. Let's add this feature to rimage tool.

developer_guides/rimage/ipc4_extended_module_config.rst

RanderWang · 2023-02-16T07:07:36Z

updated, thanks!

developer_guides/rimage/ipc4_extended_module_config.rst

RanderWang · 2023-03-14T05:56:46Z

@cujomalainey updated to use string instead of hex number for pin setting. If it is confirmed, I will start to add support in rimage tools

RanderWang · 2023-04-18T13:10:28Z

updated https://github.com/thesofproject/converged-sof-modules/blob/main/README.md to this PR

cujomalainey

looking much better, thank you, almost there

developer_guides/rimage/ipc4_extended_module_config.rst

cujomalainey · 2023-04-19T09:07:52Z

developer_guides/rimage/ipc4_extended_module_config.rst

+Module config
+-------------
+**Usage:**
+``mod_cfg PAR_0 PAR_1 PAR_2 PAR_3 IS_BYTES CPS IBS OBS MOD_FLAGS CPC OBLS``


this is a bit long, 11 args is a bit hard to read

what's your suggestion ? It defines array of config data.

The indicies are named no? Can they not be broken out?

sorry, I don't get your idea. Can you give an example ? Thanks!

Based on the description even though it is a byte array, the bytes themselves are all mapped directly to a value. Just curious why we cannot treat this as a packed struct and give everything a name and/or possible sub structs to make access and identification easier.

In my opinion, we can define struct in C language code, but this is a config file and these data blob are generated by FW tools, so like topology setting, we just include byte array.

lgirdwood

@RanderWang this is a starting point which can be built upon. Just one issue to close.

lgirdwood · 2023-07-04T10:30:32Z

developer_guides/rimage/ipc4_extended_module_config.rst

+  enum type {
+    pcm = 0,
+    mp3,
+    aac,


ack - lets remove

RanderWang · 2023-07-06T03:21:10Z

removed AAC and change init_config to init_config_has_extenstion

lgirdwood

Lets use this as the starting point.

lgirdwood · 2023-09-26T13:12:03Z

@cujomalainey @plbossart can we unblock this as the starting point as today we have zero documentation and this is better than zero. This then allows others to add more context like @btian1 (who has CPC formula's) and @mwasko (who has other manifest data). Thanks !

cujomalainey · 2023-10-23T22:31:27Z

@cujomalainey @plbossart can we unblock this as the starting point as today we have zero documentation and this is better than zero. This then allows others to add more context like @btian1 (who has CPC formula's) and @mwasko (who has other manifest data). Thanks !

Ack but can we get issues tracking the cleanup requests?

deb-intel

Numerous edits are included to ensure grammatical and style consistency throughout this doc and the rest of SOF documentation.

developer_guides/rimage/ipc4_extended_module_config.rst

This is a introduction of ipc4 extended module config and how to add this information in the xxx.toml for template. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang · 2023-10-25T05:44:09Z

refined style according to comments, thanks!

RanderWang requested review from lgirdwood, deb-intel, intelkevinputnam, greg-intel, mmaka1 and mwasko as code owners January 31, 2023 08:51

RanderWang force-pushed the ipc4_extended_module_cfg branch from 1fca027 to e035b0a Compare January 31, 2023 08:54

mmaka1 reviewed Feb 2, 2023

View reviewed changes

RanderWang mentioned this pull request Feb 3, 2023

Ipc4 : add google rtc AEC module support thesofproject/rimage#125

Closed

RanderWang force-pushed the ipc4_extended_module_cfg branch from e035b0a to 3209a52 Compare February 3, 2023 05:31

kv2019i reviewed Feb 3, 2023

View reviewed changes

lgirdwood reviewed Feb 3, 2023

View reviewed changes

kv2019i requested review from ranj063, ujfalusi and plbossart February 3, 2023 16:43

plbossart requested changes Feb 6, 2023

View reviewed changes

RanderWang force-pushed the ipc4_extended_module_cfg branch from 3209a52 to 27076d8 Compare February 7, 2023 08:29

kv2019i mentioned this pull request Feb 9, 2023

Add Init config type for modules thesofproject/rimage#130

Merged

ranj063 reviewed Feb 14, 2023

View reviewed changes

developer_guides/rimage/ipc4_extended_module_config.rst Outdated Show resolved Hide resolved

developer_guides/rimage/ipc4_extended_module_config.rst Show resolved Hide resolved

cujomalainey requested changes Feb 14, 2023

View reviewed changes

RanderWang force-pushed the ipc4_extended_module_cfg branch from 27076d8 to 7bafdad Compare February 16, 2023 07:06

ranj063 reviewed Feb 17, 2023

View reviewed changes

developer_guides/rimage/ipc4_extended_module_config.rst Outdated Show resolved Hide resolved

cujomalainey requested changes Feb 21, 2023

View reviewed changes

developer_guides/rimage/ipc4_extended_module_config.rst Outdated Show resolved Hide resolved

developer_guides/rimage/ipc4_extended_module_config.rst Outdated Show resolved Hide resolved

kv2019i mentioned this pull request Mar 10, 2023

topology2: add cpc value for gain thesofproject/sof#7242

Merged

RanderWang force-pushed the ipc4_extended_module_cfg branch from 7bafdad to 485dbdc Compare March 14, 2023 05:55

RanderWang mentioned this pull request Mar 14, 2023

topology2: add google rtc aec support thesofproject/sof#7237

Merged

marc-hb mentioned this pull request Mar 14, 2023

[FEATURE] Move rimage toml config files to SOF repository thesofproject/sof#7270

Closed

cujomalainey mentioned this pull request Mar 24, 2023

[FEATURE] rimage IPC4 manifest documentation #455

Open

2 tasks

RanderWang force-pushed the ipc4_extended_module_cfg branch from 485dbdc to b0d2b5f Compare April 18, 2023 13:09

cujomalainey reviewed Apr 19, 2023

View reviewed changes

RanderWang force-pushed the ipc4_extended_module_cfg branch from b0d2b5f to 9790ecc Compare April 19, 2023 13:52

lgirdwood reviewed Jul 4, 2023

View reviewed changes

developer_guides/rimage/ipc4_extended_module_config.rst Outdated

enum type {

pcm = 0,

mp3,

aac,

Copy link

Member

lgirdwood Jul 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack - lets remove

RanderWang force-pushed the ipc4_extended_module_cfg branch from 9790ecc to 888fbaf Compare July 6, 2023 03:20

lgirdwood previously approved these changes Jul 6, 2023

View reviewed changes

deb-intel requested changes Oct 24, 2023

View reviewed changes

rimage: add ipc4 extended module config introduction

63e3905

This is a introduction of ipc4 extended module config and how to add this information in the xxx.toml for template. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang dismissed lgirdwood’s stale review via 63e3905 October 25, 2023 05:39

RanderWang force-pushed the ipc4_extended_module_cfg branch from 888fbaf to 63e3905 Compare October 25, 2023 05:39

rimage: add ipc4 extended module config introduction #451

Are you sure you want to change the base?

rimage: add ipc4 extended module config introduction #451

Conversation

RanderWang commented Jan 31, 2023

RanderWang commented Jan 31, 2023

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

RanderWang commented Feb 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgirdwood left a comment

Choose a reason for hiding this comment

plbossart left a 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

lgirdwood commented Feb 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RanderWang Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RanderWang commented Feb 16, 2023

RanderWang commented Mar 14, 2023

RanderWang commented Apr 18, 2023

cujomalainey left a 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

lgirdwood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RanderWang commented Jul 6, 2023

lgirdwood left a comment

Choose a reason for hiding this comment

lgirdwood commented Sep 26, 2023

cujomalainey commented Oct 23, 2023

deb-intel left a comment • edited Loading

Choose a reason for hiding this comment

RanderWang commented Oct 25, 2023

RanderWang Feb 16, 2023 •

edited

Loading

deb-intel left a comment •

edited

Loading