On 07.12.2010 19:52, Stephen Warren wrote:
> 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@xxxxxx>
>>>>>> Reported-by: Jean-Yves Avenard <jyavenard@xxxxxxxxx>
>>>>>> Tested-by: Jean-Yves Avenard <jyavenard@xxxxxxxxx>
>>>>>> Cc: stable@xxxxxxxxxx
>> [...]
>>>>>> 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.
No problem, some noise is much more preferable than possible brokenness :)
--
Anssi Hannula
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
