From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Libin" Subject: Re: HDA-Intel - Patch to support detecting HD Audio devices with PCI class code Date: Tue, 11 Nov 2008 16:39:27 +0800 Message-ID: <6077B97E85E7374DAE87B42B5AA9979707BA28@sshaexmb1.amd.com> References: <6077B97E85E7374DAE87B42B5AA9979707B88A@sshaexmb1.amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from SG2EHSOBE006.bigfish.com (outbound-sin.frontbridge.com [207.46.51.80]) by alsa0.perex.cz (Postfix) with ESMTP id 1AAF124555 for ; Tue, 11 Nov 2008 09:39:54 +0100 (CET) Content-Class: urn:content-classes:message In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@alsa-project.org Errors-To: alsa-devel-bounces@alsa-project.org To: Takashi Iwai Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org Thanks for the comments and I will refine the patch. > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Tuesday, November 11, 2008 4:17 PM > To: Yang, Libin > Cc: alsa-devel@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