[PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[Nitin Daga]

Added hardware constraint in patch_hdmi.c to disable
channels 4/6 which are not supported by some older
NVIDIA GPUs.

Signed-off-by: Nitin Daga <ndaga@nvidia.com>
Acked-By: Stephen Warren <swarren@nvidia.com>
---
 pci/hda/patch_hdmi.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/pci/hda/patch_hdmi.c b/pci/hda/patch_hdmi.c
index d3e49aa..ddde59b 100644
--- a/pci/hda/patch_hdmi.c
+++ b/pci/hda/patch_hdmi.c
@@ -1166,11 +1166,53 @@ static int nvhdmi_7x_init(struct hda_codec *codec)
 	return 0;
 }
 
+static unsigned int channels_2_6_8[] = {
+	2, 6, 8
+};
+
+static unsigned int channels_2_8[] = {
+	2, 8
+};
+
+static struct snd_pcm_hw_constraint_list hw_constraints_2_6_8_channels = {
+	.count = ARRAY_SIZE(channels_2_6_8),
+	.list = channels_2_6_8,
+	.mask = 0,
+};
+
+static struct snd_pcm_hw_constraint_list hw_constraints_2_8_channels = {
+	.count = ARRAY_SIZE(channels_2_8),
+	.list = channels_2_8,
+	.mask = 0,
+};
+
 static int simple_playback_pcm_open(struct hda_pcm_stream *hinfo,
 				    struct hda_codec *codec,
 				    struct snd_pcm_substream *substream)
 {
 	struct hdmi_spec *spec = codec->spec;
+	struct snd_pcm_hw_constraint_list *hw_constraints_channels = NULL;
+
+	switch (codec->preset->id) {
+	case 0x10de0002:
+	case 0x10de0003:
+	case 0x10de0005:
+	case 0x10de0006:
+		hw_constraints_channels = &hw_constraints_2_8_channels;
+		break;
+	case 0x10de0007:
+		hw_constraints_channels = &hw_constraints_2_6_8_channels;
+		break;
+	default:
+		break;
+	}
+
+	if (hw_constraints_channels != NULL) {
+		snd_pcm_hw_constraint_list(substream->runtime, 0,
+				SNDRV_PCM_HW_PARAM_CHANNELS,
+				hw_constraints_channels);
+	}
+
 	return snd_hda_multi_out_dig_open(codec, &spec->multiout);
 }
 
-- 
1.7.0.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[Anssi Hannula]

On 10.01.2011 18:19, Nitin Daga wrote:
> Added hardware constraint in patch_hdmi.c to disable
> channels 4/6 which are not supported by some older
> NVIDIA GPUs.

And 3/5/7, which do not seem to work either with my 0x0007.

However, 3/5/7 do not work with my 0x000b system either, is this a hw
limitation as well (that should be added to the driver) or is there a
bug in the HDA driver for uneven channel counts?

(I looked at the HDA specification and it does seem to allow
odd-numbered channel counts)

Strangely, on both of my systems speaker-test -c5 fails with an error:

$ speaker-test -Dhdmi -c5

speaker-test 1.0.23

Playback device is hdmi
Stream parameters are 48000Hz, S16_LE, 5 channels
Using 16 octaves of pink noise
Rate set to 48000Hz (requested 48000Hz)
Buffer size range from 128 to 419392
Period size range from 64 to 209664
Using max buffer size 419392
Periods = 4
Unable to set nperiods 4 for playback: No such file or directory
Setting of hwparams failed: No such file or directory

While -c3 and -c7 run fine but no sound is received.


> Signed-off-by: Nitin Daga <ndaga@nvidia.com>
> Acked-By: Stephen Warren <swarren@nvidia.com>
> ---
>  pci/hda/patch_hdmi.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/pci/hda/patch_hdmi.c b/pci/hda/patch_hdmi.c
> index d3e49aa..ddde59b 100644
> --- a/pci/hda/patch_hdmi.c
> +++ b/pci/hda/patch_hdmi.c
> @@ -1166,11 +1166,53 @@ static int nvhdmi_7x_init(struct hda_codec *codec)
>  	return 0;
>  }
>  
> +static unsigned int channels_2_6_8[] = {
> +	2, 6, 8
> +};
> +
> +static unsigned int channels_2_8[] = {
> +	2, 8
> +};
> +
> +static struct snd_pcm_hw_constraint_list hw_constraints_2_6_8_channels = {
> +	.count = ARRAY_SIZE(channels_2_6_8),
> +	.list = channels_2_6_8,
> +	.mask = 0,
> +};
> +
> +static struct snd_pcm_hw_constraint_list hw_constraints_2_8_channels = {
> +	.count = ARRAY_SIZE(channels_2_8),
> +	.list = channels_2_8,
> +	.mask = 0,
> +};
> +
>  static int simple_playback_pcm_open(struct hda_pcm_stream *hinfo,
>  				    struct hda_codec *codec,
>  				    struct snd_pcm_substream *substream)
>  {
>  	struct hdmi_spec *spec = codec->spec;
> +	struct snd_pcm_hw_constraint_list *hw_constraints_channels = NULL;
> +
> +	switch (codec->preset->id) {
> +	case 0x10de0002:
> +	case 0x10de0003:
> +	case 0x10de0005:
> +	case 0x10de0006:
> +		hw_constraints_channels = &hw_constraints_2_8_channels;
> +		break;
> +	case 0x10de0007:
> +		hw_constraints_channels = &hw_constraints_2_6_8_channels;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (hw_constraints_channels != NULL) {
> +		snd_pcm_hw_constraint_list(substream->runtime, 0,
> +				SNDRV_PCM_HW_PARAM_CHANNELS,
> +				hw_constraints_channels);
> +	}
> +
>  	return snd_hda_multi_out_dig_open(codec, &spec->multiout);
>  }
>  


-- 
Anssi Hannula

Re: [PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[Stephen Warren]

Anssi Hannula wrote:
> On 10.01.2011 18:19, Nitin Daga wrote:
> > Added hardware constraint in patch_hdmi.c to disable
> > channels 4/6 which are not supported by some older
> > NVIDIA GPUs.
> 
> And 3/5/7, which do not seem to work either with my 0x0007.

Are 3/5/7 channels supported by any vendor?

> However, 3/5/7 do not work with my 0x000b system either, is this a hw
> limitation as well (that should be added to the driver) or is there a
> bug in the HDA driver for uneven channel counts?

I don't know if NVIDIA HW supports odd numbers of channels; Nitin, can
you please find out? That said, the error you quote below for 5 channels
sounds more like a software issue to me; some kind of buffer alignment
or size constraint can't be satisfied?

> (I looked at the HDA specification and it does seem to allow
> odd-numbered channel counts)

I skimmed quickly, and the only reference I could find was in section
7.6 "Audio Data Packetization" of HDMISpecification1.4a.pdf:

    Layout 1 can be used to carry one audio sample with three to
    eight channels of L-PCM audio.

... which implies support for an odd number of channels.

That said, the tables and other text near that quote indicate that audio
is transferred in subpackets which each contain a pair of channels, and
the sample_present bits each cover a subpacket, i.e. a pair of channels.
Perhaps table 5-13 "Audio Sample Subpacket"'s V.L/V.R (channel validity)
bits are intended to represent which of the two channels in a subpacket
exist?

Looking at CEA-861D, Table 17 "Audio InfoFrame Data Byte 1", CC
(Channel Count) bits, it's quite explicit that odd numbers of channels
are supported.

> Strangely, on both of my systems speaker-test -c5 fails with an error:
> 
> $ speaker-test -Dhdmi -c5
> 
> speaker-test 1.0.23
> 
> Playback device is hdmi
> Stream parameters are 48000Hz, S16_LE, 5 channels
> Using 16 octaves of pink noise
> Rate set to 48000Hz (requested 48000Hz)
> Buffer size range from 128 to 419392
> Period size range from 64 to 209664
> Using max buffer size 419392
> Periods = 4
> Unable to set nperiods 4 for playback: No such file or directory
> Setting of hwparams failed: No such file or directory
> 
> While -c3 and -c7 run fine but no sound is received.
>
> > [Nitin's patch elided]

-- 
nvpublic

Re: [PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[Takashi Iwai]

At Mon, 10 Jan 2011 21:49:31 +0530,
Nitin Daga wrote:
> 
> Added hardware constraint in patch_hdmi.c to disable
> channels 4/6 which are not supported by some older
> NVIDIA GPUs.
> 
> Signed-off-by: Nitin Daga <ndaga@nvidia.com>
> Acked-By: Stephen Warren <swarren@nvidia.com>

Applied now.  Thanks.


Takashi

> ---
>  pci/hda/patch_hdmi.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/pci/hda/patch_hdmi.c b/pci/hda/patch_hdmi.c
> index d3e49aa..ddde59b 100644
> --- a/pci/hda/patch_hdmi.c
> +++ b/pci/hda/patch_hdmi.c
> @@ -1166,11 +1166,53 @@ static int nvhdmi_7x_init(struct hda_codec *codec)
>  	return 0;
>  }
>  
> +static unsigned int channels_2_6_8[] = {
> +	2, 6, 8
> +};
> +
> +static unsigned int channels_2_8[] = {
> +	2, 8
> +};
> +
> +static struct snd_pcm_hw_constraint_list hw_constraints_2_6_8_channels = {
> +	.count = ARRAY_SIZE(channels_2_6_8),
> +	.list = channels_2_6_8,
> +	.mask = 0,
> +};
> +
> +static struct snd_pcm_hw_constraint_list hw_constraints_2_8_channels = {
> +	.count = ARRAY_SIZE(channels_2_8),
> +	.list = channels_2_8,
> +	.mask = 0,
> +};
> +
>  static int simple_playback_pcm_open(struct hda_pcm_stream *hinfo,
>  				    struct hda_codec *codec,
>  				    struct snd_pcm_substream *substream)
>  {
>  	struct hdmi_spec *spec = codec->spec;
> +	struct snd_pcm_hw_constraint_list *hw_constraints_channels = NULL;
> +
> +	switch (codec->preset->id) {
> +	case 0x10de0002:
> +	case 0x10de0003:
> +	case 0x10de0005:
> +	case 0x10de0006:
> +		hw_constraints_channels = &hw_constraints_2_8_channels;
> +		break;
> +	case 0x10de0007:
> +		hw_constraints_channels = &hw_constraints_2_6_8_channels;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (hw_constraints_channels != NULL) {
> +		snd_pcm_hw_constraint_list(substream->runtime, 0,
> +				SNDRV_PCM_HW_PARAM_CHANNELS,
> +				hw_constraints_channels);
> +	}
> +
>  	return snd_hda_multi_out_dig_open(codec, &spec->multiout);
>  }
>  
> -- 
> 1.7.0.4
> 
> 
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[Takashi Iwai]

At Tue, 11 Jan 2011 19:11:53 +0200,
Anssi Hannula wrote:
> 
> On 10.01.2011 18:19, Nitin Daga wrote:
> > Added hardware constraint in patch_hdmi.c to disable
> > channels 4/6 which are not supported by some older
> > NVIDIA GPUs.
> 
> And 3/5/7, which do not seem to work either with my 0x0007.
> 
> However, 3/5/7 do not work with my 0x000b system either, is this a hw
> limitation as well (that should be added to the driver) or is there a
> bug in the HDA driver for uneven channel counts?

In theory yes, but I don't know any really working codecs.  It seems
that codecs (or controllers?) are designed to take always
stereo-pairs.

Maybe we can put a hw_constraint in hda_intel.c.


thanks,

Takashi

> (I looked at the HDA specification and it does seem to allow
> odd-numbered channel counts)
> 
> Strangely, on both of my systems speaker-test -c5 fails with an error:
> 
> $ speaker-test -Dhdmi -c5
> 
> speaker-test 1.0.23
> 
> Playback device is hdmi
> Stream parameters are 48000Hz, S16_LE, 5 channels
> Using 16 octaves of pink noise
> Rate set to 48000Hz (requested 48000Hz)
> Buffer size range from 128 to 419392
> Period size range from 64 to 209664
> Using max buffer size 419392
> Periods = 4
> Unable to set nperiods 4 for playback: No such file or directory
> Setting of hwparams failed: No such file or directory
> 
> While -c3 and -c7 run fine but no sound is received.
> 
> 
> > Signed-off-by: Nitin Daga <ndaga@nvidia.com>
> > Acked-By: Stephen Warren <swarren@nvidia.com>
> > ---
> >  pci/hda/patch_hdmi.c |   42 ++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 42 insertions(+), 0 deletions(-)
> > 
> > diff --git a/pci/hda/patch_hdmi.c b/pci/hda/patch_hdmi.c
> > index d3e49aa..ddde59b 100644
> > --- a/pci/hda/patch_hdmi.c
> > +++ b/pci/hda/patch_hdmi.c
> > @@ -1166,11 +1166,53 @@ static int nvhdmi_7x_init(struct hda_codec *codec)
> >  	return 0;
> >  }
> >  
> > +static unsigned int channels_2_6_8[] = {
> > +	2, 6, 8
> > +};
> > +
> > +static unsigned int channels_2_8[] = {
> > +	2, 8
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list hw_constraints_2_6_8_channels = {
> > +	.count = ARRAY_SIZE(channels_2_6_8),
> > +	.list = channels_2_6_8,
> > +	.mask = 0,
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list hw_constraints_2_8_channels = {
> > +	.count = ARRAY_SIZE(channels_2_8),
> > +	.list = channels_2_8,
> > +	.mask = 0,
> > +};
> > +
> >  static int simple_playback_pcm_open(struct hda_pcm_stream *hinfo,
> >  				    struct hda_codec *codec,
> >  				    struct snd_pcm_substream *substream)
> >  {
> >  	struct hdmi_spec *spec = codec->spec;
> > +	struct snd_pcm_hw_constraint_list *hw_constraints_channels = NULL;
> > +
> > +	switch (codec->preset->id) {
> > +	case 0x10de0002:
> > +	case 0x10de0003:
> > +	case 0x10de0005:
> > +	case 0x10de0006:
> > +		hw_constraints_channels = &hw_constraints_2_8_channels;
> > +		break;
> > +	case 0x10de0007:
> > +		hw_constraints_channels = &hw_constraints_2_6_8_channels;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	if (hw_constraints_channels != NULL) {
> > +		snd_pcm_hw_constraint_list(substream->runtime, 0,
> > +				SNDRV_PCM_HW_PARAM_CHANNELS,
> > +				hw_constraints_channels);
> > +	}
> > +
> >  	return snd_hda_multi_out_dig_open(codec, &spec->multiout);
> >  }
> >  
> 
> 
> -- 
> Anssi Hannula
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[Anssi Hannula]

On 11.01.2011 19:41, Stephen Warren wrote:
> Anssi Hannula wrote:
>> On 10.01.2011 18:19, Nitin Daga wrote:
>>> Added hardware constraint in patch_hdmi.c to disable
>>> channels 4/6 which are not supported by some older
>>> NVIDIA GPUs.
>>
>> And 3/5/7, which do not seem to work either with my 0x0007.
> 
> Are 3/5/7 channels supported by any vendor?
> 
>> However, 3/5/7 do not work with my 0x000b system either, is this a hw
>> limitation as well (that should be added to the driver) or is there a
>> bug in the HDA driver for uneven channel counts?
> 
> I don't know if NVIDIA HW supports odd numbers of channels; Nitin, can
> you please find out? That said, the error you quote below for 5 channels
> sounds more like a software issue to me; some kind of buffer alignment
> or size constraint can't be satisfied?
> 
>> (I looked at the HDA specification and it does seem to allow
>> odd-numbered channel counts)
> 
> I skimmed quickly, and the only reference I could find was in section
> 7.6 "Audio Data Packetization" of HDMISpecification1.4a.pdf:
> 
>     Layout 1 can be used to carry one audio sample with three to
>     eight channels of L-PCM audio.
> 
> ... which implies support for an odd number of channels.
> 
> That said, the tables and other text near that quote indicate that audio
> is transferred in subpackets which each contain a pair of channels, and
> the sample_present bits each cover a subpacket, i.e. a pair of channels.
> Perhaps table 5-13 "Audio Sample Subpacket"'s V.L/V.R (channel validity)
> bits are intended to represent which of the two channels in a subpacket
> exist?

Well, the channel is certainly not valid if it does not exist :)
But I don't think it is that field.

> Looking at CEA-861D, Table 17 "Audio InfoFrame Data Byte 1", CC
> (Channel Count) bits, it's quite explicit that odd numbers of channels
> are supported.

Actually, I just tested and just transmitting 3-channel data with extra
4th dummy channel while setting the Audio InfoFrame fields to indicate a
3 channel stream makes it work.

So it would seem (to me) like the best approach would be:
1. Add a hw_constraint to prevent odd channel counts (easy)
2. Provide some way for alsa plug (and the user application) to indicate
   the "actual/original" channel count/layout without dummy channels,
   so that the infoframe can be set correctly.
   This is harder, and I guess should probably be implemented so that
   the channel remapping issue on older nvidia hw will be fixed as well.


>> Strangely, on both of my systems speaker-test -c5 fails with an error:
>>
>> $ speaker-test -Dhdmi -c5
>>
>> speaker-test 1.0.23
>>
>> Playback device is hdmi
>> Stream parameters are 48000Hz, S16_LE, 5 channels
>> Using 16 octaves of pink noise
>> Rate set to 48000Hz (requested 48000Hz)
>> Buffer size range from 128 to 419392
>> Period size range from 64 to 209664
>> Using max buffer size 419392
>> Periods = 4
>> Unable to set nperiods 4 for playback: No such file or directory
>> Setting of hwparams failed: No such file or directory
>>
>> While -c3 and -c7 run fine but no sound is received.
>>
>>> [Nitin's patch elided]
> 


-- 
Anssi Hannula

Re: [PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[Stephen Warren]

Anssi Hannula wrote:
> 
> On 11.01.2011 19:41, Stephen Warren wrote:
> > Anssi Hannula wrote:
> >> On 10.01.2011 18:19, Nitin Daga wrote:
> >>> Added hardware constraint in patch_hdmi.c to disable
> >>> channels 4/6 which are not supported by some older
> >>> NVIDIA GPUs.
> >>
> >> And 3/5/7, which do not seem to work either with my 0x0007.
> >
> > Are 3/5/7 channels supported by any vendor?
>...
> 
> Actually, I just tested and just transmitting 3-channel data with extra
> 4th dummy channel while setting the Audio InfoFrame fields to indicate a
> 3 channel stream makes it work.
> 
> So it would seem (to me) like the best approach would be:
> 1. Add a hw_constraint to prevent odd channel counts (easy)

That seems enough to me. I'm not sure that 3/5/7 channels are a format
that there is much use-case for?

> 2. Provide some way for alsa plug (and the user application) to indicate
>    the "actual/original" channel count/layout without dummy channels,
>    so that the infoframe can be set correctly.
>    This is harder, and I guess should probably be implemented so that
>    the channel remapping issue on older nvidia hw will be fixed as well.

Hmm. That sounds kinda complex. If constraints prevent the application
from using e.g. 3-channel, then implementing (2) means the plugin would
have to also modify the constraints to re-allow 3-channel, so the
application could actually use it, no?

-- 
nvpublic

Re: [PATCH] ALSA: hda: Disable 4/6 channels on some NVIDIA GPUs.

[Anssi Hannula]

On 12.01.2011 18:42, Stephen Warren wrote:
> Anssi Hannula wrote:
>>
>> On 11.01.2011 19:41, Stephen Warren wrote:
>>> Anssi Hannula wrote:
>>>> On 10.01.2011 18:19, Nitin Daga wrote:
>>>>> Added hardware constraint in patch_hdmi.c to disable
>>>>> channels 4/6 which are not supported by some older
>>>>> NVIDIA GPUs.
>>>>
>>>> And 3/5/7, which do not seem to work either with my 0x0007.
>>>
>>> Are 3/5/7 channels supported by any vendor?
>> ...
>>
>> Actually, I just tested and just transmitting 3-channel data with extra
>> 4th dummy channel while setting the Audio InfoFrame fields to indicate a
>> 3 channel stream makes it work.
>>
>> So it would seem (to me) like the best approach would be:
>> 1. Add a hw_constraint to prevent odd channel counts (easy)
> 
> That seems enough to me. I'm not sure that 3/5/7 channels are a format
> that there is much use-case for?

Usage of those formats is indeed somewhat limited.

This is true especially with ALSA, since if you open a device with 3
channels, it generally means left+right+rearleft instead of the likely
intended left+right+lfe or left+right+center.

And yes, (1) is enough, the below (2) was meant to be an additional
future development.

>> 2. Provide some way for alsa plug (and the user application) to indicate
>>    the "actual/original" channel count/layout without dummy channels,
>>    so that the infoframe can be set correctly.
>>    This is harder, and I guess should probably be implemented so that
>>    the channel remapping issue on older nvidia hw will be fixed as well.
> 
> Hmm. That sounds kinda complex. If constraints prevent the application
> from using e.g. 3-channel, then implementing (2) means the plugin would
> have to also modify the constraints to re-allow 3-channel, so the
> application could actually use it, no?

No, alsa-lib would insert a dummy channel, so it gets through the
hw_constraint as it is now a 4 channel stream. This is what already
happens when the constrains are added (and the alsa device is used via
'plug', e.g. "plug:'hdmi:CARD=NVidia,DEV=3'").

What (2) would mean is that alsa-lib also informs the kernel of the
actual layout (including 4th channel being a dummy one) so that
patch_hdmi can set the infoframe correctly.

(and in case of 7x hardware, libalsa would notice that the kernel driver
has a channel map different from alsa and do some channel remapping).

-- 
Anssi Hannula