diff mbox

[Precise,0/2] ALSA: hda/realtek - Finer tuning of auto-parser with badness evaluation

Message ID 20120628161325.GA3314@herton-Z68MA-D2H-B3
State New
Headers show

Commit Message

Herton Ronaldo Krzesinski June 28, 2012, 4:13 p.m. UTC 
On Wed, Jun 27, 2012 at 02:34:04PM -0400, Joseph Salisbury wrote:
> On 06/27/2012 02:26 PM, joseph.salisbury@canonical.com wrote:
> > From: Joseph Salisbury <joseph.salisbury@canonical.com>
> > 
> > BugLink: http://bugs.launchpad.net/bugs/994685
> > 
> > == Precise SRU Justification ==
> > 
> > This bug is a regression from Oneiric.  This bug prevents end users from using the headphone jack.
> > 
> > == Fix ==
> > 
> > commit 1c4a54b4513c175ba1a56d0aba8d9cf8f231d407
> > Author: Takashi Iwai <tiwai@suse.de>
> > Date:   Thu Feb 16 16:45:59 2012 +0100
> > 
> >     ALSA: hda/realtek - Finer tuning of auto-parser with badness evaluation
> >     
> > 
> > commit 07b18f69a766375736a5313c29d808e59b1e13e9
> > Author: Takashi Iwai <tiwai@suse.de>
> > Date:   Thu Nov 10 15:42:54 2011 +0100
> > 
> >     ALSA: hda/realtek - Create multi-io jacks more aggresively
> >     
> > Commit 07b18f6 allows us to cleanly cherry-pick the actual fix, commit 1c4a54b.
> > Both commits had to be backported to apply cleanly to Precise.
> > 
> > 
> > == Impact ==
> > 
> > Without this patch, the headphone jack does not work for a set of users.
> > 
> > == Test Case ==
> > A test kernel was built with these patchs and tested by the original bug reporter.
> > 
> > 
> > 
> > 
> > Takashi Iwai (2):
> >   ALSA: hda/realtek - Create multi-io jacks more aggresively
> >   ALSA: hda/realtek - Finer tuning of auto-parser with badness
> >     evaluation
> > 
> >  sound/pci/hda/patch_realtek.c |  412 +++++++++++++++++++++++++++++++++--------
> >  1 file changed, 330 insertions(+), 82 deletions(-)
> > 
> 
> There is allot of changes in these patches.  If there is too much churn
> for SRU approval, I can look into identifying the commit that introduced
> the regression in Precise and see if it can be reverted.

Joseph, thanks for handling the bug. But I'm afraid, at least to my
opinion, this seems too big of a change for an SRU.

So accordingly to the bug report, the second patch is what fixed the
issue, the first is just a pre-requisite. I looked at the changes, and
basically the second patch tries to optimize dac <-> pins assignments on
the realtek codecs, it's a pretty intrusive change for an stable kernel,
since it affects all machines with hda realtek codecs (which should be a
large number, so the potential for regressions is high).

But also I'm puzzled why this change is required. While running the
codec file through hda-emu*, the only differences in codec configuration
are that the speakers pins get a different DAC with the patched kernel,
and mixer items created are different, but couldn't find nothing that
would make plugged headphones to always be silent.

This is the codec config difference between the patched and unpatched
precise master-next:



And the mixer items created with precise master-next (1), and the
patched precise master-next (2):

(1)
> list
1: Speaker Playback Volume:0 (1)
2: Speaker Playback Switch:0 (1)
3: Bass Speaker Playback Volume:0 (1)
4: Bass Speaker Playback Switch:0 (1)
5: Headphone Playback Switch:0 (1)
6: Mic Playback Volume:0 (1)
7: Mic Playback Switch:0 (1)
8: Auto-Mute Mode:0 (1)
9: Internal Mic Boost Volume:0 (1)
10: Mic Boost Volume:0 (1)
11: Capture Switch:0 (1)
12: Capture Volume:0 (1)
13: Beep Playback Volume:0 (1)
14: Beep Playback Switch:0 (1)
15: Master Playback Volume:0 (1)
16: Master Playback Switch:0 (1)
17: Headphone Jack:0 (1)
18: Mic Jack:0 (1)

(2)
> list
1: Headphone Playback Volume:0 (1)
2: Headphone Playback Switch:0 (1)
3: Speaker Playback Switch:0 (1)
4: Speaker Playback Volume:0 (1)
5: Mic Playback Volume:0 (1)
6: Mic Playback Switch:0 (1)
7: Auto-Mute Mode:0 (1)
8: Internal Mic Boost Volume:0 (1)
9: Mic Boost Volume:0 (1)
10: Capture Switch:0 (1)
11: Capture Volume:0 (1)
12: Beep Playback Volume:0 (1)
13: Beep Playback Switch:0 (1)
14: Master Playback Volume:0 (1)
15: Master Playback Switch:0 (1)
16: Headphone Jack:0 (1)
17: Mic Jack:0 (1)

The mixer in unpatched kernel misses HP Playback Volume, but in theory
Speaker Playback Volume should control the HP volume as well, since it
controls DAC nid 0x2 and headphone pin (0x21) is connected to 0x0c mixer
that gets the DAC 0x2 output.

So, I'm wondering if the sound doesn't work in precise due to an user
space bug, may be the sound settings test, which I don't know as well
what it does exactly, may be is confused with missing/unexpected
headphone playback controls, or something else. The thing is, the
patches just make a different DAC to be selected for the Speaker pin 0x14,
which doesn't explain why the headphone test starting to work successfully,
so may be the test is not entirely right, or confused about the different
mixer items (may be a pulse issue related to "interpreting" the mixer).

So may be trying to not use the sound settings test, pluging a headphone
and test by using another application or by passing pulse, may give a
better understanding. If it's confused about the mixer, may be ok, the
kernel change could be acceptable, but it means we have an diverged 3.2
realtek/alsa codec auto config parsing structural code which we would
need to always have to keep eyes on, not much practical.

*hda-emu for who doesn't know allow to emulate/simulate hda codec
parsing/setup of the kernel without having the hardware, just with codec
info, git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/hda-emu.git

Other relevant info between patched and not patched precise-master-next,
printed out by hda-emu:

(1) non patched:
autoconfig: line_outs=2 (0x14/0x1a/0x0/0x0/0x0) type:speaker
   speaker_outs=0 (0x0/0x0/0x0/0x0/0x0)
   hp_outs=1 (0x21/0x0/0x0/0x0/0x0)
   mono: mono_out=0x0
   inputs: Internal Mic=0x12 Mic=0x18

realtek: No valid SSID, checking pincfg 0x40179a2d for NID 0x1d
realtek: Enabling init ASM_ID=0x9a2d CODEC_ID=10ec0269

realtek: Enable HP auto-muting on NID 0x21
realtek: Enable auto-mic switch on NID 0x18/0x12/0x0

(2) patched:

autoconfig: line_outs=2 (0x14/0x1a/0x0/0x0/0x0) type:speaker
   speaker_outs=0 (0x0/0x0/0x0/0x0/0x0)
   hp_outs=1 (0x21/0x0/0x0/0x0/0x0)
   mono: mono_out=0x0
   inputs: Internal Mic=0x12 Mic=0x18

==> lo_type=1, wired=1, badness=0x1000
multi_outs = 14/1a/0/0 : 2/3/0/0
hp_outs = 21/0/0/0 : 0/0/0/0
spk_outs = 0/0/0/0 : 0/0/0/0
==> lo_type=1, wired=0, badness=0x1000
multi_outs = 14/1a/0/0 : 2/3/0/0
hp_outs = 21/0/0/0 : 0/0/0/0
spk_outs = 0/0/0/0 : 0/0/0/0
==> lo_type=2, wired=1, badness=0x10
multi_outs = 21/0/0/0 : 2/0/0/0
hp_outs = 0/0/0/0 : 0/0/0/0
spk_outs = 14/1a/0/0 : 3/0/0/0
==> lo_type=2, wired=0, badness=0x10
multi_outs = 21/0/0/0 : 2/0/0/0
hp_outs = 0/0/0/0 : 0/0/0/0
spk_outs = 14/1a/0/0 : 3/0/0/0
==> Best config: lo_type=2, wired=1
multi_outs = 21/0/0/0 : 2/0/0/0
hp_outs = 0/0/0/0 : 0/0/0/0
spk_outs = 14/1a/0/0 : 3/0/0/0

realtek: No valid SSID, checking pincfg 0x40179a2d for NID 0x1d
realtek: Enabling init ASM_ID=0x9a2d CODEC_ID=10ec0269

realtek: Enable HP auto-muting on NID 0x21
realtek: Enable auto-mic switch on NID 0x18/0x12/0x0


Note on the patched kernel, the badness/Best config is the code added in
the second patch trying to optimize pin <-> dac assignment.

Comments

Herton Ronaldo Krzesinski June 28, 2012, 5:24 p.m. UTC | #1 
On Thu, Jun 28, 2012 at 01:13:26PM -0300, Herton Ronaldo Krzesinski wrote:
> The mixer in unpatched kernel misses HP Playback Volume, but in theory
> Speaker Playback Volume should control the HP volume as well, since it
> controls DAC nid 0x2 and headphone pin (0x21) is connected to 0x0c mixer
> that gets the DAC 0x2 output.
> 
> So, I'm wondering if the sound doesn't work in precise due to an user
> space bug, may be the sound settings test, which I don't know as well
> what it does exactly, may be is confused with missing/unexpected
> headphone playback controls, or something else. The thing is, the
> patches just make a different DAC to be selected for the Speaker pin 0x14,
> which doesn't explain why the headphone test starting to work successfully,
> so may be the test is not entirely right, or confused about the different
> mixer items (may be a pulse issue related to "interpreting" the mixer).
> 
> So may be trying to not use the sound settings test, pluging a headphone
> and test by using another application or by passing pulse, may give a
> better understanding. If it's confused about the mixer, may be ok, the
> kernel change could be acceptable, but it means we have an diverged 3.2
> realtek/alsa codec auto config parsing structural code which we would
> need to always have to keep eyes on, not much practical.

I forgot to check if automute had something to do with the bug, since in
theory it could mute a DAC/mixer input if considering it for an entire
pin class (like speakers vs. HP), so changing the DAC/mixer selection
could have an influence.

Checking now on the unpatched kernel, it seems to be doing nothing wrong.
If you plug the headphone (pin 0x21), it mutes the speaker pins (changes
the pin control to 0), no dac/mixer is touched:

> jack 0x21 1
send: NID=0x21, VERB=0xf09(get_pin_sense), PARM=0x0
receive: 0x80000000
send: NID=0x21, VERB=0x707(set_pin_ctl), PARM=0xc0
send: NID=0x14, VERB=0x707(set_pin_ctl), PARM=0x0
send: NID=0x1a, VERB=0x707(set_pin_ctl), PARM=0x0
CTL Notify: Headphone Jack:0, mask=1
JACK report Headphone, status 1
JACK report Mic, status 0
> jack 0x21 0
send: NID=0x21, VERB=0xf09(get_pin_sense), PARM=0x0
receive: 0x0
send: NID=0x21, VERB=0x707(set_pin_ctl), PARM=0xc0
send: NID=0x14, VERB=0x707(set_pin_ctl), PARM=0x40
send: NID=0x1a, VERB=0x707(set_pin_ctl), PARM=0x40
CTL Notify: Headphone Jack:0, mask=1
JACK report Headphone, status 0
JACK report Mic, status 0

I'll follow up on the bug, hope didn't miss anything.
David Henningsson June 28, 2012, 9:06 p.m. UTC | #2 
On 06/28/2012 07:24 PM, Herton Ronaldo Krzesinski wrote:
> On Thu, Jun 28, 2012 at 01:13:26PM -0300, Herton Ronaldo Krzesinski wrote:
>> The mixer in unpatched kernel misses HP Playback Volume, but in theory
>> Speaker Playback Volume should control the HP volume as well, since it
>> controls DAC nid 0x2 and headphone pin (0x21) is connected to 0x0c mixer
>> that gets the DAC 0x2 output.
>>
>> So, I'm wondering if the sound doesn't work in precise due to an user
>> space bug, may be the sound settings test, which I don't know as well
>> what it does exactly, may be is confused with missing/unexpected
>> headphone playback controls, or something else. The thing is, the
>> patches just make a different DAC to be selected for the Speaker pin 0x14,
>> which doesn't explain why the headphone test starting to work successfully,
>> so may be the test is not entirely right, or confused about the different
>> mixer items (may be a pulse issue related to "interpreting" the mixer).
>>
>> So may be trying to not use the sound settings test, pluging a headphone
>> and test by using another application or by passing pulse, may give a
>> better understanding. If it's confused about the mixer, may be ok, the
>> kernel change could be acceptable, but it means we have an diverged 3.2
>> realtek/alsa codec auto config parsing structural code which we would
>> need to always have to keep eyes on, not much practical.
>
> I forgot to check if automute had something to do with the bug, since in
> theory it could mute a DAC/mixer input if considering it for an entire
> pin class (like speakers vs. HP), so changing the DAC/mixer selection
> could have an influence.
>
> Checking now on the unpatched kernel, it seems to be doing nothing wrong.
> If you plug the headphone (pin 0x21), it mutes the speaker pins (changes
> the pin control to 0), no dac/mixer is touched:
>
>> jack 0x21 1
> send: NID=0x21, VERB=0xf09(get_pin_sense), PARM=0x0
> receive: 0x80000000
> send: NID=0x21, VERB=0x707(set_pin_ctl), PARM=0xc0
> send: NID=0x14, VERB=0x707(set_pin_ctl), PARM=0x0
> send: NID=0x1a, VERB=0x707(set_pin_ctl), PARM=0x0
> CTL Notify: Headphone Jack:0, mask=1
> JACK report Headphone, status 1
> JACK report Mic, status 0
>> jack 0x21 0
> send: NID=0x21, VERB=0xf09(get_pin_sense), PARM=0x0
> receive: 0x0
> send: NID=0x21, VERB=0x707(set_pin_ctl), PARM=0xc0
> send: NID=0x14, VERB=0x707(set_pin_ctl), PARM=0x40
> send: NID=0x1a, VERB=0x707(set_pin_ctl), PARM=0x40
> CTL Notify: Headphone Jack:0, mask=1
> JACK report Headphone, status 0
> JACK report Mic, status 0
>
> I'll follow up on the bug, hope didn't miss anything.

An idea could be to collect alsa-info from both a correct and a wrong 
kernel (both with headphones plugged in), and then diff them.
diff mbox

Patch

--- codec_unpatched.txt	2012-06-28 11:32:27.922394741 -0300
+++ codec_patched.txt	2012-06-28 11:36:38.582398025 -0300
@@ -15,7 +15,7 @@  GPIO: io=2, o=0, i=0, unsolicited=1, wak
   IO[0]: enable=0, dir=0, wake=0, sticky=0, data=0, unsol=0
   IO[1]: enable=0, dir=0, wake=0, sticky=0, data=0, unsol=0
 Node 0x02 [Audio Output] wcaps 0x1d: Stereo Amp-Out
-  Control: name="Speaker Playback Volume", index=0, device=0
+  Control: name="Headphone Playback Volume", index=0, device=0
     ControlAmp: chs=3, dir=Out, idx=0, ofs=0
   Device: name="ALC269VB Analog", type="Audio", device=0
   Amp-Out caps: ofs=0x57, nsteps=0x57, stepsize=0x02, mute=0
@@ -26,8 +26,6 @@  Node 0x02 [Audio Output] wcaps 0x1d: Ste
     bits [0xe]: 16 20 24
     formats [0x1]: PCM
 Node 0x03 [Audio Output] wcaps 0x1d: Stereo Amp-Out
-  Control: name="Bass Speaker Playback Volume", index=0, device=0
-    ControlAmp: chs=3, dir=Out, idx=0, ofs=0
   Amp-Out caps: ofs=0x57, nsteps=0x57, stepsize=0x02, mute=0
   Amp-Out vals:  [0x00 0x00]
   Converter: stream=0, channel=0
@@ -116,8 +114,6 @@  Node 0x12 [Pin Complex] wcaps 0x40000b:
   Pin-ctls: 0x20: IN
 Node 0x13 [Vendor Defined Widget] wcaps 0xf00000: Mono
 Node 0x14 [Pin Complex] wcaps 0x40018d: Stereo Amp-Out
-  Control: name="Speaker Playback Switch", index=0, device=0
-    ControlAmp: chs=3, dir=Out, idx=0, ofs=0
   Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
   Amp-Out vals:  [0x00 0x00]
   Pincap 0x00010014: OUT EAPD Detect
@@ -129,7 +125,7 @@  Node 0x14 [Pin Complex] wcaps 0x40018d:
   Pin-ctls: 0x40: OUT
   Unsolicited: tag=00, enabled=0
   Connection: 2
-     0x0c* 0x0d
+     0x0c 0x0d*
 Node 0x15 [Vendor Defined Widget] wcaps 0xf00000: Mono
 Node 0x16 [Vendor Defined Widget] wcaps 0xf00000: Mono
 Node 0x17 [Pin Complex] wcaps 0x40010c: Mono Amp-Out
@@ -172,8 +168,6 @@  Node 0x19 [Pin Complex] wcaps 0x40008b:
   Pin-ctls: 0x20: IN VREF_HIZ
   Unsolicited: tag=00, enabled=0
 Node 0x1a [Pin Complex] wcaps 0x40018f: Stereo Amp-In Amp-Out
-  Control: name="Bass Speaker Playback Switch", index=0, device=0
-    ControlAmp: chs=3, dir=Out, idx=0, ofs=0
   Amp-In caps: ofs=0x00, nsteps=0x03, stepsize=0x2f, mute=0
   Amp-In vals:  [0x00 0x00]
   Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1