Re: [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls

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

11.02.2013 13:28, Takashi Iwai kirjoitti:
> At Mon, 11 Feb 2013 13:20:03 +0200,
> Anssi Hannula wrote:
>>
>> On 11.02.2013 12:51, Takashi Iwai wrote:
>>> At Sun, 10 Feb 2013 13:05:14 +0200,
>>> Anssi Hannula wrote:
>>>>
>>>> 10.02.2013 12:38, Takashi Iwai kirjoitti:
>>>>> At Sat,  9 Feb 2013 00:44:39 +0200,
>>>>> Anssi Hannula wrote:
>>>>>>
>>>>>> Commit dcda5806165c155d90b9aa466a1602cf4726012b ("ALSA: hda - Add
>>>>>> workaround for conflicting IEC958 controls") added a workaround 
>>>> for
>>>>>> cards that have both an S/PDIF and an HDMI device, so that S/PDIF 
>>>> IEC958
>>>>>> controls will be moved to device=1 on such cards.
>>>>>>
>>>>>> However, the workaround did not take it into account that the 
>>>> S/PDIF and
>>>>>> HDMI devices may be on different codecs of the same card. 
>>>> Currently this
>>>>>> is always the case, and the workaround therefore fails to work.
>>>>>>
>>>>>> Fix the workaround to handle card-wide IEC958 conflicts.
>>>>>>
>>>>>> Reported-by: Stephan Raue <stephan@openelec.tv>
>>>>>> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
>>>>>> ---
>>>>>>
>>>>>> Unfortunately this seems to cause a nasty issue with alsa-lib 
>>>> 1.0.26:
>>>>>> $ amixer scontrols -c 0
>>>>>> ALSA lib simple_none.c:1551:(simple_add1) helem (MIXER,'IEC958 
>>>> Playback Switch',0,1,0) appears twice or more
>>>>>> amixer: Mixer hw:0 load error: Invalid argument
>>>>>>
>>>>>> The non-simple-mode "amixer controls -c 0" works fine, though.
>>>>>>
>>>>>> Not really sure what to do now then, do we revert the workaround
>>>>>> completely and devise a different workaround/fix for this, or do 
>>>> you
>>>>>> have some other good ideas?
>>>>>
>>>>> If the element isn't really dup'ed, it must be a bug in alsa-lib 
>>>> mixer
>>>>> abstraction, so it should be fixed there.
>>>>
>>>> This is because the simple mixer interface only identifies controls 
>>>> by
>>>> name+index:
>>>>
>>>> http://www.alsa-project.org/alsa-doc/alsa-lib/group___simple_mixer.html
>>>>
>>>> So controls that only differ by device (or subdevice?) are 
>>>> considered
>>>> duplicated. I did look at the code but saw no straight-forward way 
>>>> to
>>>> fix it, other than to introduce devices (and subdevices) to the 
>>>> simple
>>>> mixer API (which is used by outside applications).
>>>
>>> OK, so it's a limitation of alsa-lib mixer simple abst
>>> implementation.  We need to live with that for now...
>>>
>>>> Anyway, wouldn't breaking "old" alsa-lib make this way of fixing it 
>>>> a
>>>> no-go (the error is fatal and mixer creation fails completely)?
>>>
>>> No, it's a general rule in the kernel that we can't break the old
>>> user-space.
>>
>> Isn't that a "yes" for my no-go? ;)
> 
> I hate double negation :)
> 
> 
>>>>> Could you add alsa-info.sh output of this board (at best before 
>>>> and
>>>>> after your patch)?
>>>>
>>>> Here's one after patch (can't get one before patch right now, but I
>>>> guess it isn't needed since the cause is very clear):
>>>>
>>>> http://www.alsa-project.org/db/?f=bb3fc8680b372c0475719d42d7fc8b2bb7bfb4eb
>>>> (this one has alsa-lib hack applied which ignores the failure to add 
>>>> the
>>>> control so that the mixer error is non-fatal)
>>>
>>> Thinking it again, maybe an ugly but working workaround is to shift
>>> the SPDIF index to high enough not conflicting with HDMI, instead of
>>> changing the ctl device.
>>>
>>> The quick patch below is to put the SPDIF stuff into index=16+.  It
>>> also fixes the issue of multiple codecs.
>>>
>>> Of course, this will require a similar fix in alsa-lib config.
>>> Instead of changing dev to 1, just change index to 16.
>>
>> OK, patch looks fine to me. I'll test this later today or tomorrow and 
>> report
>> back.
> 
> Thanks.  The patch below is the revised alsa-lib patch corresponding
> to this fix.

Seems to work fine :)

>> Also, sorry for not catching this earlier.
> 
> Oh no, thanks for updates!
> 
> 
> thanks,
> 
> Takashi


-- 
Anssi Hannula