* [PATCH 01/13] ASoC: Intel: avs: Do not readq() u32 registers
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 02/13] ASoC: Intel: avs: Fix the minimum firmware version numbers Cezary Rojewski
` (12 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
Register reporting ROM status is 4-bytes wide.
Fixes: 092cf7b26a48 ("ASoC: Intel: avs: Code loading over HDA")
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/loader.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c
index 890efd2f1fea..37de077a9983 100644
--- a/sound/soc/intel/avs/loader.c
+++ b/sound/soc/intel/avs/loader.c
@@ -308,7 +308,7 @@ avs_hda_init_rom(struct avs_dev *adev, unsigned int dma_id, bool purge)
}
/* await ROM init */
- ret = snd_hdac_adsp_readq_poll(adev, spec->sram->rom_status_offset, reg,
+ ret = snd_hdac_adsp_readl_poll(adev, spec->sram->rom_status_offset, reg,
(reg & 0xF) == AVS_ROM_INIT_DONE ||
(reg & 0xF) == APL_ROM_FW_ENTERED,
AVS_ROM_INIT_POLLING_US, APL_ROM_INIT_TIMEOUT_US);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 02/13] ASoC: Intel: avs: Fix the minimum firmware version numbers
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
2025-01-09 12:22 ` [PATCH 01/13] ASoC: Intel: avs: Do not readq() u32 registers Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 03/13] ASoC: Intel: avs: Fix theoretical infinite loop Cezary Rojewski
` (11 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
For few TGL-based platforms the minor version number for AudioDSP
firmware is incorrect forcing users to utilize ignore_fw_version module
parameter.
Fixes: 5acb19ecd198 ("ASoC: Intel: avs: TGL-based platforms support")
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/core.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 73d4bde9b2f7..82839d0994ee 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -829,10 +829,10 @@ static const struct avs_spec jsl_desc = {
.hipc = &cnl_hipc_spec,
};
-#define AVS_TGL_BASED_SPEC(sname) \
+#define AVS_TGL_BASED_SPEC(sname, min) \
static const struct avs_spec sname##_desc = { \
.name = #sname, \
- .min_fw_version = { 10, 29, 0, 5646 }, \
+ .min_fw_version = { 10, min, 0, 5646 }, \
.dsp_ops = &avs_tgl_dsp_ops, \
.core_init_mask = 1, \
.attributes = AVS_PLATATTR_IMR, \
@@ -840,11 +840,11 @@ static const struct avs_spec sname##_desc = { \
.hipc = &cnl_hipc_spec, \
}
-AVS_TGL_BASED_SPEC(lkf);
-AVS_TGL_BASED_SPEC(tgl);
-AVS_TGL_BASED_SPEC(ehl);
-AVS_TGL_BASED_SPEC(adl);
-AVS_TGL_BASED_SPEC(adl_n);
+AVS_TGL_BASED_SPEC(lkf, 28);
+AVS_TGL_BASED_SPEC(tgl, 29);
+AVS_TGL_BASED_SPEC(ehl, 30);
+AVS_TGL_BASED_SPEC(adl, 35);
+AVS_TGL_BASED_SPEC(adl_n, 35);
static const struct pci_device_id avs_ids[] = {
{ PCI_DEVICE_DATA(INTEL, HDA_SKL_LP, &skl_desc) },
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 03/13] ASoC: Intel: avs: Fix theoretical infinite loop
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
2025-01-09 12:22 ` [PATCH 01/13] ASoC: Intel: avs: Do not readq() u32 registers Cezary Rojewski
2025-01-09 12:22 ` [PATCH 02/13] ASoC: Intel: avs: Fix the minimum firmware version numbers Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 04/13] ASoC: Intel: avs: Fix init-config parsing Cezary Rojewski
` (10 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
While 'stack_dump_size' is a u32 bitfield of 16 bits, u32 has a bigger
upper bound than the type u16 of loop counter 'offset' what in theory
may lead to infinite loop condition.
Found out by Coverity static analyzer.
Fixes: c8c960c10971 ("ASoC: Intel: avs: APL-based platforms support")
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/apl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/intel/avs/apl.c b/sound/soc/intel/avs/apl.c
index 27516ef57185..d443fe8d51ae 100644
--- a/sound/soc/intel/avs/apl.c
+++ b/sound/soc/intel/avs/apl.c
@@ -125,7 +125,7 @@ int avs_apl_coredump(struct avs_dev *adev, union avs_notify_msg *msg)
struct avs_apl_log_buffer_layout layout;
void __iomem *addr, *buf;
size_t dump_size;
- u16 offset = 0;
+ u32 offset = 0;
u8 *dump, *pos;
dump_size = AVS_FW_REGS_SIZE + msg->ext.coredump.stack_dump_size;
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 04/13] ASoC: Intel: avs: Fix init-config parsing
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (2 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 03/13] ASoC: Intel: avs: Fix theoretical infinite loop Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 05/13] ASoC: Intel: avs: Update hda component teardown sequences Cezary Rojewski
` (9 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
When parsing init configs correct token should be looked up.
Fixes: 1b4217ebbb3e ("ASoC: Intel: avs: Add topology parsing support for initial config")
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/topology.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c
index 5cda527020c7..d612f20ed989 100644
--- a/sound/soc/intel/avs/topology.c
+++ b/sound/soc/intel/avs/topology.c
@@ -1466,7 +1466,7 @@ avs_tplg_path_template_create(struct snd_soc_component *comp, struct avs_tplg *o
static const struct avs_tplg_token_parser mod_init_config_parsers[] = {
{
- .token = AVS_TKN_MOD_INIT_CONFIG_ID_U32,
+ .token = AVS_TKN_INIT_CONFIG_ID_U32,
.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
.offset = offsetof(struct avs_tplg_init_config, id),
.parse = avs_parse_word_token,
@@ -1519,7 +1519,7 @@ static int avs_tplg_parse_initial_configs(struct snd_soc_component *comp,
esize = le32_to_cpu(tuples->size) + le32_to_cpu(tmp->size);
ret = parse_dictionary_entries(comp, tuples, esize, config, 1, sizeof(*config),
- AVS_TKN_MOD_INIT_CONFIG_ID_U32,
+ AVS_TKN_INIT_CONFIG_ID_U32,
mod_init_config_parsers,
ARRAY_SIZE(mod_init_config_parsers));
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 05/13] ASoC: Intel: avs: Update hda component teardown sequences
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (3 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 04/13] ASoC: Intel: avs: Fix init-config parsing Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 06/13] ASoC: Intel: avs: Print IPC error messages in lower layer Cezary Rojewski
` (8 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
If case of failure cleanup recently created DAI and while at it, adjust
the remove() operation to match operation order of the probe() function.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/pcm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 945f9c0a6a54..5878cfdbbee5 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -1564,6 +1564,7 @@ static int avs_component_hda_probe(struct snd_soc_component *component)
if (ret < 0) {
dev_err(component->dev, "create widgets failed: %d\n",
ret);
+ snd_soc_unregister_dai(dai);
goto exit;
}
}
@@ -1578,8 +1579,8 @@ static int avs_component_hda_probe(struct snd_soc_component *component)
static void avs_component_hda_remove(struct snd_soc_component *component)
{
- avs_component_hda_unregister_dais(component);
avs_component_remove(component);
+ avs_component_hda_unregister_dais(component);
}
static int avs_component_hda_open(struct snd_soc_component *component,
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 06/13] ASoC: Intel: avs: Print IPC error messages in lower layer
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (4 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 05/13] ASoC: Intel: avs: Update hda component teardown sequences Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW Cezary Rojewski
` (7 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
It is preferred to send error message in handler itself instead of
leaving it to caller.
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/loader.c | 8 ++------
sound/soc/intel/avs/messages.c | 22 ++++++++++++++++------
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c
index 37de077a9983..4203b216ae13 100644
--- a/sound/soc/intel/avs/loader.c
+++ b/sound/soc/intel/avs/loader.c
@@ -675,16 +675,12 @@ int avs_dsp_first_boot_firmware(struct avs_dev *adev)
}
ret = avs_ipc_get_hw_config(adev, &adev->hw_cfg);
- if (ret) {
- dev_err(adev->dev, "get hw cfg failed: %d\n", ret);
+ if (ret)
return AVS_IPC_RET(ret);
- }
ret = avs_ipc_get_fw_config(adev, &adev->fw_cfg);
- if (ret) {
- dev_err(adev->dev, "get fw cfg failed: %d\n", ret);
+ if (ret)
return AVS_IPC_RET(ret);
- }
adev->core_refs = devm_kcalloc(adev->dev, adev->hw_cfg.dsp_cores,
sizeof(*adev->core_refs), GFP_KERNEL);
diff --git a/sound/soc/intel/avs/messages.c b/sound/soc/intel/avs/messages.c
index ec458bd51b10..30b666f8909b 100644
--- a/sound/soc/intel/avs/messages.c
+++ b/sound/soc/intel/avs/messages.c
@@ -400,10 +400,12 @@ int avs_ipc_get_fw_config(struct avs_dev *adev, struct avs_fw_cfg *cfg)
AVS_BASEFW_FIRMWARE_CONFIG, NULL, 0,
&payload, &payload_size);
if (ret)
- return ret;
+ goto err;
/* Non-zero payload expected for FIRMWARE_CONFIG. */
- if (!payload_size)
- return -EREMOTEIO;
+ if (!payload_size) {
+ ret = -EREMOTEIO;
+ goto err;
+ }
while (offset < payload_size) {
tlv = (struct avs_tlv *)(payload + offset);
@@ -502,6 +504,9 @@ int avs_ipc_get_fw_config(struct avs_dev *adev, struct avs_fw_cfg *cfg)
/* No longer needed, free it as it's owned by the get_large_config() caller. */
kfree(payload);
+err:
+ if (ret)
+ dev_err(adev->dev, "get fw cfg failed: %d\n", ret);
return ret;
}
@@ -517,10 +522,12 @@ int avs_ipc_get_hw_config(struct avs_dev *adev, struct avs_hw_cfg *cfg)
AVS_BASEFW_HARDWARE_CONFIG, NULL, 0,
&payload, &payload_size);
if (ret)
- return ret;
+ goto err;
/* Non-zero payload expected for HARDWARE_CONFIG. */
- if (!payload_size)
- return -EREMOTEIO;
+ if (!payload_size) {
+ ret = -EREMOTEIO;
+ goto err;
+ }
while (offset < payload_size) {
tlv = (struct avs_tlv *)(payload + offset);
@@ -590,6 +597,9 @@ int avs_ipc_get_hw_config(struct avs_dev *adev, struct avs_hw_cfg *cfg)
exit:
/* No longer needed, free it as it's owned by the get_large_config() caller. */
kfree(payload);
+err:
+ if (ret)
+ dev_err(adev->dev, "get hw cfg failed: %d\n", ret);
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (5 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 06/13] ASoC: Intel: avs: Print IPC error messages in lower layer Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-10 18:09 ` Pierre-Louis Bossart
2025-01-09 12:22 ` [PATCH 08/13] ASoC: Intel: avs: Clearly state assumptions of hw_params() Cezary Rojewski
` (6 subsequent siblings)
13 siblings, 1 reply; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
MODULE_FIRMWARE macro adds hint to module information about which FW is
expected to be present on file system.
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/core.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 82839d0994ee..0e750e9e01d9 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -902,3 +902,13 @@ MODULE_AUTHOR("Cezary Rojewski <cezary.rojewski@intel.com>");
MODULE_AUTHOR("Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>");
MODULE_DESCRIPTION("Intel cAVS sound driver");
MODULE_LICENSE("GPL");
+MODULE_FIRMWARE("intel/skl/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/apl/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/cnl/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/icl/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/jsl/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/lkf/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/tgl/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/ehl/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/adl/dsp_basefw.bin");
+MODULE_FIRMWARE("intel/adl_n/dsp_basefw.bin");
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW
2025-01-09 12:22 ` [PATCH 07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW Cezary Rojewski
@ 2025-01-10 18:09 ` Pierre-Louis Bossart
2025-01-13 9:41 ` Cezary Rojewski
0 siblings, 1 reply; 19+ messages in thread
From: Pierre-Louis Bossart @ 2025-01-10 18:09 UTC (permalink / raw)
To: Cezary Rojewski, broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski
> +MODULE_FIRMWARE("intel/skl/dsp_basefw.bin");
> +MODULE_FIRMWARE("intel/apl/dsp_basefw.bin");
> +MODULE_FIRMWARE("intel/cnl/dsp_basefw.bin");
> +MODULE_FIRMWARE("intel/icl/dsp_basefw.bin");
> +MODULE_FIRMWARE("intel/jsl/dsp_basefw.bin");
> +MODULE_FIRMWARE("intel/lkf/dsp_basefw.bin");
Lakefield? Is this really a supported platform? It was EOL'ed a year after launch, wasn't it?
> +MODULE_FIRMWARE("intel/tgl/dsp_basefw.bin");
> +MODULE_FIRMWARE("intel/ehl/dsp_basefw.bin");
> +MODULE_FIRMWARE("intel/adl/dsp_basefw.bin");
> +MODULE_FIRMWARE("intel/adl_n/dsp_basefw.bin");
If you start listing the variants of ADL, then shouldn't you also list the variants of TGL?
Same for CNL, there are multiple variants, not to mention different signing keys.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW
2025-01-10 18:09 ` Pierre-Louis Bossart
@ 2025-01-13 9:41 ` Cezary Rojewski
2025-01-17 15:59 ` Pierre-Louis Bossart
0 siblings, 1 reply; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-13 9:41 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Mark Brown
On 2025-01-10 7:09 PM, Pierre-Louis Bossart wrote:
>
>> +MODULE_FIRMWARE("intel/skl/dsp_basefw.bin");
>> +MODULE_FIRMWARE("intel/apl/dsp_basefw.bin");
>> +MODULE_FIRMWARE("intel/cnl/dsp_basefw.bin");
>> +MODULE_FIRMWARE("intel/icl/dsp_basefw.bin");
>> +MODULE_FIRMWARE("intel/jsl/dsp_basefw.bin");
>> +MODULE_FIRMWARE("intel/lkf/dsp_basefw.bin");
>
> Lakefield? Is this really a supported platform? It was EOL'ed a year after launch, wasn't it?
We still keep all the platforms, regardless of their age, in the CI to
cross check firmware-API compatibility even with newest generations such
as PanterLake or FriscoLake.
Few years ago the team did quite a job to streamline LakeField (LKF)
with other cAVS 2.5 platforms on the firmware side. The TLDR is:
- one firmware branch covers from LKF (cAVS 2.1) up to
AlderLake/RaptorLake and all their spinoffs (cAVS 2.5)
- one tgl.c file on the avs-driver side covers the exact same range
so, any kind of redundancy is greatly limited. With that in mind, we
utilize the cAVS CI against the ACE (MeteorLake onwards) equivalent to
verify API compatibility between those two firmware branches to lower
the software development cost.
>> +MODULE_FIRMWARE("intel/tgl/dsp_basefw.bin");
>> +MODULE_FIRMWARE("intel/ehl/dsp_basefw.bin");
>> +MODULE_FIRMWARE("intel/adl/dsp_basefw.bin");
>> +MODULE_FIRMWARE("intel/adl_n/dsp_basefw.bin");
>
> If you start listing the variants of ADL, then shouldn't you also list the variants of TGL?
> Same for CNL, there are multiple variants, not to mention different signing keys.
The only reason ADL and ADL-N are listed separately is binary
incompatibility - MEU differs between the two. In essence, one can use
ADL-binaries for all ADL/RPL based platforms _except_ for ADL-N based
ones. The code is the same, the verification process is different. For
all other major platforms, no MEU differences so one binary covers all
variants.
In regard to the key, the approach is: it's ignored.
Whatever is in the directory under 'dsp_basefw.bin' will be attempted
for booting the DSP. By default, what lands on the market is a
production-type-signed binaries. Internally CI runs with prod -or- debug
signed binaries but we do not intend to share the latter to the official
linux-firmware repo.
Kind regards,
Czarek
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW
2025-01-13 9:41 ` Cezary Rojewski
@ 2025-01-17 15:59 ` Pierre-Louis Bossart
2025-01-21 8:39 ` Cezary Rojewski
0 siblings, 1 reply; 19+ messages in thread
From: Pierre-Louis Bossart @ 2025-01-17 15:59 UTC (permalink / raw)
To: Cezary Rojewski
Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Mark Brown
>>> +MODULE_FIRMWARE("intel/tgl/dsp_basefw.bin");
>>> +MODULE_FIRMWARE("intel/ehl/dsp_basefw.bin");
>>> +MODULE_FIRMWARE("intel/adl/dsp_basefw.bin");
>>> +MODULE_FIRMWARE("intel/adl_n/dsp_basefw.bin");
>>
>> If you start listing the variants of ADL, then shouldn't you also list the variants of TGL?
>> Same for CNL, there are multiple variants, not to mention different signing keys.
>
> The only reason ADL and ADL-N are listed separately is binary incompatibility - MEU differs between the two. In essence, one can use ADL-binaries for all ADL/RPL based platforms _except_ for ADL-N based ones. The code is the same, the verification process is different. For all other major platforms, no MEU differences so one binary covers all variants.
>
> In regard to the key, the approach is: it's ignored.
>
> Whatever is in the directory under 'dsp_basefw.bin' will be attempted for booting the DSP. By default, what lands on the market is a production-type-signed binaries. Internally CI runs with prod -or- debug signed binaries but we do not intend to share the latter to the official linux-firmware repo.
You didn't get my point. There are also binary incompatibilities between TGL and TGL-H (different number of cores) and in the CNL/CML space there were different production keys and MEU snaffus requiring different binaries to be released.
This wasn't about the production/debug problem, I was only referring to 'production' releases.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW
2025-01-17 15:59 ` Pierre-Louis Bossart
@ 2025-01-21 8:39 ` Cezary Rojewski
0 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-21 8:39 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Mark Brown
On 2025-01-17 4:59 PM, Pierre-Louis Bossart wrote:
>
>>>> +MODULE_FIRMWARE("intel/tgl/dsp_basefw.bin");
>>>> +MODULE_FIRMWARE("intel/ehl/dsp_basefw.bin");
>>>> +MODULE_FIRMWARE("intel/adl/dsp_basefw.bin");
>>>> +MODULE_FIRMWARE("intel/adl_n/dsp_basefw.bin");
>>>
>>> If you start listing the variants of ADL, then shouldn't you also list the variants of TGL?
>>> Same for CNL, there are multiple variants, not to mention different signing keys.
>>
>> The only reason ADL and ADL-N are listed separately is binary incompatibility - MEU differs between the two. In essence, one can use ADL-binaries for all ADL/RPL based platforms _except_ for ADL-N based ones. The code is the same, the verification process is different. For all other major platforms, no MEU differences so one binary covers all variants.
>>
>> In regard to the key, the approach is: it's ignored.
>>
>> Whatever is in the directory under 'dsp_basefw.bin' will be attempted for booting the DSP. By default, what lands on the market is a production-type-signed binaries. Internally CI runs with prod -or- debug signed binaries but we do not intend to share the latter to the official linux-firmware repo.
>
> You didn't get my point. There are also binary incompatibilities between TGL and TGL-H (different number of cores) and in the CNL/CML space there were different production keys and MEU snaffus requiring different binaries to be released.
> This wasn't about the production/debug problem, I was only referring to 'production' releases.
I'm afraid there are differences between cAVS/ACE firmware and SOF. When
it comes to the cAVS/ACE, TGL devices are all handled using the exact
same binary and the firmware reads certain hardware capabilities in
runtime. CML-R (CML platform with TGP PCH) falls into the same category.
Instead, the cAVS/ACE expects the software to send BUS_HARDWARE_ID IPC
to inform it about details of the hardware revision e.g.: the stepping.
Firmware may decide to tailor certain flows depending on that detailed
information.
Kind regards,
Czarek
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 08/13] ASoC: Intel: avs: Clearly state assumptions of hw_params()
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (6 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 09/13] ASoC: Intel: avs: Improve logging of firmware loading Cezary Rojewski
` (5 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
There are no NULL-checks for fe/be_hw_params as there is an implicit
assumption that framework opens valid DPCMs only. State that clearly.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/pcm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 5878cfdbbee5..4bfbcb5a5ae8 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -161,6 +161,7 @@ static int avs_dai_be_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_dpcm *dpcm;
be = snd_soc_substream_to_rtd(substream);
+ /* dpcm_fe_dai_open() guarantees the list is not empty at this point. */
for_each_dpcm_fe(be, substream->stream, dpcm) {
fe = dpcm->fe;
fe_hw_params = &fe->dpcm[substream->stream].hw_params;
@@ -576,6 +577,7 @@ static int avs_dai_fe_hw_params(struct snd_pcm_substream *substream,
hdac_stream(host_stream)->format_val = 0;
fe = snd_soc_substream_to_rtd(substream);
+ /* dpcm_fe_dai_open() guarantees the list is not empty at this point. */
for_each_dpcm_be(fe, substream->stream, dpcm) {
be = dpcm->be;
be_hw_params = &be->dpcm[substream->stream].hw_params;
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 09/13] ASoC: Intel: avs: Improve logging of firmware loading
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (7 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 08/13] ASoC: Intel: avs: Clearly state assumptions of hw_params() Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 10/13] ASoC: Intel: avs: Update ASRC definition Cezary Rojewski
` (4 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
Crucial debug information regarding the ROM/firmware status and last
known error code is missing in the code loading functions.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/loader.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c
index 4203b216ae13..b3ea35d267e9 100644
--- a/sound/soc/intel/avs/loader.c
+++ b/sound/soc/intel/avs/loader.c
@@ -167,7 +167,8 @@ int avs_cldma_load_basefw(struct avs_dev *adev, struct firmware *fw)
(reg & AVS_ROM_INIT_DONE) == AVS_ROM_INIT_DONE,
AVS_ROM_INIT_POLLING_US, SKL_ROM_INIT_TIMEOUT_US);
if (ret < 0) {
- dev_err(adev->dev, "rom init timeout: %d\n", ret);
+ dev_err(adev->dev, "rom init failed: %d, status: 0x%08x, lec: 0x%08x\n",
+ ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
return ret;
}
@@ -180,7 +181,8 @@ int avs_cldma_load_basefw(struct avs_dev *adev, struct firmware *fw)
AVS_FW_INIT_POLLING_US, AVS_FW_INIT_TIMEOUT_US);
hda_cldma_stop(cl);
if (ret < 0) {
- dev_err(adev->dev, "transfer fw failed: %d\n", ret);
+ dev_err(adev->dev, "transfer fw failed: %d, status: 0x%08x, lec: 0x%08x\n",
+ ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
return ret;
}
@@ -313,7 +315,8 @@ avs_hda_init_rom(struct avs_dev *adev, unsigned int dma_id, bool purge)
(reg & 0xF) == APL_ROM_FW_ENTERED,
AVS_ROM_INIT_POLLING_US, APL_ROM_INIT_TIMEOUT_US);
if (ret < 0) {
- dev_err(adev->dev, "rom init timeout: %d\n", ret);
+ dev_err(adev->dev, "rom init failed: %d, status: 0x%08x, lec: 0x%08x\n",
+ ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
goto err;
}
@@ -337,15 +340,15 @@ static int avs_imr_load_basefw(struct avs_dev *adev)
/* DMA id ignored when flashing from IMR as no transfer occurs. */
ret = avs_hda_init_rom(adev, 0, false);
- if (ret < 0) {
- dev_err(adev->dev, "rom init failed: %d\n", ret);
+ if (ret < 0)
return ret;
- }
ret = wait_for_completion_timeout(&adev->fw_ready,
msecs_to_jiffies(AVS_FW_INIT_TIMEOUT_MS));
if (!ret) {
- dev_err(adev->dev, "firmware ready timeout\n");
+ dev_err(adev->dev, "firmware ready timeout, status: 0x%08x, lec: 0x%08x\n",
+ snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev)),
+ snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
return -ETIMEDOUT;
}
@@ -392,7 +395,7 @@ int avs_hda_load_basefw(struct avs_dev *adev, struct firmware *fw)
ret = avs_hda_init_rom(adev, dma_id, true);
if (!ret)
break;
- dev_info(adev->dev, "#%d rom init fail: %d\n", i + 1, ret);
+ dev_info(adev->dev, "#%d rom init failed: %d\n", i + 1, ret);
}
if (ret < 0)
goto cleanup_resources;
@@ -404,7 +407,8 @@ int avs_hda_load_basefw(struct avs_dev *adev, struct firmware *fw)
AVS_FW_INIT_POLLING_US, AVS_FW_INIT_TIMEOUT_US);
snd_hdac_dsp_trigger(hstream, false);
if (ret < 0) {
- dev_err(adev->dev, "transfer fw failed: %d\n", ret);
+ dev_err(adev->dev, "transfer fw failed: %d, status: 0x%08x, lec: 0x%08x\n",
+ ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
}
@@ -584,7 +588,9 @@ static int avs_dsp_load_basefw(struct avs_dev *adev)
ret = wait_for_completion_timeout(&adev->fw_ready,
msecs_to_jiffies(AVS_FW_INIT_TIMEOUT_MS));
if (!ret) {
- dev_err(adev->dev, "firmware ready timeout\n");
+ dev_err(adev->dev, "firmware ready timeout, status: 0x%08x, lec: 0x%08x\n",
+ snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev)),
+ snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
ret = -ETIMEDOUT;
goto release_fw;
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 10/13] ASoC: Intel: avs: Update ASRC definition
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (8 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 09/13] ASoC: Intel: avs: Improve logging of firmware loading Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 11/13] ASoC: Intel: avs: Adjust DSP status register names Cezary Rojewski
` (3 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
To support ASRC for playback streams, update its descriptor.
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/messages.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/intel/avs/messages.h b/sound/soc/intel/avs/messages.h
index d0bdb7d9266c..0378633c7f96 100644
--- a/sound/soc/intel/avs/messages.h
+++ b/sound/soc/intel/avs/messages.h
@@ -859,8 +859,7 @@ static_assert(sizeof(struct avs_aec_cfg) == 92);
struct avs_asrc_cfg {
struct avs_modcfg_base base;
u32 out_freq;
- u32 rsvd0:1;
- u32 mode:1;
+ u32 mode:2;
u32 rsvd2:2;
u32 disable_jitter_buffer:1;
u32 rsvd3:27;
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 11/13] ASoC: Intel: avs: Adjust DSP status register names
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (9 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 10/13] ASoC: Intel: avs: Update ASRC definition Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 12/13] ASoC: Intel: avs: Adjust IPC traces Cezary Rojewski
` (2 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
Both status and error are "codes". Update the wording to make code
cohesive.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/loader.c | 12 ++++++------
sound/soc/intel/avs/registers.h | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c
index b3ea35d267e9..9ff7818395cd 100644
--- a/sound/soc/intel/avs/loader.c
+++ b/sound/soc/intel/avs/loader.c
@@ -168,7 +168,7 @@ int avs_cldma_load_basefw(struct avs_dev *adev, struct firmware *fw)
AVS_ROM_INIT_POLLING_US, SKL_ROM_INIT_TIMEOUT_US);
if (ret < 0) {
dev_err(adev->dev, "rom init failed: %d, status: 0x%08x, lec: 0x%08x\n",
- ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
+ ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
return ret;
}
@@ -182,7 +182,7 @@ int avs_cldma_load_basefw(struct avs_dev *adev, struct firmware *fw)
hda_cldma_stop(cl);
if (ret < 0) {
dev_err(adev->dev, "transfer fw failed: %d, status: 0x%08x, lec: 0x%08x\n",
- ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
+ ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
return ret;
}
@@ -316,7 +316,7 @@ avs_hda_init_rom(struct avs_dev *adev, unsigned int dma_id, bool purge)
AVS_ROM_INIT_POLLING_US, APL_ROM_INIT_TIMEOUT_US);
if (ret < 0) {
dev_err(adev->dev, "rom init failed: %d, status: 0x%08x, lec: 0x%08x\n",
- ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
+ ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
goto err;
}
@@ -348,7 +348,7 @@ static int avs_imr_load_basefw(struct avs_dev *adev)
if (!ret) {
dev_err(adev->dev, "firmware ready timeout, status: 0x%08x, lec: 0x%08x\n",
snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev)),
- snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
+ snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
return -ETIMEDOUT;
}
@@ -408,7 +408,7 @@ int avs_hda_load_basefw(struct avs_dev *adev, struct firmware *fw)
snd_hdac_dsp_trigger(hstream, false);
if (ret < 0) {
dev_err(adev->dev, "transfer fw failed: %d, status: 0x%08x, lec: 0x%08x\n",
- ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
+ ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
}
@@ -590,7 +590,7 @@ static int avs_dsp_load_basefw(struct avs_dev *adev)
if (!ret) {
dev_err(adev->dev, "firmware ready timeout, status: 0x%08x, lec: 0x%08x\n",
snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev)),
- snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR_CODE(adev)));
+ snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
ret = -ETIMEDOUT;
goto release_fw;
diff --git a/sound/soc/intel/avs/registers.h b/sound/soc/intel/avs/registers.h
index f76e91cff2a9..64afd56b1897 100644
--- a/sound/soc/intel/avs/registers.h
+++ b/sound/soc/intel/avs/registers.h
@@ -74,7 +74,7 @@
/* Constants used when accessing SRAM, space shared with firmware */
#define AVS_FW_REG_BASE(adev) ((adev)->spec->sram->base_offset)
#define AVS_FW_REG_STATUS(adev) (AVS_FW_REG_BASE(adev) + 0x0)
-#define AVS_FW_REG_ERROR_CODE(adev) (AVS_FW_REG_BASE(adev) + 0x4)
+#define AVS_FW_REG_ERROR(adev) (AVS_FW_REG_BASE(adev) + 0x4)
#define AVS_WINDOW_CHUNK_SIZE SZ_4K
#define AVS_FW_REGS_SIZE AVS_WINDOW_CHUNK_SIZE
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 12/13] ASoC: Intel: avs: Adjust IPC traces
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (10 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 11/13] ASoC: Intel: avs: Adjust DSP status register names Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 12:22 ` [PATCH 13/13] ASoC: Intel: avs: Add missing includes Cezary Rojewski
2025-01-09 16:40 ` [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Mark Brown
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
The firmware status and the firmware error registers are 4-bytes wide.
Update trace macros and their usage to reflect that.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/ipc.c | 25 ++++++++++++++----------
sound/soc/intel/avs/trace.h | 38 +++++++++++++++++++------------------
2 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/sound/soc/intel/avs/ipc.c b/sound/soc/intel/avs/ipc.c
index 4fba46e77c47..08ed9d96738a 100644
--- a/sound/soc/intel/avs/ipc.c
+++ b/sound/soc/intel/avs/ipc.c
@@ -184,10 +184,11 @@ static void avs_dsp_receive_rx(struct avs_dev *adev, u64 header)
{
struct avs_ipc *ipc = adev->ipc;
union avs_reply_msg msg = AVS_MSG(header);
- u64 reg;
+ u32 sts, lec;
- reg = readq(avs_sram_addr(adev, AVS_FW_REGS_WINDOW));
- trace_avs_ipc_reply_msg(header, reg);
+ sts = snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev));
+ lec = snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev));
+ trace_avs_ipc_reply_msg(header, sts, lec);
ipc->rx.header = header;
/* Abort copying payload if request processing was unsuccessful. */
@@ -209,10 +210,11 @@ static void avs_dsp_process_notification(struct avs_dev *adev, u64 header)
union avs_notify_msg msg = AVS_MSG(header);
size_t data_size = 0;
void *data = NULL;
- u64 reg;
+ u32 sts, lec;
- reg = readq(avs_sram_addr(adev, AVS_FW_REGS_WINDOW));
- trace_avs_ipc_notify_msg(header, reg);
+ sts = snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev));
+ lec = snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev));
+ trace_avs_ipc_notify_msg(header, sts, lec);
/* Ignore spurious notifications until handshake is established. */
if (!adev->ipc->ready && msg.notify_msg_type != AVS_NOTIFY_FW_READY) {
@@ -367,13 +369,16 @@ static void avs_ipc_msg_init(struct avs_ipc *ipc, struct avs_ipc_msg *reply)
static void avs_dsp_send_tx(struct avs_dev *adev, struct avs_ipc_msg *tx, bool read_fwregs)
{
const struct avs_spec *const spec = adev->spec;
- u64 reg = ULONG_MAX;
+ u32 sts = UINT_MAX;
+ u32 lec = UINT_MAX;
tx->header |= spec->hipc->req_busy_mask;
- if (read_fwregs)
- reg = readq(avs_sram_addr(adev, AVS_FW_REGS_WINDOW));
+ if (read_fwregs) {
+ sts = snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev));
+ lec = snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev));
+ }
- trace_avs_request(tx, reg);
+ trace_avs_request(tx, sts, lec);
if (tx->size)
memcpy_toio(avs_downlink_addr(adev), tx->data, tx->size);
diff --git a/sound/soc/intel/avs/trace.h b/sound/soc/intel/avs/trace.h
index c9eaa5a60ed3..f4288d0ad5ef 100644
--- a/sound/soc/intel/avs/trace.h
+++ b/sound/soc/intel/avs/trace.h
@@ -37,60 +37,62 @@ TRACE_EVENT(avs_dsp_core_op,
void trace_avs_msg_payload(const void *data, size_t size);
-#define trace_avs_request(msg, fwregs) \
+#define trace_avs_request(msg, sts, lec) \
({ \
- trace_avs_ipc_request_msg((msg)->header, fwregs); \
+ trace_avs_ipc_request_msg((msg)->header, sts, lec); \
trace_avs_msg_payload((msg)->data, (msg)->size); \
})
-#define trace_avs_reply(msg, fwregs) \
+#define trace_avs_reply(msg, sts, lec) \
({ \
- trace_avs_ipc_reply_msg((msg)->header, fwregs); \
+ trace_avs_ipc_reply_msg((msg)->header, sts, lec); \
trace_avs_msg_payload((msg)->data, (msg)->size); \
})
-#define trace_avs_notify(msg, fwregs) \
+#define trace_avs_notify(msg, sts, lec) \
({ \
- trace_avs_ipc_notify_msg((msg)->header, fwregs); \
+ trace_avs_ipc_notify_msg((msg)->header, sts, lec); \
trace_avs_msg_payload((msg)->data, (msg)->size); \
})
#endif
DECLARE_EVENT_CLASS(avs_ipc_msg_hdr,
- TP_PROTO(u64 header, u64 fwregs),
+ TP_PROTO(u64 header, u32 sts, u32 lec),
- TP_ARGS(header, fwregs),
+ TP_ARGS(header, sts, lec),
TP_STRUCT__entry(
__field(u64, header)
- __field(u64, fwregs)
+ __field(u32, sts)
+ __field(u32, lec)
),
TP_fast_assign(
__entry->header = header;
- __entry->fwregs = fwregs;
+ __entry->sts = sts;
+ __entry->lec = lec;
),
TP_printk("primary: 0x%08X, extension: 0x%08X,\n"
- "fwstatus: 0x%08X, fwerror: 0x%08X",
+ "status: 0x%08X, error: 0x%08X",
lower_32_bits(__entry->header), upper_32_bits(__entry->header),
- lower_32_bits(__entry->fwregs), upper_32_bits(__entry->fwregs))
+ __entry->sts, __entry->lec)
);
DEFINE_EVENT(avs_ipc_msg_hdr, avs_ipc_request_msg,
- TP_PROTO(u64 header, u64 fwregs),
- TP_ARGS(header, fwregs)
+ TP_PROTO(u64 header, u32 sts, u32 lec),
+ TP_ARGS(header, sts, lec)
);
DEFINE_EVENT(avs_ipc_msg_hdr, avs_ipc_reply_msg,
- TP_PROTO(u64 header, u64 fwregs),
- TP_ARGS(header, fwregs)
+ TP_PROTO(u64 header, u32 sts, u32 lec),
+ TP_ARGS(header, sts, lec)
);
DEFINE_EVENT(avs_ipc_msg_hdr, avs_ipc_notify_msg,
- TP_PROTO(u64 header, u64 fwregs),
- TP_ARGS(header, fwregs)
+ TP_PROTO(u64 header, u32 sts, u32 lec),
+ TP_ARGS(header, sts, lec)
);
TRACE_EVENT_CONDITION(avs_ipc_msg_payload,
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 13/13] ASoC: Intel: avs: Add missing includes
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (11 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 12/13] ASoC: Intel: avs: Adjust IPC traces Cezary Rojewski
@ 2025-01-09 12:22 ` Cezary Rojewski
2025-01-09 16:40 ` [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Mark Brown
13 siblings, 0 replies; 19+ messages in thread
From: Cezary Rojewski @ 2025-01-09 12:22 UTC (permalink / raw)
To: broonie; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski, Cezary Rojewski
The debugfs file utilizes string helpers such as parse_int_array_user()
yet does not include the header.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/debugfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/intel/avs/debugfs.c b/sound/soc/intel/avs/debugfs.c
index 1767ded4d983..8c4edda97f75 100644
--- a/sound/soc/intel/avs/debugfs.c
+++ b/sound/soc/intel/avs/debugfs.c
@@ -10,6 +10,7 @@
#include <linux/kfifo.h>
#include <linux/wait.h>
#include <linux/sched/signal.h>
+#include <linux/string_helpers.h>
#include <sound/soc.h>
#include "avs.h"
#include "messages.h"
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups
2025-01-09 12:22 [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups Cezary Rojewski
` (12 preceding siblings ...)
2025-01-09 12:22 ` [PATCH 13/13] ASoC: Intel: avs: Add missing includes Cezary Rojewski
@ 2025-01-09 16:40 ` Mark Brown
13 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2025-01-09 16:40 UTC (permalink / raw)
To: Cezary Rojewski; +Cc: linux-sound, tiwai, perex, amadeuszx.slawinski
On Thu, 09 Jan 2025 13:22:03 +0100, Cezary Rojewski wrote:
> A set of loosely connected changes, fixing few outstanding issues as
> well as improving readability of the existing code.
>
> The fixes lead the series, first five patches. The goal is to make sure
> proper read() is used when accessing the registers, probe() and remove()
> sequences for HDAudio streaming are synced, minimal AudioDSP firmware
> version points to correct values and recent additions to the topology
> are parsed properly.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[01/13] ASoC: Intel: avs: Do not readq() u32 registers
commit: bca0fa5f6b5e96c03daac1ed62b1e5c5057a2048
[02/13] ASoC: Intel: avs: Fix the minimum firmware version numbers
commit: dbda5c35b88794f6e5efe1b5b20044b0b3a340d4
[03/13] ASoC: Intel: avs: Fix theoretical infinite loop
commit: cf4d74256fe103ece7b2647550e6c063048e5682
[04/13] ASoC: Intel: avs: Fix init-config parsing
commit: e9ca3db9f01a7ce91ceab35cd5fa52f0c5aca174
[05/13] ASoC: Intel: avs: Update hda component teardown sequences
commit: e3146775f05d18c667a2e26082da3ac105f87d9f
[06/13] ASoC: Intel: avs: Print IPC error messages in lower layer
commit: 33228036ff655ebed1bc4bde9c9b6329b569b27b
[07/13] ASoC: Intel: avs: Add MODULE_FIRMWARE to inform about FW
commit: 94aa347d34e079859a5378272c9452c728e4183a
[08/13] ASoC: Intel: avs: Clearly state assumptions of hw_params()
commit: 0ca529926c5d9d0a3c0b1609fb7034ab870e4770
[09/13] ASoC: Intel: avs: Improve logging of firmware loading
commit: 480d9bb9cfb7b774b22cf82ff21903eb50d64cb9
[10/13] ASoC: Intel: avs: Update ASRC definition
commit: aea305d28551bc213aab3a41a0f59412247ae2b4
[11/13] ASoC: Intel: avs: Adjust DSP status register names
commit: 3eede0fc99c684df6f3f35866761036dabf89d05
[12/13] ASoC: Intel: avs: Adjust IPC traces
commit: ef724707788325a53ffa4cf58fceb94654e4793a
[13/13] ASoC: Intel: avs: Add missing includes
commit: 0b12850ddfb0032376ef1be10b5b46be00bba4d4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 19+ messages in thread