[tmleman]

This PR adds synchronized FPI updates of HD-A gateways. Driver can define group of such gateways via module init IPC. The driver may also specify an update period for each group, different than the default one determined by the system tick frequency.

In addition, a change has been introduced that makes it possible to read config size in module init function. Each module receive its own configuration at creation (MODULE_INIT IPC). In case of a gateway those configurations can contain additional value (aux_conf). The only way to check if such config was received is to compare size of the data received via ipc and standard configuration size.

[ranj063]

can we please leave out the formatting changes to see it is easy to see that only 1 field is added? And btw won't this break IPC3? Should this be within #if CONFIG_MAJOR_IPC4?

[btian1]

can comp_ipc_config be changed? not sure why not left some redundancy space for later adding, like uint32_t valid_fmt to align internal usage.

[tmleman]

@btian1 can you explain exactly what you mean? Sorry, but I don't quite understand what your comment means.

[btian1]

I mean:

1. comp_ipc_config, normally, should not add or remove, it need keep stable, since driver and sof both need this, if change, may need change synchronous in SOF and driver。
2. this structure should have some redundant space for future extension, then for this change, you just need change a variable name, instead of adding a new variable with IPC4 limited.
   uint32_t reserved[4];

[tmleman]

From what I saw in code struct comp_ipc_config is internal SOF struct and its not expose to driver. It's created on FW side during module initialization and therefore I assume that is safe to add new value to it.

https://github.com/thesofproject/sof/blob/main/src/ipc/ipc4/helper.c#L81-L105

[btian1]

I did not have a change to check driver, just guess driver side also have a similar copy, if not, how to pass it from driver to fw(sof)?
@ranj063 , do we need update driver side as well?

[kv2019i]

Copier is getting complicated, but nothing really to complain about in the patch. It seems copier is the right place for this, and the extension handling looks correct. I have one minor nit on whether the 64bit wrap-around should be handled or not, but that's a minor thing.

[kv2019i]

Having timer logic without wrap-around handling is always a bit iffy, but given we have explicit 64bit counter, it would seem this is safe. Even with a 10x faster system clock, this would still take thousands of years to wrap around. I think current uptime record holder is still the one Cisco switch that is around 26 years now, so we have some safety margin -> https://en.wikipedia.org/wiki/Uptime

[lgirdwood]

ack, worth an inline comment here so that no one tries to fix later on.

[lgirdwood]

@tmleman only minor things here and we are good to merge.

[lyakh]

@tmleman only minor things here and we are good to merge.

@lgirdwood I think #7964 (comment) is rather significant

the allocation is uncached now, but it still should be documented, why no locking on the global list is required

[lgirdwood]

@tmleman can you check the internal CI results. Thanks !

[btian1]

can comp_ipc_config be changed? not sure why not left some redundancy space for later adding, like uint32_t valid_fmt to align internal usage.

[tmleman]

23ww31.4 Update: rebase

[btian1]

I mean:

1. comp_ipc_config, normally, should not add or remove, it need keep stable, since driver and sof both need this, if change, may need change synchronous in SOF and driver。
2. this structure should have some redundant space for future extension, then for this change, you just need change a variable name, instead of adding a new variable with IPC4 limited.
   uint32_t reserved[4];

[tmleman]

23ww31.5 Update: removing the ugly workaround and replacing it with an implementation resistant to incorrect values in IPC payload.