sgtl5000 fails after suspend/resume

sgtl5000 fails after suspend/resume

[Fabio Estevam]

Hi,

I noticed that sgtl5000 does not work after a suspend/resume cycle:

# aplay /media/a2002011001-e02.wav
Playing WAVE '/media/a2002011001-e02.wav' : Signed 16 bit Little
Endian, Rate 44100 Hz, Stereo
aplay: pcm_write:2051: write error: Input/output error

Tested several 4.16-rc1 (and also previous kernels like 4.4) and all
of them failed.

I think it worked in some point in time.

Tested on a imx53qsb and imx6 wandboard.

Does anyone have any suggestions?

Thanks,

Fabio Estevam

Re: sgtl5000 fails after suspend/resume

[Fabio Estevam]

On Thu, Feb 15, 2018 at 11:35 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi,
>
> I noticed that sgtl5000 does not work after a suspend/resume cycle:
>
> # aplay /media/a2002011001-e02.wav
> Playing WAVE '/media/a2002011001-e02.wav' : Signed 16 bit Little
> Endian, Rate 44100 Hz, Stereo
> aplay: pcm_write:2051: write error: Input/output error
>
> Tested several 4.16-rc1 (and also previous kernels like 4.4) and all
> of them failed.
>
> I think it worked in some point in time.
>
> Tested on a imx53qsb and imx6 wandboard.
>
> Does anyone have any suggestions?

Reverting the commit below on top of 4.16-rc1 makes sgtl5000
suspend/resume to work again:

commit 8419caa7270291e26f8b34b12b29680586c85d30
Author: Eric Nelson <eric@nelint.com>
Date:   Tue Jun 7 01:14:52 2016 +0200

    ASoC: sgtl5000: Do not disable regulators in SND_SOC_BIAS_OFF

    Disabling the SGTL5000 through regulators would certainly save more
    power than simply disabling the reference voltages as described in the
    data sheet, but won't properly restore things on resume.

    This driver does not support active regulators. So we simply disable the
    reference bias currents.

    Signed-off-by: Eric Nelson <eric@nelint.com>
    Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
    Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
    Signed-off-by: Mark Brown <broonie@kernel.org>

Or if I do the change below against linux-next it also works:

--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -880,8 +880,6 @@ static int sgtl5000_set_bias_level(struct
snd_soc_component *component,
                                    SGTL5000_REFTOP_POWERUP);
                break;
        case SND_SOC_BIAS_OFF:
-               snd_soc_component_update_bits(component,
SGTL5000_CHIP_ANA_POWER,
-                                   SGTL5000_REFTOP_POWERUP, 0);
                break;
        }

Any suggestions for a proper fix? Eric?

Thanks

Re: sgtl5000 fails after suspend/resume

[Fabio Estevam]

On Thu, Feb 15, 2018 at 2:36 PM, Fabio Estevam <festevam@gmail.com> wrote:

> Or if I do the change below against linux-next it also works:
>
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -880,8 +880,6 @@ static int sgtl5000_set_bias_level(struct
> snd_soc_component *component,
>                                     SGTL5000_REFTOP_POWERUP);
>                 break;
>         case SND_SOC_BIAS_OFF:
> -               snd_soc_component_update_bits(component,
> SGTL5000_CHIP_ANA_POWER,
> -                                   SGTL5000_REFTOP_POWERUP, 0);
>                 break;
>         }
>
> Any suggestions for a proper fix? Eric?

I sent a patch that that removes sgtl5000_set_bias_level() completely
and fixes the sgtl5000 suspend/resume issue.

Re: sgtl5000 fails after suspend/resume

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 671 bytes --]

On Thu, Feb 15, 2018 at 02:36:00PM -0200, Fabio Estevam wrote:

> Reverting the commit below on top of 4.16-rc1 makes sgtl5000
> suspend/resume to work again:

> @@ -880,8 +880,6 @@ static int sgtl5000_set_bias_level(struct
> snd_soc_component *component,
>                                     SGTL5000_REFTOP_POWERUP);
>                 break;
>         case SND_SOC_BIAS_OFF:
> -               snd_soc_component_update_bits(component,
> SGTL5000_CHIP_ANA_POWER,
> -                                   SGTL5000_REFTOP_POWERUP, 0);
>                 break;
>         }

> Any suggestions for a proper fix? Eric?

Does it need a register map resync when powering on again?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: sgtl5000 fails after suspend/resume

[Max Krummenacher]

Hi Fabio

On Thu, 2018-02-15 at 14:36 -0200, Fabio Estevam wrote:
> On Thu, Feb 15, 2018 at 11:35 AM, Fabio Estevam <festevam@gmail.com> wrote:
> > 
> > Hi,
> > 
> > I noticed that sgtl5000 does not work after a suspend/resume cycle:
> > 
> > # aplay /media/a2002011001-e02.wav
> > Playing WAVE '/media/a2002011001-e02.wav' : Signed 16 bit Little
> > Endian, Rate 44100 Hz, Stereo
> > aplay: pcm_write:2051: write error: Input/output error
> > 
> > Tested several 4.16-rc1 (and also previous kernels like 4.4) and all
> > of them failed.
> > 
> > I think it worked in some point in time.
> > 
> > Tested on a imx53qsb and imx6 wandboard.
> > 
> > Does anyone have any suggestions?
> 
> Reverting the commit below on top of 4.16-rc1 makes sgtl5000
> suspend/resume to work again:
> 
> commit 8419caa7270291e26f8b34b12b29680586c85d30
> Author: Eric Nelson <eric@nelint.com>
> Date:   Tue Jun 7 01:14:52 2016 +0200
> 
>     ASoC: sgtl5000: Do not disable regulators in SND_SOC_BIAS_OFF
> 
>     Disabling the SGTL5000 through regulators would certainly save more
>     power than simply disabling the reference voltages as described in the
>     data sheet, but won't properly restore things on resume.
> 
>     This driver does not support active regulators. So we simply disable the
>     reference bias currents.
> 
>     Signed-off-by: Eric Nelson <eric@nelint.com>
>     Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
>     Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
>     Signed-off-by: Mark Brown <broonie@kernel.org>
> 
> Or if I do the change below against linux-next it also works:
> 
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -880,8 +880,6 @@ static int sgtl5000_set_bias_level(struct
> snd_soc_component *component,
>                                     SGTL5000_REFTOP_POWERUP);
>                 break;
>         case SND_SOC_BIAS_OFF:
> -               snd_soc_component_update_bits(component,
> SGTL5000_CHIP_ANA_POWER,
> -                                   SGTL5000_REFTOP_POWERUP, 0);
>                 break;
>         }
> 
> Any suggestions for a proper fix? Eric?
> 

I tested this on a Colibri iMX6DL with the same results.
Vanilla 4.16-rc1 does resume but audio is silent after resume.
The interrupted aplay process is in the process list and never finishes.

Both your fixes make aplay continue playing sound again until the end
of the played sound file.

Strangely if I use our downstream kernel (based on NXP/linux-fslc
4.9-1.0.x-imx which has the 8419caa7270291e26f8b34b12b29680586c85d30
commit) suspend/resume works.

Max

Re: sgtl5000 fails after suspend/resume

[Fabio Estevam]

Hi Mark,

On Thu, Feb 15, 2018 at 4:16 PM, Mark Brown <broonie@kernel.org> wrote:

> Does it need a register map resync when powering on again?

Thanks for your suggestion! This works:

--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -871,15 +871,26 @@ static int sgtl5000_pcm_hw_params(struct
snd_pcm_substream *substream,
 static int sgtl5000_set_bias_level(struct snd_soc_component *component,
                                   enum snd_soc_bias_level level)
 {
+       struct sgtl5000_priv *sgtl = snd_soc_component_get_drvdata(component);
+       int ret;
+
        switch (level) {
        case SND_SOC_BIAS_ON:
        case SND_SOC_BIAS_PREPARE:
        case SND_SOC_BIAS_STANDBY:
+               regcache_cache_only(sgtl->regmap, false);
+               ret = regcache_sync(sgtl->regmap);
+               if (ret) {
+                       regcache_cache_only(sgtl->regmap, true);
+                       return ret;
+               }
+
                snd_soc_component_update_bits(component,
SGTL5000_CHIP_ANA_POWER,
                                    SGTL5000_REFTOP_POWERUP,
                                    SGTL5000_REFTOP_POWERUP);
                break;
        case SND_SOC_BIAS_OFF:
+               regcache_cache_only(sgtl->regmap, true);
                snd_soc_component_update_bits(component,
SGTL5000_CHIP_ANA_POWER,
                                    SGTL5000_REFTOP_POWERUP, 0);
                break;