Message ID | s5h8u5ayzyb.wl-tiwai@suse.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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