[PATCH 0/3] ALSA/ASoC/SoundWire: Intel: update maximum number of links

[PATCH 0/3] ALSA/ASoC/SoundWire: Intel: update maximum number of links

[Bard Liao]

Intel new platforms can have up to 5 SoundWire links.
This series does not apply to SoundWire tree due to recent changes in
machine driver. Can we go via ASoC tree with Vinod's Acked-by tag?

Pierre-Louis Bossart (3):
  ALSA/ASoC/SoundWire: Intel: use single definition for
    SDW_INTEL_MAX_LINKS
  soundwire: intel: increase maximum number of links
  soundwire: intel: add probe-time check on link id

 drivers/soundwire/intel.h               |  7 +++++++
 drivers/soundwire/intel_ace2x.c         | 20 ++++++++++++++++++++
 drivers/soundwire/intel_auxdevice.c     | 14 ++++++++++++++
 include/linux/soundwire/sdw_intel.h     |  8 ++++++++
 sound/hda/intel-sdw-acpi.c              |  5 ++---
 sound/soc/intel/boards/sof_sdw.c        |  4 +++-
 sound/soc/intel/boards/sof_sdw_common.h |  4 +---
 sound/soc/intel/boards/sof_sdw_hdmi.c   |  2 ++
 8 files changed, 57 insertions(+), 7 deletions(-)


-- 
2.43.0

[PATCH 1/3] ALSA/ASoC/SoundWire: Intel: use single definition for SDW_INTEL_MAX_LINKS

[Bard Liao]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The definitions are currently duplicated in intel-sdw-acpi.c and
sof_sdw.c.  Move the definition to the sdw_intel.h header, and change
the prefix to make it Intel-specific.

No functionality change in this patch.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 include/linux/soundwire/sdw_intel.h     | 5 +++++
 sound/hda/intel-sdw-acpi.c              | 5 ++---
 sound/soc/intel/boards/sof_sdw.c        | 4 +++-
 sound/soc/intel/boards/sof_sdw_common.h | 4 +---
 sound/soc/intel/boards/sof_sdw_hdmi.c   | 2 ++
 5 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index d537587b4499..87d82ea9a13a 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -447,4 +447,9 @@ extern const struct sdw_intel_hw_ops sdw_intel_lnl_hw_ops;
 
 #define SDW_INTEL_DEV_NUM_IDA_MIN           6
 
+/*
+ * Max number of links supported in hardware
+ */
+#define SDW_INTEL_MAX_LINKS                4
+
 #endif
diff --git a/sound/hda/intel-sdw-acpi.c b/sound/hda/intel-sdw-acpi.c
index f3b2a610df23..04d6b6beabca 100644
--- a/sound/hda/intel-sdw-acpi.c
+++ b/sound/hda/intel-sdw-acpi.c
@@ -17,7 +17,6 @@
 #include <linux/string.h>
 
 #define SDW_LINK_TYPE		4 /* from Intel ACPI documentation */
-#define SDW_MAX_LINKS		4
 
 static int ctrl_link_mask;
 module_param_named(sdw_link_mask, ctrl_link_mask, int, 0444);
@@ -87,9 +86,9 @@ sdw_intel_scan_controller(struct sdw_intel_acpi_info *info)
 	}
 
 	/* Check count is within bounds */
-	if (count > SDW_MAX_LINKS) {
+	if (count > SDW_INTEL_MAX_LINKS) {
 		dev_err(&adev->dev, "Link count %d exceeds max %d\n",
-			count, SDW_MAX_LINKS);
+			count, SDW_INTEL_MAX_LINKS);
 		return -EINVAL;
 	}
 
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index d258728d64cf..b1fb6fabd3b7 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -5,12 +5,14 @@
  *  sof_sdw - ASOC Machine driver for Intel SoundWire platforms
  */
 
+#include <linux/acpi.h>
 #include <linux/bitmap.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/module.h>
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_type.h>
+#include <linux/soundwire/sdw_intel.h>
 #include <sound/soc-acpi.h>
 #include "sof_sdw_common.h"
 #include "../../codecs/rt711.h"
@@ -883,7 +885,7 @@ static int create_sdw_dailinks(struct snd_soc_card *card,
 	struct intel_mc_ctx *intel_ctx = (struct intel_mc_ctx *)ctx->private;
 	int ret, i;
 
-	for (i = 0; i < SDW_MAX_LINKS; i++)
+	for (i = 0; i < SDW_INTEL_MAX_LINKS; i++)
 		intel_ctx->sdw_pin_index[i] = SOC_SDW_INTEL_BIDIR_PDI_BASE;
 
 	/* generate DAI links by each sdw link */
diff --git a/sound/soc/intel/boards/sof_sdw_common.h b/sound/soc/intel/boards/sof_sdw_common.h
index 02f3eebd019d..c10d2711b730 100644
--- a/sound/soc/intel/boards/sof_sdw_common.h
+++ b/sound/soc/intel/boards/sof_sdw_common.h
@@ -19,8 +19,6 @@
 #define SOC_SDW_MAX_CPU_DAIS 16
 #define SOC_SDW_INTEL_BIDIR_PDI_BASE 2
 
-#define SDW_MAX_LINKS		4
-
 /* 8 combinations with 4 links + unused group 0 */
 #define SDW_MAX_GROUPS 9
 
@@ -57,7 +55,7 @@ enum {
 struct intel_mc_ctx {
 	struct sof_hdmi_private hdmi;
 	/* To store SDW Pin index for each SoundWire link */
-	unsigned int sdw_pin_index[SDW_MAX_LINKS];
+	unsigned int sdw_pin_index[SDW_INTEL_MAX_LINKS];
 };
 
 extern unsigned long sof_sdw_quirk;
diff --git a/sound/soc/intel/boards/sof_sdw_hdmi.c b/sound/soc/intel/boards/sof_sdw_hdmi.c
index 4084119a9a5f..f92867deb029 100644
--- a/sound/soc/intel/boards/sof_sdw_hdmi.c
+++ b/sound/soc/intel/boards/sof_sdw_hdmi.c
@@ -5,10 +5,12 @@
  *  sof_sdw_hdmi - Helpers to handle HDMI from generic machine driver
  */
 
+#include <linux/acpi.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
+#include <linux/soundwire/sdw_intel.h>
 #include <sound/soc.h>
 #include <sound/soc-acpi.h>
 #include <sound/jack.h>
-- 
2.43.0

Re: [PATCH 1/3] ALSA/ASoC/SoundWire: Intel: use single definition for SDW_INTEL_MAX_LINKS

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 406 bytes --]

On Fri, Aug 16, 2024 at 10:33:29AM +0800, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> The definitions are currently duplicated in intel-sdw-acpi.c and
> sof_sdw.c.  Move the definition to the sdw_intel.h header, and change
> the prefix to make it Intel-specific.
> 
> No functionality change in this patch.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] ALSA/ASoC/SoundWire: Intel: use single definition for SDW_INTEL_MAX_LINKS

[Takashi Iwai]

On Fri, 16 Aug 2024 13:25:45 +0200,
Mark Brown wrote:
> 
> On Fri, Aug 16, 2024 at 10:33:29AM +0800, Bard Liao wrote:
> > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > 
> > The definitions are currently duplicated in intel-sdw-acpi.c and
> > sof_sdw.c.  Move the definition to the sdw_intel.h header, and change
> > the prefix to make it Intel-specific.
> > 
> > No functionality change in this patch.
> 
> Acked-by: Mark Brown <broonie@kernel.org>

As Bard requested, the series can go via ASoC tree.

Feel free to take my ack for HDA part.

Reviewed-by: Takashi Iwai <tiwai@suse.de>


Takashi

[PATCH 2/3] soundwire: intel: increase maximum number of links

[Bard Liao]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Intel platforms have enabled 4 links since the beginning, newer
platforms now have 5 links. Update the definition accordingly.

This patch will have no effect on older platforms where the number of
links was hard-coded. A follow-up patch will add a dynamic check that
the ACPI-reported information is aligned with hardware capabilities on
newer platforms.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 include/linux/soundwire/sdw_intel.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index 87d82ea9a13a..edbe1d4af8f8 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -450,6 +450,6 @@ extern const struct sdw_intel_hw_ops sdw_intel_lnl_hw_ops;
 /*
  * Max number of links supported in hardware
  */
-#define SDW_INTEL_MAX_LINKS                4
+#define SDW_INTEL_MAX_LINKS                5
 
 #endif
-- 
2.43.0

Re: [PATCH 2/3] soundwire: intel: increase maximum number of links

[Vinod Koul]

On 16-08-24, 10:33, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> Intel platforms have enabled 4 links since the beginning, newer
> platforms now have 5 links. Update the definition accordingly.
> 
> This patch will have no effect on older platforms where the number of
> links was hard-coded. A follow-up patch will add a dynamic check that
> the ACPI-reported information is aligned with hardware capabilities on
> newer platforms.

Should this not be the third patch, the dynamic check in 3rd should come
before otherwise your older platforms might break?


Other than order lgtm:

Acked-by: Vinod Koul <vkoul@kernel.org>


> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> ---
>  include/linux/soundwire/sdw_intel.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
> index 87d82ea9a13a..edbe1d4af8f8 100644
> --- a/include/linux/soundwire/sdw_intel.h
> +++ b/include/linux/soundwire/sdw_intel.h
> @@ -450,6 +450,6 @@ extern const struct sdw_intel_hw_ops sdw_intel_lnl_hw_ops;
>  /*
>   * Max number of links supported in hardware
>   */
> -#define SDW_INTEL_MAX_LINKS                4
> +#define SDW_INTEL_MAX_LINKS                5
>  
>  #endif
> -- 
> 2.43.0

[PATCH 3/3] soundwire: intel: add probe-time check on link id

[Bard Liao]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

In older platforms, the number of links was constant and hard-coded to
4. Newer platforms can have varying number of links, so we need to add
a probe-time check to make sure the ACPI-reported information with
_DSD properties is aligned with hardware capabilities reported in the
SoundWire LCAP register.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.h           |  7 +++++++
 drivers/soundwire/intel_ace2x.c     | 20 ++++++++++++++++++++
 drivers/soundwire/intel_auxdevice.c | 14 ++++++++++++++
 include/linux/soundwire/sdw_intel.h |  3 +++
 4 files changed, 44 insertions(+)

diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 68838e843b54..1db4d9d3a3ba 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -222,6 +222,13 @@ static inline bool sdw_intel_sync_check_cmdsync_unlocked(struct sdw_intel *sdw)
 	return false;
 }
 
+static inline int sdw_intel_get_link_count(struct sdw_intel *sdw)
+{
+	if (SDW_INTEL_CHECK_OPS(sdw, get_link_count))
+		return SDW_INTEL_OPS(sdw, get_link_count)(sdw);
+	return 4; /* default on older generations */
+}
+
 /* common bus management */
 int intel_start_bus(struct sdw_intel *sdw);
 int intel_start_bus_after_reset(struct sdw_intel *sdw);
diff --git a/drivers/soundwire/intel_ace2x.c b/drivers/soundwire/intel_ace2x.c
index 781fe0aefa68..fff312c6968d 100644
--- a/drivers/soundwire/intel_ace2x.c
+++ b/drivers/soundwire/intel_ace2x.c
@@ -706,10 +706,30 @@ static void intel_program_sdi(struct sdw_intel *sdw, int dev_num)
 			__func__, sdw->instance, dev_num);
 }
 
+static int intel_get_link_count(struct sdw_intel *sdw)
+{
+	int ret;
+
+	ret = hdac_bus_eml_get_count(sdw->link_res->hbus, true, AZX_REG_ML_LEPTR_ID_SDW);
+	if (!ret) {
+		dev_err(sdw->cdns.dev, "%s: could not retrieve link count\n", __func__);
+		return -ENODEV;
+	}
+
+	if (ret > SDW_INTEL_MAX_LINKS) {
+		dev_err(sdw->cdns.dev, "%s: link count %d exceed max %d\n", __func__, ret, SDW_INTEL_MAX_LINKS);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
 const struct sdw_intel_hw_ops sdw_intel_lnl_hw_ops = {
 	.debugfs_init = intel_ace2x_debugfs_init,
 	.debugfs_exit = intel_ace2x_debugfs_exit,
 
+	.get_link_count = intel_get_link_count,
+
 	.register_dai = intel_register_dai,
 
 	.check_clock_stop = intel_check_clock_stop,
diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
index 8807e01cbf7c..d110f2b587d5 100644
--- a/drivers/soundwire/intel_auxdevice.c
+++ b/drivers/soundwire/intel_auxdevice.c
@@ -317,6 +317,20 @@ static int intel_link_probe(struct auxiliary_device *auxdev,
 	bus->link_id = auxdev->id;
 	bus->clk_stop_timeout = 1;
 
+	/*
+	 * paranoia check: make sure ACPI-reported number of links is aligned with
+	 * hardware capabilities.
+	 */
+	ret = sdw_intel_get_link_count(sdw);
+	if (ret < 0) {
+		dev_err(dev, "%s: sdw_intel_get_link_count failed: %d\n", __func__, ret);
+		return ret;
+	}
+	if (ret <= sdw->instance) {
+		dev_err(dev, "%s: invalid link id %d, link count %d\n", __func__, auxdev->id, ret);
+		return -EINVAL;
+	}
+
 	sdw_cdns_probe(cdns);
 
 	/* Set ops */
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index edbe1d4af8f8..37ae69365fe2 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -388,6 +388,7 @@ struct sdw_intel;
 /* struct intel_sdw_hw_ops - SoundWire ops for Intel platforms.
  * @debugfs_init: initialize all debugfs capabilities
  * @debugfs_exit: close and cleanup debugfs capabilities
+ * @get_link_count: fetch link count from hardware registers
  * @register_dai: read all PDI information and register DAIs
  * @check_clock_stop: throw error message if clock is not stopped.
  * @start_bus: normal start
@@ -412,6 +413,8 @@ struct sdw_intel_hw_ops {
 	void (*debugfs_init)(struct sdw_intel *sdw);
 	void (*debugfs_exit)(struct sdw_intel *sdw);
 
+	int (*get_link_count)(struct sdw_intel *sdw);
+
 	int (*register_dai)(struct sdw_intel *sdw);
 
 	void (*check_clock_stop)(struct sdw_intel *sdw);
-- 
2.43.0

Re: [PATCH 3/3] soundwire: intel: add probe-time check on link id

[Vinod Koul]

On 16-08-24, 10:33, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> In older platforms, the number of links was constant and hard-coded to
> 4. Newer platforms can have varying number of links, so we need to add
> a probe-time check to make sure the ACPI-reported information with
> _DSD properties is aligned with hardware capabilities reported in the
> SoundWire LCAP register.

Acked-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

Re: [PATCH 0/3] ALSA/ASoC/SoundWire: Intel: update maximum number of links

[Mark Brown]

On Fri, 16 Aug 2024 10:33:28 +0800, Bard Liao wrote:
> Intel new platforms can have up to 5 SoundWire links.
> This series does not apply to SoundWire tree due to recent changes in
> machine driver. Can we go via ASoC tree with Vinod's Acked-by tag?
> 
> Pierre-Louis Bossart (3):
>   ALSA/ASoC/SoundWire: Intel: use single definition for
>     SDW_INTEL_MAX_LINKS
>   soundwire: intel: increase maximum number of links
>   soundwire: intel: add probe-time check on link id
> 
> [...]

Applied to

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

Thanks!

[1/3] ALSA/ASoC/SoundWire: Intel: use single definition for SDW_INTEL_MAX_LINKS
      commit: b27404b2bbf951a11500525dea15681cc970226c
[2/3] soundwire: intel: increase maximum number of links
      commit: 1f3662838a05f1ab6af89a417f6f252d91d0806b
[3/3] soundwire: intel: add probe-time check on link id
      commit: d2234596be2192d9c1ae34f8c7534531191bc433

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