Re: [bug] Volume at maximum when track with different frequency is played with my RME sound card

From: Takashi Iwai <tiwai@suse.de>
To: Maeda <maeda1@free.fr>
Cc: alsa-devel@alsa-project.org
Subject: Re: [bug] Volume at maximum when track with different frequency is played with my RME sound card
Date: Fri, 04 Dec 2015 12:19:37 +0100	[thread overview]
Message-ID: <s5h4mfy1mwm.wl-tiwai@suse.de> (raw)
In-Reply-To: <56616E72.20407@free.fr>

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
