[PATCH 0/6] ASoC: Intel: Skylake: probe and Kconfig simplifications

[PATCH 0/6] ASoC: Intel: Skylake: probe and Kconfig simplifications

[Pierre-Louis Bossart]

The patches add support for CoffeeLake, harden and simplify the probe
for the Skylake driver, introduce more compile-time granularity so
that platforms can be selected individually.

The main difference since the RFC posted on November 20 is the
addition of the pci class/subclass/prog-if information. This greatly
simplifies the detection of the DSP and will allow for an automatic
selection between HDaudio legacy and Skylake drivers. As suggested by
Takashi this automatic selection will be posted separately (it will
result on changes on both sound/pci/hda and sound/soc/intel sides)

Pierre-Louis Bossart (5):
  ASoC: Intel: Skylake: Harden DSP detection with PCI class/subclass
    info
  ASoC: Intel: Skylake: stop probe if HDaudio capabilities don't exist
  ASoC: Intel: Skylake: remove useless tests on HDaudio capabilities
  ASoC: Intel: Skylake: add error logs on probe, remove dependency on
    NHLT
  ASoC: Intel: Skylake: Add more platform granularity

Takashi Iwai (1):
  ASoC: Intel: Skylake: Add CFL-S support

 sound/soc/intel/Kconfig                |  73 +++++++++++++--
 sound/soc/intel/boards/Kconfig         |  16 +++-
 sound/soc/intel/skylake/skl-messages.c |   8 ++
 sound/soc/intel/skylake/skl-nhlt.c     |   3 +
 sound/soc/intel/skylake/skl.c          | 125 +++++++++++++++++--------
 5 files changed, 179 insertions(+), 46 deletions(-)

-- 
2.17.1

[PATCH 1/6] ASoC: Intel: Skylake: Add CFL-S support

[Pierre-Louis Bossart]

From: Takashi Iwai <tiwai@suse.de>

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

Keep the existing declaration style for now, at a later point we may
transition and use PCI_DEVICE_DATA().

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 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 3f0ac1312982..df36b8fe6d5e 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -1121,6 +1121,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.17.1

[PATCH 2/6] ASoC: Intel: Skylake: Harden DSP detection with PCI class/subclass info

[Pierre-Louis Bossart]

The existing PPCAP and GCAP fields cannot be used reliably to
determine if the DSP is enabled by the BIOS. Instead rely on the
class/subclass information to find out if this driver can run or
not. The values in the code don't seem to be documented in publicly
available documents but are part of recommendations made to BIOS
writers and have been verified to be accurate on a number of
platforms.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/skylake/skl.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index df36b8fe6d5e..41a084b3d48f 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -916,6 +916,21 @@ static int skl_first_init(struct hdac_bus *bus)
 	unsigned short gcap;
 	int cp_streams, pb_streams, start_idx;
 
+	/*
+	 * detect DSP by checking class/subclass/prog-id information
+	 * class=04 subclass 03 prog-if 00: no DSP, legacy driver needs to be used
+	 * class=04 subclass 01 prog-if 00: DSP is present (and may be required e.g. for DMIC or SSP support)
+	 * class=04 subclass 03 prog-if 80: either of DSP or legacy mode can be used
+	 */
+	if (pci->class == 0x040300) {
+		dev_err(bus->dev, "The DSP is not enabled on this platform, aborting probe\n");
+		return -ENODEV;
+	} else if (pci->class != 0x040100 && pci->class != 0x040380) {
+		dev_err(bus->dev, "Unknown PCI class/subclass/prog-if information (0x%06x) found, aborting probe\n", pci->class);
+		return -ENODEV;
+	}
+	dev_info(bus->dev, "DSP detected with PCI class/subclass/prog-if info 0x%06x\n", pci->class);
+
 	err = pci_request_regions(pci, "Skylake HD audio");
 	if (err < 0)
 		return err;
-- 
2.17.1

[PATCH 3/6] ASoC: Intel: Skylake: stop probe if HDaudio capabilities don't exist

[Pierre-Louis Bossart]

Check immediately if required HDaudio capabilities can't be found (no
PPCAP or no streams exposed in GCAP), and move all DMA inits after the
error tests.

PPCAP and GCAP are not reliable indicators of DSP presence, but if
they don't exist then the driver will not work.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/skylake/skl.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 41a084b3d48f..72e366bbba33 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -946,6 +946,12 @@ static int skl_first_init(struct hdac_bus *bus)
 
 	snd_hdac_bus_parse_capabilities(bus);
 
+	/* check if PPCAP exists */
+	if (!bus->ppcap) {
+		dev_err(bus->dev, "bus ppcap not set, HDaudio or DSP not present?\n");
+		return -ENODEV;
+	}
+
 	if (skl_acquire_irq(bus, 0) < 0)
 		return -EBUSY;
 
@@ -955,23 +961,25 @@ static int skl_first_init(struct hdac_bus *bus)
 	gcap = snd_hdac_chip_readw(bus, GCAP);
 	dev_dbg(bus->dev, "chipset global capabilities = 0x%x\n", gcap);
 
-	/* allow 64bit DMA address if supported by H/W */
-	if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
-		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
-	} else {
-		dma_set_mask(bus->dev, DMA_BIT_MASK(32));
-		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
-	}
-
 	/* read number of streams from GCAP register */
 	cp_streams = (gcap >> 8) & 0x0f;
 	pb_streams = (gcap >> 12) & 0x0f;
 
-	if (!pb_streams && !cp_streams)
+	if (!pb_streams && !cp_streams) {
+		dev_err(bus->dev, "no streams found in GCAP definitions?\n");
 		return -EIO;
+	}
 
 	bus->num_streams = cp_streams + pb_streams;
 
+	/* allow 64bit DMA address if supported by H/W */
+	if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
+		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
+	} else {
+		dma_set_mask(bus->dev, DMA_BIT_MASK(32));
+		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
+	}
+
 	/* initialize streams */
 	snd_hdac_ext_stream_init_all
 		(bus, 0, cp_streams, SNDRV_PCM_STREAM_CAPTURE);
-- 
2.17.1

[PATCH 4/6] ASoC: Intel: Skylake: remove useless tests on HDaudio capabilities

[Pierre-Louis Bossart]

bus->ppcap is now tested upfront, there is no need to re-check if the
hardware is exposed as needed. Remove tests and remove indentation.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/skylake/skl.c | 40 ++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 72e366bbba33..f04e9ec4432b 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -826,12 +826,10 @@ static void skl_probe_work(struct work_struct *work)
 		return;
 	}
 
-	if (bus->ppcap) {
-		err = skl_machine_device_register(skl);
-		if (err < 0) {
-			dev_err(bus->dev, "machine register failed: %d\n", err);
-			goto out_err;
-		}
+	err = skl_machine_device_register(skl);
+	if (err < 0) {
+		dev_err(bus->dev, "machine register failed: %d\n", err);
+		goto out_err;
 	}
 
 	/*
@@ -1034,25 +1032,23 @@ static int skl_probe(struct pci_dev *pci,
 
 	pci_set_drvdata(skl->pci, bus);
 
-	/* check if dsp is there */
-	if (bus->ppcap) {
-		/* create device for dsp clk */
-		err = skl_clock_device_register(skl);
-		if (err < 0)
-			goto out_clk_free;
+	/* create device for dsp clk */
+	err = skl_clock_device_register(skl);
+	if (err < 0)
+		goto out_clk_free;
 
-		err = skl_find_machine(skl, (void *)pci_id->driver_data);
-		if (err < 0)
-			goto out_nhlt_free;
+	err = skl_find_machine(skl, (void *)pci_id->driver_data);
+	if (err < 0)
+		goto out_nhlt_free;
 
-		err = skl_init_dsp(skl);
-		if (err < 0) {
-			dev_dbg(bus->dev, "error failed to register dsp\n");
-			goto out_nhlt_free;
-		}
-		skl->skl_sst->enable_miscbdcge = skl_enable_miscbdcge;
-		skl->skl_sst->clock_power_gating = skl_clock_power_gating;
+	err = skl_init_dsp(skl);
+	if (err < 0) {
+		dev_dbg(bus->dev, "error failed to register dsp\n");
+		goto out_nhlt_free;
 	}
+	skl->skl_sst->enable_miscbdcge = skl_enable_miscbdcge;
+	skl->skl_sst->clock_power_gating = skl_clock_power_gating;
+
 	if (bus->mlcap)
 		snd_hdac_ext_bus_get_ml_capabilities(bus);
 
-- 
2.17.1

[PATCH 5/6] ASoC: Intel: Skylake: add error logs on probe, remove dependency on NHLT

[Pierre-Louis Bossart]

Add error logs to make probe debug easier.

Also remove hard-coded dependency on NHLT. NHLT literally stands for
NonHdaudioLinkTable and is only required for SSP/DMIC interfaces.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/skylake/skl-nhlt.c |  3 +++
 sound/soc/intel/skylake/skl.c      | 41 +++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index 01a050cf8775..5d125a3df527 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -180,6 +180,9 @@ int skl_get_dmic_geo(struct skl *skl)
 	unsigned int dmic_geo = 0;
 	u8 j;
 
+	if (!nhlt)
+		return 0;
+
 	epnt = (struct nhlt_endpoint *)nhlt->desc;
 
 	for (j = 0; j < nhlt->endpoint_count; j++) {
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index f04e9ec4432b..f9888b1d5521 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -1010,8 +1010,10 @@ static int skl_probe(struct pci_dev *pci,
 	bus = skl_to_bus(skl);
 
 	err = skl_first_init(bus);
-	if (err < 0)
+	if (err < 0) {
+		dev_err(bus->dev, "skl_first_init failed with err: %d\n", err);
 		goto out_free;
+	}
 
 	skl->pci_id = pci->device;
 
@@ -1020,26 +1022,39 @@ static int skl_probe(struct pci_dev *pci,
 	skl->nhlt = skl_nhlt_init(bus->dev);
 
 	if (skl->nhlt == NULL) {
+#if !IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
+		dev_err(bus->dev, "no nhlt info found\n");
 		err = -ENODEV;
 		goto out_free;
-	}
+#else
+		dev_warn(bus->dev, "no nhlt info found, continuing to try to enable HDaudio codec\n");
+#endif
+	} else {
 
-	err = skl_nhlt_create_sysfs(skl);
-	if (err < 0)
-		goto out_nhlt_free;
+		err = skl_nhlt_create_sysfs(skl);
+		if (err < 0) {
+			dev_err(bus->dev, "skl_nhlt_create_sysfs failed with err: %d\n", err);
+			goto out_nhlt_free;
+		}
 
-	skl_nhlt_update_topology_bin(skl);
+		skl_nhlt_update_topology_bin(skl);
+
+		/* create device for dsp clk */
+		err = skl_clock_device_register(skl);
+		if (err < 0) {
+			dev_err(bus->dev, "skl_clock_device_register failed with err: %d\n", err);
+			goto out_clk_free;
+		}
+	}
 
 	pci_set_drvdata(skl->pci, bus);
 
-	/* create device for dsp clk */
-	err = skl_clock_device_register(skl);
-	if (err < 0)
-		goto out_clk_free;
 
 	err = skl_find_machine(skl, (void *)pci_id->driver_data);
-	if (err < 0)
+	if (err < 0) {
+		dev_err(bus->dev, "skl_find_machine failed with err: %d\n", err);
 		goto out_nhlt_free;
+	}
 
 	err = skl_init_dsp(skl);
 	if (err < 0) {
@@ -1056,8 +1071,10 @@ static int skl_probe(struct pci_dev *pci,
 
 	/* create device for soc dmic */
 	err = skl_dmic_device_register(skl);
-	if (err < 0)
+	if (err < 0) {
+		dev_err(bus->dev, "skl_dmic_device_register failed with err: %d\n", err);
 		goto out_dsp_free;
+	}
 
 	schedule_work(&skl->probe_work);
 
-- 
2.17.1

[PATCH 6/6] ASoC: Intel: Skylake: Add more platform granularity

[Pierre-Louis Bossart]

The current SKYLAKE kconfig is a all-you-can-eat selection that will
support all known plaforms. This is however not necessarily a good
thing: most platforms for SKL and KBL don't support the DSP, but a
number of CNL/WHL ones do. Selecting this driver in all cases isn't
really smart and will require users to muck with blacklists.

Partition the configs to allow distributions to select on which
platform this driver is used. Keep the existing SND_SOC_INTEL_SKYLAKE
config to select everything for backwards compatibility. This patch does
not provide new functionality, only finer-grained choices in supported
platforms.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/Kconfig        | 73 ++++++++++++++++++++++++++++++----
 sound/soc/intel/boards/Kconfig | 16 +++++++-
 sound/soc/intel/skylake/skl.c  | 12 ++++++
 3 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 18e717703685..99a62ba409df 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -102,15 +102,74 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
 	  recommended option
 
 config SND_SOC_INTEL_SKYLAKE
-	tristate "SKL/BXT/KBL/GLK/CNL... Platforms"
+	tristate "All Skylake/SST Platforms"
 	depends on PCI && ACPI
-	select SND_SOC_INTEL_SKYLAKE_COMMON
+	select SND_SOC_INTEL_SKL
+	select SND_SOC_INTEL_APL
+	select SND_SOC_INTEL_KBL
+	select SND_SOC_INTEL_GLK
+	select SND_SOC_INTEL_CNL
+	select SND_SOC_INTEL_CFL
 	help
-	  If you have a Intel Skylake/Broxton/ApolloLake/KabyLake/
-	  GeminiLake or CannonLake platform with the DSP enabled in the BIOS
-	  then enable this option by saying Y or m.
+          This is a backwards-compatible option to select all devices
+	  supported by the Intel SST/Skylake driver. This option is no
+	  longer recommended and will be deprecated when the SOF
+	  driver is introduced.  Distributions should explicitly
+	  select which platform uses this driver.
+
+config SND_SOC_INTEL_SKL
+	tristate "Skylake Platforms"
+	depends on PCI && ACPI
+	select SND_SOC_INTEL_SKYLAKE_FAMILY
+	help
+	  If you have a Intel Skylake platform with the DSP enabled
+	  in the BIOS then enable this option by saying Y or m.
+
+config SND_SOC_INTEL_APL
+	tristate "Broxton/ApolloLake Platforms"
+	depends on PCI && ACPI
+	select SND_SOC_INTEL_SKYLAKE_FAMILY
+	help
+	  If you have a Intel Broxton/ApolloLake platform with the DSP
+	  enabled in the BIOS then enable this option by saying Y or m.
+
+config SND_SOC_INTEL_KBL
+	tristate "Kabylake Platforms"
+	depends on PCI && ACPI
+	select SND_SOC_INTEL_SKYLAKE_FAMILY
+	help
+	  If you have a Intel Kabylake platform with the DSP
+	  enabled in the BIOS then enable this option by saying Y or m.
+
+config SND_SOC_INTEL_GLK
+	tristate "GeminiLake Platforms"
+	depends on PCI && ACPI
+	select SND_SOC_INTEL_SKYLAKE_FAMILY
+	help
+	  If you have a Intel GeminiLake platform with the DSP
+	  enabled in the BIOS then enable this option by saying Y or m.
+
+config SND_SOC_INTEL_CNL
+	tristate "CannonLake/WhiskyLake Platforms"
+	depends on PCI && ACPI
+	select SND_SOC_INTEL_SKYLAKE_FAMILY
+	help
+	  If you have a Intel CNL/WHL platform with the DSP
+	  enabled in the BIOS then enable this option by saying Y or m.
+
+config SND_SOC_INTEL_CFL
+	tristate "CoffeeLake Platforms"
+	depends on PCI && ACPI
+	select SND_SOC_INTEL_SKYLAKE_FAMILY
+	help
+	  If you have a Intel CoffeeLake platform with the DSP
+	  enabled in the BIOS then enable this option by saying Y or m.
+
+config SND_SOC_INTEL_SKYLAKE_FAMILY
+	tristate
+	select SND_SOC_INTEL_SKYLAKE_COMMON
 
-if  SND_SOC_INTEL_SKYLAKE
+if SND_SOC_INTEL_SKYLAKE_FAMILY
 
 config SND_SOC_INTEL_SKYLAKE_SSP_CLK
 	tristate
@@ -135,7 +194,7 @@ config SND_SOC_INTEL_SKYLAKE_COMMON
 	  GeminiLake or CannonLake platform with the DSP enabled in the BIOS
 	  then enable this option by saying Y or m.
 
-endif ## SND_SOC_INTEL_SKYLAKE
+endif ## SND_SOC_INTEL_SKYLAKE_FAMILY
 
 config SND_SOC_ACPI_INTEL_MATCH
 	tristate
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index b177db2a0dbb..980bccaeb7aa 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -172,7 +172,7 @@ config SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH
 
 endif ## SND_SST_ATOM_HIFI2_PLATFORM
 
-if SND_SOC_INTEL_SKYLAKE
+if SND_SOC_INTEL_SKL
 
 config SND_SOC_INTEL_SKL_RT286_MACH
 	tristate "SKL with RT286 I2S mode"
@@ -212,6 +212,10 @@ config SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH
 	  Say Y or m if you have such a device. This is a recommended option.
 	  If unsure select "N".
 
+endif ## SND_SOC_INTEL_SKL
+
+if SND_SOC_INTEL_APL
+
 config SND_SOC_INTEL_BXT_DA7219_MAX98357A_MACH
 	tristate "Broxton with DA7219 and MAX98357A in I2S Mode"
 	depends on MFD_INTEL_LPSS && I2C && ACPI
@@ -239,6 +243,10 @@ config SND_SOC_INTEL_BXT_RT298_MACH
 	   Say Y or m if you have such a device. This is a recommended option.
 	   If unsure select "N".
 
+endif ## SND_SOC_INTEL_APL
+
+if SND_SOC_INTEL_KBL
+
 config SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH
 	tristate "KBL with RT5663 and MAX98927 in I2S Mode"
 	depends on MFD_INTEL_LPSS && I2C && ACPI
@@ -293,6 +301,10 @@ config SND_SOC_INTEL_KBL_DA7219_MAX98927_MACH
 	  Say Y if you have such a device.
 	  If unsure select "N".
 
+endif ## SND_SOC_INTEL_KBL
+
+if SND_SOC_INTEL_GLK
+
 config SND_SOC_INTEL_GLK_RT5682_MAX98357A_MACH
 	tristate "GLK with RT5682 and MAX98357A in I2S Mode"
 	depends on MFD_INTEL_LPSS && I2C && ACPI
@@ -307,7 +319,7 @@ config SND_SOC_INTEL_GLK_RT5682_MAX98357A_MACH
 	   Say Y if you have such a device.
 	   If unsure select "N".
 
-endif ## SND_SOC_INTEL_SKYLAKE
+endif ## SND_SOC_INTEL_GLK
 
 if SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC
 
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index f9888b1d5521..5abd35ca4e41 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -1142,24 +1142,36 @@ static void skl_remove(struct pci_dev *pci)
 
 /* PCI IDs */
 static const struct pci_device_id skl_ids[] = {
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_SKL)
 	/* Sunrise Point-LP */
 	{ PCI_DEVICE(0x8086, 0x9d70),
 		.driver_data = (unsigned long)&snd_soc_acpi_intel_skl_machines},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_APL)
 	/* BXT-P */
 	{ PCI_DEVICE(0x8086, 0x5a98),
 		.driver_data = (unsigned long)&snd_soc_acpi_intel_bxt_machines},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_KBL)
 	/* KBL */
 	{ PCI_DEVICE(0x8086, 0x9D71),
 		.driver_data = (unsigned long)&snd_soc_acpi_intel_kbl_machines},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_GLK)
 	/* GLK */
 	{ PCI_DEVICE(0x8086, 0x3198),
 		.driver_data = (unsigned long)&snd_soc_acpi_intel_glk_machines},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CNL)
 	/* CNL */
 	{ PCI_DEVICE(0x8086, 0x9dc8),
 		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CFL)
 	/* CFL */
 	{ PCI_DEVICE(0x8086, 0xa348),
 		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
+#endif
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, skl_ids);
-- 
2.17.1

Re: [PATCH 6/6] ASoC: Intel: Skylake: Add more platform granularity

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 479 bytes --]

On Fri, Dec 07, 2018 at 05:50:08PM -0600, Pierre-Louis Bossart wrote:
> The current SKYLAKE kconfig is a all-you-can-eat selection that will
> support all known plaforms. This is however not necessarily a good
> thing: most platforms for SKL and KBL don't support the DSP, but a
> number of CNL/WHL ones do. Selecting this driver in all cases isn't
> really smart and will require users to muck with blacklists.

This doesn't apply against current code, please check and resend.

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

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

Re: [PATCH 6/6] ASoC: Intel: Skylake: Add more platform granularity

[Pierre-Louis Bossart]

On 12/13/18 9:37 AM, Mark Brown wrote:
> On Fri, Dec 07, 2018 at 05:50:08PM -0600, Pierre-Louis Bossart wrote:
>> The current SKYLAKE kconfig is a all-you-can-eat selection that will
>> support all known plaforms. This is however not necessarily a good
>> thing: most platforms for SKL and KBL don't support the DSP, but a
>> number of CNL/WHL ones do. Selecting this driver in all cases isn't
>> really smart and will require users to muck with blacklists.
> This doesn't apply against current code, please check and resend.

This is odd. I just rebased on your for-next branch (SHA1 below) and see 
no issues? Did you push your latest changes? Or maybe this conflicts 
with Hui Wang's new KBL machine driver stuff?

Thanks!

-Pierre

62d67c75160c92465e56eea01a3e06131d644f3a

Re: [PATCH 6/6] ASoC: Intel: Skylake: Add more platform granularity

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 296 bytes --]

On Thu, Dec 13, 2018 at 11:44:48AM -0600, Pierre-Louis Bossart wrote:

> This is odd. I just rebased on your for-next branch (SHA1 below) and see no
> issues? Did you push your latest changes? Or maybe this conflicts with Hui
> Wang's new KBL machine driver stuff?

I'm guessing it's the latter.

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

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

Re: [PATCH 6/6] ASoC: Intel: Skylake: Add more platform granularity

[Pierre-Louis Bossart]

>> This is odd. I just rebased on your for-next branch (SHA1 below) and see no
>> issues? Did you push your latest changes? Or maybe this conflicts with Hui
>> Wang's new KBL machine driver stuff?
> I'm guessing it's the latter.
ok, i'll wait a bit then for the remotes to be updated, I don't see it yet.

Applied "ASoC: Intel: Skylake: add error logs on probe, remove dependency on NHLT" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: Skylake: add error logs on probe, remove dependency on NHLT

has been applied to the asoc tree at

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

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

>From f231c34ca9772f0b0e2d1b781e9c415847aff522 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Date: Fri, 7 Dec 2018 17:50:07 -0600
Subject: [PATCH] ASoC: Intel: Skylake: add error logs on probe, remove
 dependency on NHLT

Add error logs to make probe debug easier.

Also remove hard-coded dependency on NHLT. NHLT literally stands for
NonHdaudioLinkTable and is only required for SSP/DMIC interfaces.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/skylake/skl-nhlt.c |  3 +++
 sound/soc/intel/skylake/skl.c      | 41 +++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index 01a050cf8775..5d125a3df527 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -180,6 +180,9 @@ int skl_get_dmic_geo(struct skl *skl)
 	unsigned int dmic_geo = 0;
 	u8 j;
 
+	if (!nhlt)
+		return 0;
+
 	epnt = (struct nhlt_endpoint *)nhlt->desc;
 
 	for (j = 0; j < nhlt->endpoint_count; j++) {
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index f04e9ec4432b..f9888b1d5521 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -1010,8 +1010,10 @@ static int skl_probe(struct pci_dev *pci,
 	bus = skl_to_bus(skl);
 
 	err = skl_first_init(bus);
-	if (err < 0)
+	if (err < 0) {
+		dev_err(bus->dev, "skl_first_init failed with err: %d\n", err);
 		goto out_free;
+	}
 
 	skl->pci_id = pci->device;
 
@@ -1020,26 +1022,39 @@ static int skl_probe(struct pci_dev *pci,
 	skl->nhlt = skl_nhlt_init(bus->dev);
 
 	if (skl->nhlt == NULL) {
+#if !IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
+		dev_err(bus->dev, "no nhlt info found\n");
 		err = -ENODEV;
 		goto out_free;
-	}
+#else
+		dev_warn(bus->dev, "no nhlt info found, continuing to try to enable HDaudio codec\n");
+#endif
+	} else {
 
-	err = skl_nhlt_create_sysfs(skl);
-	if (err < 0)
-		goto out_nhlt_free;
+		err = skl_nhlt_create_sysfs(skl);
+		if (err < 0) {
+			dev_err(bus->dev, "skl_nhlt_create_sysfs failed with err: %d\n", err);
+			goto out_nhlt_free;
+		}
 
-	skl_nhlt_update_topology_bin(skl);
+		skl_nhlt_update_topology_bin(skl);
+
+		/* create device for dsp clk */
+		err = skl_clock_device_register(skl);
+		if (err < 0) {
+			dev_err(bus->dev, "skl_clock_device_register failed with err: %d\n", err);
+			goto out_clk_free;
+		}
+	}
 
 	pci_set_drvdata(skl->pci, bus);
 
-	/* create device for dsp clk */
-	err = skl_clock_device_register(skl);
-	if (err < 0)
-		goto out_clk_free;
 
 	err = skl_find_machine(skl, (void *)pci_id->driver_data);
-	if (err < 0)
+	if (err < 0) {
+		dev_err(bus->dev, "skl_find_machine failed with err: %d\n", err);
 		goto out_nhlt_free;
+	}
 
 	err = skl_init_dsp(skl);
 	if (err < 0) {
@@ -1056,8 +1071,10 @@ static int skl_probe(struct pci_dev *pci,
 
 	/* create device for soc dmic */
 	err = skl_dmic_device_register(skl);
-	if (err < 0)
+	if (err < 0) {
+		dev_err(bus->dev, "skl_dmic_device_register failed with err: %d\n", err);
 		goto out_dsp_free;
+	}
 
 	schedule_work(&skl->probe_work);
 
-- 
2.19.0.rc2

Applied "ASoC: Intel: Skylake: stop probe if HDaudio capabilities don't exist" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: Skylake: stop probe if HDaudio capabilities don't exist

has been applied to the asoc tree at

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

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

>From fa11ab5688f744bc868356f3f14c3bb9f283a780 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Date: Fri, 7 Dec 2018 17:50:05 -0600
Subject: [PATCH] ASoC: Intel: Skylake: stop probe if HDaudio capabilities
 don't exist

Check immediately if required HDaudio capabilities can't be found (no
PPCAP or no streams exposed in GCAP), and move all DMA inits after the
error tests.

PPCAP and GCAP are not reliable indicators of DSP presence, but if
they don't exist then the driver will not work.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/skylake/skl.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 41a084b3d48f..72e366bbba33 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -946,6 +946,12 @@ static int skl_first_init(struct hdac_bus *bus)
 
 	snd_hdac_bus_parse_capabilities(bus);
 
+	/* check if PPCAP exists */
+	if (!bus->ppcap) {
+		dev_err(bus->dev, "bus ppcap not set, HDaudio or DSP not present?\n");
+		return -ENODEV;
+	}
+
 	if (skl_acquire_irq(bus, 0) < 0)
 		return -EBUSY;
 
@@ -955,23 +961,25 @@ static int skl_first_init(struct hdac_bus *bus)
 	gcap = snd_hdac_chip_readw(bus, GCAP);
 	dev_dbg(bus->dev, "chipset global capabilities = 0x%x\n", gcap);
 
-	/* allow 64bit DMA address if supported by H/W */
-	if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
-		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
-	} else {
-		dma_set_mask(bus->dev, DMA_BIT_MASK(32));
-		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
-	}
-
 	/* read number of streams from GCAP register */
 	cp_streams = (gcap >> 8) & 0x0f;
 	pb_streams = (gcap >> 12) & 0x0f;
 
-	if (!pb_streams && !cp_streams)
+	if (!pb_streams && !cp_streams) {
+		dev_err(bus->dev, "no streams found in GCAP definitions?\n");
 		return -EIO;
+	}
 
 	bus->num_streams = cp_streams + pb_streams;
 
+	/* allow 64bit DMA address if supported by H/W */
+	if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
+		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
+	} else {
+		dma_set_mask(bus->dev, DMA_BIT_MASK(32));
+		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
+	}
+
 	/* initialize streams */
 	snd_hdac_ext_stream_init_all
 		(bus, 0, cp_streams, SNDRV_PCM_STREAM_CAPTURE);
-- 
2.19.0.rc2

Applied "ASoC: Intel: Skylake: Harden DSP detection with PCI class/subclass info" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: Skylake: Harden DSP detection with PCI class/subclass info

has been applied to the asoc tree at

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

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

>From c746de8dbc7b0ae9df491f7a99a6dab34203b51b Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Date: Fri, 7 Dec 2018 17:50:04 -0600
Subject: [PATCH] ASoC: Intel: Skylake: Harden DSP detection with PCI
 class/subclass info

The existing PPCAP and GCAP fields cannot be used reliably to
determine if the DSP is enabled by the BIOS. Instead rely on the
class/subclass information to find out if this driver can run or
not. The values in the code don't seem to be documented in publicly
available documents but are part of recommendations made to BIOS
writers and have been verified to be accurate on a number of
platforms.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/skylake/skl.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index df36b8fe6d5e..41a084b3d48f 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -916,6 +916,21 @@ static int skl_first_init(struct hdac_bus *bus)
 	unsigned short gcap;
 	int cp_streams, pb_streams, start_idx;
 
+	/*
+	 * detect DSP by checking class/subclass/prog-id information
+	 * class=04 subclass 03 prog-if 00: no DSP, legacy driver needs to be used
+	 * class=04 subclass 01 prog-if 00: DSP is present (and may be required e.g. for DMIC or SSP support)
+	 * class=04 subclass 03 prog-if 80: either of DSP or legacy mode can be used
+	 */
+	if (pci->class == 0x040300) {
+		dev_err(bus->dev, "The DSP is not enabled on this platform, aborting probe\n");
+		return -ENODEV;
+	} else if (pci->class != 0x040100 && pci->class != 0x040380) {
+		dev_err(bus->dev, "Unknown PCI class/subclass/prog-if information (0x%06x) found, aborting probe\n", pci->class);
+		return -ENODEV;
+	}
+	dev_info(bus->dev, "DSP detected with PCI class/subclass/prog-if info 0x%06x\n", pci->class);
+
 	err = pci_request_regions(pci, "Skylake HD audio");
 	if (err < 0)
 		return err;
-- 
2.19.0.rc2

Re: [PATCH 6/6] ASoC: Intel: Skylake: Add more platform granularity

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 499 bytes --]

On Thu, Dec 13, 2018 at 12:02:58PM -0600, Pierre-Louis Bossart wrote:

> > > This is odd. I just rebased on your for-next branch (SHA1 below) and see no
> > > issues? Did you push your latest changes? Or maybe this conflicts with Hui
> > > Wang's new KBL machine driver stuff?

> > I'm guessing it's the latter.

> ok, i'll wait a bit then for the remotes to be updated, I don't see it yet.

I did a push a few minutes ago, there's no unpushed changes here - let
me know if I missed something here.

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

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

Re: [PATCH 6/6] ASoC: Intel: Skylake: Add more platform granularity

[Pierre-Louis Bossart]

On 12/13/18 12:21 PM, Mark Brown wrote:
> On Thu, Dec 13, 2018 at 12:02:58PM -0600, Pierre-Louis Bossart wrote:
>
>>>> This is odd. I just rebased on your for-next branch (SHA1 below) and see no
>>>> issues? Did you push your latest changes? Or maybe this conflicts with Hui
>>>> Wang's new KBL machine driver stuff?
>>> I'm guessing it's the latter.
>> ok, i'll wait a bit then for the remotes to be updated, I don't see it yet.
> I did a push a few minutes ago, there's no unpushed changes here - let
> me know if I missed something here.

The conflict was indeed with Hui's patch, no big deal. i just re-sent a 
v2 for this patch.

Thanks!

-Pierre