Bug 105401

Summary: MacBookPro12,1 SPDIF "red light" always on
Product: Drivers Reporter: John Flatness (john)
Component: Sound(ALSA)Assignee: Jaroslav Kysela (perex)
Status: RESOLVED CODE_FIX    
Severity: normal CC: tiwai
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 4.2.1 Subsystem:
Regression: No Bisected commit-id:
Attachments: Apply mpb11 quirk to ssid 7b00

Description John Flatness 2015-10-02 19:42:41 UTC 
This problem seems to be one of several things that was tackled for the MacBookPro11,1 in Bug 64401.

The audio codec chip seems to still be a CS4208, though the "subsystem" ID is now 013f:

00:03.0 Audio device [0403]: Intel Corporation Broadwell-U Audio Controller [8086:160c] (rev 09)
	Subsystem: Apple Inc. Broadwell-U Audio Controller [106b:013f]

As in the previous bug, muting and/or disabling the SPDIF outputs with amixer or alsamixer has no effect on the light. Manually setting model=mbp11 for snd_hda_intel seems to have no effect either. However, the same pairs of hda-verb commands given in that bug still seem to correctly control the light:

    hda-verb /dev/snd/hwC1D0 0x0e SET_POWER_STATE 0x03 (turns the light off)
    hda-verb /dev/snd/hwC1D0 0x0e SET_POWER_STATE 0x00 (turns it on)

    hda-verb /dev/snd/hwC1D0 0x21 SET_PIN_WID 0x00 (turns it off)
    hda-verb /dev/snd/hwC1D0 0x21 SET_PIN_WID 0x40 (turns it on)

This makes me assume that the same basic quirk should still work, and it's just some small change in PCI ID or mixer controls that's to blame.
Comment 1 John Flatness 2015-10-02 20:26:37 UTC 
Actually I was incorrect in part of my summary, when I said that "manually setting model=mbp11 for snd_hda_intel seems to have no effect either." It turns out I foolishly wasn't accounting for the fact that I have two snd_hda_intel cards and this is the second, so I needed to do "model=auto,mpb11"

Once I used the correct module option, the SPDIF mixer switch on the card does correctly turn the red light on and off.

This makes me think that the only change needed here should be an addition to the subsystem PCI IDs that activate the mpb11 model in cs4208_mac_fixup_tbl to include 106b:013f:

SND_PCI_QUIRK(0x106b, 0x013f, "MacBookPro 12,1", CS4208_MBP11)
Comment 2 John Flatness 2015-10-02 21:26:12 UTC 
Of course in my haste I threw in one other wrong detail: the relevant subsystem ID for the Cirrus codec is 0x7b00, not 0x013f. With that correct ID, it's just a one-liner patch to get control of the optical light correctly attached to the SPDIF mixer control.
Comment 3 John Flatness 2015-10-02 21:27:51 UTC 
Created attachment 189351 [details]
Apply mpb11 quirk to ssid 7b00
Comment 4 Takashi Iwai 2015-10-03 08:37:40 UTC 
Thanks, I applied the patch now after arranging the fixup table (it's sorted in the id number order).
Comment 5 Takashi Iwai 2015-10-05 15:47:17 UTC 
Oh, while we're at it, could you check whether the widget power saving works with this codec?  A oneliner change like below should enable it:

--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -742,6 +742,7 @@ static int patch_cs4208(struct hda_codec *codec)
        spec->gen.automute_hook = cs_automute;
        /* exclude NID 0x10 (HP) from output volumes due to different steps */
        spec->gen.out_vol_mask = 1ULL << 0x10;
+       codec->power_save_node = 1;
                                                                                
        snd_hda_pick_fixup(codec, cs4208_models, cs4208_fixup_tbl,
                           cs4208_fixups);
Comment 6 John Flatness 2015-10-05 22:02:25 UTC 
Well, it doesn't appear to cause any problems, at least. I'm not sure I'm really able to tell whether it's actually working or did anything, though.
Comment 7 Takashi Iwai 2015-10-06 06:51:38 UTC 
That's good :)  The possible problem with this new feature would be louder click noises at power saving.  If this isn't audible, it's a good sign.

If you find anything wrong with this, let me know.  If all looks good, I'm going to enable the feature later for 4.4 kernel.  Thanks.
Comment 8 John Flatness 2015-10-06 15:21:44 UTC 
I have actually noticed one difference: the SPDIF light is _always_ off if power_save_node is enabled. Toggling the SPDIF switch on the mixer does nothing.

Since I don't actually have a mini-toslink I don't know whether that's affecting the optical out actually working, or if it would behave differently with something actually plugged in, though.
Comment 9 Takashi Iwai 2015-10-06 22:47:27 UTC 
The SPDIF should start working when you actually start playing.  Try to via aplay -Dhdmi for testing.