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

[Anssi Hannula <anssi.hannula@iki.fi>]

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