From: Perry Yuan <[email protected]>
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 notifying that SW mute is completed. Then EC will do the
HW mute physically.
This patch allow codec rt715 driver to ack EC when micmute key pressed
through this micphone led control interface like hda_generic provided.
Signed-off-by: Perry Yuan <[email protected]>
Signed-off-by: Limonciello Mario <[email protected]>
---
sound/soc/codecs/rt715-sdca.c | 19 ++++++++++++++++++-
sound/soc/codecs/rt715-sdca.h | 1 +
sound/soc/codecs/rt715.c | 19 ++++++++++++++++++-
sound/soc/codecs/rt715.h | 1 +
4 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/rt715-sdca.c b/sound/soc/codecs/rt715-sdca.c
index b43ac8559e45..e168ef6efcf5 100644
--- a/sound/soc/codecs/rt715-sdca.c
+++ b/sound/soc/codecs/rt715-sdca.c
@@ -12,6 +12,7 @@
#include <linux/version.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>
@@ -268,6 +269,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
unsigned int reg2 = mc->rreg;
unsigned int reg = mc->reg;
unsigned int max = mc->max;
+ unsigned int val0, val1;
int err;
val = ucontrol->value.integer.value[0];
@@ -286,7 +288,22 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
if (err < 0)
return err;
}
-
+#if IS_ENABLED(CONFIG_DELL_PRIVACY)
+ /* Privacy LED Trigger State Changed by muted/unmute switch */
+ if (mc->invert) {
+ val0 = ucontrol->value.integer.value[0];
+ val1 = ucontrol->value.integer.value[1];
+ if (val0 == 1 && val1 == 1) {
+ rt715->micmute_led = LED_OFF;
+ ledtrig_audio_set(LED_AUDIO_MICMUTE,
+ rt715->micmute_led ? LED_ON : LED_OFF);
+ } else if (val0 == 0 && val1 == 0) {
+ rt715->micmute_led = LED_ON;
+ ledtrig_audio_set(LED_AUDIO_MICMUTE,
+ rt715->micmute_led ? LED_ON : LED_OFF);
+ }
+ }
+#endif
return 0;
}
diff --git a/sound/soc/codecs/rt715-sdca.h b/sound/soc/codecs/rt715-sdca.h
index 840c237895dd..2ab8724ae50b 100644
--- a/sound/soc/codecs/rt715-sdca.h
+++ b/sound/soc/codecs/rt715-sdca.h
@@ -31,6 +31,7 @@ struct rt715_sdca_priv {
int l_is_unmute;
int r_is_unmute;
int hw_sdw_ver;
+ int micmute_led;
};
struct rt715_sdw_stream_data {
diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c
index cdcba70146da..faa4dee6b39a 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>
@@ -88,13 +89,29 @@ 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;
+ unsigned int val0, val1;
if (strstr(ucontrol->id.name, "Main Capture Switch") ||
strstr(ucontrol->id.name, "Main Capture Volume"))
loop_cnt = 4;
else
loop_cnt = 1;
-
+#if IS_ENABLED(CONFIG_DELL_PRIVACY)
+ /* Privacy micmute led trigger for muted/unmute switch */
+ if (mc->invert) {
+ val0 = ucontrol->value.integer.value[0];
+ val1 = ucontrol->value.integer.value[1];
+ if (val0 == 1 && val1 == 1) {
+ rt715->micmute_led = LED_OFF;
+ ledtrig_audio_set(LED_AUDIO_MICMUTE,
+ rt715->micmute_led ? LED_ON : LED_OFF);
+ } else if (val0 == 0 && val1 == 0) {
+ rt715->micmute_led = LED_ON;
+ ledtrig_audio_set(LED_AUDIO_MICMUTE,
+ rt715->micmute_led ? LED_ON : LED_OFF);
+ }
+ }
+#endif
for (j = 0; j < loop_cnt; j++) {
/* Can't use update bit function, so read the original value first */
if (loop_cnt == 1) {
diff --git a/sound/soc/codecs/rt715.h b/sound/soc/codecs/rt715.h
index 009a8266f606..2d3b5b299514 100644
--- a/sound/soc/codecs/rt715.h
+++ b/sound/soc/codecs/rt715.h
@@ -22,6 +22,7 @@ struct rt715_priv {
struct sdw_bus_params params;
bool hw_init;
bool first_hw_init;
+ int micmute_led;
};
struct sdw_stream_data {
--
2.25.1
On Mon, Dec 28, 2020 at 09:38:31PM +0800, Perry Yuan wrote:
> From: Perry Yuan <[email protected]>
>
> Some new Dell system is going to support audio internal micphone
> privacy setting from hardware level with micmute led state changing
I'm missing patch 1 of this series - what's the story with dependencies
and so on?
> -----Original Message-----
> From: Mark Brown <[email protected]>
> Sent: Tuesday, December 29, 2020 8:41 PM
> To: Yuan, Perry
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Limonciello, Mario
> Subject: Re: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support
>
> On Mon, Dec 28, 2020 at 09:38:31PM +0800, Perry Yuan wrote:
> > From: Perry Yuan <[email protected]>
> >
> > Some new Dell system is going to support audio internal micphone
> > privacy setting from hardware level with micmute led state changing
>
> I'm missing patch 1 of this series - what's the story with dependencies and so
> on?
HI Mark:
Sorry to make this confused.
The two patches are not in the same module, I use different cc list for each patch.
I will send another v3 looping all the cc list and some new improvement.
> -----Original Message-----
> From: Yuan, Perry <[email protected]>
> Sent: Monday, January 4, 2021 3:10
> To: Mark Brown
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Limonciello, Mario
> Subject: RE: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support
>
> > -----Original Message-----
> > From: Mark Brown <[email protected]>
> > Sent: Tuesday, December 29, 2020 8:41 PM
> > To: Yuan, Perry
> > Cc: [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; linux-
> > [email protected]; Limonciello, Mario
> > Subject: Re: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support
> >
> > On Mon, Dec 28, 2020 at 09:38:31PM +0800, Perry Yuan wrote:
> > > From: Perry Yuan <[email protected]>
> > >
> > > Some new Dell system is going to support audio internal micphone
> > > privacy setting from hardware level with micmute led state changing
> >
> > I'm missing patch 1 of this series - what's the story with dependencies and
> so
> > on?
>
> HI Mark:
> Sorry to make this confused.
> The two patches are not in the same module, I use different cc list for each
> patch.
> I will send another v3 looping all the cc list and some new improvement.
>
Since the patches span subsystems and have a dependency, once reviewers are happy I
think we'll have to get an agreement of which subsystem to bring them through. So
yes, please keep all CC list the same for both patches for now when you submit v3.
> @@ -268,6 +269,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
> unsigned int reg2 = mc->rreg;
> unsigned int reg = mc->reg;
> unsigned int max = mc->max;
> + unsigned int val0, val1;
> int err;
>
> val = ucontrol->value.integer.value[0];
> @@ -286,7 +288,22 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
> if (err < 0)
> return err;
> }
> -
> +#if IS_ENABLED(CONFIG_DELL_PRIVACY)
> + /* Privacy LED Trigger State Changed by muted/unmute switch */
> + if (mc->invert) {
> + val0 = ucontrol->value.integer.value[0];
> + val1 = ucontrol->value.integer.value[1];
> + if (val0 == 1 && val1 == 1) {
> + rt715->micmute_led = LED_OFF;
> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> + rt715->micmute_led ? LED_ON : LED_OFF);
> + } else if (val0 == 0 && val1 == 0) {
> + rt715->micmute_led = LED_ON;
> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> + rt715->micmute_led ? LED_ON : LED_OFF);
> + }
> + }
> +#endif
Should this be activated for specific DMI quirks? This driver is used in
non-Dell platforms (I am thinking of Intel RVPs or Realtek
daughterboards), I am not sure if a build-time behavior change makes sense.
Or conversely could we just set the LEDs unconditionally if doing so is
harmless?
Hi
> -----Original Message-----
> From: Pierre-Louis Bossart <[email protected]>
> Sent: 2021年1月12日 2:07
> To: Yuan, Perry; [email protected]; [email protected]; [email protected]
> Cc: [email protected]; Limonciello, Mario; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support
>
>
> [EXTERNAL EMAIL]
>
>
>
>
> > @@ -268,6 +269,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol
> *kcontrol,
> > unsigned int reg2 = mc->rreg;
> > unsigned int reg = mc->reg;
> > unsigned int max = mc->max;
> > + unsigned int val0, val1;
> > int err;
> >
> > val = ucontrol->value.integer.value[0]; @@ -286,7 +288,22 @@ static
> > int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
> > if (err < 0)
> > return err;
> > }
> > -
> > +#if IS_ENABLED(CONFIG_DELL_PRIVACY)
> > + /* Privacy LED Trigger State Changed by muted/unmute switch */
> > + if (mc->invert) {
> > + val0 = ucontrol->value.integer.value[0];
> > + val1 = ucontrol->value.integer.value[1];
> > + if (val0 == 1 && val1 == 1) {
> > + rt715->micmute_led = LED_OFF;
> > + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> > + rt715->micmute_led ? LED_ON :
> LED_OFF);
> > + } else if (val0 == 0 && val1 == 0) {
> > + rt715->micmute_led = LED_ON;
> > + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> > + rt715->micmute_led ? LED_ON :
> LED_OFF);
> > + }
> > + }
> > +#endif
>
> Should this be activated for specific DMI quirks? This driver is used in non-Dell
> platforms (I am thinking of Intel RVPs or Realtek daughterboards), I am not
> sure if a build-time behavior change makes sense.
>
> Or conversely could we just set the LEDs unconditionally if doing so is
> harmless?
The current mic mute led setting path is not common used for other vendors, just Dell platform
support this mic mute led set operation.
Do you think that I need to add one DMI quirk in the next version ?
If so, I can add that.
Perry Yuan
Dell | Client Software Group | CDC Linux OS
> > -----Original Message-----
> > From: Pierre-Louis Bossart <[email protected]>
> > Sent: 2021年1月12日 2:07
> > To: Yuan, Perry; [email protected]; [email protected]; [email protected]
> > Cc: [email protected]; Limonciello, Mario; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support
> >
> >
> > [EXTERNAL EMAIL]
> >
> >
> >
> >
> > > @@ -268,6 +269,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol
> > *kcontrol,
> > > unsigned int reg2 = mc->rreg;
> > > unsigned int reg = mc->reg;
> > > unsigned int max = mc->max;
> > > + unsigned int val0, val1;
> > > int err;
> > >
> > > val = ucontrol->value.integer.value[0]; @@ -286,7 +288,22 @@ static
> > > int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
> > > if (err < 0)
> > > return err;
> > > }
> > > -
> > > +#if IS_ENABLED(CONFIG_DELL_PRIVACY)
> > > + /* Privacy LED Trigger State Changed by muted/unmute switch */
> > > + if (mc->invert) {
> > > + val0 = ucontrol->value.integer.value[0];
> > > + val1 = ucontrol->value.integer.value[1];
> > > + if (val0 == 1 && val1 == 1) {
> > > + rt715->micmute_led = LED_OFF;
> > > + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> > > + rt715->micmute_led ? LED_ON :
> > LED_OFF);
> > > + } else if (val0 == 0 && val1 == 0) {
> > > + rt715->micmute_led = LED_ON;
> > > + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> > > + rt715->micmute_led ? LED_ON :
> > LED_OFF);
> > > + }
> > > + }
> > > +#endif
> >
> > Should this be activated for specific DMI quirks? This driver is used in
> non-Dell
> > platforms (I am thinking of Intel RVPs or Realtek daughterboards), I am not
> > sure if a build-time behavior change makes sense.
> >
> > Or conversely could we just set the LEDs unconditionally if doing so is
> > harmless?
>
> The current mic mute led setting path is not common used for other vendors,
> just Dell platform
> support this mic mute led set operation.
>
> Do you think that I need to add one DMI quirk in the next version ?
> If so, I can add that.
>
>
In the HDA audio case this is modeled off of, the code runs whether or not a
vendor has support for a mic mute LED. The calls to ledtrig_audio_set should
be a no-op. I agree with @Pierre-Louis Bossart in this case, we should just
be running it whether or not dell-privacy is compiled in. If another vendor
chooses to add LED support they'll just need to set up their ledtrig supported
backend and not change codec code.
> -----Original Message-----
> From: Limonciello, Mario <[email protected]>
> Sent: 2021年1月12日 22:48
> To: Yuan, Perry; Pierre-Louis Bossart; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: RE: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support
>
> > > -----Original Message-----
> > > From: Pierre-Louis Bossart <[email protected]>
> > > Sent: 2021年1月12日 2:07
> > > To: Yuan, Perry; [email protected]; [email protected];
> > > [email protected]
> > > Cc: [email protected]; Limonciello, Mario;
> > > [email protected]; [email protected];
> > > [email protected]
> > > Subject: Re: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control
> > > support
> > >
> > >
> > > [EXTERNAL EMAIL]
> > >
> > >
> > >
> > >
> > > > @@ -268,6 +269,7 @@ static int rt715_sdca_put_volsw(struct
> > > > snd_kcontrol
> > > *kcontrol,
> > > > unsigned int reg2 = mc->rreg;
> > > > unsigned int reg = mc->reg;
> > > > unsigned int max = mc->max;
> > > > + unsigned int val0, val1;
> > > > int err;
> > > >
> > > > val = ucontrol->value.integer.value[0]; @@ -286,7 +288,22 @@
> > > > static int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol,
> > > > if (err < 0)
> > > > return err;
> > > > }
> > > > -
> > > > +#if IS_ENABLED(CONFIG_DELL_PRIVACY)
> > > > + /* Privacy LED Trigger State Changed by muted/unmute switch */
> > > > + if (mc->invert) {
> > > > + val0 = ucontrol->value.integer.value[0];
> > > > + val1 = ucontrol->value.integer.value[1];
> > > > + if (val0 == 1 && val1 == 1) {
> > > > + rt715->micmute_led = LED_OFF;
> > > > + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> > > > + rt715->micmute_led ? LED_ON :
> > > LED_OFF);
> > > > + } else if (val0 == 0 && val1 == 0) {
> > > > + rt715->micmute_led = LED_ON;
> > > > + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> > > > + rt715->micmute_led ? LED_ON :
> > > LED_OFF);
> > > > + }
> > > > + }
> > > > +#endif
> > >
> > > Should this be activated for specific DMI quirks? This driver is
> > > used in
> > non-Dell
> > > platforms (I am thinking of Intel RVPs or Realtek daughterboards), I
> > > am not sure if a build-time behavior change makes sense.
> > >
> > > Or conversely could we just set the LEDs unconditionally if doing so
> > > is harmless?
> >
> > The current mic mute led setting path is not common used for other
> > vendors, just Dell platform support this mic mute led set operation.
> >
> > Do you think that I need to add one DMI quirk in the next version ?
> > If so, I can add that.
> >
> >
>
>
> In the HDA audio case this is modeled off of, the code runs whether or not a
> vendor has support for a mic mute LED. The calls to ledtrig_audio_set should
> be a no-op. I agree with @Pierre-Louis Bossart in this case, we should just be
> running it whether or not dell-privacy is compiled in. If another vendor
> chooses to add LED support they'll just need to set up their ledtrig supported
> backend and not change codec code.
Hi @Pierre-Louis Bossart
Seems like that we have two way to go.
* DMI quirks,seems like that it needs to maintain the quirk list when vendors have new system to support.
* We just set the mic mute led state unconditionally .
Which way would you prefer for next patch review?
>>>>> +#if IS_ENABLED(CONFIG_DELL_PRIVACY)
>>>>> + /* Privacy LED Trigger State Changed by muted/unmute switch */
>>>>> + if (mc->invert) {
>>>>> + val0 = ucontrol->value.integer.value[0];
>>>>> + val1 = ucontrol->value.integer.value[1];
>>>>> + if (val0 == 1 && val1 == 1) {
>>>>> + rt715->micmute_led = LED_OFF;
>>>>> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
>>>>> + rt715->micmute_led ? LED_ON :
>>>> LED_OFF);
>>>>> + } else if (val0 == 0 && val1 == 0) {
>>>>> + rt715->micmute_led = LED_ON;
>>>>> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
>>>>> + rt715->micmute_led ? LED_ON :
>>>> LED_OFF);
>>>>> + }
>>>>> + }
>>>>> +#endif
>>>>
>>>> Should this be activated for specific DMI quirks? This driver is
>>>> used in
>>> non-Dell
>>>> platforms (I am thinking of Intel RVPs or Realtek daughterboards), I
>>>> am not sure if a build-time behavior change makes sense.
>>>>
>>>> Or conversely could we just set the LEDs unconditionally if doing so
>>>> is harmless?
>>>
>>> The current mic mute led setting path is not common used for other
>>> vendors, just Dell platform support this mic mute led set operation.
>>>
>>> Do you think that I need to add one DMI quirk in the next version ?
>>> If so, I can add that.
>>>
>>>
>>
>>
>> In the HDA audio case this is modeled off of, the code runs whether or not a
>> vendor has support for a mic mute LED. The calls to ledtrig_audio_set should
>> be a no-op. I agree with @Pierre-Louis Bossart in this case, we should just be
>> running it whether or not dell-privacy is compiled in. If another vendor
>> chooses to add LED support they'll just need to set up their ledtrig supported
>> backend and not change codec code.
>
> Hi @Pierre-Louis Bossart
> Seems like that we have two way to go.
> * DMI quirks,seems like that it needs to maintain the quirk list when vendors have new system to support.
> * We just set the mic mute led state unconditionally .
>
> Which way would you prefer for next patch review?
Maintaining quirks is a hassle, it's much simpler and consistent with
HDaudio if the leds are set unconditionally. Thanks!
> -----Original Message-----
> From: Pierre-Louis Bossart <[email protected]>
> Sent: 2021年1月13日 10:42
> To: Yuan, Perry; Limonciello, Mario; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support
>
>
> [EXTERNAL EMAIL]
>
>
> >>>>> +#if IS_ENABLED(CONFIG_DELL_PRIVACY)
> >>>>> + /* Privacy LED Trigger State Changed by muted/unmute switch */
> >>>>> + if (mc->invert) {
> >>>>> + val0 = ucontrol->value.integer.value[0];
> >>>>> + val1 = ucontrol->value.integer.value[1];
> >>>>> + if (val0 == 1 && val1 == 1) {
> >>>>> + rt715->micmute_led = LED_OFF;
> >>>>> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> >>>>> + rt715->micmute_led ? LED_ON :
> >>>> LED_OFF);
> >>>>> + } else if (val0 == 0 && val1 == 0) {
> >>>>> + rt715->micmute_led = LED_ON;
> >>>>> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> >>>>> + rt715->micmute_led ? LED_ON :
> >>>> LED_OFF);
> >>>>> + }
> >>>>> + }
> >>>>> +#endif
> >>>>
> >>>> Should this be activated for specific DMI quirks? This driver is
> >>>> used in
> >>> non-Dell
> >>>> platforms (I am thinking of Intel RVPs or Realtek daughterboards),
> >>>> I am not sure if a build-time behavior change makes sense.
> >>>>
> >>>> Or conversely could we just set the LEDs unconditionally if doing
> >>>> so is harmless?
> >>>
> >>> The current mic mute led setting path is not common used for other
> >>> vendors, just Dell platform support this mic mute led set operation.
> >>>
> >>> Do you think that I need to add one DMI quirk in the next version ?
> >>> If so, I can add that.
> >>>
> >>>
> >>
> >>
> >> In the HDA audio case this is modeled off of, the code runs whether
> >> or not a vendor has support for a mic mute LED. The calls to
> >> ledtrig_audio_set should be a no-op. I agree with @Pierre-Louis
> >> Bossart in this case, we should just be running it whether or not
> >> dell-privacy is compiled in. If another vendor chooses to add LED
> >> support they'll just need to set up their ledtrig supported backend and not
> change codec code.
> >
> > Hi @Pierre-Louis Bossart
> > Seems like that we have two way to go.
> > * DMI quirks,seems like that it needs to maintain the quirk list when vendors
> have new system to support.
> > * We just set the mic mute led state unconditionally .
> >
> > Which way would you prefer for next patch review?
>
> Maintaining quirks is a hassle, it's much simpler and consistent with HDaudio
> if the leds are set unconditionally. Thanks!
Thank you for your confirm.
I will take this to next patch V4.
Perry Yuan
Dell | Client Software Group | CDC Linux OS