At Wed, 3 Sep 2008 19:05:47 +0800, Wei Ni wrote: > > > Thanks for your comments. Some reply below: > > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Wednesday, September 03, 2008 4:52 PM > To: peerchen > Cc: alsa-devel; linux-kernel; akpm; Peer Chen; Wei Ni > Subject: Re: [PATCH] add the nvidia HDMI codec driver for > MCP77/79 > > At Wed, 3 Sep 2008 15:58:17 +0800, > peerchen wrote: > > > > Add the nvidia HDMI codec driver for MCP77/79. > > The patch based on kernel 2.6.27-rc5. > > > > Signed-off-by: Wei Ni <wni@xxxxxxxxxx> > > Signed-off-by: Peer Chen <peerchen@xxxxxxxxx> > > Thanks for the patch. Some comments below. > > > > diff -uprN -X linux-2.6.26-Orig/Documentation/dontdiff > linux-2.6.26-Orig/sound/pci/hda/hda_intel.c > linux-2.6.26-New/sound/pci/hda/hda_intel.c > > --- linux-2.6.26-Orig/sound/pci/hda/hda_intel.c 2008-08-28 > 13:47:20.000000000 +0800 > > +++ linux-2.6.26-New/sound/pci/hda/hda_intel.c 2008-08-28 > 15:33:51.000000000 +0800 > > @@ -223,8 +223,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO > > #define RIRB_INT_MASK 0x05 > > > > /* STATESTS int mask: SD2,SD1,SD0 */ > > -#define AZX_MAX_CODECS 3 > > -#define STATESTS_INT_MASK 0x07 > > +#define AZX_MAX_CODECS 4 > > +#define STATESTS_INT_MASK 0x0f > > This may break other platforms. Expanding STATESTS_INT_MASK is OK and > would be harmless, but AZX_MAX_CODECS is not. > AZX_MAX_CODECS is the default number of max codecs. The chipset > specific max number of codecs is defined in azx_max_codecs[]. > If Nvidia chipsets supports properly more than 3, change > azx_max_codecs[AZX_DRIVER_NVIDIA] to 4. > > ==========Wei Ni's Answer: ============== > There are 2 codec connect to the nvidia AZA controller, one is realtek, > and the other is nvidia HDMI codec. > In azx_codec_create(), the chip->codec_mask indicate codec connection. > This chip->codec_mask=0x9, bit0 indicate realtek codec, and bit3 > indicate hdmi codec. > In this routine, it use: > ...... > for (c = 0; c < AZX_MAX_CODECS; c++) { > if ((chip->codec_mask & (1 << c)) & codec_probe_mask) { > struct hda_codec *codec; > err = snd_hda_codec_new(chip->bus, c, &codec); > if (err < 0) > continue; > codecs++; > if (codec->afg) > audio_codecs++; > } > } > if (!audio_codecs) { > /* probe additional slots if no codec is found */ > for (; c < azx_max_codecs[chip->driver_type]; c++) { > if ((chip->codec_mask & (1 << c)) & > codec_probe_mask) { > err = snd_hda_codec_new(chip->bus, c, > NULL); > if (err < 0) > continue; > codecs++; > } > } > } > ...... > According to these codes, It only can get the realtek codec and can not > get hdmi codec, if just change azx_max_codecs[AZX_DRIVER_NVIDIA] to 4 > I think if remove "if (!audio_codecs)", it will be better, and I only > need to chang azx_max_codecs[AZX_DRIVER_NVIDIA] to 4 Well, the check is there for a good reason. There are many machines with broken BIOS reporting the 4th slot available although it isn't actually. It's a workaround to skip such a bogus information. So, it cannot be changed so easily. We need to sort out this in a better way... Oh well. > > +static unsigned short convert_from_spdif_status_nv(unsigned int > sbits) (snip) > These are almost identical with the functions in hda_codec.c. > Is your intention to allow a different set of IEC958 status bits > controls for HDMI *in addition* to SPDIF? If so, we should change > the codes in hda_codec.c to allow other control names. > > ==========Wei Ni's Answer: ============== > There are 2 codec connect to the nvidia AZA controller, one is realtek, > the other is nvidia HDMI codec. > If I use the default SPDIF routine: snd_hda_create_spdif_out_ctls(), the > driver can not be insmod success. > I traced this issue, the realtek's SPDIF used this IEC958 first, and > then HDMI codec can't use IEC958 again. > ( I traced it based on linux-2.6.25 ) 2.6.25 is too old as a reference. The fix for multiple SPDIF devices is already on the recent kernel. Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
