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
