[PATCH 0/5] ALSA/PCI: hda: add ARL-S support, config for MTL/LNL

[PATCH 0/5] ALSA/PCI: hda: add ARL-S support, config for MTL/LNL

[Pierre-Louis Bossart]

Add a new PCI ID and i915 connectivity check for ArrowLake-S, and
configuration updates for MTL/LNL.

Kai Vehmanen (2):
  ALSA: hda: add HD Audio PCI ID for Intel Arrow Lake-S
  ALSA: hda/i915: extend connectivity check to cover Intel ARL

Pierre-Louis Bossart (3):
  PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.
  ALSA: hda: intel-dsp-cfg: use common include for MeteorLake
  ALSA: hda: intel-dsp-cfg: add LunarLake support

 include/linux/pci_ids.h      |  1 +
 sound/hda/hdac_i915.c        | 14 ++++++++++----
 sound/hda/intel-dsp-config.c | 10 +++++++++-
 sound/pci/hda/hda_intel.c    |  2 ++
 4 files changed, 22 insertions(+), 5 deletions(-)

-- 
2.39.2

[PATCH 1/5] PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.

[Pierre-Louis Bossart]

Add part ID to common include file

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 include/linux/pci_ids.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 3066660cd39b..a6411aa4c331 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3058,6 +3058,7 @@
 #define PCI_DEVICE_ID_INTEL_HDA_RPL_S	0x7a50
 #define PCI_DEVICE_ID_INTEL_HDA_ADL_S	0x7ad0
 #define PCI_DEVICE_ID_INTEL_HDA_MTL	0x7e28
+#define PCI_DEVICE_ID_INTEL_HDA_ARL_S	0x7f50
 #define PCI_DEVICE_ID_INTEL_SCH_LPC	0x8119
 #define PCI_DEVICE_ID_INTEL_SCH_IDE	0x811a
 #define PCI_DEVICE_ID_INTEL_HDA_POULSBO	0x811b
-- 
2.39.2

Re: [PATCH 1/5] PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.

[Bjorn Helgaas]

On Wed, Aug 02, 2023 at 10:01:01AM -0500, Pierre-Louis Bossart wrote:
> Add part ID to common include file

Please drop period at end of subject and add one at the end of the
commit log.

Also mention the drivers that will use this new #define; looks like
hda_intel.c and ...

Well, actually, I only see that one use, which means we probably
shouldn't add this #define to pci_ids.h, per the comment at the top of
the file.  If there's only one use, use the hex ID in the driver (or
add a #define in the driver itself).

> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> ---
>  include/linux/pci_ids.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 3066660cd39b..a6411aa4c331 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -3058,6 +3058,7 @@
>  #define PCI_DEVICE_ID_INTEL_HDA_RPL_S	0x7a50
>  #define PCI_DEVICE_ID_INTEL_HDA_ADL_S	0x7ad0
>  #define PCI_DEVICE_ID_INTEL_HDA_MTL	0x7e28
> +#define PCI_DEVICE_ID_INTEL_HDA_ARL_S	0x7f50
>  #define PCI_DEVICE_ID_INTEL_SCH_LPC	0x8119
>  #define PCI_DEVICE_ID_INTEL_SCH_IDE	0x811a
>  #define PCI_DEVICE_ID_INTEL_HDA_POULSBO	0x811b
> -- 
> 2.39.2
> 

Re: [PATCH 1/5] PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.

[Takashi Iwai]

On Wed, 02 Aug 2023 17:52:26 +0200,
Bjorn Helgaas wrote:
> 
> On Wed, Aug 02, 2023 at 10:01:01AM -0500, Pierre-Louis Bossart wrote:
> > Add part ID to common include file
> 
> Please drop period at end of subject and add one at the end of the
> commit log.
> 
> Also mention the drivers that will use this new #define; looks like
> hda_intel.c and ...
> 
> Well, actually, I only see that one use, which means we probably
> shouldn't add this #define to pci_ids.h, per the comment at the top of
> the file.  If there's only one use, use the hex ID in the driver (or
> add a #define in the driver itself).

Judging from the previous patterns, the same ID could be required for
ASoC SOF driver, too, which isn't included in this patch set.  In
that case, it's worth to put to pci_ids.h.
(OTOH, it can be done at a later stage, too.)


thanks,

Takashi

Re: [PATCH 1/5] PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.

[Pierre-Louis Bossart]

On 8/2/23 10:57, Takashi Iwai wrote:
> On Wed, 02 Aug 2023 17:52:26 +0200,
> Bjorn Helgaas wrote:
>>
>> On Wed, Aug 02, 2023 at 10:01:01AM -0500, Pierre-Louis Bossart wrote:
>>> Add part ID to common include file
>>
>> Please drop period at end of subject and add one at the end of the
>> commit log.
>>
>> Also mention the drivers that will use this new #define; looks like
>> hda_intel.c and ...
>>
>> Well, actually, I only see that one use, which means we probably
>> shouldn't add this #define to pci_ids.h, per the comment at the top of
>> the file.  If there's only one use, use the hex ID in the driver (or
>> add a #define in the driver itself).
> 
> Judging from the previous patterns, the same ID could be required for
> ASoC SOF driver, too, which isn't included in this patch set.  In
> that case, it's worth to put to pci_ids.h.
> (OTOH, it can be done at a later stage, too.)

I am not following. we just agreed a couple of weeks ago to record ALL
Intel/HDaudio PCI IDs in the same pci_ids.h include file.

ArrowLake-S is the first addition to first file after the work done by
Cezary/Amadeusz. Yes it's required to be added since it'll be used in
other parts later on. But even if there was ONE use of this PCI ID, why
would we not add it for consistency to the global pci_ids.h file?
Takashi's hda_intel.c file would look really bad if we have a mix of
single-use PCIs and shared ones...

Oh and heads-up that I have a change for LunarLake that will require
Mark to pull the branch from Takashi :-)

Re: [PATCH 1/5] PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.

[Bjorn Helgaas]

On Wed, Aug 02, 2023 at 11:07:36AM -0500, Pierre-Louis Bossart wrote:
> On 8/2/23 10:57, Takashi Iwai wrote:
> > On Wed, 02 Aug 2023 17:52:26 +0200,
> > Bjorn Helgaas wrote:
> >> On Wed, Aug 02, 2023 at 10:01:01AM -0500, Pierre-Louis Bossart wrote:
> >>> Add part ID to common include file
> >>
> >> Please drop period at end of subject and add one at the end of the
> >> commit log.
> >>
> >> Also mention the drivers that will use this new #define; looks like
> >> hda_intel.c and ...
> >>
> >> Well, actually, I only see that one use, which means we probably
> >> shouldn't add this #define to pci_ids.h, per the comment at the top of
> >> the file.  If there's only one use, use the hex ID in the driver (or
> >> add a #define in the driver itself).
> > 
> > Judging from the previous patterns, the same ID could be required for
> > ASoC SOF driver, too, which isn't included in this patch set.  In
> > that case, it's worth to put to pci_ids.h.
> > (OTOH, it can be done at a later stage, too.)

When it becomes shared is the standard point at which we add to
pci_ids.h.

> I am not following. we just agreed a couple of weeks ago to record ALL
> Intel/HDaudio PCI IDs in the same pci_ids.h include file.

I'm not sure who "we" is here.  If it included me and I signed up to
it, I apologize for forgetting, and go ahead and add my:

  Acked-by: Bjorn Helgaas <bhelgaas@google.com>

I'm just pointing out the usual practice for pci_ids.h, as mentioned
in the file itself.

> ArrowLake-S is the first addition to first file after the work done by
> Cezary/Amadeusz. Yes it's required to be added since it'll be used in
> other parts later on. But even if there was ONE use of this PCI ID, why
> would we not add it for consistency to the global pci_ids.h file?
> Takashi's hda_intel.c file would look really bad if we have a mix of
> single-use PCIs and shared ones...

We already have a mix:

  static const struct pci_device_id azx_ids[] = {
        /* CPT */
        { PCI_DEVICE(0x8086, 0x1c20),
          .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH_NOPM },
        /* PBG */
        { PCI_DEVICE(0x8086, 0x1d20),
          .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH_NOPM },
        /* Panther Point */

I think the reason we don't add names used only once is because it
makes backporting things harder because it leads to more merge
conflicts in pci_ids.h.

Bjorn

Re: [PATCH 1/5] PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.

[Mark Brown]

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

On Wed, Aug 02, 2023 at 11:25:41AM -0500, Bjorn Helgaas wrote:
> On Wed, Aug 02, 2023 at 11:07:36AM -0500, Pierre-Louis Bossart wrote:

> > I am not following. we just agreed a couple of weeks ago to record ALL
> > Intel/HDaudio PCI IDs in the same pci_ids.h include file.

> I'm not sure who "we" is here.  If it included me and I signed up to
> it, I apologize for forgetting, and go ahead and add my:

>   Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> I'm just pointing out the usual practice for pci_ids.h, as mentioned
> in the file itself.

I think the thing with these drivers is that we know they will become
shared in fairly short order so it just becomes overhead to add then
move the identifier and update.

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

Re: [PATCH 1/5] PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.

[Pierre-Louis Bossart]

On 8/2/23 11:34, Mark Brown wrote:
> On Wed, Aug 02, 2023 at 11:25:41AM -0500, Bjorn Helgaas wrote:
>> On Wed, Aug 02, 2023 at 11:07:36AM -0500, Pierre-Louis Bossart wrote:
> 
>>> I am not following. we just agreed a couple of weeks ago to record ALL
>>> Intel/HDaudio PCI IDs in the same pci_ids.h include file.
> 
>> I'm not sure who "we" is here.  If it included me and I signed up to
>> it, I apologize for forgetting, and go ahead and add my:
> 
>>   Acked-by: Bjorn Helgaas <bhelgaas@google.com>

This was the original thread for the record

https://lore.kernel.org/alsa-devel/20230717114511.484999-3-amadeuszx.slawinski@linux.intel.com/

>> I'm just pointing out the usual practice for pci_ids.h, as mentioned
>> in the file itself.

You're actually right that we didn't talk about the minimum criterion to
add a PCI ID to this file. To me it was a central place similar to the
cpu ids, etc., if it wasn't clear to everyone than it's good to agree on
this second point.

> I think the thing with these drivers is that we know they will become
> shared in fairly short order so it just becomes overhead to add then
> move the identifier and update.

Indeed, the sharing part is not always predictable and is subject to
roadmap changes made above my pay grade.

The intended use of the devices can vary as well, some PCI IDs for
desktops are intended to be used only by snd-hda-intel, but if one OEM
starts adding digital microphones then the SOF driver becomes required.

So rather than force everyone to follow changes at Intel or Intel
customers it's simpler to just add PCI IDs in pci_ids.h. We typically
deal with 3-4 PCI IDS per year

[PATCH 2/5] ALSA: hda: add HD Audio PCI ID for Intel Arrow Lake-S

[Pierre-Louis Bossart]

From: Kai Vehmanen <kai.vehmanen@linux.intel.com>

Add HD Audio PCI ID for Intel Arrow Lake-S platform.

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/pci/hda/hda_intel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 176567f0d0e0..765d95e79861 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2499,6 +2499,8 @@ static const struct pci_device_id azx_ids[] = {
 	{ PCI_DEVICE_DATA(INTEL, HDA_MTL, AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE) },
 	/* Lunarlake-P */
 	{ PCI_DEVICE_DATA(INTEL, HDA_LNL_P, AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE) },
+	/* Arrow Lake-S */
+	{ PCI_DEVICE_DATA(INTEL, HDA_ARL_S, AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE) },
 	/* Apollolake (Broxton-P) */
 	{ PCI_DEVICE_DATA(INTEL, HDA_APL, AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON) },
 	/* Gemini-Lake */
-- 
2.39.2

[PATCH 3/5] ALSA: hda: intel-dsp-cfg: use common include for MeteorLake

[Pierre-Louis Bossart]

This was not updated in Commit 0cd0a7c2c599 ("ALSA: intel-dsp-config: Convert to PCI device IDs defines")

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/hda/intel-dsp-config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 48bd1fb06f26..1abe65f0ba1b 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -461,7 +461,7 @@ static const struct config_entry config_table[] = {
 	/* Meteorlake-P */
 	{
 		.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC_OR_SOUNDWIRE,
-		.device = 0x7e28,
+		.device = PCI_DEVICE_ID_INTEL_HDA_MTL,
 	},
 #endif
 
-- 
2.39.2

[PATCH 4/5] ALSA: hda: intel-dsp-cfg: add LunarLake support

[Pierre-Louis Bossart]

One more PCI ID for the road.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/hda/intel-dsp-config.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 1abe65f0ba1b..dcf2453138a5 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -465,6 +465,14 @@ static const struct config_entry config_table[] = {
 	},
 #endif
 
+/* Lunar Lake */
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_LUNARLAKE)
+	/* Lunarlake-P */
+	{
+		.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC_OR_SOUNDWIRE,
+		.device = PCI_DEVICE_ID_INTEL_HDA_LNL_P,
+	},
+#endif
 };
 
 static const struct config_entry *snd_intel_dsp_find_config
-- 
2.39.2

[PATCH 5/5] ALSA: hda/i915: extend connectivity check to cover Intel ARL

[Pierre-Louis Bossart]

From: Kai Vehmanen <kai.vehmanen@linux.intel.com>

Expand the HDA/I915 connectivity check to correctly handle
the PCI topology used in some Intel Arrow Lake products.

Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Tested-by: "T, Arun" <arun.t@intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/hda/hdac_i915.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index 2a451ff4fe6a..b428537f284c 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -75,16 +75,22 @@ static bool connectivity_check(struct pci_dev *i915, struct pci_dev *hdac)
 	if (bus_a == bus_b)
 		return true;
 
+	bus_a = bus_a->parent;
+	bus_b = bus_b->parent;
+
+	/* connected via parent bus (may be NULL!) */
+	if (bus_a == bus_b)
+		return true;
+
+	if (!bus_a || !bus_b)
+		return false;
+
 	/*
 	 * on i915 discrete GPUs with embedded HDA audio, the two
 	 * devices are connected via 2nd level PCI bridge
 	 */
 	bus_a = bus_a->parent;
 	bus_b = bus_b->parent;
-	if (!bus_a || !bus_b)
-		return false;
-	bus_a = bus_a->parent;
-	bus_b = bus_b->parent;
 	if (bus_a && bus_a == bus_b)
 		return true;
 
-- 
2.39.2

Re: [PATCH 0/5] ALSA/PCI: hda: add ARL-S support, config for MTL/LNL

[Takashi Iwai]

On Wed, 02 Aug 2023 17:01:00 +0200,
Pierre-Louis Bossart wrote:
> 
> Add a new PCI ID and i915 connectivity check for ArrowLake-S, and
> configuration updates for MTL/LNL.
> 
> Kai Vehmanen (2):
>   ALSA: hda: add HD Audio PCI ID for Intel Arrow Lake-S
>   ALSA: hda/i915: extend connectivity check to cover Intel ARL
> 
> Pierre-Louis Bossart (3):
>   PCI: add ArrowLake-S PCI ID for Intel HDAudio subsystem.
>   ALSA: hda: intel-dsp-cfg: use common include for MeteorLake
>   ALSA: hda: intel-dsp-cfg: add LunarLake support

Now I applied all patches.

Mark can pull the changes from tags/hda-intel-6.6 if those changes are
needed for further ASoC patches.  It's based on the previous hda
pci-id changes on top of 6.5-rc2.


thanks,

Takashi