mbox series

[00/13] PCI: Define Intel PCI IDs and use them in drivers

Message ID 20230711125726.3509391-1-amadeuszx.slawinski@linux.intel.com
Headers show
Series PCI: Define Intel PCI IDs and use them in drivers | expand

Message

Amadeusz Sławiński July 11, 2023, 12:57 p.m. UTC 
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.

Changes from RFC:
 - Sort Intel PCI IDs before adding new ones
 - Fix ordering of new PCI IDs (Andy)
 - Define all used Intel IDs (Andy)
 - Add macros for controller type detection (Andy/Bjorn)
 - Add set of patches changing to use above macro (Andy/Bjorn)
 - Use PCI_DEVICE_DATA for Intel IDs in sound/pci/hda/hda_intel.c (Andy)
 - Commit message wording (Andy)
 - Remove unnecessary tabs (Andy)

Amadeusz Sławiński (13):
  PCI: Sort Intel PCI IDs by number
  PCI: Add Intel Audio DSP devices to pci_ids.h
  ALSA: hda: Add controller matching macros
  ALSA: hda: Use global PCI match macro
  ALSA: hda/i915:  Use global PCI match macro
  ASoC: Intel: Skylake: Use global PCI match macro
  ALSA: intel-dsp-config: Convert to PCI device IDs defines
  ALSA: hda: Convert to PCI device IDs defines
  ASoC: Intel: avs: Convert to PCI device IDs defines
  ASoC: Intel: avs: Convert to PCI device IDs defines
  ASoC: Intel: Skylake: Convert to PCI device IDs defines
  ASoC: SOF: Intel: Convert to PCI device IDs defines
  ASoC: Intel: sst: Convert to PCI device IDs defines

 include/linux/pci_ids.h                | 104 +++++--
 include/sound/hda_codec.h              |   3 -
 include/sound/hdaudio.h                |  27 ++
 sound/hda/hdac_i915.c                  |   7 +-
 sound/hda/intel-dsp-config.c           | 119 ++++----
 sound/pci/hda/hda_intel.c              | 373 ++++++++++---------------
 sound/soc/intel/atom/sst/sst.c         |   3 +-
 sound/soc/intel/atom/sst/sst.h         |   1 -
 sound/soc/intel/atom/sst/sst_pci.c     |   4 +-
 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-pcm.c      |   3 +-
 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 +-
 21 files changed, 384 insertions(+), 431 deletions(-)

Comments

Amadeusz Sławiński July 12, 2023, 12:16 p.m. UTC | #1 
On 7/11/2023 4:16 PM, Andy Shevchenko wrote:
> On Tue, Jul 11, 2023 at 02:57:25PM +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.
> 
> FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Oh, additional remark below.
> 
>> Acked-by: Mark Brown <broonie@kernel.org>
>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>> ---
>>   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..083659ddfe6b 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) },
> 
> Have we ever had APL-T? What is that? I remember that we have had two or
> three BXTs inside, and then products become for Broxton and Apollo Lake
> SoC codenames. I never have heard about -T...
> 

I've talked a bit with Cezary and it seems that 0x1a98 is BXT-M (not -T) 
and it's an RVP, BXT-M B0 to be specific. From what we know no BXT is 
available on market. Perhaps we can just remove it?