[alsa-devel] [RFC] Fix for conflict of HDMI and SPDIF IEC958 controls
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
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,
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;@@ -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;@@ -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; } elsediff --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);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);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);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]);-- 1.7.12.2
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/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
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
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 */
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
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
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 */
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;@@ -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;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 */
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;
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
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
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);
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;
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?
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
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/+attachmen...
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
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/+attachmen...
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
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
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
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
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
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.
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
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 :-)
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
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
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
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
participants (5)
-
Anssi Hannula -
David Henningsson -
Jaroslav Kysela -
Raymond Yau -
Takashi Iwai