RE: [PATCH v6 2/2] ASoC: rt715:add micmute led state control supportspobrn@protonmail.com

RE: [PATCH v6 2/2] ASoC: rt715:add micmute led state control supportspobrn@protonmail.com

[Yuan, Perry]

Hi Jaroslav:


> -----Original Message-----
> From: Yuan, Perry <Perry_Yuan@Dell.com>
> Sent: Sunday, April 4, 2021 4:32 PM
> To: pobrn@protonmail.com; pierre-louis.bossart@linux.intel.com;
> oder_chiou@realtek.com; perex@perex.cz; tiwai@suse.com;
> hdegoede@redhat.com; mgross@linux.intel.com; Limonciello, Mario
> Cc: lgirdwood@gmail.com; broonie@kernel.org; alsa-devel@alsa-project.org;
> linux-kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; Yuan,
> Perry; mario.limonciello@outlook.com; Dell Client Kernel
> Subject: [PATCH v6 2/2] ASoC: rt715:add micmute led state control supports
> 
> From: Perry Yuan <perry_yuan@dell.com>
> 
> Some new Dell system is going to support audio internal micphone privacy
> setting from hardware level with micmute led state changing When micmute
> hotkey pressed by user, soft mute will need to be enabled firstly in case of
> pop noise, and codec driver need to react to mic mute event to
> EC(embedded controller) notifying that SW mute is completed Then EC will
> do the hardware mute physically within the timeout reached
> 
> This patch allow codec rt715 and rt715 sdca driver to change the local
> micmute led state. Dell privacy led trigger driver will ack EC when micmute
> key pressed through this micphone led control interface like hda_generic
> provided ACPI method defined in dell-privacy micmute led trigger will be
> called for notifying the EC that software mute has been completed, then
> hardware audio circuit solution controlled by EC will switch the audio input
> source off/on
> 
> Signed-off-by: Perry Yuan <perry_yuan@dell.com>
> 
> --------
> v5 -> v6:
> * addresed review comments from Jaroslav
> * add quirks for micmute led control as short term solution to control
>   micmute led state change
> v4 -> v5:
> * rebase to latest 5.12 rc4 upstream kernel
> v3 -> v4:
> * remove unused debug log
> * remove compile flag of DELL privacy
> * move the micmute_led to local from rt715_priv
> * when Jaroslav upstream his gerneric LED trigger driver,I will rebase
>   this patch,please consider merge this at first
>   https://lore.kernel.org/alsa-devel/20210211111400.1131020-1-
> perex@perex.cz/
> v2 -> v3:
> * simplify the patch to reuse some val value
> * add more detail to the commit info
> v1 -> v2:
> * fix some format issue
> --------
> ---
>  sound/soc/codecs/rt715-sdca.c | 41
> ++++++++++++++++++++++++++++++++++-
>  sound/soc/codecs/rt715.c      | 41
> +++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/rt715-sdca.c b/sound/soc/codecs/rt715-sdca.c
> index 20528afbdc57..7bd7ad0ba7d7 100644
> --- a/sound/soc/codecs/rt715-sdca.c
> +++ b/sound/soc/codecs/rt715-sdca.c
> @@ -11,12 +11,14 @@
>  #include <linux/moduleparam.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> +#include <linux/leds.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pm.h>
>  #include <linux/soundwire/sdw.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/platform_device.h>
> +#include <linux/dmi.h>
>  #include <sound/core.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm_params.h>
> @@ -344,6 +346,34 @@ static int rt715_sdca_get_volsw(struct snd_kcontrol
> *kcontrol,
>  	return 0;
>  }
> 
> +static bool micmute_led_set;
> +static int  dmi_matched(const struct dmi_system_id *dmi) {
> +	micmute_led_set = 1;
> +	return 1;
> +}
> +
> +/* Some systems will need to use this to trigger mic mute LED state
> +changed */ static const struct dmi_system_id micmute_led_dmi_table[] = {
> +	{
> +		.callback = dmi_matched,
> +		.ident = "Dell Latitude 9420",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude
> 9420"),
> +		},
> +	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "Dell Latitude 9520",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude
> 9520"),
> +		},
> +	},
> +	{},
> +};
> +
>  static int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
>  	struct snd_ctl_elem_value *ucontrol)
>  {
> @@ -358,6 +388,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol
> *kcontrol,
>  	unsigned int mask = (1 << fls(max)) - 1;
>  	unsigned int invert = p->invert;
>  	int err;
> +	bool micmute_led;
> 
>  	for (i = 0; i < 4; i++) {
>  		if (ucontrol->value.integer.value[i] != rt715-
> >kctl_switch_orig[i]) { @@ -393,7 +424,15 @@ static int
> rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
>  		if (err < 0)
>  			return err;
>  	}
> -
> +	/* Micmute LED state changed by muted/unmute switch */
> +	dmi_check_system(micmute_led_dmi_table);
> +	if (invert && micmute_led_set) {
> +		if (ucontrol->value.integer.value[0] || ucontrol-
> >value.integer.value[1])
> +			micmute_led = LED_OFF;
> +		else
> +			micmute_led = LED_ON;
> +		ledtrig_audio_set(LED_AUDIO_MICMUTE, micmute_led);
> +	}
>  	return k_changed;
>  }
> 
> diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c index
> 34c3357e943b..490cf9e63c2d 100644
> --- a/sound/soc/codecs/rt715.c
> +++ b/sound/soc/codecs/rt715.c
> @@ -13,6 +13,7 @@
>  #include <linux/init.h>
>  #include <linux/delay.h>
>  #include <linux/i2c.h>
> +#include <linux/leds.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pm.h>
>  #include <linux/soundwire/sdw.h>
> @@ -25,6 +26,7 @@
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_device.h>
> +#include <linux/dmi.h>
>  #include <sound/core.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm_params.h>
> @@ -70,6 +72,34 @@ static void rt715_get_gain(struct rt715_priv *rt715,
> unsigned int addr_h,
>  		pr_err("Failed to get L channel gain.\n");  }
> 
> +static bool micmute_led_set;
> +static int  dmi_matched(const struct dmi_system_id *dmi) {
> +	micmute_led_set = 1;
> +	return 1;
> +}
> +
> +/* Some systems will need to use this to trigger mic mute LED state
> +changed */ static const struct dmi_system_id micmute_led_dmi_table[] = {
> +	{
> +		.callback = dmi_matched,
> +		.ident = "Dell Latitude 9420",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude
> 9420"),
> +		},
> +	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "Dell Latitude 9520",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude
> 9520"),
> +		},
> +	},
> +	{},
> +};
> +
>  /* For Verb-Set Amplifier Gain (Verb ID = 3h) */  static int
> rt715_set_amp_gain_put(struct snd_kcontrol *kcontrol,
>  					struct snd_ctl_elem_value *ucontrol)
> @@ -88,6 +118,7 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol
> *kcontrol,
>  		RT715_SET_GAIN_MIX_ADC2_L};
>  	unsigned int addr_h, addr_l, val_h, val_ll, val_lr;
>  	unsigned int read_ll, read_rl, i, j, loop_cnt;
> +	bool micmute_led;
> 
>  	if (strstr(ucontrol->id.name, "Main Capture Switch") ||
>  		strstr(ucontrol->id.name, "Main Capture Volume")) @@ -
> 185,6 +216,16 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol
> *kcontrol,
>  	if (dapm->bias_level <= SND_SOC_BIAS_STANDBY)
>  		regmap_write(rt715->regmap,
>  				RT715_SET_AUDIO_POWER_STATE,
> AC_PWRST_D3);
> +
> +	/* Micmute LED state changed by muted/unmute switch */
> +	dmi_check_system(micmute_led_dmi_table);
> +	if (mc->invert && micmute_led_set) {
> +		if (ucontrol->value.integer.value[0] || ucontrol-
> >value.integer.value[1])
> +			micmute_led = LED_OFF;
> +		else
> +			micmute_led = LED_ON;
> +		ledtrig_audio_set(LED_AUDIO_MICMUTE, micmute_led);
> +	}
>  	return 0;
>  }
> 
> --
> 2.25.1
Could you help to review if the patch is acceptable as short term solution?
I added dmi checking when changing the led state for rt715 and rt715-sdca codec.
Thank you!
Perry