[alsa-devel] [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
Takashi Iwai
tiwai at suse.de
Mon Feb 11 12:28:45 CET 2013
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.
> Also, sorry for not catching this earlier.
Oh no, thanks for updates!
thanks,
Takashi
---
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] Add workaround for conflicting IEC958 controls for HD-audio
When both an SPDIF and an HDMI output are present on HD-audio, both
try to access IEC958 controls with index=0 although one of them must
be wrong. For avoiding this conflict, the recent kernel code (3.9 and
3.8 stable) moves the IEC958 controls of an SPDIF with index=16 once
when the conflict happens.
In this patch, the corresponding support is added in alsa-lib side.
The new "skip_rest" boolean flag is added to the hooked element
definition which indicates that the rest of element array will be
ignored once when this element is present and evaluated. With this
new flag, the HD-audio config takes device=1 primarily, then take
device=0 as fallback.
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
src/conf/cards/HDA-Intel.conf | 16 ++++++++++++++++
src/control/setup.c | 19 ++++++++++++++++---
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/src/conf/cards/HDA-Intel.conf b/src/conf/cards/HDA-Intel.conf
index d4f2667..3957c12 100644
--- a/src/conf/cards/HDA-Intel.conf
+++ b/src/conf/cards/HDA-Intel.conf
@@ -113,6 +113,22 @@ HDA-Intel.pcm.iec958.0 {
hook_args [
{
name "IEC958 Playback Default"
+ index 16
+ optional true
+ lock true
+ preserve true
+ value [ $AES0 $AES1 $AES2 $AES3 ]
+ }
+ {
+ name "IEC958 Playback Switch"
+ index 16
+ optional true
+ value true
+ # if this element is present, skip the rest
+ skip_rest true
+ }
+ {
+ name "IEC958 Playback Default"
lock true
preserve true
value [ $AES0 $AES1 $AES2 $AES3 ]
diff --git a/src/control/setup.c b/src/control/setup.c
index bd3599d..f23bf2c 100644
--- a/src/control/setup.c
+++ b/src/control/setup.c
@@ -396,7 +396,7 @@ static int snd_config_get_ctl_elem_value(snd_config_t *conf,
return 0;
}
-static int add_elem(snd_sctl_t *h, snd_config_t *_conf, snd_config_t *private_data)
+static int add_elem(snd_sctl_t *h, snd_config_t *_conf, snd_config_t *private_data, int *quit)
{
snd_config_t *conf;
snd_config_iterator_t i, next;
@@ -408,6 +408,7 @@ static int add_elem(snd_sctl_t *h, snd_config_t *_conf, snd_config_t *private_da
int lock = 0;
int preserve = 0;
int optional = 0;
+ int skip_rest = 0;
snd_config_t *value = NULL, *mask = NULL;
snd_sctl_elem_t *elem = NULL;
int err;
@@ -491,6 +492,13 @@ static int add_elem(snd_sctl_t *h, snd_config_t *_conf, snd_config_t *private_da
optional = err;
continue;
}
+ if (strcmp(id, "skip_rest") == 0) {
+ err = snd_config_get_bool(n);
+ if (err < 0)
+ goto _err;
+ skip_rest = err;
+ continue;
+ }
SNDERR("Unknown field %s", id);
return -EINVAL;
}
@@ -539,6 +547,9 @@ static int add_elem(snd_sctl_t *h, snd_config_t *_conf, snd_config_t *private_da
if (! optional)
SNDERR("Cannot obtain info for CTL elem (%s,'%s',%li,%li,%li): %s", snd_ctl_elem_iface_name(iface), name, index, device, subdevice, snd_strerror(err));
goto _err;
+ } else {
+ if (skip_rest)
+ *quit = 1;
}
snd_ctl_elem_value_set_id(elem->val, elem->id);
snd_ctl_elem_value_set_id(elem->old, elem->id);
@@ -594,7 +605,7 @@ int snd_sctl_build(snd_sctl_t **sctl, snd_ctl_t *handle, snd_config_t *conf, snd
{
snd_sctl_t *h;
snd_config_iterator_t i, next;
- int err;
+ int err, quit = 0;
assert(sctl);
assert(handle);
@@ -614,11 +625,13 @@ int snd_sctl_build(snd_sctl_t **sctl, snd_ctl_t *handle, snd_config_t *conf, snd
INIT_LIST_HEAD(&h->elems);
snd_config_for_each(i, next, conf) {
snd_config_t *n = snd_config_iterator_entry(i);
- err = add_elem(h, n, private_data);
+ err = add_elem(h, n, private_data, &quit);
if (err < 0) {
free_elems(h);
return err;
}
+ if (quit)
+ break;
}
*sctl = h;
return 0;
--
1.8.1.2
More information about the Alsa-devel
mailing list