lkml.org 
[lkml]   [2021]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v8 2/2] ASoC: rt715:add micmute led state control supports
Date
Hi Hans

> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: 2021年5月7日 21:18
> To: Yuan, Perry; pobrn@protonmail.com; pierre-
> louis.bossart@linux.intel.com; oder_chiou@realtek.com; perex@perex.cz;
> tiwai@suse.com; mgross@linux.intel.com
> Cc: lgirdwood@gmail.com; broonie@kernel.org; alsa-devel@alsa-project.org;
> linux-kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org;
> mario.limonciello@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@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>
>
> 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@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@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;
> > }
> >

\
 
 \ /
  Last update: 2021-05-11 11:47    [W:0.061 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site