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

[Jérémy Lal]

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?
>>
>> ALSA: hda/cirrus - support for iMac12,1 and 12,2 models
>>
>> Those models just need to have headphones on GPI02
>> instead of GPI01, and use BIOS pincfgs.
>>
>> 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 12:40:53.026430475 +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",
>>  };
>> @@ -1392,6 +1394,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;
>>  	}
>>  
>>
>>  
>>> 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:*.
> 
> 
> Takashi

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
@@ -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_VENDOR(0x106b2000, "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;
 	}