Scarlett 18i8 hardware out of sync with AlsaMixer
diff mbox

Message ID 20150527135325.GA12862@canonical.com
State New
Headers show

Commit Message

Chris J Arges May 27, 2015, 1:53 p.m. UTC
On Wed, May 27, 2015 at 03:24:17PM +0200, Takashi Iwai wrote:
> [Added more people to Cc]
> 
> At Wed, 27 May 2015 08:55:21 -0400,
> Adam Goode wrote:
> > 
> > The settings were not reset across a mem sleep (echo mem > /sys/power/state).
> > 
> > Hard power off / power on does make the settings get out of sync.
>

Ok did some testing on my Scarlett 18i8:
 suspend/resume -> settings not reset
 poweroff/poweron -> settings not reset
 unplug/replug usb -> settings not reset
 powercycle unit -> settings not reset

Is there a more consistent method to trigger this issue?
In addition are you manually triggering 'alsactl restore/save' in this process?

I'll do some additional testing to see if I can trigger this.
 
> Then maybe the problem is that the device keeps the old setting while
> the driver resets to the default.  I vaguely remember that scarlett
> could save the default state in a persistent area, and the original
> driver patch had a kctl to trigger it.  We didn't take it because it
> might be dangerous.
> 
>

The original issue was this:
  Hey, as Tobias mentioned this is a HW saving (to the mixer's NVRAM)
  function used for using the mixer disconnected from a computer. We
  wouldn't want to continually write the NVRAM on every control update as
  I'm unsure of how many write-cycles the device is capable of.

So if we wrote to NVRAM on each control update we might wear out the memory
very quickly. I think this functionality is more for using the device
disconnected from a computer.
 
> Takashi
> 
> 
> > Adam
> > 
> > 
> > On Wed, May 27, 2015 at 2:43 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > > At Tue, 26 May 2015 21:39:22 -0400,
> > > Adam Goode wrote:
> > >>
> > >> Linux 4.0.2-300.fc22.x86_64
> > >>
> > >> Hi,
> > >>
> > >> I have the Scarlett 18i8 USB device that is supported by the new
> > >> scarlett_mixer.c code. I am happy to say that the mixer code works in
> > >> most cases. But under some conditions, I cannot get any sound out of
> > >> the device until I go and toggle the "Matrix 01 Input" and "Matrix 02
> > >> Input" enums up and then down (from PCM 1 and PCM 2). This appears to
> > >> me to be some kind of cache invalidation bug, where the device is out
> > >> of sync with kernel mixer state.
> > >>
> > >> I will take a look at the code myself at some point, but not for a
> > >> while. But I am happy to try patches if anyone comes up with anything
> > >> in the mean time.
> > >

Does this change anything (untested)? And are non-enum settings affected?


--chris

> > > Does it happen after some S3/S4 or even during a normal operation
> > > without power-saving?  The USB-audio device supports autopm, so this
> > > needs to be checked, too.
> > >
> > >
> > > Takashi
> > 
>

Comments

Adam Goode June 9, 2015, 9:44 p.m. UTC | #1
I haven't tried the patch yet, but I will try to this week.

Non-enum settings are affected.

What firmware are you running? I have the newest beta firmware
(Scarlett MixControl 1.9b4)
http://beta.focusrite.com/releases/scarlett_mixcontrol/


The other thing to try is to use Mixcontrol and save some settings
into the hardware. I did this once on Windows to use the device before
ALSA supported its mixer.


Adam



On Wed, May 27, 2015 at 9:53 AM, Chris J Arges
<chris.j.arges@canonical.com> wrote:
> On Wed, May 27, 2015 at 03:24:17PM +0200, Takashi Iwai wrote:
>> [Added more people to Cc]
>>
>> At Wed, 27 May 2015 08:55:21 -0400,
>> Adam Goode wrote:
>> >
>> > The settings were not reset across a mem sleep (echo mem > /sys/power/state).
>> >
>> > Hard power off / power on does make the settings get out of sync.
>>
>
> Ok did some testing on my Scarlett 18i8:
>  suspend/resume -> settings not reset
>  poweroff/poweron -> settings not reset
>  unplug/replug usb -> settings not reset
>  powercycle unit -> settings not reset
>
> Is there a more consistent method to trigger this issue?
> In addition are you manually triggering 'alsactl restore/save' in this process?
>
> I'll do some additional testing to see if I can trigger this.
>
>> Then maybe the problem is that the device keeps the old setting while
>> the driver resets to the default.  I vaguely remember that scarlett
>> could save the default state in a persistent area, and the original
>> driver patch had a kctl to trigger it.  We didn't take it because it
>> might be dangerous.
>>
>>
>
> The original issue was this:
>   Hey, as Tobias mentioned this is a HW saving (to the mixer's NVRAM)
>   function used for using the mixer disconnected from a computer. We
>   wouldn't want to continually write the NVRAM on every control update as
>   I'm unsure of how many write-cycles the device is capable of.
>
> So if we wrote to NVRAM on each control update we might wear out the memory
> very quickly. I think this functionality is more for using the device
> disconnected from a computer.
>
>> Takashi
>>
>>
>> > Adam
>> >
>> >
>> > On Wed, May 27, 2015 at 2:43 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> > > At Tue, 26 May 2015 21:39:22 -0400,
>> > > Adam Goode wrote:
>> > >>
>> > >> Linux 4.0.2-300.fc22.x86_64
>> > >>
>> > >> Hi,
>> > >>
>> > >> I have the Scarlett 18i8 USB device that is supported by the new
>> > >> scarlett_mixer.c code. I am happy to say that the mixer code works in
>> > >> most cases. But under some conditions, I cannot get any sound out of
>> > >> the device until I go and toggle the "Matrix 01 Input" and "Matrix 02
>> > >> Input" enums up and then down (from PCM 1 and PCM 2). This appears to
>> > >> me to be some kind of cache invalidation bug, where the device is out
>> > >> of sync with kernel mixer state.
>> > >>
>> > >> I will take a look at the code myself at some point, but not for a
>> > >> while. But I am happy to try patches if anyone comes up with anything
>> > >> in the mean time.
>> > >
>
> Does this change anything (untested)? And are non-enum settings affected?
>
> diff --git a/sound/usb/mixer_scarlett.c b/sound/usb/mixer_scarlett.c
> index 7438e7c..ad4a452 100644
> --- a/sound/usb/mixer_scarlett.c
> +++ b/sound/usb/mixer_scarlett.c
> @@ -438,11 +438,9 @@ static int scarlett_ctl_enum_put(struct snd_kcontrol *kctl,
>
>         val = ucontrol->value.integer.value[0];
>         val = val + opt->start;
> -       if (val != oval) {
> -               snd_usb_set_cur_mix_value(elem, 0, 0, val);
> -               return 1;
> -       }
> -       return 0;
> +       /* always set cur mix value */
> +       snd_usb_set_cur_mix_value(elem, 0, 0, val);
> +       return 1;
>  }
>
>  static int scarlett_ctl_enum_resume(struct usb_mixer_elem_list *list)
>
> --chris
>
>> > > Does it happen after some S3/S4 or even during a normal operation
>> > > without power-saving?  The USB-audio device supports autopm, so this
>> > > needs to be checked, too.
>> > >
>> > >
>> > > Takashi
>> >
>>
Adam Goode June 10, 2015, 4:24 a.m. UTC | #2
On Wed, May 27, 2015 at 9:53 AM, Chris J Arges
<chris.j.arges@canonical.com> wrote:
> On Wed, May 27, 2015 at 03:24:17PM +0200, Takashi Iwai wrote:
>> [Added more people to Cc]
>>
>> At Wed, 27 May 2015 08:55:21 -0400,
>> Adam Goode wrote:
>> >
>> > The settings were not reset across a mem sleep (echo mem > /sys/power/state).
>> >
>> > Hard power off / power on does make the settings get out of sync.
>>
>
> Ok did some testing on my Scarlett 18i8:
>  suspend/resume -> settings not reset
>  poweroff/poweron -> settings not reset
>  unplug/replug usb -> settings not reset
>  powercycle unit -> settings not reset
>
> Is there a more consistent method to trigger this issue?
> In addition are you manually triggering 'alsactl restore/save' in this process?
>
> I'll do some additional testing to see if I can trigger this.
>
>> Then maybe the problem is that the device keeps the old setting while
>> the driver resets to the default.  I vaguely remember that scarlett
>> could save the default state in a persistent area, and the original
>> driver patch had a kctl to trigger it.  We didn't take it because it
>> might be dangerous.
>>
>>
>
> The original issue was this:
>   Hey, as Tobias mentioned this is a HW saving (to the mixer's NVRAM)
>   function used for using the mixer disconnected from a computer. We
>   wouldn't want to continually write the NVRAM on every control update as
>   I'm unsure of how many write-cycles the device is capable of.
>
> So if we wrote to NVRAM on each control update we might wear out the memory
> very quickly. I think this functionality is more for using the device
> disconnected from a computer.
>
>> Takashi
>>
>>
>> > Adam
>> >
>> >
>> > On Wed, May 27, 2015 at 2:43 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> > > At Tue, 26 May 2015 21:39:22 -0400,
>> > > Adam Goode wrote:
>> > >>
>> > >> Linux 4.0.2-300.fc22.x86_64
>> > >>
>> > >> Hi,
>> > >>
>> > >> I have the Scarlett 18i8 USB device that is supported by the new
>> > >> scarlett_mixer.c code. I am happy to say that the mixer code works in
>> > >> most cases. But under some conditions, I cannot get any sound out of
>> > >> the device until I go and toggle the "Matrix 01 Input" and "Matrix 02
>> > >> Input" enums up and then down (from PCM 1 and PCM 2). This appears to
>> > >> me to be some kind of cache invalidation bug, where the device is out
>> > >> of sync with kernel mixer state.
>> > >>
>> > >> I will take a look at the code myself at some point, but not for a
>> > >> while. But I am happy to try patches if anyone comes up with anything
>> > >> in the mean time.
>> > >
>
> Does this change anything (untested)? And are non-enum settings affected?
>
> diff --git a/sound/usb/mixer_scarlett.c b/sound/usb/mixer_scarlett.c
> index 7438e7c..ad4a452 100644
> --- a/sound/usb/mixer_scarlett.c
> +++ b/sound/usb/mixer_scarlett.c
> @@ -438,11 +438,9 @@ static int scarlett_ctl_enum_put(struct snd_kcontrol *kctl,
>
>         val = ucontrol->value.integer.value[0];
>         val = val + opt->start;
> -       if (val != oval) {
> -               snd_usb_set_cur_mix_value(elem, 0, 0, val);
> -               return 1;
> -       }
> -       return 0;
> +       /* always set cur mix value */
> +       snd_usb_set_cur_mix_value(elem, 0, 0, val);
> +       return 1;
>  }
>
>  static int scarlett_ctl_enum_resume(struct usb_mixer_elem_list *list)
>
> --chris
>


I had to similarly remove the other instances of (val != oval) in the
code, and then it worked as expected.

My guess is that the ALSA mixer settings are cached too soon, before
the device itself internally overwrites the settings from NVRAM. We
are either missing some invalidation event from the device itself, or
we don't do the right thing during initialization.


Adam


>> > > Does it happen after some S3/S4 or even during a normal operation
>> > > without power-saving?  The USB-audio device supports autopm, so this
>> > > needs to be checked, too.
>> > >
>> > >
>> > > Takashi
>> >
>>
Robin Gareus June 13, 2015, 11:03 p.m. UTC | #3
On 06/10/2015 06:24 AM, Adam Goode wrote:
[..]
> My guess is that the ALSA mixer settings are cached too soon, before
> the device itself internally overwrites the settings from NVRAM. We
> are either missing some invalidation event from the device itself, or
> we don't do the right thing during initialization.

I didn't follow the whole discussion, so please excuse me if this was
mentioned before:

I'm pretty sure that the NVRAM is restored on the Scarlet before the
device shows up on USB: Save a config with Hi-Z on but configuring it as
off in software. When connecting & re-powering the device, the indicator
LED will blink on and then go off.


It is not possible to read the full state from the device itself.
Trying to read the value from the device results in garbage for the most
part. The values must be set by ALSA and ALSA must remember the state.
That worked in the original alsa mixer even across suspend/resume for
the 18i6. Either something was changed or has been lost since.

This is also how the OSX and Windows drivers work for the Scarlett
series. As soon as the device is [re]connected, software pushes the last
known state (on that given machine) to the device. (At least it was that
way ~2 years ago when I reverse engineered it).

BTW the prop. driver saves state to the NVRAM quite often. I don't know
if this is a marketing trick (intentionally wear it out quickly) or if
it's OK to do that.


best,
robin
Takashi Iwai June 19, 2015, 11:34 a.m. UTC | #4
At Sun, 14 Jun 2015 01:03:42 +0200,
Robin Gareus wrote:
> 
> On 06/10/2015 06:24 AM, Adam Goode wrote:
> [..]
> > My guess is that the ALSA mixer settings are cached too soon, before
> > the device itself internally overwrites the settings from NVRAM. We
> > are either missing some invalidation event from the device itself, or
> > we don't do the right thing during initialization.
> 
> I didn't follow the whole discussion, so please excuse me if this was
> mentioned before:
> 
> I'm pretty sure that the NVRAM is restored on the Scarlet before the
> device shows up on USB: Save a config with Hi-Z on but configuring it as
> off in software. When connecting & re-powering the device, the indicator
> LED will blink on and then go off.
> 
> 
> It is not possible to read the full state from the device itself.
> Trying to read the value from the device results in garbage for the most
> part. The values must be set by ALSA and ALSA must remember the state.
> That worked in the original alsa mixer even across suspend/resume for
> the 18i6. Either something was changed or has been lost since.
> 
> This is also how the OSX and Windows drivers work for the Scarlett
> series. As soon as the device is [re]connected, software pushes the last
> known state (on that given machine) to the device. (At least it was that
> way ~2 years ago when I reverse engineered it).

OK, then the initial values seem missing.  The driver has already
cache in it, so the suspend/resume should work as is now.  The only
the initial values have to be set up explicitly.

> BTW the prop. driver saves state to the NVRAM quite often. I don't know
> if this is a marketing trick (intentionally wear it out quickly) or if
> it's OK to do that.

We have a control already in user-space (simple "alsactl restore"
should do), so there is no big merit to do it in the driver level,
IMO.


thanks,

Takashi

Patch
diff mbox

diff --git a/sound/usb/mixer_scarlett.c b/sound/usb/mixer_scarlett.c
index 7438e7c..ad4a452 100644
--- a/sound/usb/mixer_scarlett.c
+++ b/sound/usb/mixer_scarlett.c
@@ -438,11 +438,9 @@  static int scarlett_ctl_enum_put(struct snd_kcontrol *kctl,
 
 	val = ucontrol->value.integer.value[0];
 	val = val + opt->start;
-	if (val != oval) {
-		snd_usb_set_cur_mix_value(elem, 0, 0, val);
-		return 1;
-	}
-	return 0;
+	/* always set cur mix value */
+	snd_usb_set_cur_mix_value(elem, 0, 0, val);
+	return 1;
 }
 
 static int scarlett_ctl_enum_resume(struct usb_mixer_elem_list *list)