[alsa-devel] HDA-Intel - Patch to support detecting HD Audio devices with PCI class code

[Yang, Libin]

Thanks for the comments and I will refine the patch.

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Tuesday, November 11, 2008 4:17 PM
> To: Yang, Libin
> Cc: alsa-devel at alsa-project.org
> Subject: Re:
> 
> At Tue, 11 Nov 2008 10:43:12 +0800,
> Yang, Libin wrote:
> >
> > Hi Takashi,
> >
> > This patch is to support detecting new AMD HD Audio devices with HDA
PCI
> > class code. Would you please review it? Thanks a lot!
> 
> Thanks for the patch!  Some quick review comments below...
> 
> 
> > --- alsa-driver-1.0.18.orig/alsa-kernel/pci/hda/hda_intel.c
> > 2008-10-29 20:41:35.000000000 +0800
> > +++ alsa-driver-1.0.18/alsa-kernel/pci/hda/hda_intel.c
2008-11-11
> > 18:33:14.000000000 +0800
> > @@ -291,6 +291,7 @@
> >  /* Define VIA HD Audio Device ID*/
> >  #define VIA_HDAC_DEVICE_ID		0x3288
> >
> > +#define PCI_CLASS_MULTIMEDIA_HDA	0x040300
> 
> Put this into include/linux/pci_ids.h as a common definition.
> And, IMO, it'd be better to name it *_HD_AUDIO than *_HDA.
> 
> >
> >  /*
> >   */
> > @@ -410,6 +411,7 @@
> >  	AZX_DRIVER_ULI,
> >  	AZX_DRIVER_NVIDIA,
> >  	AZX_DRIVER_TERA,
> > +	AZX_DRIVER_AMD_AUTO,
> 
> We can create a more generic entry, say, AZX_DRIVER_GENERIC.
> 
> 
> > @@ -423,6 +425,7 @@
> >  	[AZX_DRIVER_ULI] = "HDA ULI M5461",
> >  	[AZX_DRIVER_NVIDIA] = "HDA NVidia",
> >  	[AZX_DRIVER_TERA] = "HDA Teradici",
> > +	[AZX_DRIVER_AMD_AUTO] = "HDA AMD",
> 
> And, make it [AZX_DRIVER_GENERIC] = "HD-Audio Generic" or so.
> 
> 
> >  static unsigned int azx_default_codecs[AZX_NUM_DRIVERS]
__devinitdata =
> > {
> >  	[AZX_DRIVER_ICH] = 3,
> >  	[AZX_DRIVER_ATI] = 3,
> > +	[AZX_DRIVER_AMD_AUTO] = 3,
> 
> I thought you'll have up to 4 codecs?
> 
> 
> >  static int __devinit azx_codec_create(struct azx *chip, const char
> > *model,
> > @@ -2146,6 +2150,7 @@
> >  			chip->playback_streams = ULI_NUM_PLAYBACK;
> >  			chip->capture_streams = ULI_NUM_CAPTURE;
> >  			break;
> > +		case AZX_DRIVER_AMD_AUTO:
> 
> AZX_DRIVER_GENERIC can go to "default".
> 
> >  		case AZX_DRIVER_ATIHDMI:
> >  			chip->playback_streams = ATIHDMI_NUM_PLAYBACK;
> >  			chip->capture_streams = ATIHDMI_NUM_CAPTURE;
> > @@ -2373,6 +2378,9 @@
> >  	{ PCI_DEVICE(0x10de, 0x0bd7), .driver_data = AZX_DRIVER_NVIDIA
> > },
> >  	/* Teradici */
> >  	{ PCI_DEVICE(0x6549, 0x1200), .driver_data = AZX_DRIVER_TERA },
> > +	/* AMD Generic, PCI class code and Vendor ID for HD Audio */
> > +	{ PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> > +	  PCI_CLASS_MULTIMEDIA_HDA, 0xffffff, AZX_DRIVER_AMD_AUTO },
> 
> Use PCI_DEVICE() macro and C99 style initialization, such as,
> 
> 	{ PCI_DEVICE(PCI_VENDOR_ID_ATI, PCI_ANY_ID),
> 	  .class = PCI_CLASS_MULTIMEDIA_HD_AUDIO,
> 	  .class_mask = 0xffffff,
> 	  .driver_data = AZX_DRIVER_GENERIC },
> 
> thanks,
> 
> Takashi