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

[Takashi Iwai]

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 at melix.org>

Thanks, applied now.


Takashi