From c50cea054e04769471d2f17a57fafd7c5dfe8df8 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:42 +0200 Subject: [PATCH 01/12] ASoC: Intel: avs: Register HDAudio ext-bus operations With ASoC representation of HDAudio codec added, update bus initiazation to complete it. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-2-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/Kconfig | 2 +- sound/soc/intel/avs/core.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index e5107a3ce16a..ded903f95b67 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -216,7 +216,7 @@ config SND_SOC_INTEL_AVS depends on COMMON_CLK select SND_SOC_ACPI if ACPI select SND_SOC_TOPOLOGY - select SND_HDA + select SND_SOC_HDA select SND_HDA_EXT_CORE select SND_HDA_DSP_LOADER select SND_INTEL_DSP_CONFIG diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c index 3a0997c3af2b..664f87c33e9d 100644 --- a/sound/soc/intel/avs/core.c +++ b/sound/soc/intel/avs/core.c @@ -23,6 +23,7 @@ #include #include #include +#include "../../codecs/hda.h" #include "avs.h" #include "cldma.h" @@ -356,7 +357,7 @@ static int avs_bus_init(struct avs_dev *adev, struct pci_dev *pci, const struct struct device *dev = &pci->dev; int ret; - ret = snd_hdac_ext_bus_init(&bus->core, dev, NULL, NULL); + ret = snd_hdac_ext_bus_init(&bus->core, dev, NULL, &soc_hda_ext_bus_ops); if (ret < 0) return ret; From 5f267aa4adad13f764e0b00926c349f8728fce77 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:43 +0200 Subject: [PATCH 02/12] ASoC: Intel: avs: Assign I2S gateway when parsing topology For formatted port - ssp%d - descriptions to have an effect, copier module templates need to be updated with specified port value. This value is later propagated to the firmware when module instances are being instantiated. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-3-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/topology.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c index 6a06fe387d13..8a9f9fc48938 100644 --- a/sound/soc/intel/avs/topology.c +++ b/sound/soc/intel/avs/topology.c @@ -808,6 +808,30 @@ static const struct avs_tplg_token_parser pin_format_parsers[] = { }, }; +static void +assign_copier_gtw_instance(struct snd_soc_component *comp, struct avs_tplg_modcfg_ext *cfg) +{ + struct snd_soc_acpi_mach *mach; + + if (!guid_equal(&cfg->type, &AVS_COPIER_MOD_UUID)) + return; + + /* Only I2S boards assign port instance in ->i2s_link_mask. */ + switch (cfg->copier.dma_type) { + case AVS_DMA_I2S_LINK_OUTPUT: + case AVS_DMA_I2S_LINK_INPUT: + break; + default: + return; + } + + mach = dev_get_platdata(comp->card->dev); + + /* Automatic assignment only when board describes single SSP. */ + if (hweight_long(mach->mach_params.i2s_link_mask) == 1 && !cfg->copier.vindex.i2s.instance) + cfg->copier.vindex.i2s.instance = __ffs(mach->mach_params.i2s_link_mask); +} + static int avs_tplg_parse_modcfg_ext(struct snd_soc_component *comp, struct avs_tplg_modcfg_ext *cfg, struct snd_soc_tplg_vendor_array *tuples, @@ -827,6 +851,9 @@ static int avs_tplg_parse_modcfg_ext(struct snd_soc_component *comp, if (ret) return ret; + /* Update copier gateway based on board's i2s_link_mask. */ + assign_copier_gtw_instance(comp, cfg); + block_size -= esize; /* Parse trailing in/out pin formats if any. */ if (block_size) { From 8192d24cccfbd93dadefd2b7553ff15e41d0e680 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:44 +0200 Subject: [PATCH 03/12] ASoC: Intel: avs: Relax DSP core transition timings To avoid any false positives when checking CPA after setting SPA, do a short wait. For stall operation, give HW more time to propagate the change before moving on. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-4-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/dsp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/avs/dsp.c b/sound/soc/intel/avs/dsp.c index 06d2f7af520f..b881100d3e02 100644 --- a/sound/soc/intel/avs/dsp.c +++ b/sound/soc/intel/avs/dsp.c @@ -13,6 +13,7 @@ #define AVS_ADSPCS_INTERVAL_US 500 #define AVS_ADSPCS_TIMEOUT_US 50000 +#define AVS_ADSPCS_DELAY_US 1000 int avs_dsp_core_power(struct avs_dev *adev, u32 core_mask, bool power) { @@ -26,6 +27,8 @@ int avs_dsp_core_power(struct avs_dev *adev, u32 core_mask, bool power) value = power ? mask : 0; snd_hdac_adsp_updatel(adev, AVS_ADSP_REG_ADSPCS, mask, value); + /* Delay the polling to avoid false positives. */ + usleep_range(AVS_ADSPCS_DELAY_US, 2 * AVS_ADSPCS_DELAY_US); mask = AVS_ADSPCS_CPA_MASK(core_mask); value = power ? mask : 0; @@ -82,11 +85,15 @@ int avs_dsp_core_stall(struct avs_dev *adev, u32 core_mask, bool stall) reg, (reg & mask) == value, AVS_ADSPCS_INTERVAL_US, AVS_ADSPCS_TIMEOUT_US); - if (ret) + if (ret) { dev_err(adev->dev, "core_mask %d %sstall failed: %d\n", core_mask, stall ? "" : "un", ret); + return ret; + } - return ret; + /* Give HW time to propagate the change. */ + usleep_range(AVS_ADSPCS_DELAY_US, 2 * AVS_ADSPCS_DELAY_US); + return 0; } int avs_dsp_core_enable(struct avs_dev *adev, u32 core_mask) From 3c1923a119a61534f8ce221766041ddf470c9307 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:45 +0200 Subject: [PATCH 04/12] ASoC: Intel: avs: Copy only as many RX bytes as necessary There is no need to copy number of bytes specified by IPC message caller if DSP firmware returned lower number. In consequence, LARGE_CONFIG_GET handler is simplified. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-5-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/ipc.c | 1 + sound/soc/intel/avs/messages.c | 6 ++---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/sound/soc/intel/avs/ipc.c b/sound/soc/intel/avs/ipc.c index d755ba8b8518..020d85c7520d 100644 --- a/sound/soc/intel/avs/ipc.c +++ b/sound/soc/intel/avs/ipc.c @@ -480,6 +480,7 @@ static int avs_dsp_do_send_msg(struct avs_dev *adev, struct avs_ipc_msg *request ret = ipc->rx.rsp.status; if (reply) { reply->header = ipc->rx.header; + reply->size = ipc->rx.size; if (reply->data && ipc->rx.size) memcpy(reply->data, ipc->rx.data, reply->size); } diff --git a/sound/soc/intel/avs/messages.c b/sound/soc/intel/avs/messages.c index 6404fce8cde4..3559fb496e0b 100644 --- a/sound/soc/intel/avs/messages.c +++ b/sound/soc/intel/avs/messages.c @@ -378,7 +378,6 @@ int avs_ipc_get_large_config(struct avs_dev *adev, u16 module_id, u8 instance_id union avs_module_msg msg = AVS_MODULE_REQUEST(LARGE_CONFIG_GET); struct avs_ipc_msg request; struct avs_ipc_msg reply = {{0}}; - size_t size; void *buf; int ret; @@ -406,15 +405,14 @@ int avs_ipc_get_large_config(struct avs_dev *adev, u16 module_id, u8 instance_id return ret; } - size = reply.rsp.ext.large_config.data_off_size; - buf = krealloc(reply.data, size, GFP_KERNEL); + buf = krealloc(reply.data, reply.size, GFP_KERNEL); if (!buf) { kfree(reply.data); return -ENOMEM; } *reply_data = buf; - *reply_size = size; + *reply_size = reply.size; return 0; } From 00566ad4ce9d394c6ebaacde12eda06eef4e5279 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:46 +0200 Subject: [PATCH 05/12] ASoC: Intel: avs: Shield LARGE_CONFIG_GETs against zero payload_size Some LARGE_CONFIG_GETs are never expected to return payload of size 0. Check for such situation and collapse if met. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-6-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/messages.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sound/soc/intel/avs/messages.c b/sound/soc/intel/avs/messages.c index 3559fb496e0b..9cf621eaec5a 100644 --- a/sound/soc/intel/avs/messages.c +++ b/sound/soc/intel/avs/messages.c @@ -474,6 +474,9 @@ int avs_ipc_get_fw_config(struct avs_dev *adev, struct avs_fw_cfg *cfg) &payload, &payload_size); if (ret) return ret; + /* Non-zero payload expected for FIRMWARE_CONFIG. */ + if (!payload_size) + return -EREMOTEIO; while (offset < payload_size) { tlv = (struct avs_tlv *)(payload + offset); @@ -587,6 +590,9 @@ int avs_ipc_get_hw_config(struct avs_dev *adev, struct avs_hw_cfg *cfg) &payload, &payload_size); if (ret) return ret; + /* Non-zero payload expected for HARDWARE_CONFIG. */ + if (!payload_size) + return -EREMOTEIO; while (offset < payload_size) { tlv = (struct avs_tlv *)(payload + offset); @@ -670,6 +676,9 @@ int avs_ipc_get_modules_info(struct avs_dev *adev, struct avs_mods_info **info) &payload, &payload_size); if (ret) return ret; + /* Non-zero payload expected for MODULES_INFO. */ + if (!payload_size) + return -EREMOTEIO; *info = (struct avs_mods_info *)payload; return 0; From daa36bbcd78bca24db84e273bcafec9a8f81c767 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:47 +0200 Subject: [PATCH 06/12] ASoC: Intel: avs: Block IPC channel on suspend To allow for driver's filesystem interfaces e.g.: debugfs, to be touched even when the device is asleep, mark IPC-channel as blocked when the device is suspended. This causes any invocation of said interfaces that do not toggle PM themselves to gracefully fail with "Operation not permitted" message. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-7-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c index 664f87c33e9d..4234adeb3d1c 100644 --- a/sound/soc/intel/avs/core.c +++ b/sound/soc/intel/avs/core.c @@ -556,6 +556,7 @@ static int __maybe_unused avs_suspend_common(struct avs_dev *adev) return AVS_IPC_RET(ret); } + avs_ipc_block(adev->ipc); avs_dsp_op(adev, int_control, false); snd_hdac_ext_bus_ppcap_int_enable(bus, false); From 8544eebc78c96f1834a46b26ade3e7ebe785d10c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?= Date: Thu, 7 Jul 2022 14:41:48 +0200 Subject: [PATCH 07/12] ASoC: Intel: avs: Set max DMA segment size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apparently it is possible for code to allocate large buffers which may cause warnings as reported in [1]. This was fixed for HDA, SOF and skylake in patchset [2], fix it also for avs driver. [1] https://github.com/thesofproject/linux/issues/3430 [2] https://lore.kernel.org/all/20220215132756.31236-1-tiwai@suse.de/ Signed-off-by: Amadeusz Sławiński Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-8-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c index 4234adeb3d1c..6a35bf45efcb 100644 --- a/sound/soc/intel/avs/core.c +++ b/sound/soc/intel/avs/core.c @@ -446,6 +446,7 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id) dma_set_mask(dev, DMA_BIT_MASK(32)); dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); } + dma_set_max_seg_size(dev, UINT_MAX); ret = avs_hdac_bus_init_streams(bus); if (ret < 0) { From a5bbbde2b81e41cea7fa1b38911e88da5febc2d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amadeusz=20S=C5=82awi=C5=84ski?= Date: Thu, 7 Jul 2022 14:41:49 +0200 Subject: [PATCH 08/12] ASoC: Intel: avs: Use helper function to set up DMA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dma_set_mask() and dma_set_coherent_mask() can be performed with one call to dma_set_mask_and_coherent(), which slightly reduces amount of code on our side. Signed-off-by: Amadeusz Sławiński Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-9-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/core.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c index 6a35bf45efcb..c50c20fd681a 100644 --- a/sound/soc/intel/avs/core.c +++ b/sound/soc/intel/avs/core.c @@ -440,12 +440,8 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id) if (bus->mlcap) snd_hdac_ext_bus_get_ml_capabilities(bus); - if (!dma_set_mask(dev, DMA_BIT_MASK(64))) { - dma_set_coherent_mask(dev, DMA_BIT_MASK(64)); - } else { - dma_set_mask(dev, DMA_BIT_MASK(32)); - dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); - } + if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); dma_set_max_seg_size(dev, UINT_MAX); ret = avs_hdac_bus_init_streams(bus); From 79c351fb50e7e37eacf93f55f1e7056148d0d814 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:50 +0200 Subject: [PATCH 09/12] ASoC: Intel: avs: Recognize FW_CFG_RESERVED If exposed by firmware, count RESERVED parameter as known one to avoid dumping noise in kernel logs. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-10-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/messages.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/intel/avs/messages.c b/sound/soc/intel/avs/messages.c index 9cf621eaec5a..28a948cf790f 100644 --- a/sound/soc/intel/avs/messages.c +++ b/sound/soc/intel/avs/messages.c @@ -562,6 +562,7 @@ int avs_ipc_get_fw_config(struct avs_dev *adev, struct avs_fw_cfg *cfg) case AVS_FW_CFG_DMA_BUFFER_CONFIG: case AVS_FW_CFG_SCHEDULER_CONFIG: case AVS_FW_CFG_CLOCKS_CONFIG: + case AVS_FW_CFG_RESERVED: break; default: From 4b38bd16ca6d8b16c1dc2cc4aa61663193b0b893 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:51 +0200 Subject: [PATCH 10/12] ASoC: Intel: avs: Replace hardcodes with SD_CTL_STREAM_RESET Improve readability of CLDMA reset operation by making use of already defined SD_CTL_STREAM_RESET. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-11-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/cldma.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sound/soc/intel/avs/cldma.c b/sound/soc/intel/avs/cldma.c index d100c6ba4d8a..d7a9390b5e48 100644 --- a/sound/soc/intel/avs/cldma.c +++ b/sound/soc/intel/avs/cldma.c @@ -176,17 +176,17 @@ int hda_cldma_reset(struct hda_cldma *cl) return ret; } - snd_hdac_stream_updateb(cl, SD_CTL, 1, 1); - ret = snd_hdac_stream_readb_poll(cl, SD_CTL, reg, (reg & 1), AVS_CL_OP_INTERVAL_US, - AVS_CL_OP_TIMEOUT_US); + snd_hdac_stream_updateb(cl, SD_CTL, SD_CTL_STREAM_RESET, SD_CTL_STREAM_RESET); + ret = snd_hdac_stream_readb_poll(cl, SD_CTL, reg, (reg & SD_CTL_STREAM_RESET), + AVS_CL_OP_INTERVAL_US, AVS_CL_OP_TIMEOUT_US); if (ret < 0) { dev_err(cl->dev, "cldma set SRST failed: %d\n", ret); return ret; } - snd_hdac_stream_updateb(cl, SD_CTL, 1, 0); - ret = snd_hdac_stream_readb_poll(cl, SD_CTL, reg, !(reg & 1), AVS_CL_OP_INTERVAL_US, - AVS_CL_OP_TIMEOUT_US); + snd_hdac_stream_updateb(cl, SD_CTL, SD_CTL_STREAM_RESET, 0); + ret = snd_hdac_stream_readb_poll(cl, SD_CTL, reg, !(reg & SD_CTL_STREAM_RESET), + AVS_CL_OP_INTERVAL_US, AVS_CL_OP_TIMEOUT_US); if (ret < 0) { dev_err(cl->dev, "cldma unset SRST failed: %d\n", ret); return ret; From 8758ae88f0f4ade16e6a1b709eb5ea7271f62320 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:52 +0200 Subject: [PATCH 11/12] ASoC: Intel: avs: Lower UNLOAD_MULTIPLE_MODULES IPC timeout Module unloading operation performs memory unmapping and the weight of the opration does not different from any other standard IPC. There is no dependency on secondary task like in module loading scenario where larger message timeout is recommended. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-12-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- sound/soc/intel/avs/messages.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/intel/avs/messages.c b/sound/soc/intel/avs/messages.c index 28a948cf790f..d4bcee1aabcf 100644 --- a/sound/soc/intel/avs/messages.c +++ b/sound/soc/intel/avs/messages.c @@ -59,7 +59,7 @@ int avs_ipc_unload_modules(struct avs_dev *adev, u16 *mod_ids, u32 num_mod_ids) request.data = mod_ids; request.size = sizeof(*mod_ids) * num_mod_ids; - ret = avs_dsp_send_msg_timeout(adev, &request, NULL, AVS_CL_TIMEOUT_MS); + ret = avs_dsp_send_msg(adev, &request, NULL); if (ret) avs_ipc_err(adev, &request, "unload multiple modules", ret); From f1eea11523e4394d6670f10a51356e9b7c8567a8 Mon Sep 17 00:00:00 2001 From: Cezary Rojewski Date: Thu, 7 Jul 2022 14:41:53 +0200 Subject: [PATCH 12/12] ASoC: Intel: avs: Update AVS_FW_INIT_TIMEOUT_US declaration To reduce the number of places to update if timeouts would have to change, modify the constant declaration. Signed-off-by: Cezary Rojewski Link: https://lore.kernel.org/r/20220707124153.1858249-13-cezary.rojewski@intel.com Signed-off-by: Mark Brown --- 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 542fd44aa501..9e3f8ff33a87 100644 --- a/sound/soc/intel/avs/loader.c +++ b/sound/soc/intel/avs/loader.c @@ -27,8 +27,8 @@ #define APL_ROM_INIT_RETRIES 3 #define AVS_FW_INIT_POLLING_US 500 -#define AVS_FW_INIT_TIMEOUT_US 3000000 #define AVS_FW_INIT_TIMEOUT_MS 3000 +#define AVS_FW_INIT_TIMEOUT_US (AVS_FW_INIT_TIMEOUT_MS * 1000) #define AVS_CLDMA_START_DELAY_MS 100