[PATCH 0/5] ASoc: SOF: misc updates for 6.10

[PATCH 0/5] ASoc: SOF: misc updates for 6.10

[Pierre-Louis Bossart]

Fixes when fw_lib_prefix is not set, updated error messages, improved
dmesg logs to SoundWire configurations not supported by ACPI
tables/topology and support for IEC61937 passthrough.

Peter Ujfalusi (2):
  ASoC: SOF: debug: Handle cases when fw_lib_prefix is not set, NULL
  ASoC: SOF: ipc4-topology: Advertise passthrough capable PCMs (using
    ChainDMA)

Pierre-Louis Bossart (2):
  ASoC: SOF: Intel: hda: simplify and reduce indentation for
    hda_sdw_machine_select()
  ASoC: SOF: Intel: hda: list SoundWire peripherals on mismatch

Yong Zhi (1):
  ASoC: SOF: topology: remove incorrect widget id in error message

 sound/soc/sof/debug.c         |  23 ++++-
 sound/soc/sof/intel/hda.c     | 154 +++++++++++++++++++---------------
 sound/soc/sof/ipc4-topology.c |  50 +++++++++++
 sound/soc/sof/topology.c      |   7 +-
 4 files changed, 160 insertions(+), 74 deletions(-)

-- 
2.40.1

[PATCH 1/5] ASoC: SOF: debug: Handle cases when fw_lib_prefix is not set, NULL

[Pierre-Louis Bossart]

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

The firmware libraries are not supported by IPC3, the fw_lib_path is not
a valid parameter and it is always NULL.
Do not create the debugfs file for IPC3 at all as it is not applicable.

With IPC4 some vendors/platforms might not support loadable libraries and
the fw_lib_prefix is left to NULL to indicate this.
Handle such case with allocating "Not supported" string.

Reviewed-by: Marc Herbert <marc.herbert@intel.com>
Fixes: 17f4041244e6 ("ASoC: SOF: debug: show firmware/topology prefix/names")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/debug.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index cf282c8b18af..bf635cc96ede 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -326,8 +326,27 @@ int snd_sof_dbg_init(struct snd_sof_dev *sdev)
 
 	debugfs_create_str("fw_path", 0444, fw_profile,
 			   (char **)&plat_data->fw_filename_prefix);
-	debugfs_create_str("fw_lib_path", 0444, fw_profile,
-			   (char **)&plat_data->fw_lib_prefix);
+	/* library path is not valid for IPC3 */
+	if (plat_data->ipc_type != SOF_IPC_TYPE_3) {
+		/*
+		 * fw_lib_prefix can be NULL if the vendor/platform does not
+		 * support loadable libraries
+		 */
+		if (plat_data->fw_lib_prefix) {
+			debugfs_create_str("fw_lib_path", 0444, fw_profile,
+					   (char **)&plat_data->fw_lib_prefix);
+		} else {
+			static char *fw_lib_path;
+
+			fw_lib_path = devm_kasprintf(sdev->dev, GFP_KERNEL,
+						     "Not supported");
+			if (!fw_lib_path)
+				return -ENOMEM;
+
+			debugfs_create_str("fw_lib_path", 0444, fw_profile,
+					   (char **)&fw_lib_path);
+		}
+	}
 	debugfs_create_str("tplg_path", 0444, fw_profile,
 			   (char **)&plat_data->tplg_filename_prefix);
 	debugfs_create_str("fw_name", 0444, fw_profile,
-- 
2.40.1

[PATCH 2/5] ASoC: SOF: topology: remove incorrect widget id in error message

[Pierre-Louis Bossart]

From: Yong Zhi <yong.zhi@intel.com>

In sof_widget_ready() function, the shift field of
struct snd_soc_tplg_dapm_widget is incorrectly used to print
widget id in dev_err(scomp->dev, "error: failed to add widget id %d ..",
this patch removes the useless tw->shift from the error output.

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Yong Zhi <yong.zhi@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/topology.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index bcdb499c96a0..ec931a26b54f 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -1531,10 +1531,9 @@ static int sof_widget_ready(struct snd_soc_component *scomp, int index,
 	/* check token parsing reply */
 	if (ret < 0) {
 		dev_err(scomp->dev,
-			"error: failed to add widget id %d type %d name : %s stream %s\n",
-			tw->shift, swidget->id, tw->name,
-			strnlen(tw->sname, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) > 0
-				? tw->sname : "none");
+			"failed to add widget type %d name : %s stream %s\n",
+			swidget->id, tw->name, strnlen(tw->sname, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) > 0
+							? tw->sname : "none");
 		goto widget_free;
 	}
 
-- 
2.40.1

[PATCH 3/5] ASoC: SOF: Intel: hda: simplify and reduce indentation for hda_sdw_machine_select()

[Pierre-Louis Bossart]

Simplify code to return when no links are enabled. No functional
change, just code cleanup before updates.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda.c | 139 +++++++++++++++++++-------------------
 1 file changed, 71 insertions(+), 68 deletions(-)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 4d0fe706ebc1..8ddc1b0ca3c9 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -1653,92 +1653,95 @@ static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev
 	hdev = pdata->hw_pdata;
 	link_mask = hdev->info.link_mask;
 
+	if (!link_mask) {
+		dev_info(sdev->dev, "SoundWire links not enabled\n");
+		return NULL;
+	}
+
 	/*
 	 * Select SoundWire machine driver if needed using the
 	 * alternate tables. This case deals with SoundWire-only
 	 * machines, for mixed cases with I2C/I2S the detection relies
 	 * on the HID list.
 	 */
-	if (link_mask) {
-		for (mach = pdata->desc->alt_machines;
-		     mach && mach->link_mask; mach++) {
-			/*
-			 * On some platforms such as Up Extreme all links
-			 * are enabled but only one link can be used by
-			 * external codec. Instead of exact match of two masks,
-			 * first check whether link_mask of mach is subset of
-			 * link_mask supported by hw and then go on searching
-			 * link_adr
-			 */
-			if (~link_mask & mach->link_mask)
-				continue;
+	for (mach = pdata->desc->alt_machines;
+	     mach && mach->link_mask; mach++) {
+		/*
+		 * On some platforms such as Up Extreme all links
+		 * are enabled but only one link can be used by
+		 * external codec. Instead of exact match of two masks,
+		 * first check whether link_mask of mach is subset of
+		 * link_mask supported by hw and then go on searching
+		 * link_adr
+		 */
+		if (~link_mask & mach->link_mask)
+			continue;
 
-			/* No need to match adr if there is no links defined */
-			if (!mach->links)
-				break;
+		/* No need to match adr if there is no links defined */
+		if (!mach->links)
+			break;
 
-			link = mach->links;
-			for (i = 0; i < hdev->info.count && link->num_adr;
-			     i++, link++) {
-				/*
-				 * Try next machine if any expected Slaves
-				 * are not found on this link.
-				 */
-				if (!snd_soc_acpi_sdw_link_slaves_found(sdev->dev, link,
-									hdev->sdw->ids,
-									hdev->sdw->num_slaves))
-					break;
-			}
-			/* Found if all Slaves are checked */
-			if (i == hdev->info.count || !link->num_adr)
+		link = mach->links;
+		for (i = 0; i < hdev->info.count && link->num_adr;
+		     i++, link++) {
+			/*
+			 * Try next machine if any expected Slaves
+			 * are not found on this link.
+			 */
+			if (!snd_soc_acpi_sdw_link_slaves_found(sdev->dev, link,
+								hdev->sdw->ids,
+								hdev->sdw->num_slaves))
 				break;
 		}
-		if (mach && mach->link_mask) {
-			int dmic_num = 0;
-			bool tplg_fixup;
-			const char *tplg_filename;
-
-			mach->mach_params.links = mach->links;
-			mach->mach_params.link_mask = mach->link_mask;
-			mach->mach_params.platform = dev_name(sdev->dev);
-
-			if (pdata->tplg_filename) {
-				tplg_fixup = false;
-			} else {
-				tplg_fixup = true;
-				tplg_filename = mach->sof_tplg_filename;
-			}
+		/* Found if all Slaves are checked */
+		if (i == hdev->info.count || !link->num_adr)
+			break;
+	}
+	if (mach && mach->link_mask) {
+		int dmic_num = 0;
+		bool tplg_fixup;
+		const char *tplg_filename;
 
-			/*
-			 * DMICs use up to 4 pins and are typically pin-muxed with SoundWire
-			 * link 2 and 3, or link 1 and 2, thus we only try to enable dmics
-			 * if all conditions are true:
-			 * a) 2 or fewer links are used by SoundWire
-			 * b) the NHLT table reports the presence of microphones
-			 */
-			if (hweight_long(mach->link_mask) <= 2) {
-				int ret;
+		mach->mach_params.links = mach->links;
+		mach->mach_params.link_mask = mach->link_mask;
+		mach->mach_params.platform = dev_name(sdev->dev);
 
-				ret = dmic_detect_topology_fixup(sdev, &tplg_filename, "",
-								 &dmic_num, tplg_fixup);
-				if (ret < 0)
-					return NULL;
-			}
-			if (tplg_fixup)
-				pdata->tplg_filename = tplg_filename;
-			mach->mach_params.dmic_num = dmic_num;
+		if (pdata->tplg_filename) {
+			tplg_fixup = false;
+		} else {
+			tplg_fixup = true;
+			tplg_filename = mach->sof_tplg_filename;
+		}
 
-			dev_dbg(sdev->dev,
-				"SoundWire machine driver %s topology %s\n",
-				mach->drv_name,
-				pdata->tplg_filename);
+		/*
+		 * DMICs use up to 4 pins and are typically pin-muxed with SoundWire
+		 * link 2 and 3, or link 1 and 2, thus we only try to enable dmics
+		 * if all conditions are true:
+		 * a) 2 or fewer links are used by SoundWire
+		 * b) the NHLT table reports the presence of microphones
+		 */
+		if (hweight_long(mach->link_mask) <= 2) {
+			int ret;
 
-			return mach;
+			ret = dmic_detect_topology_fixup(sdev, &tplg_filename, "",
+							 &dmic_num, tplg_fixup);
+			if (ret < 0)
+				return NULL;
 		}
+		if (tplg_fixup)
+			pdata->tplg_filename = tplg_filename;
+		mach->mach_params.dmic_num = dmic_num;
+
+		dev_dbg(sdev->dev,
+			"SoundWire machine driver %s topology %s\n",
+			mach->drv_name,
+			pdata->tplg_filename);
 
-		dev_info(sdev->dev, "No SoundWire machine driver found\n");
+		return mach;
 	}
 
+	dev_info(sdev->dev, "No SoundWire machine driver found\n");
+
 	return NULL;
 }
 #else
-- 
2.40.1

[PATCH 4/5] ASoC: SOF: Intel: hda: list SoundWire peripherals on mismatch

[Pierre-Louis Bossart]

Most of the SoundWire support issues come from bad ACPI information,
or configuration reported by ACPI that are not supported by the SOF
driver/topology. The users see a "No SoundWire machine driver found"
message without any details, and the fallback to HDaudio w/ HDMI is
used.

We can reduce our support load with a clear dev_info() log that will
give us a clear hint on the mismatch and why a machine driver/topology
were not found.

Example log on a MTL device:
[   13.158599] sof-audio-pci-intel-mtl 0000:00:1f.3: No SoundWire machine driver found for the ACPI-reported configuration:
[   13.158603] sof-audio-pci-intel-mtl 0000:00:1f.3: link 0 mfg_id 0x025d part_id 0x0713 version 0x3
[   13.158606] sof-audio-pci-intel-mtl 0000:00:1f.3: link 1 mfg_id 0x025d part_id 0x1316 version 0x3
[   13.158608] sof-audio-pci-intel-mtl 0000:00:1f.3: link 2 mfg_id 0x025d part_id 0x1316 version 0x3

In parallel, we will also provide an update to `alsa-info` to log all
SoundWire peripherals found in ACPI tables.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 8ddc1b0ca3c9..8ac8c10b83bd 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -1645,6 +1645,7 @@ static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev
 {
 	struct snd_sof_pdata *pdata = sdev->pdata;
 	const struct snd_soc_acpi_link_adr *link;
+	struct sdw_extended_slave_id *ids;
 	struct snd_soc_acpi_mach *mach;
 	struct sof_intel_hda_dev *hdev;
 	u32 link_mask;
@@ -1658,6 +1659,16 @@ static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev
 		return NULL;
 	}
 
+	if (!hdev->sdw) {
+		dev_dbg(sdev->dev, "SoundWire context not allocated\n");
+		return NULL;
+	}
+
+	if (!hdev->sdw->num_slaves) {
+		dev_warn(sdev->dev, "No SoundWire peripheral detected in ACPI tables\n");
+		return NULL;
+	}
+
 	/*
 	 * Select SoundWire machine driver if needed using the
 	 * alternate tables. This case deals with SoundWire-only
@@ -1740,7 +1751,11 @@ static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev
 		return mach;
 	}
 
-	dev_info(sdev->dev, "No SoundWire machine driver found\n");
+	dev_info(sdev->dev, "No SoundWire machine driver found for the ACPI-reported configuration:\n");
+	ids = hdev->sdw->ids;
+	for (i = 0; i < hdev->sdw->num_slaves; i++)
+		dev_info(sdev->dev, "link %d mfg_id 0x%04x part_id 0x%04x version %#x\n",
+			 ids[i].link_id, ids[i].id.mfg_id, ids[i].id.part_id, ids[i].id.sdw_version);
 
 	return NULL;
 }
-- 
2.40.1

[PATCH 5/5] ASoC: SOF: ipc4-topology: Advertise passthrough capable PCMs (using ChainDMA)

[Pierre-Louis Bossart]

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

If a PCM is set to use ChainDMA then add it to the card->components string
after a marker of iec61937-pcm:, for example on current HDA platforms where
HDMI is set to use ChainDMA:
iec61937-pcm:5,4,3 (the order of the PCM ids can differ)

UCM is expected to parse and use this property to allow applications to
use bytestream passthrough in a standard way.

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/ipc4-topology.c | 50 +++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index c29c1de4e925..a972b2b5122a 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -407,6 +407,52 @@ static void sof_ipc4_widget_update_kcontrol_module_id(struct snd_sof_widget *swi
 	}
 }
 
+static int
+sof_ipc4_update_card_components_string(struct snd_sof_widget *swidget,
+				       struct snd_sof_pcm *spcm, int dir)
+{
+	struct snd_sof_widget *pipe_widget = swidget->spipe->pipe_widget;
+	struct sof_ipc4_pipeline *pipeline = pipe_widget->private;
+	struct snd_soc_component *scomp = spcm->scomp;
+	struct snd_soc_card *card = scomp->card;
+	const char *pt_marker = "iec61937-pcm";
+
+	/*
+	 * Update the card's components list with iec61937-pcm and a list of PCM
+	 * ids where ChainDMA is enabled.
+	 * These PCMs can be used for bytestream passthrough.
+	 */
+	if (!pipeline->use_chain_dma)
+		return 0;
+
+	if (card->components) {
+		const char *tmp = card->components;
+
+		if (strstr(card->components, pt_marker))
+			card->components = devm_kasprintf(card->dev, GFP_KERNEL,
+							  "%s,%d",
+							  card->components,
+							  spcm->pcm.pcm_id);
+		else
+			card->components = devm_kasprintf(card->dev, GFP_KERNEL,
+							  "%s %s:%d",
+							  card->components,
+							  pt_marker,
+							  spcm->pcm.pcm_id);
+
+		devm_kfree(card->dev, tmp);
+	} else {
+		card->components = devm_kasprintf(card->dev, GFP_KERNEL,
+						  "%s:%d", pt_marker,
+						  spcm->pcm.pcm_id);
+	}
+
+	if (!card->components)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int sof_ipc4_widget_setup_pcm(struct snd_sof_widget *swidget)
 {
 	struct sof_ipc4_available_audio_format *available_fmt;
@@ -452,6 +498,10 @@ static int sof_ipc4_widget_setup_pcm(struct snd_sof_widget *swidget)
 	if (!spcm)
 		goto skip_gtw_cfg;
 
+	ret = sof_ipc4_update_card_components_string(swidget, spcm, dir);
+	if (ret)
+		goto free_available_fmt;
+
 	if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
 		struct snd_sof_pcm_stream *sps = &spcm->stream[dir];
 
-- 
2.40.1

Re: [PATCH 0/5] ASoc: SOF: misc updates for 6.10

[Mark Brown]

On Fri, 26 Apr 2024 10:38:57 -0500, Pierre-Louis Bossart wrote:
> Fixes when fw_lib_prefix is not set, updated error messages, improved
> dmesg logs to SoundWire configurations not supported by ACPI
> tables/topology and support for IEC61937 passthrough.
> 
> Peter Ujfalusi (2):
>   ASoC: SOF: debug: Handle cases when fw_lib_prefix is not set, NULL
>   ASoC: SOF: ipc4-topology: Advertise passthrough capable PCMs (using
>     ChainDMA)
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] ASoC: SOF: debug: Handle cases when fw_lib_prefix is not set, NULL
      commit: b32487ca7b51ce430f15ec785269f11c25a6a560
[2/5] ASoC: SOF: topology: remove incorrect widget id in error message
      commit: 490e6c9b81e0c22087d250246717aee26ac5002e
[3/5] ASoC: SOF: Intel: hda: simplify and reduce indentation for hda_sdw_machine_select()
      commit: 5d4788b2647fabeaeeaf331e53451e0ed6241252
[4/5] ASoC: SOF: Intel: hda: list SoundWire peripherals on mismatch
      commit: 7ff01ca730f240811c13d9c3f8f8030211b3c911
[5/5] ASoC: SOF: ipc4-topology: Advertise passthrough capable PCMs (using ChainDMA)
      commit: eed4872a4220f30de37aeca695d2881630410b66

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