[PATCH] ASoC: sun4i-codec: fix first delay on Speaker

[PATCH] ASoC: sun4i-codec: fix first delay on Speaker

[Georgii Staroselskii]

This commit fixes the same issue as bf14da7 but for another codepath.
The issue can be triggered by having Speaker in codec audio routing.

&codec {
        allwinner,pa-gpios = <&r_pio 0 11 GPIO_ACTIVE_HIGH>; /* PL11 */
        allwinner,audio-routing =
                "Speaker", "LINEOUT";

        status = "okay";
}

Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
---
 sound/soc/sunxi/sun4i-codec.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 15d08e3..e0099519 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -1329,6 +1329,15 @@ static int sun4i_codec_spk_event(struct snd_soc_dapm_widget *w,
 	gpiod_set_value_cansleep(scodec->gpio_pa,
 				 !!SND_SOC_DAPM_EVENT_ON(event));
 
+	if (SND_SOC_DAPM_EVENT_ON(event)) {
+		/*
+		 * Need a delay to have the amplifier up. 700ms seems the best
+		 * compromise between the time to let the amplifier up and the
+		 * time not to feel this delay while playing a sound.
+		 */
+		msleep(700);
+	}
+
 	return 0;
 }
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: sun4i-codec: fix first delay on Speaker

[Maxime Ripard]

Hi,

On Wed, May 15, 2019 at 03:58:40PM +0300, Georgii Staroselskii wrote:
> This commit fixes the same issue as bf14da7 but for another codepath.
> The issue can be triggered by having Speaker in codec audio routing.

Mentionning what "the issue" is in the commit log would be great.

> &codec {
>         allwinner,pa-gpios = <&r_pio 0 11 GPIO_ACTIVE_HIGH>; /* PL11 */
>         allwinner,audio-routing =
>                 "Speaker", "LINEOUT";
>
>         status = "okay";
> }
>
> Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
> ---
>  sound/soc/sunxi/sun4i-codec.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 15d08e3..e0099519 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -1329,6 +1329,15 @@ static int sun4i_codec_spk_event(struct snd_soc_dapm_widget *w,
>  	gpiod_set_value_cansleep(scodec->gpio_pa,
>  				 !!SND_SOC_DAPM_EVENT_ON(event));
>
> +	if (SND_SOC_DAPM_EVENT_ON(event)) {
> +		/*
> +		 * Need a delay to have the amplifier up. 700ms seems the best
> +		 * compromise between the time to let the amplifier up and the
> +		 * time not to feel this delay while playing a sound.
> +		 */
> +		msleep(700);
> +	}
> +

Since this is an external amplifier, I guess they would have different
warm-up time depending on the exact part being used?

If so, we should use a property and set it on a per-board basis.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: sun4i-codec: fix first delay on Speaker

[Georgii Staroselskii]

On Wed, May 15, 2019 at 03:58:13PM +0200, Maxime Ripard wrote:
Hi!

> Hi,
> 
> On Wed, May 15, 2019 at 03:58:40PM +0300, Georgii Staroselskii wrote:
> > This commit fixes the same issue as bf14da7 but for another codepath.
> > The issue can be triggered by having Speaker in codec audio routing.
> 
> Mentionning what "the issue" is in the commit log would be great.
> 

Sure, will do.

> > &codec {
> >         allwinner,pa-gpios = <&r_pio 0 11 GPIO_ACTIVE_HIGH>; /* PL11 */
> >         allwinner,audio-routing =
> >                 "Speaker", "LINEOUT";
> >
> >         status = "okay";
> > }
> >
> > Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
> > ---
> >  sound/soc/sunxi/sun4i-codec.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> > index 15d08e3..e0099519 100644
> > --- a/sound/soc/sunxi/sun4i-codec.c
> > +++ b/sound/soc/sunxi/sun4i-codec.c
> > @@ -1329,6 +1329,15 @@ static int sun4i_codec_spk_event(struct snd_soc_dapm_widget *w,
> >  	gpiod_set_value_cansleep(scodec->gpio_pa,
> >  				 !!SND_SOC_DAPM_EVENT_ON(event));
> >
> > +	if (SND_SOC_DAPM_EVENT_ON(event)) {
> > +		/*
> > +		 * Need a delay to have the amplifier up. 700ms seems the best
> > +		 * compromise between the time to let the amplifier up and the
> > +		 * time not to feel this delay while playing a sound.
> > +		 */
> > +		msleep(700);
> > +	}
> > +
> 
> Since this is an external amplifier, I guess they would have different
> warm-up time depending on the exact part being used?
> 

I guess I might've used Speaker wrong and bumped into an existing issue.
The issue first arose when I needed to connect a speaker and use a mute
GPIO pin to toggle it. I bumped into the lag similar to the one that has
been fixed in bf14da7. The word "amplifier" here in my comments might be wrong 
and misleding. Sorry for that. I just measured the latency on the speaker I'm
using and it is well under 1ms so this is the Allwinner DAC that is
pushing the data with a lag. Or some other thing, I'm not sure.

I want to stress again that I might've experienced the issue because
I was abusing "Speaker" routing. I basically just needed the analog 
audio stream and mute GPIO handling done automatically.

bf14da7 made me think that this is some hardware Allwinner issue that
needs to be fixed here. And well, it did fix the problem.

> If so, we should use a property and set it on a per-board basis.
> 

There might be a use case for that but this issue seems to be orthogonal
to this.

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Best regards,
Georgii Staroselskii

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: sun4i-codec: fix first delay on Speaker

[Maxime Ripard]

On Wed, May 15, 2019 at 06:27:49PM +0300, Georgii Staroselskii wrote:
> On Wed, May 15, 2019 at 03:58:13PM +0200, Maxime Ripard wrote:
> > > diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> > > index 15d08e3..e0099519 100644
> > > --- a/sound/soc/sunxi/sun4i-codec.c
> > > +++ b/sound/soc/sunxi/sun4i-codec.c
> > > @@ -1329,6 +1329,15 @@ static int sun4i_codec_spk_event(struct snd_soc_dapm_widget *w,
> > >  	gpiod_set_value_cansleep(scodec->gpio_pa,
> > >  				 !!SND_SOC_DAPM_EVENT_ON(event));
> > >
> > > +	if (SND_SOC_DAPM_EVENT_ON(event)) {
> > > +		/*
> > > +		 * Need a delay to have the amplifier up. 700ms seems the best
> > > +		 * compromise between the time to let the amplifier up and the
> > > +		 * time not to feel this delay while playing a sound.
> > > +		 */
> > > +		msleep(700);
> > > +	}
> > > +
> >
> > Since this is an external amplifier, I guess they would have different
> > warm-up time depending on the exact part being used?
>
> I guess I might've used Speaker wrong and bumped into an existing
> issue.  The issue first arose when I needed to connect a speaker and
> use a mute GPIO pin to toggle it. I bumped into the lag similar to
> the one that has been fixed in bf14da7. The word "amplifier" here in
> my comments might be wrong and misleding. Sorry for that. I just
> measured the latency on the speaker I'm using and it is well under
> 1ms so this is the Allwinner DAC that is pushing the data with a
> lag. Or some other thing, I'm not sure.
>
> I want to stress again that I might've experienced the issue because
> I was abusing "Speaker" routing. I basically just needed the analog
> audio stream and mute GPIO handling done automatically.

Ok. I guess the comment should just be reflecting that then.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: sun4i-codec: fix first delay on Speaker

[Chen-Yu Tsai]

On Fri, May 17, 2019 at 4:13 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Wed, May 15, 2019 at 06:27:49PM +0300, Georgii Staroselskii wrote:
> > On Wed, May 15, 2019 at 03:58:13PM +0200, Maxime Ripard wrote:
> > > > diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> > > > index 15d08e3..e0099519 100644
> > > > --- a/sound/soc/sunxi/sun4i-codec.c
> > > > +++ b/sound/soc/sunxi/sun4i-codec.c
> > > > @@ -1329,6 +1329,15 @@ static int sun4i_codec_spk_event(struct snd_soc_dapm_widget *w,
> > > >   gpiod_set_value_cansleep(scodec->gpio_pa,
> > > >                            !!SND_SOC_DAPM_EVENT_ON(event));
> > > >
> > > > + if (SND_SOC_DAPM_EVENT_ON(event)) {
> > > > +         /*
> > > > +          * Need a delay to have the amplifier up. 700ms seems the best
> > > > +          * compromise between the time to let the amplifier up and the
> > > > +          * time not to feel this delay while playing a sound.
> > > > +          */
> > > > +         msleep(700);
> > > > + }
> > > > +
> > >
> > > Since this is an external amplifier, I guess they would have different
> > > warm-up time depending on the exact part being used?
> >
> > I guess I might've used Speaker wrong and bumped into an existing
> > issue.  The issue first arose when I needed to connect a speaker and
> > use a mute GPIO pin to toggle it. I bumped into the lag similar to
> > the one that has been fixed in bf14da7. The word "amplifier" here in
> > my comments might be wrong and misleding. Sorry for that. I just
> > measured the latency on the speaker I'm using and it is well under
> > 1ms so this is the Allwinner DAC that is pushing the data with a
> > lag. Or some other thing, I'm not sure.
> >
> > I want to stress again that I might've experienced the issue because
> > I was abusing "Speaker" routing. I basically just needed the analog
> > audio stream and mute GPIO handling done automatically.
>
> Ok. I guess the comment should just be reflecting that then.

Sounds like the board has an external buffer / amplifier for the speaker.
Does it have a headphone jack that bypasses it? Then you could check for
the actual latency.

Also, I think it might be better if we could move to actual audio routing
and have a simple-amplifier instead.

ChenYu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel