| Message ID | s5h8uj71lci.wl-tiwai@suse.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0f32fd1900e6b972f289416dbd75e92772b630cb |
| Headers | show |
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Wednesday, November 19, 2014 7:28 PM > To: Kailang > Cc: (alsa-devel@alsa-project.org) > Subject: Re: HP mute led for ALC286 > > At Wed, 19 Nov 2014 07:13:16 +0000, > Kailang wrote: > > > > Hi Takashi, > > > > I had sent the discussion mail about the DMI string for HP. > > But no body response. > > So, I just send the patch to fix this issue. > > I'd like to avoid the duplicated open codes, so could you try > the patch below and rewrite your patch to follow this? You'd > just need to set like: > spec->gpio_mute_led_mask = 0x02; > spec->gpio_mic_led_mask = 0x20; > pec->mute_led_polarity = 0; > then you can use the existing alc_fixup_gpio_mute_hook and > alc_fixup_gpio_mic_mute_hook. I modified as attach patch. ^^ > > > thanks, > > Takashi > > -- 8< -- > From: Takashi Iwai <tiwai@suse.de> > Subject: [PATCH] ALSA: hda/realtek - Clean up mute/mic GPIO > LED handling > > There are a few duplicated codes handling the mute and > mic-mute LEDs via GPIO pins. Let's consolidate to single > helpers. Here we introduced two new fields to alc_spec, > gpio_mute_led_mask and gpio_mic_led_mask, to contain the bit > mask to set/clear. Also, mute_led_polarity is evaluated as well. > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > sound/pci/hda/patch_realtek.c | 81 > +++++++++++++++++++++---------------------- > 1 file changed, 40 insertions(+), 41 deletions(-) > > diff --git a/sound/pci/hda/patch_realtek.c > b/sound/pci/hda/patch_realtek.c index > 1af917f58a70..3c29a558e7db 100644 > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -96,6 +96,8 @@ struct alc_spec { > hda_nid_t cap_mute_led_nid; > > unsigned int gpio_led; /* used for alc269_fixup_hp_gpio_led() */ > + unsigned int gpio_mute_led_mask; > + unsigned int gpio_mic_led_mask; > > hda_nid_t headset_mic_pin; > hda_nid_t headphone_mic_pin; > @@ -3235,41 +3237,45 @@ static void > alc269_fixup_hp_mute_led_mic2(struct hda_codec *codec, > } > } > > -/* turn on/off mute LED per vmaster hook */ -static void > alc269_fixup_hp_gpio_mute_hook(void *private_data, int enabled) > +/* update LED status via GPIO */ > +static void alc_update_gpio_led(struct hda_codec *codec, > unsigned int mask, > + bool enabled) > { > - struct hda_codec *codec = private_data; > struct alc_spec *spec = codec->spec; > unsigned int oldval = spec->gpio_led; > > + if (spec->mute_led_polarity) > + enabled = !enabled; > + > if (enabled) > - spec->gpio_led &= ~0x08; > + spec->gpio_led &= ~mask; > else > - spec->gpio_led |= 0x08; > + spec->gpio_led |= mask; > if (spec->gpio_led != oldval) > snd_hda_codec_write(codec, 0x01, 0, > AC_VERB_SET_GPIO_DATA, > spec->gpio_led); > } > > -/* turn on/off mic-mute LED per capture hook */ -static void > alc269_fixup_hp_gpio_mic_mute_hook(struct hda_codec *codec, > - struct > snd_kcontrol *kcontrol, > - struct > snd_ctl_elem_value *ucontrol) > +/* turn on/off mute LED via GPIO per vmaster hook */ static void > +alc_fixup_gpio_mute_hook(void *private_data, int enabled) > { > + struct hda_codec *codec = private_data; > struct alc_spec *spec = codec->spec; > - unsigned int oldval = spec->gpio_led; > > - if (!ucontrol) > - return; > + alc_update_gpio_led(codec, spec->gpio_mute_led_mask, enabled); } > > - if (ucontrol->value.integer.value[0] || > - ucontrol->value.integer.value[1]) > - spec->gpio_led &= ~0x10; > - else > - spec->gpio_led |= 0x10; > - if (spec->gpio_led != oldval) > - snd_hda_codec_write(codec, 0x01, 0, > AC_VERB_SET_GPIO_DATA, > - spec->gpio_led); > +/* turn on/off mic-mute LED via GPIO per capture hook */ static void > +alc_fixup_gpio_mic_mute_hook(struct hda_codec *codec, > + struct snd_kcontrol *kcontrol, > + struct > snd_ctl_elem_value *ucontrol) { > + struct alc_spec *spec = codec->spec; > + > + if (ucontrol) > + alc_update_gpio_led(codec, spec->gpio_mic_led_mask, > + ucontrol->value.integer.value[0] || > + ucontrol->value.integer.value[1]); > } > > static void alc269_fixup_hp_gpio_led(struct hda_codec > *codec, @@ -3283,9 +3289,12 @@ static void > alc269_fixup_hp_gpio_led(struct hda_codec *codec, > }; > > if (action == HDA_FIXUP_ACT_PRE_PROBE) { > - spec->gen.vmaster_mute.hook = > alc269_fixup_hp_gpio_mute_hook; > - spec->gen.cap_sync_hook = > alc269_fixup_hp_gpio_mic_mute_hook; > + spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook; > + spec->gen.cap_sync_hook = alc_fixup_gpio_mic_mute_hook; > spec->gpio_led = 0; > + spec->mute_led_polarity = 0; > + spec->gpio_mute_led_mask = 0x08; > + spec->gpio_mic_led_mask = 0x10; > snd_hda_add_verbs(codec, gpio_init); > } > } > @@ -3327,9 +3336,11 @@ static void > alc269_fixup_hp_gpio_mic1_led(struct hda_codec *codec, > }; > > if (action == HDA_FIXUP_ACT_PRE_PROBE) { > - spec->gen.vmaster_mute.hook = > alc269_fixup_hp_gpio_mute_hook; > + spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook; > spec->gen.cap_sync_hook = > alc269_fixup_hp_cap_mic_mute_hook; > spec->gpio_led = 0; > + spec->mute_led_polarity = 0; > + spec->gpio_mute_led_mask = 0x08; > spec->cap_mute_led_nid = 0x18; > snd_hda_add_verbs(codec, gpio_init); > codec->power_filter = led_power_filter; @@ > -3348,9 +3359,11 @@ static void alc280_fixup_hp_gpio4(struct > hda_codec *codec, > }; > > if (action == HDA_FIXUP_ACT_PRE_PROBE) { > - spec->gen.vmaster_mute.hook = > alc269_fixup_hp_gpio_mute_hook; > + spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook; > spec->gen.cap_sync_hook = > alc269_fixup_hp_cap_mic_mute_hook; > spec->gpio_led = 0; > + spec->mute_led_polarity = 0; > + spec->gpio_mute_led_mask = 0x08; > spec->cap_mute_led_nid = 0x18; > snd_hda_add_verbs(codec, gpio_init); > codec->power_filter = led_power_filter; @@ > -5624,22 +5637,6 @@ static void alc_fixup_bass_chmap(struct > hda_codec *codec, > } > } > > -/* turn on/off mute LED per vmaster hook */ -static void > alc662_led_gpio1_mute_hook(void *private_data, int enabled) -{ > - struct hda_codec *codec = private_data; > - struct alc_spec *spec = codec->spec; > - unsigned int oldval = spec->gpio_led; > - > - if (enabled) > - spec->gpio_led |= 0x01; > - else > - spec->gpio_led &= ~0x01; > - if (spec->gpio_led != oldval) > - snd_hda_codec_write(codec, 0x01, 0, > AC_VERB_SET_GPIO_DATA, > - spec->gpio_led); > -} > - > /* avoid D3 for keeping GPIO up */ > static unsigned int gpio_led_power_filter(struct hda_codec *codec, > hda_nid_t nid, > @@ -5662,8 +5659,10 @@ static void > alc662_fixup_led_gpio1(struct hda_codec *codec, > }; > > if (action == HDA_FIXUP_ACT_PRE_PROBE) { > - spec->gen.vmaster_mute.hook = > alc662_led_gpio1_mute_hook; > + spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook; > spec->gpio_led = 0; > + spec->mute_led_polarity = 1; > + spec->gpio_mute_led_mask = 0x01; > snd_hda_add_verbs(codec, gpio_init); > codec->power_filter = gpio_led_power_filter; > } > -- > 2.1.3 > > > ------Please consider the environment before printing this e-mail. >
At Thu, 20 Nov 2014 07:04:01 +0000, Kailang wrote: > > > > > -----Original Message----- > > From: Takashi Iwai [mailto:tiwai@suse.de] > > Sent: Wednesday, November 19, 2014 7:28 PM > > To: Kailang > > Cc: (alsa-devel@alsa-project.org) > > Subject: Re: HP mute led for ALC286 > > > > At Wed, 19 Nov 2014 07:13:16 +0000, > > Kailang wrote: > > > > > > Hi Takashi, > > > > > > I had sent the discussion mail about the DMI string for HP. > > > But no body response. > > > So, I just send the patch to fix this issue. > > > > I'd like to avoid the duplicated open codes, so could you try > > the patch below and rewrite your patch to follow this? You'd > > just need to set like: > > spec->gpio_mute_led_mask = 0x02; > > spec->gpio_mic_led_mask = 0x20; > > pec->mute_led_polarity = 0; > > then you can use the existing alc_fixup_gpio_mute_hook and > > alc_fixup_gpio_mic_mute_hook. > > I modified as attach patch. ^^ Oh, no, I meant an additional patch on top of my previous patch, not folding into that one. For your ease, I applied my patch to for-next branch now, so that you can rewrite the patch to be applicable to there. thanks, Takashi
Hi Takashi, I'm sorry for that. Maybe I had not do git pull. BR, Kailang > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Thursday, November 20, 2014 6:06 PM > To: Kailang > Cc: (alsa-devel@alsa-project.org) > Subject: Re: HP mute led for ALC286 > > At Thu, 20 Nov 2014 07:04:01 +0000, > Kailang wrote: > > > > > > > > > -----Original Message----- > > > From: Takashi Iwai [mailto:tiwai@suse.de] > > > Sent: Wednesday, November 19, 2014 7:28 PM > > > To: Kailang > > > Cc: (alsa-devel@alsa-project.org) > > > Subject: Re: HP mute led for ALC286 > > > > > > At Wed, 19 Nov 2014 07:13:16 +0000, > > > Kailang wrote: > > > > > > > > Hi Takashi, > > > > > > > > I had sent the discussion mail about the DMI string for HP. > > > > But no body response. > > > > So, I just send the patch to fix this issue. > > > > > > I'd like to avoid the duplicated open codes, so could you try the > > > patch below and rewrite your patch to follow this? You'd > just need > > > to set like: > > > spec->gpio_mute_led_mask = 0x02; > > > spec->gpio_mic_led_mask = 0x20; > > > pec->mute_led_polarity = 0; > > > then you can use the existing alc_fixup_gpio_mute_hook and > > > alc_fixup_gpio_mic_mute_hook. > > > > I modified as attach patch. ^^ > > Oh, no, I meant an additional patch on top of my previous > patch, not folding into that one. > > For your ease, I applied my patch to for-next branch now, so > that you can rewrite the patch to be applicable to there. > > > thanks, > > Takashi > > ------Please consider the environment before printing this e-mail. >
At Fri, 21 Nov 2014 07:56:29 +0000, Kailang wrote: > > Hi Takashi, > > I'm sorry for that. Maybe I had not do git pull. Applied now, thanks. Takashi > > BR, > Kailang > > > -----Original Message----- > > From: Takashi Iwai [mailto:tiwai@suse.de] > > Sent: Thursday, November 20, 2014 6:06 PM > > To: Kailang > > Cc: (alsa-devel@alsa-project.org) > > Subject: Re: HP mute led for ALC286 > > > > At Thu, 20 Nov 2014 07:04:01 +0000, > > Kailang wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: Takashi Iwai [mailto:tiwai@suse.de] > > > > Sent: Wednesday, November 19, 2014 7:28 PM > > > > To: Kailang > > > > Cc: (alsa-devel@alsa-project.org) > > > > Subject: Re: HP mute led for ALC286 > > > > > > > > At Wed, 19 Nov 2014 07:13:16 +0000, > > > > Kailang wrote: > > > > > > > > > > Hi Takashi, > > > > > > > > > > I had sent the discussion mail about the DMI string for HP. > > > > > But no body response. > > > > > So, I just send the patch to fix this issue. > > > > > > > > I'd like to avoid the duplicated open codes, so could you try the > > > > patch below and rewrite your patch to follow this? You'd > > just need > > > > to set like: > > > > spec->gpio_mute_led_mask = 0x02; > > > > spec->gpio_mic_led_mask = 0x20; > > > > pec->mute_led_polarity = 0; > > > > then you can use the existing alc_fixup_gpio_mute_hook and > > > > alc_fixup_gpio_mic_mute_hook. > > > > > > I modified as attach patch. ^^ > > > > Oh, no, I meant an additional patch on top of my previous > > patch, not folding into that one. > > > > For your ease, I applied my patch to for-next branch now, so > > that you can rewrite the patch to be applicable to there. > > > > > > thanks, > > > > Takashi > > > > ------Please consider the environment before printing this e-mail. > > > [2 0002-supported-HP-mute-led-alc286.patch <application/octet-stream (base64)>] >
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 1af917f58a70..3c29a558e7db 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -96,6 +96,8 @@ struct alc_spec { hda_nid_t cap_mute_led_nid; unsigned int gpio_led; /* used for alc269_fixup_hp_gpio_led() */ + unsigned int gpio_mute_led_mask; + unsigned int gpio_mic_led_mask; hda_nid_t headset_mic_pin; hda_nid_t headphone_mic_pin; @@ -3235,41 +3237,45 @@ static void alc269_fixup_hp_mute_led_mic2(struct hda_codec *codec, } } -/* turn on/off mute LED per vmaster hook */ -static void alc269_fixup_hp_gpio_mute_hook(void *private_data, int enabled) +/* update LED status via GPIO */ +static void alc_update_gpio_led(struct hda_codec *codec, unsigned int mask, + bool enabled) { - struct hda_codec *codec = private_data; struct alc_spec *spec = codec->spec; unsigned int oldval = spec->gpio_led; + if (spec->mute_led_polarity) + enabled = !enabled; + if (enabled) - spec->gpio_led &= ~0x08; + spec->gpio_led &= ~mask; else - spec->gpio_led |= 0x08; + spec->gpio_led |= mask; if (spec->gpio_led != oldval) snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA, spec->gpio_led); } -/* turn on/off mic-mute LED per capture hook */ -static void alc269_fixup_hp_gpio_mic_mute_hook(struct hda_codec *codec, - struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) +/* turn on/off mute LED via GPIO per vmaster hook */ +static void alc_fixup_gpio_mute_hook(void *private_data, int enabled) { + struct hda_codec *codec = private_data; struct alc_spec *spec = codec->spec; - unsigned int oldval = spec->gpio_led; - if (!ucontrol) - return; + alc_update_gpio_led(codec, spec->gpio_mute_led_mask, enabled); +} - if (ucontrol->value.integer.value[0] || - ucontrol->value.integer.value[1]) - spec->gpio_led &= ~0x10; - else - spec->gpio_led |= 0x10; - if (spec->gpio_led != oldval) - snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA, - spec->gpio_led); +/* turn on/off mic-mute LED via GPIO per capture hook */ +static void alc_fixup_gpio_mic_mute_hook(struct hda_codec *codec, + struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct alc_spec *spec = codec->spec; + + if (ucontrol) + alc_update_gpio_led(codec, spec->gpio_mic_led_mask, + ucontrol->value.integer.value[0] || + ucontrol->value.integer.value[1]); } static void alc269_fixup_hp_gpio_led(struct hda_codec *codec, @@ -3283,9 +3289,12 @@ static void alc269_fixup_hp_gpio_led(struct hda_codec *codec, }; if (action == HDA_FIXUP_ACT_PRE_PROBE) { - spec->gen.vmaster_mute.hook = alc269_fixup_hp_gpio_mute_hook; - spec->gen.cap_sync_hook = alc269_fixup_hp_gpio_mic_mute_hook; + spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook; + spec->gen.cap_sync_hook = alc_fixup_gpio_mic_mute_hook; spec->gpio_led = 0; + spec->mute_led_polarity = 0; + spec->gpio_mute_led_mask = 0x08; + spec->gpio_mic_led_mask = 0x10; snd_hda_add_verbs(codec, gpio_init); } } @@ -3327,9 +3336,11 @@ static void alc269_fixup_hp_gpio_mic1_led(struct hda_codec *codec, }; if (action == HDA_FIXUP_ACT_PRE_PROBE) { - spec->gen.vmaster_mute.hook = alc269_fixup_hp_gpio_mute_hook; + spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook; spec->gen.cap_sync_hook = alc269_fixup_hp_cap_mic_mute_hook; spec->gpio_led = 0; + spec->mute_led_polarity = 0; + spec->gpio_mute_led_mask = 0x08; spec->cap_mute_led_nid = 0x18; snd_hda_add_verbs(codec, gpio_init); codec->power_filter = led_power_filter; @@ -3348,9 +3359,11 @@ static void alc280_fixup_hp_gpio4(struct hda_codec *codec, }; if (action == HDA_FIXUP_ACT_PRE_PROBE) { - spec->gen.vmaster_mute.hook = alc269_fixup_hp_gpio_mute_hook; + spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook; spec->gen.cap_sync_hook = alc269_fixup_hp_cap_mic_mute_hook; spec->gpio_led = 0; + spec->mute_led_polarity = 0; + spec->gpio_mute_led_mask = 0x08; spec->cap_mute_led_nid = 0x18; snd_hda_add_verbs(codec, gpio_init); codec->power_filter = led_power_filter; @@ -5624,22 +5637,6 @@ static void alc_fixup_bass_chmap(struct hda_codec *codec, } } -/* turn on/off mute LED per vmaster hook */ -static void alc662_led_gpio1_mute_hook(void *private_data, int enabled) -{ - struct hda_codec *codec = private_data; - struct alc_spec *spec = codec->spec; - unsigned int oldval = spec->gpio_led; - - if (enabled) - spec->gpio_led |= 0x01; - else - spec->gpio_led &= ~0x01; - if (spec->gpio_led != oldval) - snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA, - spec->gpio_led); -} - /* avoid D3 for keeping GPIO up */ static unsigned int gpio_led_power_filter(struct hda_codec *codec, hda_nid_t nid, @@ -5662,8 +5659,10 @@ static void alc662_fixup_led_gpio1(struct hda_codec *codec, }; if (action == HDA_FIXUP_ACT_PRE_PROBE) { - spec->gen.vmaster_mute.hook = alc662_led_gpio1_mute_hook; + spec->gen.vmaster_mute.hook = alc_fixup_gpio_mute_hook; spec->gpio_led = 0; + spec->mute_led_polarity = 1; + spec->gpio_mute_led_mask = 0x01; snd_hda_add_verbs(codec, gpio_init); codec->power_filter = gpio_led_power_filter; }