[alsa-devel] imac27 12,2 (2011) model for patch_cirrus.c

[Jérémy Lal]

On 09/01/2012 16:43, Takashi Iwai wrote:
> At Mon, 09 Jan 2012 16:18:02 +0100,
> Jérémy Lal wrote:
>>
>> On 09/01/2012 15:29, Takashi Iwai wrote:
>>> At Mon, 09 Jan 2012 15:26:07 +0100,
>>> Jérémy Lal wrote:
>>>>
>>>> On 09/01/2012 14:58, Takashi Iwai wrote:
>>>>> At Mon, 09 Jan 2012 14:53:10 +0100,
>>>>> Jérémy Lal wrote:
>>>>>>
>>>>>> On 09/01/2012 11:34, Takashi Iwai wrote:
>>>>>>> At Thu, 29 Dec 2011 13:00:01 +0100,
>>>>>>> Jérémy Lal wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>> it'd be glad to have patch_cirrus.c recognize model=imac27_122,
>>>>>>>> (i'm adding a model because it's harder to auto-detect it),
>>>>>>>> please find attached patch, tested on linux 3.2-rc7.
>>>>>>>> I don't know anything about pin configs, just that it works with
>>>>>>>> the ones in the patch.
>>>>>>>> Note that i tried with model=auto (no sound) and model=imac27 (front speakers
>>>>>>>> not muted when headphones are plugged in, and surround speakers not properly
>>>>>>>> detected).
>>>>>>>
>>>>>>> Thanks for the patch.  But I see a problem in the code, i.e.
>>>>>>>
>>>>>>>> +static const struct cs_pincfg imac27_122_pincfgs[] = {
>>>>>>>> +	{ 0x00, 0x821c9700 },
>>>>>>>> +	{ 0x01, 0x011d9700 },
>>>>>>> (snip)
>>>>>>>> +	{ 0x27, 0x401f5701 },
>>>>>>>> +	{} /* terminator */
>>>>>>>> +};
>>>>>>>
>>>>>>> This looks obviously wrong.  The pincfg table should contain only the
>>>>>>> default pin-configuration values for pin widgets while you are setting
>>>>>>> some values to all widgets.  Please minimize this entry.
>>>>>>
>>>>>> It's working without overriding pin-configurations :)
>>>>>> I tested almost everything except digital input/output.
>>>>>>
>>>>>> A kernel with the following patch and
>>>>>> options snd-hda-intel model=imac27_122
>>>>>> is enough to get sound working.
>>>>>
>>>>> OK, that's much better.
>>>>> Could you give the patch with a proper changelog and your sign-off
>>>>> so that I can merge it now?
>>>>
>>>>> Also, I guess your device should be identified via PCI SSID.
>>>>> Could you check whether the device has a unique PCI SSID (check lspci
>>>>> output), and add the corresponding entry to cs420x_cfg_tbl[]?  If it's
>>>>> about 10de:xxxx, it should be unique.  If it's 8086:xxxx, then we'd
>>>>> need to check the codec SSID instead of PCI SSID.
>>>>
>>>> The PCI SSID is 8086:1c20
>>>> How can i get the codec SSID ?
>>>
>>> Check /proc/asound/card0/codec#* file.  This id should be added to
>>>   cs420x_codec_cfg_tbl[], but before the entry
>>>   SND_PCI_QUIRK_VENDOR(0x106b, "Apple", CS420X_APPLE)
>>> which matches all 106b:*.
>>
>> I get
>> Vendor Id: 0x10134206
>> Subsystem Id: 0x106b2000
>>
>> However this code does not work :
>>
>> --- linux-source-3.2-rc7.orig/sound/pci/hda/patch_cirrus.c	2011-12-24 06:51:06.000000000 +0100
>> +++ linux-source-3.2-rc7/sound/pci/hda/patch_cirrus.c	2012-01-09 15:35:14.604457152 +0100
>> @@ -1294,6 +1296,7 @@
>>  };
>>  
>>  static const struct snd_pci_quirk cs420x_codec_cfg_tbl[] = {
>> +	SND_PCI_QUIRK_VENDOR(0x106b2000, "iMac 12,2", CS420X_IMAC27_122),
> 
> You must pass
> 	SND_PCI_QUIRK(0x106b, 0x2000, "iMac 12,2", CS420X_IMAC27_122),
> instead.

Yes, thank you, this works (updated patch and description) :


ALSA: hda/cirrus - support for iMac12,2 model

This early 2011 model just need to have headphones on GPI02
instead of GPI01, and use BIOS pincfgs.
It is detected by codec SSID.
The iMac12,1 model is known to work the same way, although maybe
not with the same codec SSID.

Signed-off-by: Jérémy Lal <kapouer at melix.org>
---

--- linux-source-3.2-rc7.orig/sound/pci/hda/patch_cirrus.c	2011-12-24 06:51:06.000000000 +0100
+++ linux-source-3.2-rc7/sound/pci/hda/patch_cirrus.c	2012-01-09 16:58:13.588097367 +0100
@@ -78,6 +78,7 @@
 	CS420X_MBP53,
 	CS420X_MBP55,
 	CS420X_IMAC27,
+	CS420X_IMAC27_122,
 	CS420X_APPLE,
 	CS420X_AUTO,
 	CS420X_MODELS
@@ -1278,6 +1279,7 @@
 	[CS420X_MBP53] = "mbp53",
 	[CS420X_MBP55] = "mbp55",
 	[CS420X_IMAC27] = "imac27",
+	[CS420X_IMAC27_122] = "imac27_122",
 	[CS420X_APPLE] = "apple",
 	[CS420X_AUTO] = "auto",
 };
@@ -1294,6 +1296,7 @@
 };
 
 static const struct snd_pci_quirk cs420x_codec_cfg_tbl[] = {
+	SND_PCI_QUIRK(0x106b, 0x2000, "iMac 12,2", CS420X_IMAC27_122),
 	SND_PCI_QUIRK_VENDOR(0x106b, "Apple", CS420X_APPLE),
 	{} /* terminator */
 };
@@ -1392,6 +1395,12 @@
 		spec->gpio_eapd_speaker = 8; /* GPIO3 = speakers */
 		spec->gpio_mask = spec->gpio_dir =
 			spec->gpio_eapd_hp | spec->gpio_eapd_speaker;
+		break;
+	case CS420X_IMAC27_122:
+		spec->gpio_eapd_hp = 4; /* GPIO2 = headphones */
+		spec->gpio_eapd_speaker = 8; /* GPIO3 = speakers */
+		spec->gpio_mask = spec->gpio_dir =
+			spec->gpio_eapd_hp | spec->gpio_eapd_speaker;
 		break;
 	}