diff mbox series

[v4,11/17] ASoC: Intel: avs: Firmware resources management utilities

Message ID 20220309204029.89040-12-cezary.rojewski@intel.com (mailing list archive)
State Superseded
Headers show
Series ASoC: Intel: AVS - Audio DSP for cAVS | expand

Commit Message

Cezary Rojewski March 9, 2022, 8:40 p.m. UTC 
With basefw runtime parameter handlers added, implement utility
functions to ease pipelines and modules allocation. IDA is enlisted to
help with that.

As firmware is modular and multiple binaries can be loaded on-demand
depending on the streaming scenario, custom firmware caching mechanism
is added.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/Makefile |   2 +-
 sound/soc/intel/avs/avs.h    |  37 +++++
 sound/soc/intel/avs/utils.c  | 301 +++++++++++++++++++++++++++++++++++
 3 files changed, 339 insertions(+), 1 deletion(-)
 create mode 100644 sound/soc/intel/avs/utils.c

Comments

Pierre-Louis Bossart March 9, 2022, 10:36 p.m. UTC | #1 
>   /*
>    * struct avs_dev - Intel HD-Audio driver data
>    *
>    * @dev: PCI device
>    * @dsp_ba: DSP bar address
>    * @spec: platform-specific descriptor
> + * @fw_cfg: Firmware configuration, obtained through FW_CONFIG message
> + * @hw_cfg: Hardware configuration, obtained through HW_CONFIG message
> + * @mods_info: Available module-types, obtained through MODULES_INFO message

is this just for the base firmware? If this includes the extensions, how 
are the module types defined?

> + * @mod_idas: Module instance ID pool, one per module-type
> + * @modres_mutex: For synchronizing any @mods_info updates
> + * @ppl_ida: Pipeline instance ID pool
> + * @fw_list: List of libraries loaded, including base firmware
>    */
>   struct avs_dev {
>   	struct hda_bus base;
> @@ -68,6 +82,14 @@ struct avs_dev {
>   	const struct avs_spec *spec;
>   	struct avs_ipc *ipc;
>   
> +	struct avs_fw_cfg fw_cfg;
> +	struct avs_hw_cfg hw_cfg;
> +	struct avs_mods_info *mods_info;
> +	struct ida **mod_idas;
> +	struct mutex modres_mutex;
> +	struct ida ppl_ida;
> +	struct list_head fw_list;
> +
>   	struct completion fw_ready;
>   };

> +/* Caller responsible for holding adev->modres_mutex. */
> +static int
> +avs_module_ida_alloc(struct avs_dev *adev, struct avs_mods_info *newinfo, bool purge)
> +{
> +	struct avs_mods_info *oldinfo = adev->mods_info;
> +	struct ida **ida_ptrs;
> +	u32 tocopy_count = 0;
> +	int i;
> +
> +	if (!purge && oldinfo) {
> +		if (oldinfo->count >= newinfo->count)
> +			dev_warn(adev->dev, "refreshing %d modules info with %d\n",
> +				 oldinfo->count, newinfo->count);
> +		tocopy_count = oldinfo->count;
> +	}
> +
> +	ida_ptrs = kcalloc(newinfo->count, sizeof(*ida_ptrs), GFP_KERNEL);
> +	if (!ida_ptrs)
> +		return -ENOMEM;
> +
> +	if (tocopy_count)
> +		memcpy(ida_ptrs, adev->mod_idas, tocopy_count * sizeof(*ida_ptrs));
> +
> +	for (i = tocopy_count; i < newinfo->count; i++) {
> +		ida_ptrs[i] = kzalloc(sizeof(**ida_ptrs), GFP_KERNEL);
> +		if (!ida_ptrs[i]) {
> +			while (i--)
> +				kfree(ida_ptrs[i]);

it's a bit hairy to play with the loop counter, I would jump to an error 
handling label to make it clearer.
> +
> +			kfree(ida_ptrs);
> +			return -ENOMEM;
> +		}
> +
> +		ida_init(ida_ptrs[i]);
> +	}
> +
> +	/* If old elements have been reused, don't wipe them. */

the comment is very odd, there's either a free() or a destroy() 
happening below...

> +	if (tocopy_count)
> +		kfree(adev->mod_idas);
> +	else
> +		avs_module_ida_destroy(adev);
> +
> +	adev->mod_idas = ida_ptrs;
> +	return 0;
> +}

> +int avs_module_id_alloc(struct avs_dev *adev, u16 module_id)
> +{
> +	int ret, idx, max_id;
> +
> +	mutex_lock(&adev->modres_mutex);
> +
> +	idx = avs_module_id_entry_index(adev, module_id);
> +	if (idx == -ENOENT) {
> +		WARN(1, "invalid module id: %d", module_id);

dev_err() seems to be more than enough, why would you add a complete 
call trace?

> +		ret = -EINVAL;
> +		goto exit;
> +	}
> +	max_id = adev->mods_info->entries[idx].instance_max_count - 1;
> +	ret = ida_alloc_max(adev->mod_idas[idx], max_id, GFP_KERNEL);
> +exit:
> +	mutex_unlock(&adev->modres_mutex);
> +	return ret;
> +}
> +
> +void avs_module_id_free(struct avs_dev *adev, u16 module_id, u8 instance_id)
> +{
> +	int idx;
> +
> +	mutex_lock(&adev->modres_mutex);
> +
> +	idx = avs_module_id_entry_index(adev, module_id);
> +	if (idx == -ENOENT) {
> +		WARN(1, "invalid module id: %d", module_id);

same WARN is over-engineered.

> +		goto exit;
> +	}
> +
> +	ida_free(adev->mod_idas[idx], instance_id);
> +exit:
> +	mutex_unlock(&adev->modres_mutex);
> +}
> +
> +/*

I am running out of time and will resume this review next week.
Cezary Rojewski March 10, 2022, 5:11 p.m. UTC | #2 
On 2022-03-09 11:36 PM, Pierre-Louis Bossart wrote:
> I am running out of time and will resume this review next week.

Hello,

I don't believe any of the comments provided are a strong reason for 
re-send. Given that last revisions were mostly related to addressing 
comments, rewords and explaining stuff, series is in good shape and 
ready for merge. As I already stated, team continues to work on the 
subject and there are more patch-series to come.


Regards,
Czarek
Mark Brown March 11, 2022, 12:10 p.m. UTC | #3 
On Thu, Mar 10, 2022 at 06:11:31PM +0100, Cezary Rojewski wrote:

> I don't believe any of the comments provided are a strong reason for
> re-send. Given that last revisions were mostly related to addressing
> comments, rewords and explaining stuff, series is in good shape and ready
> for merge. As I already stated, team continues to work on the subject and
> there are more patch-series to come.

It'd be good to get the WARN()s fixed.
Cezary Rojewski March 11, 2022, 3:28 p.m. UTC | #4 
On 2022-03-11 1:10 PM, Mark Brown wrote:
> On Thu, Mar 10, 2022 at 06:11:31PM +0100, Cezary Rojewski wrote:
> 
>> I don't believe any of the comments provided are a strong reason for
>> re-send. Given that last revisions were mostly related to addressing
>> comments, rewords and explaining stuff, series is in good shape and ready
>> for merge. As I already stated, team continues to work on the subject and
>> there are more patch-series to come.
> 
> It'd be good to get the WARN()s fixed.


Ack. Addressed in v5.
Cezary Rojewski March 11, 2022, 3:46 p.m. UTC | #5 
On 2022-03-09 11:36 PM, Pierre-Louis Bossart wrote:
> 
>>   /*
>>    * struct avs_dev - Intel HD-Audio driver data
>>    *
>>    * @dev: PCI device
>>    * @dsp_ba: DSP bar address
>>    * @spec: platform-specific descriptor
>> + * @fw_cfg: Firmware configuration, obtained through FW_CONFIG message
>> + * @hw_cfg: Hardware configuration, obtained through HW_CONFIG message
>> + * @mods_info: Available module-types, obtained through MODULES_INFO 
>> message
> 
> is this just for the base firmware? If this includes the extensions, how 
> are the module types defined?


Only base firmware is able to process MODULE_INFO getter. So, every time 
driver loads a library, this info gets updated internally on the 
firmware side. We make use of said getter to retrieve up-to-date 
information and cache in ->mods_info for later use. ->mods_info is a 
member of type struct avs_mods_info with each enter represented by 
struct avs_module_info. These are introduced with all the basefw runtime 
parameters.

>> + * @mod_idas: Module instance ID pool, one per module-type
>> + * @modres_mutex: For synchronizing any @mods_info updates
>> + * @ppl_ida: Pipeline instance ID pool
>> + * @fw_list: List of libraries loaded, including base firmware
>>    */
>>   struct avs_dev {
>>       struct hda_bus base;
>> @@ -68,6 +82,14 @@ struct avs_dev {
>>       const struct avs_spec *spec;
>>       struct avs_ipc *ipc;
>> +    struct avs_fw_cfg fw_cfg;
>> +    struct avs_hw_cfg hw_cfg;
>> +    struct avs_mods_info *mods_info;
>> +    struct ida **mod_idas;
>> +    struct mutex modres_mutex;
>> +    struct ida ppl_ida;
>> +    struct list_head fw_list;
>> +
>>       struct completion fw_ready;
>>   };

...

>> +    if (tocopy_count)
>> +        kfree(adev->mod_idas);
>> +    else
>> +        avs_module_ida_destroy(adev);
>> +
>> +    adev->mod_idas = ida_ptrs;
>> +    return 0;
>> +}
> 
>> +int avs_module_id_alloc(struct avs_dev *adev, u16 module_id)
>> +{
>> +    int ret, idx, max_id;
>> +
>> +    mutex_lock(&adev->modres_mutex);
>> +
>> +    idx = avs_module_id_entry_index(adev, module_id);
>> +    if (idx == -ENOENT) {
>> +        WARN(1, "invalid module id: %d", module_id);
> 
> dev_err() seems to be more than enough, why would you add a complete 
> call trace?
> 
>> +        ret = -EINVAL;
>> +        goto exit;
>> +    }
>> +    max_id = adev->mods_info->entries[idx].instance_max_count - 1;
>> +    ret = ida_alloc_max(adev->mod_idas[idx], max_id, GFP_KERNEL);
>> +exit:
>> +    mutex_unlock(&adev->modres_mutex);
>> +    return ret;
>> +}
>> +
>> +void avs_module_id_free(struct avs_dev *adev, u16 module_id, u8 
>> instance_id)
>> +{
>> +    int idx;
>> +
>> +    mutex_lock(&adev->modres_mutex);
>> +
>> +    idx = avs_module_id_entry_index(adev, module_id);
>> +    if (idx == -ENOENT) {
>> +        WARN(1, "invalid module id: %d", module_id);
> 
> same WARN is over-engineered.


Ack for both.


Regards,
Czarek
Pierre-Louis Bossart March 11, 2022, 3:59 p.m. UTC | #6 
On 3/11/22 09:46, Cezary Rojewski wrote:
> On 2022-03-09 11:36 PM, Pierre-Louis Bossart wrote:
>>
>>>   /*
>>>    * struct avs_dev - Intel HD-Audio driver data
>>>    *
>>>    * @dev: PCI device
>>>    * @dsp_ba: DSP bar address
>>>    * @spec: platform-specific descriptor
>>> + * @fw_cfg: Firmware configuration, obtained through FW_CONFIG message
>>> + * @hw_cfg: Hardware configuration, obtained through HW_CONFIG message
>>> + * @mods_info: Available module-types, obtained through MODULES_INFO 
>>> message
>>
>> is this just for the base firmware? If this includes the extensions, 
>> how are the module types defined?
> 
> 
> Only base firmware is able to process MODULE_INFO getter. So, every time 
> driver loads a library, this info gets updated internally on the 
> firmware side. We make use of said getter to retrieve up-to-date 
> information and cache in ->mods_info for later use. ->mods_info is a 
> member of type struct avs_mods_info with each enter represented by 
> struct avs_module_info. These are introduced with all the basefw 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?
Cezary Rojewski March 11, 2022, 5:20 p.m. UTC | #7 
On 2022-03-11 4:59 PM, Pierre-Louis Bossart wrote:
> On 3/11/22 09:46, Cezary Rojewski wrote:
>> On 2022-03-09 11:36 PM, Pierre-Louis Bossart wrote:
>>>
>>>>   /*
>>>>    * struct avs_dev - Intel HD-Audio driver data
>>>>    *
>>>>    * @dev: PCI device
>>>>    * @dsp_ba: DSP bar address
>>>>    * @spec: platform-specific descriptor
>>>> + * @fw_cfg: Firmware configuration, obtained through FW_CONFIG message
>>>> + * @hw_cfg: Hardware configuration, obtained through HW_CONFIG message
>>>> + * @mods_info: Available module-types, obtained through 
>>>> MODULES_INFO message
>>>
>>> is this just for the base firmware? If this includes the extensions, 
>>> how are the module types defined?
>>
>>
>> Only base firmware is able to process MODULE_INFO getter. So, every 
>> time driver loads a library, this info gets updated internally on the 
>> firmware side. We make use of said getter to retrieve up-to-date 
>> information and cache in ->mods_info for later use. ->mods_info is a 
>> member of type struct avs_mods_info with each enter represented by 
>> struct avs_module_info. These are introduced with all the basefw 

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.


Regards,
Czarek
Pierre-Louis Bossart March 11, 2022, 8:30 p.m. UTC | #8 
On 3/11/22 11:20, Cezary Rojewski wrote:
> On 2022-03-11 4:59 PM, Pierre-Louis Bossart wrote:
>> On 3/11/22 09:46, Cezary Rojewski wrote:
>>> On 2022-03-09 11:36 PM, Pierre-Louis Bossart wrote:
>>>>
>>>>>   /*
>>>>>    * struct avs_dev - Intel HD-Audio driver data
>>>>>    *
>>>>>    * @dev: PCI device
>>>>>    * @dsp_ba: DSP bar address
>>>>>    * @spec: platform-specific descriptor
>>>>> + * @fw_cfg: Firmware configuration, obtained through FW_CONFIG 
>>>>> message
>>>>> + * @hw_cfg: Hardware configuration, obtained through HW_CONFIG 
>>>>> message
>>>>> + * @mods_info: Available module-types, obtained through 
>>>>> MODULES_INFO message
>>>>
>>>> is this just for the base firmware? If this includes the extensions, 
>>>> how are the module types defined?
>>>
>>>
>>> Only base firmware is able to process MODULE_INFO getter. So, every 
>>> time driver loads a library, this info gets updated internally on the 
>>> firmware side. We make use of said getter to retrieve up-to-date 
>>> information and cache in ->mods_info for later use. ->mods_info is a 
>>> member of type struct avs_mods_info with each enter represented by 
>>> struct avs_module_info. These are introduced with all the basefw 
> 
> 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
"

?
Cezary Rojewski March 14, 2022, 5:59 p.m. UTC | #9 
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
diff mbox series

Patch

diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile
index c0824f30fd3b..d9f92c5f5407 100644
--- a/sound/soc/intel/avs/Makefile
+++ b/sound/soc/intel/avs/Makefile
@@ -1,5 +1,5 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 
-snd-soc-avs-objs := dsp.o ipc.o messages.o
+snd-soc-avs-objs := dsp.o ipc.o messages.o utils.o
 
 obj-$(CONFIG_SND_SOC_INTEL_AVS) += snd-soc-avs.o
diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h
index 841b8541b101..02d7591d0eac 100644
--- a/sound/soc/intel/avs/avs.h
+++ b/sound/soc/intel/avs/avs.h
@@ -53,12 +53,26 @@  struct avs_spec {
 	const u32 rom_status;
 };
 
+struct avs_fw_entry {
+	char *name;
+	const struct firmware *fw;
+
+	struct list_head node;
+};
+
 /*
  * struct avs_dev - Intel HD-Audio driver data
  *
  * @dev: PCI device
  * @dsp_ba: DSP bar address
  * @spec: platform-specific descriptor
+ * @fw_cfg: Firmware configuration, obtained through FW_CONFIG message
+ * @hw_cfg: Hardware configuration, obtained through HW_CONFIG message
+ * @mods_info: Available module-types, obtained through MODULES_INFO message
+ * @mod_idas: Module instance ID pool, one per module-type
+ * @modres_mutex: For synchronizing any @mods_info updates
+ * @ppl_ida: Pipeline instance ID pool
+ * @fw_list: List of libraries loaded, including base firmware
  */
 struct avs_dev {
 	struct hda_bus base;
@@ -68,6 +82,14 @@  struct avs_dev {
 	const struct avs_spec *spec;
 	struct avs_ipc *ipc;
 
+	struct avs_fw_cfg fw_cfg;
+	struct avs_hw_cfg hw_cfg;
+	struct avs_mods_info *mods_info;
+	struct ida **mod_idas;
+	struct mutex modres_mutex;
+	struct ida ppl_ida;
+	struct list_head fw_list;
+
 	struct completion fw_ready;
 };
 
@@ -168,4 +190,19 @@  void avs_dsp_interrupt_control(struct avs_dev *adev, bool enable);
 int avs_ipc_init(struct avs_ipc *ipc, struct device *dev);
 void avs_ipc_block(struct avs_ipc *ipc);
 
+/* Firmware resources management */
+
+int avs_get_module_entry(struct avs_dev *adev, const guid_t *uuid, struct avs_module_entry *entry);
+int avs_get_module_id_entry(struct avs_dev *adev, u32 module_id, struct avs_module_entry *entry);
+int avs_get_module_id(struct avs_dev *adev, const guid_t *uuid);
+bool avs_is_module_ida_empty(struct avs_dev *adev, u32 module_id);
+
+int avs_module_info_init(struct avs_dev *adev, bool purge);
+void avs_module_info_free(struct avs_dev *adev);
+int avs_module_id_alloc(struct avs_dev *adev, u16 module_id);
+void avs_module_id_free(struct avs_dev *adev, u16 module_id, u8 instance_id);
+int avs_request_firmware(struct avs_dev *adev, const struct firmware **fw_p, const char *name);
+void avs_release_last_firmware(struct avs_dev *adev);
+void avs_release_firmwares(struct avs_dev *adev);
+
 #endif /* __SOUND_SOC_INTEL_AVS_H */
diff --git a/sound/soc/intel/avs/utils.c b/sound/soc/intel/avs/utils.c
new file mode 100644
index 000000000000..580f3e43fa12
--- /dev/null
+++ b/sound/soc/intel/avs/utils.c
@@ -0,0 +1,301 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright(c) 2021 Intel Corporation. All rights reserved.
+//
+// Authors: Cezary Rojewski <cezary.rojewski@intel.com>
+//          Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
+//
+
+#include <linux/firmware.h>
+#include <linux/slab.h>
+#include "avs.h"
+#include "messages.h"
+
+/* Caller responsible for holding adev->modres_mutex. */
+static int avs_module_entry_index(struct avs_dev *adev, const guid_t *uuid)
+{
+	int i;
+
+	for (i = 0; i < adev->mods_info->count; i++) {
+		struct avs_module_entry *module;
+
+		module = &adev->mods_info->entries[i];
+		if (guid_equal(&module->uuid, uuid))
+			return i;
+	}
+
+	return -ENOENT;
+}
+
+/* Caller responsible for holding adev->modres_mutex. */
+static int avs_module_id_entry_index(struct avs_dev *adev, u32 module_id)
+{
+	int i;
+
+	for (i = 0; i < adev->mods_info->count; i++) {
+		struct avs_module_entry *module;
+
+		module = &adev->mods_info->entries[i];
+		if (module->module_id == module_id)
+			return i;
+	}
+
+	return -ENOENT;
+}
+
+int avs_get_module_entry(struct avs_dev *adev, const guid_t *uuid, struct avs_module_entry *entry)
+{
+	int idx;
+
+	mutex_lock(&adev->modres_mutex);
+
+	idx = avs_module_entry_index(adev, uuid);
+	if (idx >= 0)
+		memcpy(entry, &adev->mods_info->entries[idx], sizeof(*entry));
+
+	mutex_unlock(&adev->modres_mutex);
+	return (idx < 0) ? idx : 0;
+}
+
+int avs_get_module_id_entry(struct avs_dev *adev, u32 module_id, struct avs_module_entry *entry)
+{
+	int idx;
+
+	mutex_lock(&adev->modres_mutex);
+
+	idx = avs_module_id_entry_index(adev, module_id);
+	if (idx >= 0)
+		memcpy(entry, &adev->mods_info->entries[idx], sizeof(*entry));
+
+	mutex_unlock(&adev->modres_mutex);
+	return (idx < 0) ? idx : 0;
+}
+
+int avs_get_module_id(struct avs_dev *adev, const guid_t *uuid)
+{
+	struct avs_module_entry module;
+	int ret;
+
+	ret = avs_get_module_entry(adev, uuid, &module);
+	return !ret ? module.module_id : -ENOENT;
+}
+
+bool avs_is_module_ida_empty(struct avs_dev *adev, u32 module_id)
+{
+	bool ret = false;
+	int idx;
+
+	mutex_lock(&adev->modres_mutex);
+
+	idx = avs_module_id_entry_index(adev, module_id);
+	if (idx >= 0)
+		ret = ida_is_empty(adev->mod_idas[idx]);
+
+	mutex_unlock(&adev->modres_mutex);
+	return ret;
+}
+
+/* Caller responsible for holding adev->modres_mutex. */
+static void avs_module_ida_destroy(struct avs_dev *adev)
+{
+	int i = adev->mods_info ? adev->mods_info->count : 0;
+
+	while (i--) {
+		ida_destroy(adev->mod_idas[i]);
+		kfree(adev->mod_idas[i]);
+	}
+	kfree(adev->mod_idas);
+}
+
+/* Caller responsible for holding adev->modres_mutex. */
+static int
+avs_module_ida_alloc(struct avs_dev *adev, struct avs_mods_info *newinfo, bool purge)
+{
+	struct avs_mods_info *oldinfo = adev->mods_info;
+	struct ida **ida_ptrs;
+	u32 tocopy_count = 0;
+	int i;
+
+	if (!purge && oldinfo) {
+		if (oldinfo->count >= newinfo->count)
+			dev_warn(adev->dev, "refreshing %d modules info with %d\n",
+				 oldinfo->count, newinfo->count);
+		tocopy_count = oldinfo->count;
+	}
+
+	ida_ptrs = kcalloc(newinfo->count, sizeof(*ida_ptrs), GFP_KERNEL);
+	if (!ida_ptrs)
+		return -ENOMEM;
+
+	if (tocopy_count)
+		memcpy(ida_ptrs, adev->mod_idas, tocopy_count * sizeof(*ida_ptrs));
+
+	for (i = tocopy_count; i < newinfo->count; i++) {
+		ida_ptrs[i] = kzalloc(sizeof(**ida_ptrs), GFP_KERNEL);
+		if (!ida_ptrs[i]) {
+			while (i--)
+				kfree(ida_ptrs[i]);
+
+			kfree(ida_ptrs);
+			return -ENOMEM;
+		}
+
+		ida_init(ida_ptrs[i]);
+	}
+
+	/* If old elements have been reused, don't wipe them. */
+	if (tocopy_count)
+		kfree(adev->mod_idas);
+	else
+		avs_module_ida_destroy(adev);
+
+	adev->mod_idas = ida_ptrs;
+	return 0;
+}
+
+int avs_module_info_init(struct avs_dev *adev, bool purge)
+{
+	struct avs_mods_info *info;
+	int ret;
+
+	ret = avs_ipc_get_modules_info(adev, &info);
+	if (ret)
+		return AVS_IPC_RET(ret);
+
+	mutex_lock(&adev->modres_mutex);
+
+	ret = avs_module_ida_alloc(adev, info, purge);
+	if (ret < 0) {
+		dev_err(adev->dev, "initialize module idas failed: %d\n", ret);
+		goto exit;
+	}
+
+	/* Refresh current information with newly received table. */
+	kfree(adev->mods_info);
+	adev->mods_info = info;
+
+exit:
+	mutex_unlock(&adev->modres_mutex);
+	return ret;
+}
+
+void avs_module_info_free(struct avs_dev *adev)
+{
+	mutex_lock(&adev->modres_mutex);
+
+	avs_module_ida_destroy(adev);
+	kfree(adev->mods_info);
+	adev->mods_info = NULL;
+
+	mutex_unlock(&adev->modres_mutex);
+}
+
+int avs_module_id_alloc(struct avs_dev *adev, u16 module_id)
+{
+	int ret, idx, max_id;
+
+	mutex_lock(&adev->modres_mutex);
+
+	idx = avs_module_id_entry_index(adev, module_id);
+	if (idx == -ENOENT) {
+		WARN(1, "invalid module id: %d", module_id);
+		ret = -EINVAL;
+		goto exit;
+	}
+	max_id = adev->mods_info->entries[idx].instance_max_count - 1;
+	ret = ida_alloc_max(adev->mod_idas[idx], max_id, GFP_KERNEL);
+exit:
+	mutex_unlock(&adev->modres_mutex);
+	return ret;
+}
+
+void avs_module_id_free(struct avs_dev *adev, u16 module_id, u8 instance_id)
+{
+	int idx;
+
+	mutex_lock(&adev->modres_mutex);
+
+	idx = avs_module_id_entry_index(adev, module_id);
+	if (idx == -ENOENT) {
+		WARN(1, "invalid module id: %d", module_id);
+		goto exit;
+	}
+
+	ida_free(adev->mod_idas[idx], instance_id);
+exit:
+	mutex_unlock(&adev->modres_mutex);
+}
+
+/*
+ * Once driver loads FW it should keep it in memory, so we are not affected
+ * by FW removal from filesystem or even worse by loading different FW at
+ * runtime suspend/resume.
+ */
+int avs_request_firmware(struct avs_dev *adev, const struct firmware **fw_p, const char *name)
+{
+	struct avs_fw_entry *entry;
+	int ret;
+
+	/* first check in list if it is not already loaded */
+	list_for_each_entry(entry, &adev->fw_list, node) {
+		if (!strcmp(name, entry->name)) {
+			*fw_p = entry->fw;
+			return 0;
+		}
+	}
+
+	/* FW is not loaded, let's load it now and add to the list */
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->name = kstrdup(name, GFP_KERNEL);
+	if (!entry->name) {
+		kfree(entry);
+		return -ENOMEM;
+	}
+
+	ret = request_firmware(&entry->fw, name, adev->dev);
+	if (ret < 0) {
+		kfree(entry->name);
+		kfree(entry);
+		return ret;
+	}
+
+	*fw_p = entry->fw;
+
+	list_add_tail(&entry->node, &adev->fw_list);
+
+	return 0;
+}
+
+/*
+ * Release single FW entry, used to handle errors in functions calling
+ * avs_request_firmware()
+ */
+void avs_release_last_firmware(struct avs_dev *adev)
+{
+	struct avs_fw_entry *entry;
+
+	entry = list_last_entry(&adev->fw_list, typeof(*entry), node);
+
+	list_del(&entry->node);
+	release_firmware(entry->fw);
+	kfree(entry->name);
+	kfree(entry);
+}
+
+/*
+ * Release all FW entries, used on driver removal
+ */
+void avs_release_firmwares(struct avs_dev *adev)
+{
+	struct avs_fw_entry *entry, *tmp;
+
+	list_for_each_entry_safe(entry, tmp, &adev->fw_list, node) {
+		list_del(&entry->node);
+		release_firmware(entry->fw);
+		kfree(entry->name);
+		kfree(entry);
+	}
+}