LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2 0/2] ASoC: Intel: Add Cometlake PCI IDs
@ 2019-05-07 21:53 Evan Green
  2019-05-07 21:53 ` [PATCH v2 1/2] ASoC: SOF: Add Comet Lake " Evan Green
  2019-05-07 21:53 ` [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake " Evan Green
  0 siblings, 2 replies; 11+ messages in thread
From: Evan Green @ 2019-05-07 21:53 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: Pierre-Louis Bossart, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Evan Green, Jaroslav Kysela, alsa-devel,
	Rakesh Ughreja, Guenter Roeck, linux-kernel, Yu Zhao,
	Takashi Iwai, Jenny TC, Liam Girdwood, Jie Yang


This small series adds PCI IDs for Cometlake platforms, for a
dazzling audio experience.

Changes in v2:
- Add CML-H ID 0x06c8 (Pierre-Louis)
- Add 0x06c8 for CML-H (Pierre-Louis)

Evan Green (2):
  ASoC: SOF: Add Comet Lake PCI IDs
  ASoC: Intel: Skylake: Add Cometlake PCI IDs

 sound/soc/intel/Kconfig                | 18 +++++++++++++++
 sound/soc/intel/skylake/skl-messages.c | 16 +++++++++++++
 sound/soc/intel/skylake/skl.c          | 10 ++++++++
 sound/soc/sof/intel/Kconfig            | 32 ++++++++++++++++++++++++++
 sound/soc/sof/sof-pci-dev.c            |  8 +++++++
 5 files changed, 84 insertions(+)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/2] ASoC: SOF: Add Comet Lake PCI IDs
  2019-05-07 21:53 [PATCH v2 0/2] ASoC: Intel: Add Cometlake PCI IDs Evan Green
@ 2019-05-07 21:53 ` Evan Green
  2019-05-07 22:14   ` Pierre-Louis Bossart
  2019-05-07 21:53 ` [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake " Evan Green
  1 sibling, 1 reply; 11+ messages in thread
From: Evan Green @ 2019-05-07 21:53 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: Pierre-Louis Bossart, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Evan Green, Jaroslav Kysela, alsa-devel,
	linux-kernel, Takashi Iwai, Liam Girdwood

Add support for Intel Comet Lake platforms by adding a new Kconfig
for CometLake and the appropriate PCI IDs.

Signed-off-by: Evan Green <evgreen@chromium.org>
---

Changes in v2:
- Add CML-H ID 0x06c8 (Pierre-Louis)

 sound/soc/sof/intel/Kconfig | 32 ++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-pci-dev.c |  8 ++++++++
 2 files changed, 40 insertions(+)

diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig
index 32ee0fabab92..b3f4463100ce 100644
--- a/sound/soc/sof/intel/Kconfig
+++ b/sound/soc/sof/intel/Kconfig
@@ -24,6 +24,8 @@ config SND_SOC_SOF_INTEL_PCI
 	select SND_SOC_SOF_CANNONLAKE  if SND_SOC_SOF_CANNONLAKE_SUPPORT
 	select SND_SOC_SOF_COFFEELAKE  if SND_SOC_SOF_COFFEELAKE_SUPPORT
 	select SND_SOC_SOF_ICELAKE     if SND_SOC_SOF_ICELAKE_SUPPORT
+	select SND_SOC_SOF_COMETLAKE_LP if SND_SOC_SOF_COMETLAKE_LP_SUPPORT
+	select SND_SOC_SOF_COMETLAKE_H if SND_SOC_SOF_COMETLAKE_H_SUPPORT
 	help
 	  This option is not user-selectable but automagically handled by
 	  'select' statements at a higher level
@@ -179,6 +181,36 @@ config SND_SOC_SOF_ICELAKE
 	  This option is not user-selectable but automagically handled by
 	  'select' statements at a higher level
 
+config SND_SOC_SOF_COMETLAKE_LP
+	tristate
+	select SND_SOC_SOF_CANNONLAKE
+	help
+	  This option is not user-selectable but automagically handled by
+	  'select' statements at a higher level
+
+config SND_SOC_SOF_COMETLAKE_LP_SUPPORT
+	bool "SOF support for CometLake-LP"
+	help
+	  This adds support for Sound Open Firmware for Intel(R) platforms
+	  using the Cometlake-LP processors.
+	  Say Y if you have such a device.
+	  If unsure select "N".
+
+config SND_SOC_SOF_COMETLAKE_H
+	tristate
+	select SND_SOC_SOF_CANNONLAKE
+	help
+	  This option is not user-selectable but automagically handled by
+	  'select' statements at a higher level
+
+config SND_SOC_SOF_COMETLAKE_H_SUPPORT
+	bool "SOF support for CometLake-H"
+	help
+	  This adds support for Sound Open Firmware for Intel(R) platforms
+	  using the Cometlake-H processors.
+	  Say Y if you have such a device.
+	  If unsure select "N".
+
 config SND_SOC_SOF_HDA_COMMON
 	tristate
 	select SND_SOC_SOF_INTEL_COMMON
diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
index b778dffb2d25..f559a56d18ab 100644
--- a/sound/soc/sof/sof-pci-dev.c
+++ b/sound/soc/sof/sof-pci-dev.c
@@ -353,6 +353,14 @@ static const struct pci_device_id sof_pci_ids[] = {
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_ICELAKE)
 	{ PCI_DEVICE(0x8086, 0x34C8),
 		.driver_data = (unsigned long)&icl_desc},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_LP)
+	{ PCI_DEVICE(0x8086, 0x02c8),
+		.driver_data = (unsigned long)&cnl_desc},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_H)
+	{ PCI_DEVICE(0x8086, 0x06c8),
+		.driver_data = (unsigned long)&cnl_desc},
 #endif
 	{ 0, }
 };
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake PCI IDs
  2019-05-07 21:53 [PATCH v2 0/2] ASoC: Intel: Add Cometlake PCI IDs Evan Green
  2019-05-07 21:53 ` [PATCH v2 1/2] ASoC: SOF: Add Comet Lake " Evan Green
@ 2019-05-07 21:53 ` Evan Green
  2019-05-07 22:31   ` Pierre-Louis Bossart
  1 sibling, 1 reply; 11+ messages in thread
From: Evan Green @ 2019-05-07 21:53 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: Pierre-Louis Bossart, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Evan Green, Jaroslav Kysela, alsa-devel,
	Rakesh Ughreja, Guenter Roeck, Yu Zhao, linux-kernel,
	Takashi Iwai, Jenny TC, Jie Yang

Add PCI IDs for Intel CometLake platforms, which from a software
point of view are extremely similar to Cannonlake platforms.

Signed-off-by: Evan Green <evgreen@chromium.org>
---

Changes in v2:
- Add 0x06c8 for CML-H (Pierre-Louis)

 sound/soc/intel/Kconfig                | 18 ++++++++++++++++++
 sound/soc/intel/skylake/skl-messages.c | 16 ++++++++++++++++
 sound/soc/intel/skylake/skl.c          | 10 ++++++++++
 3 files changed, 44 insertions(+)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index fc1396adde71..1ebac54b7081 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -110,6 +110,8 @@ config SND_SOC_INTEL_SKYLAKE
 	select SND_SOC_INTEL_GLK
 	select SND_SOC_INTEL_CNL
 	select SND_SOC_INTEL_CFL
+	select SND_SOC_INTEL_CML_H
+	select SND_SOC_INTEL_CML_LP
 	help
           This is a backwards-compatible option to select all devices
 	  supported by the Intel SST/Skylake driver. This option is no
@@ -165,6 +167,22 @@ config SND_SOC_INTEL_CFL
 	  If you have a Intel CoffeeLake platform with the DSP
 	  enabled in the BIOS then enable this option by saying Y or m.
 
+config SND_SOC_INTEL_CML_H
+	tristate "CometLake-H Platforms"
+	depends on PCI && ACPI
+	select SND_SOC_INTEL_SKYLAKE_FAMILY
+	help
+	  If you have a Intel CometLake-H platform with the DSP
+	  enabled in the BIOS then enable this option by saying Y or m.
+
+config SND_SOC_INTEL_CML_LP
+	tristate "CometLake-LP Platforms"
+	depends on PCI && ACPI
+	select SND_SOC_INTEL_SKYLAKE_FAMILY
+	help
+	  If you have a Intel CometLake-LP platform with the DSP
+	  enabled in the BIOS then enable this option by saying Y or m.
+
 config SND_SOC_INTEL_SKYLAKE_FAMILY
 	tristate
 	select SND_SOC_INTEL_SKYLAKE_COMMON
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index 4bf70b4429f0..df01dc952521 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -255,6 +255,22 @@ static const struct skl_dsp_ops dsp_ops[] = {
 		.init_fw = cnl_sst_init_fw,
 		.cleanup = cnl_sst_dsp_cleanup
 	},
+	{
+		.id = 0x02c8,
+		.num_cores = 4,
+		.loader_ops = bxt_get_loader_ops,
+		.init = cnl_sst_dsp_init,
+		.init_fw = cnl_sst_init_fw,
+		.cleanup = cnl_sst_dsp_cleanup
+	},
+	{
+		.id = 0x06c8,
+		.num_cores = 4,
+		.loader_ops = bxt_get_loader_ops,
+		.init = cnl_sst_dsp_init,
+		.init_fw = cnl_sst_init_fw,
+		.cleanup = cnl_sst_dsp_cleanup
+	},
 };
 
 const struct skl_dsp_ops *skl_get_dsp_ops(int pci_id)
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 4ed5b7e17d44..f864f7b3df3a 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -1166,6 +1166,16 @@ static const struct pci_device_id skl_ids[] = {
 	/* CFL */
 	{ PCI_DEVICE(0x8086, 0xa348),
 		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CML_LP)
+	/* CML-LP */
+	{ PCI_DEVICE(0x8086, 0x02c8),
+		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CML_H)
+	/* CML-H */
+	{ PCI_DEVICE(0x8086, 0x06c8),
+		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
 #endif
 	{ 0, }
 };
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] ASoC: SOF: Add Comet Lake PCI IDs
  2019-05-07 21:53 ` [PATCH v2 1/2] ASoC: SOF: Add Comet Lake " Evan Green
@ 2019-05-07 22:14   ` Pierre-Louis Bossart
  2019-05-08 16:42     ` Evan Green
  0 siblings, 1 reply; 11+ messages in thread
From: Pierre-Louis Bossart @ 2019-05-07 22:14 UTC (permalink / raw)
  To: Evan Green, Liam Girdwood, Mark Brown
  Cc: Naveen M, Sathya Prakash, Ben Zhang, Rajat Jain, Jaroslav Kysela,
	alsa-devel, linux-kernel, Takashi Iwai, Liam Girdwood

Minor nit-picks below. The Kconfig would work but select CANNONLAKE even 
if you don't want it.

>   
> +config SND_SOC_SOF_COMETLAKE_LP
> +	tristate
> +	select SND_SOC_SOF_CANNONLAKE

This should be
select SND_SOF_SOF_HDA_COMMON

> +	help
> +	  This option is not user-selectable but automagically handled by
> +	  'select' statements at a higher level
> +
> +config SND_SOC_SOF_COMETLAKE_LP_SUPPORT
> +	bool "SOF support for CometLake-LP"
> +	help
> +	  This adds support for Sound Open Firmware for Intel(R) platforms
> +	  using the Cometlake-LP processors.
> +	  Say Y if you have such a device.
> +	  If unsure select "N".
> +
> +config SND_SOC_SOF_COMETLAKE_H
> +	tristate
> +	select SND_SOC_SOF_CANNONLAKE

This should be
select SND_SOF_SOF_HDA_COMMON

> +	help
> +	  This option is not user-selectable but automagically handled by
> +	  'select' statements at a higher level
> +
> +config SND_SOC_SOF_COMETLAKE_H_SUPPORT
> +	bool "SOF support for CometLake-H"
> +	help
> +	  This adds support for Sound Open Firmware for Intel(R) platforms
> +	  using the Cometlake-H processors.
> +	  Say Y if you have such a device.
> +	  If unsure select "N".


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake PCI IDs
  2019-05-07 21:53 ` [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake " Evan Green
@ 2019-05-07 22:31   ` Pierre-Louis Bossart
  2019-05-08 16:51     ` Evan Green
  0 siblings, 1 reply; 11+ messages in thread
From: Pierre-Louis Bossart @ 2019-05-07 22:31 UTC (permalink / raw)
  To: Evan Green, Liam Girdwood, Mark Brown
  Cc: Naveen M, Sathya Prakash, Ben Zhang, Rajat Jain, Jaroslav Kysela,
	alsa-devel, Rakesh Ughreja, Guenter Roeck, Yu Zhao, linux-kernel,
	Takashi Iwai, Jenny TC, Jie Yang

On 5/7/19 4:53 PM, Evan Green wrote:
> Add PCI IDs for Intel CometLake platforms, which from a software
> point of view are extremely similar to Cannonlake platforms.

Humm, I have mixed feelings here.

Yes the hardware is nearly identical, with the exception of one detail 
that's not visible to the kernel, but there is no support for DMICs with 
the Skylake driver w/ HDaudio, and Chrome platforms are only going with 
SOF, so is it wise to add these two CometLake platforms to the default 
SND_SOC_INTEL_SKYLAKE selector, which is used by a number of distributions?

I don't mind if we add those PCI IDs and people use this driver if they 
wish to, but it may be time for an explicit opt-in? The 
SND_SOC_INTEL_SKYLAKE definition should even be pruned to mean SKL, APL, 
KBL and GLK, and we can add DMI-based quirks for e.g. the Up2 board and 
GLK Chromebooks which work with SOF.

> 
> Signed-off-by: Evan Green <evgreen@chromium.org>
> ---
> 
> Changes in v2:
> - Add 0x06c8 for CML-H (Pierre-Louis)
> 
>   sound/soc/intel/Kconfig                | 18 ++++++++++++++++++
>   sound/soc/intel/skylake/skl-messages.c | 16 ++++++++++++++++
>   sound/soc/intel/skylake/skl.c          | 10 ++++++++++
>   3 files changed, 44 insertions(+)
> 
> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> index fc1396adde71..1ebac54b7081 100644
> --- a/sound/soc/intel/Kconfig
> +++ b/sound/soc/intel/Kconfig
> @@ -110,6 +110,8 @@ config SND_SOC_INTEL_SKYLAKE
>   	select SND_SOC_INTEL_GLK
>   	select SND_SOC_INTEL_CNL
>   	select SND_SOC_INTEL_CFL
> +	select SND_SOC_INTEL_CML_H
> +	select SND_SOC_INTEL_CML_LP
>   	help
>             This is a backwards-compatible option to select all devices
>   	  supported by the Intel SST/Skylake driver. This option is no
> @@ -165,6 +167,22 @@ config SND_SOC_INTEL_CFL
>   	  If you have a Intel CoffeeLake platform with the DSP
>   	  enabled in the BIOS then enable this option by saying Y or m.
>   
> +config SND_SOC_INTEL_CML_H
> +	tristate "CometLake-H Platforms"
> +	depends on PCI && ACPI
> +	select SND_SOC_INTEL_SKYLAKE_FAMILY
> +	help
> +	  If you have a Intel CometLake-H platform with the DSP
> +	  enabled in the BIOS then enable this option by saying Y or m.
> +
> +config SND_SOC_INTEL_CML_LP
> +	tristate "CometLake-LP Platforms"
> +	depends on PCI && ACPI
> +	select SND_SOC_INTEL_SKYLAKE_FAMILY
> +	help
> +	  If you have a Intel CometLake-LP platform with the DSP
> +	  enabled in the BIOS then enable this option by saying Y or m.
> +
>   config SND_SOC_INTEL_SKYLAKE_FAMILY
>   	tristate
>   	select SND_SOC_INTEL_SKYLAKE_COMMON
> diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
> index 4bf70b4429f0..df01dc952521 100644
> --- a/sound/soc/intel/skylake/skl-messages.c
> +++ b/sound/soc/intel/skylake/skl-messages.c
> @@ -255,6 +255,22 @@ static const struct skl_dsp_ops dsp_ops[] = {
>   		.init_fw = cnl_sst_init_fw,
>   		.cleanup = cnl_sst_dsp_cleanup
>   	},
> +	{
> +		.id = 0x02c8,
> +		.num_cores = 4,
> +		.loader_ops = bxt_get_loader_ops,
> +		.init = cnl_sst_dsp_init,
> +		.init_fw = cnl_sst_init_fw,
> +		.cleanup = cnl_sst_dsp_cleanup
> +	},
> +	{
> +		.id = 0x06c8,
> +		.num_cores = 4,
> +		.loader_ops = bxt_get_loader_ops,
> +		.init = cnl_sst_dsp_init,
> +		.init_fw = cnl_sst_init_fw,
> +		.cleanup = cnl_sst_dsp_cleanup
> +	},
>   };
>   
>   const struct skl_dsp_ops *skl_get_dsp_ops(int pci_id)
> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> index 4ed5b7e17d44..f864f7b3df3a 100644
> --- a/sound/soc/intel/skylake/skl.c
> +++ b/sound/soc/intel/skylake/skl.c
> @@ -1166,6 +1166,16 @@ static const struct pci_device_id skl_ids[] = {
>   	/* CFL */
>   	{ PCI_DEVICE(0x8086, 0xa348),
>   		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CML_LP)
> +	/* CML-LP */
> +	{ PCI_DEVICE(0x8086, 0x02c8),
> +		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CML_H)
> +	/* CML-H */
> +	{ PCI_DEVICE(0x8086, 0x06c8),
> +		.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
>   #endif
>   	{ 0, }
>   };
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] ASoC: SOF: Add Comet Lake PCI IDs
  2019-05-07 22:14   ` Pierre-Louis Bossart
@ 2019-05-08 16:42     ` Evan Green
  2019-05-08 17:00       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 11+ messages in thread
From: Evan Green @ 2019-05-08 16:42 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Liam Girdwood, Mark Brown, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Jaroslav Kysela, alsa-devel, LKML, Takashi Iwai,
	Liam Girdwood

On Tue, May 7, 2019 at 3:14 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
> Minor nit-picks below. The Kconfig would work but select CANNONLAKE even
> if you don't want it.
>
> >
> > +config SND_SOC_SOF_COMETLAKE_LP
> > +     tristate
> > +     select SND_SOC_SOF_CANNONLAKE
>
> This should be
> select SND_SOF_SOF_HDA_COMMON

You mean SND_SOC_SOF_HDA_COMMON I assume.
Except that I also need &cnl_desc, so I need CANNONLAKE to be on as
well. Should I select them both?
-Evan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake PCI IDs
  2019-05-07 22:31   ` Pierre-Louis Bossart
@ 2019-05-08 16:51     ` Evan Green
  2019-05-08 17:04       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 11+ messages in thread
From: Evan Green @ 2019-05-08 16:51 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Liam Girdwood, Mark Brown, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Jaroslav Kysela, alsa-devel, Rakesh Ughreja,
	Guenter Roeck, Yu Zhao, LKML, Takashi Iwai, Jenny TC, Jie Yang

On Tue, May 7, 2019 at 3:31 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
> On 5/7/19 4:53 PM, Evan Green wrote:
> > Add PCI IDs for Intel CometLake platforms, which from a software
> > point of view are extremely similar to Cannonlake platforms.
>
> Humm, I have mixed feelings here.
>
> Yes the hardware is nearly identical, with the exception of one detail
> that's not visible to the kernel, but there is no support for DMICs with
> the Skylake driver w/ HDaudio, and Chrome platforms are only going with
> SOF, so is it wise to add these two CometLake platforms to the default
> SND_SOC_INTEL_SKYLAKE selector, which is used by a number of distributions?
>
> I don't mind if we add those PCI IDs and people use this driver if they
> wish to, but it may be time for an explicit opt-in? The
> SND_SOC_INTEL_SKYLAKE definition should even be pruned to mean SKL, APL,
> KBL and GLK, and we can add DMI-based quirks for e.g. the Up2 board and
> GLK Chromebooks which work with SOF.

I don't have the context here, so feel free to ignore me. But it seems
like such a tiny amount of extra bits to add to make Cometlake work,
and then there's no hassle for the distributions when Cometlake
devices start showing up in the wild. So while things are more or less
the same, why not continue piggypacking off the default?
Or are you saying that the lack of DMIC support means the default
should be to use a different driver?
-Evan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] ASoC: SOF: Add Comet Lake PCI IDs
  2019-05-08 16:42     ` Evan Green
@ 2019-05-08 17:00       ` Pierre-Louis Bossart
  2019-05-08 21:58         ` Evan Green
  0 siblings, 1 reply; 11+ messages in thread
From: Pierre-Louis Bossart @ 2019-05-08 17:00 UTC (permalink / raw)
  To: Evan Green
  Cc: Liam Girdwood, Mark Brown, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Jaroslav Kysela, alsa-devel, LKML, Takashi Iwai,
	Liam Girdwood



On 5/8/19 11:42 AM, Evan Green wrote:
> On Tue, May 7, 2019 at 3:14 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>>
>> Minor nit-picks below. The Kconfig would work but select CANNONLAKE even
>> if you don't want it.
>>
>>>
>>> +config SND_SOC_SOF_COMETLAKE_LP
>>> +     tristate
>>> +     select SND_SOC_SOF_CANNONLAKE
>>
>> This should be
>> select SND_SOF_SOF_HDA_COMMON
> 
> You mean SND_SOC_SOF_HDA_COMMON I assume.
> Except that I also need &cnl_desc, so I need CANNONLAKE to be on as
> well. Should I select them both?

Ah I see. I'd rather use a different descriptor then, and make the two 
platforms independent, as I did for CoffeeLake. You can use the same 
descriptor for the two -H and -LP skews though.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake PCI IDs
  2019-05-08 16:51     ` Evan Green
@ 2019-05-08 17:04       ` Pierre-Louis Bossart
  2019-05-08 21:57         ` Evan Green
  0 siblings, 1 reply; 11+ messages in thread
From: Pierre-Louis Bossart @ 2019-05-08 17:04 UTC (permalink / raw)
  To: Evan Green
  Cc: Liam Girdwood, Mark Brown, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Jaroslav Kysela, alsa-devel, Rakesh Ughreja,
	Guenter Roeck, Yu Zhao, LKML, Takashi Iwai, Jenny TC, Jie Yang



On 5/8/19 11:51 AM, Evan Green wrote:
> On Tue, May 7, 2019 at 3:31 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>>
>> On 5/7/19 4:53 PM, Evan Green wrote:
>>> Add PCI IDs for Intel CometLake platforms, which from a software
>>> point of view are extremely similar to Cannonlake platforms.
>>
>> Humm, I have mixed feelings here.
>>
>> Yes the hardware is nearly identical, with the exception of one detail
>> that's not visible to the kernel, but there is no support for DMICs with
>> the Skylake driver w/ HDaudio, and Chrome platforms are only going with
>> SOF, so is it wise to add these two CometLake platforms to the default
>> SND_SOC_INTEL_SKYLAKE selector, which is used by a number of distributions?
>>
>> I don't mind if we add those PCI IDs and people use this driver if they
>> wish to, but it may be time for an explicit opt-in? The
>> SND_SOC_INTEL_SKYLAKE definition should even be pruned to mean SKL, APL,
>> KBL and GLK, and we can add DMI-based quirks for e.g. the Up2 board and
>> GLK Chromebooks which work with SOF.
> 
> I don't have the context here, so feel free to ignore me. But it seems
> like such a tiny amount of extra bits to add to make Cometlake work,
> and then there's no hassle for the distributions when Cometlake
> devices start showing up in the wild. So while things are more or less
> the same, why not continue piggypacking off the default?
> Or are you saying that the lack of DMIC support means the default
> should be to use a different driver?

Yes, it's the latter case, SOF will be the only driver supporting DMICs 
on CometLake, so it'd be better to avoid creating a conflict with SOF or 
enabling a configuration by default which is known to have restrictions. 
It's fine to add the CML IDs, but better avoid adding CML under the 
SKYLAKE all-you-can-eat selector.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake PCI IDs
  2019-05-08 17:04       ` Pierre-Louis Bossart
@ 2019-05-08 21:57         ` Evan Green
  0 siblings, 0 replies; 11+ messages in thread
From: Evan Green @ 2019-05-08 21:57 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Liam Girdwood, Mark Brown, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Jaroslav Kysela, alsa-devel, Rakesh Ughreja,
	Guenter Roeck, Yu Zhao, LKML, Takashi Iwai, Jenny TC, Jie Yang

On Wed, May 8, 2019 at 10:04 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
>
> On 5/8/19 11:51 AM, Evan Green wrote:
> > On Tue, May 7, 2019 at 3:31 PM Pierre-Louis Bossart
> > <pierre-louis.bossart@linux.intel.com> wrote:
> >>
> >> On 5/7/19 4:53 PM, Evan Green wrote:
> >>> Add PCI IDs for Intel CometLake platforms, which from a software
> >>> point of view are extremely similar to Cannonlake platforms.
> >>
> >> Humm, I have mixed feelings here.
> >>
> >> Yes the hardware is nearly identical, with the exception of one detail
> >> that's not visible to the kernel, but there is no support for DMICs with
> >> the Skylake driver w/ HDaudio, and Chrome platforms are only going with
> >> SOF, so is it wise to add these two CometLake platforms to the default
> >> SND_SOC_INTEL_SKYLAKE selector, which is used by a number of distributions?
> >>
> >> I don't mind if we add those PCI IDs and people use this driver if they
> >> wish to, but it may be time for an explicit opt-in? The
> >> SND_SOC_INTEL_SKYLAKE definition should even be pruned to mean SKL, APL,
> >> KBL and GLK, and we can add DMI-based quirks for e.g. the Up2 board and
> >> GLK Chromebooks which work with SOF.
> >
> > I don't have the context here, so feel free to ignore me. But it seems
> > like such a tiny amount of extra bits to add to make Cometlake work,
> > and then there's no hassle for the distributions when Cometlake
> > devices start showing up in the wild. So while things are more or less
> > the same, why not continue piggypacking off the default?
> > Or are you saying that the lack of DMIC support means the default
> > should be to use a different driver?
>
> Yes, it's the latter case, SOF will be the only driver supporting DMICs
> on CometLake, so it'd be better to avoid creating a conflict with SOF or
> enabling a configuration by default which is known to have restrictions.
> It's fine to add the CML IDs, but better avoid adding CML under the
> SKYLAKE all-you-can-eat selector.

Ok, I'll plan to remove the new selects from under SND_SOC_INTEL_SKYLAKE.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] ASoC: SOF: Add Comet Lake PCI IDs
  2019-05-08 17:00       ` Pierre-Louis Bossart
@ 2019-05-08 21:58         ` Evan Green
  0 siblings, 0 replies; 11+ messages in thread
From: Evan Green @ 2019-05-08 21:58 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Liam Girdwood, Mark Brown, Naveen M, Sathya Prakash, Ben Zhang,
	Rajat Jain, Jaroslav Kysela, alsa-devel, LKML, Takashi Iwai,
	Liam Girdwood

On Wed, May 8, 2019 at 10:00 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
>
> On 5/8/19 11:42 AM, Evan Green wrote:
> > On Tue, May 7, 2019 at 3:14 PM Pierre-Louis Bossart
> > <pierre-louis.bossart@linux.intel.com> wrote:
> >>
> >> Minor nit-picks below. The Kconfig would work but select CANNONLAKE even
> >> if you don't want it.
> >>
> >>>
> >>> +config SND_SOC_SOF_COMETLAKE_LP
> >>> +     tristate
> >>> +     select SND_SOC_SOF_CANNONLAKE
> >>
> >> This should be
> >> select SND_SOF_SOF_HDA_COMMON
> >
> > You mean SND_SOC_SOF_HDA_COMMON I assume.
> > Except that I also need &cnl_desc, so I need CANNONLAKE to be on as
> > well. Should I select them both?
>
> Ah I see. I'd rather use a different descriptor then, and make the two
> platforms independent, as I did for CoffeeLake. You can use the same
> descriptor for the two -H and -LP skews though.

Ok, I'll add a cml_desc and have it conditionally compiled in on
either _LP or _H Kconfig.
-Evan

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-05-08 22:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07 21:53 [PATCH v2 0/2] ASoC: Intel: Add Cometlake PCI IDs Evan Green
2019-05-07 21:53 ` [PATCH v2 1/2] ASoC: SOF: Add Comet Lake " Evan Green
2019-05-07 22:14   ` Pierre-Louis Bossart
2019-05-08 16:42     ` Evan Green
2019-05-08 17:00       ` Pierre-Louis Bossart
2019-05-08 21:58         ` Evan Green
2019-05-07 21:53 ` [PATCH v2 2/2] ASoC: Intel: Skylake: Add Cometlake " Evan Green
2019-05-07 22:31   ` Pierre-Louis Bossart
2019-05-08 16:51     ` Evan Green
2019-05-08 17:04       ` Pierre-Louis Bossart
2019-05-08 21:57         ` Evan Green

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).