[alsa-devel] [bug] Volume at maximum when track with different frequency is played with my RME sound card

[Maeda]

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
>>   }