[PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Pierre-Louis Bossart]

The module snd-intel-dspcfg, suggested by Jaroslav last year,
currently provide the means to select a PCI driver at run-time, based
on quirks, recommendations or user selection via a kernel
parameter. This capability removed a lot of confusions in
distributions and removed the need for recompilations to select legacy
HDaudio, SST or SOF drivers.

This patchset extends the concept to ACPI devices. This was driven by
the desire to at some point deprecate the Atom/SST driver for Baytrail
and Cherrytrail, which is no longer maintained by Intel. By having the
SOF driver enabled by distributions for Baytrail/Cherrytrail, we can
enable more end-user tests and make the transition easier for
distributions (likely in 2021 at this point).

This patchset provides the same solution for Broadwell, mainly to have
a single build for all Intel platforms. SOF on Broadwell remains an
option not recommended for distributions, as long as the 'catpt'
driver is maintained there is no burning desire to make SOF the
default on the three Broadwell-based platforms with the DSP
enabled.

Pierre-Louis Bossart (14):
  ASoC: Intel: broadwell: add missing pm_ops
  ASoC: Intel: bdw-rt5677: add missing pm_ops
  ALSA: hda: intel-dsp-config: add helper for ACPI DSP driver selection
  ASoC: soc-acpi: add helper to identify parent driver.
  ASoC: Intel: boards: byt/cht: set card and driver name at run time
  ASoC: Intel: byt/cht: set pm ops dynamically
  ASoC: SOF: acpi: add dynamic selection of DSP driver
  ASoC: Intel: Atom: add dynamic selection of DSP driver
  ASoC: SOF: Intel: allow for coexistence between SOF and Atom/SST
    drivers
  ALSA: hda: intel-dsp-config: add Broadwell ACPI DSP driver selection
  ASoC: Intel: broadwell: set card and driver name dynamically
  ASoC: Intel: catpt: add dynamic selection of DSP driver
  ASoC: SOF: Intel: allow for coexistence between SOF and catpt drivers
  ALSA: hda: intel-dsp-config: ignore dsp_driver parameter for PCI
    legacy devices

 include/sound/intel-dsp-config.h             |   7 ++
 include/sound/soc-acpi.h                     |   6 +
 sound/hda/intel-dsp-config.c                 | 111 +++++++++++++++++++
 sound/soc/intel/Kconfig                      |   2 +
 sound/soc/intel/atom/sst/sst_acpi.c          |   8 ++
 sound/soc/intel/boards/bdw-rt5650.c          |  17 ++-
 sound/soc/intel/boards/bdw-rt5677.c          |  18 ++-
 sound/soc/intel/boards/broadwell.c           |  20 ++--
 sound/soc/intel/boards/bytcht_cx2072x.c      |  27 +++--
 sound/soc/intel/boards/bytcht_da7213.c       |  27 +++--
 sound/soc/intel/boards/bytcht_es8316.c       |  29 +++--
 sound/soc/intel/boards/bytcr_rt5640.c        |  30 +++--
 sound/soc/intel/boards/bytcr_rt5651.c        |  27 +++--
 sound/soc/intel/boards/cht_bsw_max98090_ti.c |  29 +++--
 sound/soc/intel/boards/cht_bsw_nau8824.c     |  29 +++--
 sound/soc/intel/boards/cht_bsw_rt5645.c      |  38 ++++---
 sound/soc/intel/boards/cht_bsw_rt5672.c      |  29 +++--
 sound/soc/intel/catpt/device.c               |  12 ++
 sound/soc/sof/intel/Kconfig                  |  33 +++---
 sound/soc/sof/sof-acpi-dev.c                 |  14 ++-
 20 files changed, 392 insertions(+), 121 deletions(-)

-- 
2.25.1

[PATCH 01/14] ASoC: Intel: broadwell: add missing pm_ops

[Pierre-Louis Bossart]

For some reason this ops is missing in 2 out of the 3 broadwell
drivers. Add to make sure ASoC takes care of power management.

Tested-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/intel/boards/broadwell.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c
index 77c85f17aca6..69e0b13b47f4 100644
--- a/sound/soc/intel/boards/broadwell.c
+++ b/sound/soc/intel/boards/broadwell.c
@@ -318,6 +318,7 @@ static struct platform_driver broadwell_audio = {
 	.remove = broadwell_audio_remove,
 	.driver = {
 		.name = "broadwell-audio",
+		.pm = &snd_soc_pm_ops
 	},
 };
 
-- 
2.25.1

[PATCH 02/14] ASoC: Intel: bdw-rt5677: add missing pm_ops

[Pierre-Louis Bossart]

For some reason this ops is missing in 2 out of the 3 broadwell
drivers. Add to make sure ASoC takes care of power management.

Tested-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/intel/boards/bdw-rt5677.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c
index 7a3e773d0a1c..9cdd4164e1fb 100644
--- a/sound/soc/intel/boards/bdw-rt5677.c
+++ b/sound/soc/intel/boards/bdw-rt5677.c
@@ -446,6 +446,7 @@ static struct platform_driver bdw_rt5677_audio = {
 	.probe = bdw_rt5677_probe,
 	.driver = {
 		.name = "bdw-rt5677",
+		.pm = &snd_soc_pm_ops
 	},
 };
 
-- 
2.25.1

[PATCH 03/14] ALSA: hda: intel-dsp-config: add helper for ACPI DSP driver selection

[Pierre-Louis Bossart]

Mirror capabilities provided for PCI devices, so that distributions
can select which ACPI driver is loaded at run-time with kernel
parameters and DMI tables instead of forcing a build-time selection.

The "legacy" option supported for HDaudio has no meaning here and will
be ignored.

The 'SST' driver based on closed-source firmware has the priority to
avoid any impact on users, and the choice to use SOF is strictly
opt-in. This may change at some point when the 'SST' driver is
deprecated on Baytrail/Cherrytrail.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 include/sound/intel-dsp-config.h |  7 +++
 sound/hda/intel-dsp-config.c     | 77 ++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/include/sound/intel-dsp-config.h b/include/sound/intel-dsp-config.h
index c36622bee3f8..d4609077c258 100644
--- a/include/sound/intel-dsp-config.h
+++ b/include/sound/intel-dsp-config.h
@@ -21,6 +21,7 @@ enum {
 #if IS_ENABLED(CONFIG_SND_INTEL_DSP_CONFIG)
 
 int snd_intel_dsp_driver_probe(struct pci_dev *pci);
+int snd_intel_acpi_dsp_driver_probe(struct device *dev, const u8 acpi_hid[ACPI_ID_LEN]);
 
 #else
 
@@ -29,6 +30,12 @@ static inline int snd_intel_dsp_driver_probe(struct pci_dev *pci)
 	return SND_INTEL_DSP_DRIVER_ANY;
 }
 
+static inline
+int snd_intel_acpi_dsp_driver_probe(struct device *dev, const u8 acpi_hid[ACPI_ID_LEN])
+{
+	return SND_INTEL_DSP_DRIVER_ANY;
+}
+
 #endif
 
 #endif
diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 1c5114dedda9..7e6b8571c138 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -29,6 +29,7 @@ MODULE_PARM_DESC(dsp_driver, "Force the DSP driver for Intel DSP (0=auto, 1=lega
 struct config_entry {
 	u32 flags;
 	u16 device;
+	u8 acpi_hid[ACPI_ID_LEN];
 	const struct dmi_system_id *dmi_table;
 };
 
@@ -433,6 +434,82 @@ int snd_intel_dsp_driver_probe(struct pci_dev *pci)
 }
 EXPORT_SYMBOL_GPL(snd_intel_dsp_driver_probe);
 
+/*
+ * configuration table
+ * - the order of similar ACPI ID entries is important!
+ * - the first successful match will win
+ */
+static const struct config_entry acpi_config_table[] = {
+/* BayTrail */
+#if IS_ENABLED(CONFIG_SND_SST_ATOM_HIFI2_PLATFORM_ACPI)
+	{
+		.flags = FLAG_SST,
+		.acpi_hid = "80860F28",
+	},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
+	{
+		.flags = FLAG_SOF,
+		.acpi_hid = "80860F28",
+	},
+#endif
+/* CherryTrail */
+#if IS_ENABLED(CONFIG_SND_SST_ATOM_HIFI2_PLATFORM_ACPI)
+	{
+		.flags = FLAG_SST,
+		.acpi_hid = "808622A8",
+	},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
+	{
+		.flags = FLAG_SOF,
+		.acpi_hid = "808622A8",
+	},
+#endif
+};
+
+static const struct config_entry *snd_intel_acpi_dsp_find_config(const u8 acpi_hid[ACPI_ID_LEN],
+								 const struct config_entry *table,
+								 u32 len)
+{
+	for (; len > 0; len--, table++) {
+		if (memcmp(table->acpi_hid, acpi_hid, ACPI_ID_LEN))
+			continue;
+		if (table->dmi_table && !dmi_check_system(table->dmi_table))
+			continue;
+		return table;
+	}
+	return NULL;
+}
+
+int snd_intel_acpi_dsp_driver_probe(struct device *dev, const u8 acpi_hid[ACPI_ID_LEN])
+{
+	const struct config_entry *cfg;
+
+	if (dsp_driver > SND_INTEL_DSP_DRIVER_LEGACY && dsp_driver <= SND_INTEL_DSP_DRIVER_LAST)
+		return dsp_driver;
+
+	if (dsp_driver == SND_INTEL_DSP_DRIVER_LEGACY) {
+		dev_warn(dev, "dsp_driver parameter %d not supported, using automatic detection\n",
+			 SND_INTEL_DSP_DRIVER_LEGACY);
+	}
+
+	/* find the configuration for the specific device */
+	cfg = snd_intel_acpi_dsp_find_config(acpi_hid,  acpi_config_table,
+					     ARRAY_SIZE(acpi_config_table));
+	if (!cfg)
+		return SND_INTEL_DSP_DRIVER_ANY;
+
+	if (cfg->flags & FLAG_SST)
+		return SND_INTEL_DSP_DRIVER_SST;
+
+	if (cfg->flags & FLAG_SOF)
+		return SND_INTEL_DSP_DRIVER_SOF;
+
+	return SND_INTEL_DSP_DRIVER_SST;
+}
+EXPORT_SYMBOL_GPL(snd_intel_acpi_dsp_driver_probe);
+
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Intel DSP config driver");
 MODULE_IMPORT_NS(SOUNDWIRE_INTEL_INIT);
-- 
2.25.1

[PATCH 04/14] ASoC: soc-acpi: add helper to identify parent driver.

[Pierre-Louis Bossart]

Intel machine drivers are used by parent platform drivers based on
closed-source firmware (Atom/SST and catpt) and SOF-based ones.

In some cases for ACPI-based platforms, the behavior of machine
drivers needs to be modified depending on the parent type, typically
for card names and power management.

An initial solution based on passing a boolean flag as a platform
device parameter was tested earlier. Since it looked overkill, this
patch suggests instead a simple string comparison to identify an SOF
parent device/driver.

Suggested-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 include/sound/soc-acpi.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
index b16a844d16ef..9a43c44dcbbb 100644
--- a/include/sound/soc-acpi.h
+++ b/include/sound/soc-acpi.h
@@ -171,4 +171,10 @@ struct snd_soc_acpi_codecs {
 	u8 codecs[SND_SOC_ACPI_MAX_CODECS][ACPI_ID_LEN];
 };
 
+static inline bool snd_soc_acpi_sof_parent(struct device *dev)
+{
+	return dev->parent && dev->parent->driver && dev->parent->driver->name &&
+		!strcmp(dev->parent->driver->name, "sof-audio-acpi");
+}
+
 #endif
-- 
2.25.1

[PATCH 05/14] ASoC: Intel: boards: byt/cht: set card and driver name at run time

[Pierre-Louis Bossart]

To avoid hard-coded variations between SOF and SST drivers, set the
card name and driver dynamically depending on the parent type. This is
the first pass required to let distributions select which drivers to
use with kernel parameters instead of build-time selection.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/intel/boards/bytcht_cx2072x.c      | 20 +++++++++----
 sound/soc/intel/boards/bytcht_da7213.c       | 20 +++++++++----
 sound/soc/intel/boards/bytcht_es8316.c       | 22 +++++++++-----
 sound/soc/intel/boards/bytcr_rt5640.c        | 22 +++++++++-----
 sound/soc/intel/boards/bytcr_rt5651.c        | 20 +++++++++----
 sound/soc/intel/boards/cht_bsw_max98090_ti.c | 22 +++++++++-----
 sound/soc/intel/boards/cht_bsw_nau8824.c     | 22 +++++++++-----
 sound/soc/intel/boards/cht_bsw_rt5645.c      | 31 +++++++++++++-------
 sound/soc/intel/boards/cht_bsw_rt5672.c      | 22 +++++++++-----
 9 files changed, 141 insertions(+), 60 deletions(-)

diff --git a/sound/soc/intel/boards/bytcht_cx2072x.c b/sound/soc/intel/boards/bytcht_cx2072x.c
index 0b50b3646d55..762f09190f10 100644
--- a/sound/soc/intel/boards/bytcht_cx2072x.c
+++ b/sound/soc/intel/boards/bytcht_cx2072x.c
@@ -205,14 +205,12 @@ static struct snd_soc_dai_link byt_cht_cx2072x_dais[] = {
 	},
 };
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bytcht cx2072x" /* card name will be 'sof-bytcht cx2072x' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bytcht cx2072x" /* card name will be 'sof-bytcht cx2072x' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "bytcht-cx2072x"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* SoC card */
 static struct snd_soc_card byt_cht_cx2072x_card = {
@@ -236,6 +234,7 @@ static int snd_byt_cht_cx2072x_probe(struct platform_device *pdev)
 	struct snd_soc_acpi_mach *mach;
 	struct acpi_device *adev;
 	int dai_index = 0;
+	bool sof_parent;
 	int i, ret;
 
 	byt_cht_cx2072x_card.dev = &pdev->dev;
@@ -265,6 +264,17 @@ static int snd_byt_cht_cx2072x_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		byt_cht_cx2072x_card.name = SOF_CARD_NAME;
+		byt_cht_cx2072x_card.driver_name = SOF_DRIVER_NAME;
+	} else {
+		byt_cht_cx2072x_card.name = CARD_NAME;
+		byt_cht_cx2072x_card.driver_name = DRIVER_NAME;
+	}
+
 	return devm_snd_soc_register_card(&pdev->dev, &byt_cht_cx2072x_card);
 }
 
diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c
index e1e46b4bbac5..ef6682226a85 100644
--- a/sound/soc/intel/boards/bytcht_da7213.c
+++ b/sound/soc/intel/boards/bytcht_da7213.c
@@ -205,14 +205,12 @@ static struct snd_soc_dai_link dailink[] = {
 	},
 };
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bytcht da7213" /* card name will be 'sof-bytcht da7213' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bytcht da7213" /* card name will be 'sof-bytcht da7213' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "bytcht-da7213"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* SoC card */
 static struct snd_soc_card bytcht_da7213_card = {
@@ -237,6 +235,7 @@ static int bytcht_da7213_probe(struct platform_device *pdev)
 	struct snd_soc_acpi_mach *mach;
 	const char *platform_name;
 	struct acpi_device *adev;
+	bool sof_parent;
 	int dai_index = 0;
 	int ret_val = 0;
 	int i;
@@ -269,6 +268,17 @@ static int bytcht_da7213_probe(struct platform_device *pdev)
 	if (ret_val)
 		return ret_val;
 
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		bytcht_da7213_card.name = SOF_CARD_NAME;
+		bytcht_da7213_card.driver_name = SOF_DRIVER_NAME;
+	} else {
+		bytcht_da7213_card.name = CARD_NAME;
+		bytcht_da7213_card.driver_name = DRIVER_NAME;
+	}
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, card);
 	if (ret_val) {
 		dev_err(&pdev->dev,
diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
index 7ed869bf1a92..fbb62bab9dcc 100644
--- a/sound/soc/intel/boards/bytcht_es8316.c
+++ b/sound/soc/intel/boards/bytcht_es8316.c
@@ -406,18 +406,14 @@ static int byt_cht_es8316_resume(struct snd_soc_card *card)
 	return 0;
 }
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bytcht es8316" /* card name will be 'sof-bytcht es8316' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bytcht es8316" /* card name will be 'sof-bytcht es8316' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "bytcht-es8316"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 static struct snd_soc_card byt_cht_es8316_card = {
-	.name = CARD_NAME,
-	.driver_name = DRIVER_NAME,
 	.owner = THIS_MODULE,
 	.dai_link = byt_cht_es8316_dais,
 	.num_links = ARRAY_SIZE(byt_cht_es8316_dais),
@@ -472,6 +468,7 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
 	const char *platform_name;
 	struct acpi_device *adev;
 	struct device *codec_dev;
+	bool sof_parent;
 	unsigned int cnt = 0;
 	int dai_index = 0;
 	int i;
@@ -590,6 +587,17 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
 	byt_cht_es8316_card.long_name = long_name;
 #endif
 
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		byt_cht_es8316_card.name = SOF_CARD_NAME;
+		byt_cht_es8316_card.driver_name = SOF_DRIVER_NAME;
+	} else {
+		byt_cht_es8316_card.name = CARD_NAME;
+		byt_cht_es8316_card.driver_name = DRIVER_NAME;
+	}
+
 	/* register the soc card */
 	snd_soc_card_set_drvdata(&byt_cht_es8316_card, priv);
 
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 9dadf6561444..574b489a3283 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -1136,18 +1136,14 @@ static int byt_rt5640_resume(struct snd_soc_card *card)
 	return 0;
 }
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bytcht rt5640" /* card name will be 'sof-bytcht rt5640' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bytcht rt5640" /* card name will be 'sof-bytcht rt5640' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "bytcr-rt5640"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 static struct snd_soc_card byt_rt5640_card = {
-	.name = CARD_NAME,
-	.driver_name = DRIVER_NAME,
 	.owner = THIS_MODULE,
 	.dai_link = byt_rt5640_dais,
 	.num_links = ARRAY_SIZE(byt_rt5640_dais),
@@ -1173,6 +1169,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	struct snd_soc_acpi_mach *mach;
 	const char *platform_name;
 	struct acpi_device *adev;
+	bool sof_parent;
 	int ret_val = 0;
 	int dai_index = 0;
 	int i;
@@ -1336,6 +1333,17 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	if (ret_val)
 		return ret_val;
 
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		byt_rt5640_card.name = SOF_CARD_NAME;
+		byt_rt5640_card.driver_name = SOF_DRIVER_NAME;
+	} else {
+		byt_rt5640_card.name = CARD_NAME;
+		byt_rt5640_card.driver_name = DRIVER_NAME;
+	}
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5640_card);
 
 	if (ret_val) {
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 64d3fc4a3225..69e1d84e4de3 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -827,14 +827,12 @@ static int byt_rt5651_resume(struct snd_soc_card *card)
 	return 0;
 }
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bytcht rt5651" /* card name will be 'sof-bytcht rt5651' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bytcht rt5651" /* card name will be 'sof-bytcht rt5651' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "bytcr-rt5651"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 static struct snd_soc_card byt_rt5651_card = {
 	.name = CARD_NAME,
@@ -876,6 +874,7 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 	const char *platform_name;
 	struct acpi_device *adev;
 	struct device *codec_dev;
+	bool sof_parent;
 	bool is_bytcr = false;
 	int ret_val = 0;
 	int dai_index = 0;
@@ -1093,6 +1092,17 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 	if (ret_val)
 		return ret_val;
 
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		byt_rt5651_card.name = SOF_CARD_NAME;
+		byt_rt5651_card.driver_name = SOF_DRIVER_NAME;
+	} else {
+		byt_rt5651_card.name = CARD_NAME;
+		byt_rt5651_card.driver_name = DRIVER_NAME;
+	}
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5651_card);
 
 	if (ret_val) {
diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
index 835e9bd6b52d..a1d456d7a9c2 100644
--- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c
+++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
@@ -382,19 +382,15 @@ static struct snd_soc_dai_link cht_dailink[] = {
 	},
 };
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bytcht max98090" /* card name will be 'sof-bytcht max98090 */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bytcht max98090" /* card name will be 'sof-bytcht max98090 */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "chtmax98090"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* SoC card */
 static struct snd_soc_card snd_soc_card_cht = {
-	.name = CARD_NAME,
-	.driver_name = DRIVER_NAME,
 	.owner = THIS_MODULE,
 	.dai_link = cht_dailink,
 	.num_links = ARRAY_SIZE(cht_dailink),
@@ -540,6 +536,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 	const char *mclk_name;
 	struct snd_soc_acpi_mach *mach;
 	const char *platform_name;
+	bool sof_parent;
 
 	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
 	if (!drv)
@@ -602,6 +599,17 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 		}
 	}
 
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		snd_soc_card_cht.name = SOF_CARD_NAME;
+		snd_soc_card_cht.driver_name = SOF_DRIVER_NAME;
+	} else {
+		snd_soc_card_cht.name = CARD_NAME;
+		snd_soc_card_cht.driver_name = DRIVER_NAME;
+	}
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht);
 	if (ret_val) {
 		dev_err(&pdev->dev,
diff --git a/sound/soc/intel/boards/cht_bsw_nau8824.c b/sound/soc/intel/boards/cht_bsw_nau8824.c
index 3e12bff15fed..f173793d867b 100644
--- a/sound/soc/intel/boards/cht_bsw_nau8824.c
+++ b/sound/soc/intel/boards/cht_bsw_nau8824.c
@@ -231,19 +231,15 @@ static struct snd_soc_dai_link cht_dailink[] = {
 	},
 };
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bytcht nau8824" /* card name will be 'sof-bytcht nau8824 */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bytcht nau8824" /* card name will be 'sof-bytcht nau8824 */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "chtnau8824"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* SoC card */
 static struct snd_soc_card snd_soc_card_cht = {
-	.name = CARD_NAME,
-	.driver_name = DRIVER_NAME,
 	.owner = THIS_MODULE,
 	.dai_link = cht_dailink,
 	.num_links = ARRAY_SIZE(cht_dailink),
@@ -260,6 +256,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 	struct cht_mc_private *drv;
 	struct snd_soc_acpi_mach *mach;
 	const char *platform_name;
+	bool sof_parent;
 	int ret_val;
 
 	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
@@ -277,6 +274,17 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 	if (ret_val)
 		return ret_val;
 
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		snd_soc_card_cht.name = SOF_CARD_NAME;
+		snd_soc_card_cht.driver_name = SOF_DRIVER_NAME;
+	} else {
+		snd_soc_card_cht.name = CARD_NAME;
+		snd_soc_card_cht.driver_name = DRIVER_NAME;
+	}
+
 	/* register the soc card */
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht);
 	if (ret_val) {
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index b53c02481749..bdaf8d00fc6b 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -479,21 +479,17 @@ static struct snd_soc_dai_link cht_dailink[] = {
 	},
 };
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_RT5645_NAME "bytcht rt5645" /* card name 'sof-bytcht rt5645' */
-#define CARD_RT5650_NAME "bytcht rt5650" /* card name 'sof-bytcht rt5650' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_RT5645_NAME "bytcht rt5645" /* card name 'sof-bytcht rt5645' */
+#define SOF_CARD_RT5650_NAME "bytcht rt5650" /* card name 'sof-bytcht rt5650' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_RT5645_NAME "chtrt5645"
 #define CARD_RT5650_NAME "chtrt5650"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* SoC card */
 static struct snd_soc_card snd_soc_card_chtrt5645 = {
-	.name = CARD_RT5645_NAME,
-	.driver_name = DRIVER_NAME,
 	.owner = THIS_MODULE,
 	.dai_link = cht_dailink,
 	.num_links = ARRAY_SIZE(cht_dailink),
@@ -506,8 +502,6 @@ static struct snd_soc_card snd_soc_card_chtrt5645 = {
 };
 
 static struct snd_soc_card snd_soc_card_chtrt5650 = {
-	.name = CARD_RT5650_NAME,
-	.driver_name = DRIVER_NAME,
 	.owner = THIS_MODULE,
 	.dai_link = cht_dailink,
 	.num_links = ARRAY_SIZE(cht_dailink),
@@ -541,6 +535,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 	const char *platform_name;
 	struct cht_mc_private *drv;
 	struct acpi_device *adev;
+	bool sof_parent;
 	bool found = false;
 	bool is_bytcr = false;
 	int dai_index = 0;
@@ -680,6 +675,22 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 	}
 
 	snd_soc_card_set_drvdata(card, drv);
+
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		snd_soc_card_chtrt5645.name = SOF_CARD_RT5645_NAME;
+		snd_soc_card_chtrt5645.driver_name = SOF_DRIVER_NAME;
+		snd_soc_card_chtrt5650.name = SOF_CARD_RT5650_NAME;
+		snd_soc_card_chtrt5650.driver_name = SOF_DRIVER_NAME;
+	} else {
+		snd_soc_card_chtrt5645.name = CARD_RT5645_NAME;
+		snd_soc_card_chtrt5645.driver_name = DRIVER_NAME;
+		snd_soc_card_chtrt5650.name = CARD_RT5650_NAME;
+		snd_soc_card_chtrt5650.driver_name = DRIVER_NAME;
+	}
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, card);
 	if (ret_val) {
 		dev_err(&pdev->dev,
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index 8442be93eb1c..6c46bfc43b50 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -382,19 +382,15 @@ static int cht_resume_post(struct snd_soc_card *card)
 	return 0;
 }
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bytcht rt5672" /* card name will be 'sof-bytcht rt5672' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bytcht rt5672" /* card name will be 'sof-bytcht rt5672' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "cht-bsw-rt5672"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* SoC card */
 static struct snd_soc_card snd_soc_card_cht = {
-	.name = CARD_NAME,
-	.driver_name = DRIVER_NAME,
 	.owner = THIS_MODULE,
 	.dai_link = cht_dailink,
 	.num_links = ARRAY_SIZE(cht_dailink),
@@ -417,6 +413,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 	struct snd_soc_acpi_mach *mach = pdev->dev.platform_data;
 	const char *platform_name;
 	struct acpi_device *adev;
+	bool sof_parent;
 	int i;
 
 	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
@@ -458,6 +455,17 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 	}
 	snd_soc_card_set_drvdata(&snd_soc_card_cht, drv);
 
+	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+
+	/* set card and driver name */
+	if (sof_parent) {
+		snd_soc_card_cht.name = SOF_CARD_NAME;
+		snd_soc_card_cht.driver_name = SOF_DRIVER_NAME;
+	} else {
+		snd_soc_card_cht.name = CARD_NAME;
+		snd_soc_card_cht.driver_name = DRIVER_NAME;
+	}
+
 	/* register the soc card */
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht);
 	if (ret_val) {
-- 
2.25.1

[PATCH 06/14] ASoC: Intel: byt/cht: set pm ops dynamically

[Pierre-Louis Bossart]

The Atom/SST driver does not rely on ASoC power management, but the
SOF driver does. Rather than using a hard-coded build-time assignment,
we can set this pm_ops dynamically depending on what the parent
is. That will remove the last build-time dependency and allow for
coexistence of both SST and SOF drivers for Baytrail/Cherrytrail.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/intel/boards/bytcht_cx2072x.c      | 7 ++++---
 sound/soc/intel/boards/bytcht_da7213.c       | 7 ++++---
 sound/soc/intel/boards/bytcht_es8316.c       | 7 ++++---
 sound/soc/intel/boards/bytcr_rt5640.c        | 8 +++++---
 sound/soc/intel/boards/bytcr_rt5651.c        | 7 ++++---
 sound/soc/intel/boards/cht_bsw_max98090_ti.c | 7 ++++---
 sound/soc/intel/boards/cht_bsw_nau8824.c     | 7 ++++---
 sound/soc/intel/boards/cht_bsw_rt5645.c      | 7 ++++---
 sound/soc/intel/boards/cht_bsw_rt5672.c      | 7 ++++---
 9 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/sound/soc/intel/boards/bytcht_cx2072x.c b/sound/soc/intel/boards/bytcht_cx2072x.c
index 762f09190f10..2bfe3e4c696f 100644
--- a/sound/soc/intel/boards/bytcht_cx2072x.c
+++ b/sound/soc/intel/boards/bytcht_cx2072x.c
@@ -275,15 +275,16 @@ static int snd_byt_cht_cx2072x_probe(struct platform_device *pdev)
 		byt_cht_cx2072x_card.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		pdev->dev.driver->pm = &snd_soc_pm_ops;
+
 	return devm_snd_soc_register_card(&pdev->dev, &byt_cht_cx2072x_card);
 }
 
 static struct platform_driver snd_byt_cht_cx2072x_driver = {
 	.driver = {
 		.name = "bytcht_cx2072x",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = snd_byt_cht_cx2072x_probe,
 };
diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c
index ef6682226a85..cfeba27252ba 100644
--- a/sound/soc/intel/boards/bytcht_da7213.c
+++ b/sound/soc/intel/boards/bytcht_da7213.c
@@ -279,6 +279,10 @@ static int bytcht_da7213_probe(struct platform_device *pdev)
 		bytcht_da7213_card.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		pdev->dev.driver->pm = &snd_soc_pm_ops;
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, card);
 	if (ret_val) {
 		dev_err(&pdev->dev,
@@ -292,9 +296,6 @@ static int bytcht_da7213_probe(struct platform_device *pdev)
 static struct platform_driver bytcht_da7213_driver = {
 	.driver = {
 		.name = "bytcht_da7213",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = bytcht_da7213_probe,
 };
diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
index fbb62bab9dcc..892cf684216e 100644
--- a/sound/soc/intel/boards/bytcht_es8316.c
+++ b/sound/soc/intel/boards/bytcht_es8316.c
@@ -598,6 +598,10 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
 		byt_cht_es8316_card.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		dev->driver->pm = &snd_soc_pm_ops;
+
 	/* register the soc card */
 	snd_soc_card_set_drvdata(&byt_cht_es8316_card, priv);
 
@@ -623,9 +627,6 @@ static int snd_byt_cht_es8316_mc_remove(struct platform_device *pdev)
 static struct platform_driver snd_byt_cht_es8316_mc_driver = {
 	.driver = {
 		.name = "bytcht_es8316",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = snd_byt_cht_es8316_mc_probe,
 	.remove = snd_byt_cht_es8316_mc_remove,
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 574b489a3283..18f668fbc64c 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -1163,6 +1163,7 @@ struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
 
 static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	static const char * const map_name[] = { "dmic1", "dmic2", "in1", "in3" };
 	const struct dmi_system_id *dmi_id;
 	struct byt_rt5640_private *priv;
@@ -1344,6 +1345,10 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 		byt_rt5640_card.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		dev->driver->pm = &snd_soc_pm_ops;
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5640_card);
 
 	if (ret_val) {
@@ -1358,9 +1363,6 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 static struct platform_driver snd_byt_rt5640_mc_driver = {
 	.driver = {
 		.name = "bytcr_rt5640",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = snd_byt_rt5640_mc_probe,
 };
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 69e1d84e4de3..f289ec8563a1 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -1103,6 +1103,10 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 		byt_rt5651_card.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		pdev->dev.driver->pm = &snd_soc_pm_ops;
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5651_card);
 
 	if (ret_val) {
@@ -1117,9 +1121,6 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev)
 static struct platform_driver snd_byt_rt5651_mc_driver = {
 	.driver = {
 		.name = "bytcr_rt5651",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = snd_byt_rt5651_mc_probe,
 };
diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
index a1d456d7a9c2..131882378a59 100644
--- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c
+++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
@@ -610,6 +610,10 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 		snd_soc_card_cht.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		dev->driver->pm = &snd_soc_pm_ops;
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht);
 	if (ret_val) {
 		dev_err(&pdev->dev,
@@ -634,9 +638,6 @@ static int snd_cht_mc_remove(struct platform_device *pdev)
 static struct platform_driver snd_cht_mc_driver = {
 	.driver = {
 		.name = "cht-bsw-max98090",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = snd_cht_mc_probe,
 	.remove = snd_cht_mc_remove,
diff --git a/sound/soc/intel/boards/cht_bsw_nau8824.c b/sound/soc/intel/boards/cht_bsw_nau8824.c
index f173793d867b..8131af1730f7 100644
--- a/sound/soc/intel/boards/cht_bsw_nau8824.c
+++ b/sound/soc/intel/boards/cht_bsw_nau8824.c
@@ -285,6 +285,10 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 		snd_soc_card_cht.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		pdev->dev.driver->pm = &snd_soc_pm_ops;
+
 	/* register the soc card */
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht);
 	if (ret_val) {
@@ -300,9 +304,6 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 static struct platform_driver snd_cht_mc_driver = {
 	.driver = {
 		.name = "cht-bsw-nau8824",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = snd_cht_mc_probe,
 };
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index bdaf8d00fc6b..6fea554cfed5 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -691,6 +691,10 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 		snd_soc_card_chtrt5650.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		pdev->dev.driver->pm = &snd_soc_pm_ops;
+
 	ret_val = devm_snd_soc_register_card(&pdev->dev, card);
 	if (ret_val) {
 		dev_err(&pdev->dev,
@@ -704,9 +708,6 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 static struct platform_driver snd_cht_mc_driver = {
 	.driver = {
 		.name = "cht-bsw-rt5645",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = snd_cht_mc_probe,
 };
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index 6c46bfc43b50..10c88ef2f85d 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -466,6 +466,10 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 		snd_soc_card_cht.driver_name = DRIVER_NAME;
 	}
 
+	/* set pm ops */
+	if (sof_parent)
+		pdev->dev.driver->pm = &snd_soc_pm_ops;
+
 	/* register the soc card */
 	ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht);
 	if (ret_val) {
@@ -480,9 +484,6 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 static struct platform_driver snd_cht_mc_driver = {
 	.driver = {
 		.name = "cht-bsw-rt5672",
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
-		.pm = &snd_soc_pm_ops,
-#endif
 	},
 	.probe = snd_cht_mc_probe,
 };
-- 
2.25.1

[PATCH 07/14] ASoC: SOF: acpi: add dynamic selection of DSP driver

[Pierre-Louis Bossart]

Follow PCI example and stop the probe when another driver is desired
for the same ACPI HID.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/intel/Kconfig  |  1 +
 sound/soc/sof/sof-acpi-dev.c | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig
index b233302f05e4..b593b29789d5 100644
--- a/sound/soc/sof/intel/Kconfig
+++ b/sound/soc/sof/intel/Kconfig
@@ -78,6 +78,7 @@ config SND_SOC_SOF_BAYTRAIL_SUPPORT
 config SND_SOC_SOF_BAYTRAIL
 	tristate
 	select SND_SOC_SOF_INTEL_ATOM_HIFI_EP
+	select SND_INTEL_DSP_CONFIG
 	help
 	  This option is not user-selectable but automagically handled by
 	  'select' statements at a higher level.
diff --git a/sound/soc/sof/sof-acpi-dev.c b/sound/soc/sof/sof-acpi-dev.c
index a78b76ef37b2..2a369c2c6551 100644
--- a/sound/soc/sof/sof-acpi-dev.c
+++ b/sound/soc/sof/sof-acpi-dev.c
@@ -12,6 +12,7 @@
 #include <linux/firmware.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <sound/intel-dsp-config.h>
 #include <sound/soc-acpi.h>
 #include <sound/soc-acpi-intel-match.h>
 #include <sound/sof.h>
@@ -120,12 +121,23 @@ static void sof_acpi_probe_complete(struct device *dev)
 static int sof_acpi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	const struct acpi_device_id *id;
 	const struct sof_dev_desc *desc;
 	struct snd_sof_pdata *sof_pdata;
 	const struct snd_sof_dsp_ops *ops;
 	int ret;
 
-	dev_dbg(&pdev->dev, "ACPI DSP detected");
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!id)
+		return -ENODEV;
+
+	ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
+	if (ret != SND_INTEL_DSP_DRIVER_ANY && ret != SND_INTEL_DSP_DRIVER_SOF) {
+		dev_dbg(dev, "SOF ACPI driver not selected, aborting probe\n");
+		return -ENODEV;
+	}
+
+	dev_dbg(dev, "ACPI DSP detected");
 
 	sof_pdata = devm_kzalloc(dev, sizeof(*sof_pdata), GFP_KERNEL);
 	if (!sof_pdata)
-- 
2.25.1

[PATCH 08/14] ASoC: Intel: Atom: add dynamic selection of DSP driver

[Pierre-Louis Bossart]

Follow PCI example and stop the probe when another driver is desired
for the same ACPI HID.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/intel/Kconfig             | 1 +
 sound/soc/intel/atom/sst/sst_acpi.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index a5b446d5af19..4e9f910751a9 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -55,6 +55,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
 	depends on X86 && ACPI && PCI
 	select SND_SST_ATOM_HIFI2_PLATFORM
 	select SND_SOC_ACPI_INTEL_MATCH
+	select SND_INTEL_DSP_CONFIG
 	select IOSF_MBI
 	help
 	  If you have a Intel Baytrail or Cherrytrail platform with an I2S
diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c
index f943a0884976..2c1b8a2e3506 100644
--- a/sound/soc/intel/atom/sst/sst_acpi.c
+++ b/sound/soc/intel/atom/sst/sst_acpi.c
@@ -21,6 +21,7 @@
 #include <linux/acpi.h>
 #include <asm/platform_sst_audio.h>
 #include <sound/core.h>
+#include <sound/intel-dsp-config.h>
 #include <sound/soc.h>
 #include <sound/compress_driver.h>
 #include <acpi/acbuffer.h>
@@ -246,6 +247,13 @@ static int sst_acpi_probe(struct platform_device *pdev)
 	id = acpi_match_device(dev->driver->acpi_match_table, dev);
 	if (!id)
 		return -ENODEV;
+
+	ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
+	if (ret != SND_INTEL_DSP_DRIVER_ANY && ret != SND_INTEL_DSP_DRIVER_SST) {
+		dev_dbg(dev, "SST ACPI driver not selected, aborting probe\n");
+		return -ENODEV;
+	}
+
 	dev_dbg(dev, "for %s\n", id->id);
 
 	mach = (struct snd_soc_acpi_mach *)id->driver_data;
-- 
2.25.1

[PATCH 09/14] ASoC: SOF: Intel: allow for coexistence between SOF and Atom/SST drivers

[Pierre-Louis Bossart]

Now that we have all the support needed for coexistence between ACPI
drivers for Baytrail and Cherrytrail, remove mutual exclusion in the
Kconfig file. The selection is done by playing with the snd_intel_dsp
module parameter.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/intel/Kconfig | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig
index b593b29789d5..59b35fa87e0e 100644
--- a/sound/soc/sof/intel/Kconfig
+++ b/sound/soc/sof/intel/Kconfig
@@ -61,17 +61,16 @@ if SND_SOC_SOF_INTEL_ACPI
 
 config SND_SOC_SOF_BAYTRAIL_SUPPORT
 	bool "SOF support for Baytrail, Braswell and Cherrytrail"
-	depends on SND_SST_ATOM_HIFI2_PLATFORM_ACPI=n
 	help
 	  This adds support for Sound Open Firmware for Intel(R) platforms
 	  using the Baytrail, Braswell or Cherrytrail processors.
-	  This option is mutually exclusive with the Atom/SST and Baytrail
-	  legacy drivers. If you want to enable SOF on Baytrail/Cherrytrail,
-	  you need to deselect those options first.
-	  SOF does not support Baytrail-CR for now, so this option is not
-	  recommended for distros. At some point all legacy drivers will be
-	  deprecated but not before all userspace firmware/topology/UCM files
-	  are made available to downstream distros.
+	  This option can coexist in the same build with the Atom legacy
+	  drivers, currently the default but which will be deprecated
+	  at some point.
+	  Existing firmware/topology binaries and UCM configurations
+	  typically located in the root file system are already
+	  compatible with both SOF or Atom/SST legacy drivers.
+	  This is a recommended option for distributions.
 	  Say Y if you want to enable SOF on Baytrail/Cherrytrail.
 	  If unsure select "N".
 
-- 
2.25.1

[PATCH 10/14] ALSA: hda: intel-dsp-config: add Broadwell ACPI DSP driver selection

[Pierre-Louis Bossart]

Add ACPI IDs for Broadwell (and Haswell for consistency). This
addition is required for dynamic selection of drivers on those
devices.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/hda/intel-dsp-config.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 7e6b8571c138..0dc079ba02ff 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -466,6 +466,26 @@ static const struct config_entry acpi_config_table[] = {
 		.acpi_hid = "808622A8",
 	},
 #endif
+/* Broadwell */
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CATPT)
+	{
+		.flags = FLAG_SST,
+		.acpi_hid = "INT3438"
+	},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL)
+	{
+		.flags = FLAG_SOF,
+		.acpi_hid = "INT3438"
+	},
+#endif
+/* Haswell - not supported by SOF but added for consistency */
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CATPT)
+	{
+		.flags = FLAG_SST,
+		.acpi_hid = "INT33C8"
+	},
+#endif
 };
 
 static const struct config_entry *snd_intel_acpi_dsp_find_config(const u8 acpi_hid[ACPI_ID_LEN],
-- 
2.25.1

[PATCH 11/14] ASoC: Intel: broadwell: set card and driver name dynamically

[Pierre-Louis Bossart]

Remove last hard-coded build-time dependency

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/intel/boards/bdw-rt5650.c | 17 ++++++++++++-----
 sound/soc/intel/boards/bdw-rt5677.c | 17 ++++++++++++-----
 sound/soc/intel/boards/broadwell.c  | 19 ++++++++++++-------
 3 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c
index aa420b201848..c5122d3b0e6c 100644
--- a/sound/soc/intel/boards/bdw-rt5650.c
+++ b/sound/soc/intel/boards/bdw-rt5650.c
@@ -262,14 +262,12 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = {
 	},
 };
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bdw rt5650" /* card name will be 'sof-bdw rt5650' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bdw rt5650" /* card name will be 'sof-bdw rt5650' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "bdw-rt5650"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* ASoC machine driver for Broadwell DSP + RT5650 */
 static struct snd_soc_card bdw_rt5650_card = {
@@ -309,6 +307,15 @@ static int bdw_rt5650_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	/* set card and driver name */
+	if (snd_soc_acpi_sof_parent(&pdev->dev)) {
+		bdw_rt5650_card.name = SOF_CARD_NAME;
+		bdw_rt5650_card.driver_name = SOF_DRIVER_NAME;
+	} else {
+		bdw_rt5650_card.name = CARD_NAME;
+		bdw_rt5650_card.driver_name = DRIVER_NAME;
+	}
+
 	snd_soc_card_set_drvdata(&bdw_rt5650_card, bdw_rt5650);
 
 	return devm_snd_soc_register_card(&pdev->dev, &bdw_rt5650_card);
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c
index 9cdd4164e1fb..021bc59aac80 100644
--- a/sound/soc/intel/boards/bdw-rt5677.c
+++ b/sound/soc/intel/boards/bdw-rt5677.c
@@ -387,14 +387,12 @@ static int bdw_rt5677_resume_post(struct snd_soc_card *card)
 	return 0;
 }
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bdw rt5677" /* card name will be 'sof-bdw rt5677' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bdw rt5677" /* card name will be 'sof-bdw rt5677' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "bdw-rt5677"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* ASoC machine driver for Broadwell DSP + RT5677 */
 static struct snd_soc_card bdw_rt5677_card = {
@@ -437,6 +435,15 @@ static int bdw_rt5677_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	/* set card and driver name */
+	if (snd_soc_acpi_sof_parent(&pdev->dev)) {
+		bdw_rt5677_card.name = SOF_CARD_NAME;
+		bdw_rt5677_card.driver_name = SOF_DRIVER_NAME;
+	} else {
+		bdw_rt5677_card.name = CARD_NAME;
+		bdw_rt5677_card.driver_name = DRIVER_NAME;
+	}
+
 	snd_soc_card_set_drvdata(&bdw_rt5677_card, bdw_rt5677);
 
 	return devm_snd_soc_register_card(&pdev->dev, &bdw_rt5677_card);
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c
index 69e0b13b47f4..3c3aff9c61cc 100644
--- a/sound/soc/intel/boards/broadwell.c
+++ b/sound/soc/intel/boards/broadwell.c
@@ -262,19 +262,15 @@ static int broadwell_resume(struct snd_soc_card *card){
 	return 0;
 }
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL)
 /* use space before codec name to simplify card ID, and simplify driver name */
-#define CARD_NAME "bdw rt286" /* card name will be 'sof-bdw rt286' */
-#define DRIVER_NAME "SOF"
-#else
+#define SOF_CARD_NAME "bdw rt286" /* card name will be 'sof-bdw rt286' */
+#define SOF_DRIVER_NAME "SOF"
+
 #define CARD_NAME "broadwell-rt286"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
-#endif
 
 /* broadwell audio machine driver for WPT + RT286S */
 static struct snd_soc_card broadwell_rt286 = {
-	.name = CARD_NAME,
-	.driver_name = DRIVER_NAME,
 	.owner = THIS_MODULE,
 	.dai_link = broadwell_rt286_dais,
 	.num_links = ARRAY_SIZE(broadwell_rt286_dais),
@@ -303,6 +299,15 @@ static int broadwell_audio_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	/* set card and driver name */
+	if (snd_soc_acpi_sof_parent(&pdev->dev)) {
+		broadwell_rt286.name = SOF_CARD_NAME;
+		broadwell_rt286.driver_name = SOF_DRIVER_NAME;
+	} else {
+		broadwell_rt286.name = CARD_NAME;
+		broadwell_rt286.driver_name = DRIVER_NAME;
+	}
+
 	return devm_snd_soc_register_card(&pdev->dev, &broadwell_rt286);
 }
 
-- 
2.25.1

[PATCH 12/14] ASoC: Intel: catpt: add dynamic selection of DSP driver

[Pierre-Louis Bossart]

Follow PCI example and stop the probe when another driver is desired
for the same ACPI HID.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/intel/Kconfig        |  1 +
 sound/soc/intel/catpt/device.c | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 4e9f910751a9..051687d97039 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -24,6 +24,7 @@ config SND_SOC_INTEL_CATPT
 	depends on DMADEVICES && SND_DMA_SGBUF
 	select DW_DMAC_CORE
 	select SND_SOC_ACPI_INTEL_MATCH
+	select SND_INTEL_DSP_CONFIG
 	help
 	  Enable support for Intel(R) Haswell and Broadwell platforms
 	  with I2S codec present. This is a recommended option.
diff --git a/sound/soc/intel/catpt/device.c b/sound/soc/intel/catpt/device.c
index a70179959795..613585c3016f 100644
--- a/sound/soc/intel/catpt/device.c
+++ b/sound/soc/intel/catpt/device.c
@@ -19,6 +19,7 @@
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <sound/intel-dsp-config.h>
 #include <sound/soc.h>
 #include <sound/soc-acpi.h>
 #include <sound/soc-acpi-intel-match.h>
@@ -239,9 +240,20 @@ static int catpt_acpi_probe(struct platform_device *pdev)
 	const struct catpt_spec *spec;
 	struct catpt_dev *cdev;
 	struct device *dev = &pdev->dev;
+	const struct acpi_device_id *id;
 	struct resource *res;
 	int ret;
 
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!id)
+		return -ENODEV;
+
+	ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
+	if (ret != SND_INTEL_DSP_DRIVER_ANY && ret != SND_INTEL_DSP_DRIVER_SST) {
+		dev_dbg(dev, "CATPT ACPI driver not selected, aborting probe\n");
+		return -ENODEV;
+	}
+
 	spec = device_get_match_data(dev);
 	if (!spec)
 		return -ENODEV;
-- 
2.25.1

[PATCH 13/14] ASoC: SOF: Intel: allow for coexistence between SOF and catpt drivers

[Pierre-Louis Bossart]

Now that we have all the support needed for coexistence between ACPI
drivers for Broadwell, remove mutual exclusion in the Kconfig
file. The selection is done by playing with the snd_intel_dspcfg
module 'dsp_driver' parameter.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/intel/Kconfig | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig
index 59b35fa87e0e..084f5993f043 100644
--- a/sound/soc/sof/intel/Kconfig
+++ b/sound/soc/sof/intel/Kconfig
@@ -84,17 +84,18 @@ config SND_SOC_SOF_BAYTRAIL
 
 config SND_SOC_SOF_BROADWELL_SUPPORT
 	bool "SOF support for Broadwell"
-	depends on SND_SOC_INTEL_HASWELL=n
+	select SND_INTEL_DSP_CONFIG
 	help
 	  This adds support for Sound Open Firmware for Intel(R) platforms
 	  using the Broadwell processors.
-	  This option is mutually exclusive with the Haswell/Broadwell legacy
-	  driver. If you want to enable SOF on Broadwell you need to deselect
-	  the legacy driver first.
-	  SOF does not fully support Broadwell yet, so this option is not
-	  recommended for distros. At some point all legacy drivers will be
-	  deprecated but not before all userspace firmware/topology/UCM files
-	  are made available to downstream distros.
+	  This option can coexist in the same build with the default 'catpt'
+	  driver.
+	  Existing firmware/topology binaries and UCM configurations typically
+	  located in the root file system are already compatible with both SOF
+	  or catpt drivers.
+	  SOF does not fully support Broadwell and has limitations related to
+	  DMA and suspend-resume, this is not a recommended option for
+	  distributions.
 	  Say Y if you want to enable SOF on Broadwell.
 	  If unsure select "N".
 
-- 
2.25.1

[PATCH 14/14] ALSA: hda: intel-dsp-config: ignore dsp_driver parameter for PCI legacy devices

[Pierre-Louis Bossart]

On Haswell/Broadwell/Baytrail/Braswell, the DSP is not used for the
HDMI/DP interface, and setting the dsp_driver parameter to a value > 1
has the side effect of preventing the HDaudio legacy driver from
probing.

The DSP driver selection should really only handle cases where a DSP
is actually used. This patch traps all known PCI devices and makes
sure the HDaudio driver can always be probed.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/hda/intel-dsp-config.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 0dc079ba02ff..6a0d070c60c9 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -379,6 +379,20 @@ int snd_intel_dsp_driver_probe(struct pci_dev *pci)
 	if (pci->vendor != 0x8086)
 		return SND_INTEL_DSP_DRIVER_ANY;
 
+	/*
+	 * Legacy devices don't have a PCI-based DSP and use HDaudio
+	 * for HDMI/DP support, ignore kernel parameter
+	 */
+	switch (pci->device) {
+	case 0x160c: /* Broadwell */
+	case 0x0a0c: /* Haswell */
+	case 0x0c0c:
+	case 0x0d0c:
+	case 0x0f04: /* Baytrail */
+	case 0x2284: /* Braswell */
+		return SND_INTEL_DSP_DRIVER_ANY;
+	}
+
 	if (dsp_driver > 0 && dsp_driver <= SND_INTEL_DSP_DRIVER_LAST)
 		return dsp_driver;
 
-- 
2.25.1

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-12 11:38 PM, Pierre-Louis Bossart wrote:
> The module snd-intel-dspcfg, suggested by Jaroslav last year,
> currently provide the means to select a PCI driver at run-time, based
> on quirks, recommendations or user selection via a kernel
> parameter. This capability removed a lot of confusions in
> distributions and removed the need for recompilations to select legacy
> HDaudio, SST or SOF drivers.
> 
> This patchset extends the concept to ACPI devices. This was driven by
> the desire to at some point deprecate the Atom/SST driver for Baytrail
> and Cherrytrail, which is no longer maintained by Intel. By having the
> SOF driver enabled by distributions for Baytrail/Cherrytrail, we can
> enable more end-user tests and make the transition easier for
> distributions (likely in 2021 at this point).
> 
> This patchset provides the same solution for Broadwell, mainly to have
> a single build for all Intel platforms. SOF on Broadwell remains an
> option not recommended for distributions, as long as the 'catpt'
> driver is maintained there is no burning desire to make SOF the
> default on the three Broadwell-based platforms with the DSP
> enabled.
> 

Hello,

Don't understand why I was omitted in CC for catpt-related patches.

I'll try to do proper review tomorrow as it's late here but for
starters: why do we need any intel-dsp-config changes for non-HDA
platforms, what's the technical reason behind these?

Czarek

RE: [PATCH 01/14] ASoC: Intel: broadwell: add missing pm_ops

[Rojewski, Cezary]

On 2020-11-12 11:38 PM, Pierre-Louis Bossart wrote:
> For some reason this ops is missing in 2 out of the 3 broadwell
> drivers. Add to make sure ASoC takes care of power management.
> 

Changes provided with this patch are not connected to the overall idea
behind your patchset. I suggest splitting pm-changes into a separate
one.

The very first sentence in commit message isn't providing any value,
guess one-liner will suffice.

> Tested-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> ---
>   sound/soc/intel/boards/broadwell.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c
> index 77c85f17aca6..69e0b13b47f4 100644
> --- a/sound/soc/intel/boards/broadwell.c
> +++ b/sound/soc/intel/boards/broadwell.c
> @@ -318,6 +318,7 @@ static struct platform_driver broadwell_audio = {
>   	.remove = broadwell_audio_remove,
>   	.driver = {
>   		.name = "broadwell-audio",
> +		.pm = &snd_soc_pm_ops
>   	},
>   };
>   
>

RE: [PATCH 02/14] ASoC: Intel: bdw-rt5677: add missing pm_ops

[Rojewski, Cezary]

On 2020-11-12 11:38 PM, Pierre-Louis Bossart wrote:
> For some reason this ops is missing in 2 out of the 3 broadwell
> drivers. Add to make sure ASoC takes care of power management.
> 

Changes provided with this patch are not connected to the overall idea
behind your patchset. I suggest splitting pm-changes into a separate
one.

The very first sentence in commit message isn't providing any value,
guess one-liner will suffice.

> Tested-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> ---
>   sound/soc/intel/boards/bdw-rt5677.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c
> index 7a3e773d0a1c..9cdd4164e1fb 100644
> --- a/sound/soc/intel/boards/bdw-rt5677.c
> +++ b/sound/soc/intel/boards/bdw-rt5677.c
> @@ -446,6 +446,7 @@ static struct platform_driver bdw_rt5677_audio = {
>   	.probe = bdw_rt5677_probe,
>   	.driver = {
>   		.name = "bdw-rt5677",
> +		.pm = &snd_soc_pm_ops
>   	},
>   };
>   
>

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-13 12:04 AM, Rojewski, Cezary wrote:
> On 2020-11-12 11:38 PM, Pierre-Louis Bossart wrote:
>> The module snd-intel-dspcfg, suggested by Jaroslav last year,
>> currently provide the means to select a PCI driver at run-time, based
>> on quirks, recommendations or user selection via a kernel
>> parameter. This capability removed a lot of confusions in
>> distributions and removed the need for recompilations to select legacy
>> HDaudio, SST or SOF drivers.
>>
>> This patchset extends the concept to ACPI devices. This was driven by
>> the desire to at some point deprecate the Atom/SST driver for Baytrail
>> and Cherrytrail, which is no longer maintained by Intel. By having the
>> SOF driver enabled by distributions for Baytrail/Cherrytrail, we can
>> enable more end-user tests and make the transition easier for
>> distributions (likely in 2021 at this point).
>>
>> This patchset provides the same solution for Broadwell, mainly to have
>> a single build for all Intel platforms. SOF on Broadwell remains an
>> option not recommended for distributions, as long as the 'catpt'
>> driver is maintained there is no burning desire to make SOF the
>> default on the three Broadwell-based platforms with the DSP
>> enabled.
>>
> 
> Hello,
> 
> Don't understand why I was omitted in CC for catpt-related patches.
> 
> I'll try to do proper review tomorrow as it's late here but for
> starters: why do we need any intel-dsp-config changes for non-HDA
> platforms, what's the technical reason behind these?
> 
> Czarek
> 

As almost all of the patches share the concerns, decided to provide
entire output here so it's easier to navigate later on.

For a very long time upstream was filled with "flavors" of drivers for
Intel solutions. Having three available for BYT is a very good example
of that. The division of what goes where wasn't exactly explicit either.
This all leads to confusion - while community and users may feel
confused about what's recommended and what they should actually be
using, surprisingly (unsurprisingly?) developers were too.

Latest changes provided by Intel employees were addressing exactly that.
Removal of obsolete and redundant code. Together with fixing several
issues that were bothering users of older solutions, net gain was:
reduction of confusion and complexity factors.

Patchset presented here goes directly against that goal. We, Intel
developers, are tasked with providing reliable, working solutions
exposing best possible experience for our customers when dealing with
our products. And thus solutions provided are called recommended. We
don't deal with flavors and try-it-out-on-your-own-risk.

Moreover, intel-dsp-config module was created to address completely
different problem - problem of selecting correct HDA driver given the
circumstances. This is true as one cannot always rely on DSP-capability
bit being enabled when HDA-legacy is meant to be the default solution on
given platform. In future maybe we should revisit that subject once
again as perhaps even the existing solution isn't resolving the problem
completely.

Neither HSW/BDW nor atom-based platforms are HDA-based. Those are ACPI
devices and you know upfront what we're dealing with. There is no
DSP-capability bit to check for to know whether we're dealing with
legacy solution or not. As these are explicit configurations, one needs
not to bother with additional magic enums.

As long as Intel recommendation stays with /atom/ for atom-based
products, so should linux kernel.
Same for hsw/bdw - Intel recommends closed firmware solution and thus
this should remain as the only available choice - leaving no place for
confusion.

Once and if SOF is ready to support all available atom configuration, it
should deprecate and replace it with the same fashion catpt replaced its
predecessor. Until that moment, things should remain as they are. No
additional quirks or magic, just plain simple ACPI-ID based selection.

Users that are making use of optional and opportunistic changes
especially in regard to selecting not-recommended choices are
experienced enough to rebuild their kernel and merge out-of-tree changes
and they are free to do so if they want to. Upstream kernel, however, is
not place for keeping such code.

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Pierre-Louis Bossart]

> Once and if SOF is ready to support all available atom configuration, it
> should deprecate and replace it with the same fashion catpt replaced its
> predecessor. Until that moment, things should remain as they are. No
> additional quirks or magic, just plain simple ACPI-ID based selection.

We have lots of users for the existing legacy Baytrail/Cherrytrail 
driver and we cannot ask distributions to switch one sunny day.

I reached out to Hans and Jaroslav to understand how this deprecation 
might happen, and it has to be done in steps. First include the 
SOF/BYT-CHT driver in builds, experiment and test and after a successful 
test period switch over.

Keep in mind that Intel folks have only a very limited subset of the 
hardware based on BYT/CHT so we have to work with the community to get 
feedback, that's very different to Broadwell where there are only 3 
platforms to be supported and an extremely limited number of users 
impacted by a switch since we are out of the ChromeOS support period.

In all cases, distributions and users, not Intel, make the call.

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

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

On Fri, Nov 13, 2020 at 01:06:48PM +0000, Rojewski, Cezary wrote:

> For a very long time upstream was filled with "flavors" of drivers for
> Intel solutions. Having three available for BYT is a very good example
> of that. The division of what goes where wasn't exactly explicit either.
> This all leads to confusion - while community and users may feel
> confused about what's recommended and what they should actually be
> using, surprisingly (unsurprisingly?) developers were too.

...

> Patchset presented here goes directly against that goal. We, Intel
> developers, are tasked with providing reliable, working solutions
> exposing best possible experience for our customers when dealing with
> our products. And thus solutions provided are called recommended. We
> don't deal with flavors and try-it-out-on-your-own-risk.

My feeling here was that this is helping with this goal in that it's not
changing the defaults but is rather pushing the decision making process
from build time to runtime.  This means that distributions are able to
ship the preferred implementations for all the platforms without causing
any issues for the hopefully small set of users who need or want to work
on a different firmware, if they've been doing something like sticking
with an alternative firmware for old users since things were working
they'll be able to smoothly transition over to the current recommended
default, eg leaving old users on the old firmware by default.  That's a
bit of a niche use case but then hopefully all use cases for selecting a
non-default firmware are niche.

It also means that people don't have to think about this so much at
build time, they can just turn everything on and not worry they'll cause
problems for people using the binary and still get the recommended
runtime behaviour by default unless the user actively does something.

At any rate I'm not clear that I see this actively causing problems.

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

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Hans de Goede]

Hi,

On 11/13/20 5:49 PM, Mark Brown wrote:
> On Fri, Nov 13, 2020 at 01:06:48PM +0000, Rojewski, Cezary wrote:
> 
>> For a very long time upstream was filled with "flavors" of drivers for
>> Intel solutions. Having three available for BYT is a very good example
>> of that. The division of what goes where wasn't exactly explicit either.
>> This all leads to confusion - while community and users may feel
>> confused about what's recommended and what they should actually be
>> using, surprisingly (unsurprisingly?) developers were too.
> 
> ...
> 
>> Patchset presented here goes directly against that goal. We, Intel
>> developers, are tasked with providing reliable, working solutions
>> exposing best possible experience for our customers when dealing with
>> our products. And thus solutions provided are called recommended. We
>> don't deal with flavors and try-it-out-on-your-own-risk.
> 
> My feeling here was that this is helping with this goal in that it's not
> changing the defaults but is rather pushing the decision making process
> from build time to runtime.  This means that distributions are able to
> ship the preferred implementations for all the platforms without causing
> any issues for the hopefully small set of users who need or want to work
> on a different firmware, if they've been doing something like sticking
> with an alternative firmware for old users since things were working
> they'll be able to smoothly transition over to the current recommended
> default, eg leaving old users on the old firmware by default.  That's a
> bit of a niche use case but then hopefully all use cases for selecting a
> non-default firmware are niche.
> 
> It also means that people don't have to think about this so much at
> build time, they can just turn everything on and not worry they'll cause
> problems for people using the binary and still get the recommended
> runtime behaviour by default unless the user actively does something

Exactly. As Pierre-Louis mentions the Intel team does not have most
of the affected hardware. Since I've been working on making BYT and CHT
based devices work better with Linux as a side project for the last
couple of years I have been buying these kinda devices 2nd hand when ever
I can get one cheap and I've built a big collection of these (one might
say this has gotten out of hand a bit) see here:

https://github.com/jwrdegoede/sunxi-fedora-scripts/blob/master/x86-tablet-info
https://github.com/jwrdegoede/sunxi-fedora-scripts/blob/master/x86-codec-info

For my device collection (mostly the first link).

As Pierre-Louis mentioned I've already been working with him on getting
ready to move everything BYT/CHT related over to SOF. I've already been
testing SOF on various devices with mostly ok results so far.
But this is a process not a switch which we can simply throw.

So I'm all in favor of this patch-set. With some luck we can switch the
BYT/CHT default to SOF in Fedora for F34 beta (*), but doing that really
sorta hinges on this patch-set so that users can easily try the old
driver, both as a workaround for issues and to check if the problem
is caused by the switch to SOF.

Talking about doing this for Fedora 34, I think that switching the
default there is a good idea (and I can make this happen) what do
others think about doing this?

Regards,

Hans

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-13 6:06 PM, Hans de Goede wrote:
> On 11/13/20 5:49 PM, Mark Brown wrote:
>> On Fri, Nov 13, 2020 at 01:06:48PM +0000, Rojewski, Cezary wrote:
>>
>>> For a very long time upstream was filled with "flavors" of drivers for
>>> Intel solutions. Having three available for BYT is a very good example
>>> of that. The division of what goes where wasn't exactly explicit either.
>>> This all leads to confusion - while community and users may feel
>>> confused about what's recommended and what they should actually be
>>> using, surprisingly (unsurprisingly?) developers were too.
>>
>> ...
>>
>>> Patchset presented here goes directly against that goal. We, Intel
>>> developers, are tasked with providing reliable, working solutions
>>> exposing best possible experience for our customers when dealing with
>>> our products. And thus solutions provided are called recommended. We
>>> don't deal with flavors and try-it-out-on-your-own-risk.
>>
>> My feeling here was that this is helping with this goal in that it's not
>> changing the defaults but is rather pushing the decision making process
>> from build time to runtime.  This means that distributions are able to
>> ship the preferred implementations for all the platforms without causing
>> any issues for the hopefully small set of users who need or want to work
>> on a different firmware, if they've been doing something like sticking
>> with an alternative firmware for old users since things were working
>> they'll be able to smoothly transition over to the current recommended
>> default, eg leaving old users on the old firmware by default.  That's a
>> bit of a niche use case but then hopefully all use cases for selecting a
>> non-default firmware are niche.
>>
>> It also means that people don't have to think about this so much at
>> build time, they can just turn everything on and not worry they'll cause
>> problems for people using the binary and still get the recommended
>> runtime behaviour by default unless the user actively does something

Thanks for your input, Mark.

The ultimate goal is OK but the execution is not. Take a look at the
following:

+static inline bool snd_soc_acpi_sof_parent(struct device *dev)
+{
+	return dev->parent && dev->parent->driver && dev->parent->driver->name &&
+		!strcmp(dev->parent->driver->name, "sof-audio-acpi");
+}
+

-and-

+	/* set pm ops */
+	if (sof_parent)
+		pdev->dev.driver->pm = &snd_soc_pm_ops;
+

-and-

+	/* set card and driver name */
+	if (snd_soc_acpi_sof_parent(&pdev->dev)) {
+		bdw_rt5650_card.name = SOF_CARD_NAME;
+		bdw_rt5650_card.driver_name = SOF_DRIVER_NAME;
+	} else {
+		bdw_rt5650_card.name = CARD_NAME;
+		bdw_rt5650_card.driver_name = DRIVER_NAME;
+	}
+

pieces that are appearing several times throughout the series.
ASoC is a framework on its own. It is by all means an extension to an
older, more general ALSA framework. With every passing month SOF code
found in /sound/soc/sof gets closer to becoming yet another framework,
one that is placed on top of ASoC. Samples taken from this series
augment this statement. If ASoC framework is missing means for its child
drivers to do specific things, it's better to update the framework than
creating yet another one.

Explicit 'ifs' asking whether we're dealing with SOF or other solution
is at best a code smell. I believe this is unnecessary complexity added
to the code especially once you realize user needs to play with module
parameters to switch between solutions. If we assume user is able to
play with module parameters then why not simply make use of blacklist
mechanism?

And last but not least: intel-dsp-config is (as already stated) a mean
for solving the HDA-runtime-driver-selection problem. Mixing it with
ACPI devices is another layer of confusion.

> 
> Exactly. As Pierre-Louis mentions the Intel team does not have most
> of the affected hardware. Since I've been working on making BYT and CHT
> based devices work better with Linux as a side project for the last
> couple of years I have been buying these kinda devices 2nd hand when ever
... 
> As Pierre-Louis mentioned I've already been working with him on getting
> ready to move everything BYT/CHT related over to SOF. I've already been
> testing SOF on various devices with mostly ok results so far.
> But this is a process not a switch which we can simply throw.

Hans, thanks for sharing your concerns.

I'm afraid it's basically impossible to be fully prepared as you and
Pierre pointed out. Even when speaking about catpt and BDW, we too
didn't have all the available production stuff.

And thus I don't believe there will ever be a "good moment" to switch.
Once internal validation confirms driver is stable it's better to switch
entirely to the new solution with CI and devs on standby - ready to
address any upcoming reports. Don't believe /atom/ has clean slide
anyway given the patches and issues being posted from time to time
related to said solution.

I understand you're here for atom-based products yet the patchset also
touches on catpt aka hsw/bdw. While to my knowledge old /atom/ has no
active CI running - so the switch is desirable - the same cannot be said
about catpt. Because of that, I don't see any reason for appending any
catpt-related changes here. Leave no place for confusion. One solution
for one architecture that's recommended and maintained.

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Pierre-Louis Bossart]

> +static inline bool snd_soc_acpi_sof_parent(struct device *dev)
> +{
> +	return dev->parent && dev->parent->driver && dev->parent->driver->name &&
> +		!strcmp(dev->parent->driver->name, "sof-audio-acpi");
> +}
> +
> 
> -and-
> 
> +	/* set pm ops */
> +	if (sof_parent)
> +		pdev->dev.driver->pm = &snd_soc_pm_ops;
> +

The legacy Baytrail/Cherrytrail driver uses its own power management 
flow instead of the ASoC one. This patch does not cause the problem, it 
recognizes instead that this legacy driver cannot use the same pm ops.

I wish we didn't have to do this but there's just no alternative.

Dynamically assigning the .pm ops is not illegal and has been done in 
other drivers.

> -and-
> 
> +	/* set card and driver name */
> +	if (snd_soc_acpi_sof_parent(&pdev->dev)) {
> +		bdw_rt5650_card.name = SOF_CARD_NAME;
> +		bdw_rt5650_card.driver_name = SOF_DRIVER_NAME;
> +	} else {
> +		bdw_rt5650_card.name = CARD_NAME;
> +		bdw_rt5650_card.driver_name = DRIVER_NAME;
> +	}
> +

That is also intentional. We modified the card names based on Jaroslav's 
feedback, and this code change is just the mirror of what happened 
before with build-time code changes. Should we have changed the legacy 
card names? Maybe, but that might have added issues for users so we left 
the names untouched.

> pieces that are appearing several times throughout the series.
> ASoC is a framework on its own. It is by all means an extension to an
> older, more general ALSA framework. With every passing month SOF code
> found in /sound/soc/sof gets closer to becoming yet another framework,
> one that is placed on top of ASoC. Samples taken from this series
> augment this statement. If ASoC framework is missing means for its child
> drivers to do specific things, it's better to update the framework than
> creating yet another one.

There are no ASoC devices or drivers, we use platform devices/drivers. I 
don't see any need for an ASoC extension here.

> Explicit 'ifs' asking whether we're dealing with SOF or other solution
> is at best a code smell. I believe this is unnecessary complexity added
> to the code especially once you realize user needs to play with module
> parameters to switch between solutions. If we assume user is able to
> play with module parameters then why not simply make use of blacklist
> mechanism?

Been there, done that. We don't want to use either denylist of kernel 
parameters.

denylists create confusion for users, it's an endless stream of false 
errors and time lost in bug reports.

The use of the kernel parameter is ONLY for expert users who want to 
tinker with the system or debug issues, the average user should not have 
to play with either denylists or kernel parameters.

> And last but not least: intel-dsp-config is (as already stated) a mean
> for solving the HDA-runtime-driver-selection problem. Mixing it with
> ACPI devices is another layer of confusion.

Why? Who said it was PCI only? We already take care of DMIC, SoundWire, 
Google Chromebooks, open platforms, why not ACPI? It's just one API to 
be used when more than one driver can register to the same device.

>> Exactly. As Pierre-Louis mentions the Intel team does not have most
>> of the affected hardware. Since I've been working on making BYT and CHT
>> based devices work better with Linux as a side project for the last
>> couple of years I have been buying these kinda devices 2nd hand when ever
> ...
>> As Pierre-Louis mentioned I've already been working with him on getting
>> ready to move everything BYT/CHT related over to SOF. I've already been
>> testing SOF on various devices with mostly ok results so far.
>> But this is a process not a switch which we can simply throw.
> 
> Hans, thanks for sharing your concerns.
> 
> I'm afraid it's basically impossible to be fully prepared as you and
> Pierre pointed out. Even when speaking about catpt and BDW, we too
> didn't have all the available production stuff.
> 
> And thus I don't believe there will ever be a "good moment" to switch.
> Once internal validation confirms driver is stable it's better to switch
> entirely to the new solution with CI and devs on standby - ready to
> address any upcoming reports. Don't believe /atom/ has clean slide
> anyway given the patches and issues being posted from time to time
> related to said solution.

You refer to tests made by Intel when we are talking about community 
based tests here. We precisely do not have 'CI and devs on standby', the 
transition will be made by distributions themselves.

Besides, CI cannot test jack detection and all the flavors of 
microphone/speaker placements, which are the source of 99% of the issues.

> I understand you're here for atom-based products yet the patchset also
> touches on catpt aka hsw/bdw. While to my knowledge old /atom/ has no
> active CI running - so the switch is desirable - the same cannot be said
> about catpt. Because of that, I don't see any reason for appending any
> catpt-related changes here. Leave no place for confusion. One solution
> for one architecture that's recommended and maintained.

There is no confusion, the wording is explicit

"
SOF does not fully support Broadwell and has limitations related to
+	  DMA and suspend-resume, this is not a recommended option for
+	  distributions.
"

But there are niche users who prefer it for their own experiments, so 
what's the issue in making their life simpler?

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Takashi Iwai]

On Mon, 16 Nov 2020 18:47:22 +0100,
Pierre-Louis Bossart wrote:
> 
> > Explicit 'ifs' asking whether we're dealing with SOF or other solution
> > is at best a code smell. I believe this is unnecessary complexity added
> > to the code especially once you realize user needs to play with module
> > parameters to switch between solutions. If we assume user is able to
> > play with module parameters then why not simply make use of blacklist
> > mechanism?
> 
> Been there, done that. We don't want to use either denylist of kernel
> parameters.
> 
> denylists create confusion for users, it's an endless stream of false
> errors and time lost in bug reports.
> 
> The use of the kernel parameter is ONLY for expert users who want to
> tinker with the system or debug issues, the average user should not
> have to play with either denylists or kernel parameters.

I guess Cezary mean the modprobe blacklist?  This was used in the
early stage of ASoC Skylake driver development, but in the end, it's
more cumbersome because user needs to change multiple places.  The
single module parameter was easier to handle.

> > And last but not least: intel-dsp-config is (as already stated) a mean
> > for solving the HDA-runtime-driver-selection problem. Mixing it with
> > ACPI devices is another layer of confusion.
> 
> Why? Who said it was PCI only? We already take care of DMIC,
> SoundWire, Google Chromebooks, open platforms, why not ACPI? It's just
> one API to be used when more than one driver can register to the same
> device.

Well, currently intel-dsp-config sits in sound/hda, which isn't really
intuitive.  Though, Intel driver file paths are already fairly
scattered, so it doesn't matter too much :)

I don't mind to move it to another directory, but which one...?
x86 might match, but shuffling the place won't help for maintenance.

I personally find this move good, at least it makes things easier for
distros.  There are small details like the above, but technically
seen, I see no big problem.


thanks,

Takashi

Re: [PATCH 06/14] ASoC: Intel: byt/cht: set pm ops dynamically

[Mark Brown]

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

On Thu, Nov 12, 2020 at 04:38:17PM -0600, Pierre-Louis Bossart wrote:

> +	/* set pm ops */
> +	if (sof_parent)
> +		pdev->dev.driver->pm = &snd_soc_pm_ops;

This might need revisiting in future since we should be able to have the
driver PM ops be static const and hence unwritable but that's more of a
robustness thing for the time being given that only a limited set of
systems have this hardware and we know that there can't be multiple
devices.

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

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

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

On Tue, Nov 17, 2020 at 03:04:31PM +0100, Takashi Iwai wrote:
> Pierre-Louis Bossart wrote:

> > Why? Who said it was PCI only? We already take care of DMIC,
> > SoundWire, Google Chromebooks, open platforms, why not ACPI? It's just
> > one API to be used when more than one driver can register to the same
> > device.

> Well, currently intel-dsp-config sits in sound/hda, which isn't really
> intuitive.  Though, Intel driver file paths are already fairly
> scattered, so it doesn't matter too much :)

> I don't mind to move it to another directory, but which one...?
> x86 might match, but shuffling the place won't help for maintenance.

Yeah, I don't have a strong opinion here on location.  I'm not sure
there is any decision which won't have downsides.

> I personally find this move good, at least it makes things easier for
> distros.  There are small details like the above, but technically
> seen, I see no big problem.

Indeed - this isn't ideal but that's more a product of the situation
we're in, this seems to improve it so it seems like a win to me and the
distros and other people dealing with end users seem happy in so far as
they've spoken up.  Would you be OK with me applying the ALSA patches in
here?

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

Re: [PATCH 06/14] ASoC: Intel: byt/cht: set pm ops dynamically

[Pierre-Louis Bossart]

On 11/17/20 11:18 AM, Mark Brown wrote:
> On Thu, Nov 12, 2020 at 04:38:17PM -0600, Pierre-Louis Bossart wrote:
> 
>> +	/* set pm ops */
>> +	if (sof_parent)
>> +		pdev->dev.driver->pm = &snd_soc_pm_ops;
> 
> This might need revisiting in future since we should be able to have the
> driver PM ops be static const and hence unwritable but that's more of a
> robustness thing for the time being given that only a limited set of
> systems have this hardware and we know that there can't be multiple
> devices.

FWIW it's been done in other places, e.g.

drivers/net/wireless/ti/wlcore/main.c:  wl->dev->driver->pm = 
&wlcore_pm_ops;
drivers/net/wireless/ti/wlcore/main.c:  wl->dev->driver->pm = NULL;

The alternative would be to add an .ops whose callbacks conditionally 
call snd_soc_suspend/resume/poweroff. Not much cleaner, is it?

The check on the 'sof_parent' was not present in initial versions, I had 
an additional 'machine parameter' set by the SOF driver. But early 
reviewers suggested a check on the parent name was enough. It achieves 
the same thing in the end, make sure that we don't change anything for 
power management when the Atom/SST driver is used.

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Takashi Iwai]

On Tue, 17 Nov 2020 18:31:45 +0100,
Mark Brown wrote:
> 
> On Tue, Nov 17, 2020 at 03:04:31PM +0100, Takashi Iwai wrote:
> > Pierre-Louis Bossart wrote:
> 
> > > Why? Who said it was PCI only? We already take care of DMIC,
> > > SoundWire, Google Chromebooks, open platforms, why not ACPI? It's just
> > > one API to be used when more than one driver can register to the same
> > > device.
> 
> > Well, currently intel-dsp-config sits in sound/hda, which isn't really
> > intuitive.  Though, Intel driver file paths are already fairly
> > scattered, so it doesn't matter too much :)
> 
> > I don't mind to move it to another directory, but which one...?
> > x86 might match, but shuffling the place won't help for maintenance.
> 
> Yeah, I don't have a strong opinion here on location.  I'm not sure
> there is any decision which won't have downsides.
> 
> > I personally find this move good, at least it makes things easier for
> > distros.  There are small details like the above, but technically
> > seen, I see no big problem.
> 
> Indeed - this isn't ideal but that's more a product of the situation
> we're in, this seems to improve it so it seems like a win to me and the
> distros and other people dealing with end users seem happy in so far as
> they've spoken up.  Would you be OK with me applying the ALSA patches in
> here?

Yes, feel free to take my ack:
  Acked-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-17 3:04 PM, Takashi Iwai wrote:
> On Mon, 16 Nov 2020 18:47:22 +0100,
> Pierre-Louis Bossart wrote:
>>
>>> Explicit 'ifs' asking whether we're dealing with SOF or other solution
>>> is at best a code smell. I believe this is unnecessary complexity added
>>> to the code especially once you realize user needs to play with module
>>> parameters to switch between solutions. If we assume user is able to
>>> play with module parameters then why not simply make use of blacklist
>>> mechanism?
>>
>> Been there, done that. We don't want to use either denylist of kernel
>> parameters.
>>
>> denylists create confusion for users, it's an endless stream of false
>> errors and time lost in bug reports.
>>
>> The use of the kernel parameter is ONLY for expert users who want to
>> tinker with the system or debug issues, the average user should not
>> have to play with either denylists or kernel parameters.
> 
> I guess Cezary mean the modprobe blacklist?  This was used in the
> early stage of ASoC Skylake driver development, but in the end, it's
> more cumbersome because user needs to change multiple places.  The
> single module parameter was easier to handle.
> 

Thanks for joining the discussion, Takashi.

If the switch of solution for atom-based products is imminent, why add
code which becomes redundant soon after?

Yes, indeed I meant the modprobe blacklisting as it solves the problem
without addition of any code. Doubt alsa-driver entries are scattered in
/etc/modprobe.d/ so switching between one solution to another via
blacklist becomes as easy as changing 'options intel-dsp-config
<param>==<value>' entry.

In regard to catpt, solution is even simpler: just remove
sound/soc/sof/intel/bdw.c as that code is not valid & recommended
anyway and linux kernel is not place for such. There shouldn't be really
any options for not recommended stuff. Leave the selection explicit.

>>> And last but not least: intel-dsp-config is (as already stated) a mean
>>> for solving the HDA-runtime-driver-selection problem. Mixing it with
>>> ACPI devices is another layer of confusion.
>>
>> Why? Who said it was PCI only? We already take care of DMIC,
>> SoundWire, Google Chromebooks, open platforms, why not ACPI? It's just
>> one API to be used when more than one driver can register to the same
>> device.
> 
> Well, currently intel-dsp-config sits in sound/hda, which isn't really
> intuitive.  Though, Intel driver file paths are already fairly
> scattered, so it doesn't matter too much :)
> 
> I don't mind to move it to another directory, but which one...?
> x86 might match, but shuffling the place won't help for maintenance.
> 
> I personally find this move good, at least it makes things easier for
> distros.  There are small details like the above, but technically
> seen, I see no big problem.

Well, if non-Intel guys see the localization of code counter-intuitive
then how about those who play with it daily..
The new "sof-parent" checks won't make maintaining any easier and I
believe there are easier solutions as written above.

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Pierre-Louis Bossart]

On 11/17/20 4:13 PM, Rojewski, Cezary wrote:
> On 2020-11-17 3:04 PM, Takashi Iwai wrote:
>> On Mon, 16 Nov 2020 18:47:22 +0100,
>> Pierre-Louis Bossart wrote:
>>>
>>>> Explicit 'ifs' asking whether we're dealing with SOF or other solution
>>>> is at best a code smell. I believe this is unnecessary complexity added
>>>> to the code especially once you realize user needs to play with module
>>>> parameters to switch between solutions. If we assume user is able to
>>>> play with module parameters then why not simply make use of blacklist
>>>> mechanism?
>>>
>>> Been there, done that. We don't want to use either denylist of kernel
>>> parameters.
>>>
>>> denylists create confusion for users, it's an endless stream of false
>>> errors and time lost in bug reports.
>>>
>>> The use of the kernel parameter is ONLY for expert users who want to
>>> tinker with the system or debug issues, the average user should not
>>> have to play with either denylists or kernel parameters.
>>
>> I guess Cezary mean the modprobe blacklist?  This was used in the
>> early stage of ASoC Skylake driver development, but in the end, it's
>> more cumbersome because user needs to change multiple places.  The
>> single module parameter was easier to handle.
>>
> 
> Thanks for joining the discussion, Takashi.
> 
> If the switch of solution for atom-based products is imminent, why add
> code which becomes redundant soon after?

To be clear: there is *no plan* to *remove* the Atom/sst code any time 
'soon', only to *deprecate* it.

In the best case distributions would transition in 2021. Some distros 
are faster than others, neither you nor I have any control over this.

Removing code from the kernel is not something we can do unless there is 
demonstrated evidence that the number of impacted users is close to zero 
and distributions no longer support that code. The case of Baytrail 
legacy is telling, you removed it earlier this Fall but after a 
recommended alternative was provided for more than 3 years.

Again, there is no planned 'switch' but a gradual transition, and that 
patchset helps with the transition.

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Takashi Iwai]

On Tue, 17 Nov 2020 23:13:13 +0100,
Rojewski, Cezary wrote:
> 
> On 2020-11-17 3:04 PM, Takashi Iwai wrote:
> > On Mon, 16 Nov 2020 18:47:22 +0100,
> > Pierre-Louis Bossart wrote:
> >>
> >>> Explicit 'ifs' asking whether we're dealing with SOF or other solution
> >>> is at best a code smell. I believe this is unnecessary complexity added
> >>> to the code especially once you realize user needs to play with module
> >>> parameters to switch between solutions. If we assume user is able to
> >>> play with module parameters then why not simply make use of blacklist
> >>> mechanism?
> >>
> >> Been there, done that. We don't want to use either denylist of kernel
> >> parameters.
> >>
> >> denylists create confusion for users, it's an endless stream of false
> >> errors and time lost in bug reports.
> >>
> >> The use of the kernel parameter is ONLY for expert users who want to
> >> tinker with the system or debug issues, the average user should not
> >> have to play with either denylists or kernel parameters.
> > 
> > I guess Cezary mean the modprobe blacklist?  This was used in the
> > early stage of ASoC Skylake driver development, but in the end, it's
> > more cumbersome because user needs to change multiple places.  The
> > single module parameter was easier to handle.
> > 
> 
> Thanks for joining the discussion, Takashi.
> 
> If the switch of solution for atom-based products is imminent, why add
> code which becomes redundant soon after?
> 
> Yes, indeed I meant the modprobe blacklisting as it solves the problem
> without addition of any code. Doubt alsa-driver entries are scattered in
> /etc/modprobe.d/ so switching between one solution to another via
> blacklist becomes as easy as changing 'options intel-dsp-config
> <param>==<value>' entry.

Ideally blacklist would work well, but practically it can be more
problematic.  When you *switch* between multiple drivers via
blacklist, you'll have to mask one of them while keeping another
untouched, so either:
  blacklist A
or
  blacklist B

Now, imagine that distro sets "blacklist A" to choose B as the
default.  What user has to do?  They have to modify "blacklist A"
line with "blacklist B".  But it can't be done with an additional
modprobe.d/*.config file; otherwise this blacklist remains.  It means
they have to scratch the system configuration file itself -- which
might be again overridden by a package update or whatever.

This will be more complex if there are more than three choices, of
course.

Admittedly, the situation with the system config file be same for
module option if distro sets the option in modprobe.d/*, too.  But,
there is another difference: the default option value can be set in
the kernel code, while the blacklist approach is to let all open and
choose via blacklist.  IOW, devs have some control for choosing the
default value for the module option but for blacklist they are all
done by user-space side.


> In regard to catpt, solution is even simpler: just remove
> sound/soc/sof/intel/bdw.c as that code is not valid & recommended
> anyway and linux kernel is not place for such. There shouldn't be really
> any options for not recommended stuff. Leave the selection explicit.
> 
> >>> And last but not least: intel-dsp-config is (as already stated) a mean
> >>> for solving the HDA-runtime-driver-selection problem. Mixing it with
> >>> ACPI devices is another layer of confusion.
> >>
> >> Why? Who said it was PCI only? We already take care of DMIC,
> >> SoundWire, Google Chromebooks, open platforms, why not ACPI? It's just
> >> one API to be used when more than one driver can register to the same
> >> device.
> > 
> > Well, currently intel-dsp-config sits in sound/hda, which isn't really
> > intuitive.  Though, Intel driver file paths are already fairly
> > scattered, so it doesn't matter too much :)
> > 
> > I don't mind to move it to another directory, but which one...?
> > x86 might match, but shuffling the place won't help for maintenance.
> > 
> > I personally find this move good, at least it makes things easier for
> > distros.  There are small details like the above, but technically
> > seen, I see no big problem.
> 
> Well, if non-Intel guys see the localization of code counter-intuitive
> then how about those who play with it daily..

I play it and maintain it daily, that's why I find unintuitive :)
I guess most users don't notice the file path, as the module loading
or option is done only by the module name.

> The new "sof-parent" checks won't make maintaining any easier and I
> believe there are easier solutions as written above.

If you find a good way to overcome the disadvantage, that's great.
Let's see.


thanks,

Takashi

Re: [PATCH 06/14] ASoC: Intel: byt/cht: set pm ops dynamically

[Mark Brown]

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

On Tue, Nov 17, 2020 at 11:39:48AM -0600, Pierre-Louis Bossart wrote:
> On 11/17/20 11:18 AM, Mark Brown wrote:
> > On Thu, Nov 12, 2020 at 04:38:17PM -0600, Pierre-Louis Bossart wrote:

> > > +	/* set pm ops */
> > > +	if (sof_parent)
> > > +		pdev->dev.driver->pm = &snd_soc_pm_ops;

> > This might need revisiting in future since we should be able to have the
> > driver PM ops be static const and hence unwritable but that's more of a
> > robustness thing for the time being given that only a limited set of
> > systems have this hardware and we know that there can't be multiple
> > devices.

> FWIW it's been done in other places, e.g.

> drivers/net/wireless/ti/wlcore/main.c:  wl->dev->driver->pm =
> &wlcore_pm_ops;
> drivers/net/wireless/ti/wlcore/main.c:  wl->dev->driver->pm = NULL;

> The alternative would be to add an .ops whose callbacks conditionally call
> snd_soc_suspend/resume/poweroff. Not much cleaner, is it?

It's not really about cleanliness, it's about being able to mark the ops
struct as const and therefore make it read only which the security
people like.

> The check on the 'sof_parent' was not present in initial versions, I had an
> additional 'machine parameter' set by the SOF driver. But early reviewers
> suggested a check on the parent name was enough. It achieves the same thing
> in the end, make sure that we don't change anything for power management
> when the Atom/SST driver is used.

The check is fine.

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

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-17 11:53 PM, Pierre-Louis Bossart wrote:
> On 11/17/20 4:13 PM, Rojewski, Cezary wrote:
>> On 2020-11-17 3:04 PM, Takashi Iwai wrote:
...

>>> I guess Cezary mean the modprobe blacklist?  This was used in the
>>> early stage of ASoC Skylake driver development, but in the end, it's
>>> more cumbersome because user needs to change multiple places.  The
>>> single module parameter was easier to handle.
>>>
>>
>> Thanks for joining the discussion, Takashi.
>>
>> If the switch of solution for atom-based products is imminent, why add
>> code which becomes redundant soon after?
> 
> To be clear: there is *no plan* to *remove* the Atom/sst code any time 
> 'soon', only to *deprecate* it.
> 
> In the best case distributions would transition in 2021. Some distros 
> are faster than others, neither you nor I have any control over this.
> 
> Removing code from the kernel is not something we can do unless there is 
> demonstrated evidence that the number of impacted users is close to zero 
> and distributions no longer support that code. The case of Baytrail 
> legacy is telling, you removed it earlier this Fall but after a 
> recommended alternative was provided for more than 3 years.
> 
> Again, there is no planned 'switch' but a gradual transition, and that 
> patchset helps with the transition.

Hmm, then maybe I misunderstood Hans. Given his feedback it seemed like
Fedora is about to switch to SOF right now.

Indeed, before I've sent patches removing Baytrail, basically every
support-team had been asked about its usage and the answers were
negative. /atom/ was covering basically every case anyway like you
pointed out so /baytrail/ solution felt more like a duplication.

As SOF is the desired solution for atom-based products, I can see the
need for some sort of selection mechanism. The same cannot be said for
hsw/bdw though. Let's leave catpt out this, shall we?

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Pierre-Louis Bossart]

>>>> I guess Cezary mean the modprobe blacklist?  This was used in the
>>>> early stage of ASoC Skylake driver development, but in the end, it's
>>>> more cumbersome because user needs to change multiple places.  The
>>>> single module parameter was easier to handle.
>>>>
>>>
>>> Thanks for joining the discussion, Takashi.
>>>
>>> If the switch of solution for atom-based products is imminent, why add
>>> code which becomes redundant soon after?
>>
>> To be clear: there is *no plan* to *remove* the Atom/sst code any time
>> 'soon', only to *deprecate* it.
>>
>> In the best case distributions would transition in 2021. Some distros
>> are faster than others, neither you nor I have any control over this.
>>
>> Removing code from the kernel is not something we can do unless there is
>> demonstrated evidence that the number of impacted users is close to zero
>> and distributions no longer support that code. The case of Baytrail
>> legacy is telling, you removed it earlier this Fall but after a
>> recommended alternative was provided for more than 3 years.
>>
>> Again, there is no planned 'switch' but a gradual transition, and that
>> patchset helps with the transition.
> 
> Hmm, then maybe I misunderstood Hans. Given his feedback it seemed like
> Fedora is about to switch to SOF right now.
> 
> Indeed, before I've sent patches removing Baytrail, basically every
> support-team had been asked about its usage and the answers were
> negative. /atom/ was covering basically every case anyway like you
> pointed out so /baytrail/ solution felt more like a duplication.
> 
> As SOF is the desired solution for atom-based products, I can see the
> need for some sort of selection mechanism. The same cannot be said for
> hsw/bdw though. Let's leave catpt out this, shall we?

It helps everyone to have a single build, e.g. 'make allmodconfig' or 
'make allyesconfig' would select all possible drivers and bots can run wild.

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-18 8:49 AM, Takashi Iwai wrote:
> On Tue, 17 Nov 2020 23:13:13 +0100, Rojewski, Cezary wrote:

...

>>
>> Thanks for joining the discussion, Takashi.
>>
>> If the switch of solution for atom-based products is imminent, why add
>> code which becomes redundant soon after?
>>
>> Yes, indeed I meant the modprobe blacklisting as it solves the problem
>> without addition of any code. Doubt alsa-driver entries are scattered in
>> /etc/modprobe.d/ so switching between one solution to another via
>> blacklist becomes as easy as changing 'options intel-dsp-config
>> <param>==<value>' entry.
> 
> Ideally blacklist would work well, but practically it can be more
> problematic.  When you *switch* between multiple drivers via
> blacklist, you'll have to mask one of them while keeping another
> untouched, so either:
>    blacklist A
> or
>    blacklist B
> 
> Now, imagine that distro sets "blacklist A" to choose B as the
> default.  What user has to do?  They have to modify "blacklist A"
> line with "blacklist B".  But it can't be done with an additional
> modprobe.d/*.config file; otherwise this blacklist remains.  It means
> they have to scratch the system configuration file itself -- which
> might be again overridden by a package update or whatever.
> 
> This will be more complex if there are more than three choices, of
> course.
> 
> Admittedly, the situation with the system config file be same for
> module option if distro sets the option in modprobe.d/*, too.  But,
> there is another difference: the default option value can be set in
> the kernel code, while the blacklist approach is to let all open and
> choose via blacklist.  IOW, devs have some control for choosing the
> default value for the module option but for blacklist they are all
> done by user-space side.
> 

I agree, module param is ultimately easier to handle than denylist. The
reason I had mentioned that is: if user is capable of changing value for
module param, then we might as well assume he or she is the experienced
one and playing with denylist isn't a problem either.

And hopefully we don't reach a point in time where again 3 flavours for
atom-based products are available : )

>> In regard to catpt, solution is even simpler: just remove
>> sound/soc/sof/intel/bdw.c as that code is not valid & recommended
>> anyway and linux kernel is not place for such. There shouldn't be really
>> any options for not recommended stuff. Leave the selection explicit.
>>

...

>> Well, if non-Intel guys see the localization of code counter-intuitive
>> then how about those who play with it daily..
> 
> I play it and maintain it daily, that's why I find unintuitive :)
> I guess most users don't notice the file path, as the module loading
> or option is done only by the module name.
> 

Perhaps I misworded my previous statement. What I meant is: you don't
have access to all the stuff we - Intel employees - have like specs,
firmware documentation, hardware specifics and yet you see the problem.
And this tells me there's still a lot to be done.

>> The new "sof-parent" checks won't make maintaining any easier and I
>> believe there are easier solutions as written above.
> 
> If you find a good way to overcome the disadvantage, that's great.
> Let's see.

Well, the disadvantage is: weight of maintenance of newly added code.
All in all, as mentioned in other reply, we could settle with selection
mechanism for atom while leaving hsw/bdw out of it.

Czarek

Re: [PATCH 13/14] ASoC: SOF: Intel: allow for coexistence between SOF and catpt drivers

[Mark Brown]

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

On Thu, Nov 12, 2020 at 04:38:24PM -0600, Pierre-Louis Bossart wrote:
> Now that we have all the support needed for coexistence between ACPI
> drivers for Broadwell, remove mutual exclusion in the Kconfig
> file. The selection is done by playing with the snd_intel_dspcfg
> module 'dsp_driver' parameter.

This breaks x86 allmodconfig builds for me:

/mnt/kernel/sound/soc/intel/atom/sst/sst_acpi.c: In function 'sst_acpi_probe':
/mnt/kernel/sound/soc/intel/atom/sst/sst_acpi.c:251:8: error: implicit declaration of function 'snd_intel_acpi_dsp_driver_probe'; did you mean 'snd_intel_dsp_driver_probe'? [-Werror=implicit-function-declaration]
  ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        snd_intel_dsp_driver_probe
/mnt/kernel/sound/soc/intel/catpt/device.c: In function 'catpt_acpi_probe':
/mnt/kernel/sound/soc/intel/catpt/device.c:251:8: error: implicit declaration of function 'snd_intel_acpi_dsp_driver_probe'; did you mean 'snd_intel_dsp_driver_probe'? [-Werror=implicit-function-declaration]
  ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        snd_intel_dsp_driver_probe
/mnt/kernel/sound/soc/sof/sof-acpi-dev.c: In function 'sof_acpi_probe':
/mnt/kernel/sound/soc/sof/sof-acpi-dev.c:134:8: error: implicit declaration of function 'snd_intel_acpi_dsp_driver_probe'; did you mean 'snd_intel_dsp_driver_probe'? [-Werror=implicit-function-declaration]
  ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        snd_intel_dsp_driver_probe


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

Re: [PATCH 13/14] ASoC: SOF: Intel: allow for coexistence between SOF and catpt drivers

[Pierre-Louis Bossart]

On 11/19/20 8:06 AM, Mark Brown wrote:
> On Thu, Nov 12, 2020 at 04:38:24PM -0600, Pierre-Louis Bossart wrote:
>> Now that we have all the support needed for coexistence between ACPI
>> drivers for Broadwell, remove mutual exclusion in the Kconfig
>> file. The selection is done by playing with the snd_intel_dspcfg
>> module 'dsp_driver' parameter.
> 
> This breaks x86 allmodconfig builds for me:
> 
> /mnt/kernel/sound/soc/intel/atom/sst/sst_acpi.c: In function 'sst_acpi_probe':
> /mnt/kernel/sound/soc/intel/atom/sst/sst_acpi.c:251:8: error: implicit declaration of function 'snd_intel_acpi_dsp_driver_probe'; did you mean 'snd_intel_dsp_driver_probe'? [-Werror=implicit-function-declaration]
>    ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          snd_intel_dsp_driver_probe
> /mnt/kernel/sound/soc/intel/catpt/device.c: In function 'catpt_acpi_probe':
> /mnt/kernel/sound/soc/intel/catpt/device.c:251:8: error: implicit declaration of function 'snd_intel_acpi_dsp_driver_probe'; did you mean 'snd_intel_dsp_driver_probe'? [-Werror=implicit-function-declaration]
>    ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          snd_intel_dsp_driver_probe
> /mnt/kernel/sound/soc/sof/sof-acpi-dev.c: In function 'sof_acpi_probe':
> /mnt/kernel/sound/soc/sof/sof-acpi-dev.c:134:8: error: implicit declaration of function 'snd_intel_acpi_dsp_driver_probe'; did you mean 'snd_intel_dsp_driver_probe'? [-Werror=implicit-function-declaration]
>    ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          snd_intel_dsp_driver_probe
> 

Humm, I just tried and it works fine for me on top of your for-5.11 
branch. That error across the board seems weird, there's even a 
fall-back with a static inline if the Kconfig is not selected.

Could it be that Patch3 was not applied somehow? That's where the 
prototype was added.

Re: [PATCH 13/14] ASoC: SOF: Intel: allow for coexistence between SOF and catpt drivers

[Mark Brown]

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

On Thu, Nov 19, 2020 at 11:52:22AM -0600, Pierre-Louis Bossart wrote:

> Humm, I just tried and it works fine for me on top of your for-5.11 branch.
> That error across the board seems weird, there's even a fall-back with a
> static inline if the Kconfig is not selected.

> Could it be that Patch3 was not applied somehow? That's where the prototype
> was added.

Looks like the patches got reordered because I had to manually add
Takashi's acks since he added some spaces which confused b4.

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

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-18 9:25 PM, Pierre-Louis Bossart wrote:

...

>>
>> Hmm, then maybe I misunderstood Hans. Given his feedback it seemed like
>> Fedora is about to switch to SOF right now.
>>
>> Indeed, before I've sent patches removing Baytrail, basically every
>> support-team had been asked about its usage and the answers were
>> negative. /atom/ was covering basically every case anyway like you
>> pointed out so /baytrail/ solution felt more like a duplication.
>>
>> As SOF is the desired solution for atom-based products, I can see the
>> need for some sort of selection mechanism. The same cannot be said for
>> hsw/bdw though. Let's leave catpt out this, shall we?
> 
> It helps everyone to have a single build, e.g. 'make allmodconfig' or 
> 'make allyesconfig' would select all possible drivers and bots can run 
> wild.
> 

Why should bots care about not recommended code?
I'm against adding external dependency (intel-dsp-config) for
catpt for reasons I'd mentioned several times already.

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

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

On Fri, Nov 20, 2020 at 03:40:21PM +0000, Rojewski, Cezary wrote:
> On 2020-11-18 9:25 PM, Pierre-Louis Bossart wrote:

> > It helps everyone to have a single build, e.g. 'make allmodconfig' or 
> > 'make allyesconfig' would select all possible drivers and bots can run 
> > wild.

> Why should bots care about not recommended code?
> I'm against adding external dependency (intel-dsp-config) for
> catpt for reasons I'd mentioned several times already.

People care about any code that's in the kernel, especially people doing
anything treewide.  The fewer configurations people need to build to get
code coverage the better.

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

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-20 5:48 PM, Mark Brown wrote:
> On Fri, Nov 20, 2020 at 03:40:21PM +0000, Rojewski, Cezary wrote:
>> On 2020-11-18 9:25 PM, Pierre-Louis Bossart wrote:
> 
>>> It helps everyone to have a single build, e.g. 'make allmodconfig' or
>>> 'make allyesconfig' would select all possible drivers and bots can run
>>> wild.
> 
>> Why should bots care about not recommended code?
>> I'm against adding external dependency (intel-dsp-config) for
>> catpt for reasons I'd mentioned several times already.
> 
> People care about any code that's in the kernel, especially people doing
> anything treewide.  The fewer configurations people need to build to get
> code coverage the better.
> 

Sure, but in this particular case there really shouldn't be "another
option". If catpt is the sole option, why add intel-dsp-config
dependency? The alternative shouldn't even exist in the kernel and be
instead removed just like /haswell/ and /baytrail/ were.

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

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

On Fri, Nov 20, 2020 at 05:10:30PM +0000, Rojewski, Cezary wrote:
> On 2020-11-20 5:48 PM, Mark Brown wrote:

> > People care about any code that's in the kernel, especially people doing
> > anything treewide.  The fewer configurations people need to build to get
> > code coverage the better.

> Sure, but in this particular case there really shouldn't be "another
> option". If catpt is the sole option, why add intel-dsp-config
> dependency? The alternative shouldn't even exist in the kernel and be
> instead removed just like /haswell/ and /baytrail/ were.

If all the alternatives actually get removed then there'd be no need for
it, while they're there it is reasonable to have it - it does make it
easier for people like distros to try converting, it means they can
deploy the recommended setup without needing to ship new binaries to
people who run into trouble.  Besides TBH while there's several DSP
implementations in the tree having the code there makes it obvious that
this case works the same way as all the others to anyone looking at the
code.

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

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-20 7:06 PM, Mark Brown wrote:
> On Fri, Nov 20, 2020 at 05:10:30PM +0000, Rojewski, Cezary wrote:
>> On 2020-11-20 5:48 PM, Mark Brown wrote:
> 
>>> People care about any code that's in the kernel, especially people doing
>>> anything treewide.  The fewer configurations people need to build to get
>>> code coverage the better.
> 
>> Sure, but in this particular case there really shouldn't be "another
>> option". If catpt is the sole option, why add intel-dsp-config
>> dependency? The alternative shouldn't even exist in the kernel and be
>> instead removed just like /haswell/ and /baytrail/ were.
> 
> If all the alternatives actually get removed then there'd be no need for
> it, while they're there it is reasonable to have it - it does make it
> easier for people like distros to try converting, it means they can
> deploy the recommended setup without needing to ship new binaries to
> people who run into trouble.  Besides TBH while there's several DSP
> implementations in the tree having the code there makes it obvious that
> this case works the same way as all the others to anyone looking at the
> code.

I can understand that in atom's case. That's why I'm fine with the
section mechanism being applied there. At the beginning I'd thought SOF
is already prepared to take over /atom/. As that's not the case, to ease
the transition, dynamic switch could prove useful.

There are no circumstances under which Intel recommends distros to try
to convert out of catpt though. Don't believe aligning all the drivers
to some general idea just for the sake of aligning is a good move.
That's why drivers have their own specifics in the first place -
their complexity and performance could have been negatively impacted
otherwise.

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

On Thu, 12 Nov 2020 16:38:11 -0600, Pierre-Louis Bossart wrote:
> The module snd-intel-dspcfg, suggested by Jaroslav last year,
> currently provide the means to select a PCI driver at run-time, based
> on quirks, recommendations or user selection via a kernel
> parameter. This capability removed a lot of confusions in
> distributions and removed the need for recompilations to select legacy
> HDaudio, SST or SOF drivers.
> 
> [...]

Applied to

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

Thanks!

[01/14] ASoC: Intel: broadwell: add missing pm_ops
        commit: 7998c168a94de9c593ab07455924e827ad5f1bd7
[02/14] ASoC: Intel: bdw-rt5677: add missing pm_ops
        commit: cf7f4a5320cda6fc533ae96601b4ce767d1af0f8
[03/14] ALSA: hda: intel-dsp-config: add helper for ACPI DSP driver selection
        commit: b5682305297db24b456e55ba209574cb8f9318f9
[04/14] ASoC: soc-acpi: add helper to identify parent driver.
        commit: 644eebdbbf1154c995d6319c133d7d5b898c5ed2
[05/14] ASoC: Intel: boards: byt/cht: set card and driver name at run time
        commit: 41656c3dc2acfe2aef3d7c4e1cd2b92f49b6e3a7
[06/14] ASoC: Intel: byt/cht: set pm ops dynamically
        commit: 05ff312badb6079f18c0b05d89e21733a9dafe32
[07/14] ASoC: SOF: acpi: add dynamic selection of DSP driver
        commit: f7313f9fc28781ad0801d8b9c692222445e664ca
[08/14] ASoC: Intel: Atom: add dynamic selection of DSP driver
        commit: df5f5edaef4b653fa731dcf3753e71766f95c2cd
[09/14] ASoC: SOF: Intel: allow for coexistence between SOF and Atom/SST drivers
        commit: b405b4318c77db061fdf1c8c4b9329ea30e807ee
[10/14] ALSA: hda: intel-dsp-config: add Broadwell ACPI DSP driver selection
        commit: 803e591337e6f7953350e0f56284ebbabb600808
[11/14] ASoC: Intel: broadwell: set card and driver name dynamically
        commit: 8643e85aab878fe0d8031ae4622b40cfb78d4172
[12/14] ASoC: Intel: catpt: add dynamic selection of DSP driver
        commit: ec8a15d3a7c7d6e9acd2e0637d2020ac17fb7820
[13/14] ASoC: SOF: Intel: allow for coexistence between SOF and catpt drivers
        commit: d512ef22d77b0779e9b0e9a91a63b291357079f9
[14/14] ALSA: hda: intel-dsp-config: ignore dsp_driver parameter for PCI legacy devices
        commit: 0e5cc22162e55c19255f4e25dadf9fda76eac11c

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

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

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

On Fri, Nov 20, 2020 at 09:02:24PM +0000, Rojewski, Cezary wrote:

> There are no circumstances under which Intel recommends distros to try
> to convert out of catpt though. Don't believe aligning all the drivers
> to some general idea just for the sake of aligning is a good move.
> That's why drivers have their own specifics in the first place -
> their complexity and performance could have been negatively impacted
> otherwise.

It could equally be that someone has stuck with the older, now
deprecated, implementations due to compatibility fears and this could
help them deploy the catpt implementation without worrying so much about
breaking things for users.

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

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-23 6:35 PM, Mark Brown wrote:
> On Fri, Nov 20, 2020 at 09:02:24PM +0000, Rojewski, Cezary wrote:
> 
>> There are no circumstances under which Intel recommends distros to try
>> to convert out of catpt though. Don't believe aligning all the drivers
>> to some general idea just for the sake of aligning is a good move.
>> That's why drivers have their own specifics in the first place -
>> their complexity and performance could have been negatively impacted
>> otherwise.
> 
> It could equally be that someone has stuck with the older, now
> deprecated, implementations due to compatibility fears and this could
> help them deploy the catpt implementation without worrying so much about
> breaking things for users.
> 

Except that it (i.e.: patchset) doesn't touch old _HASWELL kconfig at
all as the code behind it is already removed.

Believe we are desync'ed here.

What the patchset presents catpt vs SOF. /sof/ runs through SOF firmware
so it cannot be account as old-implementation. It's a mix of not
recommended fw + incorrect sw flow. As old /haswell/ is no more, there
is no worrying about catpt deployment - it's your only option. As there
is no userspace involved (lack of topology files), base firmware binary
remains the same and amixer kcontrols behave 1:1 when compared to its
predecessor, compatibility is left intact.

That's exactly why we should be explicit in driver selection. Pretty
sure hsw/bdw case is still mistakenly addressed to as if it was
atom-based platform.

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

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

On Tue, Nov 24, 2020 at 11:56:36AM +0000, Rojewski, Cezary wrote:

> What the patchset presents catpt vs SOF. /sof/ runs through SOF firmware
> so it cannot be account as old-implementation. It's a mix of not
> recommended fw + incorrect sw flow. As old /haswell/ is no more, there
> is no worrying about catpt deployment - it's your only option. As there
> is no userspace involved (lack of topology files), base firmware binary
> remains the same and amixer kcontrols behave 1:1 when compared to its
> predecessor, compatibility is left intact.

> That's exactly why we should be explicit in driver selection. Pretty
> sure hsw/bdw case is still mistakenly addressed to as if it was
> atom-based platform.

It's not just the userspace interface that worries people, it's also any
board specific quirks that might turn up.  A good chunk of the work with
x86 sound support is quirking around platform specifics - look at all
the patches Hans sends for example.  In an ideal world this would just
be people worrying too much but the general history with getting generic
code working well on a wide range of x86 hardware it's hard to blame
anyone for being conservative about substantial changes in the software
stack.

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

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Takashi Iwai]

On Tue, 24 Nov 2020 15:01:19 +0100,
Mark Brown wrote:
> 
> On Tue, Nov 24, 2020 at 11:56:36AM +0000, Rojewski, Cezary wrote:
> 
> > What the patchset presents catpt vs SOF. /sof/ runs through SOF firmware
> > so it cannot be account as old-implementation. It's a mix of not
> > recommended fw + incorrect sw flow. As old /haswell/ is no more, there
> > is no worrying about catpt deployment - it's your only option. As there
> > is no userspace involved (lack of topology files), base firmware binary
> > remains the same and amixer kcontrols behave 1:1 when compared to its
> > predecessor, compatibility is left intact.
> 
> > That's exactly why we should be explicit in driver selection. Pretty
> > sure hsw/bdw case is still mistakenly addressed to as if it was
> > atom-based platform.
> 
> It's not just the userspace interface that worries people, it's also any
> board specific quirks that might turn up.  A good chunk of the work with
> x86 sound support is quirking around platform specifics - look at all
> the patches Hans sends for example.  In an ideal world this would just
> be people worrying too much but the general history with getting generic
> code working well on a wide range of x86 hardware it's hard to blame
> anyone for being conservative about substantial changes in the software
> stack.

I guess Cezary's point is that CATPT is the only driver for Haswell,
hence the intel-dsp-config is useless for it.

But I thought CATPT also covers Broadwell, and Broadwell can be
supported by both CATPT and SOF?  If so, the dynamic switching makes
sense.


Takashi

RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Rojewski, Cezary]

On 2020-11-24 3:15 PM, Takashi Iwai wrote:
> On Tue, 24 Nov 2020 15:01:19 +0100, Mark Brown wrote:
>>
>> On Tue, Nov 24, 2020 at 11:56:36AM +0000, Rojewski, Cezary wrote:
>>
>>> What the patchset presents catpt vs SOF. /sof/ runs through SOF firmware
>>> so it cannot be account as old-implementation. It's a mix of not
>>> recommended fw + incorrect sw flow. As old /haswell/ is no more, there
>>> is no worrying about catpt deployment - it's your only option. As there
>>> is no userspace involved (lack of topology files), base firmware binary
>>> remains the same and amixer kcontrols behave 1:1 when compared to its
>>> predecessor, compatibility is left intact.
>>
>>> That's exactly why we should be explicit in driver selection. Pretty
>>> sure hsw/bdw case is still mistakenly addressed to as if it was
>>> atom-based platform.
>>
>> It's not just the userspace interface that worries people, it's also any
>> board specific quirks that might turn up.  A good chunk of the work with
>> x86 sound support is quirking around platform specifics - look at all
>> the patches Hans sends for example.  In an ideal world this would just
>> be people worrying too much but the general history with getting generic
>> code working well on a wide range of x86 hardware it's hard to blame
>> anyone for being conservative about substantial changes in the software
>> stack.

Mark, there is not a single word I don't agree with in your statement.

In regard to quirks - I was surprised how much detail Hans found out
regarding atom platforms. That's a lot of good input. And that's
probably one of the key reasons why atom is properly supported in linux.

My point has more "basic" nature.
 
> I guess Cezary's point is that CATPT is the only driver for Haswell,
> hence the intel-dsp-config is useless for it.

This! and..

> But I thought CATPT also covers Broadwell, and Broadwell can be
> supported by both CATPT and SOF?  If so, the dynamic switching makes
> sense.

..more. Dynamic selection made sense if you're in transition period as
it is the case for atoms. There is no transition period for hsw/bdw. BDW
as "supported" by SOF would be a strong claim. There is no commitment
and Intel does not recommend using it for hsw/bdw for any scenario. And
as such, selection-subject does not apply here.

Believe removal of /sof/intel/bdw.c is in order?

Czarek

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

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

On Tue, Nov 24, 2020 at 03:15:18PM +0100, Takashi Iwai wrote:

> I guess Cezary's point is that CATPT is the only driver for Haswell,
> hence the intel-dsp-config is useless for it.

> But I thought CATPT also covers Broadwell, and Broadwell can be
> supported by both CATPT and SOF?  If so, the dynamic switching makes
> sense.

Right, like I said previously if there's only one option upstream then
we can drop this stuff entirely - like you say I understood that there
were some SoCs that could use both.

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

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Mark Brown]

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

On Tue, Nov 24, 2020 at 04:07:19PM +0000, Rojewski, Cezary wrote:

> Believe removal of /sof/intel/bdw.c is in order?

Right, like I've said several times if we can remove all the other
support then we can remove the conditional code.

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

Re: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

[Takashi Iwai]

On Tue, 24 Nov 2020 17:07:19 +0100,
Rojewski, Cezary wrote:
> 
> On 2020-11-24 3:15 PM, Takashi Iwai wrote:
> > On Tue, 24 Nov 2020 15:01:19 +0100, Mark Brown wrote:
> >>
> >> On Tue, Nov 24, 2020 at 11:56:36AM +0000, Rojewski, Cezary wrote:
> >>
> >>> What the patchset presents catpt vs SOF. /sof/ runs through SOF firmware
> >>> so it cannot be account as old-implementation. It's a mix of not
> >>> recommended fw + incorrect sw flow. As old /haswell/ is no more, there
> >>> is no worrying about catpt deployment - it's your only option. As there
> >>> is no userspace involved (lack of topology files), base firmware binary
> >>> remains the same and amixer kcontrols behave 1:1 when compared to its
> >>> predecessor, compatibility is left intact.
> >>
> >>> That's exactly why we should be explicit in driver selection. Pretty
> >>> sure hsw/bdw case is still mistakenly addressed to as if it was
> >>> atom-based platform.
> >>
> >> It's not just the userspace interface that worries people, it's also any
> >> board specific quirks that might turn up.  A good chunk of the work with
> >> x86 sound support is quirking around platform specifics - look at all
> >> the patches Hans sends for example.  In an ideal world this would just
> >> be people worrying too much but the general history with getting generic
> >> code working well on a wide range of x86 hardware it's hard to blame
> >> anyone for being conservative about substantial changes in the software
> >> stack.
> 
> Mark, there is not a single word I don't agree with in your statement.
> 
> In regard to quirks - I was surprised how much detail Hans found out
> regarding atom platforms. That's a lot of good input. And that's
> probably one of the key reasons why atom is properly supported in linux.
> 
> My point has more "basic" nature.
>  
> > I guess Cezary's point is that CATPT is the only driver for Haswell,
> > hence the intel-dsp-config is useless for it.
> 
> This! and..
> 
> > But I thought CATPT also covers Broadwell, and Broadwell can be
> > supported by both CATPT and SOF?  If so, the dynamic switching makes
> > sense.
> 
> ..more. Dynamic selection made sense if you're in transition period as
> it is the case for atoms. There is no transition period for hsw/bdw. BDW
> as "supported" by SOF would be a strong claim. There is no commitment
> and Intel does not recommend using it for hsw/bdw for any scenario. And
> as such, selection-subject does not apply here.
> 
> Believe removal of /sof/intel/bdw.c is in order?

This requires the agreement rather in Intel side at first.

The upstream will follow that decision, and eventually drop the
dynamic selection for HSW/BDW, too.


Takashi

Re: [PATCH 05/14] ASoC: Intel: boards: byt/cht: set card and driver name at run time

[youling257]

What`s the mean? how to use kernel parameter at run time?

Re: [PATCH 05/14] ASoC: Intel: boards: byt/cht: set card and driver name at run time

[Pierre-Louis Bossart]

On 4/25/21 1:13 PM, youling257 wrote:
> What`s the mean? how to use kernel parameter at run time?

see the options for the snd-intel-dspcfg module

For byt/cht, if you use dsp_driver=3 you will select SOF. if you use 
dsp_driver=2 or don't set the value at all, you will select the legacy 
Atom/sst driver, which is still the default