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 16:08:57 +0100	[thread overview]
Message-ID: <s5hfuziz1x2.wl-tiwai@suse.de> (raw)
In-Reply-To: <5661AAE7.9080806@free.fr>

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
> >>>>    }
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel