PCI IDs for Intel HDA are duplicated across quite a few drivers, due to
various configurations and historical reasons. Currently almost all uses
of HDA PCI IDs have corresponding comment telling which platform it is.
Additionally there are some inconsistencies between drivers about which
ID corresponds to which device.
Simplify things, by adding PCI IDs to global header and make use of them
in drivers. This allows for removal of comments by having IDs themselves
being self explanatory. Additionally it allows for removal of existing
inconsistencies by having one source of truth.
Amadeusz Sławiński (8):
PCI: Add Intel Audio DSP devices to pci_ids.h
ALSA: intel-dsp-config: Update PCI ID list
ALSA: hda: Update PCI ID list
ALSA: hda/i915: Update PCI IDs
ASoC: Intel: avs: Update PCI ID list
ASoC: Intel: avs: Update PCI ID list
ASoC: Intel: Skylake: Update PCI ID list
ASoC: SOF: Intel: Update PCI ID list
include/linux/pci_ids.h | 44 ++++
sound/hda/hdac_i915.c | 8 +-
sound/hda/intel-dsp-config.c | 119 +++++-----
sound/pci/hda/hda_intel.c | 292 ++++++++++++-------------
sound/soc/intel/avs/board_selection.c | 10 +-
sound/soc/intel/avs/core.c | 16 +-
sound/soc/intel/skylake/skl-messages.c | 16 +-
sound/soc/intel/skylake/skl.c | 36 +--
sound/soc/sof/intel/pci-apl.c | 9 +-
sound/soc/sof/intel/pci-cnl.c | 15 +-
sound/soc/sof/intel/pci-icl.c | 12 +-
sound/soc/sof/intel/pci-mtl.c | 3 +-
sound/soc/sof/intel/pci-skl.c | 6 +-
sound/soc/sof/intel/pci-tgl.c | 45 ++--
sound/soc/sof/intel/pci-tng.c | 3 +-
15 files changed, 311 insertions(+), 323 deletions(-)
--
2.34.1
Use PCI device IDs from pci_ids.h header and while at it change to using
PCI_DEVICE_DATA macro, to simplify declarations.
Signed-off-by: Amadeusz Sławiński <[email protected]>
---
sound/soc/sof/intel/pci-apl.c | 9 +++----
sound/soc/sof/intel/pci-cnl.c | 15 ++++--------
sound/soc/sof/intel/pci-icl.c | 12 ++++------
sound/soc/sof/intel/pci-mtl.c | 3 +--
sound/soc/sof/intel/pci-skl.c | 6 ++---
sound/soc/sof/intel/pci-tgl.c | 45 ++++++++++++-----------------------
sound/soc/sof/intel/pci-tng.c | 3 +--
7 files changed, 31 insertions(+), 62 deletions(-)
diff --git a/sound/soc/sof/intel/pci-apl.c b/sound/soc/sof/intel/pci-apl.c
index 69cad5a6bc72..aadacc283220 100644
--- a/sound/soc/sof/intel/pci-apl.c
+++ b/sound/soc/sof/intel/pci-apl.c
@@ -85,12 +85,9 @@ static const struct sof_dev_desc glk_desc = {
/* PCI IDs */
static const struct pci_device_id sof_pci_ids[] = {
- { PCI_DEVICE(0x8086, 0x5a98), /* BXT-P (ApolloLake) */
- .driver_data = (unsigned long)&bxt_desc},
- { PCI_DEVICE(0x8086, 0x1a98),/* BXT-T */
- .driver_data = (unsigned long)&bxt_desc},
- { PCI_DEVICE(0x8086, 0x3198), /* GeminiLake */
- .driver_data = (unsigned long)&glk_desc},
+ { PCI_DEVICE_DATA(INTEL, HDA_APL, &bxt_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_APL_T, &bxt_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_GML, &glk_desc) },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, sof_pci_ids);
diff --git a/sound/soc/sof/intel/pci-cnl.c b/sound/soc/sof/intel/pci-cnl.c
index 8895508a0be6..7fbfaff7ea16 100644
--- a/sound/soc/sof/intel/pci-cnl.c
+++ b/sound/soc/sof/intel/pci-cnl.c
@@ -120,16 +120,11 @@ static const struct sof_dev_desc cml_desc = {
/* PCI IDs */
static const struct pci_device_id sof_pci_ids[] = {
- { PCI_DEVICE(0x8086, 0x9dc8), /* CNL-LP */
- .driver_data = (unsigned long)&cnl_desc},
- { PCI_DEVICE(0x8086, 0xa348), /* CNL-H */
- .driver_data = (unsigned long)&cfl_desc},
- { PCI_DEVICE(0x8086, 0x02c8), /* CML-LP */
- .driver_data = (unsigned long)&cml_desc},
- { PCI_DEVICE(0x8086, 0x06c8), /* CML-H */
- .driver_data = (unsigned long)&cml_desc},
- { PCI_DEVICE(0x8086, 0xa3f0), /* CML-S */
- .driver_data = (unsigned long)&cml_desc},
+ { PCI_DEVICE_DATA(INTEL, HDA_CNL_LP, &cnl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_CNL_H, &cfl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_CML_LP, &cml_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_CML_H, &cml_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_CML_S, &cml_desc) },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, sof_pci_ids);
diff --git a/sound/soc/sof/intel/pci-icl.c b/sound/soc/sof/intel/pci-icl.c
index 5fb5a820693e..db3e2965eb4b 100644
--- a/sound/soc/sof/intel/pci-icl.c
+++ b/sound/soc/sof/intel/pci-icl.c
@@ -86,14 +86,10 @@ static const struct sof_dev_desc jsl_desc = {
/* PCI IDs */
static const struct pci_device_id sof_pci_ids[] = {
- { PCI_DEVICE(0x8086, 0x34C8), /* ICL-LP */
- .driver_data = (unsigned long)&icl_desc},
- { PCI_DEVICE(0x8086, 0x3dc8), /* ICL-H */
- .driver_data = (unsigned long)&icl_desc},
- { PCI_DEVICE(0x8086, 0x38c8), /* ICL-N */
- .driver_data = (unsigned long)&jsl_desc},
- { PCI_DEVICE(0x8086, 0x4dc8), /* JSL-N */
- .driver_data = (unsigned long)&jsl_desc},
+ { PCI_DEVICE_DATA(INTEL, HDA_ICL_LP, &icl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_ICL_H, &icl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_ICL_N, &jsl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_JSL_N, &jsl_desc) },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, sof_pci_ids);
diff --git a/sound/soc/sof/intel/pci-mtl.c b/sound/soc/sof/intel/pci-mtl.c
index e276e1e37fed..7868b0827e84 100644
--- a/sound/soc/sof/intel/pci-mtl.c
+++ b/sound/soc/sof/intel/pci-mtl.c
@@ -52,8 +52,7 @@ static const struct sof_dev_desc mtl_desc = {
/* PCI IDs */
static const struct pci_device_id sof_pci_ids[] = {
- { PCI_DEVICE(0x8086, 0x7E28), /* MTL */
- .driver_data = (unsigned long)&mtl_desc},
+ { PCI_DEVICE_DATA(INTEL, HDA_MTL, &mtl_desc) },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, sof_pci_ids);
diff --git a/sound/soc/sof/intel/pci-skl.c b/sound/soc/sof/intel/pci-skl.c
index 5e69af6eed34..a6588b138a8c 100644
--- a/sound/soc/sof/intel/pci-skl.c
+++ b/sound/soc/sof/intel/pci-skl.c
@@ -69,10 +69,8 @@ static struct sof_dev_desc kbl_desc = {
/* PCI IDs */
static const struct pci_device_id sof_pci_ids[] = {
- /* Sunrise Point-LP */
- { PCI_DEVICE(0x8086, 0x9d70), .driver_data = (unsigned long)&skl_desc},
- /* KBL */
- { PCI_DEVICE(0x8086, 0x9d71), .driver_data = (unsigned long)&kbl_desc},
+ { PCI_DEVICE_DATA(INTEL, HDA_SKL_LP, &skl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_KBL_LP, &kbl_desc) },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, sof_pci_ids);
diff --git a/sound/soc/sof/intel/pci-tgl.c b/sound/soc/sof/intel/pci-tgl.c
index ca37ff1bbd2a..c7108ea3c02b 100644
--- a/sound/soc/sof/intel/pci-tgl.c
+++ b/sound/soc/sof/intel/pci-tgl.c
@@ -284,36 +284,21 @@ static const struct sof_dev_desc rpl_desc = {
/* PCI IDs */
static const struct pci_device_id sof_pci_ids[] = {
- { PCI_DEVICE(0x8086, 0xa0c8), /* TGL-LP */
- .driver_data = (unsigned long)&tgl_desc},
- { PCI_DEVICE(0x8086, 0x43c8), /* TGL-H */
- .driver_data = (unsigned long)&tglh_desc},
- { PCI_DEVICE(0x8086, 0x4b55), /* EHL */
- .driver_data = (unsigned long)&ehl_desc},
- { PCI_DEVICE(0x8086, 0x4b58), /* EHL */
- .driver_data = (unsigned long)&ehl_desc},
- { PCI_DEVICE(0x8086, 0x7ad0), /* ADL-S */
- .driver_data = (unsigned long)&adls_desc},
- { PCI_DEVICE(0x8086, 0x7a50), /* RPL-S */
- .driver_data = (unsigned long)&rpls_desc},
- { PCI_DEVICE(0x8086, 0x51c8), /* ADL-P */
- .driver_data = (unsigned long)&adl_desc},
- { PCI_DEVICE(0x8086, 0x51c9), /* ADL-PS */
- .driver_data = (unsigned long)&adl_desc},
- { PCI_DEVICE(0x8086, 0x51ca), /* RPL-P */
- .driver_data = (unsigned long)&rpl_desc},
- { PCI_DEVICE(0x8086, 0x51cb), /* RPL-P */
- .driver_data = (unsigned long)&rpl_desc},
- { PCI_DEVICE(0x8086, 0x51cc), /* ADL-M */
- .driver_data = (unsigned long)&adl_desc},
- { PCI_DEVICE(0x8086, 0x51cd), /* ADL-P */
- .driver_data = (unsigned long)&adl_desc},
- { PCI_DEVICE(0x8086, 0x51ce), /* RPL-M */
- .driver_data = (unsigned long)&rpl_desc},
- { PCI_DEVICE(0x8086, 0x51cf), /* RPL-PX */
- .driver_data = (unsigned long)&rpl_desc},
- { PCI_DEVICE(0x8086, 0x54c8), /* ADL-N */
- .driver_data = (unsigned long)&adl_n_desc},
+ { PCI_DEVICE_DATA(INTEL, HDA_TGL_LP, &tgl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_TGL_H, &tglh_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_EHL_0, &ehl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_EHL_3, &ehl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_ADL_S, &adls_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_RPL_S, &rpls_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_ADL_P, &adl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_ADL_PS, &adl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_RPL_P_0, &rpl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_RPL_P_1, &rpl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_ADL_M, &adl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_ADL_PX, &adl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_RPL_M, &rpl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_RPL_PX, &rpl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_ADL_N, &adl_n_desc) },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, sof_pci_ids);
diff --git a/sound/soc/sof/intel/pci-tng.c b/sound/soc/sof/intel/pci-tng.c
index 8c22a00266c0..8a98c6229807 100644
--- a/sound/soc/sof/intel/pci-tng.c
+++ b/sound/soc/sof/intel/pci-tng.c
@@ -225,8 +225,7 @@ static const struct sof_dev_desc tng_desc = {
/* PCI IDs */
static const struct pci_device_id sof_pci_ids[] = {
- { PCI_DEVICE(0x8086, 0x119a),
- .driver_data = (unsigned long)&tng_desc},
+ { PCI_DEVICE_DATA(INTEL, HDA_MRFLD, &tng_desc) },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, sof_pci_ids);
--
2.34.1
Use PCI device IDs from pci_ids.h header and while at it change to using
PCI_DEVICE_DATA macro, to simplify declarations.
Signed-off-by: Amadeusz Sławiński <[email protected]>
---
sound/soc/intel/avs/core.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 637501850728..2ba179e31e43 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -745,14 +745,14 @@ static const struct avs_spec apl_desc = {
};
static const struct pci_device_id avs_ids[] = {
- { PCI_VDEVICE(INTEL, 0x9d70), (unsigned long)&skl_desc }, /* SKL */
- { PCI_VDEVICE(INTEL, 0xa170), (unsigned long)&skl_desc }, /* SKL-H */
- { PCI_VDEVICE(INTEL, 0x9d71), (unsigned long)&skl_desc }, /* KBL */
- { PCI_VDEVICE(INTEL, 0xa171), (unsigned long)&skl_desc }, /* KBL-H */
- { PCI_VDEVICE(INTEL, 0xa2f0), (unsigned long)&skl_desc }, /* KBL-S */
- { PCI_VDEVICE(INTEL, 0xa3f0), (unsigned long)&skl_desc }, /* CML-V */
- { PCI_VDEVICE(INTEL, 0x5a98), (unsigned long)&apl_desc }, /* APL */
- { PCI_VDEVICE(INTEL, 0x3198), (unsigned long)&apl_desc }, /* GML */
+ { PCI_DEVICE_DATA(INTEL, HDA_SKL_LP, &skl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_SKL, &skl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_KBL_LP, &skl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_KBL, &skl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_KBL_H, &skl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_CML_S, &skl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_APL, &apl_desc) },
+ { PCI_DEVICE_DATA(INTEL, HDA_GML, &apl_desc) },
{ 0 }
};
MODULE_DEVICE_TABLE(pci, avs_ids);
--
2.34.1
On Wed, Jun 28, 2023 at 10:51:27PM +0200, Amadeusz Sławiński wrote:
> PCI IDs for Intel HDA are duplicated across quite a few drivers, due to
> various configurations and historical reasons. Currently almost all uses
> of HDA PCI IDs have corresponding comment telling which platform it is.
> Additionally there are some inconsistencies between drivers about which
> ID corresponds to which device.
Acked-by: Mark Brown <[email protected]>
On Wed, Jun 28, 2023 at 10:51:27PM +0200, Amadeusz Sławiński wrote:
> PCI IDs for Intel HDA are duplicated across quite a few drivers, due to
> various configurations and historical reasons. Currently almost all uses
> of HDA PCI IDs have corresponding comment telling which platform it is.
> Additionally there are some inconsistencies between drivers about which
> ID corresponds to which device.
>
> Simplify things, by adding PCI IDs to global header and make use of them
> in drivers. This allows for removal of comments by having IDs themselves
> being self explanatory. Additionally it allows for removal of existing
> inconsistencies by having one source of truth.
I'm in favour of this series. It allows to use PCI_DEVICE_DATA() in many places.
With that said, I think you can also add some more definitions to PCI IDs header
for the sake of being able to use that macro.
--
With Best Regards,
Andy Shevchenko
On Wed, Jun 28, 2023 at 10:51:35PM +0200, Amadeusz Sławiński wrote:
> Use PCI device IDs from pci_ids.h header and while at it change to using
> PCI_DEVICE_DATA macro, to simplify declarations.
PCI_DEVICE_DATA()
...
> + { PCI_DEVICE_DATA(INTEL, HDA_APL, &bxt_desc) },
TAB(s) vs single space?
...
> static const struct pci_device_id sof_pci_ids[] = {
> - { PCI_DEVICE(0x8086, 0x119a),
> - .driver_data = (unsigned long)&tng_desc},
> + { PCI_DEVICE_DATA(INTEL, HDA_MRFLD, &tng_desc) },
Is it HDA? IIRC it was called Intel SST or so.
> { 0, }
> };
Also with the file name, I think the ID itself should use TNG suffix.
--
With Best Regards,
Andy Shevchenko
On Wed, Jun 28, 2023 at 10:51:32PM +0200, Amadeusz Sławiński wrote:
> Use PCI device IDs from pci_ids.h header and while at it change to using
> PCI_DEVICE_DATA macro, to simplify declarations.
PCI_DEVICE_DATA()
--
With Best Regards,
Andy Shevchenko
On 6/28/23 16:49, Andy Shevchenko wrote:
> On Wed, Jun 28, 2023 at 10:51:27PM +0200, Amadeusz Sławiński wrote:
>> PCI IDs for Intel HDA are duplicated across quite a few drivers, due to
>> various configurations and historical reasons. Currently almost all uses
>> of HDA PCI IDs have corresponding comment telling which platform it is.
>> Additionally there are some inconsistencies between drivers about which
>> ID corresponds to which device.
>>
>> Simplify things, by adding PCI IDs to global header and make use of them
>> in drivers. This allows for removal of comments by having IDs themselves
>> being self explanatory. Additionally it allows for removal of existing
>> inconsistencies by having one source of truth.
>
> I'm in favour of this series. It allows to use PCI_DEVICE_DATA() in many places.
> With that said, I think you can also add some more definitions to PCI IDs header
> for the sake of being able to use that macro.
I don't have any objections on the change.
The big open is how we add new definitions without a 3-way deadlock
between PCI, sound and ASoC trees, and how those definitions can be
added to the -stable trees.
This isn't an hypothetical case, we have 2 pending submissions for
LunarLake [1] and ArrowLake [2] which will be provided as soon as the
merge window closes.
It's not clear to me if Bjorn is ok to let those audio-specific PCI IDs
go the audio trees, and how things would work between Mark and Takashi.
[1] https://github.com/thesofproject/linux/pull/4425
[2] https://github.com/thesofproject/linux/pull/4437