| Message ID | 20241128060812.433362-1-bo.liu@senarytech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] ALSA: hda/conexant: fix Z60MR100 startup pop issue | expand |
On Thu, 28 Nov 2024 07:08:12 +0100, bo liu wrote: > > When Z60MR100 startup, speaker will output a pop. To fix this issue, > we mute codec by init verbs in bios when system startup, and set GPIO > to low to unmute codec in codec driver when it loaded . > > Signed-off-by: bo liu <bo.liu@senarytech.com> > --- > sound/pci/hda/patch_conexant.c | 35 ++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c > index e851785ff058..62c53e64bcad 100644 > --- a/sound/pci/hda/patch_conexant.c > +++ b/sound/pci/hda/patch_conexant.c > @@ -42,6 +42,7 @@ struct conexant_spec { > unsigned int gpio_led; > unsigned int gpio_mute_led_mask; > unsigned int gpio_mic_led_mask; > + unsigned int gpio_unmute_bit_mask; > bool is_cx8070_sn6140; > }; > > @@ -308,6 +309,7 @@ enum { > CXT_FIXUP_HP_MIC_NO_PRESENCE, > CXT_PINCFG_SWS_JS201D, > CXT_PINCFG_TOP_SPEAKER, > + CXT_FIXUP_HP_A_U, > }; > > /* for hda_fixup_thinkpad_acpi() */ > @@ -762,6 +764,24 @@ static void cxt_setup_mute_led(struct hda_codec *codec, > } > } > > +static void cxt_setup_mute_gpio_and_unmute(struct hda_codec *codec, > + unsigned int gpio_mute_mask) > +{ > + struct conexant_spec *spec = codec->spec; > + > + if (gpio_mute_mask) { > + spec->gpio_unmute_bit_mask = gpio_mute_mask; Any reason to store this in spec? As far as I see the code below, it's used only locally here, so it doesn't have to be stored there. > + > + //set gpio data to 0. Put a space after "//". > + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA, 0); > + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_MASK, > + spec->gpio_unmute_bit_mask); > + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DIRECTION, > + spec->gpio_unmute_bit_mask); > + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_STICKY_MASK, 0); > + } > +} > + > static void cxt_fixup_mute_led_gpio(struct hda_codec *codec, > const struct hda_fixup *fix, int action) > { > @@ -776,6 +796,15 @@ static void cxt_fixup_hp_zbook_mute_led(struct hda_codec *codec, > cxt_setup_mute_led(codec, 0x10, 0x20); > } > > +static void cxt_fixup_hp_a_u(struct hda_codec *codec, > + const struct hda_fixup *fix, int action) > +{ > + //Init vers in BIOS mute the spk/hp by set gpio high to avoid pop noise, > + //so need to unmute once by clearing the gpio data when runs into the system. Ditto. thanks, Takashi
> On Thu, 28 Nov 2024 07:08:12 +0100, > bo liu wrote: > > > > When Z60MR100 startup, speaker will output a pop. To fix this issue, > > we mute codec by init verbs in bios when system startup, and set GPIO > > to low to unmute codec in codec driver when it loaded . > > > > Signed-off-by: bo liu <bo.liu@senarytech.com> > > --- > > sound/pci/hda/patch_conexant.c | 35 > > ++++++++++++++++++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > > > diff --git a/sound/pci/hda/patch_conexant.c > > b/sound/pci/hda/patch_conexant.c index e851785ff058..62c53e64bcad > > 100644 > > --- a/sound/pci/hda/patch_conexant.c > > +++ b/sound/pci/hda/patch_conexant.c > > @@ -42,6 +42,7 @@ struct conexant_spec { > > unsigned int gpio_led; > > unsigned int gpio_mute_led_mask; > > unsigned int gpio_mic_led_mask; > > + unsigned int gpio_unmute_bit_mask; > > bool is_cx8070_sn6140; > > }; > > > > @@ -308,6 +309,7 @@ enum { > > CXT_FIXUP_HP_MIC_NO_PRESENCE, > > CXT_PINCFG_SWS_JS201D, > > CXT_PINCFG_TOP_SPEAKER, > > + CXT_FIXUP_HP_A_U, > > }; > > > > /* for hda_fixup_thinkpad_acpi() */ > > @@ -762,6 +764,24 @@ static void cxt_setup_mute_led(struct hda_codec *codec, > > } > > } > > > > +static void cxt_setup_mute_gpio_and_unmute(struct hda_codec *codec, > > + unsigned int gpio_mute_mask) > > +{ > > + struct conexant_spec *spec = codec->spec; > > + > > + if (gpio_mute_mask) { > > + spec->gpio_unmute_bit_mask = gpio_mute_mask; > > Any reason to store this in spec? As far as I see the code below, it's used only locally here, so it doesn't have to be stored there. The purpose of adding this is to differentiate the configuration of the LED_MUTE GPIO and to retain debugging information. Of course, this field can be removed if desired. > > + > > + //set gpio data to 0. > Put a space after "//". OK. > > + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA, 0); > > + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_MASK, > > + spec->gpio_unmute_bit_mask); > > + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DIRECTION, > > + spec->gpio_unmute_bit_mask); > > + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_STICKY_MASK, 0); > > + } > > +} > > + > > static void cxt_fixup_mute_led_gpio(struct hda_codec *codec, > > const struct hda_fixup *fix, int action) { @@ -776,6 +796,15 @@ > > static void cxt_fixup_hp_zbook_mute_led(struct hda_codec *codec, > > cxt_setup_mute_led(codec, 0x10, 0x20); } > > > > +static void cxt_fixup_hp_a_u(struct hda_codec *codec, > > + const struct hda_fixup *fix, int action) { > > + //Init vers in BIOS mute the spk/hp by set gpio high to avoid pop noise, > > + //so need to unmute once by clearing the gpio data when runs into the system. > Ditto. thanks, Takashi
On Thu, 28 Nov 2024 09:25:22 +0100, <bo.liu@senarytech.com> wrote: > > > On Thu, 28 Nov 2024 07:08:12 +0100, > > bo liu wrote: > > > > > > When Z60MR100 startup, speaker will output a pop. To fix this issue, > > > we mute codec by init verbs in bios when system startup, and set GPIO > > > to low to unmute codec in codec driver when it loaded . > > > > > > Signed-off-by: bo liu <bo.liu@senarytech.com> > > > --- > > > sound/pci/hda/patch_conexant.c | 35 > > > ++++++++++++++++++++++++++++++++++ > > > 1 file changed, 35 insertions(+) > > > > > > diff --git a/sound/pci/hda/patch_conexant.c > > > b/sound/pci/hda/patch_conexant.c index e851785ff058..62c53e64bcad > > > 100644 > > > --- a/sound/pci/hda/patch_conexant.c > > > +++ b/sound/pci/hda/patch_conexant.c > > > @@ -42,6 +42,7 @@ struct conexant_spec { > > > unsigned int gpio_led; > > > unsigned int gpio_mute_led_mask; > > > unsigned int gpio_mic_led_mask; > > > + unsigned int gpio_unmute_bit_mask; > > > bool is_cx8070_sn6140; > > > }; > > > > > > @@ -308,6 +309,7 @@ enum { > > > CXT_FIXUP_HP_MIC_NO_PRESENCE, > > > CXT_PINCFG_SWS_JS201D, > > > CXT_PINCFG_TOP_SPEAKER, > > > + CXT_FIXUP_HP_A_U, > > > }; > > > > > > /* for hda_fixup_thinkpad_acpi() */ > > > @@ -762,6 +764,24 @@ static void cxt_setup_mute_led(struct hda_codec > *codec, > > > } > > > } > > > > > > +static void cxt_setup_mute_gpio_and_unmute(struct hda_codec *codec, > > > + unsigned int gpio_mute_mask) > > > +{ > > > + struct conexant_spec *spec = codec->spec; > > > + > > > + if (gpio_mute_mask) { > > > + spec->gpio_unmute_bit_mask = gpio_mute_mask; > > > > Any reason to store this in spec? As far as I see the code below, it's > used only locally here, so it doesn't have to be stored there. > > The purpose of adding this is to differentiate the configuration of the > LED_MUTE GPIO and to retain debugging information. Of course, this field can > be removed if desired. Yes, please remove. Takashi
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index e851785ff058..62c53e64bcad 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -42,6 +42,7 @@ struct conexant_spec { unsigned int gpio_led; unsigned int gpio_mute_led_mask; unsigned int gpio_mic_led_mask; + unsigned int gpio_unmute_bit_mask; bool is_cx8070_sn6140; }; @@ -308,6 +309,7 @@ enum { CXT_FIXUP_HP_MIC_NO_PRESENCE, CXT_PINCFG_SWS_JS201D, CXT_PINCFG_TOP_SPEAKER, + CXT_FIXUP_HP_A_U, }; /* for hda_fixup_thinkpad_acpi() */ @@ -762,6 +764,24 @@ static void cxt_setup_mute_led(struct hda_codec *codec, } } +static void cxt_setup_mute_gpio_and_unmute(struct hda_codec *codec, + unsigned int gpio_mute_mask) +{ + struct conexant_spec *spec = codec->spec; + + if (gpio_mute_mask) { + spec->gpio_unmute_bit_mask = gpio_mute_mask; + + //set gpio data to 0. + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA, 0); + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_MASK, + spec->gpio_unmute_bit_mask); + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DIRECTION, + spec->gpio_unmute_bit_mask); + snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_STICKY_MASK, 0); + } +} + static void cxt_fixup_mute_led_gpio(struct hda_codec *codec, const struct hda_fixup *fix, int action) { @@ -776,6 +796,15 @@ static void cxt_fixup_hp_zbook_mute_led(struct hda_codec *codec, cxt_setup_mute_led(codec, 0x10, 0x20); } +static void cxt_fixup_hp_a_u(struct hda_codec *codec, + const struct hda_fixup *fix, int action) +{ + //Init vers in BIOS mute the spk/hp by set gpio high to avoid pop noise, + //so need to unmute once by clearing the gpio data when runs into the system. + if (action == HDA_FIXUP_ACT_INIT) + cxt_setup_mute_gpio_and_unmute(codec, 0x2); +} + /* ThinkPad X200 & co with cxt5051 */ static const struct hda_pintbl cxt_pincfg_lenovo_x200[] = { { 0x16, 0x042140ff }, /* HP (seq# overridden) */ @@ -982,6 +1011,10 @@ static const struct hda_fixup cxt_fixups[] = { { } }, }, + [CXT_FIXUP_HP_A_U] = { + .type = HDA_FIXUP_FUNC, + .v.func = cxt_fixup_hp_a_u, + }, }; static const struct snd_pci_quirk cxt5045_fixups[] = { @@ -1055,6 +1088,7 @@ static const struct snd_pci_quirk cxt5066_fixups[] = { SND_PCI_QUIRK(0x103c, 0x8457, "HP Z2 G4 mini", CXT_FIXUP_HP_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x103c, 0x8458, "HP Z2 G4 mini premium", CXT_FIXUP_HP_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1043, 0x138d, "Asus", CXT_FIXUP_HEADPHONE_MIC_PIN), + SND_PCI_QUIRK(0x14f1, 0x0252, "MBX-Z60MR100", CXT_FIXUP_HP_A_U), SND_PCI_QUIRK(0x14f1, 0x0265, "SWS JS201D", CXT_PINCFG_SWS_JS201D), SND_PCI_QUIRK(0x152d, 0x0833, "OLPC XO-1.5", CXT_FIXUP_OLPC_XO), SND_PCI_QUIRK(0x17aa, 0x20f2, "Lenovo T400", CXT_PINCFG_LENOVO_TP410), @@ -1100,6 +1134,7 @@ static const struct hda_model_fixup cxt5066_fixup_models[] = { { .id = CXT_PINCFG_LENOVO_NOTEBOOK, .name = "lenovo-20149" }, { .id = CXT_PINCFG_SWS_JS201D, .name = "sws-js201d" }, { .id = CXT_PINCFG_TOP_SPEAKER, .name = "sirius-top-speaker" }, + { .id = CXT_FIXUP_HP_A_U, .name = "HP-U-support" }, {} };
When Z60MR100 startup, speaker will output a pop. To fix this issue, we mute codec by init verbs in bios when system startup, and set GPIO to low to unmute codec in codec driver when it loaded . Signed-off-by: bo liu <bo.liu@senarytech.com> --- sound/pci/hda/patch_conexant.c | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)