[PATCH 0/2] ALSA/ASoC/SOF: SSP dev_type support

[PATCH 0/2] ALSA/ASoC/SOF: SSP dev_type support

[Peter Ujfalusi]

Hi,

SOF always passed 0 as dev_type to intel_nhlt_get_endpoint_blob() when looking
up the blob from the NHLT table. This causes issues since alsa-utils commit
3a47ef2487ed ("topology: nhlt: intel: support more device types and directions")

The dev_type is no longer always 0 in the topology embedded NHLT table resulting
lookup failures for analog codecs since they will have dev_type=4.

With SOF the dev_type is not used, we always use the SSP port index for looking
up the configuration blob.

The solution for the issue is to fetch the dev_type for the SSP port and feed it
back to the intel_nhlt_get_endpoint_blob() to be able to find the configuration
regardless of the assigned dev_type.


Regards,
Peter
---
Brent Lu (2):
  ALSA: hda: intel-nhlt: add intel_nhlt_ssp_device_type() function
  ASoC: SOF: ipc4-topology: support NHLT device type

 include/sound/intel-nhlt.h    | 10 ++++++++++
 sound/hda/intel-nhlt.c        | 26 ++++++++++++++++++++++++++
 sound/soc/sof/ipc4-topology.c | 19 ++++++++++++++++---
 3 files changed, 52 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/2] ALSA: hda: intel-nhlt: add intel_nhlt_ssp_device_type() function

[Peter Ujfalusi]

From: Brent Lu <brent.lu@intel.com>

Add a helper function intel_nhlt_ssp_device_type() to detect the type
of specific SSP port. The result is nhlt_device_type enum type which
could be NHLT_DEVICE_BT or NHLT_DEVICE_I2S.

Signed-off-by: Brent Lu <brent.lu@intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 include/sound/intel-nhlt.h | 10 ++++++++++
 sound/hda/intel-nhlt.c     | 26 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/include/sound/intel-nhlt.h b/include/sound/intel-nhlt.h
index 53470d6a28d6..24dbe16684ae 100644
--- a/include/sound/intel-nhlt.h
+++ b/include/sound/intel-nhlt.h
@@ -143,6 +143,9 @@ intel_nhlt_get_endpoint_blob(struct device *dev, struct nhlt_acpi_table *nhlt,
 			     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
 			     u8 num_ch, u32 rate, u8 dir, u8 dev_type);
 
+int intel_nhlt_ssp_device_type(struct device *dev, struct nhlt_acpi_table *nhlt,
+			       u8 virtual_bus_id);
+
 #else
 
 static inline struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
@@ -184,6 +187,13 @@ intel_nhlt_get_endpoint_blob(struct device *dev, struct nhlt_acpi_table *nhlt,
 	return NULL;
 }
 
+static inline int intel_nhlt_ssp_device_type(struct device *dev,
+					     struct nhlt_acpi_table *nhlt,
+					     u8 virtual_bus_id)
+{
+	return -EINVAL;
+}
+
 #endif
 
 #endif
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index 2c4dfc0b7e34..67d3e834c647 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -318,3 +318,29 @@ intel_nhlt_get_endpoint_blob(struct device *dev, struct nhlt_acpi_table *nhlt,
 	return NULL;
 }
 EXPORT_SYMBOL(intel_nhlt_get_endpoint_blob);
+
+int intel_nhlt_ssp_device_type(struct device *dev, struct nhlt_acpi_table *nhlt,
+			       u8 virtual_bus_id)
+{
+	struct nhlt_endpoint *epnt;
+	int i;
+
+	if (!nhlt)
+		return -EINVAL;
+
+	epnt = (struct nhlt_endpoint *)nhlt->desc;
+	for (i = 0; i < nhlt->endpoint_count; i++) {
+		/* for SSP link the virtual bus id is the SSP port number */
+		if (epnt->linktype == NHLT_LINK_SSP &&
+		    epnt->virtual_bus_id == virtual_bus_id) {
+			dev_dbg(dev, "SSP%d: dev_type=%d\n", virtual_bus_id,
+				epnt->device_type);
+			return epnt->device_type;
+		}
+
+		epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL(intel_nhlt_ssp_device_type);
-- 
2.43.0

[PATCH 2/2] ASoC: SOF: ipc4-topology: support NHLT device type

[Peter Ujfalusi]

From: Brent Lu <brent.lu@intel.com>

The endpoint in NHLT table for a SSP port could have the device type
NHLT_DEVICE_BT or NHLT_DEVICE_I2S. Use intel_nhlt_ssp_device_type()
function to retrieve the device type before querying the endpoint
blob to make sure we are always using correct device type parameter.

Signed-off-by: Brent Lu <brent.lu@intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/ipc4-topology.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index b89065abb511..f034fed06669 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -1348,6 +1348,7 @@ static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_s
 	int sample_rate, channel_count;
 	int bit_depth, ret;
 	u32 nhlt_type;
+	int dev_type = 0;
 
 	/* convert to NHLT type */
 	switch (linktype) {
@@ -1363,18 +1364,30 @@ static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_s
 						   &bit_depth);
 		if (ret < 0)
 			return ret;
+
+		/*
+		 * We need to know the type of the external device attached to a SSP
+		 * port to retrieve the blob from NHLT. However, device type is not
+		 * specified in topology.
+		 * Query the type for the port and then pass that information back
+		 * to the blob lookup function.
+		 */
+		dev_type = intel_nhlt_ssp_device_type(sdev->dev, ipc4_data->nhlt,
+						      dai_index);
+		if (dev_type < 0)
+			return dev_type;
 		break;
 	default:
 		return 0;
 	}
 
-	dev_dbg(sdev->dev, "dai index %d nhlt type %d direction %d\n",
-		dai_index, nhlt_type, dir);
+	dev_dbg(sdev->dev, "dai index %d nhlt type %d direction %d dev type %d\n",
+		dai_index, nhlt_type, dir, dev_type);
 
 	/* find NHLT blob with matching params */
 	cfg = intel_nhlt_get_endpoint_blob(sdev->dev, ipc4_data->nhlt, dai_index, nhlt_type,
 					   bit_depth, bit_depth, channel_count, sample_rate,
-					   dir, 0);
+					   dir, dev_type);
 
 	if (!cfg) {
 		dev_err(sdev->dev,
-- 
2.43.0

Re: [PATCH 2/2] ASoC: SOF: ipc4-topology: support NHLT device type

[Mark Brown]

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

On Mon, Nov 27, 2023 at 02:06:57PM +0200, Peter Ujfalusi wrote:
> From: Brent Lu <brent.lu@intel.com>
> 
> The endpoint in NHLT table for a SSP port could have the device type
> NHLT_DEVICE_BT or NHLT_DEVICE_I2S. Use intel_nhlt_ssp_device_type()
> function to retrieve the device type before querying the endpoint
> blob to make sure we are always using correct device type parameter.

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

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

Re: [PATCH 0/2] ALSA/ASoC/SOF: SSP dev_type support

[Péter Ujfalusi]

Hi Takashi,

On 27/11/2023 14:06, Peter Ujfalusi wrote:
> Hi,
> 
> SOF always passed 0 as dev_type to intel_nhlt_get_endpoint_blob() when looking
> up the blob from the NHLT table. This causes issues since alsa-utils commit
> 3a47ef2487ed ("topology: nhlt: intel: support more device types and directions")
> 
> The dev_type is no longer always 0 in the topology embedded NHLT table resulting
> lookup failures for analog codecs since they will have dev_type=4.
> 
> With SOF the dev_type is not used, we always use the SSP port index for looking
> up the configuration blob.
> 
> The solution for the issue is to fetch the dev_type for the SSP port and feed it
> back to the intel_nhlt_get_endpoint_blob() to be able to find the configuration
> regardless of the assigned dev_type.

Can you take these patches via the ALSA tree, I have hoped that it would
land in 6.9.

Thank you,
Peter

> Regards,
> Peter
> ---
> Brent Lu (2):
>   ALSA: hda: intel-nhlt: add intel_nhlt_ssp_device_type() function
>   ASoC: SOF: ipc4-topology: support NHLT device type
> 
>  include/sound/intel-nhlt.h    | 10 ++++++++++
>  sound/hda/intel-nhlt.c        | 26 ++++++++++++++++++++++++++
>  sound/soc/sof/ipc4-topology.c | 19 ++++++++++++++++---
>  3 files changed, 52 insertions(+), 3 deletions(-)
> 

-- 
Péter

Re: [PATCH 0/2] ALSA/ASoC/SOF: SSP dev_type support

[Takashi Iwai]

On Fri, 22 Mar 2024 12:30:44 +0100,
Péter Ujfalusi wrote:
> 
> Hi Takashi,
> 
> On 27/11/2023 14:06, Peter Ujfalusi wrote:
> > Hi,
> > 
> > SOF always passed 0 as dev_type to intel_nhlt_get_endpoint_blob() when looking
> > up the blob from the NHLT table. This causes issues since alsa-utils commit
> > 3a47ef2487ed ("topology: nhlt: intel: support more device types and directions")
> > 
> > The dev_type is no longer always 0 in the topology embedded NHLT table resulting
> > lookup failures for analog codecs since they will have dev_type=4.
> > 
> > With SOF the dev_type is not used, we always use the SSP port index for looking
> > up the configuration blob.
> > 
> > The solution for the issue is to fetch the dev_type for the SSP port and feed it
> > back to the intel_nhlt_get_endpoint_blob() to be able to find the configuration
> > regardless of the assigned dev_type.
> 
> Can you take these patches via the ALSA tree, I have hoped that it would
> land in 6.9.

It seems like to be overlooked.  I applied those now to for-linus
branch.  But it missed the PR to 6.9-rc1, so at earliest for 6.9-rc2.


thanks,

Takashi

Re: [PATCH 0/2] ALSA/ASoC/SOF: SSP dev_type support

[Péter Ujfalusi]

On 22/03/2024 13:42, Takashi Iwai wrote:
> On Fri, 22 Mar 2024 12:30:44 +0100,
>> Can you take these patches via the ALSA tree, I have hoped that it would
>> land in 6.9.
> 
> It seems like to be overlooked.  I applied those now to for-linus
> branch.  But it missed the PR to 6.9-rc1, so at earliest for 6.9-rc2.

Thank you! 6.9-rc is perfect

-- 
Péter