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@suse.de] Sent: Wednesday, September 03, 2008 4:52 PM To: peerchen Cc: alsa-devel; linux-kernel; akpm; Peer Chen; Wei Ni Subject: Re: [alsa-devel][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@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
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