On 2022-03-11 9:30 PM, Pierre-Louis Bossart wrote:
On 3/11/22 11:20, Cezary Rojewski wrote:
...
Sorry for the typo: s/avs_module_info/avs_module_entry/.
runtime parameters.
you clarified the mechanism but not the definition of 'module-type'?
the definition doesn't really help.
struct avs_module_type { u32 load_type:4; u32 auto_start:1; u32 domain_ll:1; u32 domain_dp:1; u32 lib_code:1; u32 rsvd:24; } __packed;
I see nothing that would e.g. identify a mixer from a gain. The definition of 'type' seems to refer to low-level properties, not what the module actually does?
There is no "module-type" enum that software can rely on. We rely on hardcoded GUIDs instead. "module-type" is represented by struct avs_module_entry (per type) in context of MODULE_INFO IPC. All these names are indented to match firmware equivalents to make it easier to switch between the two worlds.
So the initial kernel-doc I commented on is still quite convoluted, you are referring to a 'module-type' that's not really well defined or useful for a driver.
Given the definition:
struct avs_mods_info { u32 count; struct avs_module_entry entries[]; } __packed;
wouldn't this be simpler/less confusing:
" @mods_info: Available array of module entries, obtained through MODULES_INFO message "
The major problem is the convoluted naming within the firmware itself.
The decision for the naming all the members as they are is dictated by the fact that there is much, much higher chance for Intel audio software or firmware developer to do some major/meaningful changes to the avs-driver as the kernel grows than a person outside that circle. Not everyone might agree, but that's the democratic decision made by the team in the early days of this driver. And thus, having close name-matching between the driver and the firmware makes it easier for given developer to switch between the two projects.
Agree on the rewording. There are probably more of such wordings within the code so this might span more than 1 file.
Regards, Czarek