Skip to content

Commit fd688ce

Browse files
brooniebroonie
committed
ASoC: Intel: avs: Fixes and cleanups
Merge series from Cezary Rojewski <cezary.rojewski@intel.com>: 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. The only patch that points to 'new functionality' is: ASoC: Intel: avs: Update ASRC definition as with the struct definition updates, one can utilize the ASRC module in both streaming directions now (previously limited to Capture). Everything else either improves the logging or provides comments vital for long-term maintenance of the code.
2 parents 82a0a3e + 0b12850 commit fd688ce

11 files changed

Lines changed: 97 additions & 65 deletions

File tree

‎sound/soc/intel/avs/apl.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ int avs_apl_coredump(struct avs_dev *adev, union avs_notify_msg *msg)
125125
struct avs_apl_log_buffer_layout layout;
126126
void __iomem *addr, *buf;
127127
size_t dump_size;
128-
u16 offset = 0;
128+
u32 offset = 0;
129129
u8 *dump, *pos;
130130

131131
dump_size = AVS_FW_REGS_SIZE + msg->ext.coredump.stack_dump_size;

‎sound/soc/intel/avs/core.c‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -829,22 +829,22 @@ static const struct avs_spec jsl_desc = {
829829
.hipc = &cnl_hipc_spec,
830830
};
831831

832-
#define AVS_TGL_BASED_SPEC(sname) \
832+
#define AVS_TGL_BASED_SPEC(sname, min) \
833833
static const struct avs_spec sname##_desc = { \
834834
.name = #sname, \
835-
.min_fw_version = { 10, 29, 0, 5646 }, \
835+
.min_fw_version = { 10, min, 0, 5646 }, \
836836
.dsp_ops = &avs_tgl_dsp_ops, \
837837
.core_init_mask = 1, \
838838
.attributes = AVS_PLATATTR_IMR, \
839839
.sram = &apl_sram_spec, \
840840
.hipc = &cnl_hipc_spec, \
841841
}
842842

843-
AVS_TGL_BASED_SPEC(lkf);
844-
AVS_TGL_BASED_SPEC(tgl);
845-
AVS_TGL_BASED_SPEC(ehl);
846-
AVS_TGL_BASED_SPEC(adl);
847-
AVS_TGL_BASED_SPEC(adl_n);
843+
AVS_TGL_BASED_SPEC(lkf, 28);
844+
AVS_TGL_BASED_SPEC(tgl, 29);
845+
AVS_TGL_BASED_SPEC(ehl, 30);
846+
AVS_TGL_BASED_SPEC(adl, 35);
847+
AVS_TGL_BASED_SPEC(adl_n, 35);
848848

849849
static const struct pci_device_id avs_ids[] = {
850850
{ PCI_DEVICE_DATA(INTEL, HDA_SKL_LP, &skl_desc) },
@@ -902,3 +902,13 @@ MODULE_AUTHOR("Cezary Rojewski <cezary.rojewski@intel.com>");
902902
MODULE_AUTHOR("Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>");
903903
MODULE_DESCRIPTION("Intel cAVS sound driver");
904904
MODULE_LICENSE("GPL");
905+
MODULE_FIRMWARE("intel/skl/dsp_basefw.bin");
906+
MODULE_FIRMWARE("intel/apl/dsp_basefw.bin");
907+
MODULE_FIRMWARE("intel/cnl/dsp_basefw.bin");
908+
MODULE_FIRMWARE("intel/icl/dsp_basefw.bin");
909+
MODULE_FIRMWARE("intel/jsl/dsp_basefw.bin");
910+
MODULE_FIRMWARE("intel/lkf/dsp_basefw.bin");
911+
MODULE_FIRMWARE("intel/tgl/dsp_basefw.bin");
912+
MODULE_FIRMWARE("intel/ehl/dsp_basefw.bin");
913+
MODULE_FIRMWARE("intel/adl/dsp_basefw.bin");
914+
MODULE_FIRMWARE("intel/adl_n/dsp_basefw.bin");

‎sound/soc/intel/avs/debugfs.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <linux/kfifo.h>
1111
#include <linux/wait.h>
1212
#include <linux/sched/signal.h>
13+
#include <linux/string_helpers.h>
1314
#include <sound/soc.h>
1415
#include "avs.h"
1516
#include "messages.h"

‎sound/soc/intel/avs/ipc.c‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,11 @@ static void avs_dsp_receive_rx(struct avs_dev *adev, u64 header)
184184
{
185185
struct avs_ipc *ipc = adev->ipc;
186186
union avs_reply_msg msg = AVS_MSG(header);
187-
u64 reg;
187+
u32 sts, lec;
188188

189-
reg = readq(avs_sram_addr(adev, AVS_FW_REGS_WINDOW));
190-
trace_avs_ipc_reply_msg(header, reg);
189+
sts = snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev));
190+
lec = snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev));
191+
trace_avs_ipc_reply_msg(header, sts, lec);
191192

192193
ipc->rx.header = header;
193194
/* Abort copying payload if request processing was unsuccessful. */
@@ -209,10 +210,11 @@ static void avs_dsp_process_notification(struct avs_dev *adev, u64 header)
209210
union avs_notify_msg msg = AVS_MSG(header);
210211
size_t data_size = 0;
211212
void *data = NULL;
212-
u64 reg;
213+
u32 sts, lec;
213214

214-
reg = readq(avs_sram_addr(adev, AVS_FW_REGS_WINDOW));
215-
trace_avs_ipc_notify_msg(header, reg);
215+
sts = snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev));
216+
lec = snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev));
217+
trace_avs_ipc_notify_msg(header, sts, lec);
216218

217219
/* Ignore spurious notifications until handshake is established. */
218220
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)
367369
static void avs_dsp_send_tx(struct avs_dev *adev, struct avs_ipc_msg *tx, bool read_fwregs)
368370
{
369371
const struct avs_spec *const spec = adev->spec;
370-
u64 reg = ULONG_MAX;
372+
u32 sts = UINT_MAX;
373+
u32 lec = UINT_MAX;
371374

372375
tx->header |= spec->hipc->req_busy_mask;
373-
if (read_fwregs)
374-
reg = readq(avs_sram_addr(adev, AVS_FW_REGS_WINDOW));
376+
if (read_fwregs) {
377+
sts = snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev));
378+
lec = snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev));
379+
}
375380

376-
trace_avs_request(tx, reg);
381+
trace_avs_request(tx, sts, lec);
377382

378383
if (tx->size)
379384
memcpy_toio(avs_downlink_addr(adev), tx->data, tx->size);

‎sound/soc/intel/avs/loader.c‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ int avs_cldma_load_basefw(struct avs_dev *adev, struct firmware *fw)
167167
(reg & AVS_ROM_INIT_DONE) == AVS_ROM_INIT_DONE,
168168
AVS_ROM_INIT_POLLING_US, SKL_ROM_INIT_TIMEOUT_US);
169169
if (ret < 0) {
170-
dev_err(adev->dev, "rom init timeout: %d\n", ret);
170+
dev_err(adev->dev, "rom init failed: %d, status: 0x%08x, lec: 0x%08x\n",
171+
ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
171172
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
172173
return ret;
173174
}
@@ -180,7 +181,8 @@ int avs_cldma_load_basefw(struct avs_dev *adev, struct firmware *fw)
180181
AVS_FW_INIT_POLLING_US, AVS_FW_INIT_TIMEOUT_US);
181182
hda_cldma_stop(cl);
182183
if (ret < 0) {
183-
dev_err(adev->dev, "transfer fw failed: %d\n", ret);
184+
dev_err(adev->dev, "transfer fw failed: %d, status: 0x%08x, lec: 0x%08x\n",
185+
ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
184186
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
185187
return ret;
186188
}
@@ -308,12 +310,13 @@ avs_hda_init_rom(struct avs_dev *adev, unsigned int dma_id, bool purge)
308310
}
309311

310312
/* await ROM init */
311-
ret = snd_hdac_adsp_readq_poll(adev, spec->sram->rom_status_offset, reg,
313+
ret = snd_hdac_adsp_readl_poll(adev, spec->sram->rom_status_offset, reg,
312314
(reg & 0xF) == AVS_ROM_INIT_DONE ||
313315
(reg & 0xF) == APL_ROM_FW_ENTERED,
314316
AVS_ROM_INIT_POLLING_US, APL_ROM_INIT_TIMEOUT_US);
315317
if (ret < 0) {
316-
dev_err(adev->dev, "rom init timeout: %d\n", ret);
318+
dev_err(adev->dev, "rom init failed: %d, status: 0x%08x, lec: 0x%08x\n",
319+
ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
317320
goto err;
318321
}
319322

@@ -337,15 +340,15 @@ static int avs_imr_load_basefw(struct avs_dev *adev)
337340

338341
/* DMA id ignored when flashing from IMR as no transfer occurs. */
339342
ret = avs_hda_init_rom(adev, 0, false);
340-
if (ret < 0) {
341-
dev_err(adev->dev, "rom init failed: %d\n", ret);
343+
if (ret < 0)
342344
return ret;
343-
}
344345

345346
ret = wait_for_completion_timeout(&adev->fw_ready,
346347
msecs_to_jiffies(AVS_FW_INIT_TIMEOUT_MS));
347348
if (!ret) {
348-
dev_err(adev->dev, "firmware ready timeout\n");
349+
dev_err(adev->dev, "firmware ready timeout, status: 0x%08x, lec: 0x%08x\n",
350+
snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev)),
351+
snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
349352
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
350353
return -ETIMEDOUT;
351354
}
@@ -392,7 +395,7 @@ int avs_hda_load_basefw(struct avs_dev *adev, struct firmware *fw)
392395
ret = avs_hda_init_rom(adev, dma_id, true);
393396
if (!ret)
394397
break;
395-
dev_info(adev->dev, "#%d rom init fail: %d\n", i + 1, ret);
398+
dev_info(adev->dev, "#%d rom init failed: %d\n", i + 1, ret);
396399
}
397400
if (ret < 0)
398401
goto cleanup_resources;
@@ -404,7 +407,8 @@ int avs_hda_load_basefw(struct avs_dev *adev, struct firmware *fw)
404407
AVS_FW_INIT_POLLING_US, AVS_FW_INIT_TIMEOUT_US);
405408
snd_hdac_dsp_trigger(hstream, false);
406409
if (ret < 0) {
407-
dev_err(adev->dev, "transfer fw failed: %d\n", ret);
410+
dev_err(adev->dev, "transfer fw failed: %d, status: 0x%08x, lec: 0x%08x\n",
411+
ret, reg, snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
408412
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
409413
}
410414

@@ -584,7 +588,9 @@ static int avs_dsp_load_basefw(struct avs_dev *adev)
584588
ret = wait_for_completion_timeout(&adev->fw_ready,
585589
msecs_to_jiffies(AVS_FW_INIT_TIMEOUT_MS));
586590
if (!ret) {
587-
dev_err(adev->dev, "firmware ready timeout\n");
591+
dev_err(adev->dev, "firmware ready timeout, status: 0x%08x, lec: 0x%08x\n",
592+
snd_hdac_adsp_readl(adev, AVS_FW_REG_STATUS(adev)),
593+
snd_hdac_adsp_readl(adev, AVS_FW_REG_ERROR(adev)));
588594
avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
589595
ret = -ETIMEDOUT;
590596
goto release_fw;
@@ -675,16 +681,12 @@ int avs_dsp_first_boot_firmware(struct avs_dev *adev)
675681
}
676682

677683
ret = avs_ipc_get_hw_config(adev, &adev->hw_cfg);
678-
if (ret) {
679-
dev_err(adev->dev, "get hw cfg failed: %d\n", ret);
684+
if (ret)
680685
return AVS_IPC_RET(ret);
681-
}
682686

683687
ret = avs_ipc_get_fw_config(adev, &adev->fw_cfg);
684-
if (ret) {
685-
dev_err(adev->dev, "get fw cfg failed: %d\n", ret);
688+
if (ret)
686689
return AVS_IPC_RET(ret);
687-
}
688690

689691
adev->core_refs = devm_kcalloc(adev->dev, adev->hw_cfg.dsp_cores,
690692
sizeof(*adev->core_refs), GFP_KERNEL);

‎sound/soc/intel/avs/messages.c‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,12 @@ int avs_ipc_get_fw_config(struct avs_dev *adev, struct avs_fw_cfg *cfg)
400400
AVS_BASEFW_FIRMWARE_CONFIG, NULL, 0,
401401
&payload, &payload_size);
402402
if (ret)
403-
return ret;
403+
goto err;
404404
/* Non-zero payload expected for FIRMWARE_CONFIG. */
405-
if (!payload_size)
406-
return -EREMOTEIO;
405+
if (!payload_size) {
406+
ret = -EREMOTEIO;
407+
goto err;
408+
}
407409

408410
while (offset < payload_size) {
409411
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)
502504

503505
/* No longer needed, free it as it's owned by the get_large_config() caller. */
504506
kfree(payload);
507+
err:
508+
if (ret)
509+
dev_err(adev->dev, "get fw cfg failed: %d\n", ret);
505510
return ret;
506511
}
507512

@@ -517,10 +522,12 @@ int avs_ipc_get_hw_config(struct avs_dev *adev, struct avs_hw_cfg *cfg)
517522
AVS_BASEFW_HARDWARE_CONFIG, NULL, 0,
518523
&payload, &payload_size);
519524
if (ret)
520-
return ret;
525+
goto err;
521526
/* Non-zero payload expected for HARDWARE_CONFIG. */
522-
if (!payload_size)
523-
return -EREMOTEIO;
527+
if (!payload_size) {
528+
ret = -EREMOTEIO;
529+
goto err;
530+
}
524531

525532
while (offset < payload_size) {
526533
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)
590597
exit:
591598
/* No longer needed, free it as it's owned by the get_large_config() caller. */
592599
kfree(payload);
600+
err:
601+
if (ret)
602+
dev_err(adev->dev, "get hw cfg failed: %d\n", ret);
593603
return ret;
594604
}
595605

‎sound/soc/intel/avs/messages.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -859,8 +859,7 @@ static_assert(sizeof(struct avs_aec_cfg) == 92);
859859
struct avs_asrc_cfg {
860860
struct avs_modcfg_base base;
861861
u32 out_freq;
862-
u32 rsvd0:1;
863-
u32 mode:1;
862+
u32 mode:2;
864863
u32 rsvd2:2;
865864
u32 disable_jitter_buffer:1;
866865
u32 rsvd3:27;

‎sound/soc/intel/avs/pcm.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ static int avs_dai_be_hw_params(struct snd_pcm_substream *substream,
161161
struct snd_soc_dpcm *dpcm;
162162

163163
be = snd_soc_substream_to_rtd(substream);
164+
/* dpcm_fe_dai_open() guarantees the list is not empty at this point. */
164165
for_each_dpcm_fe(be, substream->stream, dpcm) {
165166
fe = dpcm->fe;
166167
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,
576577
hdac_stream(host_stream)->format_val = 0;
577578

578579
fe = snd_soc_substream_to_rtd(substream);
580+
/* dpcm_fe_dai_open() guarantees the list is not empty at this point. */
579581
for_each_dpcm_be(fe, substream->stream, dpcm) {
580582
be = dpcm->be;
581583
be_hw_params = &be->dpcm[substream->stream].hw_params;
@@ -1564,6 +1566,7 @@ static int avs_component_hda_probe(struct snd_soc_component *component)
15641566
if (ret < 0) {
15651567
dev_err(component->dev, "create widgets failed: %d\n",
15661568
ret);
1569+
snd_soc_unregister_dai(dai);
15671570
goto exit;
15681571
}
15691572
}
@@ -1578,8 +1581,8 @@ static int avs_component_hda_probe(struct snd_soc_component *component)
15781581

15791582
static void avs_component_hda_remove(struct snd_soc_component *component)
15801583
{
1581-
avs_component_hda_unregister_dais(component);
15821584
avs_component_remove(component);
1585+
avs_component_hda_unregister_dais(component);
15831586
}
15841587

15851588
static int avs_component_hda_open(struct snd_soc_component *component,

‎sound/soc/intel/avs/registers.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
/* Constants used when accessing SRAM, space shared with firmware */
7575
#define AVS_FW_REG_BASE(adev) ((adev)->spec->sram->base_offset)
7676
#define AVS_FW_REG_STATUS(adev) (AVS_FW_REG_BASE(adev) + 0x0)
77-
#define AVS_FW_REG_ERROR_CODE(adev) (AVS_FW_REG_BASE(adev) + 0x4)
77+
#define AVS_FW_REG_ERROR(adev) (AVS_FW_REG_BASE(adev) + 0x4)
7878

7979
#define AVS_WINDOW_CHUNK_SIZE SZ_4K
8080
#define AVS_FW_REGS_SIZE AVS_WINDOW_CHUNK_SIZE

‎sound/soc/intel/avs/topology.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1466,7 +1466,7 @@ avs_tplg_path_template_create(struct snd_soc_component *comp, struct avs_tplg *o
14661466

14671467
static const struct avs_tplg_token_parser mod_init_config_parsers[] = {
14681468
{
1469-
.token = AVS_TKN_MOD_INIT_CONFIG_ID_U32,
1469+
.token = AVS_TKN_INIT_CONFIG_ID_U32,
14701470
.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
14711471
.offset = offsetof(struct avs_tplg_init_config, id),
14721472
.parse = avs_parse_word_token,
@@ -1519,7 +1519,7 @@ static int avs_tplg_parse_initial_configs(struct snd_soc_component *comp,
15191519
esize = le32_to_cpu(tuples->size) + le32_to_cpu(tmp->size);
15201520

15211521
ret = parse_dictionary_entries(comp, tuples, esize, config, 1, sizeof(*config),
1522-
AVS_TKN_MOD_INIT_CONFIG_ID_U32,
1522+
AVS_TKN_INIT_CONFIG_ID_U32,
15231523
mod_init_config_parsers,
15241524
ARRAY_SIZE(mod_init_config_parsers));
15251525

0 commit comments

Comments
 (0)