[PATCH 0/5] updates for Intel SandyBridge/CougarPoint HDMI codec

[PATCH 0/5] updates for Intel SandyBridge/CougarPoint HDMI codec

[Wu Fengguang]

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

[PATCH 1/5] allow up to 32 PCM devices

[Wu Fengguang]

[-- Attachment #1: alsa-minor.patch --]
[-- Type: text/plain, Size: 2469 bytes --]

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)

Re: [PATCH 1/5] allow up to 32 PCM devices

[Jaroslav Kysela]

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.

Re: [PATCH 1/5] allow up to 32 PCM devices

[Takashi Iwai]

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

Re: [PATCH 1/5] allow up to 32 PCM devices

[Wu Fengguang]

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

Re: [PATCH 1/5] allow up to 32 PCM devices

[Takashi Iwai]

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

Re: [PATCH 1/5] allow up to 32 PCM devices

[Wu Fengguang]

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

Re: [PATCH 1/5] allow up to 32 PCM devices

[Eliot Blennerhassett]

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,3 
> 
> How can I do the job when PCM substreams are used?

         speaker-test -Dhw:0,0,3

Re: [PATCH 1/5] allow up to 32 PCM devices

[Clemens Ladisch]

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.


-- 
Regards,
Clemens


cat /proc/asound/cards
 0 [SB             ]: HDA-Intel - HDA ATI SB
                      HDA ATI SB at 0xfe7f4000 irq 16
 1 [HDMI           ]: HDA-Intel - HDA ATI HDMI
                      HDA ATI HDMI at 0xfe9e8000 irq 19
 2 [D2             ]: AV200 - Xonar D2
                      Asus Virtuoso 200 (rev 2) at 0xe800, irq 20
 3 [YMF754         ]: YMF754 - Yamaha DS-1E (YMF754)
                      Yamaha DS-1E (YMF754) at 0xfebf8000, irq 22
 4 [UA1A           ]: USB-Audio - EDIROL UA-1A
                      Roland EDIROL UA-1A at usb-0000:00:12.2-4.3, full speed
 5 [SC8820         ]: USB-Audio - SC-8820
                      Roland SC-8820 at usb-0000:00:13.0-2, full speed
 6 [UM2            ]: USB-Audio - UM-2
                      EDIROL UM-2 at usb-0000:00:12.0-2, full speed
 7 [NX             ]: USB-Audio - SB Audigy 2 NX
                      Creative Technology Ltd SB Audigy 2 NX at usb-0000:00:12.2-3, high speed
 8 [UART16550A     ]: Serial - Serial MIDI (UART16550A)
                      Serial MIDI (UART16550A) [Soundcanvas] at 0x3f8, irq 4
 9 [VirMIDI        ]: VirMIDI - VirMIDI
                      Virtual MIDI Card 1
10 [Dummy          ]: Dummy - Dummy
                      Dummy 1
11 [Live           ]: EMU10K1 - SB Live! Value [CT4670]
                      SB Live! Value [CT4670] (rev.5, serial:0x201102) at 0xec00, irq 21
12 [pcsp           ]: PC-Speaker - pcsp
                      Internal PC-Speaker at port 0x61
13 [AudioFire2     ]: FireWorks - AudioFire2
                      Echo Digital Audio AudioFire2 (v3.2) at fw1.0, S400

Re: [PATCH 1/5] allow up to 32 PCM devices

[Takashi Iwai]

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

Re: [PATCH 1/5] allow up to 32 PCM devices

[Wu Fengguang]

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

Re: [PATCH 1/5] allow up to 32 PCM devices

[Takashi Iwai]

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
> 

Re: [PATCH 1/5] allow up to 32 PCM devices

[Wu 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

Re: [PATCH 1/5] allow up to 32 PCM devices

[Jaroslav Kysela]

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.

[PATCH 2/5] hda - allow up to 10 Azalia codecs

[Wu Fengguang]

[-- Attachment #1: alsa-max-codecs.patch --]
[-- Type: text/plain, Size: 1343 bytes --]

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);

Re: [PATCH 2/5] hda - allow up to 10 Azalia codecs

[Takashi Iwai]

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);
> 
> 

Re: [PATCH 2/5] hda - allow up to 10 Azalia codecs

[Wu Fengguang]

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

[PATCH 3/5] intelhdmi - user friendly codec name

[Wu Fengguang]

[-- Attachment #1: hdmi-codenames.patch --]
[-- Type: text/plain, Size: 1694 bytes --]

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");

[PATCH 4/5] intelhdmi - add id for the CougarPoint chipset

[Wu Fengguang]

[-- Attachment #1: hdmi-snb-id.patch --]
[-- Type: text/plain, Size: 1582 bytes --]

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");

[PATCH 5/5] hdmi - dont fail on extra nodes

[Wu Fengguang]

[-- Attachment #1: hdmi-permissive-to-extra-nodes.patch --]
[-- Type: text/plain, Size: 1514 bytes --]

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;
 		}
 	}