[alsa-devel] [PATCH] ALSA: hda: Disable odd numbered channels on HDMI controllers.
Added code in hda_intel.c to disable odd numbered channels not supported by HDMI controllers.
Signed-off-by: Nitin Daga ndaga@nvidia.com Acked-By: Stephen Warren swarren@nvidia.com --- pci/hda/hda_intel.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/pci/hda/hda_intel.c b/pci/hda/hda_intel.c index a78ea34..293c685 100644 --- a/pci/hda/hda_intel.c +++ b/pci/hda/hda_intel.c @@ -1539,6 +1539,16 @@ struct azx_pcm { struct hda_pcm_stream *hinfo[2]; };
+static unsigned int channels_2_4_6_8[] = { + 2, 4, 6, 8 +}; + +static struct snd_pcm_hw_constraint_list hw_constraints_2_4_6_8_channels = { + .count = ARRAY_SIZE(channels_2_4_6_8), + .list = channels_2_4_6_8, + .mask = 0, +}; + static int azx_pcm_open(struct snd_pcm_substream *substream) { struct azx_pcm *apcm = snd_pcm_substream_chip(substream); @@ -1566,6 +1576,9 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) 128); snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128); + snd_pcm_hw_constraint_list(runtime, 0, + SNDRV_PCM_HW_PARAM_CHANNELS, + &hw_constraints_2_4_6_8_channels); snd_hda_power_up(apcm->codec); err = hinfo->ops.open(hinfo, apcm->codec, substream); if (err < 0) {
On 13.01.2011 22:19, Nitin Daga wrote:
Added code in hda_intel.c to disable odd numbered channels not supported by HDMI controllers.
Signed-off-by: Nitin Daga ndaga@nvidia.com Acked-By: Stephen Warren swarren@nvidia.com
pci/hda/hda_intel.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/pci/hda/hda_intel.c b/pci/hda/hda_intel.c index a78ea34..293c685 100644 --- a/pci/hda/hda_intel.c +++ b/pci/hda/hda_intel.c @@ -1539,6 +1539,16 @@ struct azx_pcm { struct hda_pcm_stream *hinfo[2]; };
+static unsigned int channels_2_4_6_8[] = {
- 2, 4, 6, 8
+};
+static struct snd_pcm_hw_constraint_list hw_constraints_2_4_6_8_channels = {
- .count = ARRAY_SIZE(channels_2_4_6_8),
- .list = channels_2_4_6_8,
- .mask = 0,
+};
Hmm, can't these be const? OTOH none of the other drivers use const for snd_pcm_hw_constraint_list either, so I guess it is ok.
static int azx_pcm_open(struct snd_pcm_substream *substream) { struct azx_pcm *apcm = snd_pcm_substream_chip(substream); @@ -1566,6 +1576,9 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) 128); snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128);
- snd_pcm_hw_constraint_list(runtime, 0,
SNDRV_PCM_HW_PARAM_CHANNELS,&hw_constraints_2_4_6_8_channels);
This would seem simpler and more generic (there are codecs that have over 8 channels): snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, 2);
However, either of those would AFAICS break Si3054 Modem codec which is mono-only.
Maybe just add the constraint to patch_hdmi.c? Or use the patch as is but add 1, 10, 12, 14, 16 (or so) to the allowed channel counts list.
Not really sure what is the preferred approach, though.
snd_hda_power_up(apcm->codec); err = hinfo->ops.open(hinfo, apcm->codec, substream); if (err < 0) {
At Thu, 13 Jan 2011 22:42:39 +0200, Anssi Hannula wrote:
On 13.01.2011 22:19, Nitin Daga wrote:
Added code in hda_intel.c to disable odd numbered channels not supported by HDMI controllers.
Signed-off-by: Nitin Daga ndaga@nvidia.com Acked-By: Stephen Warren swarren@nvidia.com
pci/hda/hda_intel.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/pci/hda/hda_intel.c b/pci/hda/hda_intel.c index a78ea34..293c685 100644 --- a/pci/hda/hda_intel.c +++ b/pci/hda/hda_intel.c @@ -1539,6 +1539,16 @@ struct azx_pcm { struct hda_pcm_stream *hinfo[2]; };
+static unsigned int channels_2_4_6_8[] = {
- 2, 4, 6, 8
+};
+static struct snd_pcm_hw_constraint_list hw_constraints_2_4_6_8_channels = {
- .count = ARRAY_SIZE(channels_2_4_6_8),
- .list = channels_2_4_6_8,
- .mask = 0,
+};
Hmm, can't these be const? OTOH none of the other drivers use const for snd_pcm_hw_constraint_list either, so I guess it is ok.
static int azx_pcm_open(struct snd_pcm_substream *substream) { struct azx_pcm *apcm = snd_pcm_substream_chip(substream); @@ -1566,6 +1576,9 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) 128); snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128);
- snd_pcm_hw_constraint_list(runtime, 0,
SNDRV_PCM_HW_PARAM_CHANNELS,&hw_constraints_2_4_6_8_channels);This would seem simpler and more generic (there are codecs that have over 8 channels): snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, 2);
However, either of those would AFAICS break Si3054 Modem codec which is mono-only.
Maybe just add the constraint to patch_hdmi.c?
Yes, looks so. I applied the following patch instead.
Alternatively, we can check channels_min and channels_max that are already set in hda_intel.c::azx_pcm_open(), and apply the hw_constraint only when channels_min%2==0 or such.
But, as we aren't 100% sure whether the odd number of channels work with other (analog) codecs, it's safer to limit only for HDMI for the time being.
thanks,
Takashi
--- From ad09fc9d2156f3d37537b34418a6b79309013d33 Mon Sep 17 00:00:00 2001 From: Takashi Iwai tiwai@suse.de Date: Fri, 14 Jan 2011 09:42:27 +0100 Subject: [PATCH] ALSA: hda - Suppress the odd number of channels for HDMI
It looks like that HDMI codecs don't support the odd number of channels although HD-audio spec doesn't have the restriction. Add the hw_constraint to limit to only the even number of channels.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index f29b97b..2d28879 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1238,6 +1238,9 @@ static int simple_playback_pcm_open(struct hda_pcm_stream *hinfo, snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, hw_constraints_channels); + } else { + snd_pcm_hw_constraint_step(substream->runtime, 0, + SNDRV_PCM_HW_PARAM_CHANNELS, 2); }
return snd_hda_multi_out_dig_open(codec, &spec->multiout);
On 14.01.2011 10:49, Takashi Iwai wrote:
At Thu, 13 Jan 2011 22:42:39 +0200, Anssi Hannula wrote:
On 13.01.2011 22:19, Nitin Daga wrote:
Added code in hda_intel.c to disable odd numbered channels not supported by HDMI controllers.
Signed-off-by: Nitin Daga ndaga@nvidia.com Acked-By: Stephen Warren swarren@nvidia.com
pci/hda/hda_intel.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
[...]
This would seem simpler and more generic (there are codecs that have over 8 channels): snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, 2);
However, either of those would AFAICS break Si3054 Modem codec which is mono-only.
Maybe just add the constraint to patch_hdmi.c?
Yes, looks so. I applied the following patch instead.
It is ineffective, the generic hdmi devices use hdmi_pcm_open() instead of simple_playback_pcm_open().
Alternatively, we can check channels_min and channels_max that are already set in hda_intel.c::azx_pcm_open(), and apply the hw_constraint only when channels_min%2==0 or such.
But, as we aren't 100% sure whether the odd number of channels work with other (analog) codecs, it's safer to limit only for HDMI for the time being.
Well, snd_hda_multi_out_analog_open() already sets such a constraint, but I didn't check if all the analog multichannel codecs use that.
thanks,
Takashi
From ad09fc9d2156f3d37537b34418a6b79309013d33 Mon Sep 17 00:00:00 2001 From: Takashi Iwai tiwai@suse.de Date: Fri, 14 Jan 2011 09:42:27 +0100 Subject: [PATCH] ALSA: hda - Suppress the odd number of channels for HDMI
It looks like that HDMI codecs don't support the odd number of channels although HD-audio spec doesn't have the restriction. Add the hw_constraint to limit to only the even number of channels.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index f29b97b..2d28879 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1238,6 +1238,9 @@ static int simple_playback_pcm_open(struct hda_pcm_stream *hinfo, snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, hw_constraints_channels);
} else {
snd_pcm_hw_constraint_step(substream->runtime, 0,SNDRV_PCM_HW_PARAM_CHANNELS, 2);}
return snd_hda_multi_out_dig_open(codec, &spec->multiout);
At Fri, 14 Jan 2011 11:14:16 +0200, Anssi Hannula wrote:
On 14.01.2011 10:49, Takashi Iwai wrote:
At Thu, 13 Jan 2011 22:42:39 +0200, Anssi Hannula wrote:
On 13.01.2011 22:19, Nitin Daga wrote:
Added code in hda_intel.c to disable odd numbered channels not supported by HDMI controllers.
Signed-off-by: Nitin Daga ndaga@nvidia.com Acked-By: Stephen Warren swarren@nvidia.com
pci/hda/hda_intel.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
[...]
This would seem simpler and more generic (there are codecs that have over 8 channels): snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, 2);
However, either of those would AFAICS break Si3054 Modem codec which is mono-only.
Maybe just add the constraint to patch_hdmi.c?
Yes, looks so. I applied the following patch instead.
It is ineffective, the generic hdmi devices use hdmi_pcm_open() instead of simple_playback_pcm_open().
Ah, indeed. Fixed now.
Meanwhile, I found another bug in patch_hdmi.c. The updated PCM parameters aren't reflected properly at the first time because runtime->hw was already set. Fixed by patch below.
Alternatively, we can check channels_min and channels_max that are already set in hda_intel.c::azx_pcm_open(), and apply the hw_constraint only when channels_min%2==0 or such.
But, as we aren't 100% sure whether the odd number of channels work with other (analog) codecs, it's safer to limit only for HDMI for the time being.
Well, snd_hda_multi_out_analog_open() already sets such a constraint, but I didn't check if all the analog multichannel codecs use that.
Right. OTOH, the spec doesn't restrict it, so limiting it in the controller driver side (hda_intel.c) doesn't sound right. It's rather likely a codec-specific issue. So, restriction should be applied in the codec side, IMO.
thanks,
Takashi
--- From 639cef0eb6df05d5516520aa89b0c9fe62ee2d3b Mon Sep 17 00:00:00 2001 From: Takashi Iwai tiwai@suse.de Date: Fri, 14 Jan 2011 10:30:46 +0100 Subject: [PATCH] ALSA: hda - Store PCM parameters properly in HDMI open callback
In hdmi_pcm_open(), the evaluated PCM hw parameters are stored in hinfo, but these aren't properly set back to the current runtime record since these have been set beforehand in azx_pcm_open(). This patch fixes the behavior.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 2d28879..5980552 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -817,6 +817,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo, struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld; struct hda_pcm_stream *codec_pars; + struct snd_pcm_runtime *runtime = substream->runtime; unsigned int idx;
for (idx = 0; idx < spec->num_cvts; idx++) @@ -844,6 +845,11 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo, hinfo->formats = codec_pars->formats; hinfo->maxbps = codec_pars->maxbps; } + /* store the updated parameters */ + runtime->hw.channels_min = hinfo->channels_min; + runtime->hw.channels_max = hinfo->channels_max; + runtime->hw.formats = hinfo->formats; + runtime->hw.rates = hinfo->rates; return 0; }
participants (3)
-
Anssi Hannula -
Nitin Daga -
Takashi Iwai