Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: hda - Add Coffelake PCI ID
@ 2017-06-14  4:21 Subhransu S. Prusty
  2017-06-14  4:45 ` Vinod Koul
  2017-06-14  7:34 ` Takashi Iwai
  0 siblings, 2 replies; 8+ messages in thread
From: Subhransu S. Prusty @ 2017-06-14  4:21 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, lgirdwood, patches.audio, broonie, Subhransu S. Prusty,
	Megha Dey

From: Megha Dey <megha.dey@intel.com>

Coffelake is another Intel part, so need to add PCI ID for it.

Signed-off-by: Megha Dey <megha.dey@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/pci/hda/hda_intel.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 1770f085c2a6..e3c696c46a21 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -371,9 +371,10 @@ enum {
 #define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
 #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
 #define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
+#define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
 #define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
 			IS_KBL(pci) || IS_KBL_LP(pci) || IS_KBL_H(pci)	|| \
-			IS_GLK(pci)
+			IS_GLK(pci) || IS_CFL(pci)
 
 static char *driver_short_names[] = {
 	[AZX_DRIVER_ICH] = "HDA Intel",
@@ -2378,6 +2379,9 @@ static void azx_shutdown(struct pci_dev *pci)
 	/* Kabylake-H */
 	{ PCI_DEVICE(0x8086, 0xa2f0),
 	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
+	/* Coffelake */
+	{ PCI_DEVICE(0x8086, 0xa348),
+	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE},
 	/* Broxton-P(Apollolake) */
 	{ PCI_DEVICE(0x8086, 0x5a98),
 	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_BROXTON },
-- 
1.9.1

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

* Re: [PATCH] ALSA: hda - Add Coffelake PCI ID
  2017-06-14  4:21 [PATCH] ALSA: hda - Add Coffelake PCI ID Subhransu S. Prusty
@ 2017-06-14  4:45 ` Vinod Koul
  2017-06-14  5:25   ` Takashi Iwai
  2017-06-14  7:34 ` Takashi Iwai
  1 sibling, 1 reply; 8+ messages in thread
From: Vinod Koul @ 2017-06-14  4:45 UTC (permalink / raw)
  To: Subhransu S. Prusty
  Cc: alsa-devel, tiwai, lgirdwood, patches.audio, broonie, Megha Dey

On Wed, Jun 14, 2017 at 09:51:56AM +0530, Subhransu S. Prusty wrote:
> From: Megha Dey <megha.dey@intel.com>
> 
> Coffelake is another Intel part, so need to add PCI ID for it.
> 
> Signed-off-by: Megha Dey <megha.dey@intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>

Acked-By: Vinod Koul <vinod.koul@intel.com>


> ---
>  sound/pci/hda/hda_intel.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index 1770f085c2a6..e3c696c46a21 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -371,9 +371,10 @@ enum {
>  #define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
>  #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
>  #define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
> +#define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
>  #define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
>  			IS_KBL(pci) || IS_KBL_LP(pci) || IS_KBL_H(pci)	|| \
> -			IS_GLK(pci)
> +			IS_GLK(pci) || IS_CFL(pci)

Takashi, given the pattern here that device are SKL derivatives, this would
blow up, can we do something here please.

I am thinking using AXZ_DCAPS foo rather than IS_foo()

>  
>  static char *driver_short_names[] = {
>  	[AZX_DRIVER_ICH] = "HDA Intel",
> @@ -2378,6 +2379,9 @@ static void azx_shutdown(struct pci_dev *pci)
>  	/* Kabylake-H */
>  	{ PCI_DEVICE(0x8086, 0xa2f0),
>  	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
> +	/* Coffelake */
> +	{ PCI_DEVICE(0x8086, 0xa348),
> +	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE},
>  	/* Broxton-P(Apollolake) */
>  	{ PCI_DEVICE(0x8086, 0x5a98),
>  	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_BROXTON },
> -- 
> 1.9.1
> 

-- 
~Vinod

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

* Re: [PATCH] ALSA: hda - Add Coffelake PCI ID
  2017-06-14  4:45 ` Vinod Koul
@ 2017-06-14  5:25   ` Takashi Iwai
  2017-06-14  5:41     ` Takashi Iwai
  2017-06-16  7:54     ` Vinod Koul
  0 siblings, 2 replies; 8+ messages in thread
From: Takashi Iwai @ 2017-06-14  5:25 UTC (permalink / raw)
  To: Vinod Koul
  Cc: alsa-devel, patches.audio, lgirdwood, broonie,
	Subhransu S. Prusty, Megha Dey

On Wed, 14 Jun 2017 06:45:48 +0200,
Vinod Koul wrote:
> > ---
> >  sound/pci/hda/hda_intel.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index 1770f085c2a6..e3c696c46a21 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -371,9 +371,10 @@ enum {
> >  #define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
> >  #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
> >  #define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
> > +#define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
> >  #define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
> >  			IS_KBL(pci) || IS_KBL_LP(pci) || IS_KBL_H(pci)	|| \
> > -			IS_GLK(pci)
> > +			IS_GLK(pci) || IS_CFL(pci)
> 
> Takashi, given the pattern here that device are SKL derivatives, this would
> blow up, can we do something here please.
> 
> I am thinking using AXZ_DCAPS foo rather than IS_foo()

AZX_DCAPS bit is precious.  An easier option is to add a new
AZX_DRIVER_SKL.  An untested patch is like below.  The shortname
string is kept for not breaking the compatibility.

But while writing this, I noticed that Broxton-T seems forgotten.
Doesn't it need the similar workaround for SKL+ chips?


thanks,

Takashi

---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index e3c696c46a21..07ea7f48aa01 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -263,6 +263,7 @@ enum {
 	AZX_DRIVER_ICH,
 	AZX_DRIVER_PCH,
 	AZX_DRIVER_SCH,
+	AZX_DRIVER_SKL,
 	AZX_DRIVER_HDMI,
 	AZX_DRIVER_ATI,
 	AZX_DRIVER_ATIHDMI,
@@ -364,22 +365,13 @@ enum {
 					((pci)->device == 0x0d0c) || \
 					((pci)->device == 0x160c))
 
-#define IS_SKL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa170)
-#define IS_SKL_LP(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9d70)
-#define IS_KBL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa171)
-#define IS_KBL_LP(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9d71)
-#define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
 #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
-#define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
-#define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
-#define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
-			IS_KBL(pci) || IS_KBL_LP(pci) || IS_KBL_H(pci)	|| \
-			IS_GLK(pci) || IS_CFL(pci)
 
 static char *driver_short_names[] = {
 	[AZX_DRIVER_ICH] = "HDA Intel",
 	[AZX_DRIVER_PCH] = "HDA Intel PCH",
 	[AZX_DRIVER_SCH] = "HDA Intel MID",
+	[AZX_DRIVER_SKL] = "HDA Intel PCH", /* kept old name for compatibility */
 	[AZX_DRIVER_HDMI] = "HDA Intel HDMI",
 	[AZX_DRIVER_ATI] = "HDA ATI SB",
 	[AZX_DRIVER_ATIHDMI] = "HDA ATI HDMI",
@@ -643,13 +635,13 @@ static void hda_intel_init_chip(struct azx *chip, bool full_reset)
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, true);
-	if (IS_SKL_PLUS(pci)) {
+	if (chip->driver_type == AZX_DRIVER_SKL) {
 		pci_read_config_dword(pci, INTEL_HDA_CGCTL, &val);
 		val = val & ~INTEL_HDA_CGCTL_MISCBDCGE;
 		pci_write_config_dword(pci, INTEL_HDA_CGCTL, val);
 	}
 	azx_init_chip(chip, full_reset);
-	if (IS_SKL_PLUS(pci)) {
+	if (chip->driver_type == AZX_DRIVER_SKL) {
 		pci_read_config_dword(pci, INTEL_HDA_CGCTL, &val);
 		val = val | INTEL_HDA_CGCTL_MISCBDCGE;
 		pci_write_config_dword(pci, INTEL_HDA_CGCTL, val);
@@ -1074,9 +1066,11 @@ static int azx_resume(struct device *dev)
  */
 static int azx_freeze_noirq(struct device *dev)
 {
+	struct snd_card *card = dev_get_drvdata(dev);
+	struct azx *chip = card->private_data;
 	struct pci_dev *pci = to_pci_dev(dev);
 
-	if (IS_SKL_PLUS(pci))
+	if (chip->driver_type == AZX_DRIVER_SKL)
 		pci_set_power_state(pci, PCI_D3hot);
 
 	return 0;
@@ -1084,9 +1078,11 @@ static int azx_freeze_noirq(struct device *dev)
 
 static int azx_thaw_noirq(struct device *dev)
 {
+	struct snd_card *card = dev_get_drvdata(dev);
+	struct azx *chip = card->private_data;
 	struct pci_dev *pci = to_pci_dev(dev);
 
-	if (IS_SKL_PLUS(pci))
+	if (chip->driver_type == AZX_DRIVER_SKL)
 		pci_set_power_state(pci, PCI_D0);
 
 	return 0;
@@ -1496,7 +1492,7 @@ static int check_position_fix(struct azx *chip, int fix)
 		dev_dbg(chip->card->dev, "Using LPIB position fix\n");
 		return POS_FIX_LPIB;
 	}
-	if (IS_SKL_PLUS(chip->pci)) {
+	if (chip->driver_type == AZX_DRIVER_SKL) {
 		dev_dbg(chip->card->dev, "Using SKL position fix\n");
 		return POS_FIX_SKL;
 	}
@@ -1797,7 +1793,7 @@ static int azx_first_init(struct azx *chip)
 		return -ENXIO;
 	}
 
-	if (IS_SKL_PLUS(pci))
+	if (chip->driver_type == AZX_DRIVER_SKL)
 		snd_hdac_bus_parse_capabilities(bus);
 
 	/*
@@ -2366,31 +2362,31 @@ static const struct pci_device_id azx_ids[] = {
 	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH },
 	/* Sunrise Point */
 	{ PCI_DEVICE(0x8086, 0xa170),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Sunrise Point-LP */
 	{ PCI_DEVICE(0x8086, 0x9d70),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Kabylake */
 	{ PCI_DEVICE(0x8086, 0xa171),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Kabylake-LP */
 	{ PCI_DEVICE(0x8086, 0x9d71),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Kabylake-H */
 	{ PCI_DEVICE(0x8086, 0xa2f0),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Coffelake */
 	{ PCI_DEVICE(0x8086, 0xa348),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE},
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
 	/* Broxton-P(Apollolake) */
 	{ PCI_DEVICE(0x8086, 0x5a98),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_BROXTON },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
 	/* Broxton-T */
 	{ PCI_DEVICE(0x8086, 0x1a98),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_BROXTON },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
 	/* Gemini-Lake */
 	{ PCI_DEVICE(0x8086, 0x3198),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_BROXTON },
+	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
 	/* Haswell */
 	{ PCI_DEVICE(0x8086, 0x0a0c),
 	  .driver_data = AZX_DRIVER_HDMI | AZX_DCAPS_INTEL_HASWELL },

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

* Re: [PATCH] ALSA: hda - Add Coffelake PCI ID
  2017-06-14  5:25   ` Takashi Iwai
@ 2017-06-14  5:41     ` Takashi Iwai
  2017-06-16  6:36       ` Takashi Iwai
  2017-06-16  7:54     ` Vinod Koul
  1 sibling, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2017-06-14  5:41 UTC (permalink / raw)
  To: Vinod Koul
  Cc: alsa-devel, patches.audio, lgirdwood, broonie,
	Subhransu S. Prusty, Megha Dey

On Wed, 14 Jun 2017 07:25:17 +0200,
Takashi Iwai wrote:
> 
> But while writing this, I noticed that Broxton-T seems forgotten.
> Doesn't it need the similar workaround for SKL+ chips?

I meant like below.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda - Apply quirks to Broxton-T, too

Broxton-T was a forgotten child and we didn't apply the quirks
properly.  Let's fix it.

Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_intel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index e3c696c46a21..774f72b26d13 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -369,7 +369,8 @@ enum {
 #define IS_KBL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa171)
 #define IS_KBL_LP(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9d71)
 #define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
-#define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
+#define IS_BXT(pci) ((pci)->vendor == 0x8086 && \
+		     ((pci)->device == 0x5a98 || (pci)->device == 0x1a98))
 #define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
 #define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
 #define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
-- 
2.13.1

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

* Re: [PATCH] ALSA: hda - Add Coffelake PCI ID
  2017-06-14  4:21 [PATCH] ALSA: hda - Add Coffelake PCI ID Subhransu S. Prusty
  2017-06-14  4:45 ` Vinod Koul
@ 2017-06-14  7:34 ` Takashi Iwai
  1 sibling, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2017-06-14  7:34 UTC (permalink / raw)
  To: Subhransu S. Prusty
  Cc: patches.audio, alsa-devel, broonie, lgirdwood, Megha Dey

On Wed, 14 Jun 2017 06:21:56 +0200,
Subhransu S. Prusty wrote:
> 
> From: Megha Dey <megha.dey@intel.com>
> 
> Coffelake is another Intel part, so need to add PCI ID for it.
> 
> Signed-off-by: Megha Dey <megha.dey@intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>

Applied, thanks.


Takashi

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

* Re: [PATCH] ALSA: hda - Add Coffelake PCI ID
  2017-06-14  5:41     ` Takashi Iwai
@ 2017-06-16  6:36       ` Takashi Iwai
  2017-06-16  7:56         ` Vinod Koul
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2017-06-16  6:36 UTC (permalink / raw)
  To: Vinod Koul
  Cc: alsa-devel, patches.audio, lgirdwood, broonie,
	Subhransu S. Prusty, Megha Dey

On Wed, 14 Jun 2017 07:41:10 +0200,
Takashi Iwai wrote:
> 
> On Wed, 14 Jun 2017 07:25:17 +0200,
> Takashi Iwai wrote:
> > 
> > But while writing this, I noticed that Broxton-T seems forgotten.
> > Doesn't it need the similar workaround for SKL+ chips?
> 
> I meant like below.

Can anyone confirm whether it is required or not?

There are two things applied specific to BXT: one is the generic SKL+
quirk and another is a quick to reduce DMA latency.  Maybe the latter
one is superfluous for BXT-T but only the former one is needed?


thanks,

Takashi

> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: hda - Apply quirks to Broxton-T, too
> 
> Broxton-T was a forgotten child and we didn't apply the quirks
> properly.  Let's fix it.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/pci/hda/hda_intel.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index e3c696c46a21..774f72b26d13 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -369,7 +369,8 @@ enum {
>  #define IS_KBL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa171)
>  #define IS_KBL_LP(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9d71)
>  #define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
> -#define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
> +#define IS_BXT(pci) ((pci)->vendor == 0x8086 && \
> +		     ((pci)->device == 0x5a98 || (pci)->device == 0x1a98))
>  #define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
>  #define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
>  #define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
> -- 
> 2.13.1
> 

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

* Re: [PATCH] ALSA: hda - Add Coffelake PCI ID
  2017-06-14  5:25   ` Takashi Iwai
  2017-06-14  5:41     ` Takashi Iwai
@ 2017-06-16  7:54     ` Vinod Koul
  1 sibling, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2017-06-16  7:54 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, patches.audio, lgirdwood, broonie,
	Subhransu S. Prusty, Megha Dey

On Wed, Jun 14, 2017 at 07:25:17AM +0200, Takashi Iwai wrote:
> On Wed, 14 Jun 2017 06:45:48 +0200,
> Vinod Koul wrote:
> > > ---
> > >  sound/pci/hda/hda_intel.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > > index 1770f085c2a6..e3c696c46a21 100644
> > > --- a/sound/pci/hda/hda_intel.c
> > > +++ b/sound/pci/hda/hda_intel.c
> > > @@ -371,9 +371,10 @@ enum {
> > >  #define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
> > >  #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
> > >  #define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
> > > +#define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
> > >  #define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
> > >  			IS_KBL(pci) || IS_KBL_LP(pci) || IS_KBL_H(pci)	|| \
> > > -			IS_GLK(pci)
> > > +			IS_GLK(pci) || IS_CFL(pci)
> > 
> > Takashi, given the pattern here that device are SKL derivatives, this would
> > blow up, can we do something here please.
> > 
> > I am thinking using AXZ_DCAPS foo rather than IS_foo()
> 
> AZX_DCAPS bit is precious.  An easier option is to add a new
> AZX_DRIVER_SKL.  An untested patch is like below.  The shortname
> string is kept for not breaking the compatibility.

This makese sense to me.. Will get it next first thing on monday

> But while writing this, I noticed that Broxton-T seems forgotten.
> Doesn't it need the similar workaround for SKL+ chips?

It should have same stuff as other BXTN ones, I will try to find this part,
and if possible test this

> 
> 
> thanks,
> 
> Takashi
> 
> ---
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index e3c696c46a21..07ea7f48aa01 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -263,6 +263,7 @@ enum {
>  	AZX_DRIVER_ICH,
>  	AZX_DRIVER_PCH,
>  	AZX_DRIVER_SCH,
> +	AZX_DRIVER_SKL,
>  	AZX_DRIVER_HDMI,
>  	AZX_DRIVER_ATI,
>  	AZX_DRIVER_ATIHDMI,
> @@ -364,22 +365,13 @@ enum {
>  					((pci)->device == 0x0d0c) || \
>  					((pci)->device == 0x160c))
>  
> -#define IS_SKL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa170)
> -#define IS_SKL_LP(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9d70)
> -#define IS_KBL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa171)
> -#define IS_KBL_LP(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9d71)
> -#define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
>  #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
> -#define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
> -#define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
> -#define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
> -			IS_KBL(pci) || IS_KBL_LP(pci) || IS_KBL_H(pci)	|| \
> -			IS_GLK(pci) || IS_CFL(pci)
>  
>  static char *driver_short_names[] = {
>  	[AZX_DRIVER_ICH] = "HDA Intel",
>  	[AZX_DRIVER_PCH] = "HDA Intel PCH",
>  	[AZX_DRIVER_SCH] = "HDA Intel MID",
> +	[AZX_DRIVER_SKL] = "HDA Intel PCH", /* kept old name for compatibility */
>  	[AZX_DRIVER_HDMI] = "HDA Intel HDMI",
>  	[AZX_DRIVER_ATI] = "HDA ATI SB",
>  	[AZX_DRIVER_ATIHDMI] = "HDA ATI HDMI",
> @@ -643,13 +635,13 @@ static void hda_intel_init_chip(struct azx *chip, bool full_reset)
>  
>  	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
>  		snd_hdac_set_codec_wakeup(bus, true);
> -	if (IS_SKL_PLUS(pci)) {
> +	if (chip->driver_type == AZX_DRIVER_SKL) {
>  		pci_read_config_dword(pci, INTEL_HDA_CGCTL, &val);
>  		val = val & ~INTEL_HDA_CGCTL_MISCBDCGE;
>  		pci_write_config_dword(pci, INTEL_HDA_CGCTL, val);
>  	}
>  	azx_init_chip(chip, full_reset);
> -	if (IS_SKL_PLUS(pci)) {
> +	if (chip->driver_type == AZX_DRIVER_SKL) {
>  		pci_read_config_dword(pci, INTEL_HDA_CGCTL, &val);
>  		val = val | INTEL_HDA_CGCTL_MISCBDCGE;
>  		pci_write_config_dword(pci, INTEL_HDA_CGCTL, val);
> @@ -1074,9 +1066,11 @@ static int azx_resume(struct device *dev)
>   */
>  static int azx_freeze_noirq(struct device *dev)
>  {
> +	struct snd_card *card = dev_get_drvdata(dev);
> +	struct azx *chip = card->private_data;
>  	struct pci_dev *pci = to_pci_dev(dev);
>  
> -	if (IS_SKL_PLUS(pci))
> +	if (chip->driver_type == AZX_DRIVER_SKL)
>  		pci_set_power_state(pci, PCI_D3hot);
>  
>  	return 0;
> @@ -1084,9 +1078,11 @@ static int azx_freeze_noirq(struct device *dev)
>  
>  static int azx_thaw_noirq(struct device *dev)
>  {
> +	struct snd_card *card = dev_get_drvdata(dev);
> +	struct azx *chip = card->private_data;
>  	struct pci_dev *pci = to_pci_dev(dev);
>  
> -	if (IS_SKL_PLUS(pci))
> +	if (chip->driver_type == AZX_DRIVER_SKL)
>  		pci_set_power_state(pci, PCI_D0);
>  
>  	return 0;
> @@ -1496,7 +1492,7 @@ static int check_position_fix(struct azx *chip, int fix)
>  		dev_dbg(chip->card->dev, "Using LPIB position fix\n");
>  		return POS_FIX_LPIB;
>  	}
> -	if (IS_SKL_PLUS(chip->pci)) {
> +	if (chip->driver_type == AZX_DRIVER_SKL) {
>  		dev_dbg(chip->card->dev, "Using SKL position fix\n");
>  		return POS_FIX_SKL;
>  	}
> @@ -1797,7 +1793,7 @@ static int azx_first_init(struct azx *chip)
>  		return -ENXIO;
>  	}
>  
> -	if (IS_SKL_PLUS(pci))
> +	if (chip->driver_type == AZX_DRIVER_SKL)
>  		snd_hdac_bus_parse_capabilities(bus);
>  
>  	/*
> @@ -2366,31 +2362,31 @@ static const struct pci_device_id azx_ids[] = {
>  	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH },
>  	/* Sunrise Point */
>  	{ PCI_DEVICE(0x8086, 0xa170),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
>  	/* Sunrise Point-LP */
>  	{ PCI_DEVICE(0x8086, 0x9d70),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
>  	/* Kabylake */
>  	{ PCI_DEVICE(0x8086, 0xa171),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
>  	/* Kabylake-LP */
>  	{ PCI_DEVICE(0x8086, 0x9d71),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
>  	/* Kabylake-H */
>  	{ PCI_DEVICE(0x8086, 0xa2f0),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
>  	/* Coffelake */
>  	{ PCI_DEVICE(0x8086, 0xa348),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE},
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
>  	/* Broxton-P(Apollolake) */
>  	{ PCI_DEVICE(0x8086, 0x5a98),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_BROXTON },
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
>  	/* Broxton-T */
>  	{ PCI_DEVICE(0x8086, 0x1a98),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_BROXTON },
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
>  	/* Gemini-Lake */
>  	{ PCI_DEVICE(0x8086, 0x3198),
> -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_BROXTON },
> +	  .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
>  	/* Haswell */
>  	{ PCI_DEVICE(0x8086, 0x0a0c),
>  	  .driver_data = AZX_DRIVER_HDMI | AZX_DCAPS_INTEL_HASWELL },

-- 
~Vinod

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

* Re: [PATCH] ALSA: hda - Add Coffelake PCI ID
  2017-06-16  6:36       ` Takashi Iwai
@ 2017-06-16  7:56         ` Vinod Koul
  0 siblings, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2017-06-16  7:56 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, patches.audio, lgirdwood, broonie,
	Subhransu S. Prusty, Megha Dey

On Fri, Jun 16, 2017 at 08:36:50AM +0200, Takashi Iwai wrote:
> On Wed, 14 Jun 2017 07:41:10 +0200,
> Takashi Iwai wrote:
> > 
> > On Wed, 14 Jun 2017 07:25:17 +0200,
> > Takashi Iwai wrote:
> > > 
> > > But while writing this, I noticed that Broxton-T seems forgotten.
> > > Doesn't it need the similar workaround for SKL+ chips?
> > 
> > I meant like below.
> 
> Can anyone confirm whether it is required or not?

Sorry for the delay

> There are two things applied specific to BXT: one is the generic SKL+
> quirk and another is a quick to reduce DMA latency.  Maybe the latter
> one is superfluous for BXT-T but only the former one is needed?

Lets keep former and if someone complains add the later

> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > Takashi
> > 
> > -- 8< --
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] ALSA: hda - Apply quirks to Broxton-T, too
> > 
> > Broxton-T was a forgotten child and we didn't apply the quirks
> > properly.  Let's fix it.
> > 
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/pci/hda/hda_intel.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index e3c696c46a21..774f72b26d13 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -369,7 +369,8 @@ enum {
> >  #define IS_KBL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa171)
> >  #define IS_KBL_LP(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9d71)
> >  #define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0)
> > -#define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
> > +#define IS_BXT(pci) ((pci)->vendor == 0x8086 && \
> > +		     ((pci)->device == 0x5a98 || (pci)->device == 0x1a98))
> >  #define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198)
> >  #define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
> >  #define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \
> > -- 
> > 2.13.1
> > 

-- 
~Vinod

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

end of thread, other threads:[~2017-06-16  7:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-14  4:21 [PATCH] ALSA: hda - Add Coffelake PCI ID Subhransu S. Prusty
2017-06-14  4:45 ` Vinod Koul
2017-06-14  5:25   ` Takashi Iwai
2017-06-14  5:41     ` Takashi Iwai
2017-06-16  6:36       ` Takashi Iwai
2017-06-16  7:56         ` Vinod Koul
2017-06-16  7:54     ` Vinod Koul
2017-06-14  7:34 ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox