thesofproject / rimage

DSP firmware image creation and signing tool
Other
7 stars 62 forks source link

Add Init config type for modules #130

Closed ranj063 closed 1 year ago

ranj063 commented 1 year ago

USe some reverved bits to add the init config information for modules that will be used to indicate whether the base config extension needs to be added to the init module payload or not.

pblaszko commented 1 year ago

This implementation does not break anything itself, as it just reuses some reserved bits of module type in manifest. But what is a purpose of this change? Modules with only base_cfg or with base_cfg+base_cfg_extension are only a subset of overall FW modules. Many infrastructure modules have more data in payload. Is your change intended for modules built with IADK?

kv2019i commented 1 year ago

@pblaszko wrote:

This implementation does not break anything itself, as it just reuses some reserved bits of module type in manifest. But what is > a purpose of this change? Modules with only base_cfg or with base_cfg+base_cfg_extension are only a subset of overall FW modules. Many infrastructure modules have more data in payload. Is your change intended for modules built with IADK?

This is related to kernel PR 4160 and specifically this comment https://github.com/thesofproject/linux/pull/4160#discussion_r1090912209 .

AFAIU, for base modules (like copier) the kernel can have hard-coded knowledge of each module, but for generic processing modules (like IADK ones), it's not scalable to need separate changes to Linux for each new type of module. I.e. the manifest needs to tell kernel if a module absolutely needs the base-ext to function correctly (smart amp is one example). Our earlier alternative was to use the topology file, but during the discussion it became apparent this is not the correct place to store the meta information.

This of course leaves still the case where a module has a completely custom initialization interface and it needs the complete set to function correctly. For these, the Linux kernel has to be modified to support the module. This is possible, but a very slow to deploy path -- this should really only be used for base modules such as copier.

pblaszko commented 1 year ago

@pblaszko wrote:

This implementation does not break anything itself, as it just reuses some reserved bits of module type in manifest. But what is > a purpose of this change? Modules with only base_cfg or with base_cfg+base_cfg_extension are only a subset of overall FW modules. Many infrastructure modules have more data in payload. Is your change intended for modules built with IADK?

This is related to kernel PR 4160 and specifically this comment thesofproject/linux#4160 (comment) .

AFAIU, for base modules (like copier) the kernel can have hard-coded knowledge of each module, but for generic processing modules (like IADK ones), it's not scalable to need separate changes to Linux for each new type of module. I.e. the manifest needs to tell kernel if a module absolutely needs the base-ext to function correctly (smart amp is one example). Our earlier alternative was to use the topology file, but during the discussion it became apparent this is not the correct place to store the meta information.

This of course leaves still the case where a module has a completely custom initialization interface and it needs the complete set to function correctly. For these, the Linux kernel has to be modified to support the module. This is possible, but a very slow to deploy path -- this should really only be used for base modules such as copier.

Thank you @kv2019i ! So this will be an optimization for modules that fit into specified structures, but other modules with different structure can still be used (preferably only FW infrastructure). This is fine for me.