Re: [PATCH] ASoC: Intel: hdac_hdmi: add Icelake support

[Takashi Iwai <tiwai@suse.de>]

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

On Wed, 14 Nov 2018 15:53:23 +0100,
Takashi Iwai wrote:
> 
> On Mon, 12 Nov 2018 15:00:36 +0100,
> Pierre-Louis Bossart wrote:
> > 
> > And btw the big topic is still how we provide distributions the means
> > to handle a 'graceful' fallback from DSP-enabled solutions (SST or
> > SOF) to legacy HDAudio, it's already popped up for cases where we have
> > HDaudio solutions with DMICs.
> 
> Yeah, that's a long-standing problem.  I've experimented some
> scenarios in the past, and the conclusion is that there is no really
> working fallback mechanism in general in Linux driver binding.
> That is, the only reasonable way seems to make a dedicated driver for
> the specific PCI ID (SKL+) doing the probe-and-fallback by itself,
> while excluding these IDs from other existing driver entries.
> 
> I'd love to proceed this but unfortunately I have no machine that can
> run SKL+ SST driver right now.  I have a new CFL devel box, but it has
> no support (PCI ID 8086:a348) as well as no firmware...

So, with a working machine, I could finally hack this a bit.
Below is a freshly cooked patch(set).  The first one is just to add
the support for my CFL machine, and another one is to add the fallback
binding with the legacy HD-audio on snd-soc-skl.

The changes aren't that big.  And you can still control the binding
via a module option, e.g. snd_soc_skl.legacy=1 will let it bound only
with the legacy driver, snd_soc_skl.legacy=2 for ASoC only.  The
default is 0, the fallback to legacy if ASoC binding fails.

It's still a PoC, no proper patch description is put yet.

Comments / suggestions appreciated.


thanks,

Takashi


[-- Attachment #2: 0001-ASoC-intel-skl-Add-CFL-S-support.patch --]
[-- Type: application/octet-stream, Size: 1585 bytes --]

>From 5dc96e698b71a55a71c667a352c0664d8e9c9d8e Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Fri, 16 Nov 2018 11:46:29 +0100
Subject: [PATCH 1/2] ASoC: intel: skl: Add CFL-S support

It's with CNP, supposed to be equivalent with CNL entry.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/soc/intel/skylake/skl-messages.c | 8 ++++++++
 sound/soc/intel/skylake/skl.c          | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index 8bfb8b0fa3d5..b0e6fb93eaf8 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -247,6 +247,14 @@ static const struct skl_dsp_ops dsp_ops[] = {
 		.init_fw = cnl_sst_init_fw,
 		.cleanup = cnl_sst_dsp_cleanup
 	},
+	{
+		.id = 0xa348,
+		.num_cores = 4,
+		.loader_ops = bxt_get_loader_ops,
+		.init = cnl_sst_dsp_init,
+		.init_fw = cnl_sst_init_fw,
+		.cleanup = cnl_sst_dsp_cleanup
+	},
 };
 
 const struct skl_dsp_ops *skl_get_dsp_ops(int pci_id)
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 29225623b4b4..f6d8cc23c471 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -1102,6 +1102,9 @@ static const struct pci_device_id skl_ids[] = {
 	/* CNL */
 	{ PCI_DEVICE(0x8086, 0x9dc8),
 		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
+	/* CFL */
+	{ PCI_DEVICE(0x8086, 0xa348),
+		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, skl_ids);
-- 
2.19.1


[-- Attachment #3: 0002-ALSA-hda-Allow-fallback-binding-with-legacy-HD-audio.patch --]
[-- Type: application/octet-stream, Size: 10950 bytes --]

>From fc6edd975ea6f21f23fec93ace130a482c1ad9bd Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Fri, 16 Nov 2018 12:57:46 +0100
Subject: [PATCH 2/2] ALSA: hda: Allow fallback binding with legacy HD-audio
 for Intel SKL+

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/hdaudio.h        |  6 +++
 sound/pci/hda/hda_controller.h |  2 +-
 sound/pci/hda/hda_intel.c      | 50 ++++++++++++++++----
 sound/soc/intel/Kconfig        |  8 ++++
 sound/soc/intel/skylake/skl.c  | 83 ++++++++++++++++++++++++++++++++--
 5 files changed, 137 insertions(+), 12 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index cd1773d0e08f..2ed67f315962 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -8,6 +8,7 @@
 
 #include <linux/device.h>
 #include <linux/interrupt.h>
+#include <linux/pci.h>
 #include <linux/pm_runtime.h>
 #include <linux/timecounter.h>
 #include <sound/core.h>
@@ -634,4 +635,9 @@ static inline unsigned int snd_array_index(struct snd_array *array, void *ptr)
 	for ((idx) = 0, (ptr) = (array)->list; (idx) < (array)->used; \
 	     (ptr) = snd_array_elem(array, ++(idx)))
 
+/* shared resource with ASoC and legacy HD-audio drivers */
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT
+const struct pci_driver *snd_hda_intel_probe(struct pci_dev *pci);
+#endif
+
 #endif /* __SOUND_HDAUDIO_H */
diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h
index c95097bb5a0c..2351115f922e 100644
--- a/sound/pci/hda/hda_controller.h
+++ b/sound/pci/hda/hda_controller.h
@@ -37,7 +37,7 @@
 #else
 #define AZX_DCAPS_I915_COMPONENT 0		/* NOP */
 #endif
-/* 14 unused */
+#define AZX_DCAPS_INTEL_SHARED	(1 << 14)	/* shared with ASoC */
 #define AZX_DCAPS_CTX_WORKAROUND (1 << 15)	/* X-Fi workaround */
 #define AZX_DCAPS_POSFIX_LPIB	(1 << 16)	/* Use LPIB as default */
 /* 17 unused */
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index d8eb2b5f51ae..92d1b96f6ef1 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2081,8 +2081,8 @@ static const struct hda_controller_ops pci_hda_ops = {
 	.link_power = azx_intel_link_power,
 };
 
-static int azx_probe(struct pci_dev *pci,
-		     const struct pci_device_id *pci_id)
+static int __azx_probe(struct pci_dev *pci, const struct pci_device_id *pci_id,
+		       bool skip_shared)
 {
 	static int dev;
 	struct snd_card *card;
@@ -2091,6 +2091,10 @@ static int azx_probe(struct pci_dev *pci,
 	bool schedule_probe;
 	int err;
 
+	/* skip the entry if it's shared with ASoC */
+	if (skip_shared && (pci_id->driver_data & AZX_DCAPS_INTEL_SHARED))
+		return -ENODEV;
+
 	if (dev >= SNDRV_CARDS)
 		return -ENODEV;
 	if (!enable[dev]) {
@@ -2158,6 +2162,12 @@ static int azx_probe(struct pci_dev *pci,
 	return err;
 }
 
+static int azx_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
+{
+	return __azx_probe(pci, pci_id,
+			   IS_ENABLED(CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT));
+}
+
 #ifdef CONFIG_PM
 /* On some boards setting power_save to a non 0 value leads to clicking /
  * popping sounds when ever we enter/leave powersaving mode. Ideally we would
@@ -2406,34 +2416,40 @@ static const struct pci_device_id azx_ids[] = {
 	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Sunrise Point-LP */
 	{ PCI_DEVICE(0x8086, 0x9d70),
-	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
+	  AZX_DCAPS_INTEL_SHARED },
 	/* Kabylake */
 	{ PCI_DEVICE(0x8086, 0xa171),
 	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Kabylake-LP */
 	{ PCI_DEVICE(0x8086, 0x9d71),
-	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
+	  AZX_DCAPS_INTEL_SHARED },
 	/* Kabylake-H */
 	{ PCI_DEVICE(0x8086, 0xa2f0),
 	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Coffelake */
 	{ PCI_DEVICE(0x8086, 0xa348),
-	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
+	  AZX_DCAPS_INTEL_SHARED },
 	/* Cannonlake */
 	{ PCI_DEVICE(0x8086, 0x9dc8),
-	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
+	  AZX_DCAPS_INTEL_SHARED },
 	/* Icelake */
 	{ PCI_DEVICE(0x8086, 0x34c8),
 	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
 	/* Broxton-P(Apollolake) */
 	{ PCI_DEVICE(0x8086, 0x5a98),
-	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON |
+	  AZX_DCAPS_INTEL_SHARED },
 	/* Broxton-T */
 	{ PCI_DEVICE(0x8086, 0x1a98),
 	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
 	/* Gemini-Lake */
 	{ PCI_DEVICE(0x8086, 0x3198),
-	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON |
+	  AZX_DCAPS_INTEL_SHARED },
 	/* Haswell */
 	{ PCI_DEVICE(0x8086, 0x0a0c),
 	  .driver_data = AZX_DRIVER_HDMI | AZX_DCAPS_INTEL_HASWELL },
@@ -2650,4 +2666,22 @@ static struct pci_driver azx_driver = {
 	},
 };
 
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT
+const struct pci_driver *
+snd_hda_intel_probe(struct pci_dev *pci)
+{
+	const struct pci_device_id *pci_id;
+	int ret;
+
+	pci_id = pci_match_id(azx_ids, pci);
+	if (!pci_id)
+		return ERR_PTR(-ENODEV);
+	ret = __azx_probe(pci, pci_id, false);
+	if (ret < 0)
+		return ERR_PTR(ret);
+	return &azx_driver;
+}
+EXPORT_SYMBOL_GPL(snd_hda_intel_probe);
+#endif
+
 module_pci_driver(azx_driver);
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 0caa1f4eb94d..f19ac24af272 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -117,6 +117,14 @@ config SND_SOC_INTEL_SKYLAKE
 	  GeminiLake or CannonLake platform with the DSP enabled in the BIOS
 	  then enable this option by saying Y or m.
 
+config SND_SOC_INTEL_SKL_LEGACY_SUPPORT
+	bool "Fallback legacy HD-audio binding"
+	depends on SND_SOC_INTEL_SKYLAKE
+	depends on SND_HDA_INTEL=y || SND_SOC_INTEL_SKYLAKE=SND_HDA_INTEL
+	help
+	  Fallback binding with the legacy HD-audio driver when no DSP is
+	  found
+
 config SND_SOC_ACPI_INTEL_MATCH
 	tristate
 	select SND_SOC_ACPI if ACPI
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index f6d8cc23c471..427bbda38805 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -39,6 +39,31 @@
 #include "skl-sst-ipc.h"
 #include "../../../soc/codecs/hdac_hda.h"
 
+enum {
+	SKL_BIND_AUTO,		/* fallback to legacy driver */
+	SKL_BIND_LEGACY,	/* bind only with legacy driver */
+	SKL_BIND_ASOC		/* bind only with ASoC driver */
+};
+
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT
+static const struct pci_driver *skl_fallback_driver;
+#define FALLBACK_PM_CALL(dev, method)					\
+	do {								\
+		if (skl_fallback_driver &&				\
+		    skl_fallback_driver->driver.pm->method)		\
+			return skl_fallback_driver->driver.pm->method(dev); \
+	} while (0)
+
+static int skl_legacy_binding;
+module_param_named(legacy, skl_legacy_binding, int, 0444);
+MODULE_PARM_DESC(legacy, "Binding with legacy HD-audio (0=fallback, 1=only legacy, 2=only asoc");
+#else
+#define skl_fallback_driver	NULL
+#define FALLBACK_PM_CALL(dev, method) do {} while (0)
+#define skl_legacy_binding	SKL_BIND_ASOC
+#endif
+
+
 /*
  * initialize the PCI registers
  */
@@ -262,6 +287,9 @@ static int skl_suspend_late(struct device *dev)
 	struct hdac_bus *bus = pci_get_drvdata(pci);
 	struct skl *skl = bus_to_skl(bus);
 
+	if (skl_fallback_driver)
+		return 0;
+
 	return skl_suspend_late_dsp(skl);
 }
 
@@ -311,6 +339,8 @@ static int skl_suspend(struct device *dev)
 	struct skl *skl  = bus_to_skl(bus);
 	int ret = 0;
 
+	FALLBACK_PM_CALL(dev, suspend);
+
 	/*
 	 * Do not suspend if streams which are marked ignore suspend are
 	 * running, we need to save the state for these and continue
@@ -349,6 +379,8 @@ static int skl_resume(struct device *dev)
 	struct hdac_ext_link *hlink = NULL;
 	int ret;
 
+	FALLBACK_PM_CALL(dev, resume);
+
 	/* Turned OFF in HDMI codec driver after codec reconfiguration */
 	if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) {
 		ret = snd_hdac_display_power(bus, true);
@@ -403,6 +435,8 @@ static int skl_runtime_suspend(struct device *dev)
 	struct pci_dev *pci = to_pci_dev(dev);
 	struct hdac_bus *bus = pci_get_drvdata(pci);
 
+	FALLBACK_PM_CALL(dev, runtime_suspend);
+
 	dev_dbg(bus->dev, "in %s\n", __func__);
 
 	return _skl_suspend(bus);
@@ -413,15 +447,24 @@ static int skl_runtime_resume(struct device *dev)
 	struct pci_dev *pci = to_pci_dev(dev);
 	struct hdac_bus *bus = pci_get_drvdata(pci);
 
+	FALLBACK_PM_CALL(dev, runtime_resume);
+
 	dev_dbg(bus->dev, "in %s\n", __func__);
 
 	return _skl_resume(bus);
 }
+
+static int skl_runtime_idle(struct device *dev)
+{
+	FALLBACK_PM_CALL(dev, runtime_idle);
+	return 0;
+}
 #endif /* CONFIG_PM */
 
 static const struct dev_pm_ops skl_pm = {
 	SET_SYSTEM_SLEEP_PM_OPS(skl_suspend, skl_resume)
-	SET_RUNTIME_PM_OPS(skl_runtime_suspend, skl_runtime_resume, NULL)
+	SET_RUNTIME_PM_OPS(skl_runtime_suspend, skl_runtime_resume,
+			   skl_runtime_idle)
 	.suspend_late = skl_suspend_late,
 };
 
@@ -955,8 +998,8 @@ static int skl_first_init(struct hdac_bus *bus)
 	return skl_init_chip(bus, true);
 }
 
-static int skl_probe(struct pci_dev *pci,
-		     const struct pci_device_id *pci_id)
+static int __skl_probe(struct pci_dev *pci,
+		       const struct pci_device_id *pci_id)
 {
 	struct skl *skl;
 	struct hdac_bus *bus = NULL;
@@ -1037,6 +1080,25 @@ static int skl_probe(struct pci_dev *pci,
 	return err;
 }
 
+static int skl_probe(struct pci_dev *pci,
+		     const struct pci_device_id *pci_id)
+{
+	int ret = -ENODEV;
+
+	if (skl_legacy_binding != SKL_BIND_LEGACY)
+		ret = __skl_probe(pci, pci_id);
+
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT
+	if (ret < 0 && skl_legacy_binding != SKL_BIND_ASOC) {
+		skl_fallback_driver = snd_hda_intel_probe(pci);
+		ret = PTR_ERR_OR_ZERO(skl_fallback_driver);
+		if (ret)
+			skl_fallback_driver = NULL;
+	}
+#endif
+	return ret;
+}
+
 static void skl_shutdown(struct pci_dev *pci)
 {
 	struct hdac_bus *bus = pci_get_drvdata(pci);
@@ -1044,6 +1106,13 @@ static void skl_shutdown(struct pci_dev *pci)
 	struct hdac_ext_stream *stream;
 	struct skl *skl;
 
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT
+	if (skl_fallback_driver) {
+		skl_fallback_driver->shutdown(pci);
+		return;
+	}
+#endif
+
 	if (!bus)
 		return;
 
@@ -1066,6 +1135,14 @@ static void skl_remove(struct pci_dev *pci)
 	struct hdac_bus *bus = pci_get_drvdata(pci);
 	struct skl *skl = bus_to_skl(bus);
 
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT
+	if (skl_fallback_driver) {
+		skl_fallback_driver->remove(pci);
+		skl_fallback_driver = NULL;
+		return;
+	}
+#endif
+
 	release_firmware(skl->tplg);
 
 	pm_runtime_get_noresume(&pci->dev);
-- 
2.19.1


[-- Attachment #4: Type: text/plain, Size: 0 bytes --]