linux-kernel - 答复: [PATCH V2] ALSA: hda/conexant: fix Z60MR100 startup pop issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <022d01db416f$116441c0$342cc540$@senarytech.com>
Date: Thu, 28 Nov 2024 16:25:22 +0800
From: <bo.liu@...arytech.com>
To: "'Takashi Iwai'" <tiwai@...e.de>
Cc: <perex@...ex.cz>,
	<tiwai@...e.com>,
	<linux-sound@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	<feng.liu@...arytech.com>
Subject: 答复: [PATCH V2] ALSA: hda/conexant: fix Z60MR100 startup pop issue

> 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@...arytech.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