diff mbox

ALSA: hda - Fix wrong detection of "Headphone+LO" or "Speaker+LO"

Message ID 54F5DB0B.8020507@canonical.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Henningsson March 3, 2015, 4:02 p.m. UTC
On 2015-03-03 16:04, Ingo Brückl wrote:
> David Henningsson wrote on Tue, 03 Mar 2015 14:24:28 +0100:
>
>> If I understand correctly, you have three DACs, one internal speaker,
>> one headphone jack, and three jacks that are both used for 5.1 surround
>> and line out/mic/line in. Is this correct?
>
> This is correct. In addition I have a front mic jack.

Could you upload/pastebin your alsa-info somewhere? Then we could run it 
through the emulator and reproduce the problem.

>> How do the DACs get assigned in this case? One would assume that you'd
>> get 02 -> Front LO, HP, Speaker, 03 -> Rear LO, 04 -> CLFE LO.
>
> With my private patch to enforce multi-io I seem to lose the internal speaker
> which isn't bad because it isn't connected (and probably never won't be).

Since you're using private patches you're somewhat out on "unsupported" 
ground, but anyhow...

>
> You are right concerning the remaining assignments:
>
> multi_outs = 14/0/0/0 : 2/3/4/0 (type LO)
>    out path: depth=3 '02:0c:14'
> multi_ios(2) = 1a/18 : 3/4
>    mio path: depth=3 '03:0d:1a'
>    mio path: depth=3 '04:0e:18'
> hp_outs = 1b/0/0/0 : 2/0/0/0
>    hp  path: depth=3 '02:0c:1b'
> spk_outs = 15/0/0/0 : 0/0/0/0
>
>> And then the volume/mute control for DAC node 02 would be called "PCM"
>> (since both hp_lo_shared and spk_lo_shared are true), but in fact it would
>> be more appropriate to call it "Front".
>
> I've got a Master that would only affect the Front (PCM) and a PCM that
> affected Surround/CLFE which was very unpleasant.
>
> With the patch I'm getting a real Master (for all Front/Surround/CLFE) and a
> separate Front Volume Control in addition to Surround, Center and LFE which
> is exactly how it should be.

Hmm. What do you think of the attached patch - would it work as well? It 
removes the part that returns early for all three, letting it fall 
through to the part that returns "Front" etc.

Comments

Ingo Brueckl March 5, 2015, 10:06 a.m. UTC | #1
David Henningsson wrote on Tue, 03 Mar 2015 17:02:19 +0100:

> Hmm. What do you think of the attached patch - would it work as well? It
> removes the part that returns early for all three, letting it fall
> through to the part that returns "Front" etc.

> +++ b/sound/pci/hda/hda_generic.c
> @@ -1100,11 +1100,9 @@ static const char *get_line_out_pfx(struct
> hda_codec *codec, int ch,
>                 if (!ch && cfg->speaker_outs && cfg->hp_outs) {
>                         bool hp_lo_shared = !path_has_mixer(codec, spec->hp_paths[0], ctl_type);
>                         bool spk_lo_shared = !path_has_mixer(codec, spec->speaker_paths[0], ctl_type);
> -                       if (hp_lo_shared && spk_lo_shared)
> -                               return spec->vmaster_mute.hook ? "PCM" : "Master";
> -                       if (hp_lo_shared)
> +                       if (hp_lo_shared && !spk_lo_shared)
>                                 return "Headphone+LO";
> -                       if (spk_lo_shared)
> +                       if (spk_lo_shared && !hp_lo_shared)
>                                 return "Speaker+LO";
>                 }
>         }

After a quick check, this seems better. Master is a real master now and there
is a Front control.

However, due to the speaker having no path with my forced config, I cannot
say whether this will work if the parser could find the multi-io config by
itself (and until then there probably shouldn't be patches I suggested). And,
by the way, as a result of the no-path I'm getting a Speaker+LO control.

Here a little additional debug output during the get_line_out_pfx() calls.
The "path_has_mixer" output refers to a call which output is in the following
line, i. e. the call for getting spk_lo_shared:

[    0.648407] sound hdaudioC1D0: get_line_out_pfx, AUTO_PIN_LINE_OUT: hp_lo_shared=1
[    0.648407] sound hdaudioC1D0: path_has_mixer: no path!
[    0.648408] sound hdaudioC1D0: get_line_out_pfx, AUTO_PIN_LINE_OUT: spk_lo_shared=1
[    0.648409] sound hdaudioC1D0: get_line_out_pfx, AUTO_PIN_LINE_OUT: hp_lo_shared=0
[    0.648410] sound hdaudioC1D0: path_has_mixer: no path!
[    0.648410] sound hdaudioC1D0: get_line_out_pfx, AUTO_PIN_LINE_OUT: spk_lo_shared=1

Ingo
Raymond Yau March 5, 2015, 11:43 a.m. UTC | #2
>
> > Hmm. What do you think of the attached patch - would it work as well? It
> > removes the part that returns early for all three, letting it fall
> > through to the part that returns "Front" etc.
>
> > +++ b/sound/pci/hda/hda_generic.c
> > @@ -1100,11 +1100,9 @@ static const char *get_line_out_pfx(struct
> > hda_codec *codec, int ch,
> >                 if (!ch && cfg->speaker_outs && cfg->hp_outs) {
> >                         bool hp_lo_shared = !path_has_mixer(codec,
spec->hp_paths[0], ctl_type);
> >                         bool spk_lo_shared = !path_has_mixer(codec,
spec->speaker_paths[0], ctl_type);
> > -                       if (hp_lo_shared && spk_lo_shared)
> > -                               return spec->vmaster_mute.hook ? "PCM"
: "Master";
> > -                       if (hp_lo_shared)
> > +                       if (hp_lo_shared && !spk_lo_shared)
> >                                 return "Headphone+LO";
> > -                       if (spk_lo_shared)
> > +                       if (spk_lo_shared && !hp_lo_shared)
> >                                 return "Speaker+LO";
> >                 }
> >         }
>
> After a quick check, this seems better. Master is a real master now and
there
> is a Front control.

>> >> With the patch I'm getting a real Master (for all
Front/Surround/CLFE) and a
>> separate Front Volume Control in addition to Surround, Center and LFE
which >> is exactly how it should be.
> > > Hmm. What do you think of the attached patch - would it work as well?
It removes the part that returns early for all three, letting it fall
through to the part that returns "Front" etc.

http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/src/modules/alsa/mixer/paths/analog-output-headphones.conf?id=
96369919e5100865e2469e42fb8f4b8e38e41aef

Do you mean front playback volume is set to maximum when headphone is
plugged?

How do Pulseaudio calculate dB min of headphone path and lineout path?

Do it just put dB min of front playback volume in headphone path only when
there is no headphone playback volume?

BTW codec alc888 in buglink can use amp out in node 0x26 as Headphone
Playback volume

>
> However, due to the speaker having no path with my forced config, I cannot
> say whether this will work if the parser could find the multi-io config by
> itself (and until then there probably shouldn't be patches I suggested).
And,
> by the way, as a result of the no-path I'm getting a Speaker+LO control.
>
> Here a little additional debug output during the get_line_out_pfx() calls.
> The "path_has_mixer" output refers to a call which output is in the
following
> line, i. e. the call for getting spk_lo_shared:
>
> [    0.648407] sound hdaudioC1D0: get_line_out_pfx, AUTO_PIN_LINE_OUT:
hp_lo_shared=1
> [    0.648407] sound hdaudioC1D0: path_has_mixer: no path!
> [    0.648408] sound hdaudioC1D0: get_line_out_pfx, AUTO_PIN_LINE_OUT:
spk_lo_shared=1
> [    0.648409] sound hdaudioC1D0: get_line_out_pfx, AUTO_PIN_LINE_OUT:
hp_lo_shared=0
> [    0.648410] sound hdaudioC1D0: path_has_mixer: no path!
> [    0.648410] sound hdaudioC1D0: get_line_out_pfx, AUTO_PIN_LINE_OUT:
spk_lo_shared=1
>
> Ingo
diff mbox

Patch

diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index b680b4e..9f0be7c 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -1100,11 +1100,9 @@  static const char *get_line_out_pfx(struct hda_codec *codec, int ch,
 		if (!ch && cfg->speaker_outs && cfg->hp_outs) {
 			bool hp_lo_shared = !path_has_mixer(codec, spec->hp_paths[0], ctl_type);
 			bool spk_lo_shared = !path_has_mixer(codec, spec->speaker_paths[0], ctl_type);
-			if (hp_lo_shared && spk_lo_shared)
-				return spec->vmaster_mute.hook ? "PCM" : "Master";
-			if (hp_lo_shared)
+			if (hp_lo_shared && !spk_lo_shared)
 				return "Headphone+LO";
-			if (spk_lo_shared)
+			if (spk_lo_shared && !hp_lo_shared)
 				return "Speaker+LO";
 		}
 	}