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.
Yes, re-reading the CEA-861-D spec, that makes sense. Comments inline
though.
BTW, I noticed that no SADs are required if only basic audio is
supported. Is that case handled by the ELD parser? I.e. instead of:
pcm->channels_max = 0;
Should it be:
pcm->channels_max = 0;
if supports basic audio:
pcm->channels_max = 2;
or something like that?
> Signed-off-by: Anssi Hannula <[email protected]>
> Reported-by: Jean-Yves Avenard <[email protected]>
> Tested-by: Jean-Yves Avenard <[email protected]>
> Cc: [email protected]
> ---
>
> I think this should go to 2.6.36 stable tree as well. It does not affect
> earlier stable releases.
>
> sound/pci/hda/hda_eld.c | 4 ----
> sound/pci/hda/patch_hdmi.c | 1 -
> 2 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> index cb0c23a..47ef8aa 100644
> --- a/sound/pci/hda/hda_eld.c
> +++ b/sound/pci/hda/hda_eld.c
> @@ -601,13 +601,10 @@ void hdmi_eld_update_pcm_info(struct hdmi_eld *eld,
> struct
> hda_pcm_stream *pcm,
> pcm->rates = 0;
> pcm->formats = 0;
> pcm->maxbps = 0;
> - pcm->channels_min = -1;
> pcm->channels_max = 0;
> for (i = 0; i < eld->sad_count; i++) {
> struct cea_sad *a = &eld->sad[i];
> pcm->rates |= a->rates;
> - if (a->channels < pcm->channels_min)
> - pcm->channels_min = a->channels;
> if (a->channels > pcm->channels_max)
> pcm->channels_max = a->channels;
> if (a->format == AUDIO_CODING_TYPE_LPCM) {
That all looks OK.
> @@ -635,7 +632,6 @@ void hdmi_eld_update_pcm_info(struct hdmi_eld *eld, struct
> hda_pcm_stream *pcm,
> /* restrict the parameters by the values the codec provides */
> pcm->rates &= codec_pars->rates;
> pcm->formats &= codec_pars->formats;
> - pcm->channels_min = max(pcm->channels_min, codec_pars->channels_min);
Shouldn't that be:
pcm->channels_min = pcm->channels_min;
Or, is that assignment already made earlier as a default?
> 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?
> hinfo->channels_max = codec_pars->channels_max;
> hinfo->rates = codec_pars->rates;
> hinfo->formats = codec_pars->formats;
> --
> 1.7.3
--
nvpublic
_______________________________________________
stable mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/stable
