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

[Takashi Iwai]

At Tue, 12 Feb 2013 17:51:37 +0200,
Anssi Hannula wrote:
> 
> 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 at openelec.tv>
> >>>>>> Signed-off-by: Anssi Hannula <anssi.hannula at 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 :)

OK, merged to sound git tree now.
I'm going to apply the fix for alsa-lib config later.


thanks,

Takashi