[PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups for 6.10
Set of changes targeting the avs-driver only. No new features, patchset either fixes or fortifies existing code.
Patchset starts off with a fix for debugbility on ICL+ platforms which I have forgotten to fixup when providing support for these initially. The next two address copier module initialization, most importantly, silence the gcc 'field-spanning write' false-positive.
The following four: 6/13 ASoC: Intel: avs: Replace risky functions with safer variants 7/13 ASoC: Intel: avs: Fix potential integer overflow 8/13 ASoC: Intel: avs: Test result of avs_get_module_entry() 9/13 ASoC: Intel: avs: Remove dead code
address problems found out by Coverity static analysis tool.
The last two worth mentioning are: recommendation from the firmware team to wake subsystem from D0ix when starting any pipeline -and- shielding against invalid period/buffer sizes. Audio format shall be taken into consideration when calculating either of these.
Amadeusz Sławiński (2): ASoC: Intel: avs: Restore stream decoupling on prepare ASoC: Intel: avs: Add assert_static to guarantee ABI sizes
Cezary Rojewski (11): ASoC: Intel: avs: Fix debug-slot offset calculation ASoC: Intel: avs: Silence false-positive memcpy() warnings ASoC: Intel: avs: Fix config_length for config-less copiers ASoC: Intel: avs: Fix ASRC module initialization ASoC: Intel: avs: Replace risky functions with safer variants ASoC: Intel: avs: Fix potential integer overflow ASoC: Intel: avs: Test result of avs_get_module_entry() ASoC: Intel: avs: Remove dead code ASoC: Intel: avs: Wake from D0ix when starting streaming ASoC: Intel: avs: Init debugfs before booting firmware ASoC: Intel: avs: Rule invalid buffer and period sizes out
sound/soc/intel/avs/avs.h | 1 + sound/soc/intel/avs/cldma.c | 2 +- sound/soc/intel/avs/core.c | 4 +-- sound/soc/intel/avs/icl.c | 12 ++++++--- sound/soc/intel/avs/loader.c | 6 +++-- sound/soc/intel/avs/messages.h | 47 ++++++++++++++++++++++++++++++++-- sound/soc/intel/avs/path.c | 13 ++++------ sound/soc/intel/avs/pcm.c | 34 +++++++++++++++++++++++- sound/soc/intel/avs/probes.c | 14 ++++++---- 9 files changed, 109 insertions(+), 24 deletions(-)
From: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com
Revert changes from commit b87b8f43afd5 ("ASoC: Intel: avs: Drop superfluous stream decoupling") to restore working streaming during S3.
Fixes: b87b8f43afd5 ("ASoC: Intel: avs: Drop superfluous stream decoupling") 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/pcm.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c index 2cafbc392cdb..72f1bc3b7b1f 100644 --- a/sound/soc/intel/avs/pcm.c +++ b/sound/soc/intel/avs/pcm.c @@ -356,6 +356,7 @@ static int avs_dai_hda_be_prepare(struct snd_pcm_substream *substream, struct sn stream_info->sig_bits); format_val = snd_hdac_stream_format(runtime->channels, bits, runtime->rate);
+ snd_hdac_ext_stream_decouple(bus, link_stream, true); snd_hdac_ext_stream_reset(link_stream); snd_hdac_ext_stream_setup(link_stream, format_val);
@@ -611,6 +612,7 @@ static int avs_dai_fe_prepare(struct snd_pcm_substream *substream, struct snd_so struct avs_dev *adev = to_avs_dev(dai->dev); struct hdac_ext_stream *host_stream; unsigned int format_val; + struct hdac_bus *bus; unsigned int bits; int ret;
@@ -620,6 +622,8 @@ static int avs_dai_fe_prepare(struct snd_pcm_substream *substream, struct snd_so if (hdac_stream(host_stream)->prepared) return 0;
+ bus = hdac_stream(host_stream)->bus; + snd_hdac_ext_stream_decouple(bus, data->host_stream, true); snd_hdac_stream_reset(hdac_stream(host_stream));
stream_info = snd_soc_dai_get_pcm_stream(dai, substream->stream);
For resources with ID other than 0 the current calculus is incorrect.
Fixes: 275b583d047a ("ASoC: Intel: avs: ICL-based platforms support") Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/icl.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/avs/icl.c b/sound/soc/intel/avs/icl.c index 9d9921e1cd4d..3e0716160f5a 100644 --- a/sound/soc/intel/avs/icl.c +++ b/sound/soc/intel/avs/icl.c @@ -66,7 +66,7 @@ struct avs_icl_memwnd2 { struct avs_icl_memwnd2_desc slot_desc[AVS_ICL_MEMWND2_SLOTS_COUNT]; u8 rsvd[PAGE_SIZE]; }; - u8 slot_array[AVS_ICL_MEMWND2_SLOTS_COUNT][PAGE_SIZE]; + u8 slot_array[AVS_ICL_MEMWND2_SLOTS_COUNT][SZ_4K]; } __packed;
#define AVS_ICL_SLOT_UNUSED \ @@ -89,8 +89,7 @@ static int avs_icl_slot_offset(struct avs_dev *adev, union avs_icl_memwnd2_slot_
for (i = 0; i < AVS_ICL_MEMWND2_SLOTS_COUNT; i++) if (desc[i].slot_id.val == slot_type.val) - return offsetof(struct avs_icl_memwnd2, slot_array) + - avs_skl_log_buffer_offset(adev, i); + return offsetof(struct avs_icl_memwnd2, slot_array) + i * SZ_4K; return -ENXIO; }
Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") enforced strict flex array declarations. This generates false-positive in form of: "memcpy: detected field-spanning write". Avoid it by utilizing the DECLARE_FLEX_ARRAY() macro.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/messages.h | 4 ++-- sound/soc/intel/avs/path.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/avs/messages.h b/sound/soc/intel/avs/messages.h index 4e609a08863c..007bc4fb6d99 100644 --- a/sound/soc/intel/avs/messages.h +++ b/sound/soc/intel/avs/messages.h @@ -752,9 +752,9 @@ struct avs_copier_gtw_cfg { union avs_connector_node_id node_id; u32 dma_buffer_size; u32 config_length; - struct { + union { union avs_gtw_attributes attrs; - u32 blob[]; + DECLARE_FLEX_ARRAY(u32, blob); } config; } __packed;
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index e785fc2a7008..5944865a1193 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -254,7 +254,7 @@ static int avs_copier_create(struct avs_dev *adev, struct avs_path_module *mod) /* config_length in DWORDs */ cfg->gtw_cfg.config_length = DIV_ROUND_UP(data_size, 4); if (data) - memcpy(&cfg->gtw_cfg.config, data, data_size); + memcpy(&cfg->gtw_cfg.config.blob, data, data_size);
mod->gtw_attrs = cfg->gtw_cfg.config.attrs;
Copier's config_length shall always be at least one even if there is no configuration payload to carry. While the firmware treats config_length=0 or 1 in the same manner, the driver shall initialize the module properly.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/path.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index 5944865a1193..9bd03a44fcb9 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -148,11 +148,12 @@ static int avs_copier_create(struct avs_dev *adev, struct avs_path_module *mod) struct avs_copier_cfg *cfg; struct nhlt_specific_cfg *ep_blob; union avs_connector_node_id node_id = {0}; - size_t cfg_size, data_size = 0; + size_t cfg_size, data_size; void *data = NULL; u32 dma_type; int ret;
+ data_size = sizeof(cfg->gtw_cfg.config); dma_type = t->cfg_ext->copier.dma_type; node_id.dma_type = dma_type;
@@ -233,10 +234,7 @@ static int avs_copier_create(struct avs_dev *adev, struct avs_path_module *mod) break; }
- cfg_size = sizeof(*cfg) + data_size; - /* Every config-BLOB contains gateway attributes. */ - if (data_size) - cfg_size -= sizeof(cfg->gtw_cfg.config.attrs); + cfg_size = offsetof(struct avs_copier_cfg, gtw_cfg.config) + data_size; if (cfg_size > AVS_MAILBOX_SIZE) return -EINVAL;
The ASRC module configuration consists of several reserved fields. Zero them out when initializing the module to avoid sending invalid data.
Fixes: 274d79e51875 ("ASoC: Intel: avs: Configure modules according to their type") Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/path.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index 9bd03a44fcb9..8dfd90587427 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -365,6 +365,7 @@ static int avs_asrc_create(struct avs_dev *adev, struct avs_path_module *mod) struct avs_tplg_module *t = mod->template; struct avs_asrc_cfg cfg;
+ memset(&cfg, 0, sizeof(cfg)); cfg.base.cpc = t->cfg_base->cpc; cfg.base.ibs = t->cfg_base->ibs; cfg.base.obs = t->cfg_base->obs;
strscpy() and snprintf() are the recommended equivalents of their riskier friends.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/loader.c | 4 ++-- sound/soc/intel/avs/pcm.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c index 8e34d3536082..57370f0905dc 100644 --- a/sound/soc/intel/avs/loader.c +++ b/sound/soc/intel/avs/loader.c @@ -535,7 +535,7 @@ int avs_dsp_load_libraries(struct avs_dev *adev, struct avs_tplg_library *libs, if (ret) return ret;
- strncpy(adev->lib_names[id], man->name, AVS_LIB_NAME_SIZE); + strscpy(adev->lib_names[id], man->name, AVS_LIB_NAME_SIZE); id++; next_lib: i++; @@ -698,7 +698,7 @@ int avs_dsp_first_boot_firmware(struct avs_dev *adev) }
/* basefw always occupies slot 0 */ - strcpy(&adev->lib_names[0][0], "BASEFW"); + strscpy(adev->lib_names[0], "BASEFW", AVS_LIB_NAME_SIZE);
ida_init(&adev->ppl_ida);
diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c index 72f1bc3b7b1f..405de1d58178 100644 --- a/sound/soc/intel/avs/pcm.c +++ b/sound/soc/intel/avs/pcm.c @@ -1420,7 +1420,7 @@ static void avs_component_hda_unregister_dais(struct snd_soc_component *componen
mach = dev_get_platdata(component->card->dev); codec = mach->pdata; - sprintf(name, "%s-cpu", dev_name(&codec->core.dev)); + snprintf(name, sizeof(name), "%s-cpu", dev_name(&codec->core.dev));
for_each_component_dais_safe(component, dai, save) { int stream;
While stream_tag for CLDMA on SKL-based platforms is always 1, function hda_cldma_setup() uses AZX_SD_CTL_STRM() macro which does: stream_tag << 20
what combined with stream_tag type of 'unsigned int' generates a potential overflow issue. Update the field type to fix that.
Fixes: 45864e49a05a ("ASoC: Intel: avs: Implement CLDMA transfer") Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/cldma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/intel/avs/cldma.c b/sound/soc/intel/avs/cldma.c index d7a9390b5e48..585579840b64 100644 --- a/sound/soc/intel/avs/cldma.c +++ b/sound/soc/intel/avs/cldma.c @@ -35,7 +35,7 @@ struct hda_cldma {
unsigned int buffer_size; unsigned int num_periods; - unsigned int stream_tag; + unsigned char stream_tag; void __iomem *sd_addr;
struct snd_dma_buffer dmab_data;
While PROBE_MOD_UUID is always part of the base AudioDSP firmware manifest, from maintenance point of view it is better to check the result.
Fixes: dab8d000e25c ("ASoC: Intel: avs: Add data probing requests") Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/probes.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/avs/probes.c b/sound/soc/intel/avs/probes.c index 817e543036f2..7e781a315690 100644 --- a/sound/soc/intel/avs/probes.c +++ b/sound/soc/intel/avs/probes.c @@ -19,8 +19,11 @@ static int avs_dsp_init_probe(struct avs_dev *adev, union avs_connector_node_id struct avs_probe_cfg cfg = {{0}}; struct avs_module_entry mentry; u8 dummy; + int ret;
- avs_get_module_entry(adev, &AVS_PROBE_MOD_UUID, &mentry); + ret = avs_get_module_entry(adev, &AVS_PROBE_MOD_UUID, &mentry); + if (ret) + return ret;
/* * Probe module uses no cycles, audio data format and input and output @@ -39,11 +42,12 @@ static int avs_dsp_init_probe(struct avs_dev *adev, union avs_connector_node_id static void avs_dsp_delete_probe(struct avs_dev *adev) { struct avs_module_entry mentry; + int ret;
- avs_get_module_entry(adev, &AVS_PROBE_MOD_UUID, &mentry); - - /* There is only ever one probe module instance. */ - avs_dsp_delete_module(adev, mentry.module_id, 0, INVALID_PIPELINE_ID, 0); + ret = avs_get_module_entry(adev, &AVS_PROBE_MOD_UUID, &mentry); + if (!ret) + /* There is only ever one probe module instance. */ + avs_dsp_delete_module(adev, mentry.module_id, 0, INVALID_PIPELINE_ID, 0); }
static inline struct hdac_ext_stream *avs_compr_get_host_stream(struct snd_compr_stream *cstream)
The result of list_next_entry()/list_last_entry() is never null.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/path.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index 8dfd90587427..fa3fec339548 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -709,8 +709,6 @@ static int avs_path_pipeline_arm(struct avs_dev *adev, /* bind current module to next module on list */ source = mod; sink = list_next_entry(mod, node); - if (!source || !sink) - return -EINVAL;
ret = avs_ipc_bind(adev, source->module_id, source->instance_id, sink->module_id, sink->instance_id, 0, 0);
It is recommended to keep the DSP domain in full-power when starting DMA engines.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/icl.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/intel/avs/icl.c b/sound/soc/intel/avs/icl.c index 3e0716160f5a..d279ec1e0ad0 100644 --- a/sound/soc/intel/avs/icl.c +++ b/sound/soc/intel/avs/icl.c @@ -109,6 +109,10 @@ int avs_icl_log_buffer_offset(struct avs_dev *adev, u32 core)
bool avs_icl_d0ix_toggle(struct avs_dev *adev, struct avs_ipc_msg *tx, bool wake) { + /* Full-power when starting DMA engines. */ + if (tx->glb.set_ppl_state.state == AVS_PPL_STATE_RUNNING) + return true; + /* Payload-less IPCs do not take part in d0ix toggling. */ return tx->size; }
When bringing up setups it's vital to have access to debug functionality even if firmware boot fails. As order of probe()ing operations is changed, update remove() procedure accordingly.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c index d7f8940099ce..76782a0f32bc 100644 --- a/sound/soc/intel/avs/core.c +++ b/sound/soc/intel/avs/core.c @@ -209,6 +209,7 @@ static void avs_hda_probe_work(struct work_struct *work)
snd_hdac_ext_bus_ppcap_enable(bus, true); snd_hdac_ext_bus_ppcap_int_enable(bus, true); + avs_debugfs_init(adev);
ret = avs_dsp_first_boot_firmware(adev); if (ret < 0) @@ -217,7 +218,6 @@ static void avs_hda_probe_work(struct work_struct *work) adev->nhlt = intel_nhlt_init(adev->dev); if (!adev->nhlt) dev_info(bus->dev, "platform has no NHLT\n"); - avs_debugfs_init(adev);
avs_register_all_boards(adev);
@@ -548,9 +548,9 @@ static void avs_pci_remove(struct pci_dev *pci)
avs_unregister_all_boards(adev);
- avs_debugfs_exit(adev); if (adev->nhlt) intel_nhlt_free(adev->nhlt); + avs_debugfs_exit(adev);
if (avs_platattr_test(adev, CLDMA)) hda_cldma_free(&code_loader);
From: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com
In order to make sure that IPC interface is stable use assert_static to check union and struct sizes that describe communication interface.
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/avs.h | 1 + sound/soc/intel/avs/icl.c | 3 +++ sound/soc/intel/avs/loader.c | 2 ++ sound/soc/intel/avs/messages.h | 43 ++++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+)
diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h index f80f79415344..39bdec591d58 100644 --- a/sound/soc/intel/avs/avs.h +++ b/sound/soc/intel/avs/avs.h @@ -381,6 +381,7 @@ struct avs_apl_log_buffer_layout { u32 write_ptr; u8 buffer[]; } __packed; +static_assert(sizeof(struct avs_apl_log_buffer_layout) == 8);
#define avs_apl_log_payload_size(adev) \ (avs_log_buffer_size(adev) - sizeof(struct avs_apl_log_buffer_layout)) diff --git a/sound/soc/intel/avs/icl.c b/sound/soc/intel/avs/icl.c index d279ec1e0ad0..e8b4983e03e9 100644 --- a/sound/soc/intel/avs/icl.c +++ b/sound/soc/intel/avs/icl.c @@ -52,12 +52,14 @@ union avs_icl_memwnd2_slot_type { u32 type:24; }; } __packed; +static_assert(sizeof(union avs_icl_memwnd2_slot_type) == 4);
struct avs_icl_memwnd2_desc { u32 resource_id; union avs_icl_memwnd2_slot_type slot_id; u32 vma; } __packed; +static_assert(sizeof(struct avs_icl_memwnd2_desc) == 12);
#define AVS_ICL_MEMWND2_SLOTS_COUNT 15
@@ -68,6 +70,7 @@ struct avs_icl_memwnd2 { }; u8 slot_array[AVS_ICL_MEMWND2_SLOTS_COUNT][SZ_4K]; } __packed; +static_assert(sizeof(struct avs_icl_memwnd2) == 65536);
#define AVS_ICL_SLOT_UNUSED \ ((union avs_icl_memwnd2_slot_type) { 0x00000000U }) diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c index 57370f0905dc..c255c898b7a8 100644 --- a/sound/soc/intel/avs/loader.c +++ b/sound/soc/intel/avs/loader.c @@ -56,6 +56,7 @@ struct avs_fw_manifest { u32 feature_mask; struct avs_fw_version version; } __packed; +static_assert(sizeof(struct avs_fw_manifest) == 36);
struct avs_fw_ext_manifest { u32 id; @@ -64,6 +65,7 @@ struct avs_fw_ext_manifest { u16 version_minor; u32 entries; } __packed; +static_assert(sizeof(struct avs_fw_ext_manifest) == 16);
static int avs_fw_ext_manifest_strip(struct firmware *fw) { diff --git a/sound/soc/intel/avs/messages.h b/sound/soc/intel/avs/messages.h index 007bc4fb6d99..285d89607b6a 100644 --- a/sound/soc/intel/avs/messages.h +++ b/sound/soc/intel/avs/messages.h @@ -93,12 +93,14 @@ union avs_global_msg { } ext; }; } __packed; +static_assert(sizeof(union avs_global_msg) == 8);
struct avs_tlv { u32 type; u32 length; u32 value[]; } __packed; +static_assert(sizeof(struct avs_tlv) == 8);
enum avs_module_msg_type { AVS_MOD_INIT_INSTANCE = 0, @@ -155,6 +157,7 @@ union avs_module_msg { } ext; }; } __packed; +static_assert(sizeof(union avs_module_msg) == 8);
#define AVS_IPC_NOT_SUPPORTED 15
@@ -190,6 +193,7 @@ union avs_reply_msg { } ext; }; } __packed; +static_assert(sizeof(union avs_reply_msg) == 8);
enum avs_notify_msg_type { AVS_NOTIFY_PHRASE_DETECTED = 4, @@ -226,6 +230,7 @@ union avs_notify_msg { } ext; }; } __packed; +static_assert(sizeof(union avs_notify_msg) == 8);
#define AVS_MSG(hdr) { .val = hdr }
@@ -264,6 +269,7 @@ struct avs_notify_voice_data { u16 kpd_score; u16 reserved; } __packed; +static_assert(sizeof(struct avs_notify_voice_data) == 4);
struct avs_notify_res_data { u32 resource_type; @@ -272,6 +278,7 @@ struct avs_notify_res_data { u32 reserved; u32 data[6]; } __packed; +static_assert(sizeof(struct avs_notify_res_data) == 40);
struct avs_notify_mod_data { u32 module_instance_id; @@ -279,6 +286,7 @@ struct avs_notify_mod_data { u32 data_size; u32 data[]; } __packed; +static_assert(sizeof(struct avs_notify_mod_data) == 12);
/* ROM messages */ enum avs_rom_control_msg_type { @@ -332,6 +340,7 @@ struct avs_dxstate_info { u32 core_mask; /* which cores are subject for power transition */ u32 dx_mask; /* bit[n]=1 core n goes to D0, bit[n]=0 it goes to D3 */ } __packed; +static_assert(sizeof(struct avs_dxstate_info) == 8);
int avs_ipc_set_dx(struct avs_dev *adev, u32 core_mask, bool powerup); int avs_ipc_set_d0ix(struct avs_dev *adev, bool enable_pg, bool streaming); @@ -367,11 +376,13 @@ struct avs_skl_log_state { u32 enable; u32 min_priority; } __packed; +static_assert(sizeof(struct avs_skl_log_state) == 8);
struct avs_skl_log_state_info { u32 core_mask; struct avs_skl_log_state logs_core[]; } __packed; +static_assert(sizeof(struct avs_skl_log_state_info) == 4);
struct avs_apl_log_state_info { u32 aging_timer_period; @@ -379,6 +390,7 @@ struct avs_apl_log_state_info { u32 core_mask; struct avs_skl_log_state logs_core[]; } __packed; +static_assert(sizeof(struct avs_apl_log_state_info) == 12);
enum avs_icl_log_priority { AVS_ICL_LOG_CRITICAL = 0, @@ -403,6 +415,7 @@ struct avs_icl_log_state_info { u32 enable; u32 logs_priorities_mask[]; } __packed; +static_assert(sizeof(struct avs_icl_log_state_info) == 12);
int avs_ipc_set_enable_logs(struct avs_dev *adev, u8 *log_info, size_t size);
@@ -521,6 +534,7 @@ struct avs_module_type { u32 lib_code:1; u32 rsvd:24; } __packed; +static_assert(sizeof(struct avs_module_type) == 4);
union avs_segment_flags { u32 ul; @@ -537,12 +551,14 @@ union avs_segment_flags { u32 length:16; }; } __packed; +static_assert(sizeof(union avs_segment_flags) == 4);
struct avs_segment_desc { union avs_segment_flags flags; u32 v_base_addr; u32 file_offset; } __packed; +static_assert(sizeof(struct avs_segment_desc) == 12);
struct avs_module_entry { u16 module_id; @@ -559,11 +575,13 @@ struct avs_module_entry { u16 instance_bss_size; struct avs_segment_desc segments[3]; } __packed; +static_assert(sizeof(struct avs_module_entry) == 116);
struct avs_mods_info { u32 count; struct avs_module_entry entries[]; } __packed; +static_assert(sizeof(struct avs_mods_info) == 4);
static inline bool avs_module_entry_is_loaded(struct avs_module_entry *mentry) { @@ -577,6 +595,7 @@ struct avs_sys_time { u32 val_l; u32 val_u; } __packed; +static_assert(sizeof(struct avs_sys_time) == 8);
int avs_ipc_set_system_time(struct avs_dev *adev);
@@ -680,6 +699,7 @@ struct avs_audio_format { u32 sample_type:8; u32 reserved:8; } __packed; +static_assert(sizeof(struct avs_audio_format) == 24);
struct avs_modcfg_base { u32 cpc; @@ -688,12 +708,14 @@ struct avs_modcfg_base { u32 is_pages; struct avs_audio_format audio_fmt; } __packed; +static_assert(sizeof(struct avs_modcfg_base) == 40);
struct avs_pin_format { u32 pin_index; u32 iobs; struct avs_audio_format audio_fmt; } __packed; +static_assert(sizeof(struct avs_pin_format) == 32);
struct avs_modcfg_ext { struct avs_modcfg_base base; @@ -703,6 +725,7 @@ struct avs_modcfg_ext { /* input pin formats followed by output ones */ struct avs_pin_format pin_fmts[]; } __packed; +static_assert(sizeof(struct avs_modcfg_ext) == 56);
enum avs_dma_type { AVS_DMA_HDA_HOST_OUTPUT = 0, @@ -726,6 +749,7 @@ union avs_virtual_index { u8 instance:3; } dmic; } __packed; +static_assert(sizeof(union avs_virtual_index) == 1);
union avs_connector_node_id { u32 val; @@ -735,6 +759,7 @@ union avs_connector_node_id { u32 rsvd:19; }; } __packed; +static_assert(sizeof(union avs_connector_node_id) == 4);
#define INVALID_PIPELINE_ID 0xFF #define INVALID_NODE_ID \ @@ -747,6 +772,7 @@ union avs_gtw_attributes { u32 rsvd:31; }; } __packed; +static_assert(sizeof(union avs_gtw_attributes) == 4);
struct avs_copier_gtw_cfg { union avs_connector_node_id node_id; @@ -757,6 +783,7 @@ struct avs_copier_gtw_cfg { DECLARE_FLEX_ARRAY(u32, blob); } config; } __packed; +static_assert(sizeof(struct avs_copier_gtw_cfg) == 16);
struct avs_copier_cfg { struct avs_modcfg_base base; @@ -764,6 +791,7 @@ struct avs_copier_cfg { u32 feature_mask; struct avs_copier_gtw_cfg gtw_cfg; } __packed; +static_assert(sizeof(struct avs_copier_cfg) == 84);
struct avs_volume_cfg { u32 channel_id; @@ -772,22 +800,26 @@ struct avs_volume_cfg { u32 reserved; /* alignment */ u64 curve_duration; } __packed; +static_assert(sizeof(struct avs_volume_cfg) == 24);
struct avs_peakvol_cfg { struct avs_modcfg_base base; struct avs_volume_cfg vols[]; } __packed; +static_assert(sizeof(struct avs_peakvol_cfg) == 40);
struct avs_micsel_cfg { struct avs_modcfg_base base; struct avs_audio_format out_fmt; } __packed; +static_assert(sizeof(struct avs_micsel_cfg) == 64);
struct avs_mux_cfg { struct avs_modcfg_base base; struct avs_audio_format ref_fmt; struct avs_audio_format out_fmt; } __packed; +static_assert(sizeof(struct avs_mux_cfg) == 88);
struct avs_updown_mixer_cfg { struct avs_modcfg_base base; @@ -796,21 +828,25 @@ struct avs_updown_mixer_cfg { s32 coefficients[AVS_CHANNELS_MAX]; u32 channel_map; } __packed; +static_assert(sizeof(struct avs_updown_mixer_cfg) == 84);
struct avs_src_cfg { struct avs_modcfg_base base; u32 out_freq; } __packed; +static_assert(sizeof(struct avs_src_cfg) == 44);
struct avs_probe_gtw_cfg { union avs_connector_node_id node_id; u32 dma_buffer_size; } __packed; +static_assert(sizeof(struct avs_probe_gtw_cfg) == 8);
struct avs_probe_cfg { struct avs_modcfg_base base; struct avs_probe_gtw_cfg gtw_cfg; } __packed; +static_assert(sizeof(struct avs_probe_cfg) == 48);
struct avs_aec_cfg { struct avs_modcfg_base base; @@ -818,6 +854,7 @@ struct avs_aec_cfg { struct avs_audio_format out_fmt; u32 cpc_lp_mode; } __packed; +static_assert(sizeof(struct avs_aec_cfg) == 92);
struct avs_asrc_cfg { struct avs_modcfg_base base; @@ -828,11 +865,13 @@ struct avs_asrc_cfg { u32 disable_jitter_buffer:1; u32 rsvd3:27; } __packed; +static_assert(sizeof(struct avs_asrc_cfg) == 48);
struct avs_wov_cfg { struct avs_modcfg_base base; u32 cpc_lp_mode; } __packed; +static_assert(sizeof(struct avs_wov_cfg) == 44);
/* Module runtime parameters */
@@ -845,6 +884,7 @@ struct avs_copier_sink_format { struct avs_audio_format src_fmt; struct avs_audio_format sink_fmt; } __packed; +static_assert(sizeof(struct avs_copier_sink_format) == 52);
int avs_ipc_copier_set_sink_format(struct avs_dev *adev, u16 module_id, u8 instance_id, u32 sink_id, @@ -878,6 +918,7 @@ struct avs_probe_dma { union avs_connector_node_id node_id; u32 dma_buffer_size; } __packed; +static_assert(sizeof(struct avs_probe_dma) == 8);
enum avs_probe_type { AVS_PROBE_TYPE_INPUT = 0, @@ -894,6 +935,7 @@ union avs_probe_point_id { u32 index:6; } id; } __packed; +static_assert(sizeof(union avs_probe_point_id) == 4);
enum avs_connection_purpose { AVS_CONNECTION_PURPOSE_EXTRACT = 0, @@ -906,6 +948,7 @@ struct avs_probe_point_desc { u32 purpose; union avs_connector_node_id node_id; } __packed; +static_assert(sizeof(struct avs_probe_point_desc) == 12);
int avs_ipc_probe_get_dma(struct avs_dev *adev, struct avs_probe_dma **dmas, size_t *num_dmas); int avs_ipc_probe_attach_dma(struct avs_dev *adev, struct avs_probe_dma *dmas, size_t num_dmas);
While HDAudio controller supports buffer packets up to 128 bytes low, audio format shall be taken into consideration when calculating buffer and period sizes to avoid undesired xruns.
As *_size in ALSA terms means frames (channels times bit-depth-bytes), hw_rules can calculate minimal buffer and period sizes solely from sample rate and the number of milliseconds commonly used on the AudioDSP firmware side.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/pcm.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c index 405de1d58178..77a7e8f93951 100644 --- a/sound/soc/intel/avs/pcm.c +++ b/sound/soc/intel/avs/pcm.c @@ -457,6 +457,26 @@ static const struct snd_pcm_hw_constraint_list hw_rates = {
const struct snd_soc_dai_ops avs_dai_fe_ops;
+static int hw_rule_param_size(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) +{ + struct snd_interval *interval = hw_param_interval(params, rule->var); + struct snd_interval to; + + snd_interval_any(&to); + to.integer = interval->integer; + to.max = interval->max; + /* + * Commonly 2ms buffer size is used in HDA scenarios whereas 4ms is used + * when streaming through GPDMA. Align to the latter to account for both. + */ + to.min = params_rate(params) / 1000 * 4; + + if (rule->var == SNDRV_PCM_HW_PARAM_PERIOD_SIZE) + to.min /= params_periods(params); + + return snd_interval_refine(interval, &to); +} + static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -492,6 +512,14 @@ static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_so if (ret < 0) goto err;
+ /* Adjust buffer and period size based on the audio format. */ + snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, hw_rule_param_size, NULL, + SNDRV_PCM_HW_PARAM_FORMAT, SNDRV_PCM_HW_PARAM_CHANNELS, + SNDRV_PCM_HW_PARAM_RATE, -1); + snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, hw_rule_param_size, NULL, + SNDRV_PCM_HW_PARAM_FORMAT, SNDRV_PCM_HW_PARAM_CHANNELS, + SNDRV_PCM_HW_PARAM_RATE, -1); + snd_pcm_set_sync(substream);
dev_dbg(dai->dev, "%s fe STARTUP tag %d str %p",
On Fri, 05 Apr 2024 11:09:16 +0200, Cezary Rojewski wrote:
Set of changes targeting the avs-driver only. No new features, patchset either fixes or fortifies existing code.
Patchset starts off with a fix for debugbility on ICL+ platforms which I have forgotten to fixup when providing support for these initially. The next two address copier module initialization, most importantly, silence the gcc 'field-spanning write' false-positive.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[01/13] ASoC: Intel: avs: Restore stream decoupling on prepare commit: 680507581e025d16a0b6d3782603ca8c598fbe2b [02/13] ASoC: Intel: avs: Fix debug-slot offset calculation commit: c91b692781c1839fcc389b2a9120e46593c6424b [03/13] ASoC: Intel: avs: Silence false-positive memcpy() warnings commit: 6dd68c2da44d92c25b323bcc2603421463437a64 [04/13] ASoC: Intel: avs: Fix config_length for config-less copiers commit: beeeee9686affef32ee04d8ae30db8c53cbc7aee [05/13] ASoC: Intel: avs: Fix ASRC module initialization commit: 9d2e26f31c7cc3fa495c423af9b4902ec0dc7be3 [06/13] ASoC: Intel: avs: Replace risky functions with safer variants commit: 47714847592b574ff594ceca7ebe0ada70dbac3c [07/13] ASoC: Intel: avs: Fix potential integer overflow commit: c7e832cabe635df47c2bf6df7801e97bf3045b1e [08/13] ASoC: Intel: avs: Test result of avs_get_module_entry() commit: 41bf4525fadb3d8df3860420d6ac9025c51a3bac [09/13] ASoC: Intel: avs: Remove dead code commit: d58275f474b4a27b4e97839ffe8d9fe55c0cc40a [10/13] ASoC: Intel: avs: Wake from D0ix when starting streaming commit: 30df76bbcb59254ce646477e3e05f00021a10117 [11/13] ASoC: Intel: avs: Init debugfs before booting firmware commit: ff0aefe2d217ce6fec6487b225737b2019eb88c0 [12/13] ASoC: Intel: avs: Add assert_static to guarantee ABI sizes commit: c2b10acb62c195db2c976d614d9d8092ad6339ae [13/13] ASoC: Intel: avs: Rule invalid buffer and period sizes out commit: 9a385993504e47a0fd6fd34b5384827b4abdee60
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
participants (2)
-
Cezary Rojewski -
Mark Brown