* Re: [sound:for-linus 29/29] sound/pci/rme96.c:1035:1-7: preceding lock on line 991
[not found] <201512050138.Hx1BUH8H%fengguang.wu@intel.com>
@ 2015-12-04 17:39 ` Julia Lawall
2015-12-04 19:42 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: Julia Lawall @ 2015-12-04 17:39 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, kbuild-all
Looks suspicious. Please check.
julia
On Sat, 5 Dec 2015, kbuild test robot wrote:
> CC: kbuild-all@01.org
> CC: alsa-devel@alsa-project.org
> TO: Takashi Iwai <tiwai@suse.de>
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-linus
> head: ec20c9c7a63f7b8676eb574406a8bc8f32b7ba7e
> commit: ec20c9c7a63f7b8676eb574406a8bc8f32b7ba7e [29/29] ALSA: rme96: Fix unexpected volume reset after rate changes
> :::::: branch date: 2 hours ago
> :::::: commit date: 2 hours ago
>
> >> sound/pci/rme96.c:1035:1-7: preceding lock on line 991
>
> git remote add sound https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> git remote update sound
> git checkout ec20c9c7a63f7b8676eb574406a8bc8f32b7ba7e
> vim +1035 sound/pci/rme96.c
>
> ^1da177e Linus Torvalds 2005-04-16 985
> 4d23359b Clemens Ladisch 2005-09-05 986 runtime->dma_area = (void __force *)(rme96->iobase +
> 4d23359b Clemens Ladisch 2005-09-05 987 RME96_IO_PLAY_BUFFER);
> ^1da177e Linus Torvalds 2005-04-16 988 runtime->dma_addr = rme96->port + RME96_IO_PLAY_BUFFER;
> ^1da177e Linus Torvalds 2005-04-16 989 runtime->dma_bytes = RME96_BUFFER_SIZE;
> ^1da177e Linus Torvalds 2005-04-16 990
> ^1da177e Linus Torvalds 2005-04-16 @991 spin_lock_irq(&rme96->lock);
> ^1da177e Linus Torvalds 2005-04-16 992 if (!(rme96->wcreg & RME96_WCR_MASTER) &&
> ^1da177e Linus Torvalds 2005-04-16 993 snd_rme96_getinputtype(rme96) != RME96_INPUT_ANALOG &&
> ^1da177e Linus Torvalds 2005-04-16 994 (rate = snd_rme96_capture_getrate(rme96, &dummy)) > 0)
> ^1da177e Linus Torvalds 2005-04-16 995 {
> ^1da177e Linus Torvalds 2005-04-16 996 /* slave clock */
> ^1da177e Linus Torvalds 2005-04-16 997 if ((int)params_rate(params) != rate) {
> ec20c9c7 Takashi Iwai 2015-12-04 998 err = -EIO;
> ec20c9c7 Takashi Iwai 2015-12-04 999 goto error;
> ^1da177e Linus Torvalds 2005-04-16 1000 }
> ec20c9c7 Takashi Iwai 2015-12-04 1001 } else {
> ec20c9c7 Takashi Iwai 2015-12-04 1002 err = snd_rme96_playback_setrate(rme96, params_rate(params));
> ec20c9c7 Takashi Iwai 2015-12-04 1003 if (err < 0)
> ec20c9c7 Takashi Iwai 2015-12-04 1004 goto error;
> ec20c9c7 Takashi Iwai 2015-12-04 1005 apply_dac_volume = err > 0; /* need to restore volume later? */
> ^1da177e Linus Torvalds 2005-04-16 1006 }
> ec20c9c7 Takashi Iwai 2015-12-04 1007 if ((err = snd_rme96_playback_setformat(rme96, params_format(params))) < 0)
> ec20c9c7 Takashi Iwai 2015-12-04 1008 goto error;
> ^1da177e Linus Torvalds 2005-04-16 1009 snd_rme96_setframelog(rme96, params_channels(params), 1);
> ^1da177e Linus Torvalds 2005-04-16 1010 if (rme96->capture_periodsize != 0) {
> ^1da177e Linus Torvalds 2005-04-16 1011 if (params_period_size(params) << rme96->playback_frlog !=
> ^1da177e Linus Torvalds 2005-04-16 1012 rme96->capture_periodsize)
> ^1da177e Linus Torvalds 2005-04-16 1013 {
> ec20c9c7 Takashi Iwai 2015-12-04 1014 err = -EBUSY;
> ec20c9c7 Takashi Iwai 2015-12-04 1015 goto error;
> ^1da177e Linus Torvalds 2005-04-16 1016 }
> ^1da177e Linus Torvalds 2005-04-16 1017 }
> ^1da177e Linus Torvalds 2005-04-16 1018 rme96->playback_periodsize =
> ^1da177e Linus Torvalds 2005-04-16 1019 params_period_size(params) << rme96->playback_frlog;
> ^1da177e Linus Torvalds 2005-04-16 1020 snd_rme96_set_period_properties(rme96, rme96->playback_periodsize);
> ^1da177e Linus Torvalds 2005-04-16 1021 /* S/PDIF setup */
> ^1da177e Linus Torvalds 2005-04-16 1022 if ((rme96->wcreg & RME96_WCR_ADAT) == 0) {
> ^1da177e Linus Torvalds 2005-04-16 1023 rme96->wcreg &= ~(RME96_WCR_PRO | RME96_WCR_DOLBY | RME96_WCR_EMP);
> ^1da177e Linus Torvalds 2005-04-16 1024 writel(rme96->wcreg |= rme96->wcreg_spdif_stream, rme96->iobase + RME96_IO_CONTROL_REGISTER);
> ^1da177e Linus Torvalds 2005-04-16 1025 }
> ^1da177e Linus Torvalds 2005-04-16 1026 spin_unlock_irq(&rme96->lock);
> ec20c9c7 Takashi Iwai 2015-12-04 1027 err = 0;
> ^1da177e Linus Torvalds 2005-04-16 1028
> ec20c9c7 Takashi Iwai 2015-12-04 1029 error:
> ec20c9c7 Takashi Iwai 2015-12-04 1030 if (apply_dac_volume) {
> ec20c9c7 Takashi Iwai 2015-12-04 1031 usleep_range(3000, 10000);
> ec20c9c7 Takashi Iwai 2015-12-04 1032 snd_rme96_apply_dac_volume(rme96);
> ec20c9c7 Takashi Iwai 2015-12-04 1033 }
> ec20c9c7 Takashi Iwai 2015-12-04 1034
> ec20c9c7 Takashi Iwai 2015-12-04 @1035 return err;
> ^1da177e Linus Torvalds 2005-04-16 1036 }
> ^1da177e Linus Torvalds 2005-04-16 1037
> ^1da177e Linus Torvalds 2005-04-16 1038 static int
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [sound:for-linus 29/29] sound/pci/rme96.c:1035:1-7: preceding lock on line 991
2015-12-04 17:39 ` [sound:for-linus 29/29] sound/pci/rme96.c:1035:1-7: preceding lock on line 991 Julia Lawall
@ 2015-12-04 19:42 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2015-12-04 19:42 UTC (permalink / raw)
To: Julia Lawall; +Cc: alsa-devel, kbuild-all
On Fri, 04 Dec 2015 18:39:17 +0100,
Julia Lawall wrote:
>
> Looks suspicious. Please check.
Gah, indeed it was a crap. Fixed now as below.
Thanks for catching it.
Takashi
-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH v2] 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 | 41 ++++++++++++++++++++++++++---------------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c
index 714df906249e..41c31db65039 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,26 @@ 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? */
}
+
+ err = snd_rme96_playback_setformat(rme96, params_format(params));
+ if (err < 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 =
@@ -1021,9 +1025,16 @@ snd_rme96_playback_hw_params(struct snd_pcm_substream *substream,
rme96->wcreg &= ~(RME96_WCR_PRO | RME96_WCR_DOLBY | RME96_WCR_EMP);
writel(rme96->wcreg |= rme96->wcreg_spdif_stream, rme96->iobase + RME96_IO_CONTROL_REGISTER);
}
+
+ err = 0;
+ error:
spin_unlock_irq(&rme96->lock);
-
- return 0;
+ if (apply_dac_volume) {
+ usleep_range(3000, 10000);
+ snd_rme96_apply_dac_volume(rme96);
+ }
+
+ return err;
}
static int
--
2.6.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-12-04 19:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <201512050138.Hx1BUH8H%fengguang.wu@intel.com>
2015-12-04 17:39 ` [sound:for-linus 29/29] sound/pci/rme96.c:1035:1-7: preceding lock on line 991 Julia Lawall
2015-12-04 19:42 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.