[alsa-devel] [PATCH 0/5] updates for Intel SandyBridge/CougarPoint HDMI codec
Hi Takashi,
The Intel SandyBridge/CougarPoint HDMI codec is very similar to the IbexPeak HDMI codec, except that it exports one more Audio Converter.
Now we have 3 Audio Converters, each of them present one HDMI PCM device: (and expect a 4th HDMI PCM device in future)
$ aplay -l | grep HDMI card 0: PCH [HDA Intel PCH], device 3: INTEL HDMI 0 [INTEL HDMI 0] card 0: PCH [HDA Intel PCH], device 7: INTEL HDMI 1 [INTEL HDMI 1] card 0: PCH [HDA Intel PCH], device 8: INTEL HDMI 2 [INTEL HDMI 2]
So ALSA has to support more than 8 PCM playback devices now. The first two patches do this dirty work, however I'm not sure if they will introduce some side effects.
The last three patches are more straightforward and safe.
Thanks, Fengguang
Reserve 32 minor numbers for PCM playback devices.
The Intel SandyBridge HDMI audio codec provides 3 PCM devices with indices 3, 7, 8. Among which the device 8's minor number will be overlapped with the first capture device's minor number in the current static minor number allocation scheme.
Also increase SNDRV_PCM_DEVICES to make pcm_dev_bits big enough to hold the increasing number of PCM devices.
Signed-off-by: Wu Fengguang fengguang.wu@intel.com --- include/sound/minors.h | 16 ++++++++-------- include/sound/pcm.h | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-)
--- sound-2.6.orig/include/sound/minors.h 2010-04-16 16:03:28.000000000 +0800 +++ sound-2.6/include/sound/minors.h 2010-05-11 13:02:48.000000000 +0800 @@ -23,23 +23,23 @@
#define SNDRV_OS_MINORS 256
-#define SNDRV_MINOR_DEVICES 32 -#define SNDRV_MINOR_CARD(minor) ((minor) >> 5) -#define SNDRV_MINOR_DEVICE(minor) ((minor) & 0x001f) -#define SNDRV_MINOR(card, dev) (((card) << 5) | (dev)) +#define SNDRV_MINOR_DEVICES 64 +#define SNDRV_MINOR_CARD(minor) ((minor) >> 6) +#define SNDRV_MINOR_DEVICE(minor) ((minor) & 0x003f) +#define SNDRV_MINOR(card, dev) (((card) << 6) | (dev))
/* these minors can still be used for autoloading devices (/dev/aload*) */ #define SNDRV_MINOR_CONTROL 0 /* 0 */ #define SNDRV_MINOR_GLOBAL 1 /* 1 */ -#define SNDRV_MINOR_SEQUENCER (SNDRV_MINOR_GLOBAL + 0 * 32) -#define SNDRV_MINOR_TIMER (SNDRV_MINOR_GLOBAL + 1 * 32) +#define SNDRV_MINOR_SEQUENCER (SNDRV_MINOR_GLOBAL + 0 * 64) +#define SNDRV_MINOR_TIMER (SNDRV_MINOR_GLOBAL + 1 * 64)
#ifndef CONFIG_SND_DYNAMIC_MINORS /* 2 - 3 (reserved) */ #define SNDRV_MINOR_HWDEP 4 /* 4 - 7 */ #define SNDRV_MINOR_RAWMIDI 8 /* 8 - 15 */ -#define SNDRV_MINOR_PCM_PLAYBACK 16 /* 16 - 23 */ -#define SNDRV_MINOR_PCM_CAPTURE 24 /* 24 - 31 */ +#define SNDRV_MINOR_PCM_PLAYBACK 16 /* 16 - 31 */ +#define SNDRV_MINOR_PCM_CAPTURE 32 /* 32 - 63 */
/* same as first respective minor number to make minor allocation easier */ #define SNDRV_DEVICE_TYPE_CONTROL SNDRV_MINOR_CONTROL --- sound-2.6.orig/include/sound/pcm.h 2010-04-16 16:03:28.000000000 +0800 +++ sound-2.6/include/sound/pcm.h 2010-05-11 13:02:48.000000000 +0800 @@ -88,7 +88,7 @@ struct snd_pcm_ops { #if defined(CONFIG_SND_DYNAMIC_MINORS) #define SNDRV_PCM_DEVICES (SNDRV_OS_MINORS-2) #else -#define SNDRV_PCM_DEVICES 8 +#define SNDRV_PCM_DEVICES 32 #endif
#define SNDRV_PCM_IOCTL1_FALSE ((void *)0)
On Wed, 12 May 2010, Wu Fengguang wrote:
Reserve 32 minor numbers for PCM playback devices.
The Intel SandyBridge HDMI audio codec provides 3 PCM devices with indices 3, 7, 8. Among which the device 8's minor number will be overlapped with the first capture device's minor number in the current static minor number allocation scheme.
Also increase SNDRV_PCM_DEVICES to make pcm_dev_bits big enough to hold the increasing number of PCM devices.
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Wed, 12 May 2010 09:29:57 +0200 (CEST), Jaroslav Kysela wrote:
On Wed, 12 May 2010, Wu Fengguang wrote:
Reserve 32 minor numbers for PCM playback devices.
The Intel SandyBridge HDMI audio codec provides 3 PCM devices with indices 3, 7, 8. Among which the device 8's minor number will be overlapped with the first capture device's minor number in the current static minor number allocation scheme.
Also increase SNDRV_PCM_DEVICES to make pcm_dev_bits big enough to hold the increasing number of PCM devices.
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Dropping such a base feature is really no good option. Better to give simply an error for more than 8 PCMs in such a case, IMO.
I also wonder whether having 4 individual PCMs is a way to go. We may have PCM substreams, if any. OTOH, the current IEC958 stream assignment mechanism doesn't consider multiple substreams well, e.g. we have no proper way to match the IEC958 status bits control to a PCM substream.
thanks,
Takashi
On Wed, May 12, 2010 at 04:03:43PM +0800, Takashi Iwai wrote:
At Wed, 12 May 2010 09:29:57 +0200 (CEST), Jaroslav Kysela wrote:
On Wed, 12 May 2010, Wu Fengguang wrote:
Reserve 32 minor numbers for PCM playback devices.
The Intel SandyBridge HDMI audio codec provides 3 PCM devices with indices 3, 7, 8. Among which the device 8's minor number will be overlapped with the first capture device's minor number in the current static minor number allocation scheme.
Also increase SNDRV_PCM_DEVICES to make pcm_dev_bits big enough to hold the increasing number of PCM devices.
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Jaroslav, will there be so many sound cards in one system?
Dropping such a base feature is really no good option. Better to give simply an error for more than 8 PCMs in such a case, IMO.
Agreed.
I also wonder whether having 4 individual PCMs is a way to go. We may have PCM substreams, if any. OTOH, the current IEC958 stream assignment mechanism doesn't consider multiple substreams well, e.g. we have no proper way to match the IEC958 status bits control to a PCM substream.
Hmm, what if there are two monitors attached, each want to play a different music track?
I think the 3 HDMI devices are inherently independent. Each one can have different hw rates, bits, and speaker allocation. And if implemented as PCM substreams, how can the user space specify "please play this music on this monitor"? AFAIK the windows driver also exports 3 independent HDMI playback devices.
Thanks, Fengguang
At Wed, 12 May 2010 16:39:50 +0800, Wu Fengguang wrote:
On Wed, May 12, 2010 at 04:03:43PM +0800, Takashi Iwai wrote:
At Wed, 12 May 2010 09:29:57 +0200 (CEST), Jaroslav Kysela wrote:
On Wed, 12 May 2010, Wu Fengguang wrote:
Reserve 32 minor numbers for PCM playback devices.
The Intel SandyBridge HDMI audio codec provides 3 PCM devices with indices 3, 7, 8. Among which the device 8's minor number will be overlapped with the first capture device's minor number in the current static minor number allocation scheme.
Also increase SNDRV_PCM_DEVICES to make pcm_dev_bits big enough to hold the increasing number of PCM devices.
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Jaroslav, will there be so many sound cards in one system?
In the old time, yes. Now we have less and less PCI slots. In theory, we may have lots of USB audio devices, though :)
Another possible solution would be to change the minor number assignment to a really dynamic one. So far, due to legacy /dev/aload and co, we have some static restriction per card basis.
Dropping such a base feature is really no good option. Better to give simply an error for more than 8 PCMs in such a case, IMO.
Agreed.
I also wonder whether having 4 individual PCMs is a way to go. We may have PCM substreams, if any. OTOH, the current IEC958 stream assignment mechanism doesn't consider multiple substreams well, e.g. we have no proper way to match the IEC958 status bits control to a PCM substream.
Hmm, what if there are two monitors attached, each want to play a different music track?
PCM substreams are really independent from each other. So, technically seen, substreams are feasible.
I think the 3 HDMI devices are inherently independent. Each one can have different hw rates, bits, and speaker allocation. And if implemented as PCM substreams, how can the user space specify "please play this music on this monitor"? AFAIK the windows driver also exports 3 independent HDMI playback devices.
Yeah, as mentioned, SPDIF status mapping isn't provided properly as is. Thus using PCM substreams may give some confusion indeed.
Takashi
On Wed, May 12, 2010 at 05:01:57PM +0800, Takashi Iwai wrote:
At Wed, 12 May 2010 16:39:50 +0800, Wu Fengguang wrote:
On Wed, May 12, 2010 at 04:03:43PM +0800, Takashi Iwai wrote:
At Wed, 12 May 2010 09:29:57 +0200 (CEST), Jaroslav Kysela wrote:
On Wed, 12 May 2010, Wu Fengguang wrote:
Reserve 32 minor numbers for PCM playback devices.
The Intel SandyBridge HDMI audio codec provides 3 PCM devices with indices 3, 7, 8. Among which the device 8's minor number will be overlapped with the first capture device's minor number in the current static minor number allocation scheme.
Also increase SNDRV_PCM_DEVICES to make pcm_dev_bits big enough to hold the increasing number of PCM devices.
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Jaroslav, will there be so many sound cards in one system?
In the old time, yes. Now we have less and less PCI slots. In theory, we may have lots of USB audio devices, though :)
Another possible solution would be to change the minor number assignment to a really dynamic one. So far, due to legacy /dev/aload and co, we have some static restriction per card basis.
Another simple option is to "borrow" 2 slots from one of
- SNDRV_MINOR_HWDEP : has 4 slots - SNDRV_MINOR_RAWMIDI : has 8 slots - SNDRV_MINOR_PCM_CAPTURE: has 8 slots
What do you think?
Dropping such a base feature is really no good option. Better to give simply an error for more than 8 PCMs in such a case, IMO.
Agreed.
I also wonder whether having 4 individual PCMs is a way to go. We may have PCM substreams, if any. OTOH, the current IEC958 stream assignment mechanism doesn't consider multiple substreams well, e.g. we have no proper way to match the IEC958 status bits control to a PCM substream.
Hmm, what if there are two monitors attached, each want to play a different music track?
PCM substreams are really independent from each other. So, technically seen, substreams are feasible.
I think the 3 HDMI devices are inherently independent. Each one can have different hw rates, bits, and speaker allocation. And if implemented as PCM substreams, how can the user space specify "please play this music on this monitor"? AFAIK the windows driver also exports 3 independent HDMI playback devices.
Yeah, as mentioned, SPDIF status mapping isn't provided properly as is. Thus using PCM substreams may give some confusion indeed.
Can I ask a dumb question? Currently I select the target playback device with the -D option:
speaker-test -Dhw:0,3
How can I do the job when PCM substreams are used?
Thanks, Fengguang
On 12/05/10 22:06, Wu Fengguang wrote:
Can I ask a dumb question? Currently I select the target playback device with the -D option:
speaker-test -Dhw:0,3How can I do the job when PCM substreams are used?
speaker-test -Dhw:0,0,3
Takashi Iwai wrote:
Wu Fengguang wrote:
Jaroslav Kysela wrote:
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Jaroslav, will there be so many sound cards in one system?
In the old time, yes. Now we have less and less PCI slots. In theory, we may have lots of USB audio devices, though :)
I implemented CONFIG_SND_DYNAMIC_MINORS because people had been asking for more than eight cards. (And by now I have lots of cards too, although my computer probably isn't very typical.)
Anyway, static numbering is needed only for systems without udev/devfs, and there we shouldn't change it for backwards compatibility. The HDA driver already requires kernels >= 2.6, so I don't see a problem with requiring CONFIG_SND_DYNAMIC_MINORS to get all HDMI outputs.
Another possible solution would be to change the minor number assignment to a really dynamic one. So far, due to legacy /dev/aload and co, we have some static restriction per card basis.
What restriction would that be? With CONFIG_SND_DYNAMIC_MINORS, we don't allocate minors that would be used by /dev/aload*, but there are no restrictions on the number of cards or devices.
At Wed, 12 May 2010 12:20:33 +0200, Clemens Ladisch wrote:
Takashi Iwai wrote:
Wu Fengguang wrote:
Jaroslav Kysela wrote:
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Jaroslav, will there be so many sound cards in one system?
In the old time, yes. Now we have less and less PCI slots. In theory, we may have lots of USB audio devices, though :)
I implemented CONFIG_SND_DYNAMIC_MINORS because people had been asking for more than eight cards. (And by now I have lots of cards too, although my computer probably isn't very typical.)
Anyway, static numbering is needed only for systems without udev/devfs, and there we shouldn't change it for backwards compatibility. The HDA driver already requires kernels >= 2.6, so I don't see a problem with requiring CONFIG_SND_DYNAMIC_MINORS to get all HDMI outputs.
Right. We can make it dependent.
Another possible solution would be to change the minor number assignment to a really dynamic one. So far, due to legacy /dev/aload and co, we have some static restriction per card basis.
What restriction would that be? With CONFIG_SND_DYNAMIC_MINORS, we don't allocate minors that would be used by /dev/aload*, but there are no restrictions on the number of cards or devices.
Ah, OK, I misread the mapping of SNDRV_MINOR_CONTROL and GLOBAL.
thanks,
Takashi
On Wed, May 12, 2010 at 06:55:09PM +0800, Takashi Iwai wrote:
At Wed, 12 May 2010 12:20:33 +0200, Clemens Ladisch wrote:
Takashi Iwai wrote:
Wu Fengguang wrote:
Jaroslav Kysela wrote:
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Jaroslav, will there be so many sound cards in one system?
In the old time, yes. Now we have less and less PCI slots. In theory, we may have lots of USB audio devices, though :)
I implemented CONFIG_SND_DYNAMIC_MINORS because people had been asking for more than eight cards. (And by now I have lots of cards too, although my computer probably isn't very typical.)
Anyway, static numbering is needed only for systems without udev/devfs, and there we shouldn't change it for backwards compatibility. The HDA driver already requires kernels >= 2.6, so I don't see a problem with requiring CONFIG_SND_DYNAMIC_MINORS to get all HDMI outputs.
Right. We can make it dependent.
Like this?
config SND_HDA_CODEC_INTELHDMI bool "Build INTEL HDMI HD-audio codec support" + select SND_DYNAMIC_MINORS default y
That will effectively turn on CONFIG_SND_DYNAMIC_MINORS by default (distribution kernels will have to enable it).
Another backwards compatible solution is to fall back to the two reserved values. This makes me a bit more comfortable :)
--- drm-intel.orig/include/sound/minors.h 2010-05-13 10:06:52.000000000 +0800 +++ drm-intel/include/sound/minors.h 2010-05-13 10:10:06.000000000 +0800 @@ -35,7 +35,9 @@ #define SNDRV_MINOR_TIMER (SNDRV_MINOR_GLOBAL + 1 * 32)
#ifndef CONFIG_SND_DYNAMIC_MINORS - /* 2 - 3 (reserved) */ + +#define SNDRV_MINOR_BACKUP1 2 +#define SNDRV_MINOR_BACKUP2 3 #define SNDRV_MINOR_HWDEP 4 /* 4 - 7 */ #define SNDRV_MINOR_RAWMIDI 8 /* 8 - 15 */ #define SNDRV_MINOR_PCM_PLAYBACK 16 /* 16 - 23 */ --- drm-intel.orig/sound/core/sound.c 2010-05-13 10:06:04.000000000 +0800 +++ drm-intel/sound/core/sound.c 2010-05-13 10:11:19.000000000 +0800 @@ -269,8 +269,14 @@ int snd_register_device_for_dev(int type minor = snd_find_free_minor(); #else minor = snd_kernel_minor(type, card, dev); - if (minor >= 0 && snd_minors[minor]) - minor = -EBUSY; + if (minor >= 0) { + if (snd_minors[minor]) + minor = SNDRV_MINOR_BACKUP1; + if (snd_minors[minor]) + minor = SNDRV_MINOR_BACKUP2; + if (snd_minors[minor]) + minor = -EBUSY; + } #endif if (minor < 0) { mutex_unlock(&sound_mutex);
Thanks, Fengguang
At Thu, 13 May 2010 10:21:26 +0800, Wu Fengguang wrote:
On Wed, May 12, 2010 at 06:55:09PM +0800, Takashi Iwai wrote:
At Wed, 12 May 2010 12:20:33 +0200, Clemens Ladisch wrote:
Takashi Iwai wrote:
Wu Fengguang wrote:
Jaroslav Kysela wrote: > I don't agree to have only 4 slots for soundcards in the static minor > numbering. Maybe the driver should be converted to use subdevices or we > might drop the static minor number allocation at all (it might have only > impact for old distros).
Jaroslav, will there be so many sound cards in one system?
In the old time, yes. Now we have less and less PCI slots. In theory, we may have lots of USB audio devices, though :)
I implemented CONFIG_SND_DYNAMIC_MINORS because people had been asking for more than eight cards. (And by now I have lots of cards too, although my computer probably isn't very typical.)
Anyway, static numbering is needed only for systems without udev/devfs, and there we shouldn't change it for backwards compatibility. The HDA driver already requires kernels >= 2.6, so I don't see a problem with requiring CONFIG_SND_DYNAMIC_MINORS to get all HDMI outputs.
Right. We can make it dependent.
Like this?
config SND_HDA_CODEC_INTELHDMI bool "Build INTEL HDMI HD-audio codec support"
- select SND_DYNAMIC_MINORS default y
That will effectively turn on CONFIG_SND_DYNAMIC_MINORS by default (distribution kernels will have to enable it).
It's always a question whether we use "depends on" or "select". The former is safer while the latter is more intuitive.
In this particular case, I think "select" would be OK. If other guys have no objection, please repost the patch so that I can pick it up later.
Another backwards compatible solution is to fall back to the two reserved values. This makes me a bit more comfortable :)
As mentioned in another post, I don't see a big merit by it. In short: don't touch a working code :) The non-dynamic minors are only for old stuff, better to keep as is.
thanks,
Takashi
--- drm-intel.orig/include/sound/minors.h 2010-05-13 10:06:52.000000000 +0800 +++ drm-intel/include/sound/minors.h 2010-05-13 10:10:06.000000000 +0800 @@ -35,7 +35,9 @@ #define SNDRV_MINOR_TIMER (SNDRV_MINOR_GLOBAL + 1 * 32)
#ifndef CONFIG_SND_DYNAMIC_MINORS
/* 2 - 3 (reserved) */
+#define SNDRV_MINOR_BACKUP1 2 +#define SNDRV_MINOR_BACKUP2 3 #define SNDRV_MINOR_HWDEP 4 /* 4 - 7 */ #define SNDRV_MINOR_RAWMIDI 8 /* 8 - 15 */ #define SNDRV_MINOR_PCM_PLAYBACK 16 /* 16 - 23 */ --- drm-intel.orig/sound/core/sound.c 2010-05-13 10:06:04.000000000 +0800 +++ drm-intel/sound/core/sound.c 2010-05-13 10:11:19.000000000 +0800 @@ -269,8 +269,14 @@ int snd_register_device_for_dev(int type minor = snd_find_free_minor(); #else minor = snd_kernel_minor(type, card, dev);
- if (minor >= 0 && snd_minors[minor])
minor = -EBUSY;
- if (minor >= 0) {
if (snd_minors[minor])minor = SNDRV_MINOR_BACKUP1;if (snd_minors[minor])minor = SNDRV_MINOR_BACKUP2;if (snd_minors[minor])minor = -EBUSY;- }
#endif if (minor < 0) { mutex_unlock(&sound_mutex);
Thanks, Fengguang
On Fri, May 14, 2010 at 04:21:09PM +0800, Takashi Iwai wrote:
At Thu, 13 May 2010 10:21:26 +0800, Wu Fengguang wrote:
On Wed, May 12, 2010 at 06:55:09PM +0800, Takashi Iwai wrote:
At Wed, 12 May 2010 12:20:33 +0200, Clemens Ladisch wrote:
Takashi Iwai wrote:
Wu Fengguang wrote:
> Jaroslav Kysela wrote: > > I don't agree to have only 4 slots for soundcards in the static minor > > numbering. Maybe the driver should be converted to use subdevices or we > > might drop the static minor number allocation at all (it might have only > > impact for old distros).
Jaroslav, will there be so many sound cards in one system?
In the old time, yes. Now we have less and less PCI slots. In theory, we may have lots of USB audio devices, though :)
I implemented CONFIG_SND_DYNAMIC_MINORS because people had been asking for more than eight cards. (And by now I have lots of cards too, although my computer probably isn't very typical.)
Anyway, static numbering is needed only for systems without udev/devfs, and there we shouldn't change it for backwards compatibility. The HDA driver already requires kernels >= 2.6, so I don't see a problem with requiring CONFIG_SND_DYNAMIC_MINORS to get all HDMI outputs.
Right. We can make it dependent.
Like this?
config SND_HDA_CODEC_INTELHDMI bool "Build INTEL HDMI HD-audio codec support"
- select SND_DYNAMIC_MINORS default y
That will effectively turn on CONFIG_SND_DYNAMIC_MINORS by default (distribution kernels will have to enable it).
It's always a question whether we use "depends on" or "select". The former is safer while the latter is more intuitive.
In this particular case, I think "select" would be OK. If other guys have no objection, please repost the patch so that I can pick it up later.
OK. Wait a minute :)
Another backwards compatible solution is to fall back to the two reserved values. This makes me a bit more comfortable :)
As mentioned in another post, I don't see a big merit by it. In short: don't touch a working code :) The non-dynamic minors are only for old stuff, better to keep as is.
OK. I'll drop that idea.
Thanks, Fengguang
On Wed, 12 May 2010, Takashi Iwai wrote:
At Wed, 12 May 2010 09:29:57 +0200 (CEST), Jaroslav Kysela wrote:
On Wed, 12 May 2010, Wu Fengguang wrote:
Reserve 32 minor numbers for PCM playback devices.
The Intel SandyBridge HDMI audio codec provides 3 PCM devices with indices 3, 7, 8. Among which the device 8's minor number will be overlapped with the first capture device's minor number in the current static minor number allocation scheme.
Also increase SNDRV_PCM_DEVICES to make pcm_dev_bits big enough to hold the increasing number of PCM devices.
I don't agree to have only 4 slots for soundcards in the static minor numbering. Maybe the driver should be converted to use subdevices or we might drop the static minor number allocation at all (it might have only impact for old distros).
Dropping such a base feature is really no good option. Better to give simply an error for more than 8 PCMs in such a case, IMO.
I also wonder whether having 4 individual PCMs is a way to go. We may have PCM substreams, if any. OTOH, the current IEC958 stream
Yes, I noted this above.
assignment mechanism doesn't consider multiple substreams well, e.g. we have no proper way to match the IEC958 status bits control to a PCM substream.
We have subdevice member in snd_ctl_elem_id, so I don't see a problem.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
Currently AZX_MAX_CODECS is defined to 8. Increase it to 10 in order to support the HDMI device indices {3, 7, 8, 9}.
The HD audio spec allows up to 14 codecs. So we are still within the hardware capacity.
Signed-off-by: Wu Fengguang fengguang.wu@intel.com --- sound/pci/hda/hda_intel.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
--- sound-2.6.orig/sound/pci/hda/hda_intel.c 2010-05-10 09:40:20.000000000 +0800 +++ sound-2.6/sound/pci/hda/hda_intel.c 2010-05-11 13:02:51.000000000 +0800 @@ -267,7 +267,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO #define RIRB_INT_MASK 0x05
/* STATESTS int mask: S3,SD2,SD1,SD0 */ -#define AZX_MAX_CODECS 8 +#define AZX_MAX_CODECS 10 #define AZX_DEFAULT_CODECS 4 #define STATESTS_INT_MASK ((1 << AZX_MAX_CODECS) - 1)
@@ -866,7 +866,7 @@ static int azx_reset(struct azx *chip, i goto __skip;
/* clear STATESTS */ - azx_writeb(chip, STATESTS, STATESTS_INT_MASK); + azx_writew(chip, STATESTS, STATESTS_INT_MASK);
/* reset controller */ azx_writel(chip, GCTL, azx_readl(chip, GCTL) & ~ICH6_GCTL_RESET); @@ -956,7 +956,7 @@ static void azx_int_clear(struct azx *ch }
/* clear STATESTS */ - azx_writeb(chip, STATESTS, STATESTS_INT_MASK); + azx_writew(chip, STATESTS, STATESTS_INT_MASK);
/* clear rirb status */ azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
At Wed, 12 May 2010 09:30:17 +0800, Wu Fengguang wrote:
Currently AZX_MAX_CODECS is defined to 8. Increase it to 10 in order to support the HDMI device indices {3, 7, 8, 9}.
If I understand correctly, the codec slot number is basically independent from the PCM device number assigned. Hence this change shouldn't be necessary unless you really have more than 8 codec chips. Or will be such a high codec address expected soon on new Intel platforms?
The HD audio spec allows up to 14 codecs. So we are still within the hardware capacity.
I thought this was at most 8, so I used the value 0x100 as a special bit flag for probe_mask module option :-< We'd need to change this if we extend the max codecs...
thanks,
Takashi
Signed-off-by: Wu Fengguang fengguang.wu@intel.com
sound/pci/hda/hda_intel.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
--- sound-2.6.orig/sound/pci/hda/hda_intel.c 2010-05-10 09:40:20.000000000 +0800 +++ sound-2.6/sound/pci/hda/hda_intel.c 2010-05-11 13:02:51.000000000 +0800 @@ -267,7 +267,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO #define RIRB_INT_MASK 0x05
/* STATESTS int mask: S3,SD2,SD1,SD0 */ -#define AZX_MAX_CODECS 8 +#define AZX_MAX_CODECS 10 #define AZX_DEFAULT_CODECS 4 #define STATESTS_INT_MASK ((1 << AZX_MAX_CODECS) - 1)
@@ -866,7 +866,7 @@ static int azx_reset(struct azx *chip, i goto __skip;
/* clear STATESTS */
- azx_writeb(chip, STATESTS, STATESTS_INT_MASK);
azx_writew(chip, STATESTS, STATESTS_INT_MASK);
/* reset controller */ azx_writel(chip, GCTL, azx_readl(chip, GCTL) & ~ICH6_GCTL_RESET);
@@ -956,7 +956,7 @@ static void azx_int_clear(struct azx *ch }
/* clear STATESTS */
- azx_writeb(chip, STATESTS, STATESTS_INT_MASK);
azx_writew(chip, STATESTS, STATESTS_INT_MASK);
/* clear rirb status */ azx_writeb(chip, RIRBSTS, RIRB_INT_MASK);
On Wed, May 12, 2010 at 10:35:46PM +0800, Takashi Iwai wrote:
At Wed, 12 May 2010 09:30:17 +0800, Wu Fengguang wrote:
Currently AZX_MAX_CODECS is defined to 8. Increase it to 10 in order to support the HDMI device indices {3, 7, 8, 9}.
If I understand correctly, the codec slot number is basically independent from the PCM device number assigned. Hence this change shouldn't be necessary unless you really have more than 8 codec chips. Or will be such a high codec address expected soon on new Intel platforms?
You are right, it works without this patch. Sorry for the confusion!
The HD audio spec allows up to 14 codecs. So we are still within the hardware capacity.
I thought this was at most 8, so I used the value 0x100 as a special bit flag for probe_mask module option :-< We'd need to change this if we extend the max codecs...
It may allow 15 codecs in fact. I get the max number from "Figure 32. Codec Address Assignment Frame":
CODEC Samples Address ** Address 15 is reserved for 0 1 2 3 ......... 14 link protocol purposes and may not be assigned to CODECs
And STATESTS has bits 14:0 allocated for "SDIN State Change Status Flags (SDIWAKE)".
However we don't need to increase AZX_MAX_CODECS and probe_mask for now.
Thanks, Fengguang
Use the full chipset codename as codec name. They are more user friendly than the spec abbrs.
Signed-off-by: Wu Fengguang fengguang.wu@intel.com --- sound/pci/hda/patch_intelhdmi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c 2010-05-11 13:03:50.000000000 +0800 +++ sound-2.6/sound/pci/hda/patch_intelhdmi.c 2010-05-11 13:04:39.000000000 +0800 @@ -185,14 +185,14 @@ static int patch_intel_hdmi(struct hda_c }
static struct hda_codec_preset snd_hda_preset_intelhdmi[] = { - { .id = 0x808629fb, .name = "G45 DEVCL", .patch = patch_intel_hdmi }, - { .id = 0x80862801, .name = "G45 DEVBLC", .patch = patch_intel_hdmi }, - { .id = 0x80862802, .name = "G45 DEVCTG", .patch = patch_intel_hdmi }, - { .id = 0x80862803, .name = "G45 DEVELK", .patch = patch_intel_hdmi }, - { .id = 0x80862804, .name = "G45 DEVIBX", .patch = patch_intel_hdmi }, - { .id = 0x80860054, .name = "Q57 DEVIBX", .patch = patch_intel_hdmi }, - { .id = 0x10951392, .name = "SiI1392 HDMI", .patch = patch_intel_hdmi }, - {} /* terminator */ +{ .id = 0x808629fb, .name = "Crestline HDMI", .patch = patch_intel_hdmi }, +{ .id = 0x80862801, .name = "Bearlake HDMI", .patch = patch_intel_hdmi }, +{ .id = 0x80862802, .name = "Cantiga HDMI", .patch = patch_intel_hdmi }, +{ .id = 0x80862803, .name = "Eaglelake HDMI", .patch = patch_intel_hdmi }, +{ .id = 0x80862804, .name = "IbexPeak HDMI", .patch = patch_intel_hdmi }, +{ .id = 0x80860054, .name = "IbexPeak HDMI", .patch = patch_intel_hdmi }, +{ .id = 0x10951392, .name = "SiI1392 HDMI", .patch = patch_intel_hdmi }, +{} /* terminator */ };
MODULE_ALIAS("snd-hda-codec-id:808629fb");
Add id for Intel CougarPoint HDMI audio codec.
CougarPoint provides 3 Audio Converters. Increase MAX_HDMI_CVTS/MAX_HDMI_PINS accordingly.
Signed-off-by: Wu Fengguang fengguang.wu@intel.com --- sound/pci/hda/patch_intelhdmi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c 2010-05-11 13:04:39.000000000 +0800 +++ sound-2.6/sound/pci/hda/patch_intelhdmi.c 2010-05-11 13:22:30.000000000 +0800 @@ -40,7 +40,7 @@ * * The HDA correspondence of pipes/ports are converter/pin nodes. */ -#define MAX_HDMI_CVTS 2 +#define MAX_HDMI_CVTS 3 #define MAX_HDMI_PINS 3
#include "patch_hdmi.c" @@ -48,6 +48,7 @@ static char *intel_hdmi_pcm_names[MAX_HDMI_CVTS] = { "INTEL HDMI 0", "INTEL HDMI 1", + "INTEL HDMI 2", };
/* @@ -191,6 +192,7 @@ static struct hda_codec_preset snd_hda_p { .id = 0x80862803, .name = "Eaglelake HDMI", .patch = patch_intel_hdmi }, { .id = 0x80862804, .name = "IbexPeak HDMI", .patch = patch_intel_hdmi }, { .id = 0x80860054, .name = "IbexPeak HDMI", .patch = patch_intel_hdmi }, +{ .id = 0x80862805, .name = "CougarPoint HDMI", .patch = patch_intel_hdmi }, { .id = 0x10951392, .name = "SiI1392 HDMI", .patch = patch_intel_hdmi }, {} /* terminator */ }; @@ -200,6 +202,7 @@ MODULE_ALIAS("snd-hda-codec-id:80862801" MODULE_ALIAS("snd-hda-codec-id:80862802"); MODULE_ALIAS("snd-hda-codec-id:80862803"); MODULE_ALIAS("snd-hda-codec-id:80862804"); +MODULE_ALIAS("snd-hda-codec-id:80862805"); MODULE_ALIAS("snd-hda-codec-id:80860054"); MODULE_ALIAS("snd-hda-codec-id:10951392");
The number of HDMI nodes is expected to go up in future. So don't fail hard on seeing extra converter/pin nodes.
We can still operate safely on the nodes within MAX_HDMI_CVTS/MAX_HDMI_PINS.
Signed-off-by: Wu Fengguang fengguang.wu@intel.com --- sound/pci/hda/patch_hdmi.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
--- sound-2.6.orig/sound/pci/hda/patch_hdmi.c 2010-05-11 13:25:35.000000000 +0800 +++ sound-2.6/sound/pci/hda/patch_hdmi.c 2010-05-11 13:53:47.000000000 +0800 @@ -766,7 +766,7 @@ static int hdmi_add_pin(struct hda_codec if (spec->num_pins >= MAX_HDMI_PINS) { snd_printk(KERN_WARNING "HDMI: no space for pin %d\n", pin_nid); - return -EINVAL; + return -E2BIG; }
hdmi_present_sense(codec, pin_nid, &spec->sink_eld[spec->num_pins]); @@ -788,7 +788,7 @@ static int hdmi_add_cvt(struct hda_codec if (spec->num_cvts >= MAX_HDMI_CVTS) { snd_printk(KERN_WARNING "HDMI: no space for converter %d\n", nid); - return -EINVAL; + return -E2BIG; }
spec->cvt[spec->num_cvts] = nid; @@ -820,15 +820,13 @@ static int hdmi_parse_codec(struct hda_c
switch (type) { case AC_WID_AUD_OUT: - if (hdmi_add_cvt(codec, nid) < 0) - return -EINVAL; + hdmi_add_cvt(codec, nid); break; case AC_WID_PIN: caps = snd_hda_param_read(codec, nid, AC_PAR_PIN_CAP); if (!(caps & (AC_PINCAP_HDMI | AC_PINCAP_DP))) continue; - if (hdmi_add_pin(codec, nid) < 0) - return -EINVAL; + hdmi_add_pin(codec, nid); break; } }
participants (5)
-
Clemens Ladisch -
Eliot Blennerhassett -
Jaroslav Kysela -
Takashi Iwai -
Wu Fengguang