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

[Maeda <maeda1@free.fr>]

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