[alsa-devel] [PATCH] ALSA: hda - Do not wrongly restrict min_channels based on ELD

[Stephen Warren]

Anssi Hannula wrote:
> 
> On 07.12.2010 19:26, Stephen Warren wrote:
> > Anssi Hannula wrote:
> >>
> >> On 07.12.2010 18:58, Stephen Warren wrote:
> >>> Anssi Hannula wrote:
> >>>> Commit bbbe33900d1f3c added functionality to restrict PCM parameters
> >>>> based on ELD info (derived from EDID data) of the audio sink.
> >>>>
> >>>> However, it wrongly assumes that the bits 0-2 of the first byte of
> >>>> CEA Short Audio Descriptors mean a supported number of channels. In
> >>>> reality, they mean the maximum number of channels (as per CEA-861-D
> >>>> 7.5.2). This means that the channel count can only be used to restrict
> >>>> max_channels, not min_channels.
> >>>>
> >>>> Restricting min_channels causes us to deny opening the device in stereo
> >>>> mode if the sink only has SADs that declare larger numbers of channels
> >>>> (like Primare SP32 AV Processor does).
> >>>>
> >>>> Fix that by not restricting min_channels based on ELD information.
> [...]
> >>>> Signed-off-by: Anssi Hannula <anssi.hannula at iki.fi>
> >>>> Reported-by: Jean-Yves Avenard <jyavenard at gmail.com>
> >>>> Tested-by: Jean-Yves Avenard <jyavenard at gmail.com>
> >>>> Cc: stable at kernel.org
> [...]
> >>>>  	pcm->channels_max = min(pcm->channels_max, codec_pars->channels_max);
> >>>>  	pcm->maxbps = min(pcm->maxbps, codec_pars->maxbps);
> >>>>  }
> >>>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> >>>> index d3e49aa..31df774 100644
> >>>> --- a/sound/pci/hda/patch_hdmi.c
> >>>> +++ b/sound/pci/hda/patch_hdmi.c
> >>>> @@ -834,7 +834,6 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> >>>>  			return -ENODEV;
> >>>>  	} else {
> >>>>  		/* fallback to the codec default */
> >>>> -		hinfo->channels_min = codec_pars->channels_min;
> >>>
> >>> Isn't this still required to default the value?
> >>
> >> Not AFAICS. By default *hinfo has the codec provided parameters, and if
> >> we never update it from the ELD info, we don't need to restore it either.
> >
> > OK. Doesn't the same argument apply to the following 3 lines too then?
> 
> Nope. hinfo is persistent across open() calls, so we need to reset them
> if they were ever updated from ELD earlier.
> 
> >>>>  		hinfo->channels_max = codec_pars->channels_max;
> >>>>  		hinfo->rates = codec_pars->rates;
> >>>>  		hinfo->formats = codec_pars->formats;

OK. It seems a little risky to assume that hdmi_eld_update_pcm_info will
never touch channels_min, but it's certainly true after this patch, so the
patch will function correctly.

Sorry for the confusion/noise.

-- 
nvpublic