ALSA: hda - Make path_has_mixer() tristate

PATCH In review Ingo Brückl 4 participants 15 messages Mar 3, 2015 – Mar 6, 2015 Sound

lore

Share & tools

lore

Ingo Brückl

author

2015-03-03 11:25 UTC #0 lore raw

PATCH 1/1 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 <[email protected]>

sound/pci/hda/hda_generic.c +3 -3

index b680b4e..ecee349 100644
@@ -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

Takashi Iwai

2015-03-04 11:48 UTC #1 lore raw

At Tue, 03 Mar 2015 12:20:02 +0100,

Quoted 7 lines · Ingo Brückl

... 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

Quoted 24 lines

Signed-off-by Ingo Brückl <[email protected]>

sound/pci/hda/hda_generic.c +3 -4

index b680b4e..ecee349 100644
@@ -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

Quoted 5 lines

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Ingo Brückl

author

2015-03-05 10:31 UTC #2 lore raw

Takashi Iwai wrote on Wed, 04 Mar 2015 12:48:38 +0100:

Quoted 5 lines

At Tue, 03 Mar 2015 12:20:02 +0100,
Quoted 3 lines · Ingo Brückl
... in order to distinguish whether there is no ctl or no path at all.

Quoted 7 lines

Quoted 4 lines
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.

Quoted 1 line

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

Takashi Iwai

2015-03-05 11:38 UTC #3 lore raw

At Thu, 05 Mar 2015 11:20:50 +0100,

Quoted 33 lines · Ingo Brückl

Takashi Iwai wrote on Wed, 04 Mar 2015 12:48:38 +0100:
Quoted 5 lines
At Tue, 03 Mar 2015 12:20:02 +0100,
Quoted 3 lines · Ingo Brückl
... in order to distinguish whether there is no ctl or no path at all.
Quoted 7 lines
Quoted 4 lines
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.
Quoted 1 line
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:

Quoted 6 lines

==> 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?

Quoted 4 lines

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
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Ingo Brückl

author

2015-03-05 16:54 UTC #4 lore raw

Takashi Iwai wrote on Thu, 05 Mar 2015 12:38:33 +0100:

Quoted 9 lines

Quoted 9 lines
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

Quoted 4 lines

Quoted 1 line
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

Takashi Iwai

2015-03-06 09:38 UTC #5 lore raw

At Thu, 05 Mar 2015 17:44:13 +0100,

Quoted 12 lines · Ingo Brückl

Takashi Iwai wrote on Thu, 05 Mar 2015 12:38:33 +0100:
Quoted 9 lines
Quoted 9 lines
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

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
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
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Ingo Brückl

author

2015-03-06 10:04 UTC #6 lore raw

Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:

Quoted 3 lines

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.

Ingo

David Henningsson

2015-03-06 10:20 UTC #7 lore raw

Quoted 7 lines · Ingo Brückl

Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:
Quoted 3 lines
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.

Signature · 4 lines

David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

[1] or the kernel interfaces that hdajackretask is a front-end for

Takashi Iwai

2015-03-06 10:54 UTC #8 lore raw

At Fri, 06 Mar 2015 11:20:22 +0100,

Quoted 19 lines · David Henningsson

Quoted 7 lines · Ingo Brückl
Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:
Quoted 3 lines
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.

Quoted 5 lines

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

Quoted 7 lines

Signature · 5 lines

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

[1] or the kernel interfaces that hdajackretask is a front-end for

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Ingo Brückl

author

2015-03-06 11:10 UTC #9 lore raw

Takashi Iwai wrote on Fri, 06 Mar 2015 11:54:01 +0100:

Quoted 19 lines

At Fri, 06 Mar 2015 11:20:22 +0100,
Quoted 8 lines · David Henningsson
Quoted 6 lines · Ingo Brückl
Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:
Quoted 3 lines
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.
Quoted 8 lines
Quoted 1 line
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.

Quoted 4 lines

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.

Quoted 2 lines

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

Takashi Iwai

2015-03-06 11:33 UTC #10 lore raw

At Fri, 06 Mar 2015 12:07:56 +0100,

Quoted 27 lines · Ingo Brückl

Takashi Iwai wrote on Fri, 06 Mar 2015 11:54:01 +0100:
Quoted 19 lines
At Fri, 06 Mar 2015 11:20:22 +0100,
Quoted 8 lines · David Henningsson
Quoted 6 lines · Ingo Brückl
Takashi Iwai wrote on Fri, 06 Mar 2015 10:38:49 +0100:
Quoted 3 lines
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.
Quoted 8 lines
Quoted 1 line
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.
Quoted 4 lines
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.

Quoted 8 lines

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.
Quoted 2 lines
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

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Ingo Brückl

author

2015-03-06 12:53 UTC #11 lore raw

Takashi Iwai wrote on Fri, 06 Mar 2015 12:33:17 +0100:

Quoted 24 lines

Quoted 5 lines
Quoted 4 lines
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.
Quoted 8 lines
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.
Quoted 2 lines
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:

Quoted 4 lines

[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

Takashi Iwai

2015-03-06 13:13 UTC #12 lore raw

At Fri, 06 Mar 2015 13:33:05 +0100,

Quoted 37 lines · Ingo Brückl

Takashi Iwai wrote on Fri, 06 Mar 2015 12:33:17 +0100:
Quoted 24 lines
Quoted 5 lines
Quoted 4 lines
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.
Quoted 8 lines
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.
Quoted 2 lines
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:
Quoted 4 lines
[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):

Quoted 8 lines

.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
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Ingo Brückl

author

2015-03-06 11:10 UTC #13 lore raw

In reply to David Henningsson

David Henningsson wrote on Fri, 06 Mar 2015 11:20:22 +0100:

Quoted 2 lines

and then you want the official Linux kernel to fix up your private half broken scenario.

I've already realized that my suggestions for patches don't make sense.

Ingo

Raymond Yau

2015-03-04 14:08 UTC #14 lore raw

In reply to Ingo Brückl

2015-03-03 19:20 GMT+08:00 Ingo Brückl <[email protected]>:

Quoted 9 lines

... 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/ad1983-dell-optiplex-745

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);
	}

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel