[PATCH v8 2/2] ASoC: rt715:add micmute led state control supports
Yuan, Perry
Perry.Yuan at dell.com
Tue May 11 11:45:07 CEST 2021
Hi Hans
> -----Original Message-----
> From: Hans de Goede <hdegoede at redhat.com>
> Sent: 2021年5月7日 21:18
> To: Yuan, Perry; pobrn at protonmail.com; pierre-
> louis.bossart at linux.intel.com; oder_chiou at realtek.com; perex at perex.cz;
> tiwai at suse.com; mgross at linux.intel.com
> Cc: lgirdwood at gmail.com; broonie at kernel.org; alsa-devel at alsa-project.org;
> linux-kernel at vger.kernel.org; platform-driver-x86 at vger.kernel.org;
> mario.limonciello at outlook.com; 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 at 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 at dell.com>
>
> 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 at redhat.com/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 at perex.cz/__;!!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;
> > }
> >
More information about the Alsa-devel
mailing list