[PATCH 0/2] ASoC: Intel: avs: Switch to acpi-nhlt

[PATCH 0/2] ASoC: Intel: avs: Switch to acpi-nhlt

[Cezary Rojewski]

The change is based on rafael/acpi-nhlt [1] immutable branch which
Rafael kindly prepared for me. Without the topmost changes to ACPI/NHLT,
the patches present will fail to compile.

Recent changes for the ACPI tree [2] refactored interfaces of the NHLT
table. Currently we have two implementations - one found in acpi
subsystem (unused) and one in sound/hda/. As NHLT is part of ACPI, idea
is to make the former useful and then switch all users of existing
sound/hda/intel-nhlt.c to this new interface over time and remove the
duplicate afterward.

Two patches present here migrate the avs-driver and the intel-dsp-config
module.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/?h=acpi-nhlt
[2]: https://lore.kernel.org/linux-acpi/20240319083018.3159716-1-cezary.rojewski@intel.com/

Amadeusz Sławiński (2):
  ALSA: hda: intel-dsp-config: Switch to ACPI NHLT
  ASoC: Intel: avs: Switch to ACPI NHLT

 sound/hda/Kconfig                     |  1 +
 sound/hda/intel-dsp-config.c          | 16 ++++++++------
 sound/soc/intel/Kconfig               |  1 +
 sound/soc/intel/avs/avs.h             |  1 -
 sound/soc/intel/avs/board_selection.c |  7 +++---
 sound/soc/intel/avs/core.c            | 10 ++++-----
 sound/soc/intel/avs/path.c            | 32 +++++++++++++--------------
 7 files changed, 34 insertions(+), 34 deletions(-)


base-commit: a640acab545b21ed1f347376f34d34e461ea92ba
-- 
2.25.1

[PATCH 1/2] ALSA: hda: intel-dsp-config: Switch to ACPI NHLT

[Cezary Rojewski]

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

Now that NHLT support in ACPI framework was introduced, migrate
intel-dsp-config driver to new API.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/hda/Kconfig            |  1 +
 sound/hda/intel-dsp-config.c | 16 +++++++++-------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/sound/hda/Kconfig b/sound/hda/Kconfig
index 741179ccbd4e..e2ac247fc1d4 100644
--- a/sound/hda/Kconfig
+++ b/sound/hda/Kconfig
@@ -42,6 +42,7 @@ config SND_INTEL_NHLT
 
 config SND_INTEL_DSP_CONFIG
 	tristate
+	select ACPI_NHLT if ACPI
 	select SND_INTEL_NHLT if ACPI
 	select SND_INTEL_SOUNDWIRE_ACPI if ACPI
 	# this config should be selected only for Intel DSP platforms.
diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 6a384b922e4f..ea050805c20f 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -13,6 +13,8 @@
 #include <sound/intel-nhlt.h>
 #include <sound/soc-acpi.h>
 
+#include <acpi/nhlt.h>
+
 static int dsp_driver;
 
 module_param(dsp_driver, int, 0444);
@@ -570,15 +572,15 @@ static const struct config_entry *snd_intel_dsp_find_config
 
 static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
 {
-	struct nhlt_acpi_table *nhlt;
 	int ret = 0;
 
-	nhlt = intel_nhlt_init(&pci->dev);
-	if (nhlt) {
-		if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
-			ret = 1;
-		intel_nhlt_free(nhlt);
-	}
+	acpi_nhlt_get_gbl_table();
+
+	if (acpi_nhlt_find_endpoint(ACPI_NHLT_LINKTYPE_PDM, -1, -1, -1))
+		ret = 1;
+
+	acpi_nhlt_put_gbl_table();
+
 	return ret;
 }
 
-- 
2.25.1

Re: [PATCH 1/2] ALSA: hda: intel-dsp-config: Switch to ACPI NHLT

[Cezary Rojewski]

On 2024-04-19 10:43 AM, Cezary Rojewski wrote:
> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> 
> Now that NHLT support in ACPI framework was introduced, migrate
> intel-dsp-config driver to new API.

Hello,

I see that this patch has not landed in Mark's for-next. Takashi, what's 
the verdict here? Should this go entirely (entire patchset) through Mark 
or should the NHLT changes [1] plus this patch alone be incorporated 
into for-next of yours separately?


[1]: 
https://lore.kernel.org/linux-acpi/20240319083018.3159716-1-cezary.rojewski@intel.com/


Kind regards,
Czarek

Re: [PATCH 1/2] ALSA: hda: intel-dsp-config: Switch to ACPI NHLT

[Takashi Iwai]

On Thu, 09 May 2024 13:15:15 +0200,
Cezary Rojewski wrote:
> 
> On 2024-04-19 10:43 AM, Cezary Rojewski wrote:
> > From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> > 
> > Now that NHLT support in ACPI framework was introduced, migrate
> > intel-dsp-config driver to new API.
> 
> Hello,
> 
> I see that this patch has not landed in Mark's for-next. Takashi,
> what's the verdict here? Should this go entirely (entire patchset)
> through Mark or should the NHLT changes [1] plus this patch alone be
> incorporated into for-next of yours separately?

OK, I took the patch now to my tree.


thanks,

Takashi


> 
> 
> [1]:
> https://lore.kernel.org/linux-acpi/20240319083018.3159716-1-cezary.rojewski@intel.com/
> 
> 
> Kind regards,
> Czarek

Re: [PATCH 1/2] ALSA: hda: intel-dsp-config: Switch to ACPI NHLT

[Takashi Iwai]

On Thu, 09 May 2024 14:35:46 +0200,
Takashi Iwai wrote:
> 
> On Thu, 09 May 2024 13:15:15 +0200,
> Cezary Rojewski wrote:
> > 
> > On 2024-04-19 10:43 AM, Cezary Rojewski wrote:
> > > From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> > > 
> > > Now that NHLT support in ACPI framework was introduced, migrate
> > > intel-dsp-config driver to new API.
> > 
> > Hello,
> > 
> > I see that this patch has not landed in Mark's for-next. Takashi,
> > what's the verdict here? Should this go entirely (entire patchset)
> > through Mark or should the NHLT changes [1] plus this patch alone be
> > incorporated into for-next of yours separately?
> 
> OK, I took the patch now to my tree.

... and it turned out that this doesn't work easily because
include/linux/acpi/nhlt.h isn't a part of Linus tree yet.  I'd need to
pull the change via an immutable branch or such at first.

I don't know how Mark applied the ASoC patch, but if that actually
worked, it's better to take from his tree.  Or, in such a case, at
best to be merged through the tree where the new API got introduced
(i.e. ACPI tree).

In anyway, feel free to take my ack for the patch

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


thanks,

Takashi

> 
> 
> thanks,
> 
> Takashi
> 
> 
> > 
> > 
> > [1]:
> > https://lore.kernel.org/linux-acpi/20240319083018.3159716-1-cezary.rojewski@intel.com/
> > 
> > 
> > Kind regards,
> > Czarek

Re: [PATCH 1/2] ALSA: hda: intel-dsp-config: Switch to ACPI NHLT

[Mark Brown]

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

On Thu, May 09, 2024 at 03:08:28PM +0200, Takashi Iwai wrote:

> I don't know how Mark applied the ASoC patch, but if that actually
> worked, it's better to take from his tree.  Or, in such a case, at
> best to be merged through the tree where the new API got introduced
> (i.e. ACPI tree).

The cover letter mentioned a branch that's a dependency - TBH I'd been
expecting you to apply the core ALSA patch to your tree, but I can apply
to mine if you prefer?

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

Re: [PATCH 1/2] ALSA: hda: intel-dsp-config: Switch to ACPI NHLT

[Takashi Iwai]

On Thu, 09 May 2024 17:41:56 +0200,
Mark Brown wrote:
> 
> On Thu, May 09, 2024 at 03:08:28PM +0200, Takashi Iwai wrote:
> 
> > I don't know how Mark applied the ASoC patch, but if that actually
> > worked, it's better to take from his tree.  Or, in such a case, at
> > best to be merged through the tree where the new API got introduced
> > (i.e. ACPI tree).
> 
> The cover letter mentioned a branch that's a dependency - TBH I'd been
> expecting you to apply the core ALSA patch to your tree, but I can apply
> to mine if you prefer?

Ah thanks, I overlooked that.  Now applied to my tree.


Takashi

[PATCH 2/2] ASoC: Intel: avs: Switch to ACPI NHLT

[Cezary Rojewski]

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

Now that NHLT support in ACPI framework was introduced, migrate avs
driver to new API.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/Kconfig               |  1 +
 sound/soc/intel/avs/avs.h             |  1 -
 sound/soc/intel/avs/board_selection.c |  7 +++---
 sound/soc/intel/avs/core.c            | 10 ++++-----
 sound/soc/intel/avs/path.c            | 32 +++++++++++++--------------
 5 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 4b9e498e3303..38b61dfd1487 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -214,6 +214,7 @@ config SND_SOC_INTEL_AVS
 	depends on X86 || COMPILE_TEST
 	depends on PCI
 	depends on COMMON_CLK
+	select ACPI_NHLT if ACPI
 	select SND_SOC_ACPI if ACPI
 	select SND_SOC_TOPOLOGY
 	select SND_SOC_HDA
diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h
index f80f79415344..900786eb81e4 100644
--- a/sound/soc/intel/avs/avs.h
+++ b/sound/soc/intel/avs/avs.h
@@ -151,7 +151,6 @@ struct avs_dev {
 	struct completion fw_ready;
 	struct work_struct probe_work;
 
-	struct nhlt_acpi_table *nhlt;
 	struct list_head comp_list;
 	struct mutex comp_list_mutex;
 	struct list_head path_list;
diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
index 8360ce557401..290ea314ace8 100644
--- a/sound/soc/intel/avs/board_selection.c
+++ b/sound/soc/intel/avs/board_selection.c
@@ -10,10 +10,10 @@
 #include <linux/module.h>
 #include <linux/dmi.h>
 #include <linux/pci.h>
+#include <acpi/nhlt.h>
 #include <linux/platform_device.h>
 #include <sound/hda_codec.h>
 #include <sound/hda_register.h>
-#include <sound/intel-nhlt.h>
 #include <sound/soc-acpi.h>
 #include <sound/soc-component.h>
 #include "avs.h"
@@ -434,8 +434,7 @@ static int avs_register_dmic_board(struct avs_dev *adev)
 	struct snd_soc_acpi_mach mach = {{0}};
 	int ret;
 
-	if (!adev->nhlt ||
-	    !intel_nhlt_has_endpoint_type(adev->nhlt, NHLT_LINK_DMIC)) {
+	if (!acpi_nhlt_find_endpoint(ACPI_NHLT_LINKTYPE_PDM, -1, -1, -1)) {
 		dev_dbg(adev->dev, "no DMIC endpoints present\n");
 		return 0;
 	}
@@ -523,7 +522,7 @@ static int avs_register_i2s_boards(struct avs_dev *adev)
 	struct snd_soc_acpi_mach *mach;
 	int ret;
 
-	if (!adev->nhlt || !intel_nhlt_has_endpoint_type(adev->nhlt, NHLT_LINK_SSP)) {
+	if (!acpi_nhlt_find_endpoint(ACPI_NHLT_LINKTYPE_SSP, -1, -1, -1)) {
 		dev_dbg(adev->dev, "no I2S endpoints present\n");
 		return 0;
 	}
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index d7f8940099ce..a61ce42b426c 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -14,15 +14,16 @@
 // foundation of this driver
 //
 
+#include <linux/acpi.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <acpi/nhlt.h>
 #include <sound/hda_codec.h>
 #include <sound/hda_i915.h>
 #include <sound/hda_register.h>
 #include <sound/hdaudio.h>
 #include <sound/hdaudio_ext.h>
 #include <sound/intel-dsp-config.h>
-#include <sound/intel-nhlt.h>
 #include "../../codecs/hda.h"
 #include "avs.h"
 #include "cldma.h"
@@ -214,9 +215,7 @@ static void avs_hda_probe_work(struct work_struct *work)
 	if (ret < 0)
 		return;
 
-	adev->nhlt = intel_nhlt_init(adev->dev);
-	if (!adev->nhlt)
-		dev_info(bus->dev, "platform has no NHLT\n");
+	acpi_nhlt_get_gbl_table();
 	avs_debugfs_init(adev);
 
 	avs_register_all_boards(adev);
@@ -549,8 +548,7 @@ 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);
+	acpi_nhlt_put_gbl_table();
 
 	if (avs_platattr_test(adev, CLDMA))
 		hda_cldma_free(&code_loader);
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
index e785fc2a7008..531a086642be 100644
--- a/sound/soc/intel/avs/path.c
+++ b/sound/soc/intel/avs/path.c
@@ -6,7 +6,8 @@
 //          Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
 //
 
-#include <sound/intel-nhlt.h>
+#include <linux/acpi.h>
+#include <acpi/nhlt.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include "avs.h"
@@ -143,10 +144,10 @@ static bool avs_dma_type_is_input(u32 dma_type)
 
 static int avs_copier_create(struct avs_dev *adev, struct avs_path_module *mod)
 {
-	struct nhlt_acpi_table *nhlt = adev->nhlt;
 	struct avs_tplg_module *t = mod->template;
 	struct avs_copier_cfg *cfg;
-	struct nhlt_specific_cfg *ep_blob;
+	struct acpi_nhlt_format_config *ep_blob;
+	struct acpi_nhlt_endpoint *ep;
 	union avs_connector_node_id node_id = {0};
 	size_t cfg_size, data_size = 0;
 	void *data = NULL;
@@ -174,18 +175,18 @@ static int avs_copier_create(struct avs_dev *adev, struct avs_path_module *mod)
 		else
 			fmt = t->cfg_ext->copier.out_fmt;
 
-		ep_blob = intel_nhlt_get_endpoint_blob(adev->dev,
-			nhlt, t->cfg_ext->copier.vindex.i2s.instance,
-			NHLT_LINK_SSP, fmt->valid_bit_depth, fmt->bit_depth,
-			fmt->num_channels, fmt->sampling_freq, direction,
-			NHLT_DEVICE_I2S);
+		ep = acpi_nhlt_find_endpoint(ACPI_NHLT_LINKTYPE_SSP,
+					     ACPI_NHLT_DEVICETYPE_CODEC, direction,
+					     t->cfg_ext->copier.vindex.i2s.instance);
+		ep_blob = acpi_nhlt_endpoint_find_fmtcfg(ep, fmt->num_channels, fmt->sampling_freq,
+							 fmt->valid_bit_depth, fmt->bit_depth);
 		if (!ep_blob) {
 			dev_err(adev->dev, "no I2S ep_blob found\n");
 			return -ENOENT;
 		}
 
-		data = ep_blob->caps;
-		data_size = ep_blob->size;
+		data = ep_blob->config.capabilities;
+		data_size = ep_blob->config.capabilities_size;
 		/* I2S gateway's vindex is statically assigned in topology */
 		node_id.vindex = t->cfg_ext->copier.vindex.val;
 
@@ -199,17 +200,16 @@ static int avs_copier_create(struct avs_dev *adev, struct avs_path_module *mod)
 		else
 			fmt = t->in_fmt;
 
-		ep_blob = intel_nhlt_get_endpoint_blob(adev->dev, nhlt, 0,
-				NHLT_LINK_DMIC, fmt->valid_bit_depth,
-				fmt->bit_depth, fmt->num_channels,
-				fmt->sampling_freq, direction, NHLT_DEVICE_DMIC);
+		ep = acpi_nhlt_find_endpoint(ACPI_NHLT_LINKTYPE_PDM, -1, direction, 0);
+		ep_blob = acpi_nhlt_endpoint_find_fmtcfg(ep, fmt->num_channels, fmt->sampling_freq,
+							 fmt->valid_bit_depth, fmt->bit_depth);
 		if (!ep_blob) {
 			dev_err(adev->dev, "no DMIC ep_blob found\n");
 			return -ENOENT;
 		}
 
-		data = ep_blob->caps;
-		data_size = ep_blob->size;
+		data = ep_blob->config.capabilities;
+		data_size = ep_blob->config.capabilities_size;
 		/* DMIC gateway's vindex is statically assigned in topology */
 		node_id.vindex = t->cfg_ext->copier.vindex.val;
 
-- 
2.25.1

Re: (subset) [PATCH 0/2] ASoC: Intel: avs: Switch to acpi-nhlt

[Mark Brown]

On Fri, 19 Apr 2024 10:43:05 +0200, Cezary Rojewski wrote:
> The change is based on rafael/acpi-nhlt [1] immutable branch which
> Rafael kindly prepared for me. Without the topmost changes to ACPI/NHLT,
> the patches present will fail to compile.
> 
> Recent changes for the ACPI tree [2] refactored interfaces of the NHLT
> table. Currently we have two implementations - one found in acpi
> subsystem (unused) and one in sound/hda/. As NHLT is part of ACPI, idea
> is to make the former useful and then switch all users of existing
> sound/hda/intel-nhlt.c to this new interface over time and remove the
> duplicate afterward.
> 
> [...]

Applied to

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

Thanks!

[2/2] ASoC: Intel: avs: Switch to ACPI NHLT
      commit: f5d20b253d1a51aadb8881d899caaaa989217e89

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