ipc4: add get &set large config support #4808

RanderWang · 2021-09-27T08:26:39Z

The module id of basefw is zero. Driver get & set global
states by this id, such as platform info, memory inf, hw
info, power info.

This patch add limited support of features in basefw module.

Signed-off-by: Rander Wang rander.wang@intel.com

dbaluta · 2021-09-27T10:05:57Z

src/audio/CMakeLists.txt

Out of curiosity, what is basefw? Can you add some details in the commit message.

sure. There are three types of elf binary in SOF: boot_loader, basefw (built from all sof entities selected in kconfig) and external libraries (developed by 3rd party, may be close source). Rimage packs boot_loader and basefw into sof-xxx.ri

lgirdwood

ABI needs packed and aligned. Open on the data whether it's manifest or not ?

lgirdwood · 2021-09-27T14:53:03Z

src/audio/base_fw.c

Is this part of the extended manifest ?

no, it is part of base fw.

the comments for IPC4_MEMORY_RECLAIMED_FW_CFG is "Indicates whether legacy DMA memory is managed by FW". A little confusing. In my opinion, the legacy DMA should be GPDMA on BDW and BYT to transfer data between host and dsp.

I also checked other items and manifest. these items are not included by manifest.

ok, so it's sent to host upon boot ? This is probably not a good use of memory, but this can be optimised via ipc tuples later on.

yes, host query these values upon boot. Some items are read from hw registers, so we can't set them in manifest. We can store these tuples in dynamic memory and free it when get function return since get function is a low-frequency case.

ok, this should be in short term.

src/include/ipc4/base_fw.h

lgirdwood · 2021-09-28T14:45:18Z

src/audio/base_fw.c

ok, so it's sent to host upon boot ? This is probably not a good use of memory, but this can be optimised via ipc tuples later on.

src/audio/base_fw.c

src/include/ipc4/base_fw.h

lgirdwood

All the IPC ABI structs need to be packed and aligned of 4 bytes.

src/include/ipc4/base_fw.h

src/audio/base_fw.c

RanderWang · 2021-10-11T08:02:06Z

thanks, updated

src/audio/Kconfig

src/include/ipc4/module.h

plbossart · 2021-10-11T15:00:48Z

src/include/ipc4/module.h

bitfields in IPC definition, who thought that was a good idea? Not portable!

agree, however we have already stated this is not portable. We can address this with ipc-tuples in the future.

src/include/ipc4/module.h

src/ipc/ipc4/handler.c

src/ipc/ipc4/helper.c

The module id of basefw is zero. Driver get & set global states by this id, such as platform info, memory inf, hw info, power info. There are three types of elf binary built by SOF: boot_loader, basefw which is built from all sof entities selected in kconfig and external libraries developed by 3rd party and may be close-source. Rimage packs boot_loader and basefw into sof-xxx.ri. This patch adds limited support of features in basefw module and more features will be added in future. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang · 2021-10-12T08:41:58Z

updated, thanks!

lgirdwood

I think we just need that comments for the error flow and a statement about the portability of the bitfields (if not already in the header)

src/ipc/ipc4/handler.c

RanderWang · 2021-10-14T08:33:23Z

updated, thanks!

Get_large_config is used for driver to query module information and set_large_config is used to change module config dynamically Signed-off-by: Rander Wang <rander.wang@intel.com>

Ipc4 checks manifest in fw binary, so it needs to use header files in rimage Signed-off-by: Rander Wang <rander.wang@intel.com>

lgirdwood · 2021-10-14T12:53:41Z

BYT test looks like script failure, but unrelated to this PR.

marc-hb · 2021-10-15T03:25:32Z

src/ipc/ipc4/helper.c


-	/* skip basefw of module 0 in manifest */
-	mod = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(module_id));
+	mod = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(entry_index));


Will IPC4 ever be used with Zephyr? Zephyr CI does not have rimage right now.

Should these definitions be copied to src/platform/intel/cavs/include/cavs/ext_manifest.h ?

Will the kernel too? Are they part of the IPC?

IPC4 is not used by Zephyr currently. They are part of manifest in FW image in memory, not ext manifest.

Thanks @ranj063 for answering my question offline. IPC4 will be the default for some future platforms Zephyr or not. Zephyr has no effect on that decision / timeline. This means SOF will have a harder dependency on rimage than now and it will have to be mirrored by Zephyr. Right now only the main SOF ELF file does not depend on it, only the boot_module and base_module builds depend on rimage.

If rimage becomes a harder SOF dependency then it means sof/src/include/kernel/ext_manifest.h can probably be deleted at that time?

cc: @dbaluta

RanderWang requested review from dbaluta, lbetlej, lgirdwood, mmaka1 and plbossart as code owners September 27, 2021 08:26

RanderWang force-pushed the basefw_ipc4 branch 2 times, most recently from 5fb7987 to 12e29dc Compare September 27, 2021 08:30

dbaluta reviewed Sep 27, 2021

View reviewed changes

lgirdwood requested changes Sep 27, 2021

View reviewed changes

lgirdwood added the ABI ABI change involved label Sep 27, 2021

RanderWang force-pushed the basefw_ipc4 branch from 12e29dc to 9db3b8a Compare September 28, 2021 07:26

RanderWang changed the title ~~ipc4: add basefw component support~~ ipc4: add get &set large config support Sep 28, 2021

RanderWang force-pushed the basefw_ipc4 branch from 9db3b8a to a39ea2e Compare September 28, 2021 07:29

bkokoszx mentioned this pull request Sep 28, 2021

[BUG][APL] APL zephyr firmware build failed due to Zephyr-sdk incompatibility #4803

Closed

lgirdwood reviewed Sep 28, 2021

View reviewed changes

RanderWang force-pushed the basefw_ipc4 branch 4 times, most recently from 672ff94 to e7dcaf8 Compare September 30, 2021 06:02

lgirdwood requested changes Sep 30, 2021

View reviewed changes

src/include/ipc4/base_fw.h Outdated Show resolved Hide resolved

src/include/ipc4/base_fw.h Outdated Show resolved Hide resolved

src/audio/base_fw.c Outdated Show resolved Hide resolved

src/audio/base_fw.c Outdated Show resolved Hide resolved

RanderWang force-pushed the basefw_ipc4 branch from e7dcaf8 to 540b3d0 Compare October 11, 2021 08:01

plbossart requested changes Oct 11, 2021

View reviewed changes

RanderWang force-pushed the basefw_ipc4 branch 2 times, most recently from 4faab3c to e71e88e Compare October 12, 2021 07:04

lgirdwood reviewed Oct 13, 2021

View reviewed changes

src/ipc/ipc4/handler.c Outdated Show resolved Hide resolved

RanderWang force-pushed the basefw_ipc4 branch from e71e88e to c063dc9 Compare October 14, 2021 08:32

RanderWang added 2 commits October 14, 2021 16:37

ipc4: add get & set large config support in ipc framework

89753bc

Get_large_config is used for driver to query module information and set_large_config is used to change module config dynamically Signed-off-by: Rander Wang <rander.wang@intel.com>

ipc4: fix ipc4 build failed

c063dc9

Ipc4 checks manifest in fw binary, so it needs to use header files in rimage Signed-off-by: Rander Wang <rander.wang@intel.com>

lgirdwood approved these changes Oct 14, 2021

View reviewed changes

lgirdwood merged commit cbf5aa6 into thesofproject:main Oct 14, 2021

marc-hb reviewed Oct 15, 2021

View reviewed changes

kv2019i mentioned this pull request Nov 26, 2021

ASoC: SOF: bump ABI to 3.20 thesofproject/linux#3298

Closed

marc-hb mentioned this pull request Feb 9, 2022

[BUG] logger "ABI" dictionary check ignores configuration and other changes #3890

Closed

marc-hb mentioned this pull request Oct 5, 2023

[FEATURE] remove SOF build time dependency to rimage headers #8178

Closed

ipc4: add get &set large config support #4808

ipc4: add get &set large config support #4808

Uh oh!

Conversation

RanderWang commented Sep 27, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang Sep 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RanderWang commented Oct 11, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RanderWang commented Oct 12, 2021

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RanderWang commented Oct 14, 2021

Uh oh!

lgirdwood commented Oct 14, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

RanderWang Sep 28, 2021 •

edited

Loading

marc-hb Oct 15, 2021 •

edited

Loading