[PATCH] ALSA: hda - Make path_has_mixer() tristate

[PATCH] ALSA: hda - Make path_has_mixer() tristate

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

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Takashi Iwai]

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Ingo Brückl]

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Takashi Iwai]

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Ingo Brückl]

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Takashi Iwai]

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Ingo Brückl]

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.

Ingo

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[David Henningsson]

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.

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

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Ingo Brückl]

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

> 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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Takashi Iwai]

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Ingo Brückl]

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Takashi Iwai]

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



_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Ingo Brückl]

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

> 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

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

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Takashi Iwai]

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).
> 
> > 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
> 
> 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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: hda - Make path_has_mixer() tristate

[Raymond Yau]

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/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
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel