[alsa-devel] [PATCH] ALSA: hda: Disable odd numbered channels on HDMI controllers.

[Takashi Iwai]

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 at nvidia.com>
> >>> Acked-By: Stephen Warren <swarren at 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 at 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 at 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;
 }
 
-- 
1.7.3.4