Hi !

Thanks for answering me. Yes, turning the output volume up or down as soon as it's playing at full output : it plays at the % it should play.

I'm OK to try the patch, but I don't find the /rme96.c/ file. Where is it located ? I think a recompilation's needed then ? Never done that.

Thanks for help.

Kind regards.

Le 20/11/2015 17:34, Takashi Iwai a écrit :

On Tue, 10 Nov 2015 13:41:44 +0100, Maeda wrote:

...

Hi !

Someone's here ?

Thanks.

Le 23/10/2015 18:30, Maeda a écrit :

...

Hi all !

I don't know if the bug is known from your team, but some people advice me to send an email to the Alsa devel list if nobody answer in the kernel's bugtracking.

Here is the bug I have : https://bugzilla.kernel.org/show_bug.cgi?id=105771

What do you think about it ?

Does the volume go down when you adjust DAC volume by mixer application when this happens?

Through a quick glance at the driver code, the only smelling part is that it's calling snd_rme96_reset_dac() from snd_rme96_playback_setrate(). If the DAC volume adjustment really works, the patch below might work. Please give it a try.

The patch has a code to give some delay that is commented out for now. The delay is found in the resume path, but I'm not sure whether it's mandatory. So let's try at first without the delay. If it's unstable, you can uncomment the line and retry.

thanks,

Takashi

---

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..a9d8a66fc3da 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,6 +741,9 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96);

• /* usleep_range(3000, 10000); */
  

• if (RME96_HAS_ANALOG_OUT(rme96))
  

• 	snd_rme96_apply_dac_volume(rme96);
  

  } else { writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER); }

OK that's what I thought. I'm using Archlinux, I'll see on the archwiki's website, and report results here.

Stay tuned :)

Le 21/11/2015 20:45, Takashi Iwai a écrit :

On Sat, 21 Nov 2015 18:42:06 +0100, Maeda wrote:

...

Hi !

Thanks for answering me. Yes, turning the output volume up or down as soon as it's playing at full output : it plays at the % it should play.

I'm OK to try the patch, but I don't find the /rme96.c/ file. Where is it located ? I think a recompilation's needed then ? Never done that.

Well, you need to learn how to compile the kernel. It's possible to compile only a module from the current running tree, too.

Which distro are you using? You can ask distro guys for assistance.

Takashi

...

Thanks for help.

Kind regards.

Le 20/11/2015 17:34, Takashi Iwai a écrit :

...

On Tue, 10 Nov 2015 13:41:44 +0100, Maeda wrote:

...

Hi !

Someone's here ?

Thanks.

Le 23/10/2015 18:30, Maeda a écrit :

...

Hi all !

I don't know if the bug is known from your team, but some people advice me to send an email to the Alsa devel list if nobody answer in the kernel's bugtracking.

Here is the bug I have : https://bugzilla.kernel.org/show_bug.cgi?id=105771

What do you think about it ?

Does the volume go down when you adjust DAC volume by mixer application when this happens?

Through a quick glance at the driver code, the only smelling part is that it's calling snd_rme96_reset_dac() from snd_rme96_playback_setrate(). If the DAC volume adjustment really works, the patch below might work. Please give it a try.

The patch has a code to give some delay that is commented out for now. The delay is found in the resume path, but I'm not sure whether it's mandatory. So let's try at first without the delay. If it's unstable, you can uncomment the line and retry.

thanks,

Takashi

---

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..a9d8a66fc3da 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,6 +741,9 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96);

• /* usleep_range(3000, 10000); */
  

• if (RME96_HAS_ANALOG_OUT(rme96))
  

• 	snd_rme96_apply_dac_volume(rme96);
  

  } else { writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER); }

On Wed, 02 Dec 2015 11:11:27 +0100, Maeda wrote:

Hi Takashi !

Sorry for the late answer. I do compile the 'test' kernel after editing the file with the three lines as you said on your previous mail.

The result is that the bug isn't occurring anymore, tested with 96KHz sample :

`aplay -fcd -d3 /dev/zero ; aplay -fS32_LE -r96000 -c2 -d3 /dev/zero ; speaker-test -c2 -twav`

_BUT_, I only have the right speaker that works, I need to change the volume to restore sound on the left channel.

So the patch prevents the wrong max value reset but it's still incomplete for the left channel volume?

Did you try to enable the usleep_range() that was commented out in the patch?

Takashi

Maeda.

Le 21/11/2015 20:45, Takashi Iwai a écrit :

...

On Sat, 21 Nov 2015 18:42:06 +0100, Maeda wrote:

...

Hi !

Thanks for answering me. Yes, turning the output volume up or down as soon as it's playing at full output : it plays at the % it should play.

I'm OK to try the patch, but I don't find the /rme96.c/ file. Where is it located ? I think a recompilation's needed then ? Never done that.

Well, you need to learn how to compile the kernel. It's possible to compile only a module from the current running tree, too.

Which distro are you using? You can ask distro guys for assistance.

Takashi

...

Thanks for help.

Kind regards.

Le 20/11/2015 17:34, Takashi Iwai a écrit :

...

On Tue, 10 Nov 2015 13:41:44 +0100, Maeda wrote:

...

Hi !

Someone's here ?

Thanks.

Le 23/10/2015 18:30, Maeda a écrit :

...

Hi all !

I don't know if the bug is known from your team, but some people advice me to send an email to the Alsa devel list if nobody answer in the kernel's bugtracking.

Here is the bug I have : https://bugzilla.kernel.org/show_bug.cgi?id=105771

What do you think about it ?

Does the volume go down when you adjust DAC volume by mixer application when this happens?

Through a quick glance at the driver code, the only smelling part is that it's calling snd_rme96_reset_dac() from snd_rme96_playback_setrate(). If the DAC volume adjustment really works, the patch below might work. Please give it a try.

The patch has a code to give some delay that is commented out for now. The delay is found in the resume path, but I'm not sure whether it's mandatory. So let's try at first without the delay. If it's unstable, you can uncomment the line and retry.

thanks,

Takashi

---

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..a9d8a66fc3da 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,6 +741,9 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96);

• /* usleep_range(3000, 10000); */
  

• if (RME96_HAS_ANALOG_OUT(rme96))
  

• 	snd_rme96_apply_dac_volume(rme96);
  

  } else { writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER); }

[2 <text/html; utf-8 (8bit)>]

On Wed, 02 Dec 2015 21:43:48 +0100, Maeda wrote:

/So the patch prevents the wrong max value reset but it's still incomplete for the left channel volume?/

-> Yes, worst : it plays nothing on left channel -no loud at all ;)

OK. Just tried with the line 'usleep(range)' uncommented. It's better, no loud bug (just a big POP sound on speakers on first time, nothing after), but I have errors in output. I manage to get them with `dmesg`. See attachment.

Use mdelay(3) instead of usleep_range() call. I forgot that it's in a spinlock. It's not ideal, but good just for a test.

Takashi

No error nor bug when playing two times the same sample frequency, as expected... Playing alternatively r48000 and r96000 will output errors each time, despite of no bug, no loud sound, just expected play. Better then.

That seems not very 'clean'. What do you think about it ?

Le 02/12/2015 12:36, Takashi Iwai a écrit :

...

On Wed, 02 Dec 2015 11:11:27 +0100, Maeda wrote:

...

Hi Takashi !

Sorry for the late answer. I do compile the 'test' kernel after editing the file with the three lines as you said on your previous mail.

The result is that the bug isn't occurring anymore, tested with 96KHz sample :

`aplay -fcd -d3 /dev/zero ; aplay -fS32_LE -r96000 -c2 -d3 /dev/zero ; speaker-test -c2 -twav`

_BUT_, I only have the right speaker that works, I need to change the volume to restore sound on the left channel.

So the patch prevents the wrong max value reset but it's still incomplete for the left channel volume?

Did you try to enable the usleep_range() that was commented out in the patch?

Takashi

...

Maeda.

Le 21/11/2015 20:45, Takashi Iwai a écrit :

...

On Sat, 21 Nov 2015 18:42:06 +0100, Maeda wrote:

...

Hi !

Thanks for answering me. Yes, turning the output volume up or down as soon as it's playing at full output : it plays at the % it should play.

I'm OK to try the patch, but I don't find the /rme96.c/ file. Where is it located ? I think a recompilation's needed then ? Never done that.

Well, you need to learn how to compile the kernel. It's possible to compile only a module from the current running tree, too.

Which distro are you using? You can ask distro guys for assistance.

Takashi

...

Thanks for help.

Kind regards.

Le 20/11/2015 17:34, Takashi Iwai a écrit :

...

On Tue, 10 Nov 2015 13:41:44 +0100, Maeda wrote: > Hi ! > > Someone's here ? > > Thanks. > > Le 23/10/2015 18:30, Maeda a écrit : >> Hi all ! >> >> I don't know if the bug is known from your team, but some people >> advice me to send an email to the Alsa devel list if nobody answer in >> the kernel's bugtracking. >> >> Here is the bug I have : >> https://bugzilla.kernel.org/show_bug.cgi?id=105771 >> >> What do you think about it ? Does the volume go down when you adjust DAC volume by mixer application when this happens?

Through a quick glance at the driver code, the only smelling part is that it's calling snd_rme96_reset_dac() from snd_rme96_playback_setrate(). If the DAC volume adjustment really works, the patch below might work. Please give it a try.

The patch has a code to give some delay that is commented out for now. The delay is found in the resume path, but I'm not sure whether it's mandatory. So let's try at first without the delay. If it's unstable, you can uncomment the line and retry.

thanks,

Takashi

---

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..a9d8a66fc3da 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,6 +741,9 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96);

• /* usleep_range(3000, 10000); */
  

• if (RME96_HAS_ANALOG_OUT(rme96))
  

• 	snd_rme96_apply_dac_volume(rme96);
  } else {
  	writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER);
  }
  
  
  
  
  

[2 <text/html; utf-8 (8bit)>]

[ 713.579803] BUG: scheduling while atomic: aplay/2752/0x00000002 [ 713.579869] Modules linked in: joydev mousedev hidp ppdev parport_pc parport fuse vmw_vsock_vmci_transport vsock vmw_vmci cfg80211 ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack iptable_filter nct6775 hwmon_vid bnep snd_hda_codec_hdmi intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_codec_realtek snd_hda_codec_generic kvm_intel kvm eeepc_wmi snd_hda_intel asus_wmi iTCO_wdt crct10dif_pclmul btusb iTCO_vendor_support crc32_pclmul sparse_keymap snd_hda_codec crc32c_intel btrtl btbcm snd_hda_core btintel snd_rme96 snd_hwdep bluetooth snd_pcm aesni_intel input_leds aes_x86_64 r8169 snd_timer lrw led_class 8139too gf128mul snd mei_me glue_helper evdev ablk_helper 8139cp rfkill mac_hid cryptd mii mei [ 713.583684] psmouse soundcore lpc_ich i2c_i801 shpchp serio_raw pcspkr wmi thermal fan battery video processor button sch_fq_codel nfsd auth_rpcgss nfs oid_registry nfs_acl lockd grace sunrpc fscache ip_tables x_tables ext4 crc16 mbcache jbd2 hid_generic usbhid hid sd_mod sr_mod cdrom atkbd libps2 ahci libahci libata xhci_pci ehci_pci scsi_mod xhci_hcd ehci_hcd usbcore usb_common i8042 serio [ 713.586275] CPU: 6 PID: 2752 Comm: aplay Tainted: G W 4.2.6-ARCH-TESTRME96 #1 [ 713.586353] Hardware name: ASUS All Series/H87-PRO, BIOS 2102 07/29/2014 [ 713.586418] 0000000000000000 000000007e73a456 ffff880819b97b98 ffffffff81570f3a [ 713.586694] 0000000000000000 ffff88083ed95200 ffff880819b97ba8 ffffffff81099e9b [ 713.586968] ffff880819b97bf8 ffffffff815728cb 000000000000f800 ffff8807fcdfc4c0 [ 713.587239] Call Trace: [ 713.587307] [<ffffffff81570f3a>] dump_stack+0x4c/0x6e [ 713.587374] [<ffffffff81099e9b>] __schedule_bug+0x4b/0x60 [ 713.587441] [<ffffffff815728cb>] __schedule+0x89b/0xa00 [ 713.587508] [<ffffffff81572a6e>] schedule+0x3e/0x90 [ 713.587572] [<ffffffff81575737>] schedule_hrtimeout_range_clock.part.7+0x97/0x100 [ 713.587651] [<ffffffff810ddde0>] ? hrtimer_init+0x110/0x110 [ 713.587717] [<ffffffff8157572b>] ? schedule_hrtimeout_range_clock.part.7+0x8b/0x100 [ 713.587794] [<ffffffff815757b9>] schedule_hrtimeout_range_clock+0x19/0x40 [ 713.587861] [<ffffffff815757f3>] schedule_hrtimeout_range+0x13/0x20 [ 713.587927] [<ffffffff8157525f>] usleep_range+0x4f/0x70 [ 713.587994] [<ffffffffa056f0d9>] snd_rme96_playback_hw_params+0x1a9/0x330 [snd_rme96] [ 713.588072] [<ffffffffa055bb0b>] snd_pcm_hw_params+0xbb/0x380 [snd_pcm] [ 713.588139] [<ffffffff81175f46>] ? memdup_user+0x46/0x80 [ 713.588207] [<ffffffffa055d1df>] snd_pcm_common_ioctl1+0x34f/0xb50 [snd_pcm] [ 713.588275] [<ffffffffa055daf3>] snd_pcm_playback_ioctl1+0x113/0x270 [snd_pcm] [ 713.588352] [<ffffffffa055dc78>] snd_pcm_playback_ioctl+0x28/0x40 [snd_pcm] [ 713.588419] [<ffffffff811e2c35>] do_vfs_ioctl+0x295/0x480 [ 713.588485] [<ffffffff811e2e99>] SyS_ioctl+0x79/0x90 [ 713.588551] [<ffffffff815764ae>] entry_SYSCALL_64_fastpath+0x12/0x71 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

On Fri, 04 Dec 2015 11:44:02 +0100, Maeda wrote:

Hi !

Well done ! It plays without max loud volume, and no errors on output. I tried with the following test :

`aplay -fcd -d3 /dev/zero ;aplay -r96000 -c2 -d3 /dev/zero
;speaker-test  -c2 -twav`

And it's OK. Then I tried with mplayer and some flac files (44.1 / 48 and 96 KHz sample) : no problem.

I think you found the solution. If there are some future problem this patch generates, I'll only see when using "it" each day. Then, for now, you can commit it ;)

Good to hear. Could you try the revised patch below instead? It does restore DAC volume at the end, after the spinlock, so that we can avoid the ugly delay.

thanks,

Takashi

--- diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..e4229d01cf6a 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,10 +741,11 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96); + return 1; /* need to restore volume */ } else { writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER); + return 0; } - return 0; }

static int @@ -980,6 +981,7 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; int err, rate, dummy; + bool apply_dac_volume = false;

runtime->dma_area = (void __force *)(rme96->iobase + RME96_IO_PLAY_BUFFER); @@ -993,24 +995,24 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, { /* slave clock */ if ((int)params_rate(params) != rate) { - spin_unlock_irq(&rme96->lock); - return -EIO; - } - } else if ((err = snd_rme96_playback_setrate(rme96, params_rate(params))) < 0) { - spin_unlock_irq(&rme96->lock); - return err; - } - if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0) { - spin_unlock_irq(&rme96->lock); - return err; + err = -EIO; + goto error; + } + } else { + err = snd_rme96_playback_setrate(rme96, params_rate(params)); + if (err < 0) + goto error; + apply_dac_volume = err > 0; /* need to restore volume later? */ } + if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0) + goto error; snd_rme96_setframelog(rme96, params_channels(params), 1); if (rme96->capture_periodsize != 0) { if (params_period_size(params) << rme96->playback_frlog != rme96->capture_periodsize) { - spin_unlock_irq(&rme96->lock); - return -EBUSY; + err = -EBUSY; + goto error; } } rme96->playback_periodsize = @@ -1022,8 +1024,15 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, writel(rme96->wcreg |= rme96->wcreg_spdif_stream, rme96->iobase + RME96_IO_CONTROL_REGISTER); } spin_unlock_irq(&rme96->lock); + err = 0; - return 0; + error: + if (apply_dac_volume) { + usleep_range(3000, 10000); + snd_rme96_apply_dac_volume(rme96); + } + + return err; }

static int

On Fri, 04 Dec 2015 13:12:41 +0100, Maeda wrote:

I've problem with compiling rme96.c (see attachment). Maybe I forgot something, i double checked, but my rme96.c seems to be good.

Any clue ?

You must have patched wrongly. You reverted the former patches, right?

Takashi

Le 04/12/2015 12:19, Takashi Iwai a écrit :

...

On Fri, 04 Dec 2015 11:44:02 +0100, Maeda wrote:

...

Hi !

Well done ! It plays without max loud volume, and no errors on output. I tried with the following test :

 `aplay -fcd -d3 /dev/zero ;aplay -r96000 -c2 -d3 /dev/zero
 ;speaker-test  -c2 -twav`

And it's OK. Then I tried with mplayer and some flac files (44.1 / 48 and 96 KHz sample) : no problem.

I think you found the solution. If there are some future problem this patch generates, I'll only see when using "it" each day. Then, for now, you can commit it ;)

Good to hear. Could you try the revised patch below instead? It does restore DAC volume at the end, after the spinlock, so that we can avoid the ugly delay.

thanks,

Takashi

---

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..e4229d01cf6a 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,10 +741,11 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96);

• return 1; /* need to restore volume */
  

  } else { writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER);

• return 0;
  

  }

• return 0; }

  static int

@@ -980,6 +981,7 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; int err, rate, dummy;

• bool apply_dac_volume = false;

  runtime->dma_area = (void __force *)(rme96->iobase + RME96_IO_PLAY_BUFFER);

@@ -993,24 +995,24 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, { /* slave clock */ if ((int)params_rate(params) != rate) {

• 	spin_unlock_irq(&rme96->lock);
  

• 	return -EIO;
  

•            }
  

• } else if ((err = snd_rme96_playback_setrate(rme96, params_rate(params))) < 0) {

• spin_unlock_irq(&rme96->lock);
  

• return err;
  

• }
• if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0) {

• spin_unlock_irq(&rme96->lock);
  

• return err;
  

• 	err = -EIO;
  

• 	goto error;
  

• }
  

• } else {

• err = snd_rme96_playback_setrate(rme96, params_rate(params));
  

• if (err < 0)
  

• 	goto error;
  

• apply_dac_volume = err > 0; /* need to restore volume later? */
  

  }
• if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0)

• goto error;
  

  snd_rme96_setframelog(rme96, params_channels(params), 1); if (rme96->capture_periodsize != 0) { if (params_period_size(params) << rme96->playback_frlog != rme96->capture_periodsize) {

• 	spin_unlock_irq(&rme96->lock);
  

• 	return -EBUSY;
  

• 	err = -EBUSY;
  

• 	goto error;
  

  } } rme96->playback_periodsize =

@@ -1022,8 +1024,15 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, writel(rme96->wcreg |= rme96->wcreg_spdif_stream, rme96->iobase + RME96_IO_CONTROL_REGISTER); } spin_unlock_irq(&rme96->lock);

• err = 0;

• return 0;

• error:

• if (apply_dac_volume) {

• usleep_range(3000, 10000);
  

• snd_rme96_apply_dac_volume(rme96);
  

• }

• return err; }

  static int

CC [M] sound/pci/rme96.o sound/pci/rme96.c: Dans la fonction ‘snd_rme96_playback_hw_params’: sound/pci/rme96.c:1008:3: erreur: label ‘error’ used but not defined goto error; ^ sound/pci/rme96.c:1009:2: attention : « return » manquant dans une fonction devant retourner une valeur [-Wreturn-type] } ^ sound/pci/rme96.c: Hors de toute fonction : sound/pci/rme96.c:1010:46: erreur: expected ‘)’ before ‘(’ token snd_rme96_setframelog(rme96, params_channels(params), 1); ^ sound/pci/rme96.c:1011:2: erreur: expected identifier or ‘(’ before ‘if’ if (rme96->capture_periodsize != 0) { ^ sound/pci/rme96.c:1019:7: erreur: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token rme96->playback_periodsize = ^ sound/pci/rme96.c:1021:46: erreur: expected ‘)’ before ‘->’ token snd_rme96_set_period_properties(rme96, rme96->playback_periodsize); ^ sound/pci/rme96.c:1023:2: erreur: expected identifier or ‘(’ before ‘if’ if ((rme96->wcreg & RME96_WCR_ADAT) == 0) { ^ sound/pci/rme96.c:1027:18: erreur: expected declaration specifiers or ‘...’ before ‘&’ token spin_unlock_irq(&rme96->lock); ^ sound/pci/rme96.c:1028:2: attention : la définition de données n'a pas de type ni de classe de stockage err = 0; ^ sound/pci/rme96.c:1028:2: erreur: type defaults to ‘int’ in declaration of ‘err’ [-Werror=implicit-int] sound/pci/rme96.c:1029:7: erreur: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘:’ token error: ^ sound/pci/rme96.c:1035:2: erreur: expected identifier or ‘(’ before ‘return’ return err; ^ sound/pci/rme96.c:1036:1: erreur: expected identifier or ‘(’ before ‘}’ token }

Last patch applied and all's right. What next ?

Thanks.

Le 04/12/2015 13:47, Takashi Iwai a écrit :

On Fri, 04 Dec 2015 13:12:41 +0100, Maeda wrote:

...

I've problem with compiling rme96.c (see attachment). Maybe I forgot something, i double checked, but my rme96.c seems to be good.

Any clue ?

You must have patched wrongly. You reverted the former patches, right?

Takashi

...

Le 04/12/2015 12:19, Takashi Iwai a écrit :

...

On Fri, 04 Dec 2015 11:44:02 +0100, Maeda wrote:

...

Hi !

Well done ! It plays without max loud volume, and no errors on output. I tried with the following test :

  `aplay -fcd -d3 /dev/zero ;aplay -r96000 -c2 -d3 /dev/zero
  ;speaker-test  -c2 -twav`

And it's OK. Then I tried with mplayer and some flac files (44.1 / 48 and 96 KHz sample) : no problem.

I think you found the solution. If there are some future problem this patch generates, I'll only see when using "it" each day. Then, for now, you can commit it ;)

Good to hear. Could you try the revised patch below instead? It does restore DAC volume at the end, after the spinlock, so that we can avoid the ugly delay.

thanks,

Takashi

---

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..e4229d01cf6a 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,10 +741,11 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96);

• return 1; /* need to restore volume */
  

  } else { writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER);

• return 0;
  

  }

• return 0; }

  static int

@@ -980,6 +981,7 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; int err, rate, dummy;

• bool apply_dac_volume = false;

  runtime->dma_area = (void __force *)(rme96->iobase + RME96_IO_PLAY_BUFFER);

@@ -993,24 +995,24 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, { /* slave clock */ if ((int)params_rate(params) != rate) {

• 	spin_unlock_irq(&rme96->lock);
  

• 	return -EIO;
  

•            }
  

• } else if ((err = snd_rme96_playback_setrate(rme96, params_rate(params))) < 0) {

• spin_unlock_irq(&rme96->lock);
  

• return err;
  

• }
• if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0) {

• spin_unlock_irq(&rme96->lock);
  

• return err;
  

• 	err = -EIO;
  

• 	goto error;
  

• }
  

• } else {

• err = snd_rme96_playback_setrate(rme96, params_rate(params));
  

• if (err < 0)
  

• 	goto error;
  

• apply_dac_volume = err > 0; /* need to restore volume later? */
  

  }
• if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0)

• goto error;
  

  snd_rme96_setframelog(rme96, params_channels(params), 1); if (rme96->capture_periodsize != 0) { if (params_period_size(params) << rme96->playback_frlog != rme96->capture_periodsize) {

• 	spin_unlock_irq(&rme96->lock);
  

• 	return -EBUSY;
  

• 	err = -EBUSY;
  

• 	goto error;
  }
  

  } rme96->playback_periodsize =

@@ -1022,8 +1024,15 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, writel(rme96->wcreg |= rme96->wcreg_spdif_stream, rme96->iobase + RME96_IO_CONTROL_REGISTER); } spin_unlock_irq(&rme96->lock);

• err = 0;

• return 0;

• error:

• if (apply_dac_volume) {

• usleep_range(3000, 10000);
  

• snd_rme96_apply_dac_volume(rme96);
  

• }

• return err; }

  static int

CC [M] sound/pci/rme96.o sound/pci/rme96.c: Dans la fonction ‘snd_rme96_playback_hw_params’: sound/pci/rme96.c:1008:3: erreur: label ‘error’ used but not defined goto error; ^ sound/pci/rme96.c:1009:2: attention : « return » manquant dans une fonction devant retourner une valeur [-Wreturn-type] } ^ sound/pci/rme96.c: Hors de toute fonction : sound/pci/rme96.c:1010:46: erreur: expected ‘)’ before ‘(’ token snd_rme96_setframelog(rme96, params_channels(params), 1); ^ sound/pci/rme96.c:1011:2: erreur: expected identifier or ‘(’ before ‘if’ if (rme96->capture_periodsize != 0) { ^ sound/pci/rme96.c:1019:7: erreur: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token rme96->playback_periodsize = ^ sound/pci/rme96.c:1021:46: erreur: expected ‘)’ before ‘->’ token snd_rme96_set_period_properties(rme96, rme96->playback_periodsize); ^ sound/pci/rme96.c:1023:2: erreur: expected identifier or ‘(’ before ‘if’ if ((rme96->wcreg & RME96_WCR_ADAT) == 0) { ^ sound/pci/rme96.c:1027:18: erreur: expected declaration specifiers or ‘...’ before ‘&’ token spin_unlock_irq(&rme96->lock); ^ sound/pci/rme96.c:1028:2: attention : la définition de données n'a pas de type ni de classe de stockage err = 0; ^ sound/pci/rme96.c:1028:2: erreur: type defaults to ‘int’ in declaration of ‘err’ [-Werror=implicit-int] sound/pci/rme96.c:1029:7: erreur: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘:’ token error: ^ sound/pci/rme96.c:1035:2: erreur: expected identifier or ‘(’ before ‘return’ return err; ^ sound/pci/rme96.c:1036:1: erreur: expected identifier or ‘(’ before ‘}’ token }

I'm not use to do that. Do you mean adding a tags in the bug I opened (https://bugzilla.kernel.org/show_bug.cgi?id=105771) ?

Le 04/12/2015 15:36, Takashi Iwai a écrit :

On Fri, 04 Dec 2015 15:24:44 +0100, Maeda wrote:

...

Last patch applied and all's right. What next ?

At best, give your proper tested-by tag. It's like

Tested-by: Your Real Name mail@address

It's included in the git commit log as your contribution. Then I'll merge the fix into the upstream.

thanks,

Takashi

...

Thanks.

Le 04/12/2015 13:47, Takashi Iwai a écrit :

...

On Fri, 04 Dec 2015 13:12:41 +0100, Maeda wrote:

...

I've problem with compiling rme96.c (see attachment). Maybe I forgot something, i double checked, but my rme96.c seems to be good.

Any clue ?

You must have patched wrongly. You reverted the former patches, right?

Takashi

...

Le 04/12/2015 12:19, Takashi Iwai a écrit :

...

On Fri, 04 Dec 2015 11:44:02 +0100, Maeda wrote:

...

Hi !

Well done ! It plays without max loud volume, and no errors on output. I tried with the following test :

   `aplay -fcd -d3 /dev/zero ;aplay -r96000 -c2 -d3 /dev/zero
   ;speaker-test  -c2 -twav`

And it's OK. Then I tried with mplayer and some flac files (44.1 / 48 and 96 KHz sample) : no problem.

I think you found the solution. If there are some future problem this patch generates, I'll only see when using "it" each day. Then, for now, you can commit it ;)

Good to hear. Could you try the revised patch below instead? It does restore DAC volume at the end, after the spinlock, so that we can avoid the ugly delay.

thanks,

Takashi

---

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..e4229d01cf6a 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,10 +741,11 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96);

• return 1; /* need to restore volume */
  } else {
  	writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER);
  

• return 0;
  }
  

• return 0; }

  static int

@@ -980,6 +981,7 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; int err, rate, dummy;

• bool apply_dac_volume = false;

  runtime->dma_area = (void __force *)(rme96->iobase +
  				     RME96_IO_PLAY_BUFFER);
  

@@ -993,24 +995,24 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, { /* slave clock */ if ((int)params_rate(params) != rate) {

• 	spin_unlock_irq(&rme96->lock);
  

• 	return -EIO;
  

•            }
  

• } else if ((err = snd_rme96_playback_setrate(rme96, params_rate(params))) < 0) {

• spin_unlock_irq(&rme96->lock);
  

• return err;
  

• }
• if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0) {

• spin_unlock_irq(&rme96->lock);
  

• return err;
  

• 	err = -EIO;
  

• 	goto error;
  

• }
  

• } else {

• err = snd_rme96_playback_setrate(rme96, params_rate(params));
  

• if (err < 0)
  

• 	goto error;
  

• apply_dac_volume = err > 0; /* need to restore volume later? */
  }
  

• if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0)

• goto error;
  snd_rme96_setframelog(rme96, params_channels(params), 1);
  if (rme96->capture_periodsize != 0) {
  	if (params_period_size(params) << rme96->playback_frlog !=
  	    rme96->capture_periodsize)
  	{
  

• 	spin_unlock_irq(&rme96->lock);
  

• 	return -EBUSY;
  

• 	err = -EBUSY;
  

• 	goto error;
  	}
  }
  rme96->playback_periodsize =
  

@@ -1022,8 +1024,15 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, writel(rme96->wcreg |= rme96->wcreg_spdif_stream, rme96->iobase + RME96_IO_CONTROL_REGISTER); } spin_unlock_irq(&rme96->lock);

• err = 0;

• return 0;

• error:

• if (apply_dac_volume) {

• usleep_range(3000, 10000);
  

• snd_rme96_apply_dac_volume(rme96);
  

• }

• return err; }

  static int

CC [M]  sound/pci/rme96.o

sound/pci/rme96.c: Dans la fonction ‘snd_rme96_playback_hw_params’: sound/pci/rme96.c:1008:3: erreur: label ‘error’ used but not defined goto error; ^ sound/pci/rme96.c:1009:2: attention : « return » manquant dans une fonction devant retourner une valeur [-Wreturn-type] } ^ sound/pci/rme96.c: Hors de toute fonction : sound/pci/rme96.c:1010:46: erreur: expected ‘)’ before ‘(’ token snd_rme96_setframelog(rme96, params_channels(params), 1); ^ sound/pci/rme96.c:1011:2: erreur: expected identifier or ‘(’ before ‘if’ if (rme96->capture_periodsize != 0) { ^ sound/pci/rme96.c:1019:7: erreur: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token rme96->playback_periodsize = ^ sound/pci/rme96.c:1021:46: erreur: expected ‘)’ before ‘->’ token snd_rme96_set_period_properties(rme96, rme96->playback_periodsize); ^ sound/pci/rme96.c:1023:2: erreur: expected identifier or ‘(’ before ‘if’ if ((rme96->wcreg & RME96_WCR_ADAT) == 0) { ^ sound/pci/rme96.c:1027:18: erreur: expected declaration specifiers or ‘...’ before ‘&’ token spin_unlock_irq(&rme96->lock); ^ sound/pci/rme96.c:1028:2: attention : la définition de données n'a pas de type ni de classe de stockage err = 0; ^ sound/pci/rme96.c:1028:2: erreur: type defaults to ‘int’ in declaration of ‘err’ [-Werror=implicit-int] sound/pci/rme96.c:1029:7: erreur: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘:’ token error: ^ sound/pci/rme96.c:1035:2: erreur: expected identifier or ‘(’ before ‘return’ return err; ^ sound/pci/rme96.c:1036:1: erreur: expected identifier or ‘(’ before ‘}’ token }

OK :)

Tested-by: Sylvain LABOISNE maeda1@free.fr

Le 04/12/2015 16:08, Takashi Iwai a écrit :

On Fri, 04 Dec 2015 16:01:59 +0100, Maeda wrote:

...

I'm not use to do that. Do you mean adding a tags in the bug I opened (https://bugzilla.kernel.org/show_bug.cgi?id=105771) ?

Much simpler. Just drop me a mail with the line including your Tested-by tag text.

Takashi

...

Le 04/12/2015 15:36, Takashi Iwai a écrit :

...

On Fri, 04 Dec 2015 15:24:44 +0100, Maeda wrote:

...

Last patch applied and all's right. What next ?

At best, give your proper tested-by tag. It's like

Tested-by: Your Real Name mail@address

It's included in the git commit log as your contribution. Then I'll merge the fix into the upstream.

thanks,

Takashi

...

Thanks.

Le 04/12/2015 13:47, Takashi Iwai a écrit :

...

On Fri, 04 Dec 2015 13:12:41 +0100, Maeda wrote:

...

I've problem with compiling rme96.c (see attachment). Maybe I forgot something, i double checked, but my rme96.c seems to be good.

Any clue ?

You must have patched wrongly. You reverted the former patches, right?

Takashi

...

Le 04/12/2015 12:19, Takashi Iwai a écrit : > On Fri, 04 Dec 2015 11:44:02 +0100, > Maeda wrote: >> Hi ! >> >> Well done ! It plays without max loud volume, and no errors on output. >> I tried with the following test : >> >> `aplay -fcd -d3 /dev/zero ;aplay -r96000 -c2 -d3 /dev/zero >> ;speaker-test -c2 -twav` >> >> And it's OK. Then I tried with mplayer and some flac files (44.1 / 48 >> and 96 KHz sample) : no problem. >> >> I think you found the solution. If there are some future problem this >> patch generates, I'll only see when using "it" each day. Then, for now, >> you can commit it ;) > Good to hear. Could you try the revised patch below instead? > It does restore DAC volume at the end, after the spinlock, so that we > can avoid the ugly delay. > > > thanks, > > Takashi > > --- > diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c > index 714df906249e..e4229d01cf6a 100644 > --- a/sound/pci/rme96.c > +++ b/sound/pci/rme96.c > @@ -741,10 +741,11 @@ snd_rme96_playback_setrate(struct rme96 *rme96, > { > /* change to/from double-speed: reset the DAC (if available) */ > snd_rme96_reset_dac(rme96); > + return 1; /* need to restore volume */ > } else { > writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER); > + return 0; > } > - return 0; > } > > static int > @@ -980,6 +981,7 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, > struct rme96 *rme96 = snd_pcm_substream_chip(substream); > struct snd_pcm_runtime *runtime = substream->runtime; > int err, rate, dummy; > + bool apply_dac_volume = false; > > runtime->dma_area = (void __force *)(rme96->iobase + > RME96_IO_PLAY_BUFFER); > @@ -993,24 +995,24 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, > { > /* slave clock */ > if ((int)params_rate(params) != rate) { > - spin_unlock_irq(&rme96->lock); > - return -EIO; > - } > - } else if ((err = snd_rme96_playback_setrate(rme96, params_rate(params))) < 0) { > - spin_unlock_irq(&rme96->lock); > - return err; > - } > - if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0) { > - spin_unlock_irq(&rme96->lock); > - return err; > + err = -EIO; > + goto error; > + } > + } else { > + err = snd_rme96_playback_setrate(rme96, params_rate(params)); > + if (err < 0) > + goto error; > + apply_dac_volume = err > 0; /* need to restore volume later? */ > } > + if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0) > + goto error; > snd_rme96_setframelog(rme96, params_channels(params), 1); > if (rme96->capture_periodsize != 0) { > if (params_period_size(params) << rme96->playback_frlog != > rme96->capture_periodsize) > { > - spin_unlock_irq(&rme96->lock); > - return -EBUSY; > + err = -EBUSY; > + goto error; > } > } > rme96->playback_periodsize = > @@ -1022,8 +1024,15 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, > writel(rme96->wcreg |= rme96->wcreg_spdif_stream, rme96->iobase + RME96_IO_CONTROL_REGISTER); > } > spin_unlock_irq(&rme96->lock); > + err = 0; > > - return 0; > + error: > + if (apply_dac_volume) { > + usleep_range(3000, 10000); > + snd_rme96_apply_dac_volume(rme96); > + } > + > + return err; > } > > static int CC [M] sound/pci/rme96.o sound/pci/rme96.c: Dans la fonction ‘snd_rme96_playback_hw_params’: sound/pci/rme96.c:1008:3: erreur: label ‘error’ used but not defined goto error; ^ sound/pci/rme96.c:1009:2: attention : « return » manquant dans une fonction devant retourner une valeur [-Wreturn-type] } ^ sound/pci/rme96.c: Hors de toute fonction : sound/pci/rme96.c:1010:46: erreur: expected ‘)’ before ‘(’ token snd_rme96_setframelog(rme96, params_channels(params), 1); ^ sound/pci/rme96.c:1011:2: erreur: expected identifier or ‘(’ before ‘if’ if (rme96->capture_periodsize != 0) { ^ sound/pci/rme96.c:1019:7: erreur: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token rme96->playback_periodsize = ^ sound/pci/rme96.c:1021:46: erreur: expected ‘)’ before ‘->’ token snd_rme96_set_period_properties(rme96, rme96->playback_periodsize); ^ sound/pci/rme96.c:1023:2: erreur: expected identifier or ‘(’ before ‘if’ if ((rme96->wcreg & RME96_WCR_ADAT) == 0) { ^ sound/pci/rme96.c:1027:18: erreur: expected declaration specifiers or ‘...’ before ‘&’ token spin_unlock_irq(&rme96->lock); ^ sound/pci/rme96.c:1028:2: attention : la définition de données n'a pas de type ni de classe de stockage err = 0; ^ sound/pci/rme96.c:1028:2: erreur: type defaults to ‘int’ in declaration of ‘err’ [-Werror=implicit-int] sound/pci/rme96.c:1029:7: erreur: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘:’ token error: ^ sound/pci/rme96.c:1035:2: erreur: expected identifier or ‘(’ before ‘return’ return err; ^ sound/pci/rme96.c:1036:1: erreur: expected identifier or ‘(’ before ‘}’ token }

Good job :) Thanks for help and debug.

I'm waiting for the release then. I don't log anything to the bug report I did (https://bugzilla.kernel.org/show_bug.cgi?id=105771). Let me know if I have to do something in addition.

Le 04/12/2015 16:51, Takashi Iwai a écrit :

On Fri, 04 Dec 2015 16:14:56 +0100, Maeda wrote:

...

OK :)

Tested-by: Sylvain LABOISNE maeda1@free.fr

Thanks, the official fix is below. It'll be included in 4.4-rc5 (slipped from rc4) and backported to stable kernels later.

Takashi

-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: rme96: Fix unexpected volume reset after rate changes

rme96 driver needs to reset DAC depending on the sample rate, and this results in resetting to the max volume suddenly. It's because of the missing call of snd_rme96_apply_dac_volume().

However, calling this function right after the DAC reset still may not work, and we need some delay before this call. Since the DAC reset and the procedure after that are performed in the spinlock, we delay the DAC volume restore at the end after the spinlock.

Reported-and-tested-by: Sylvain LABOISNE maeda1@free.fr Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de

---

sound/pci/rme96.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 714df906249e..e4229d01cf6a 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -741,10 +741,11 @@ snd_rme96_playback_setrate(struct rme96 *rme96, { /* change to/from double-speed: reset the DAC (if available) */ snd_rme96_reset_dac(rme96);

• return 1; /* need to restore volume */
  

  } else { writel(rme96->wcreg, rme96->iobase + RME96_IO_CONTROL_REGISTER);

• return 0;
  

  }

• return 0; }

  static int

@@ -980,6 +981,7 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; int err, rate, dummy;

• bool apply_dac_volume = false;

  runtime->dma_area = (void __force *)(rme96->iobase + RME96_IO_PLAY_BUFFER);

@@ -993,24 +995,24 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, { /* slave clock */ if ((int)params_rate(params) != rate) {

• 	spin_unlock_irq(&rme96->lock);
  

• 	return -EIO;
  

•            }
  

• } else if ((err = snd_rme96_playback_setrate(rme96, params_rate(params))) < 0) {

• spin_unlock_irq(&rme96->lock);
  

• return err;
  

• }
• if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0) {

• spin_unlock_irq(&rme96->lock);
  

• return err;
  

• 	err = -EIO;
  

• 	goto error;
  

• }
  

• } else {

• err = snd_rme96_playback_setrate(rme96, params_rate(params));
  

• if (err < 0)
  

• 	goto error;
  

• apply_dac_volume = err > 0; /* need to restore volume later? */
  

  }
• if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0)

• goto error;
  

  snd_rme96_setframelog(rme96, params_channels(params), 1); if (rme96->capture_periodsize != 0) { if (params_period_size(params) << rme96->playback_frlog != rme96->capture_periodsize) {

• 	spin_unlock_irq(&rme96->lock);
  

• 	return -EBUSY;
  

• 	err = -EBUSY;
  

• 	goto error;
  

  } } rme96->playback_periodsize =

@@ -1022,8 +1024,15 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream, writel(rme96->wcreg |= rme96->wcreg_spdif_stream, rme96->iobase + RME96_IO_CONTROL_REGISTER); } spin_unlock_irq(&rme96->lock);

• err = 0;

• return 0;

• error:

• if (apply_dac_volume) {

• usleep_range(3000, 10000);
  

• snd_rme96_apply_dac_volume(rme96);
  

• }

• return err; }

  static int