[PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups for 6.10

[PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups for 6.10

[Cezary Rojewski]

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(-)

-- 
2.25.1

[PATCH 01/13] ASoC: Intel: avs: Restore stream decoupling on prepare

[Cezary Rojewski]

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);
-- 
2.25.1

[PATCH 02/13] ASoC: Intel: avs: Fix debug-slot offset calculation

[Cezary Rojewski]

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;
 }
 
-- 
2.25.1

[PATCH 03/13] ASoC: Intel: avs: Silence false-positive memcpy() warnings

[Cezary Rojewski]

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;
 
-- 
2.25.1

[PATCH 04/13] ASoC: Intel: avs: Fix config_length for config-less copiers

[Cezary Rojewski]

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;
 
-- 
2.25.1

[PATCH 05/13] ASoC: Intel: avs: Fix ASRC module initialization

[Cezary Rojewski]

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;
-- 
2.25.1

[PATCH 06/13] ASoC: Intel: avs: Replace risky functions with safer variants

[Cezary Rojewski]

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;
-- 
2.25.1

[PATCH 07/13] ASoC: Intel: avs: Fix potential integer overflow

[Cezary Rojewski]

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;
-- 
2.25.1

[PATCH 08/13] ASoC: Intel: avs: Test result of avs_get_module_entry()

[Cezary Rojewski]

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)
-- 
2.25.1

[PATCH 09/13] ASoC: Intel: avs: Remove dead code

[Cezary Rojewski]

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);
-- 
2.25.1

[PATCH 10/13] ASoC: Intel: avs: Wake from D0ix when starting streaming

[Cezary Rojewski]

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;
 }
-- 
2.25.1

[PATCH 11/13] ASoC: Intel: avs: Init debugfs before booting firmware

[Cezary Rojewski]

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);
-- 
2.25.1

[PATCH 12/13] ASoC: Intel: avs: Add assert_static to guarantee ABI sizes

[Cezary Rojewski]

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);
-- 
2.25.1

[PATCH 13/13] ASoC: Intel: avs: Rule invalid buffer and period sizes out

[Cezary Rojewski]

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",
-- 
2.25.1

Re: [PATCH 00/13] ASoC: Intel: avs: Fixes and cleanups for 6.10

[Mark Brown]

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