* [PATCH] ASoC: rt5682: remove jack detect delay @ 2021-02-17 21:49 Curtis Malainey 2021-02-17 21:51 ` Curtis Malainey [not found] ` <5b90530b77744937b87bbbd35901e320@realtek.com> 0 siblings, 2 replies; 10+ messages in thread From: Curtis Malainey @ 2021-02-17 21:49 UTC (permalink / raw) To: alsa-devel Cc: Oder Chiou, Takashi Iwai, Liam Girdwood, Mark Brown, Shuming Fan, Curtis Malainey, Bard liao 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); return IRQ_HANDLED; } -- 2.30.0.478.g8a0d178c01-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt5682: remove jack detect delay 2021-02-17 21:49 [PATCH] ASoC: rt5682: remove jack detect delay Curtis Malainey @ 2021-02-17 21:51 ` Curtis Malainey [not found] ` <5b90530b77744937b87bbbd35901e320@realtek.com> 1 sibling, 0 replies; 10+ messages in thread From: Curtis Malainey @ 2021-02-17 21:51 UTC (permalink / raw) To: Curtis Malainey, Arava, Jairaj Cc: Oder Chiou, ALSA development, Takashi Iwai, Liam Girdwood, Mark Brown, Shuming Fan, Bard liao Hello Realtek +Arava, Jairaj <jairaj.arava@intel.com> On Wed, Feb 17, 2021 at 1:49 PM Curtis Malainey <cujomalainey@chromium.org> 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); > > This change is posted to start a discussion as to the purpose of this delay, we are seeing noticeable UI delay and button masking. Is there an electro/mechanical purpose to it? If not I think I should post a V2 to remove the workqueue since this is a threaded irq. Please advise, thanks. > return IRQ_HANDLED; > } > -- > 2.30.0.478.g8a0d178c01-goog > > ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <5b90530b77744937b87bbbd35901e320@realtek.com>]
* Re: [PATCH] ASoC: rt5682: remove jack detect delay [not found] ` <5b90530b77744937b87bbbd35901e320@realtek.com> @ 2021-02-18 8:44 ` Takashi Iwai 2021-02-18 21:06 ` Curtis Malainey 0 siblings, 1 reply; 10+ messages in thread From: Takashi Iwai @ 2021-02-18 8:44 UTC (permalink / raw) To: "Shuming [范書銘]" Cc: Oder Chiou, Jack Yu, alsa-devel, Takashi Iwai, Liam Girdwood, Mark Brown, "Derek [方德義]", Curtis Malainey, Bard liao, Flove(HsinFu) 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt5682: remove jack detect delay 2021-02-18 8:44 ` Takashi Iwai @ 2021-02-18 21:06 ` Curtis Malainey 2021-02-22 13:45 ` Mark Brown 0 siblings, 1 reply; 10+ messages in thread From: Curtis Malainey @ 2021-02-18 21:06 UTC (permalink / raw) To: Takashi Iwai Cc: Oder Chiou, Jack Yu, alsa-devel, Takashi Iwai, Liam Girdwood, Mark Brown, Derek [方德義], Shuming [范書銘], Curtis Malainey, Bard liao, Flove(HsinFu) 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt5682: remove jack detect delay 2021-02-18 21:06 ` Curtis Malainey @ 2021-02-22 13:45 ` Mark Brown 2021-02-22 18:59 ` Curtis Malainey 0 siblings, 1 reply; 10+ messages in thread From: Mark Brown @ 2021-02-22 13:45 UTC (permalink / raw) To: Curtis Malainey Cc: Oder Chiou, Jack Yu, alsa-devel, Takashi Iwai, Takashi Iwai, Liam Girdwood, Derek [方德義], Shuming [范書銘], Curtis Malainey, Bard liao, Flove(HsinFu) [-- Attachment #1: Type: text/plain, Size: 651 bytes --] On Thu, Feb 18, 2021 at 01:06:27PM -0800, Curtis Malainey wrote: > 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. It's quite common to have lots of issues with debounce on jacks, especially around insert/removal - it looks like this delay covers both insert/removal and button presses so it may well be needed for robust handling of the actual jack insert. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt5682: remove jack detect delay 2021-02-22 13:45 ` Mark Brown @ 2021-02-22 18:59 ` Curtis Malainey 2021-02-23 12:42 ` Mark Brown 0 siblings, 1 reply; 10+ messages in thread From: Curtis Malainey @ 2021-02-22 18:59 UTC (permalink / raw) To: Mark Brown Cc: Oder Chiou, Jack Yu, alsa-devel, Takashi Iwai, Takashi Iwai, Liam Girdwood, Derek [方德義], Shuming [范書銘], Curtis Malainey, Bard liao, Flove(HsinFu) On Mon, Feb 22, 2021 at 5:46 AM Mark Brown <broonie@kernel.org> wrote: > On Thu, Feb 18, 2021 at 01:06:27PM -0800, Curtis Malainey wrote: > > > 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. > Would a acceptable solution to everyone be if inserted && buttonAction respond now else run workqueue > > It's quite common to have lots of issues with debounce on jacks, > especially around insert/removal - it looks like this delay covers both > insert/removal and button presses so it may well be needed for robust > handling of the actual jack insert. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt5682: remove jack detect delay 2021-02-22 18:59 ` Curtis Malainey @ 2021-02-23 12:42 ` Mark Brown 2021-02-23 18:50 ` Curtis Malainey 0 siblings, 1 reply; 10+ messages in thread From: Mark Brown @ 2021-02-23 12:42 UTC (permalink / raw) To: Curtis Malainey Cc: Oder Chiou, Jack Yu, alsa-devel, Takashi Iwai, Takashi Iwai, Liam Girdwood, Derek [方德義], Shuming [范書銘], Curtis Malainey, Bard liao, Flove(HsinFu) [-- Attachment #1: Type: text/plain, Size: 296 bytes --] On Mon, Feb 22, 2021 at 10:59:34AM -0800, Curtis Malainey wrote: > if inserted && buttonAction > respond now > else > run workqueue Are you sure that *zero* debounce is needed for the button presses? 250ms does look like a lot of time but zero might be going from one extreme to the other. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt5682: remove jack detect delay 2021-02-23 12:42 ` Mark Brown @ 2021-02-23 18:50 ` Curtis Malainey 2021-02-23 19:04 ` Mark Brown 0 siblings, 1 reply; 10+ messages in thread From: Curtis Malainey @ 2021-02-23 18:50 UTC (permalink / raw) To: Mark Brown Cc: Oder Chiou, Jack Yu, alsa-devel, Takashi Iwai, Takashi Iwai, Liam Girdwood, Derek [方德義], Shuming [范書銘], Curtis Malainey, Bard liao, Flove(HsinFu) > > Are you sure that *zero* debounce is needed for the button presses? > 250ms does look like a lot of time but zero might be going from one > extreme to the other. Fair point, I was looking at some other codecs and why they respond so quickly, it appears they have no fixed delay and just call schedule work. That being said, I can easily double tap <100ms. So Ideally i would like to keep this on the order of ~50ms at most. I am guessing Realtek will want to keep the 250ms for jack detect still. Would queueing two separate jobs with two different delays be the simple way to go? Realtek does this sound fine to you? Curtis ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt5682: remove jack detect delay 2021-02-23 18:50 ` Curtis Malainey @ 2021-02-23 19:04 ` Mark Brown 2021-02-23 20:07 ` Curtis Malainey 0 siblings, 1 reply; 10+ messages in thread From: Mark Brown @ 2021-02-23 19:04 UTC (permalink / raw) To: Curtis Malainey Cc: Oder Chiou, Jack Yu, alsa-devel, Takashi Iwai, Takashi Iwai, Liam Girdwood, Derek [方德義], Shuming [范書銘], Curtis Malainey, Bard liao, Flove(HsinFu) [-- Attachment #1: Type: text/plain, Size: 932 bytes --] On Tue, Feb 23, 2021 at 10:50:18AM -0800, Curtis Malainey wrote: > > Are you sure that *zero* debounce is needed for the button presses? > > 250ms does look like a lot of time but zero might be going from one > > extreme to the other. > Fair point, I was looking at some other codecs and why they respond so > quickly, it appears they have no fixed delay and just call schedule > work. That being said, I can easily double tap <100ms. So Ideally i > would like to keep this on the order of ~50ms at most. I am guessing > Realtek will want to keep the 250ms for jack detect still. Those feel like plausible numbers to me assuming there's no hardware debounce. > Would queueing two separate jobs with two different delays be the > simple way to go? Realtek does this sound fine to you? Possibly just queuing the same job with different timeouts? I don't have particularly strong feelings assuming the resulting code make sense. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt5682: remove jack detect delay 2021-02-23 19:04 ` Mark Brown @ 2021-02-23 20:07 ` Curtis Malainey 0 siblings, 0 replies; 10+ messages in thread From: Curtis Malainey @ 2021-02-23 20:07 UTC (permalink / raw) To: Mark Brown Cc: Oder Chiou, Jack Yu, alsa-devel, Takashi Iwai, Takashi Iwai, Liam Girdwood, Derek [方德義], Shuming [范書銘], Curtis Malainey, Bard liao, Flove(HsinFu) > > > Would queueing two separate jobs with two different delays be the > > simple way to go? Realtek does this sound fine to you? > > Possibly just queuing the same job with different timeouts? I don't > have particularly strong feelings assuming the resulting code make > sense. And just track how we were last scheduled so we only check the correct thing? Sure that works too. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-23 20:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-17 21:49 [PATCH] ASoC: rt5682: remove jack detect delay Curtis Malainey 2021-02-17 21:51 ` Curtis Malainey [not found] ` <5b90530b77744937b87bbbd35901e320@realtek.com> 2021-02-18 8:44 ` Takashi Iwai 2021-02-18 21:06 ` Curtis Malainey 2021-02-22 13:45 ` Mark Brown 2021-02-22 18:59 ` Curtis Malainey 2021-02-23 12:42 ` Mark Brown 2021-02-23 18:50 ` Curtis Malainey 2021-02-23 19:04 ` Mark Brown 2021-02-23 20:07 ` Curtis Malainey
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.