* [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
@ 2012-10-12 15:18 Takashi Iwai
2012-10-12 15:24 ` [PATCH RFC] ALSA: hda - Add workaround for conflicting " Takashi Iwai
` (5 more replies)
0 siblings, 6 replies; 32+ messages in thread
From: Takashi Iwai @ 2012-10-12 15:18 UTC (permalink / raw)
To: alsa-devel
Hi,
there is a long-standing problem in HD-audio regarding IEC958
controls. When both an SPDIF and an HDMI are created on the same
card (e.g. one from analog codec and one from graphics chip), the
driver assigns the IEC958 controls just with new indices, 0, 1, 2...
The problem is that there is no way to connect between this index and
the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
configuration:
spdif -> IEC958 xxx index=0, PCM dev=1
hdmi,0 -> IEC958 xxx index=0, PCM dev=3
hdmi,1 -> IEC958 xxx index=1, PCM dev=7
hdmi,2 -> IEC958 xxx index=2, PCM dev=8
hdmi,3 -> IEC958 xxx index=3, PCM dev=9
So obviously spdif and the first hdmi conflict.
Basically this can be fixed by reassigning each IEC958 control with
the same device number corresponding to the PCM device. That is, for
SPDIF, assign a control element with device=1, for HDMI,
device=3,7...
However, this obviously breaks the old configuration unless user
upgrades the alsa-lib configuration. Thus this is no-go.
Now here is a compromise: the IEC958 control for SPDIF is reassigned
to device=1 *only* when SPDIF and HDMI coexist. Since the
configuration is anyway broken as is now in such a case, it's no big
deal to fix one side in an incompatible way. (The reason why SPDIF
is re-assigned is that I guess majority of user require more HDMI than
SPDIF in such a configuration.)
In addition, we need a fix in alsa-lib. Also not for breaking the
compatibility with older kernel, we need some fallback check. I did a
quick hack to alsa-lib conf code and added "skip_rest" option to the
hook element, so that only the first matching element is taken.
Long story short, I cooked up three patches. One patch is for kernel,
to add the workaround above, and the two are for alsa-lib, one clean
up and one to introduce the skip_rest option (and application to
HDA-Intel.conf).
My plan is to merge this to 3.8 tree, so it's no urgent issue.
But it's of course always good to fix something.
thanks,
Takashi
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH RFC] ALSA: hda - Add workaround for conflicting IEC958 controls
2012-10-12 15:18 [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Takashi Iwai
@ 2012-10-12 15:24 ` Takashi Iwai
2012-10-17 8:17 ` Takashi Iwai
2013-02-08 22:44 ` [PATCH] ALSA: hda - Fix the " Anssi Hannula
2012-10-12 15:25 ` [PATCH RFC 1/2] control: Simplify using snd_config_get_bool() Takashi Iwai
` (4 subsequent siblings)
5 siblings, 2 replies; 32+ messages in thread
From: Takashi Iwai @ 2012-10-12 15:24 UTC (permalink / raw)
To: alsa-devel
When both an SPDIF and an HDMI device are created on the same card
instance, multiple IEC958 controls are created with indices=0, 1, ...
But the alsa-lib configuration can't know which index corresponds
actually to which PCM device, and both the SPDIF and the HDMI
configurations point to the first IEC958 control wrongly.
This patch introduces a (hackish and ugly) workaround: the IEC958
controls for the SPDIF device are re-labeled with device=1 when HDMI
coexists. The device=1 corresponds to the actual PCM device for
SPDIF, so it's anyway a better representation. In future, HDMI
controls should be moved with the corresponding PCM device number,
too.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/pci/hda/hda_codec.c | 60 +++++++++++++++++++++++++++++-------------
sound/pci/hda/hda_codec.h | 1 +
sound/pci/hda/hda_local.h | 8 +++---
sound/pci/hda/patch_cirrus.c | 5 ++--
sound/pci/hda/patch_hdmi.c | 7 ++---
sound/pci/hda/patch_realtek.c | 7 ++---
sound/pci/hda/patch_sigmatel.c | 7 ++---
7 files changed, 63 insertions(+), 32 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 70d4848..2878bab 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2151,12 +2151,12 @@ EXPORT_SYMBOL_HDA(snd_hda_set_vmaster_tlv);
/* find a mixer control element with the given name */
static struct snd_kcontrol *
-_snd_hda_find_mixer_ctl(struct hda_codec *codec,
- const char *name, int idx)
+find_mixer_ctl(struct hda_codec *codec, const char *name, int dev, int idx)
{
struct snd_ctl_elem_id id;
memset(&id, 0, sizeof(id));
id.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+ id.device = dev;
id.index = idx;
if (snd_BUG_ON(strlen(name) >= sizeof(id.name)))
return NULL;
@@ -2174,15 +2174,16 @@ _snd_hda_find_mixer_ctl(struct hda_codec *codec,
struct snd_kcontrol *snd_hda_find_mixer_ctl(struct hda_codec *codec,
const char *name)
{
- return _snd_hda_find_mixer_ctl(codec, name, 0);
+ return find_mixer_ctl(codec, name, 0, 0);
}
EXPORT_SYMBOL_HDA(snd_hda_find_mixer_ctl);
-static int find_empty_mixer_ctl_idx(struct hda_codec *codec, const char *name)
+static int find_empty_mixer_ctl_idx(struct hda_codec *codec, const char *name,
+ int dev)
{
int idx;
for (idx = 0; idx < 16; idx++) { /* 16 ctlrs should be large enough */
- if (!_snd_hda_find_mixer_ctl(codec, name, idx))
+ if (!find_mixer_ctl(codec, name, dev, idx))
return idx;
}
return -EBUSY;
@@ -3133,26 +3134,48 @@ static struct snd_kcontrol_new dig_mixes[] = {
};
/**
- * snd_hda_create_spdif_out_ctls - create Output SPDIF-related controls
+ * snd_hda_create_dig_out_ctls - create Output SPDIF-related controls
* @codec: the HDA codec
- * @nid: audio out widget NID
- *
- * Creates controls related with the SPDIF output.
- * Called from each patch supporting the SPDIF out.
+ * @associated_nid: NID that new ctls associated with
+ * @cvt_nid: converter NID
+ * @type: HDA_PCM_TYPE_*
+ * Creates controls related with the digital output.
+ * Called from each patch supporting the digital out.
*
* Returns 0 if successful, or a negative error code.
*/
-int snd_hda_create_spdif_out_ctls(struct hda_codec *codec,
- hda_nid_t associated_nid,
- hda_nid_t cvt_nid)
+int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
+ hda_nid_t associated_nid,
+ hda_nid_t cvt_nid,
+ int type)
{
int err;
struct snd_kcontrol *kctl;
struct snd_kcontrol_new *dig_mix;
- int idx;
+ int idx, dev = 0;
+ const int spdif_pcm_dev = 1;
struct hda_spdif_out *spdif;
- idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch");
+ if (codec->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
+ type == HDA_PCM_TYPE_SPDIF) {
+ dev = spdif_pcm_dev;
+ } else if (codec->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
+ type == HDA_PCM_TYPE_HDMI) {
+ for (idx = 0; idx < codec->spdif_out.used; idx++) {
+ spdif = snd_array_elem(&codec->spdif_out, idx);
+ for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
+ kctl = find_mixer_ctl(codec, dig_mix->name, 0, idx);
+ if (!kctl)
+ break;
+ kctl->id.device = spdif_pcm_dev;
+ }
+ }
+ codec->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
+ }
+ if (!codec->primary_dig_out_type)
+ codec->primary_dig_out_type = type;
+
+ idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch", dev);
if (idx < 0) {
printk(KERN_ERR "hda_codec: too many IEC958 outputs\n");
return -EBUSY;
@@ -3162,6 +3185,7 @@ int snd_hda_create_spdif_out_ctls(struct hda_codec *codec,
kctl = snd_ctl_new1(dig_mix, codec);
if (!kctl)
return -ENOMEM;
+ kctl->id.device = dev;
kctl->id.index = idx;
kctl->private_value = codec->spdif_out.used - 1;
err = snd_hda_ctl_add(codec, associated_nid, kctl);
@@ -3174,7 +3198,7 @@ int snd_hda_create_spdif_out_ctls(struct hda_codec *codec,
spdif->status = convert_to_spdif_status(spdif->ctls);
return 0;
}
-EXPORT_SYMBOL_HDA(snd_hda_create_spdif_out_ctls);
+EXPORT_SYMBOL_HDA(snd_hda_create_dig_out_ctls);
/* get the hda_spdif_out entry from the given NID
* call within spdif_mutex lock
@@ -3349,7 +3373,7 @@ int snd_hda_create_spdif_in_ctls(struct hda_codec *codec, hda_nid_t nid)
struct snd_kcontrol_new *dig_mix;
int idx;
- idx = find_empty_mixer_ctl_idx(codec, "IEC958 Capture Switch");
+ idx = find_empty_mixer_ctl_idx(codec, "IEC958 Capture Switch", 0);
if (idx < 0) {
printk(KERN_ERR "hda_codec: too many IEC958 inputs\n");
return -EBUSY;
@@ -4449,7 +4473,7 @@ int snd_hda_add_new_ctls(struct hda_codec *codec,
addr = codec->addr;
else if (!idx && !knew->index) {
idx = find_empty_mixer_ctl_idx(codec,
- knew->name);
+ knew->name, 0);
if (idx <= 0)
return err;
} else
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 507fe8a..e972c23 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -836,6 +836,7 @@ struct hda_codec {
struct mutex hash_mutex;
struct snd_array spdif_out;
unsigned int spdif_in_enable; /* SPDIF input enable? */
+ int primary_dig_out_type; /* primary digital out PCM type */
const hda_nid_t *slave_dig_outs; /* optional digital out slave widgets */
struct snd_array init_pins; /* initial (BIOS) pin configurations */
struct snd_array driver_pins; /* pin configs set by codec parser */
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 09dbdc3..8c43198 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -240,9 +240,11 @@ int snd_hda_mixer_bind_tlv(struct snd_kcontrol *kcontrol, int op_flag,
/*
* SPDIF I/O
*/
-int snd_hda_create_spdif_out_ctls(struct hda_codec *codec,
- hda_nid_t associated_nid,
- hda_nid_t cvt_nid);
+int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
+ hda_nid_t associated_nid,
+ hda_nid_t cvt_nid, int type);
+#define snd_hda_create_spdif_out_ctls(codec, anid, cnid) \
+ snd_hda_create_dig_out_ctls(codec, anid, cnid, HDA_PCM_TYPE_SPDIF)
int snd_hda_create_spdif_in_ctls(struct hda_codec *codec, hda_nid_t nid);
/*
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 61a7113..a7f8790 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -873,8 +873,9 @@ static int build_digital_output(struct hda_codec *codec)
if (!spec->multiout.dig_out_nid)
return 0;
- err = snd_hda_create_spdif_out_ctls(codec, spec->multiout.dig_out_nid,
- spec->multiout.dig_out_nid);
+ err = snd_hda_create_dig_out_ctls(codec, spec->multiout.dig_out_nid,
+ spec->multiout.dig_out_nid,
+ spec->pcm_rec[1].pcm_type);
if (err < 0)
return err;
err = snd_hda_create_spdif_share_sw(codec, &spec->multiout);
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 71555cc..39ca100 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1589,9 +1589,10 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
if (err < 0)
return err;
- err = snd_hda_create_spdif_out_ctls(codec,
- per_pin->pin_nid,
- per_pin->mux_nids[0]);
+ err = snd_hda_create_dig_out_ctls(codec,
+ per_pin->pin_nid,
+ per_pin->mux_nids[0],
+ HDA_PCM_TYPE_HDMI);
if (err < 0)
return err;
snd_hda_spdif_ctls_unassign(codec, pin_idx);
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 8253b4e..2d2bb66 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -1836,9 +1836,10 @@ static int __alc_build_controls(struct hda_codec *codec)
return err;
}
if (spec->multiout.dig_out_nid) {
- err = snd_hda_create_spdif_out_ctls(codec,
- spec->multiout.dig_out_nid,
- spec->multiout.dig_out_nid);
+ err = snd_hda_create_dig_out_ctls(codec,
+ spec->multiout.dig_out_nid,
+ spec->multiout.dig_out_nid,
+ spec->pcm_rec[1].pcm_type);
if (err < 0)
return err;
if (!spec->no_analog) {
diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
index 770013f..6214165 100644
--- a/sound/pci/hda/patch_sigmatel.c
+++ b/sound/pci/hda/patch_sigmatel.c
@@ -1136,9 +1136,10 @@ static int stac92xx_build_controls(struct hda_codec *codec)
}
if (spec->multiout.dig_out_nid) {
- err = snd_hda_create_spdif_out_ctls(codec,
- spec->multiout.dig_out_nid,
- spec->multiout.dig_out_nid);
+ err = snd_hda_create_dig_out_ctls(codec,
+ spec->multiout.dig_out_nid,
+ spec->multiout.dig_out_nid,
+ spec->autocfg.dig_out_type[0]);
if (err < 0)
return err;
err = snd_hda_create_spdif_share_sw(codec,
--
1.7.12.2
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH RFC 1/2] control: Simplify using snd_config_get_bool()
2012-10-12 15:18 [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Takashi Iwai
2012-10-12 15:24 ` [PATCH RFC] ALSA: hda - Add workaround for conflicting " Takashi Iwai
@ 2012-10-12 15:25 ` Takashi Iwai
2012-10-12 15:25 ` [PATCH RFC 2/2] Add workaround for conflicting IEC958 controls for HD-audio Takashi Iwai
` (3 subsequent siblings)
5 siblings, 0 replies; 32+ messages in thread
From: Takashi Iwai @ 2012-10-12 15:25 UTC (permalink / raw)
To: alsa-devel
snd_config_get_bool() was improved to parse also ASCII strings now,
so we don't have to open-code the boolean parser in
src/control/setup.c any longer.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
src/control/setup.c | 37 ++++++-------------------------------
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/control/setup.c b/src/control/setup.c
index eecda45..bd3599d 100644
--- a/src/control/setup.c
+++ b/src/control/setup.c
@@ -400,7 +400,6 @@ static int add_elem(snd_sctl_t *h, snd_config_t *_conf, snd_config_t *private_da
{
snd_config_t *conf;
snd_config_iterator_t i, next;
- char *tmp;
int iface = SND_CTL_ELEM_IFACE_MIXER;
const char *name = NULL;
long index = 0;
@@ -464,33 +463,17 @@ static int add_elem(snd_sctl_t *h, snd_config_t *_conf, snd_config_t *private_da
continue;
}
if (strcmp(id, "lock") == 0) {
- if ((err = snd_config_get_ascii(n, &tmp)) < 0) {
- SNDERR("field %s has an invalid type", id);
- goto _err;
- }
- err = snd_config_get_bool_ascii(tmp);
- if (err < 0) {
- SNDERR("field %s is not a boolean", id);
- free(tmp);
+ err = snd_config_get_bool(n);
+ if (err < 0)
goto _err;
- }
lock = err;
- free(tmp);
continue;
}
if (strcmp(id, "preserve") == 0) {
- if ((err = snd_config_get_ascii(n, &tmp)) < 0) {
- SNDERR("field %s has an invalid type", id);
- goto _err;
- }
- err = snd_config_get_bool_ascii(tmp);
- if (err < 0) {
- SNDERR("field %s is not a boolean", id);
- free(tmp);
+ err = snd_config_get_bool(n);
+ if (err < 0)
goto _err;
- }
preserve = err;
- free(tmp);
continue;
}
if (strcmp(id, "value") == 0) {
@@ -502,18 +485,10 @@ static int add_elem(snd_sctl_t *h, snd_config_t *_conf, snd_config_t *private_da
continue;
}
if (strcmp(id, "optional") == 0) {
- if ((err = snd_config_get_ascii(n, &tmp)) < 0) {
- SNDERR("field %s has an invalid type", id);
- goto _err;
- }
- err = snd_config_get_bool_ascii(tmp);
- if (err < 0) {
- SNDERR("field %s is not a boolean", id);
- free(tmp);
+ err = snd_config_get_bool(n);
+ if (err < 0)
goto _err;
- }
optional = err;
- free(tmp);
continue;
}
SNDERR("Unknown field %s", id);
--
1.7.12.2
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH RFC 2/2] Add workaround for conflicting IEC958 controls for HD-audio
2012-10-12 15:18 [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Takashi Iwai
2012-10-12 15:24 ` [PATCH RFC] ALSA: hda - Add workaround for conflicting " Takashi Iwai
2012-10-12 15:25 ` [PATCH RFC 1/2] control: Simplify using snd_config_get_bool() Takashi Iwai
@ 2012-10-12 15:25 ` Takashi Iwai
2013-02-03 16:40 ` Anssi Hannula
2012-10-15 2:31 ` [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Raymond Yau
` (2 subsequent siblings)
5 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2012-10-12 15:25 UTC (permalink / raw)
To: alsa-devel
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 moves
the IEC958 controls of an SPDIF with device=1 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@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..55fb624 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"
+ device 1
+ optional true
+ lock true
+ preserve true
+ value [ $AES0 $AES1 $AES2 $AES3 ]
+ }
+ {
+ name "IEC958 Playback Switch"
+ device 1
+ 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.7.12.2
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-12 15:18 [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Takashi Iwai
` (2 preceding siblings ...)
2012-10-12 15:25 ` [PATCH RFC 2/2] Add workaround for conflicting IEC958 controls for HD-audio Takashi Iwai
@ 2012-10-15 2:31 ` Raymond Yau
2012-10-15 7:46 ` Takashi Iwai
2012-10-15 10:09 ` David Henningsson
2012-10-15 10:21 ` Jaroslav Kysela
5 siblings, 1 reply; 32+ messages in thread
From: Raymond Yau @ 2012-10-15 2:31 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
2012-10-12 下午11:19 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
>
> Hi,
>
> there is a long-standing problem in HD-audio regarding IEC958
> controls. When both an SPDIF and an HDMI are created on the same
> card (e.g. one from analog codec and one from graphics chip), the
> driver assigns the IEC958 controls just with new indices, 0, 1, 2...
>
> The problem is that there is no way to connect between this index and
> the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> configuration:
> spdif -> IEC958 xxx index=0, PCM dev=1
> hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> hdmi,3 -> IEC958 xxx index=3, PCM dev=9
>
> So obviously spdif and the first hdmi conflict.
>
> Basically this can be fixed by reassigning each IEC958 control with
> the same device number corresponding to the PCM device. That is, for
> SPDIF, assign a control element with device=1, for HDMI,
> device=3,7...
> However, this obviously breaks the old configuration unless user
> upgrades the alsa-lib configuration. Thus this is no-go.
>
> Now here is a compromise: the IEC958 control for SPDIF is reassigned
> to device=1 *only* when SPDIF and HDMI coexist. Since the
> configuration is anyway broken as is now in such a case, it's no big
> deal to fix one side in an incompatible way. (The reason why SPDIF
> is re-assigned is that I guess majority of user require more HDMI than
> SPDIF in such a configuration.)
>
> In addition, we need a fix in alsa-lib. Also not for breaking the
> compatibility with older kernel, we need some fallback check. I did a
> quick hack to alsa-lib conf code and added "skip_rest" option to the
> hook element, so that only the first matching element is taken.
how about those hdmi jack and iec958 on ad1989b?
there is no device 3 but [Jack] SPDIF Out at Ext Rear and [Jack] Digital
Out at Int HDMI
**** List of PLAYBACK Hardware Devices ****
card 0: Intel [HDA Intel], device 0: AD198x Analog [AD198x Analog]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: Intel [HDA Intel], device 1: AD198x Digital [AD198x Digital]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: Intel [HDA Intel], device 2: AD198x Headphone [AD198x Headphone]
Subdevices: 1/1
Subdevice #0: subdevice #0
https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/995169/+attachment/3132070/+files/Card0.Codecs.codec.0.txt
Node 0x1b [Pin Complex] wcaps 0x40030d: Stereo Digital Amp-Out
Control: name="IEC958 Playback Volume", index=0, device=0
ControlAmp: chs=3, dir=Out, idx=0, ofs=0
Amp-Out caps: ofs=0x27, nsteps=0x27, stepsize=0x05, mute=1
Amp-Out vals: [0x00 0x00]
Pincap 0x00000010: OUT
Pin Default 0x0145f1a0: [Jack] SPDIF Out at Ext Rear
Conn = Optical, Color = Other
DefAssociation = 0xa, Sequence = 0x0
Misc = NO_PRESENCE
Pin-ctls: 0x40: OUT
Connection: 1
0x02
Node 0x1d [Pin Complex] wcaps 0x40030d: Stereo Digital Amp-Out
Control: name="HDMI Playback Volume", index=0, device=0
ControlAmp: chs=3, dir=Out, idx=0, ofs=0
Amp-Out caps: ofs=0x27, nsteps=0x27, stepsize=0x05, mute=1
Amp-Out vals: [0x27 0x27]
Pincap 0x00000010: OUT
Pin Default 0x1856f1b0: [Jack] Digital Out at Int HDMI
Conn = Digital, Color = Other
DefAssociation = 0xb, Sequence = 0x0
Misc = NO_PRESENCE
Pin-ctls: 0x40: OUT
Connection: 1
0x0b
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 2:31 ` [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Raymond Yau
@ 2012-10-15 7:46 ` Takashi Iwai
2012-10-15 8:15 ` Raymond Yau
0 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2012-10-15 7:46 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel
At Mon, 15 Oct 2012 10:31:50 +0800,
Raymond Yau wrote:
>
> 2012-10-12 下午11:19 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> >
> > Hi,
> >
> > there is a long-standing problem in HD-audio regarding IEC958
> > controls. When both an SPDIF and an HDMI are created on the same
> > card (e.g. one from analog codec and one from graphics chip), the
> > driver assigns the IEC958 controls just with new indices, 0, 1, 2...
> >
> > The problem is that there is no way to connect between this index and
> > the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> > configuration:
> > spdif -> IEC958 xxx index=0, PCM dev=1
> > hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> > hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> > hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> > hdmi,3 -> IEC958 xxx index=3, PCM dev=9
> >
> > So obviously spdif and the first hdmi conflict.
> >
> > Basically this can be fixed by reassigning each IEC958 control with
> > the same device number corresponding to the PCM device. That is, for
> > SPDIF, assign a control element with device=1, for HDMI,
> > device=3,7...
> > However, this obviously breaks the old configuration unless user
> > upgrades the alsa-lib configuration. Thus this is no-go.
> >
> > Now here is a compromise: the IEC958 control for SPDIF is reassigned
> > to device=1 *only* when SPDIF and HDMI coexist. Since the
> > configuration is anyway broken as is now in such a case, it's no big
> > deal to fix one side in an incompatible way. (The reason why SPDIF
> > is re-assigned is that I guess majority of user require more HDMI than
> > SPDIF in such a configuration.)
> >
> > In addition, we need a fix in alsa-lib. Also not for breaking the
> > compatibility with older kernel, we need some fallback check. I did a
> > quick hack to alsa-lib conf code and added "skip_rest" option to the
> > hook element, so that only the first matching element is taken.
>
> how about those hdmi jack and iec958 on ad1989b?
On AD codecs, all digital outputs are exposed as a single cloned
device with HDA_PCM_TYPE_SPDIF.
Takashi
>
> there is no device 3 but [Jack] SPDIF Out at Ext Rear and [Jack] Digital
> Out at Int HDMI
>
> **** List of PLAYBACK Hardware Devices ****
> card 0: Intel [HDA Intel], device 0: AD198x Analog [AD198x Analog]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
> card 0: Intel [HDA Intel], device 1: AD198x Digital [AD198x Digital]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
> card 0: Intel [HDA Intel], device 2: AD198x Headphone [AD198x Headphone]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
>
> https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/995169/+attachment/3132070/+files/Card0.Codecs.codec.0.txt
>
> Node 0x1b [Pin Complex] wcaps 0x40030d: Stereo Digital Amp-Out
> Control: name="IEC958 Playback Volume", index=0, device=0
> ControlAmp: chs=3, dir=Out, idx=0, ofs=0
> Amp-Out caps: ofs=0x27, nsteps=0x27, stepsize=0x05, mute=1
> Amp-Out vals: [0x00 0x00]
> Pincap 0x00000010: OUT
> Pin Default 0x0145f1a0: [Jack] SPDIF Out at Ext Rear
> Conn = Optical, Color = Other
> DefAssociation = 0xa, Sequence = 0x0
> Misc = NO_PRESENCE
> Pin-ctls: 0x40: OUT
> Connection: 1
> 0x02
>
> Node 0x1d [Pin Complex] wcaps 0x40030d: Stereo Digital Amp-Out
> Control: name="HDMI Playback Volume", index=0, device=0
> ControlAmp: chs=3, dir=Out, idx=0, ofs=0
> Amp-Out caps: ofs=0x27, nsteps=0x27, stepsize=0x05, mute=1
> Amp-Out vals: [0x27 0x27]
> Pincap 0x00000010: OUT
> Pin Default 0x1856f1b0: [Jack] Digital Out at Int HDMI
> Conn = Digital, Color = Other
> DefAssociation = 0xb, Sequence = 0x0
> Misc = NO_PRESENCE
> Pin-ctls: 0x40: OUT
> Connection: 1
> 0x0b
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 7:46 ` Takashi Iwai
@ 2012-10-15 8:15 ` Raymond Yau
2012-10-15 8:35 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Raymond Yau @ 2012-10-15 8:15 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
2012-10-15 下午3:46 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
>
> At Mon, 15 Oct 2012 10:31:50 +0800,
> Raymond Yau wrote:
> >
> > 2012-10-12 下午11:19 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> > >
> > > Hi,
> > >
> > > there is a long-standing problem in HD-audio regarding IEC958
> > > controls. When both an SPDIF and an HDMI are created on the same
> > > card (e.g. one from analog codec and one from graphics chip), the
> > > driver assigns the IEC958 controls just with new indices, 0, 1, 2...
> > >
> > > The problem is that there is no way to connect between this index and
> > > the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> > > configuration:
> > > spdif -> IEC958 xxx index=0, PCM dev=1
> > > hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> > > hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> > > hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> > > hdmi,3 -> IEC958 xxx index=3, PCM dev=9
> > >
> > > So obviously spdif and the first hdmi conflict.
> > >
> > > Basically this can be fixed by reassigning each IEC958 control with
> > > the same device number corresponding to the PCM device. That is, for
> > > SPDIF, assign a control element with device=1, for HDMI,
> > > device=3,7...
> > > However, this obviously breaks the old configuration unless user
> > > upgrades the alsa-lib configuration. Thus this is no-go.
> > >
> > > Now here is a compromise: the IEC958 control for SPDIF is reassigned
> > > to device=1 *only* when SPDIF and HDMI coexist. Since the
> > > configuration is anyway broken as is now in such a case, it's no big
> > > deal to fix one side in an incompatible way. (The reason why SPDIF
> > > is re-assigned is that I guess majority of user require more HDMI than
> > > SPDIF in such a configuration.)
> > >
> > > In addition, we need a fix in alsa-lib. Also not for breaking the
> > > compatibility with older kernel, we need some fallback check. I did a
> > > quick hack to alsa-lib conf code and added "skip_rest" option to the
> > > hook element, so that only the first matching element is taken.
> >
> > how about those hdmi jack and iec958 on ad1989b?
>
> On AD codecs, all digital outputs are exposed as a single cloned
> device with HDA_PCM_TYPE_SPDIF.
how about ad1988b with nvidia codec ?
from user point of view , how can they differenitate hdmi jack is digital
ouput and hdmi output ?
https://bugs.launchpad.net/ubuntu/+source/pulseaudio/+bug/881826
**** List of PLAYBACK Hardware Devices ****
card 0: NVidia [HDA NVidia], device 0: AD198x Analog [AD198x Analog]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: NVidia [HDA NVidia], device 1: AD198x Digital [AD198x Digital]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: NVidia [HDA NVidia], device 3: HDMI 0 [HDMI 0]
Subdevices: 1/1
Subdevice #0: subdevice #0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 8:15 ` Raymond Yau
@ 2012-10-15 8:35 ` Takashi Iwai
2012-10-15 8:49 ` Raymond Yau
0 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2012-10-15 8:35 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel
At Mon, 15 Oct 2012 16:15:20 +0800,
Raymond Yau wrote:
>
> 2012-10-15 下午3:46 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> >
> > At Mon, 15 Oct 2012 10:31:50 +0800,
> > Raymond Yau wrote:
> > >
> > > 2012-10-12 下午11:19 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> > > >
> > > > Hi,
> > > >
> > > > there is a long-standing problem in HD-audio regarding IEC958
> > > > controls. When both an SPDIF and an HDMI are created on the same
> > > > card (e.g. one from analog codec and one from graphics chip), the
> > > > driver assigns the IEC958 controls just with new indices, 0, 1, 2...
> > > >
> > > > The problem is that there is no way to connect between this index and
> > > > the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> > > > configuration:
> > > > spdif -> IEC958 xxx index=0, PCM dev=1
> > > > hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> > > > hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> > > > hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> > > > hdmi,3 -> IEC958 xxx index=3, PCM dev=9
> > > >
> > > > So obviously spdif and the first hdmi conflict.
> > > >
> > > > Basically this can be fixed by reassigning each IEC958 control with
> > > > the same device number corresponding to the PCM device. That is, for
> > > > SPDIF, assign a control element with device=1, for HDMI,
> > > > device=3,7...
> > > > However, this obviously breaks the old configuration unless user
> > > > upgrades the alsa-lib configuration. Thus this is no-go.
> > > >
> > > > Now here is a compromise: the IEC958 control for SPDIF is reassigned
> > > > to device=1 *only* when SPDIF and HDMI coexist. Since the
> > > > configuration is anyway broken as is now in such a case, it's no big
> > > > deal to fix one side in an incompatible way. (The reason why SPDIF
> > > > is re-assigned is that I guess majority of user require more HDMI than
> > > > SPDIF in such a configuration.)
> > > >
> > > > In addition, we need a fix in alsa-lib. Also not for breaking the
> > > > compatibility with older kernel, we need some fallback check. I did a
> > > > quick hack to alsa-lib conf code and added "skip_rest" option to the
> > > > hook element, so that only the first matching element is taken.
> > >
> > > how about those hdmi jack and iec958 on ad1989b?
> >
> > On AD codecs, all digital outputs are exposed as a single cloned
> > device with HDA_PCM_TYPE_SPDIF.
>
> how about ad1988b with nvidia codec ?
Nvidia codec provides devices only with HDA_PCM_TYPE_HDMI, of course.
> from user point of view , how can they differenitate hdmi jack is digital
> ouput and hdmi output ?
The device 1 is from AD and the device 3 is from Nvidia codec, as you
can see below. And this is exactly the case the conflict happens as I
mentioned in the original mail.
Which is connected to what output, you can't know exactly unless you
compare the obtained ELD. The HD-audio spec isn't good enough to
identify the actual output.
Takashi
> https://bugs.launchpad.net/ubuntu/+source/pulseaudio/+bug/881826
>
> **** List of PLAYBACK Hardware Devices ****
> card 0: NVidia [HDA NVidia], device 0: AD198x Analog [AD198x Analog]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
> card 0: NVidia [HDA NVidia], device 1: AD198x Digital [AD198x Digital]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
> card 0: NVidia [HDA NVidia], device 3: HDMI 0 [HDMI 0]
> Subdevices: 1/1
> Subdevice #0: subdevice #0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 8:35 ` Takashi Iwai
@ 2012-10-15 8:49 ` Raymond Yau
2012-10-15 8:55 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Raymond Yau @ 2012-10-15 8:49 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
2012-10-15 下午4:35 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
>
> At Mon, 15 Oct 2012 16:15:20 +0800,
> Raymond Yau wrote:
> >
> > 2012-10-15 下午3:46 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> > >
> > > At Mon, 15 Oct 2012 10:31:50 +0800,
> > > Raymond Yau wrote:
> > > >
> > > > 2012-10-12 下午11:19 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> > > > >
> > > > > Hi,
> > > > >
> > > > > there is a long-standing problem in HD-audio regarding IEC958
> > > > > controls. When both an SPDIF and an HDMI are created on the same
> > > > > card (e.g. one from analog codec and one from graphics chip), the
> > > > > driver assigns the IEC958 controls just with new indices, 0, 1,
2...
> > > > >
> > > > > The problem is that there is no way to connect between this index
and
> > > > > the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a
fixed
> > > > > configuration:
> > > > > spdif -> IEC958 xxx index=0, PCM dev=1
> > > > > hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> > > > > hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> > > > > hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> > > > > hdmi,3 -> IEC958 xxx index=3, PCM dev=9
> > > > >
> > > > > So obviously spdif and the first hdmi conflict.
> > > > >
> > > > > Basically this can be fixed by reassigning each IEC958 control
with
> > > > > the same device number corresponding to the PCM device. That is,
for
> > > > > SPDIF, assign a control element with device=1, for HDMI,
> > > > > device=3,7...
> > > > > However, this obviously breaks the old configuration unless user
> > > > > upgrades the alsa-lib configuration. Thus this is no-go.
> > > > >
> > > > > Now here is a compromise: the IEC958 control for SPDIF is
reassigned
> > > > > to device=1 *only* when SPDIF and HDMI coexist. Since the
> > > > > configuration is anyway broken as is now in such a case, it's no
big
> > > > > deal to fix one side in an incompatible way. (The reason why
SPDIF
> > > > > is re-assigned is that I guess majority of user require more HDMI
than
> > > > > SPDIF in such a configuration.)
> > > > >
> > > > > In addition, we need a fix in alsa-lib. Also not for breaking the
> > > > > compatibility with older kernel, we need some fallback check. I
did a
> > > > > quick hack to alsa-lib conf code and added "skip_rest" option to
the
> > > > > hook element, so that only the first matching element is taken.
> > > >
> > > > how about those hdmi jack and iec958 on ad1989b?
> > >
> > > On AD codecs, all digital outputs are exposed as a single cloned
> > > device with HDA_PCM_TYPE_SPDIF.
> >
> > how about ad1988b with nvidia codec ?
>
> Nvidia codec provides devices only with HDA_PCM_TYPE_HDMI, of course.
>
> > from user point of view , how can they differenitate hdmi jack is
digital
> > ouput and hdmi output ?
>
> The device 1 is from AD and the device 3 is from Nvidia codec, as you
> can see below. And this is exactly the case the conflict happens as I
> mentioned in the original mail.
will your fix change the spdif of those motherboard with ad1988b which
have iec958 but no hdmi jack ?
the current implementation does not expose the number of output streams
supported by the hda controller
ich8 seem support 4 ouput streams , how many output streams supported by
the nvidia controller
>
> Which is connected to what output, you can't know exactly unless you
> compare the obtained ELD. The HD-audio spec isn't good enough to
> identify the actual output.
it seem that there is no presence detect on this codec
Codec: Nvidia MCP77/78 HDMI
Address: 3
AFG Function Id: 0x1 (unsol 0)
Vendor Id: 0x10de0002
Subsystem Id: 0x10de0101
Revision Id: 0x100000
No Modem Function Group found
Default PCM:
rates [0x0]:
bits [0x0]:
formats [0x0]:
Default Amp-In caps: N/A
Default Amp-Out caps: N/A
GPIO: io=0, o=0, i=0, unsolicited=0, wake=0
Node 0x04 [Audio Output] wcaps 0x211: Stereo Digital
Control: name="IEC958 Playback Con Mask", index=1, device=0
Control: name="IEC958 Playback Pro Mask", index=1, device=0
Control: name="IEC958 Playback Default", index=1, device=0
Control: name="IEC958 Playback Switch", index=1, device=0
Device: name="HDMI 0", type="HDMI", device=3
Converter: stream=0, channel=0
Digital:
Digital category: 0x0
PCM:
rates [0xc0]: 48000 88200
bits [0xf]: 8 16 20 24
formats [0x1]: PCM
Node 0x05 [Pin Complex] wcaps 0x400381: Stereo Digital
Pincap 0x00000014: OUT Detect
Pin Default 0x18560110: [Jack] Digital Out at Int HDMI
Conn = Digital, Color = Unknown
DefAssociation = 0x1, Sequence = 0x0
Misc = NO_PRESENCE
Pin-ctls: 0x40: OUT
Unsolicited: tag=00, enabled=0
Connection: 1
0x04
>
>
> Takashi
>
>
> > https://bugs.launchpad.net/ubuntu/+source/pulseaudio/+bug/881826
> >
> > **** List of PLAYBACK Hardware Devices ****
> > card 0: NVidia [HDA NVidia], device 0: AD198x Analog [AD198x Analog]
> > Subdevices: 1/1
> > Subdevice #0: subdevice #0
> > card 0: NVidia [HDA NVidia], device 1: AD198x Digital [AD198x Digital]
> > Subdevices: 1/1
> > Subdevice #0: subdevice #0
> > card 0: NVidia [HDA NVidia], device 3: HDMI 0 [HDMI 0]
> > Subdevices: 1/1
> > Subdevice #0: subdevice #0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 8:49 ` Raymond Yau
@ 2012-10-15 8:55 ` Takashi Iwai
[not found] ` <CAN8ccib4Z9VpHcdGxP3q5kofikU6zt6risGDAbOBiRKyKVcsxA@mail.gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2012-10-15 8:55 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel
At Mon, 15 Oct 2012 16:49:58 +0800,
Raymond Yau wrote:
>
> 2012-10-15 下午4:35 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> >
> > At Mon, 15 Oct 2012 16:15:20 +0800,
> > Raymond Yau wrote:
> > >
> > > 2012-10-15 下午3:46 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> > > >
> > > > At Mon, 15 Oct 2012 10:31:50 +0800,
> > > > Raymond Yau wrote:
> > > > >
> > > > > 2012-10-12 下午11:19 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > there is a long-standing problem in HD-audio regarding IEC958
> > > > > > controls. When both an SPDIF and an HDMI are created on the same
> > > > > > card (e.g. one from analog codec and one from graphics chip), the
> > > > > > driver assigns the IEC958 controls just with new indices, 0, 1,
> 2...
> > > > > >
> > > > > > The problem is that there is no way to connect between this index
> and
> > > > > > the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a
> fixed
> > > > > > configuration:
> > > > > > spdif -> IEC958 xxx index=0, PCM dev=1
> > > > > > hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> > > > > > hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> > > > > > hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> > > > > > hdmi,3 -> IEC958 xxx index=3, PCM dev=9
> > > > > >
> > > > > > So obviously spdif and the first hdmi conflict.
> > > > > >
> > > > > > Basically this can be fixed by reassigning each IEC958 control
> with
> > > > > > the same device number corresponding to the PCM device. That is,
> for
> > > > > > SPDIF, assign a control element with device=1, for HDMI,
> > > > > > device=3,7...
> > > > > > However, this obviously breaks the old configuration unless user
> > > > > > upgrades the alsa-lib configuration. Thus this is no-go.
> > > > > >
> > > > > > Now here is a compromise: the IEC958 control for SPDIF is
> reassigned
> > > > > > to device=1 *only* when SPDIF and HDMI coexist. Since the
> > > > > > configuration is anyway broken as is now in such a case, it's no
> big
> > > > > > deal to fix one side in an incompatible way. (The reason why
> SPDIF
> > > > > > is re-assigned is that I guess majority of user require more HDMI
> than
> > > > > > SPDIF in such a configuration.)
> > > > > >
> > > > > > In addition, we need a fix in alsa-lib. Also not for breaking the
> > > > > > compatibility with older kernel, we need some fallback check. I
> did a
> > > > > > quick hack to alsa-lib conf code and added "skip_rest" option to
> the
> > > > > > hook element, so that only the first matching element is taken.
> > > > >
> > > > > how about those hdmi jack and iec958 on ad1989b?
> > > >
> > > > On AD codecs, all digital outputs are exposed as a single cloned
> > > > device with HDA_PCM_TYPE_SPDIF.
> > >
> > > how about ad1988b with nvidia codec ?
> >
> > Nvidia codec provides devices only with HDA_PCM_TYPE_HDMI, of course.
> >
> > > from user point of view , how can they differenitate hdmi jack is
> digital
> > > ouput and hdmi output ?
> >
> > The device 1 is from AD and the device 3 is from Nvidia codec, as you
> > can see below. And this is exactly the case the conflict happens as I
> > mentioned in the original mail.
>
> will your fix change the spdif of those motherboard with ad1988b which
> have iec958 but no hdmi jack ?
It won't change anything unless both SPDIF and HDMI *devices* are
actually created.
> the current implementation does not expose the number of output streams
> supported by the hda controller
>
> ich8 seem support 4 ouput streams , how many output streams supported by
> the nvidia controller
It's an irrelevant issue. Don't mix up the problems.
> > Which is connected to what output, you can't know exactly unless you
> > compare the obtained ELD. The HD-audio spec isn't good enough to
> > identify the actual output.
>
> it seem that there is no presence detect on this codec
ELD has nothing to do with the presence detect bit.
Takashi
> Codec: Nvidia MCP77/78 HDMI
> Address: 3
> AFG Function Id: 0x1 (unsol 0)
> Vendor Id: 0x10de0002
> Subsystem Id: 0x10de0101
> Revision Id: 0x100000
> No Modem Function Group found
> Default PCM:
> rates [0x0]:
> bits [0x0]:
> formats [0x0]:
> Default Amp-In caps: N/A
> Default Amp-Out caps: N/A
> GPIO: io=0, o=0, i=0, unsolicited=0, wake=0
> Node 0x04 [Audio Output] wcaps 0x211: Stereo Digital
> Control: name="IEC958 Playback Con Mask", index=1, device=0
> Control: name="IEC958 Playback Pro Mask", index=1, device=0
> Control: name="IEC958 Playback Default", index=1, device=0
> Control: name="IEC958 Playback Switch", index=1, device=0
> Device: name="HDMI 0", type="HDMI", device=3
> Converter: stream=0, channel=0
> Digital:
> Digital category: 0x0
> PCM:
> rates [0xc0]: 48000 88200
> bits [0xf]: 8 16 20 24
> formats [0x1]: PCM
> Node 0x05 [Pin Complex] wcaps 0x400381: Stereo Digital
> Pincap 0x00000014: OUT Detect
> Pin Default 0x18560110: [Jack] Digital Out at Int HDMI
> Conn = Digital, Color = Unknown
> DefAssociation = 0x1, Sequence = 0x0
> Misc = NO_PRESENCE
> Pin-ctls: 0x40: OUT
> Unsolicited: tag=00, enabled=0
> Connection: 1
> 0x04
>
> >
> >
> > Takashi
> >
> >
> > > https://bugs.launchpad.net/ubuntu/+source/pulseaudio/+bug/881826
> > >
> > > **** List of PLAYBACK Hardware Devices ****
> > > card 0: NVidia [HDA NVidia], device 0: AD198x Analog [AD198x Analog]
> > > Subdevices: 1/1
> > > Subdevice #0: subdevice #0
> > > card 0: NVidia [HDA NVidia], device 1: AD198x Digital [AD198x Digital]
> > > Subdevices: 1/1
> > > Subdevice #0: subdevice #0
> > > card 0: NVidia [HDA NVidia], device 3: HDMI 0 [HDMI 0]
> > > Subdevices: 1/1
> > > Subdevice #0: subdevice #0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-12 15:18 [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Takashi Iwai
` (3 preceding siblings ...)
2012-10-15 2:31 ` [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Raymond Yau
@ 2012-10-15 10:09 ` David Henningsson
2012-10-15 10:18 ` Takashi Iwai
2012-10-15 10:21 ` Jaroslav Kysela
5 siblings, 1 reply; 32+ messages in thread
From: David Henningsson @ 2012-10-15 10:09 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On 10/12/2012 05:18 PM, Takashi Iwai wrote:
> Hi,
>
> there is a long-standing problem in HD-audio regarding IEC958
> controls. When both an SPDIF and an HDMI are created on the same
> card (e.g. one from analog codec and one from graphics chip), the
> driver assigns the IEC958 controls just with new indices, 0, 1, 2...
>
> The problem is that there is no way to connect between this index and
> the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> configuration:
> spdif -> IEC958 xxx index=0, PCM dev=1
> hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> hdmi,3 -> IEC958 xxx index=3, PCM dev=9
>
> So obviously spdif and the first hdmi conflict.
If these mixer controls need to be set to specific value for
playback/capture to work, why isn't this logic handled entirely in
kernel space in the first place?
I e, instead of relying on alsa-lib to setup "IEC958 Playback Default"
correctly for us, we always do what setting "IEC958 Playback Default"
would have done, but in the kernel directly instead.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 10:09 ` David Henningsson
@ 2012-10-15 10:18 ` Takashi Iwai
2012-10-15 10:58 ` David Henningsson
0 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2012-10-15 10:18 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
At Mon, 15 Oct 2012 12:09:40 +0200,
David Henningsson wrote:
>
> On 10/12/2012 05:18 PM, Takashi Iwai wrote:
> > Hi,
> >
> > there is a long-standing problem in HD-audio regarding IEC958
> > controls. When both an SPDIF and an HDMI are created on the same
> > card (e.g. one from analog codec and one from graphics chip), the
> > driver assigns the IEC958 controls just with new indices, 0, 1, 2...
> >
> > The problem is that there is no way to connect between this index and
> > the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> > configuration:
> > spdif -> IEC958 xxx index=0, PCM dev=1
> > hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> > hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> > hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> > hdmi,3 -> IEC958 xxx index=3, PCM dev=9
> >
> > So obviously spdif and the first hdmi conflict.
>
> If these mixer controls need to be set to specific value for
> playback/capture to work, why isn't this logic handled entirely in
> kernel space in the first place?
>
> I e, instead of relying on alsa-lib to setup "IEC958 Playback Default"
> correctly for us, we always do what setting "IEC958 Playback Default"
> would have done, but in the kernel directly instead.
Hm, I don't get your point. Could you elaborate?
Takashi
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-12 15:18 [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Takashi Iwai
` (4 preceding siblings ...)
2012-10-15 10:09 ` David Henningsson
@ 2012-10-15 10:21 ` Jaroslav Kysela
2012-10-15 10:31 ` Takashi Iwai
5 siblings, 1 reply; 32+ messages in thread
From: Jaroslav Kysela @ 2012-10-15 10:21 UTC (permalink / raw)
To: alsa-devel; +Cc: Takashi Iwai
Date 12.10.2012 17:18, Takashi Iwai wrote:
> Hi,
>
> there is a long-standing problem in HD-audio regarding IEC958
> controls. When both an SPDIF and an HDMI are created on the same
> card (e.g. one from analog codec and one from graphics chip), the
> driver assigns the IEC958 controls just with new indices, 0, 1, 2...
>
> The problem is that there is no way to connect between this index and
> the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> configuration:
> spdif -> IEC958 xxx index=0, PCM dev=1
> hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> hdmi,3 -> IEC958 xxx index=3, PCM dev=9
>
> So obviously spdif and the first hdmi conflict.
>
> Basically this can be fixed by reassigning each IEC958 control with
> the same device number corresponding to the PCM device. That is, for
> SPDIF, assign a control element with device=1, for HDMI,
> device=3,7...
> However, this obviously breaks the old configuration unless user
> upgrades the alsa-lib configuration. Thus this is no-go.
>
> Now here is a compromise: the IEC958 control for SPDIF is reassigned
> to device=1 *only* when SPDIF and HDMI coexist. Since the
> configuration is anyway broken as is now in such a case, it's no big
> deal to fix one side in an incompatible way. (The reason why SPDIF
> is re-assigned is that I guess majority of user require more HDMI than
> SPDIF in such a configuration.)
>
> In addition, we need a fix in alsa-lib. Also not for breaking the
> compatibility with older kernel, we need some fallback check. I did a
> quick hack to alsa-lib conf code and added "skip_rest" option to the
> hook element, so that only the first matching element is taken.
>
> Long story short, I cooked up three patches. One patch is for kernel,
> to add the workaround above, and the two are for alsa-lib, one clean
> up and one to introduce the skip_rest option (and application to
> HDA-Intel.conf).
>
> My plan is to merge this to 3.8 tree, so it's no urgent issue.
> But it's of course always good to fix something.
What about to introduce more cleaner solution - export this information
using TLV to user space, so alsa-lib can enumerate (distinguish) the
spdif and hdmi devices properly.
Jaroslav
--
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 10:21 ` Jaroslav Kysela
@ 2012-10-15 10:31 ` Takashi Iwai
2012-10-15 11:11 ` Jaroslav Kysela
0 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2012-10-15 10:31 UTC (permalink / raw)
To: Jaroslav Kysela; +Cc: alsa-devel
At Mon, 15 Oct 2012 12:21:18 +0200,
Jaroslav Kysela wrote:
>
> Date 12.10.2012 17:18, Takashi Iwai wrote:
> > Hi,
> >
> > there is a long-standing problem in HD-audio regarding IEC958
> > controls. When both an SPDIF and an HDMI are created on the same
> > card (e.g. one from analog codec and one from graphics chip), the
> > driver assigns the IEC958 controls just with new indices, 0, 1, 2...
> >
> > The problem is that there is no way to connect between this index and
> > the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> > configuration:
> > spdif -> IEC958 xxx index=0, PCM dev=1
> > hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> > hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> > hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> > hdmi,3 -> IEC958 xxx index=3, PCM dev=9
> >
> > So obviously spdif and the first hdmi conflict.
> >
> > Basically this can be fixed by reassigning each IEC958 control with
> > the same device number corresponding to the PCM device. That is, for
> > SPDIF, assign a control element with device=1, for HDMI,
> > device=3,7...
> > However, this obviously breaks the old configuration unless user
> > upgrades the alsa-lib configuration. Thus this is no-go.
> >
> > Now here is a compromise: the IEC958 control for SPDIF is reassigned
> > to device=1 *only* when SPDIF and HDMI coexist. Since the
> > configuration is anyway broken as is now in such a case, it's no big
> > deal to fix one side in an incompatible way. (The reason why SPDIF
> > is re-assigned is that I guess majority of user require more HDMI than
> > SPDIF in such a configuration.)
> >
> > In addition, we need a fix in alsa-lib. Also not for breaking the
> > compatibility with older kernel, we need some fallback check. I did a
> > quick hack to alsa-lib conf code and added "skip_rest" option to the
> > hook element, so that only the first matching element is taken.
> >
> > Long story short, I cooked up three patches. One patch is for kernel,
> > to add the workaround above, and the two are for alsa-lib, one clean
> > up and one to introduce the skip_rest option (and application to
> > HDA-Intel.conf).
> >
> > My plan is to merge this to 3.8 tree, so it's no urgent issue.
> > But it's of course always good to fix something.
>
> What about to introduce more cleaner solution - export this information
> using TLV to user space, so alsa-lib can enumerate (distinguish) the
> spdif and hdmi devices properly.
Yes, this is the next step I've planned. The additional information
doesn't conflict with this fix.
The point of this fix is to keep / fix the things working as much as
possible even without changing user-space. If a user-space upgrade is
mandatory, it can be regarded as an obvious regression.
In short, my patch is:
- A quick fix
- Doesn't break the cases without SPDIF/HDMI conflict
- Fixes HDMI for the conflicting case even without changing user-space
- Would break the SPDIF output (only when SPDIF/HDMI conflicts)
without config update
Takashi
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 10:18 ` Takashi Iwai
@ 2012-10-15 10:58 ` David Henningsson
0 siblings, 0 replies; 32+ messages in thread
From: David Henningsson @ 2012-10-15 10:58 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On 10/15/2012 12:18 PM, Takashi Iwai wrote:
> At Mon, 15 Oct 2012 12:09:40 +0200,
> David Henningsson wrote:
>>
>> On 10/12/2012 05:18 PM, Takashi Iwai wrote:
>>> Hi,
>>>
>>> there is a long-standing problem in HD-audio regarding IEC958
>>> controls. When both an SPDIF and an HDMI are created on the same
>>> card (e.g. one from analog codec and one from graphics chip), the
>>> driver assigns the IEC958 controls just with new indices, 0, 1, 2...
>>>
>>> The problem is that there is no way to connect between this index and
>>> the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
>>> configuration:
>>> spdif -> IEC958 xxx index=0, PCM dev=1
>>> hdmi,0 -> IEC958 xxx index=0, PCM dev=3
>>> hdmi,1 -> IEC958 xxx index=1, PCM dev=7
>>> hdmi,2 -> IEC958 xxx index=2, PCM dev=8
>>> hdmi,3 -> IEC958 xxx index=3, PCM dev=9
>>>
>>> So obviously spdif and the first hdmi conflict.
>>
>> If these mixer controls need to be set to specific value for
>> playback/capture to work, why isn't this logic handled entirely in
>> kernel space in the first place?
>>
>> I e, instead of relying on alsa-lib to setup "IEC958 Playback Default"
>> correctly for us, we always do what setting "IEC958 Playback Default"
>> would have done, but in the kernel directly instead.
>
> Hm, I don't get your point. Could you elaborate?
Never mind. I missed that the mixer control is needed to transfer IEC958
status bit information.
I guess it would be cleaner to somehow associate this information
transfer to the pcm device rather than the control device, but maybe
that suggestion is 10 years late or so :-)
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 10:31 ` Takashi Iwai
@ 2012-10-15 11:11 ` Jaroslav Kysela
2012-10-15 12:06 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Jaroslav Kysela @ 2012-10-15 11:11 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
Date 15.10.2012 12:31, Takashi Iwai wrote:
> At Mon, 15 Oct 2012 12:21:18 +0200,
> Jaroslav Kysela wrote:
>>
>> Date 12.10.2012 17:18, Takashi Iwai wrote:
>>> Hi,
>>>
>>> there is a long-standing problem in HD-audio regarding IEC958
>>> controls. When both an SPDIF and an HDMI are created on the same
>>> card (e.g. one from analog codec and one from graphics chip), the
>>> driver assigns the IEC958 controls just with new indices, 0, 1, 2...
>>>
>>> The problem is that there is no way to connect between this index and
>>> the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
>>> configuration:
>>> spdif -> IEC958 xxx index=0, PCM dev=1
>>> hdmi,0 -> IEC958 xxx index=0, PCM dev=3
>>> hdmi,1 -> IEC958 xxx index=1, PCM dev=7
>>> hdmi,2 -> IEC958 xxx index=2, PCM dev=8
>>> hdmi,3 -> IEC958 xxx index=3, PCM dev=9
>>>
>>> So obviously spdif and the first hdmi conflict.
>>>
>>> Basically this can be fixed by reassigning each IEC958 control with
>>> the same device number corresponding to the PCM device. That is, for
>>> SPDIF, assign a control element with device=1, for HDMI,
>>> device=3,7...
>>> However, this obviously breaks the old configuration unless user
>>> upgrades the alsa-lib configuration. Thus this is no-go.
>>>
>>> Now here is a compromise: the IEC958 control for SPDIF is reassigned
>>> to device=1 *only* when SPDIF and HDMI coexist. Since the
>>> configuration is anyway broken as is now in such a case, it's no big
>>> deal to fix one side in an incompatible way. (The reason why SPDIF
>>> is re-assigned is that I guess majority of user require more HDMI than
>>> SPDIF in such a configuration.)
>>>
>>> In addition, we need a fix in alsa-lib. Also not for breaking the
>>> compatibility with older kernel, we need some fallback check. I did a
>>> quick hack to alsa-lib conf code and added "skip_rest" option to the
>>> hook element, so that only the first matching element is taken.
>>>
>>> Long story short, I cooked up three patches. One patch is for kernel,
>>> to add the workaround above, and the two are for alsa-lib, one clean
>>> up and one to introduce the skip_rest option (and application to
>>> HDA-Intel.conf).
>>>
>>> My plan is to merge this to 3.8 tree, so it's no urgent issue.
>>> But it's of course always good to fix something.
>>
>> What about to introduce more cleaner solution - export this information
>> using TLV to user space, so alsa-lib can enumerate (distinguish) the
>> spdif and hdmi devices properly.
>
> Yes, this is the next step I've planned. The additional information
> doesn't conflict with this fix.
>
> The point of this fix is to keep / fix the things working as much as
> possible even without changing user-space. If a user-space upgrade is
> mandatory, it can be regarded as an obvious regression.
>
> In short, my patch is:
> - A quick fix
> - Doesn't break the cases without SPDIF/HDMI conflict
> - Fixes HDMI for the conflicting case even without changing user-space
> - Would break the SPDIF output (only when SPDIF/HDMI conflicts)
> without config update
I understand, but because the config update mostly depends on the whole
alsa-lib package update, then I believe, the proper fix may be
implemented now rather than introducing a config hack option. But the
step-by-step improvement is ok to me, too.
Jaroslav
--
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 11:11 ` Jaroslav Kysela
@ 2012-10-15 12:06 ` Takashi Iwai
0 siblings, 0 replies; 32+ messages in thread
From: Takashi Iwai @ 2012-10-15 12:06 UTC (permalink / raw)
To: Jaroslav Kysela; +Cc: alsa-devel
At Mon, 15 Oct 2012 13:11:00 +0200,
Jaroslav Kysela wrote:
>
> Date 15.10.2012 12:31, Takashi Iwai wrote:
> > At Mon, 15 Oct 2012 12:21:18 +0200,
> > Jaroslav Kysela wrote:
> >>
> >> Date 12.10.2012 17:18, Takashi Iwai wrote:
> >>> Hi,
> >>>
> >>> there is a long-standing problem in HD-audio regarding IEC958
> >>> controls. When both an SPDIF and an HDMI are created on the same
> >>> card (e.g. one from analog codec and one from graphics chip), the
> >>> driver assigns the IEC958 controls just with new indices, 0, 1, 2...
> >>>
> >>> The problem is that there is no way to connect between this index and
> >>> the actual PCM device. Currently, alsa-lib HDA-Intel.conf has a fixed
> >>> configuration:
> >>> spdif -> IEC958 xxx index=0, PCM dev=1
> >>> hdmi,0 -> IEC958 xxx index=0, PCM dev=3
> >>> hdmi,1 -> IEC958 xxx index=1, PCM dev=7
> >>> hdmi,2 -> IEC958 xxx index=2, PCM dev=8
> >>> hdmi,3 -> IEC958 xxx index=3, PCM dev=9
> >>>
> >>> So obviously spdif and the first hdmi conflict.
> >>>
> >>> Basically this can be fixed by reassigning each IEC958 control with
> >>> the same device number corresponding to the PCM device. That is, for
> >>> SPDIF, assign a control element with device=1, for HDMI,
> >>> device=3,7...
> >>> However, this obviously breaks the old configuration unless user
> >>> upgrades the alsa-lib configuration. Thus this is no-go.
> >>>
> >>> Now here is a compromise: the IEC958 control for SPDIF is reassigned
> >>> to device=1 *only* when SPDIF and HDMI coexist. Since the
> >>> configuration is anyway broken as is now in such a case, it's no big
> >>> deal to fix one side in an incompatible way. (The reason why SPDIF
> >>> is re-assigned is that I guess majority of user require more HDMI than
> >>> SPDIF in such a configuration.)
> >>>
> >>> In addition, we need a fix in alsa-lib. Also not for breaking the
> >>> compatibility with older kernel, we need some fallback check. I did a
> >>> quick hack to alsa-lib conf code and added "skip_rest" option to the
> >>> hook element, so that only the first matching element is taken.
> >>>
> >>> Long story short, I cooked up three patches. One patch is for kernel,
> >>> to add the workaround above, and the two are for alsa-lib, one clean
> >>> up and one to introduce the skip_rest option (and application to
> >>> HDA-Intel.conf).
> >>>
> >>> My plan is to merge this to 3.8 tree, so it's no urgent issue.
> >>> But it's of course always good to fix something.
> >>
> >> What about to introduce more cleaner solution - export this information
> >> using TLV to user space, so alsa-lib can enumerate (distinguish) the
> >> spdif and hdmi devices properly.
> >
> > Yes, this is the next step I've planned. The additional information
> > doesn't conflict with this fix.
> >
> > The point of this fix is to keep / fix the things working as much as
> > possible even without changing user-space. If a user-space upgrade is
> > mandatory, it can be regarded as an obvious regression.
> >
> > In short, my patch is:
> > - A quick fix
> > - Doesn't break the cases without SPDIF/HDMI conflict
> > - Fixes HDMI for the conflicting case even without changing user-space
> > - Would break the SPDIF output (only when SPDIF/HDMI conflicts)
> > without config update
>
> I understand, but because the config update mostly depends on the whole
> alsa-lib package update, then I believe, the proper fix may be
> implemented now rather than introducing a config hack option. But the
> step-by-step improvement is ok to me, too.
Yep, a "proper" fix would be to improve the alsa-lib config.
I wouldn't take such a too hackish option, too, if no big rewrite is
needed...
Takashi
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
[not found] ` <CAN8cciY0TfZ+50EuoYbFdz1AzgNuDKZAaDE-o0v-YD_MpXnO1g@mail.gmail.com>
@ 2012-10-15 13:10 ` Raymond Yau
2012-10-15 13:20 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Raymond Yau @ 2012-10-15 13:10 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, 881826
> > > > > >
> > > > > > how about those hdmi jack and iec958 on ad1989b?
> > > > >
> > > > > On AD codecs, all digital outputs are exposed as a single cloned
> > > > > device with HDA_PCM_TYPE_SPDIF.
> > > >
> > > > how about ad1988b with nvidia codec ?
> > >
> > > Nvidia codec provides devices only with HDA_PCM_TYPE_HDMI, of course.
> > >
> > > > from user point of view , how can they differenitate hdmi jack is
> > digital
> > > > ouput and hdmi output ?
> > >
> > > The device 1 is from AD and the device 3 is from Nvidia codec, as you
> > > can see below. And this is exactly the case the conflict happens as I
> > > mentioned in the original mail.
> >
> > will your fix change the spdif of those motherboard with ad1988b which
> > have iec958 but no hdmi jack ?
>
> It won't change anything unless both SPDIF and HDMI *devices* are
> actually created.
>
>
>
> > > Which is connected to what output, you can't know exactly unless you
> > > compare the obtained ELD. The HD-audio spec isn't good enough to
> > > identify the actual output.
> >
> > it seem that there is no presence detect on this codec
>
> ELD has nothing to do with the presence detect bit.
>
ftp://download.nvidia.com/XFree86/gpu-hdmi-audio-document/gpu-hdmi-audio.html#_examples
6.1.2. Chipset with 8-channel support, single stream
This configuration is used in MCP77, MCP78, MCP79, MCP7A, and ION.
In the case where multiple HDMI display connectors are present, the audio
stream is broadcast to all HDMI connectors at once. A single ALSA device is
exposed.
ELD and PD information is not available on these chipsets.
In these chipsets, the multiple 2-channel converters are aggregated by the
ALSA driver and exposed as a single 8-channel device. Some chipsets support
2, or 8 channels (MCP77). Other chipsets support 2, 6, or 8 channels
(MCP79).
id = 0x10de0002, .name = "MCP77/78 HDMI", .patch = patch_nvhdmi_8ch_7x },
does it mean that this hdmi codec support 8 channels but no ELD info ?
>
> > Codec: Nvidia MCP77/78 HDMI
> > Address: 3
> > AFG Function Id: 0x1 (unsol 0)
> > Vendor Id: 0x10de0002
> > Subsystem Id: 0x10de0101
> > Revision Id: 0x100000
> > No Modem Function Group found
> > Default PCM:
> > rates [0x0]:
> > bits [0x0]:
> > formats [0x0]:
> > Default Amp-In caps: N/A
> > Default Amp-Out caps: N/A
> > GPIO: io=0, o=0, i=0, unsolicited=0, wake=0
> > Node 0x04 [Audio Output] wcaps 0x211: Stereo Digital
> > Control: name="IEC958 Playback Con Mask", index=1, device=0
> > Control: name="IEC958 Playback Pro Mask", index=1, device=0
> > Control: name="IEC958 Playback Default", index=1, device=0
> > Control: name="IEC958 Playback Switch", index=1, device=0
> > Device: name="HDMI 0", type="HDMI", device=3
> > Converter: stream=0, channel=0
> > Digital:
> > Digital category: 0x0
> > PCM:
> > rates [0xc0]: 48000 88200
> > bits [0xf]: 8 16 20 24
> > formats [0x1]: PCM
> > Node 0x05 [Pin Complex] wcaps 0x400381: Stereo Digital
> > Pincap 0x00000014: OUT Detect
> > Pin Default 0x18560110: [Jack] Digital Out at Int HDMI
> > Conn = Digital, Color = Unknown
> > DefAssociation = 0x1, Sequence = 0x0
> > Misc = NO_PRESENCE
> > Pin-ctls: 0x40: OUT
> > Unsolicited: tag=00, enabled=0
> > Connection: 1
> > 0x04
> >
> > >
> > >
> > > > https://bugs.launchpad.net/ubuntu/+source/pulseaudio/+bug/881826
> > > >
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
2012-10-15 13:10 ` Raymond Yau
@ 2012-10-15 13:20 ` Takashi Iwai
0 siblings, 0 replies; 32+ messages in thread
From: Takashi Iwai @ 2012-10-15 13:20 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel, 881826
At Mon, 15 Oct 2012 21:10:44 +0800,
Raymond Yau wrote:
>
> > > > > > >
> > > > > > > how about those hdmi jack and iec958 on ad1989b?
> > > > > >
> > > > > > On AD codecs, all digital outputs are exposed as a single cloned
> > > > > > device with HDA_PCM_TYPE_SPDIF.
> > > > >
> > > > > how about ad1988b with nvidia codec ?
> > > >
> > > > Nvidia codec provides devices only with HDA_PCM_TYPE_HDMI, of course.
> > > >
> > > > > from user point of view , how can they differenitate hdmi jack is
> > > digital
> > > > > ouput and hdmi output ?
> > > >
> > > > The device 1 is from AD and the device 3 is from Nvidia codec, as you
> > > > can see below. And this is exactly the case the conflict happens as I
> > > > mentioned in the original mail.
> > >
> > > will your fix change the spdif of those motherboard with ad1988b which
> > > have iec958 but no hdmi jack ?
> >
> > It won't change anything unless both SPDIF and HDMI *devices* are
> > actually created.
> >
> >
> >
> > > > Which is connected to what output, you can't know exactly unless you
> > > > compare the obtained ELD. The HD-audio spec isn't good enough to
> > > > identify the actual output.
> > >
> > > it seem that there is no presence detect on this codec
> >
> > ELD has nothing to do with the presence detect bit.
> >
>
> ftp://download.nvidia.com/XFree86/gpu-hdmi-audio-document/gpu-hdmi-audio.html#_examples
>
> 6.1.2. Chipset with 8-channel support, single stream
>
> This configuration is used in MCP77, MCP78, MCP79, MCP7A, and ION.
>
> In the case where multiple HDMI display connectors are present, the audio
> stream is broadcast to all HDMI connectors at once. A single ALSA device is
> exposed.
>
> ELD and PD information is not available on these chipsets.
>
> In these chipsets, the multiple 2-channel converters are aggregated by the
> ALSA driver and exposed as a single 8-channel device. Some chipsets support
> 2, or 8 channels (MCP77). Other chipsets support 2, 6, or 8 channels
> (MCP79).
>
> id = 0x10de0002, .name = "MCP77/78 HDMI", .patch = patch_nvhdmi_8ch_7x },
>
> does it mean that this hdmi codec support 8 channels but no ELD info ?
Right.
Takashi
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH RFC] ALSA: hda - Add workaround for conflicting IEC958 controls
2012-10-12 15:24 ` [PATCH RFC] ALSA: hda - Add workaround for conflicting " Takashi Iwai
@ 2012-10-17 8:17 ` Takashi Iwai
2012-10-17 12:43 ` Raymond Yau
2013-02-08 22:44 ` [PATCH] ALSA: hda - Fix the " Anssi Hannula
1 sibling, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2012-10-17 8:17 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
At Fri, 12 Oct 2012 17:24:51 +0200,
Takashi Iwai wrote:
>
> When both an SPDIF and an HDMI device are created on the same card
> instance, multiple IEC958 controls are created with indices=0, 1, ...
> But the alsa-lib configuration can't know which index corresponds
> actually to which PCM device, and both the SPDIF and the HDMI
> configurations point to the first IEC958 control wrongly.
>
> This patch introduces a (hackish and ugly) workaround: the IEC958
> controls for the SPDIF device are re-labeled with device=1 when HDMI
> coexists. The device=1 corresponds to the actual PCM device for
> SPDIF, so it's anyway a better representation. In future, HDMI
> controls should be moved with the corresponding PCM device number,
> too.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Since there was no big objection to this move, I queued the kernel
patch to for-next branch now.
About the alsa-lib hack, it's still in consideration. But this kernel
part doesn't conflict with other possible fix, so let it be in.
thanks,
Takashi
> ---
> sound/pci/hda/hda_codec.c | 60 +++++++++++++++++++++++++++++-------------
> sound/pci/hda/hda_codec.h | 1 +
> sound/pci/hda/hda_local.h | 8 +++---
> sound/pci/hda/patch_cirrus.c | 5 ++--
> sound/pci/hda/patch_hdmi.c | 7 ++---
> sound/pci/hda/patch_realtek.c | 7 ++---
> sound/pci/hda/patch_sigmatel.c | 7 ++---
> 7 files changed, 63 insertions(+), 32 deletions(-)
>
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index 70d4848..2878bab 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -2151,12 +2151,12 @@ EXPORT_SYMBOL_HDA(snd_hda_set_vmaster_tlv);
>
> /* find a mixer control element with the given name */
> static struct snd_kcontrol *
> -_snd_hda_find_mixer_ctl(struct hda_codec *codec,
> - const char *name, int idx)
> +find_mixer_ctl(struct hda_codec *codec, const char *name, int dev, int idx)
> {
> struct snd_ctl_elem_id id;
> memset(&id, 0, sizeof(id));
> id.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> + id.device = dev;
> id.index = idx;
> if (snd_BUG_ON(strlen(name) >= sizeof(id.name)))
> return NULL;
> @@ -2174,15 +2174,16 @@ _snd_hda_find_mixer_ctl(struct hda_codec *codec,
> struct snd_kcontrol *snd_hda_find_mixer_ctl(struct hda_codec *codec,
> const char *name)
> {
> - return _snd_hda_find_mixer_ctl(codec, name, 0);
> + return find_mixer_ctl(codec, name, 0, 0);
> }
> EXPORT_SYMBOL_HDA(snd_hda_find_mixer_ctl);
>
> -static int find_empty_mixer_ctl_idx(struct hda_codec *codec, const char *name)
> +static int find_empty_mixer_ctl_idx(struct hda_codec *codec, const char *name,
> + int dev)
> {
> int idx;
> for (idx = 0; idx < 16; idx++) { /* 16 ctlrs should be large enough */
> - if (!_snd_hda_find_mixer_ctl(codec, name, idx))
> + if (!find_mixer_ctl(codec, name, dev, idx))
> return idx;
> }
> return -EBUSY;
> @@ -3133,26 +3134,48 @@ static struct snd_kcontrol_new dig_mixes[] = {
> };
>
> /**
> - * snd_hda_create_spdif_out_ctls - create Output SPDIF-related controls
> + * snd_hda_create_dig_out_ctls - create Output SPDIF-related controls
> * @codec: the HDA codec
> - * @nid: audio out widget NID
> - *
> - * Creates controls related with the SPDIF output.
> - * Called from each patch supporting the SPDIF out.
> + * @associated_nid: NID that new ctls associated with
> + * @cvt_nid: converter NID
> + * @type: HDA_PCM_TYPE_*
> + * Creates controls related with the digital output.
> + * Called from each patch supporting the digital out.
> *
> * Returns 0 if successful, or a negative error code.
> */
> -int snd_hda_create_spdif_out_ctls(struct hda_codec *codec,
> - hda_nid_t associated_nid,
> - hda_nid_t cvt_nid)
> +int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
> + hda_nid_t associated_nid,
> + hda_nid_t cvt_nid,
> + int type)
> {
> int err;
> struct snd_kcontrol *kctl;
> struct snd_kcontrol_new *dig_mix;
> - int idx;
> + int idx, dev = 0;
> + const int spdif_pcm_dev = 1;
> struct hda_spdif_out *spdif;
>
> - idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch");
> + if (codec->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
> + type == HDA_PCM_TYPE_SPDIF) {
> + dev = spdif_pcm_dev;
> + } else if (codec->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
> + type == HDA_PCM_TYPE_HDMI) {
> + for (idx = 0; idx < codec->spdif_out.used; idx++) {
> + spdif = snd_array_elem(&codec->spdif_out, idx);
> + for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
> + kctl = find_mixer_ctl(codec, dig_mix->name, 0, idx);
> + if (!kctl)
> + break;
> + kctl->id.device = spdif_pcm_dev;
> + }
> + }
> + codec->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
> + }
> + if (!codec->primary_dig_out_type)
> + codec->primary_dig_out_type = type;
> +
> + idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch", dev);
> if (idx < 0) {
> printk(KERN_ERR "hda_codec: too many IEC958 outputs\n");
> return -EBUSY;
> @@ -3162,6 +3185,7 @@ int snd_hda_create_spdif_out_ctls(struct hda_codec *codec,
> kctl = snd_ctl_new1(dig_mix, codec);
> if (!kctl)
> return -ENOMEM;
> + kctl->id.device = dev;
> kctl->id.index = idx;
> kctl->private_value = codec->spdif_out.used - 1;
> err = snd_hda_ctl_add(codec, associated_nid, kctl);
> @@ -3174,7 +3198,7 @@ int snd_hda_create_spdif_out_ctls(struct hda_codec *codec,
> spdif->status = convert_to_spdif_status(spdif->ctls);
> return 0;
> }
> -EXPORT_SYMBOL_HDA(snd_hda_create_spdif_out_ctls);
> +EXPORT_SYMBOL_HDA(snd_hda_create_dig_out_ctls);
>
> /* get the hda_spdif_out entry from the given NID
> * call within spdif_mutex lock
> @@ -3349,7 +3373,7 @@ int snd_hda_create_spdif_in_ctls(struct hda_codec *codec, hda_nid_t nid)
> struct snd_kcontrol_new *dig_mix;
> int idx;
>
> - idx = find_empty_mixer_ctl_idx(codec, "IEC958 Capture Switch");
> + idx = find_empty_mixer_ctl_idx(codec, "IEC958 Capture Switch", 0);
> if (idx < 0) {
> printk(KERN_ERR "hda_codec: too many IEC958 inputs\n");
> return -EBUSY;
> @@ -4449,7 +4473,7 @@ int snd_hda_add_new_ctls(struct hda_codec *codec,
> addr = codec->addr;
> else if (!idx && !knew->index) {
> idx = find_empty_mixer_ctl_idx(codec,
> - knew->name);
> + knew->name, 0);
> if (idx <= 0)
> return err;
> } else
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 507fe8a..e972c23 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -836,6 +836,7 @@ struct hda_codec {
> struct mutex hash_mutex;
> struct snd_array spdif_out;
> unsigned int spdif_in_enable; /* SPDIF input enable? */
> + int primary_dig_out_type; /* primary digital out PCM type */
> const hda_nid_t *slave_dig_outs; /* optional digital out slave widgets */
> struct snd_array init_pins; /* initial (BIOS) pin configurations */
> struct snd_array driver_pins; /* pin configs set by codec parser */
> diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> index 09dbdc3..8c43198 100644
> --- a/sound/pci/hda/hda_local.h
> +++ b/sound/pci/hda/hda_local.h
> @@ -240,9 +240,11 @@ int snd_hda_mixer_bind_tlv(struct snd_kcontrol *kcontrol, int op_flag,
> /*
> * SPDIF I/O
> */
> -int snd_hda_create_spdif_out_ctls(struct hda_codec *codec,
> - hda_nid_t associated_nid,
> - hda_nid_t cvt_nid);
> +int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
> + hda_nid_t associated_nid,
> + hda_nid_t cvt_nid, int type);
> +#define snd_hda_create_spdif_out_ctls(codec, anid, cnid) \
> + snd_hda_create_dig_out_ctls(codec, anid, cnid, HDA_PCM_TYPE_SPDIF)
> int snd_hda_create_spdif_in_ctls(struct hda_codec *codec, hda_nid_t nid);
>
> /*
> diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
> index 61a7113..a7f8790 100644
> --- a/sound/pci/hda/patch_cirrus.c
> +++ b/sound/pci/hda/patch_cirrus.c
> @@ -873,8 +873,9 @@ static int build_digital_output(struct hda_codec *codec)
> if (!spec->multiout.dig_out_nid)
> return 0;
>
> - err = snd_hda_create_spdif_out_ctls(codec, spec->multiout.dig_out_nid,
> - spec->multiout.dig_out_nid);
> + err = snd_hda_create_dig_out_ctls(codec, spec->multiout.dig_out_nid,
> + spec->multiout.dig_out_nid,
> + spec->pcm_rec[1].pcm_type);
> if (err < 0)
> return err;
> err = snd_hda_create_spdif_share_sw(codec, &spec->multiout);
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 71555cc..39ca100 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1589,9 +1589,10 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
> if (err < 0)
> return err;
>
> - err = snd_hda_create_spdif_out_ctls(codec,
> - per_pin->pin_nid,
> - per_pin->mux_nids[0]);
> + err = snd_hda_create_dig_out_ctls(codec,
> + per_pin->pin_nid,
> + per_pin->mux_nids[0],
> + HDA_PCM_TYPE_HDMI);
> if (err < 0)
> return err;
> snd_hda_spdif_ctls_unassign(codec, pin_idx);
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 8253b4e..2d2bb66 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -1836,9 +1836,10 @@ static int __alc_build_controls(struct hda_codec *codec)
> return err;
> }
> if (spec->multiout.dig_out_nid) {
> - err = snd_hda_create_spdif_out_ctls(codec,
> - spec->multiout.dig_out_nid,
> - spec->multiout.dig_out_nid);
> + err = snd_hda_create_dig_out_ctls(codec,
> + spec->multiout.dig_out_nid,
> + spec->multiout.dig_out_nid,
> + spec->pcm_rec[1].pcm_type);
> if (err < 0)
> return err;
> if (!spec->no_analog) {
> diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
> index 770013f..6214165 100644
> --- a/sound/pci/hda/patch_sigmatel.c
> +++ b/sound/pci/hda/patch_sigmatel.c
> @@ -1136,9 +1136,10 @@ static int stac92xx_build_controls(struct hda_codec *codec)
> }
>
> if (spec->multiout.dig_out_nid) {
> - err = snd_hda_create_spdif_out_ctls(codec,
> - spec->multiout.dig_out_nid,
> - spec->multiout.dig_out_nid);
> + err = snd_hda_create_dig_out_ctls(codec,
> + spec->multiout.dig_out_nid,
> + spec->multiout.dig_out_nid,
> + spec->autocfg.dig_out_type[0]);
> if (err < 0)
> return err;
> err = snd_hda_create_spdif_share_sw(codec,
> --
> 1.7.12.2
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH RFC] ALSA: hda - Add workaround for conflicting IEC958 controls
2012-10-17 8:17 ` Takashi Iwai
@ 2012-10-17 12:43 ` Raymond Yau
2012-10-17 12:44 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Raymond Yau @ 2012-10-17 12:43 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
2012-10-17 下午4:17 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
>
> At Fri, 12 Oct 2012 17:24:51 +0200,
> Takashi Iwai wrote:
> >
> > When both an SPDIF and an HDMI device are created on the same card
> > instance, multiple IEC958 controls are created with indices=0, 1, ...
> > But the alsa-lib configuration can't know which index corresponds
> > actually to which PCM device, and both the SPDIF and the HDMI
> > configurations point to the first IEC958 control wrongly.
> >
> > This patch introduces a (hackish and ugly) workaround: the IEC958
> > controls for the SPDIF device are re-labeled with device=1 when HDMI
> > coexists. The device=1 corresponds to the actual PCM device for
> > SPDIF, so it's anyway a better representation. In future, HDMI
> > controls should be moved with the corresponding PCM device number,
> > too.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
>
> Since there was no big objection to this move, I queued the kernel
> patch to for-next branch now.
>
> About the alsa-lib hack, it's still in consideration. But this kernel
> part doesn't conflict with other possible fix, so let it be in.
>
but your patch still not contain ad1988b , conexant and via vt1708s hda
codecs since there are computers have those audio codecs with nvidia mcp77
codecs
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH RFC] ALSA: hda - Add workaround for conflicting IEC958 controls
2012-10-17 12:43 ` Raymond Yau
@ 2012-10-17 12:44 ` Takashi Iwai
0 siblings, 0 replies; 32+ messages in thread
From: Takashi Iwai @ 2012-10-17 12:44 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel
At Wed, 17 Oct 2012 20:43:22 +0800,
Raymond Yau wrote:
>
> 2012-10-17 下午4:17 於 "Takashi Iwai" <tiwai@suse.de> 寫道:
> >
> > At Fri, 12 Oct 2012 17:24:51 +0200,
> > Takashi Iwai wrote:
> > >
> > > When both an SPDIF and an HDMI device are created on the same card
> > > instance, multiple IEC958 controls are created with indices=0, 1, ...
> > > But the alsa-lib configuration can't know which index corresponds
> > > actually to which PCM device, and both the SPDIF and the HDMI
> > > configurations point to the first IEC958 control wrongly.
> > >
> > > This patch introduces a (hackish and ugly) workaround: the IEC958
> > > controls for the SPDIF device are re-labeled with device=1 when HDMI
> > > coexists. The device=1 corresponds to the actual PCM device for
> > > SPDIF, so it's anyway a better representation. In future, HDMI
> > > controls should be moved with the corresponding PCM device number,
> > > too.
> > >
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >
> > Since there was no big objection to this move, I queued the kernel
> > patch to for-next branch now.
> >
> > About the alsa-lib hack, it's still in consideration. But this kernel
> > part doesn't conflict with other possible fix, so let it be in.
> >
>
> but your patch still not contain ad1988b , conexant and via vt1708s hda
> codecs since there are computers have those audio codecs with nvidia mcp77
> codecs
They are already covered by passing HDA_PCM_TYPE_SPDIF implicitly in
the old API function.
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH RFC 2/2] Add workaround for conflicting IEC958 controls for HD-audio
2012-10-12 15:25 ` [PATCH RFC 2/2] Add workaround for conflicting IEC958 controls for HD-audio Takashi Iwai
@ 2013-02-03 16:40 ` Anssi Hannula
2013-02-04 9:34 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Anssi Hannula @ 2013-02-03 16:40 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
12.10.2012 18:25, Takashi Iwai kirjoitti:
> 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 moves
> the IEC958 controls of an SPDIF with device=1 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@suse.de>
> ---
> src/conf/cards/HDA-Intel.conf | 16 ++++++++++++++++
> src/control/setup.c | 19 ++++++++++++++++---
> 2 files changed, 32 insertions(+), 3 deletions(-)
[...]
AFAICS this patch was never applied. Was there a reason for that?
--
Anssi Hannula
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH RFC 2/2] Add workaround for conflicting IEC958 controls for HD-audio
2013-02-03 16:40 ` Anssi Hannula
@ 2013-02-04 9:34 ` Takashi Iwai
0 siblings, 0 replies; 32+ messages in thread
From: Takashi Iwai @ 2013-02-04 9:34 UTC (permalink / raw)
To: Anssi Hannula; +Cc: alsa-devel
At Sun, 03 Feb 2013 18:40:30 +0200,
Anssi Hannula wrote:
>
> 12.10.2012 18:25, Takashi Iwai kirjoitti:
> > 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 moves
> > the IEC958 controls of an SPDIF with device=1 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@suse.de>
> > ---
> > src/conf/cards/HDA-Intel.conf | 16 ++++++++++++++++
> > src/control/setup.c | 19 ++++++++++++++++---
> > 2 files changed, 32 insertions(+), 3 deletions(-)
> [...]
>
> AFAICS this patch was never applied. Was there a reason for that?
I also don't remember :)
If anyone won't object, I'll apply it later.
Takashi
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
2012-10-12 15:24 ` [PATCH RFC] ALSA: hda - Add workaround for conflicting " Takashi Iwai
2012-10-17 8:17 ` Takashi Iwai
@ 2013-02-08 22:44 ` Anssi Hannula
2013-02-10 10:38 ` Takashi Iwai
1 sibling, 1 reply; 32+ messages in thread
From: Anssi Hannula @ 2013-02-08 22:44 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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?
sound/pci/hda/hda_codec.c | 27 +++++++++++++++------------
sound/pci/hda/hda_codec.h | 4 +++-
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 822df97..fe5d6fc 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -3135,25 +3135,28 @@ int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
int idx, dev = 0;
const int spdif_pcm_dev = 1;
struct hda_spdif_out *spdif;
+ struct hda_codec *c;
- if (codec->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
+ if (codec->bus->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
type == HDA_PCM_TYPE_SPDIF) {
dev = spdif_pcm_dev;
- } else if (codec->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
+ } else if (codec->bus->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
type == HDA_PCM_TYPE_HDMI) {
- for (idx = 0; idx < codec->spdif_out.used; idx++) {
- spdif = snd_array_elem(&codec->spdif_out, idx);
- for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
- kctl = find_mixer_ctl(codec, dig_mix->name, 0, idx);
- if (!kctl)
- break;
- kctl->id.device = spdif_pcm_dev;
+ list_for_each_entry(c, &codec->bus->codec_list, list) {
+ for (idx = 0; idx < c->spdif_out.used; idx++) {
+ spdif = snd_array_elem(&c->spdif_out, idx);
+ for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
+ kctl = find_mixer_ctl(c, dig_mix->name, 0, idx);
+ if (!kctl)
+ break;
+ kctl->id.device = spdif_pcm_dev;
+ }
}
}
- codec->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
+ codec->bus->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
}
- if (!codec->primary_dig_out_type)
- codec->primary_dig_out_type = type;
+ if (!codec->bus->primary_dig_out_type)
+ codec->bus->primary_dig_out_type = type;
idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch", dev);
if (idx < 0) {
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 8665540..ab807f7 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -671,6 +671,9 @@ struct hda_bus {
unsigned int response_reset:1; /* controller was reset */
unsigned int in_reset:1; /* during reset operation */
unsigned int power_keep_link_on:1; /* don't power off HDA link */
+
+ /* primary digital out PCM type */
+ int primary_dig_out_type;
};
/*
@@ -837,7 +840,6 @@ struct hda_codec {
struct mutex hash_mutex;
struct snd_array spdif_out;
unsigned int spdif_in_enable; /* SPDIF input enable? */
- int primary_dig_out_type; /* primary digital out PCM type */
const hda_nid_t *slave_dig_outs; /* optional digital out slave widgets */
struct snd_array init_pins; /* initial (BIOS) pin configurations */
struct snd_array driver_pins; /* pin configs set by codec parser */
--
1.7.10
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
2013-02-08 22:44 ` [PATCH] ALSA: hda - Fix the " Anssi Hannula
@ 2013-02-10 10:38 ` Takashi Iwai
2013-02-10 11:05 ` Anssi Hannula
0 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2013-02-10 10:38 UTC (permalink / raw)
To: Anssi Hannula; +Cc: alsa-devel
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.
Could you add alsa-info.sh output of this board (at best before and
after your patch)?
thanks,
Takashi
>
>
> sound/pci/hda/hda_codec.c | 27 +++++++++++++++------------
> sound/pci/hda/hda_codec.h | 4 +++-
> 2 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index 822df97..fe5d6fc 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -3135,25 +3135,28 @@ int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
> int idx, dev = 0;
> const int spdif_pcm_dev = 1;
> struct hda_spdif_out *spdif;
> + struct hda_codec *c;
>
> - if (codec->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
> + if (codec->bus->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
> type == HDA_PCM_TYPE_SPDIF) {
> dev = spdif_pcm_dev;
> - } else if (codec->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
> + } else if (codec->bus->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
> type == HDA_PCM_TYPE_HDMI) {
> - for (idx = 0; idx < codec->spdif_out.used; idx++) {
> - spdif = snd_array_elem(&codec->spdif_out, idx);
> - for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
> - kctl = find_mixer_ctl(codec, dig_mix->name, 0, idx);
> - if (!kctl)
> - break;
> - kctl->id.device = spdif_pcm_dev;
> + list_for_each_entry(c, &codec->bus->codec_list, list) {
> + for (idx = 0; idx < c->spdif_out.used; idx++) {
> + spdif = snd_array_elem(&c->spdif_out, idx);
> + for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
> + kctl = find_mixer_ctl(c, dig_mix->name, 0, idx);
> + if (!kctl)
> + break;
> + kctl->id.device = spdif_pcm_dev;
> + }
> }
> }
> - codec->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
> + codec->bus->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
> }
> - if (!codec->primary_dig_out_type)
> - codec->primary_dig_out_type = type;
> + if (!codec->bus->primary_dig_out_type)
> + codec->bus->primary_dig_out_type = type;
>
> idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch", dev);
> if (idx < 0) {
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 8665540..ab807f7 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -671,6 +671,9 @@ struct hda_bus {
> unsigned int response_reset:1; /* controller was reset */
> unsigned int in_reset:1; /* during reset operation */
> unsigned int power_keep_link_on:1; /* don't power off HDA link */
> +
> + /* primary digital out PCM type */
> + int primary_dig_out_type;
> };
>
> /*
> @@ -837,7 +840,6 @@ struct hda_codec {
> struct mutex hash_mutex;
> struct snd_array spdif_out;
> unsigned int spdif_in_enable; /* SPDIF input enable? */
> - int primary_dig_out_type; /* primary digital out PCM type */
> const hda_nid_t *slave_dig_outs; /* optional digital out slave widgets */
> struct snd_array init_pins; /* initial (BIOS) pin configurations */
> struct snd_array driver_pins; /* pin configs set by codec parser */
> --
> 1.7.10
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
2013-02-10 10:38 ` Takashi Iwai
@ 2013-02-10 11:05 ` Anssi Hannula
2013-02-11 10:51 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Anssi Hannula @ 2013-02-10 11:05 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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).
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)?
> 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)
>
> thanks,
>
> Takashi
>
>
>>
>>
>> sound/pci/hda/hda_codec.c | 27 +++++++++++++++------------
>> sound/pci/hda/hda_codec.h | 4 +++-
>> 2 files changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
>> index 822df97..fe5d6fc 100644
>> --- a/sound/pci/hda/hda_codec.c
>> +++ b/sound/pci/hda/hda_codec.c
>> @@ -3135,25 +3135,28 @@ int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
>> int idx, dev = 0;
>> const int spdif_pcm_dev = 1;
>> struct hda_spdif_out *spdif;
>> + struct hda_codec *c;
>>
>> - if (codec->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
>> + if (codec->bus->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
>> type == HDA_PCM_TYPE_SPDIF) {
>> dev = spdif_pcm_dev;
>> - } else if (codec->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
>> + } else if (codec->bus->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
>> type == HDA_PCM_TYPE_HDMI) {
>> - for (idx = 0; idx < codec->spdif_out.used; idx++) {
>> - spdif = snd_array_elem(&codec->spdif_out, idx);
>> - for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
>> - kctl = find_mixer_ctl(codec, dig_mix->name, 0, idx);
>> - if (!kctl)
>> - break;
>> - kctl->id.device = spdif_pcm_dev;
>> + list_for_each_entry(c, &codec->bus->codec_list, list) {
>> + for (idx = 0; idx < c->spdif_out.used; idx++) {
>> + spdif = snd_array_elem(&c->spdif_out, idx);
>> + for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
>> + kctl = find_mixer_ctl(c, dig_mix->name, 0, idx);
>> + if (!kctl)
>> + break;
>> + kctl->id.device = spdif_pcm_dev;
>> + }
>> }
>> }
>> - codec->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
>> + codec->bus->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
>> }
>> - if (!codec->primary_dig_out_type)
>> - codec->primary_dig_out_type = type;
>> + if (!codec->bus->primary_dig_out_type)
>> + codec->bus->primary_dig_out_type = type;
>>
>> idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch", dev);
>> if (idx < 0) {
>> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
>> index 8665540..ab807f7 100644
>> --- a/sound/pci/hda/hda_codec.h
>> +++ b/sound/pci/hda/hda_codec.h
>> @@ -671,6 +671,9 @@ struct hda_bus {
>> unsigned int response_reset:1; /* controller was reset */
>> unsigned int in_reset:1; /* during reset operation */
>> unsigned int power_keep_link_on:1; /* don't power off HDA link */
>> +
>> + /* primary digital out PCM type */
>> + int primary_dig_out_type;
>> };
>>
>> /*
>> @@ -837,7 +840,6 @@ struct hda_codec {
>> struct mutex hash_mutex;
>> struct snd_array spdif_out;
>> unsigned int spdif_in_enable; /* SPDIF input enable? */
>> - int primary_dig_out_type; /* primary digital out PCM type */
>> const hda_nid_t *slave_dig_outs; /* optional digital out slave widgets */
>> struct snd_array init_pins; /* initial (BIOS) pin configurations */
>> struct snd_array driver_pins; /* pin configs set by codec parser */
>> --
>> 1.7.10
>>
>
--
Anssi Hannula
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
2013-02-10 11:05 ` Anssi Hannula
@ 2013-02-11 10:51 ` Takashi Iwai
2013-02-11 11:20 ` Anssi Hannula
0 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2013-02-11 10:51 UTC (permalink / raw)
To: Anssi Hannula; +Cc: alsa-devel
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.
> > 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.
Takashi
---
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index e80f835..04b5738 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2332,11 +2332,12 @@ struct snd_kcontrol *snd_hda_find_mixer_ctl(struct hda_codec *codec,
EXPORT_SYMBOL_HDA(snd_hda_find_mixer_ctl);
static int find_empty_mixer_ctl_idx(struct hda_codec *codec, const char *name,
- int dev)
+ int start_idx)
{
- int idx;
- for (idx = 0; idx < 16; idx++) { /* 16 ctlrs should be large enough */
- if (!find_mixer_ctl(codec, name, dev, idx))
+ int i, idx;
+ /* 16 ctlrs should be large enough */
+ for (i = 0, idx = start_idx; i < 16; i++, idx++) {
+ if (!find_mixer_ctl(codec, name, 0, idx))
return idx;
}
return -EBUSY;
@@ -3305,30 +3306,29 @@ int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
int err;
struct snd_kcontrol *kctl;
struct snd_kcontrol_new *dig_mix;
- int idx, dev = 0;
- const int spdif_pcm_dev = 1;
+ int idx = 0;
+ const int spdif_index = 16;
struct hda_spdif_out *spdif;
+ struct hda_bus *bus = codec->bus;
- if (codec->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
+ if (bus->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
type == HDA_PCM_TYPE_SPDIF) {
- dev = spdif_pcm_dev;
- } else if (codec->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
+ idx = spdif_index;
+ } else if (bus->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
type == HDA_PCM_TYPE_HDMI) {
- for (idx = 0; idx < codec->spdif_out.used; idx++) {
- spdif = snd_array_elem(&codec->spdif_out, idx);
- for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
- kctl = find_mixer_ctl(codec, dig_mix->name, 0, idx);
- if (!kctl)
- break;
- kctl->id.device = spdif_pcm_dev;
- }
+ /* suppose a single SPDIF device */
+ for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
+ kctl = find_mixer_ctl(codec, dig_mix->name, 0, 0);
+ if (!kctl)
+ break;
+ kctl->id.index = spdif_index;
}
- codec->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
+ bus->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
}
- if (!codec->primary_dig_out_type)
- codec->primary_dig_out_type = type;
+ if (!bus->primary_dig_out_type)
+ bus->primary_dig_out_type = type;
- idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch", dev);
+ idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch", idx);
if (idx < 0) {
printk(KERN_ERR "hda_codec: too many IEC958 outputs\n");
return -EBUSY;
@@ -3338,7 +3338,6 @@ int snd_hda_create_dig_out_ctls(struct hda_codec *codec,
kctl = snd_ctl_new1(dig_mix, codec);
if (!kctl)
return -ENOMEM;
- kctl->id.device = dev;
kctl->id.index = idx;
kctl->private_value = codec->spdif_out.used - 1;
err = snd_hda_ctl_add(codec, associated_nid, kctl);
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index e8c9442..23ca172 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -679,6 +679,8 @@ struct hda_bus {
unsigned int response_reset:1; /* controller was reset */
unsigned int in_reset:1; /* during reset operation */
unsigned int power_keep_link_on:1; /* don't power off HDA link */
+
+ int primary_dig_out_type; /* primary digital out PCM type */
};
/*
@@ -846,7 +848,6 @@ struct hda_codec {
struct mutex hash_mutex;
struct snd_array spdif_out;
unsigned int spdif_in_enable; /* SPDIF input enable? */
- int primary_dig_out_type; /* primary digital out PCM type */
const hda_nid_t *slave_dig_outs; /* optional digital out slave widgets */
struct snd_array init_pins; /* initial (BIOS) pin configurations */
struct snd_array driver_pins; /* pin configs set by codec parser */
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
2013-02-11 10:51 ` Takashi Iwai
@ 2013-02-11 11:20 ` Anssi Hannula
2013-02-11 11:28 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Anssi Hannula @ 2013-02-11 11:20 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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? ;)
>> > 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.
Also, sorry for not catching this earlier.
Thanks.
>
> Takashi
>
> ---
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index e80f835..04b5738 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -2332,11 +2332,12 @@ struct snd_kcontrol
> *snd_hda_find_mixer_ctl(struct hda_codec *codec,
> EXPORT_SYMBOL_HDA(snd_hda_find_mixer_ctl);
>
> static int find_empty_mixer_ctl_idx(struct hda_codec *codec, const
> char *name,
> - int dev)
> + int start_idx)
> {
> - int idx;
> - for (idx = 0; idx < 16; idx++) { /* 16 ctlrs should be large enough
> */
> - if (!find_mixer_ctl(codec, name, dev, idx))
> + int i, idx;
> + /* 16 ctlrs should be large enough */
> + for (i = 0, idx = start_idx; i < 16; i++, idx++) {
> + if (!find_mixer_ctl(codec, name, 0, idx))
> return idx;
> }
> return -EBUSY;
> @@ -3305,30 +3306,29 @@ int snd_hda_create_dig_out_ctls(struct
> hda_codec *codec,
> int err;
> struct snd_kcontrol *kctl;
> struct snd_kcontrol_new *dig_mix;
> - int idx, dev = 0;
> - const int spdif_pcm_dev = 1;
> + int idx = 0;
> + const int spdif_index = 16;
> struct hda_spdif_out *spdif;
> + struct hda_bus *bus = codec->bus;
>
> - if (codec->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
> + if (bus->primary_dig_out_type == HDA_PCM_TYPE_HDMI &&
> type == HDA_PCM_TYPE_SPDIF) {
> - dev = spdif_pcm_dev;
> - } else if (codec->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
> + idx = spdif_index;
> + } else if (bus->primary_dig_out_type == HDA_PCM_TYPE_SPDIF &&
> type == HDA_PCM_TYPE_HDMI) {
> - for (idx = 0; idx < codec->spdif_out.used; idx++) {
> - spdif = snd_array_elem(&codec->spdif_out, idx);
> - for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
> - kctl = find_mixer_ctl(codec, dig_mix->name, 0, idx);
> - if (!kctl)
> - break;
> - kctl->id.device = spdif_pcm_dev;
> - }
> + /* suppose a single SPDIF device */
> + for (dig_mix = dig_mixes; dig_mix->name; dig_mix++) {
> + kctl = find_mixer_ctl(codec, dig_mix->name, 0, 0);
> + if (!kctl)
> + break;
> + kctl->id.index = spdif_index;
> }
> - codec->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
> + bus->primary_dig_out_type = HDA_PCM_TYPE_HDMI;
> }
> - if (!codec->primary_dig_out_type)
> - codec->primary_dig_out_type = type;
> + if (!bus->primary_dig_out_type)
> + bus->primary_dig_out_type = type;
>
> - idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch",
> dev);
> + idx = find_empty_mixer_ctl_idx(codec, "IEC958 Playback Switch",
> idx);
> if (idx < 0) {
> printk(KERN_ERR "hda_codec: too many IEC958 outputs\n");
> return -EBUSY;
> @@ -3338,7 +3338,6 @@ int snd_hda_create_dig_out_ctls(struct
> hda_codec *codec,
> kctl = snd_ctl_new1(dig_mix, codec);
> if (!kctl)
> return -ENOMEM;
> - kctl->id.device = dev;
> kctl->id.index = idx;
> kctl->private_value = codec->spdif_out.used - 1;
> err = snd_hda_ctl_add(codec, associated_nid, kctl);
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index e8c9442..23ca172 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -679,6 +679,8 @@ struct hda_bus {
> unsigned int response_reset:1; /* controller was reset */
> unsigned int in_reset:1; /* during reset operation */
> unsigned int power_keep_link_on:1; /* don't power off HDA link */
> +
> + int primary_dig_out_type; /* primary digital out PCM type */
> };
>
> /*
> @@ -846,7 +848,6 @@ struct hda_codec {
> struct mutex hash_mutex;
> struct snd_array spdif_out;
> unsigned int spdif_in_enable; /* SPDIF input enable? */
> - int primary_dig_out_type; /* primary digital out PCM type */
> const hda_nid_t *slave_dig_outs; /* optional digital out slave
> widgets */
> struct snd_array init_pins; /* initial (BIOS) pin configurations */
> struct snd_array driver_pins; /* pin configs set by codec parser */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
2013-02-11 11:20 ` Anssi Hannula
@ 2013-02-11 11:28 ` Takashi Iwai
2013-02-12 15:51 ` Anssi Hannula
0 siblings, 1 reply; 32+ messages in thread
From: Takashi Iwai @ 2013-02-11 11:28 UTC (permalink / raw)
To: Anssi Hannula; +Cc: alsa-devel
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.
> Also, sorry for not catching this earlier.
Oh no, thanks for updates!
thanks,
Takashi
---
From: Takashi Iwai <tiwai@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@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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
2013-02-11 11:28 ` Takashi Iwai
@ 2013-02-12 15:51 ` Anssi Hannula
2013-02-12 17:40 ` Takashi Iwai
0 siblings, 1 reply; 32+ messages in thread
From: Anssi Hannula @ 2013-02-12 15:51 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] ALSA: hda - Fix the workaround for conflicting IEC958 controls
2013-02-12 15:51 ` Anssi Hannula
@ 2013-02-12 17:40 ` Takashi Iwai
0 siblings, 0 replies; 32+ messages in thread
From: Takashi Iwai @ 2013-02-12 17:40 UTC (permalink / raw)
To: Anssi Hannula; +Cc: alsa-devel
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@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 :)
OK, merged to sound git tree now.
I'm going to apply the fix for alsa-lib config later.
thanks,
Takashi
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2013-02-12 17:40 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-12 15:18 [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Takashi Iwai
2012-10-12 15:24 ` [PATCH RFC] ALSA: hda - Add workaround for conflicting " Takashi Iwai
2012-10-17 8:17 ` Takashi Iwai
2012-10-17 12:43 ` Raymond Yau
2012-10-17 12:44 ` Takashi Iwai
2013-02-08 22:44 ` [PATCH] ALSA: hda - Fix the " Anssi Hannula
2013-02-10 10:38 ` Takashi Iwai
2013-02-10 11:05 ` Anssi Hannula
2013-02-11 10:51 ` Takashi Iwai
2013-02-11 11:20 ` Anssi Hannula
2013-02-11 11:28 ` Takashi Iwai
2013-02-12 15:51 ` Anssi Hannula
2013-02-12 17:40 ` Takashi Iwai
2012-10-12 15:25 ` [PATCH RFC 1/2] control: Simplify using snd_config_get_bool() Takashi Iwai
2012-10-12 15:25 ` [PATCH RFC 2/2] Add workaround for conflicting IEC958 controls for HD-audio Takashi Iwai
2013-02-03 16:40 ` Anssi Hannula
2013-02-04 9:34 ` Takashi Iwai
2012-10-15 2:31 ` [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls Raymond Yau
2012-10-15 7:46 ` Takashi Iwai
2012-10-15 8:15 ` Raymond Yau
2012-10-15 8:35 ` Takashi Iwai
2012-10-15 8:49 ` Raymond Yau
2012-10-15 8:55 ` Takashi Iwai
[not found] ` <CAN8ccib4Z9VpHcdGxP3q5kofikU6zt6risGDAbOBiRKyKVcsxA@mail.gmail.com>
[not found] ` <CAN8cciY0TfZ+50EuoYbFdz1AzgNuDKZAaDE-o0v-YD_MpXnO1g@mail.gmail.com>
2012-10-15 13:10 ` Raymond Yau
2012-10-15 13:20 ` Takashi Iwai
2012-10-15 10:09 ` David Henningsson
2012-10-15 10:18 ` Takashi Iwai
2012-10-15 10:58 ` David Henningsson
2012-10-15 10:21 ` Jaroslav Kysela
2012-10-15 10:31 ` Takashi Iwai
2012-10-15 11:11 ` Jaroslav Kysela
2012-10-15 12:06 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).