[alsa-devel] [PATCH] ALSA: hda - Make path_has_mixer() tristate
... in order to distinguish whether there is no ctl or no path at all.
It is used as !path_has_mixer() which means that it is true even if there is no path at all. For headphone or speaker outs without path, because they don't have a DAC assign to them, this would give a false positive result.
Signed-off-by: Ingo Brückl ib@wupperonline.de
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index b680b4e..ecee349 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -1035,11 +1035,11 @@ static int hda_gen_bind_mute_put(struct snd_kcontrol *kcontrol, return snd_hda_mixer_bind_switch_put(kcontrol, ucontrol); }
-/* any ctl assigned to the path with the given index? */ -static bool path_has_mixer(struct hda_codec *codec, int path_idx, int ctl_type) +/* any ctl assigned to the path (if one at all) with the given index? */ +static int path_has_mixer(struct hda_codec *codec, int path_idx, int ctl_type) { struct nid_path *path = snd_hda_get_path_from_idx(codec, path_idx); - return path && path->ctls[ctl_type]; + return path ? path->ctls[ctl_type] : -1; }
static const char * const channel_name[4] = { -- 1.7.10
At Tue, 03 Mar 2015 12:20:02 +0100, Ingo Brückl wrote:
... in order to distinguish whether there is no ctl or no path at all.
It is used as !path_has_mixer() which means that it is true even if there is no path at all. For headphone or speaker outs without path, because they don't have a DAC assign to them, this would give a false positive result.
Do you mean the code checking hp_lo_shared and spk_lo_shared in get_line_out_pfx()? How can they be no path?
Takashi
Signed-off-by: Ingo Brückl ib@wupperonline.de
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index b680b4e..ecee349 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -1035,11 +1035,11 @@ static int hda_gen_bind_mute_put(struct snd_kcontrol *kcontrol, return snd_hda_mixer_bind_switch_put(kcontrol, ucontrol); }
-/* any ctl assigned to the path with the given index? */ -static bool path_has_mixer(struct hda_codec *codec, int path_idx, int ctl_type) +/* any ctl assigned to the path (if one at all) with the given index? */ +static int path_has_mixer(struct hda_codec *codec, int path_idx, int ctl_type) { struct nid_path *path = snd_hda_get_path_from_idx(codec, path_idx);
- return path && path->ctls[ctl_type];
- return path ? path->ctls[ctl_type] : -1;
}
static const char * const channel_name[4] = {
1.7.10
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Takashi Iwai wrote on Wed, 04 Mar 2015 12:48:38 +0100:
At Tue, 03 Mar 2015 12:20:02 +0100, Ingo Brückl wrote:
... in order to distinguish whether there is no ctl or no path at all.
It is used as !path_has_mixer() which means that it is true even if there is no path at all. For headphone or speaker outs without path, because they don't have a DAC assign to them, this would give a false positive result.
Do you mean the code checking hp_lo_shared and spk_lo_shared in get_line_out_pfx()?
Exactly.
How can they be no path?
This is due to me forcing a mutli-io config even if there is no DAC for the speaker (please see the "Intel HDA / ALC662 analog surround problem" thread):
==> Best config: lo_type=0, wired=0, mio=1 multi_outs = 14/0/0/0 : 2/3/4/0 (type LO) multi_ios(2) = 1a/18 : 3/4 hp_outs = 1b/0/0/0 : 2/0/0/0 spk_outs = 15/0/0/0 : 0/0/0/0
I would love the auto-parser to assign the speaker to node 0x02 (or simply disable the speaker), but unfortunately I'm not getting a surround config with the current code, but only:
==> lo_type=0, wired=1, mio=0 multi_outs = 14/0/0/0 : 2/0/0/0 (type LO) hp_outs = 1b/0/0/0 : 4/0/0/0 spk_outs = 15/0/0/0 : 3/0/0/0
This is unsatisfactory.
Concerning my path_has_mixer() tristate patch: Raymond stated that path_has_mixer() does not imply shared volume. Although I'm not familiar enough with all this, I would agree that if there happens to be no path at all, there isn't any sharing also.
Ingo
At Thu, 05 Mar 2015 11:20:50 +0100, Ingo Brückl wrote:
Takashi Iwai wrote on Wed, 04 Mar 2015 12:48:38 +0100:
At Tue, 03 Mar 2015 12:20:02 +0100, Ingo Brückl wrote:
... in order to distinguish whether there is no ctl or no path at all.
It is used as !path_has_mixer() which means that it is true even if there is no path at all. For headphone or speaker outs without path, because they don't have a DAC assign to them, this would give a false positive result.
Do you mean the code checking hp_lo_shared and spk_lo_shared in get_line_out_pfx()?
Exactly.
How can they be no path?
This is due to me forcing a mutli-io config even if there is no DAC for the speaker (please see the "Intel HDA / ALC662 analog surround problem" thread):
==> Best config: lo_type=0, wired=0, mio=1 multi_outs = 14/0/0/0 : 2/3/4/0 (type LO) multi_ios(2) = 1a/18 : 3/4 hp_outs = 1b/0/0/0 : 2/0/0/0 spk_outs = 15/0/0/0 : 0/0/0/0
I would love the auto-parser to assign the speaker to node 0x02 (or simply disable the speaker), but unfortunately I'm not getting a surround config with the current code, but only:
==> lo_type=0, wired=1, mio=0 multi_outs = 14/0/0/0 : 2/0/0/0 (type LO) hp_outs = 1b/0/0/0 : 4/0/0/0 spk_outs = 15/0/0/0 : 3/0/0/0
This is unsatisfactory.
You can force the DAC assignment by specifying spec->gen.perferred_dacs. Did you try it?
Concerning my path_has_mixer() tristate patch: Raymond stated that path_has_mixer() does not imply shared volume. Although I'm not familiar enough with all this, I would agree that if there happens to be no path at all, there isn't any sharing also.
Changing path_hax_miser() makes little sense. A path must be present when the I/O is activated. That's the whole point of using paths. So, enabling the output without path is fundamentally wrong.
Takashi
Takashi Iwai wrote on Thu, 05 Mar 2015 12:38:33 +0100:
I would love the auto-parser to assign the speaker to node 0x02 (or simply disable the speaker), but unfortunately I'm not getting a surround config with the current code, but only:
==> lo_type=0, wired=1, mio=0 multi_outs = 14/0/0/0 : 2/0/0/0 (type LO) hp_outs = 1b/0/0/0 : 4/0/0/0 spk_outs = 15/0/0/0 : 3/0/0/0
This is unsatisfactory.
You can force the DAC assignment by specifying spec->gen.perferred_dacs. Did you try it?
Yes, no success. I'm now ignoring speaker 0x15.
Ingo
At Thu, 05 Mar 2015 17:44:13 +0100, Ingo Brückl wrote:
Takashi Iwai wrote on Thu, 05 Mar 2015 12:38:33 +0100:
I would love the auto-parser to assign the speaker to node 0x02 (or simply disable the speaker), but unfortunately I'm not getting a surround config with the current code, but only:
==> lo_type=0, wired=1, mio=0 multi_outs = 14/0/0/0 : 2/0/0/0 (type LO) hp_outs = 1b/0/0/0 : 4/0/0/0 spk_outs = 15/0/0/0 : 3/0/0/0
This is unsatisfactory.
You can force the DAC assignment by specifying spec->gen.perferred_dacs. Did you try it?
Yes, no success. I'm now ignoring speaker 0x15.
Even if you try parsing the topology at first with ignoring 0x15, you'll need to add the path over 0x15 manually back to the information. Otherwise it can't work.
In anyway, could you post alsa-info.sh? David already asked it but you didn't provide...
Takashi
On 2015-03-06 10:57, Ingo Brückl wrote:
Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:
Even if you try parsing the topology at first with ignoring 0x15, you'll need to add the path over 0x15 manually back to the information. Otherwise it can't work.
Why can't it work? I'm not going to use the speaker.
There is no known hardware that has an speaker with no connection to it (and it would be a BIOS error if there was one), so it is not a priority (perhaps not even of interest) of the driver to support it. Especially not as the option of using e g hdajackretask [1] to disable the speaker permanently is an easier and better way than starting to mess around with the connections and paths manually.
I understand that your intentions are good and that you want to help other users as well, and I don't want to scare you off, but in this case it seems to me like you have some private patches that makes the driver half broken, and then you want the official Linux kernel to fix up your private half broken scenario. Which doesn't really make sense.
At Fri, 06 Mar 2015 11:20:22 +0100, David Henningsson wrote:
On 2015-03-06 10:57, Ingo Brückl wrote:
Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:
Even if you try parsing the topology at first with ignoring 0x15, you'll need to add the path over 0x15 manually back to the information. Otherwise it can't work.
Why can't it work? I'm not going to use the speaker.
There is no known hardware that has an speaker with no connection to it (and it would be a BIOS error if there was one), so it is not a priority (perhaps not even of interest) of the driver to support it. Especially not as the option of using e g hdajackretask [1] to disable the speaker permanently is an easier and better way than starting to mess around with the connections and paths manually.
I understand that your intentions are good and that you want to help other users as well, and I don't want to scare you off, but in this case it seems to me like you have some private patches that makes the driver half broken, and then you want the official Linux kernel to fix up your private half broken scenario. Which doesn't really make sense.
Right. The big difference is "not using" and "disabling". The former is user's decision, but without telling the driver to disable the functionality, the driver must still assume that it might be used in future. So, a more desirable solution for the former would be to implement the automatic speaker on/off (and limit the auto-mute behavior) depending on the channel mode configuration. This will need a lot of work.
OTOH, the latter can be achieved easily by changing the pin default configuration, even dynamically reconfigurable via hdajackretask, as David suggested.
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
[1] or the kernel interfaces that hdajackretask is a front-end for
Takashi Iwai wrote on Fri, 06 Mar 2015 11:54:01 +0100:
At Fri, 06 Mar 2015 11:20:22 +0100, David Henningsson wrote:
On 2015-03-06 10:57, Ingo Brückl wrote:
Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:
Even if you try parsing the topology at first with ignoring 0x15, you'll need to add the path over 0x15 manually back to the information. Otherwise it can't work.
Why can't it work? I'm not going to use the speaker.
There is no known hardware that has an speaker with no connection to it (and it would be a BIOS error if there was one), so it is not a priority (perhaps not even of interest) of the driver to support it. Especially not as the option of using e g hdajackretask [1] to disable the speaker permanently is an easier and better way than starting to mess around with the connections and paths manually.
Right. The big difference is "not using" and "disabling". The former is user's decision, but without telling the driver to disable the functionality, the driver must still assume that it might be used in future.
Thank you very much for this explanation. As a simple user, not being deep into hardware and driver internals, I wasn't aware of that difference.
OTOH, the latter can be achieved easily by changing the pin default configuration
Fine, if just someone could please advise what to do (hdajackretask isn't an option, no GUI, only console access).
Ingo
At Fri, 06 Mar 2015 12:07:56 +0100, Ingo Brückl wrote:
Takashi Iwai wrote on Fri, 06 Mar 2015 11:54:01 +0100:
At Fri, 06 Mar 2015 11:20:22 +0100, David Henningsson wrote:
On 2015-03-06 10:57, Ingo Brückl wrote:
Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:
Even if you try parsing the topology at first with ignoring 0x15, you'll need to add the path over 0x15 manually back to the information. Otherwise it can't work.
Why can't it work? I'm not going to use the speaker.
There is no known hardware that has an speaker with no connection to it (and it would be a BIOS error if there was one), so it is not a priority (perhaps not even of interest) of the driver to support it. Especially not as the option of using e g hdajackretask [1] to disable the speaker permanently is an easier and better way than starting to mess around with the connections and paths manually.
Right. The big difference is "not using" and "disabling". The former is user's decision, but without telling the driver to disable the functionality, the driver must still assume that it might be used in future.
Thank you very much for this explanation. As a simple user, not being deep into hardware and driver internals, I wasn't aware of that difference.
OTOH, the latter can be achieved easily by changing the pin default configuration
Fine, if just someone could please advise what to do (hdajackretask isn't an option, no GUI, only console access).
1. Kill all sound apps and backends 2. Run below as root echo "0x15 0x411111f0" > /sys/class/sound/hwC0D0/user_pin_configs echo 1 > /sys/class/sound/hwC0D0/reconfig
That's all. If you want to have this from the beginning, create a firmware "patch" file and pass it via patch option of snd-hda-intel module. See Documentation/sound/alsa/HD-audio.txt. In your case, you need entries like:
[codec] 0xxxxx 0xxxx 0 [pincfg] 0x15 0x411111f0
Takashi
Takashi Iwai wrote on Fri, 06 Mar 2015 12:33:17 +0100:
Right. The big difference is "not using" and "disabling". The former is user's decision, but without telling the driver to disable the functionality, the driver must still assume that it might be used in future.
Thank you very much for this explanation. As a simple user, not being deep into hardware and driver internals, I wasn't aware of that difference.
OTOH, the latter can be achieved easily by changing the pin default configuration
Fine, if just someone could please advise what to do (hdajackretask isn't an option, no GUI, only console access).
- Kill all sound apps and backends
- Run below as root echo "0x15 0x411111f0" > /sys/class/sound/hwC0D0/user_pin_configs echo 1 > /sys/class/sound/hwC0D0/reconfig
That's all. If you want to have this from the beginning, create a firmware "patch" file and pass it via patch option of snd-hda-intel module. See Documentation/sound/alsa/HD-audio.txt. In your case, you need entries like:
[codec] 0xxxxx 0xxxx 0 [pincfg] 0x15 0x411111f0
Thank you very much, I'll do.
Supposing that the pin config could be given as a model for the ALC662, something like (according to Raymond):
.type = HDA_FIXUP_PINS, .v.pins = (const struct hda_pintbl[]) { {0x15, 0x411111f0}, {} }
would such a "no-speaker" model make sense (for people suffering from the same issue as me)?
Ingo
At Fri, 06 Mar 2015 13:33:05 +0100, Ingo Brückl wrote:
Takashi Iwai wrote on Fri, 06 Mar 2015 12:33:17 +0100:
Right. The big difference is "not using" and "disabling". The former is user's decision, but without telling the driver to disable the functionality, the driver must still assume that it might be used in future.
Thank you very much for this explanation. As a simple user, not being deep into hardware and driver internals, I wasn't aware of that difference.
OTOH, the latter can be achieved easily by changing the pin default configuration
Fine, if just someone could please advise what to do (hdajackretask isn't an option, no GUI, only console access).
- Kill all sound apps and backends
- Run below as root echo "0x15 0x411111f0" > /sys/class/sound/hwC0D0/user_pin_configs echo 1 > /sys/class/sound/hwC0D0/reconfig
That's all. If you want to have this from the beginning, create a firmware "patch" file and pass it via patch option of snd-hda-intel module. See Documentation/sound/alsa/HD-audio.txt. In your case, you need entries like:
[codec] 0xxxxx 0xxxx 0 [pincfg] 0x15 0x411111f0
Thank you very much, I'll do.
Supposing that the pin config could be given as a model for the ALC662, something like (according to Raymond):
.type = HDA_FIXUP_PINS, .v.pins = (const struct hda_pintbl[]) { {0x15, 0x411111f0}, {} }
would such a "no-speaker" model make sense (for people suffering from the same issue as me)?
Yeah, that's acceptable, but try to name it to be specific. The speaker isn't always assigned to pin 0x15.
Takashi
2015-03-03 19:20 GMT+08:00 Ingo Brückl ib@wupperonline.de:
... in order to distinguish whether there is no ctl or no path at all.
It is used as !path_has_mixer() which means that it is true even if there is no path at all. For headphone or speaker outs without path, because they don't have a DAC assign to them, this would give a false positive result.
your previous patch which ignore BAD_NO_PRIMARY_DAC is not enough, you
need to assign DAC to speaker path
e.g. for hda codec with single DAC with internal speaker, headphone and lineout , the three paths contain same DAC node 0x03
http://git.kernel.org/cgit/linux/kernel/git/tiwai/hda-emu.git/tree/codecs/ad...
seem snd_hda_multi_out_analog_prepare() and snd_hda_multi_out_analog_cleanup() can handle same DAC in three paths
/* front */ snd_hda_codec_setup_stream(codec, nids[HDA_FRONT], stream_tag, 0, format); if (!mout->no_share_stream && mout->hp_nid && mout->hp_nid != nids[HDA_FRONT]) /* headphone out will just decode front left/right (stereo) */ snd_hda_codec_setup_stream(codec, mout->hp_nid, stream_tag, 0, format); /* extra outputs copied from front */ for (i = 0; i < ARRAY_SIZE(mout->hp_out_nid); i++) if (!mout->no_share_stream && mout->hp_out_nid[i]) snd_hda_codec_setup_stream(codec, mout->hp_out_nid[i], stream_tag, 0, format);
/* surrounds */ for (i = 1; i < mout->num_dacs; i++) { if (chs >= (i + 1) * 2) /* independent out */ snd_hda_codec_setup_stream(codec, nids[i], stream_tag, i * 2, format); else if (!mout->no_share_stream) /* copy front */ snd_hda_codec_setup_stream(codec, nids[i], stream_tag, 0, format); }
/* extra surrounds */ for (i = 0; i < ARRAY_SIZE(mout->extra_out_nid); i++) { int ch = 0; if (!mout->extra_out_nid[i]) break; if (chs >= (i + 1) * 2) ch = i * 2; else if (!mout->no_share_stream) break; snd_hda_codec_setup_stream(codec, mout->extra_out_nid[i], stream_tag, ch, format); }
participants (4)
-
David Henningsson -
Ingo Brückl -
Raymond Yau -
Takashi Iwai