[PATCH] ALSA: hda - Always allow basic audio irrespective of ELD info

[PATCH] ALSA: hda - Always allow basic audio irrespective of ELD info

[Anssi Hannula]

Commit bbbe33900d1f3c added functionality to restrict PCM parameters
based on ELD info (derived from EDID data) of the audio sink.

However, according to CEA-861-D no SAD is needed for basic audio
(32/44.1/48kHz stereo 16-bit audio), which is instead indicated with a
basic audio flag in the CEA EDID Extension.

The flag is not present in ELD. However, as all audio capable sinks are
required to support basic audio, we can assume it to be always
available.

Fix allowed audio formats with sinks that have SADs (Short Audio
Descriptors) which do not completely overlap with the basic audio
formats (there are no reports of affected devices so far) by always
assuming that basic audio is supported.

Reported-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
---

Tested on 2.6.36 and I saw no obvious regressions.

The issue affects stable 2.6.36 as well, but as there are no reports of
affected devices, I'll leave the decision on whether to send this to
stable@ for the ALSA subsystem maintainers.

 sound/pci/hda/hda_eld.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index 47ef8aa..009031f 100644
--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -598,21 +598,19 @@ void hdmi_eld_update_pcm_info(struct hdmi_eld *eld, struct hda_pcm_stream *pcm,
 {
 	int i;
 
-	pcm->rates = 0;
-	pcm->formats = 0;
-	pcm->maxbps = 0;
-	pcm->channels_max = 0;
+	/* assume basic audio support (the basic audio flag is not in ELD;
+	 * however, all audio capable sinks are required to support basic
+	 * audio) */
+	pcm->rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000;
+	pcm->formats = SNDRV_PCM_FMTBIT_S16_LE;
+	pcm->maxbps = 16;
+	pcm->channels_max = 2;
 	for (i = 0; i < eld->sad_count; i++) {
 		struct cea_sad *a = &eld->sad[i];
 		pcm->rates |= a->rates;
 		if (a->channels > pcm->channels_max)
 			pcm->channels_max = a->channels;
 		if (a->format == AUDIO_CODING_TYPE_LPCM) {
-			if (a->sample_bits & AC_SUPPCM_BITS_16) {
-				pcm->formats |= SNDRV_PCM_FMTBIT_S16_LE;
-				if (pcm->maxbps < 16)
-					pcm->maxbps = 16;
-			}
 			if (a->sample_bits & AC_SUPPCM_BITS_20) {
 				pcm->formats |= SNDRV_PCM_FMTBIT_S32_LE;
 				if (pcm->maxbps < 20)
-- 
1.7.3

Re: [PATCH] ALSA: hda - Always allow basic audio irrespective of ELD info

[Takashi Iwai]

At Tue,  7 Dec 2010 20:56:19 +0200,
Anssi Hannula wrote:
> 
> Commit bbbe33900d1f3c added functionality to restrict PCM parameters
> based on ELD info (derived from EDID data) of the audio sink.
> 
> However, according to CEA-861-D no SAD is needed for basic audio
> (32/44.1/48kHz stereo 16-bit audio), which is instead indicated with a
> basic audio flag in the CEA EDID Extension.
> 
> The flag is not present in ELD. However, as all audio capable sinks are
> required to support basic audio, we can assume it to be always
> available.
> 
> Fix allowed audio formats with sinks that have SADs (Short Audio
> Descriptors) which do not completely overlap with the basic audio
> formats (there are no reports of affected devices so far) by always
> assuming that basic audio is supported.
> 
> Reported-by: Stephen Warren <swarren@nvidia.com>
> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
> ---
> 
> Tested on 2.6.36 and I saw no obvious regressions.
> 
> The issue affects stable 2.6.36 as well, but as there are no reports of
> affected devices, I'll leave the decision on whether to send this to
> stable@ for the ALSA subsystem maintainers.

I'm inclined for adding stable in this case, because this is more
"safer" move for users.

Will the patch "ALSA: hda - Do not wrongly restrict min_channels based
on ELD" be resent or shall I apply it as is now together with this?


thanks,

Takashi


> 
>  sound/pci/hda/hda_eld.c |   16 +++++++---------
>  1 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> index 47ef8aa..009031f 100644
> --- a/sound/pci/hda/hda_eld.c
> +++ b/sound/pci/hda/hda_eld.c
> @@ -598,21 +598,19 @@ void hdmi_eld_update_pcm_info(struct hdmi_eld *eld, struct hda_pcm_stream *pcm,
>  {
>  	int i;
>  
> -	pcm->rates = 0;
> -	pcm->formats = 0;
> -	pcm->maxbps = 0;
> -	pcm->channels_max = 0;
> +	/* assume basic audio support (the basic audio flag is not in ELD;
> +	 * however, all audio capable sinks are required to support basic
> +	 * audio) */
> +	pcm->rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000;
> +	pcm->formats = SNDRV_PCM_FMTBIT_S16_LE;
> +	pcm->maxbps = 16;
> +	pcm->channels_max = 2;
>  	for (i = 0; i < eld->sad_count; i++) {
>  		struct cea_sad *a = &eld->sad[i];
>  		pcm->rates |= a->rates;
>  		if (a->channels > pcm->channels_max)
>  			pcm->channels_max = a->channels;
>  		if (a->format == AUDIO_CODING_TYPE_LPCM) {
> -			if (a->sample_bits & AC_SUPPCM_BITS_16) {
> -				pcm->formats |= SNDRV_PCM_FMTBIT_S16_LE;
> -				if (pcm->maxbps < 16)
> -					pcm->maxbps = 16;
> -			}
>  			if (a->sample_bits & AC_SUPPCM_BITS_20) {
>  				pcm->formats |= SNDRV_PCM_FMTBIT_S32_LE;
>  				if (pcm->maxbps < 20)
> -- 
> 1.7.3
> 

Re: [PATCH] ALSA: hda - Always allow basic audio irrespective of ELD info

[Anssi Hannula]

On 07.12.2010 21:09, Takashi Iwai wrote:
> At Tue,  7 Dec 2010 20:56:19 +0200,
> Anssi Hannula wrote:
>>
>> Commit bbbe33900d1f3c added functionality to restrict PCM parameters
>> based on ELD info (derived from EDID data) of the audio sink.
>>
>> However, according to CEA-861-D no SAD is needed for basic audio
>> (32/44.1/48kHz stereo 16-bit audio), which is instead indicated with a
>> basic audio flag in the CEA EDID Extension.
>>
>> The flag is not present in ELD. However, as all audio capable sinks are
>> required to support basic audio, we can assume it to be always
>> available.
>>
>> Fix allowed audio formats with sinks that have SADs (Short Audio
>> Descriptors) which do not completely overlap with the basic audio
>> formats (there are no reports of affected devices so far) by always
>> assuming that basic audio is supported.
>>
>> Reported-by: Stephen Warren <swarren@nvidia.com>
>> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
>> ---
>>
>> Tested on 2.6.36 and I saw no obvious regressions.
>>
>> The issue affects stable 2.6.36 as well, but as there are no reports of
>> affected devices, I'll leave the decision on whether to send this to
>> stable@ for the ALSA subsystem maintainers.
> 
> I'm inclined for adding stable in this case, because this is more
> "safer" move for users.

Agreed.

> Will the patch "ALSA: hda - Do not wrongly restrict min_channels based
> on ELD" be resent or shall I apply it as is now together with this?

IMO it is ok as is.

> thanks,
> 
> Takashi
> 
> 
>>
>>  sound/pci/hda/hda_eld.c |   16 +++++++---------
>>  1 files changed, 7 insertions(+), 9 deletions(-)
>>
>> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
>> index 47ef8aa..009031f 100644
>> --- a/sound/pci/hda/hda_eld.c
>> +++ b/sound/pci/hda/hda_eld.c
>> @@ -598,21 +598,19 @@ void hdmi_eld_update_pcm_info(struct hdmi_eld *eld, struct hda_pcm_stream *pcm,
>>  {
>>  	int i;
>>  
>> -	pcm->rates = 0;
>> -	pcm->formats = 0;
>> -	pcm->maxbps = 0;
>> -	pcm->channels_max = 0;
>> +	/* assume basic audio support (the basic audio flag is not in ELD;
>> +	 * however, all audio capable sinks are required to support basic
>> +	 * audio) */
>> +	pcm->rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000;
>> +	pcm->formats = SNDRV_PCM_FMTBIT_S16_LE;
>> +	pcm->maxbps = 16;
>> +	pcm->channels_max = 2;
>>  	for (i = 0; i < eld->sad_count; i++) {
>>  		struct cea_sad *a = &eld->sad[i];
>>  		pcm->rates |= a->rates;
>>  		if (a->channels > pcm->channels_max)
>>  			pcm->channels_max = a->channels;
>>  		if (a->format == AUDIO_CODING_TYPE_LPCM) {
>> -			if (a->sample_bits & AC_SUPPCM_BITS_16) {
>> -				pcm->formats |= SNDRV_PCM_FMTBIT_S16_LE;
>> -				if (pcm->maxbps < 16)
>> -					pcm->maxbps = 16;
>> -			}
>>  			if (a->sample_bits & AC_SUPPCM_BITS_20) {
>>  				pcm->formats |= SNDRV_PCM_FMTBIT_S32_LE;
>>  				if (pcm->maxbps < 20)
>> -- 
>> 1.7.3
>>


-- 
Anssi Hannula

Re: [PATCH] ALSA: hda - Always allow basic audio irrespective of ELD info

[Takashi Iwai]

At Tue, 07 Dec 2010 21:11:03 +0200,
Anssi Hannula wrote:
> 
> On 07.12.2010 21:09, Takashi Iwai wrote:
> > At Tue,  7 Dec 2010 20:56:19 +0200,
> > Anssi Hannula wrote:
> >>
> >> Commit bbbe33900d1f3c added functionality to restrict PCM parameters
> >> based on ELD info (derived from EDID data) of the audio sink.
> >>
> >> However, according to CEA-861-D no SAD is needed for basic audio
> >> (32/44.1/48kHz stereo 16-bit audio), which is instead indicated with a
> >> basic audio flag in the CEA EDID Extension.
> >>
> >> The flag is not present in ELD. However, as all audio capable sinks are
> >> required to support basic audio, we can assume it to be always
> >> available.
> >>
> >> Fix allowed audio formats with sinks that have SADs (Short Audio
> >> Descriptors) which do not completely overlap with the basic audio
> >> formats (there are no reports of affected devices so far) by always
> >> assuming that basic audio is supported.
> >>
> >> Reported-by: Stephen Warren <swarren@nvidia.com>
> >> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
> >> ---
> >>
> >> Tested on 2.6.36 and I saw no obvious regressions.
> >>
> >> The issue affects stable 2.6.36 as well, but as there are no reports of
> >> affected devices, I'll leave the decision on whether to send this to
> >> stable@ for the ALSA subsystem maintainers.
> > 
> > I'm inclined for adding stable in this case, because this is more
> > "safer" move for users.
> 
> Agreed.
> 
> > Will the patch "ALSA: hda - Do not wrongly restrict min_channels based
> > on ELD" be resent or shall I apply it as is now together with this?
> 
> IMO it is ok as is.

OK, applied both patches now.
Thanks!


Takashi