diff mbox series

ASoC: rt5682: remove jack detect delay

Message ID 20210217214914.700751-1-cujomalainey@chromium.org
State New
Headers show
Series ASoC: rt5682: remove jack detect delay | expand

Commit Message

Curtis Malainey Feb. 17, 2021, 9:49 p.m. UTC
There is a 250ms delay on the jack detect interrupt currently, this
delay is observable to users who are using inline controls. It can also
mask multiple presses which is a negative experience.

Cc: Bard liao <yung-chuan.liao@linux.intel.com>
Cc: Shuming Fan <shumingf@realtek.com>

Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5682-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Takashi Iwai Feb. 18, 2021, 8:44 a.m. UTC | #1
On Thu, 18 Feb 2021 09:38:53 +0100,
Shuming [范書銘] <shumingf@realtek.com> wrote:
> 
> > There is a 250ms delay on the jack detect interrupt currently, this delay is
> > observable to users who are using inline controls. It can also mask multiple
> > presses which is a negative experience.
> > 
> > Cc: Bard liao <yung-chuan.liao@linux.intel.com>
> > Cc: Shuming Fan <shumingf@realtek.com>
> > 
> > Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
> > ---
> >  sound/soc/codecs/rt5682-i2c.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/sound/soc/codecs/rt5682-i2c.c b/sound/soc/codecs/rt5682-i2c.c
> > index 93c1603b42f1..b15c3e7d1f59 100644
> > --- a/sound/soc/codecs/rt5682-i2c.c
> > +++ b/sound/soc/codecs/rt5682-i2c.c
> > @@ -78,7 +78,7 @@ static irqreturn_t rt5682_irq(int irq, void *data)
> >  	struct rt5682_priv *rt5682 = data;
> > 
> >  	mod_delayed_work(system_power_efficient_wq,
> > -		&rt5682->jack_detect_work, msecs_to_jiffies(250));
> > +		&rt5682->jack_detect_work, 0);
> 
> How about using the device property to adjust the delay time?
> I think it should keep the workqueue to do the jack/button detection because the jack type detection will take some times to do.

One might check twice (or more) if it's not certain, too.  That is,
check the jack immediately, and if the jack state really changed,
report it so.  OTOH, if the jack state doesn't change from the old
state, it can retry after some delay.


thanks,

Takashi
Curtis Malainey Feb. 18, 2021, 9:06 p.m. UTC | #2
Thanks Takashi and Shuming


On Thu, Feb 18, 2021 at 12:44 AM Takashi Iwai <tiwai@suse.de> wrote:

> On Thu, 18 Feb 2021 09:38:53 +0100,
> Shuming [范書銘] <shumingf@realtek.com> wrote:
> >
> > > There is a 250ms delay on the jack detect interrupt currently, this
> delay is
> > > observable to users who are using inline controls. It can also mask
> multiple
> > > presses which is a negative experience.
> > >
> > > Cc: Bard liao <yung-chuan.liao@linux.intel.com>
> > > Cc: Shuming Fan <shumingf@realtek.com>
> > >
> > > Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
> > > ---
> > >  sound/soc/codecs/rt5682-i2c.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/sound/soc/codecs/rt5682-i2c.c
> b/sound/soc/codecs/rt5682-i2c.c
> > > index 93c1603b42f1..b15c3e7d1f59 100644
> > > --- a/sound/soc/codecs/rt5682-i2c.c
> > > +++ b/sound/soc/codecs/rt5682-i2c.c
> > > @@ -78,7 +78,7 @@ static irqreturn_t rt5682_irq(int irq, void *data)
> > >     struct rt5682_priv *rt5682 = data;
> > >
> > >     mod_delayed_work(system_power_efficient_wq,
> > > -           &rt5682->jack_detect_work, msecs_to_jiffies(250));
> > > +           &rt5682->jack_detect_work, 0);
> >
> > How about using the device property to adjust the delay time?
> > I think it should keep the workqueue to do the jack/button detection
> because the jack type detection will take some times to do.
>

I am trying to understand the purpose of this delay currently, won't
the press already be registered since we have an interrupt? Or does it need
to stabilize? The reason is 250ms is well within human perception or even
double tap time, which results in users possibly double tapping buttons but
only seeing one press come through.


>
> One might check twice (or more) if it's not certain, too.  That is,
> check the jack immediately, and if the jack state really changed,
> report it so.  OTOH, if the jack state doesn't change from the old
> state, it can retry after some delay.
>

I feel like this logic would cause more problems with fast presses unless
the window was restricted down to sub 50ms.


>
>
> thanks,
>
> Takashi
>
diff mbox series

Patch

diff --git a/sound/soc/codecs/rt5682-i2c.c b/sound/soc/codecs/rt5682-i2c.c
index 93c1603b42f1..b15c3e7d1f59 100644
--- a/sound/soc/codecs/rt5682-i2c.c
+++ b/sound/soc/codecs/rt5682-i2c.c
@@ -78,7 +78,7 @@  static irqreturn_t rt5682_irq(int irq, void *data)
 	struct rt5682_priv *rt5682 = data;
 
 	mod_delayed_work(system_power_efficient_wq,
-		&rt5682->jack_detect_work, msecs_to_jiffies(250));
+		&rt5682->jack_detect_work, 0);
 
 	return IRQ_HANDLED;
 }