Hi Hans > -----Original Message----- > From: Hans de Goede <hdegoede@xxxxxxxxxx> > Sent: 2021年5月7日 21:18 > To: Yuan, Perry; pobrn@xxxxxxxxxxxxxx; pierre- > louis.bossart@xxxxxxxxxxxxxxx; oder_chiou@xxxxxxxxxxx; perex@xxxxxxxx; > tiwai@xxxxxxxx; mgross@xxxxxxxxxxxxxxx > Cc: lgirdwood@xxxxxxxxx; broonie@xxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx; > mario.limonciello@xxxxxxxxxxx; Dell Client Kernel > Subject: Re: [PATCH v8 2/2] ASoC: rt715:add micmute led state control > supports > > > [EXTERNAL EMAIL] > > Hi Perry, > > On 5/6/21 1:56 PM, Perry Yuan wrote: > > From: Perry Yuan <perry_yuan@xxxxxxxx> > > > > 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@xxxxxxxx> > > NACK, as explained before we want the binding of the control to the LED- > trigger to be done from the UCM profile. > > Support for this has landed kernel-side in Linux tree now (this will be part of > 5.13-rc1). Together with the latest git alsa-lib and alsa-utils code, you can now > do what this patch does from an UCM profile file and AFAIK that is the > preferred way to do this. > > See here for an example UCM profile patch implementing this: > > https://urldefense.com/v3/__https://lore.kernel.org/alsa- > devel/20210507131139.10231-3- > hdegoede@xxxxxxxxxx/T/*u__;Iw!!LpKI!zM9rVPYaTjLEYUmBrIayi3aj_QvB5p1 > u2luEckfQCUeVbnRPk7DL8hwdhIDLjEc$ [lore[.]kernel[.]org] > > Note that if you test this under Fedora you will hit a selinux denial, to > workaround that you can put selinux in permissive mode. This selinux issue is > being tracked here: > > https://urldefense.com/v3/__https://bugzilla.redhat.com/show_bug.cgi?id=1 > 958210__;!!LpKI!zM9rVPYaTjLEYUmBrIayi3aj_QvB5p1u2luEckfQCUeVbnRPk7 > DL8hwd_fmxo0I$ [bugzilla[.]redhat[.]com] > > Regards, > > Hans Thanks for the feedback a lot. I am trying to make one new patch for the ucm2 profile Could you help to check if there is some other changes I need to make for the privacy driver ? Perry > > > > > > > > > > -------- > > v7 -> v8: > > * N/A > > v6 -> v7: > > * addresed review comments from Jaroslav > > * use device id in the quirk list > > v5 -> v6: > > * add quirks for micmute led control as short term solution to control > > micmute led state change > > * add comments for the invert checking > > 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://urldefense.com/v3/__https://lore.kernel.org/alsa-devel/2021021 > > 1111400.1131020-1- > perex@xxxxxxxx/__;!!LpKI!zM9rVPYaTjLEYUmBrIayi3aj_Qv > > B5p1u2luEckfQCUeVbnRPk7DL8hwdy240518$ [lore[.]kernel[.]org] > > 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 | 42 > +++++++++++++++++++++++++++++++++++ > > sound/soc/codecs/rt715.c | 42 > +++++++++++++++++++++++++++++++++++ > > 2 files changed, 84 insertions(+) > > > > diff --git a/sound/soc/codecs/rt715-sdca.c > > b/sound/soc/codecs/rt715-sdca.c index 936e3061ca1e..de46514e0207 > > 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,32 @@ 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, > > + .matches = { > > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > > + DMI_MATCH(DMI_PRODUCT_SKU, "0A32"), > > + }, > > + }, > > + { > > + .callback = dmi_matched, > > + .matches = { > > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > > + DMI_MATCH(DMI_PRODUCT_SKU, "0A3E"), > > + }, > > + }, > > + {}, > > +}; > > + > > static int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol, > > struct snd_ctl_elem_value *ucontrol) { @@ -358,6 +386,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]) > > { @@ -394,6 +423,18 @@ static int rt715_sdca_put_volsw(struct > snd_kcontrol *kcontrol, > > return err; > > } > > > > + /* Micmute LED state changed by muted/unmute switch > > + * to keep syncing with actual hardware mic mute led state > > + * invert will be checked to change the state switch > > + */ > > + 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; > > } > > > > @@ -1069,6 +1110,7 @@ int rt715_sdca_io_init(struct device *dev, > > struct sdw_slave *slave) > > > > pm_runtime_mark_last_busy(&slave->dev); > > pm_runtime_put_autosuspend(&slave->dev); > > + dmi_check_system(micmute_led_dmi_table); > > > > return 0; > > } > > diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c index > > 1352869cc086..4dbd870009b8 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,32 @@ 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, > > + .matches = { > > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > > + DMI_MATCH(DMI_PRODUCT_SKU, "0A32"), > > + }, > > + }, > > + { > > + .callback = dmi_matched, > > + .matches = { > > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > > + DMI_MATCH(DMI_PRODUCT_SKU, "0A3E"), > > + }, > > + }, > > + {}, > > +}; > > + > > /* 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) > @@ -83,6 +111,7 @@ static > > int rt715_set_amp_gain_put(struct snd_kcontrol *kcontrol, > > unsigned int addr_h, addr_l, val_h, val_ll, val_lr; > > unsigned int read_ll, read_rl, i; > > unsigned int k_vol_changed = 0; > > + bool micmute_led; > > > > for (i = 0; i < 2; i++) { > > if (ucontrol->value.integer.value[i] != rt715- > >kctl_2ch_vol_ori[i]) > > { @@ -155,6 +184,18 @@ static int rt715_set_amp_gain_put(struct > snd_kcontrol *kcontrol, > > break; > > } > > > > + /* Micmute LED state changed by muted/unmute switch > > + * to keep syncing with actual hardware mic mute led state > > + * invert will be checked to change the state switch > > + */ > > + if (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); > > + } > > + > > /* D0:power on state, D3: power saving mode */ > > if (dapm->bias_level <= SND_SOC_BIAS_STANDBY) > > regmap_write(rt715->regmap, > > @@ -1088,6 +1129,7 @@ int rt715_io_init(struct device *dev, struct > > sdw_slave *slave) > > > > pm_runtime_mark_last_busy(&slave->dev); > > pm_runtime_put_autosuspend(&slave->dev); > > + dmi_check_system(micmute_led_dmi_table); > > > > return 0; > > } > >
