[alsa-devel] imac27 12,2 (2011) model for patch_cirrus.c
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).
Regards, Jérémy.
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.
thanks,
Takashi
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.
--- 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; }
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.
thanks,
Takashi
--- 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;
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@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 ?
Jérémy.
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@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;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
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@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;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; }
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?
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@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;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),
You must pass SND_PCI_QUIRK(0x106b, 0x2000, "iMac 12,2", CS420X_IMAC27_122), instead.
Takashi
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;
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@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; }
At Mon, 09 Jan 2012 17:19:45 +0100, Jérémy Lal wrote:
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@melix.org
Thanks, applied now.
Takashi
participants (2)
-
Jérémy Lal -
Takashi Iwai