Patchwork [Quantal,1/1] Revert "ALSA: hda/realtek - Call alc_auto_parse_customize_define() always after fixup"

login
register
mail settings
Submitter Joseph Salisbury
Date Sept. 26, 2012, 4:17 p.m.
Message ID <9a9344c855b09e830185dfa4c36d8d41b1765620.1347983033.git.joseph.salisbury@canonical.com>
Download mbox | patch
Permalink /patch/187122/
State New
Headers show

Comments

Joseph Salisbury - Sept. 26, 2012, 4:17 p.m.
From: Joseph Salisbury <joseph.salisbury@canonical.com>

This reverts commit af741c150f66db8d1da6f82ac75e2571f7f1dd38.

BugLink:  http://bugs.launchpad.net/bugs/1006690
Backport required to apply this commit to Quantal.

Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
---
 sound/pci/hda/patch_realtek.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
Herton Ronaldo Krzesinski - Sept. 26, 2012, 7:19 p.m.
On Wed, Sep 26, 2012 at 12:17:36PM -0400, joseph.salisbury@canonical.com wrote:
> From: Joseph Salisbury <joseph.salisbury@canonical.com>
> 
> This reverts commit af741c150f66db8d1da6f82ac75e2571f7f1dd38.
> 
> BugLink:  http://bugs.launchpad.net/bugs/1006690
> Backport required to apply this commit to Quantal.
> 
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
> ---
>  sound/pci/hda/patch_realtek.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 

The real problem with commit af741c150f66db8d1da6f82ac75e2571f7f1dd38 is
that alc269 for example has this check:

        if (codec->vendor_id == 0x10ec0269) {
                spec->codec_variant = ALC269_TYPE_ALC269VA;
                switch (alc_get_coef0(codec) & 0x00f0) {
                case 0x0010:
                        if (codec->bus->pci->subsystem_vendor == 0x1025 &&
                            spec->cdefine.platform_type == 1)
                                err = alc_codec_rename(codec, "ALC271X");
                        spec->codec_variant = ALC269_TYPE_ALC269VB;
                        break;

When alc_auto_parse_customize_define was moved after this,
spec->cdefine.platform_type will always be zero, since it's filled by
alc_auto_parse_customize_define. So it prevents alc271_fixup_dmic to
being run, and from that I expect the mic recording failure.

A similar issue happens with alc662. But the patch was not at all wrong,
we need to run alc_auto_parse_customize_define after
alc_apply_fixup(codec, ALC_FIXUP_ACT_PRE_PROBE) so we can properly take
ALC_FIXUP_SKU_IGNORE quirks into account. So I'm not acking the revert.

Chicken and egg problem... I'll try to cook up a patch to fix the
problem.
Joseph Salisbury - Sept. 26, 2012, 7:24 p.m.
On 09/26/2012 03:19 PM, Herton Ronaldo Krzesinski wrote:
> On Wed, Sep 26, 2012 at 12:17:36PM -0400, joseph.salisbury@canonical.com wrote:
>> From: Joseph Salisbury<joseph.salisbury@canonical.com>
>>
>> This reverts commit af741c150f66db8d1da6f82ac75e2571f7f1dd38.
>>
>> BugLink:  http://bugs.launchpad.net/bugs/1006690
>> Backport required to apply this commit to Quantal.
>>
>> Signed-off-by: Joseph Salisbury<joseph.salisbury@canonical.com>
>> ---
>>   sound/pci/hda/patch_realtek.c |   10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
> The real problem with commit af741c150f66db8d1da6f82ac75e2571f7f1dd38 is
> that alc269 for example has this check:
>
>          if (codec->vendor_id == 0x10ec0269) {
>                  spec->codec_variant = ALC269_TYPE_ALC269VA;
>                  switch (alc_get_coef0(codec)&  0x00f0) {
>                  case 0x0010:
>                          if (codec->bus->pci->subsystem_vendor == 0x1025&&
>                              spec->cdefine.platform_type == 1)
>                                  err = alc_codec_rename(codec, "ALC271X");
>                          spec->codec_variant = ALC269_TYPE_ALC269VB;
>                          break;
>
> When alc_auto_parse_customize_define was moved after this,
> spec->cdefine.platform_type will always be zero, since it's filled by
> alc_auto_parse_customize_define. So it prevents alc271_fixup_dmic to
> being run, and from that I expect the mic recording failure.
>
> A similar issue happens with alc662. But the patch was not at all wrong,
> we need to run alc_auto_parse_customize_define after
> alc_apply_fixup(codec, ALC_FIXUP_ACT_PRE_PROBE) so we can properly take
> ALC_FIXUP_SKU_IGNORE quirks into account. So I'm not acking the revert.
>
> Chicken and egg problem... I'll try to cook up a patch to fix the
> problem.
Thanks, Herton.

>

Patch

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index c4c4b01..62b2206 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5596,13 +5596,13 @@  static int patch_alc262(struct hda_codec *codec)
 	snd_hda_codec_write(codec, 0x1a, 0, AC_VERB_SET_PROC_COEF, tmp | 0x80);
 	}
 #endif
+	alc_auto_parse_customize_define(codec);
+
 	alc_fix_pll_init(codec, 0x20, 0x0a, 10);
 
 	alc_pick_fixup(codec, NULL, alc262_fixup_tbl, alc262_fixups);
 	alc_apply_fixup(codec, ALC_FIXUP_ACT_PRE_PROBE);
 
-	alc_auto_parse_customize_define(codec);
-
 	/* automatic parse from the BIOS config */
 	err = alc262_parse_auto_config(codec);
 	if (err < 0)
@@ -6267,6 +6267,8 @@  static int patch_alc269(struct hda_codec *codec)
 	struct alc_spec *spec;
 	int err;
 
+	alc_auto_parse_customize_define(codec);
+	
 	err = alc_alloc_spec(codec, 0x0b);
 	if (err < 0)
 		return err;
@@ -6304,8 +6306,6 @@  static int patch_alc269(struct hda_codec *codec)
 		       alc269_fixup_tbl, alc269_fixups);
 	alc_apply_fixup(codec, ALC_FIXUP_ACT_PRE_PROBE);
 
-	alc_auto_parse_customize_define(codec);
-
 	/* automatic parse from the BIOS config */
 	err = alc269_parse_auto_config(codec);
 	if (err < 0)
@@ -6917,6 +6917,8 @@  static int patch_alc662(struct hda_codec *codec)
 	/* handle multiple HPs as is */
 	spec->parse_flags = HDA_PINCFG_NO_HP_FIXUP;
 
+	alc_auto_parse_customize_define(codec);
+
 	alc_fix_pll_init(codec, 0x20, 0x04, 15);
 
 	spec->init_hook = alc662_fill_coef;