[alsa-devel] [PATCH 0/4] ALSA: Updates for the CA0132 codec
Takashi Iwai
tiwai at suse.de
Fri Mar 15 09:25:27 CET 2013
At Thu, 14 Mar 2013 17:34:34 -0700,
Dylan Reid wrote:
>
> On Sun, Feb 10, 2013 at 2:57 AM, Takashi Iwai <tiwai at suse.de> wrote:
> > Hi Ian,
> >
> > At Fri, 8 Feb 2013 18:31:41 -0800,
> > Ian Minett wrote:
> >>
> >> From: Ian Minett <ian_minett at creativelabs.com>
> >>
> >> Hi,
> >>
> >> This series contains the following updates to CA0132:
> >>
> >> 1: Add firmware loading for the SpeakerEQ DSP effect.
> >> 2: Improve DSP transfer timeout calculations.
> >> 3: Add stream and effect latency monitoring routines
> >> 4: Update hp unsolicited event handler to manage queued work.
> >
> > Thanks, I took the one now but leave others. See my reply to each.
> >
> >>
> >> These were based on the latest sound.git, but please let me know if
> >> they should be against sound-unstable tree instead.
> >
> > sound.git tree for-next branch would be the place to look at.
> > If I start a new branch for ca0132, I'll let you know.
> >
> >
> > But, before going further: as I already mailed you, I could get a test
> > board and started testing. The result wasn't good. At the first time
> > the driver loads, the DSP loading fails always. When the driver is
> > reloaded after that, it's working. So, something is still pretty
> > fragile.
>
> I tried this today and I can't get it to work. It is failing in
> snd_hda_lock_devices called from azx_load_dsp_prepare. The DSP is
> loaded from ca0132_init which is called as the result of open/resume.
> Both the check for open control files and attached substreams are
> failing. How can that be avoided?
snd_hda_lock_devices() is obviously not suitable for the power-saving
or PM resume. OK, this must be fixed indeed...
How about the patch below? (Note that it's just compile tested.)
thanks,
Takashi
---
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] ALSA: hda - Fix abuse of snd_hda_lock_devices() for DSP
loader
The current DSP loader code abuses snd_hda_lock_devices() for ensuring
the DSP loader not conflicting with the other normal operations. But
this trick obviously doesn't work for the PM resume since the streams
are kept opened there where snd_hda_lock_devices() returns -EBUSY.
That means we need another lock mechanism instead of abuse.
This patch provides the new lock state to azx_dev. Theoretically it's
possible that the DSP loader conflicts with the stream that has been
already assigned for another PCM. If it's running, the DSP loader
should simply fail. If not -- it's the case for PM resume --, we
should assign this stream temporarily to the DSP loader, and take it
back to the PCM after finishing DSP loading. If the PCM is operated
during the DSP loading, it should get an error, too.
Reported-by: Dylan Reid <dgreid at chromium.org>
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
sound/pci/hda/hda_intel.c | 119 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 100 insertions(+), 19 deletions(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 4cea6bb6..61abf8b 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -415,6 +415,8 @@ struct azx_dev {
unsigned int opened :1;
unsigned int running :1;
unsigned int irq_pending :1;
+ unsigned int prepared:1;
+ unsigned int locked:1;
/*
* For VIA:
* A flag to ensure DMA position is 0
@@ -426,8 +428,25 @@ struct azx_dev {
struct timecounter azx_tc;
struct cyclecounter azx_cc;
+
+#ifdef CONFIG_SND_HDA_DSP_LOADER
+ struct mutex dsp_mutex;
+#endif
};
+/* DSP lock helpers */
+#ifdef CONFIG_SND_HDA_DSP_LOADER
+#define dsp_lock_init(dev) mutex_init(&(dev)->dsp_mutex)
+#define dsp_lock(dev) mutex_lock(&(dev)->dsp_mutex)
+#define dsp_unlock(dev) mutex_unlock(&(dev)->dsp_mutex)
+#define dsp_is_locked(dev) ((dev)->locked)
+#else
+#define dsp_lock_init(dev) do {} while (0)
+#define dsp_lock(dev) do {} while (0)
+#define dsp_unlock(dev) do {} while (0)
+#define dsp_is_locked(dev) 0
+#endif
+
/* CORB/RIRB */
struct azx_rb {
u32 *buf; /* CORB/RIRB buffer
@@ -527,6 +546,10 @@ struct azx {
/* card list (for power_save trigger) */
struct list_head list;
+
+#ifdef CONFIG_SND_HDA_DSP_LOADER
+ struct azx_dev saved_azx_dev;
+#endif
};
#define CREATE_TRACE_POINTS
@@ -1794,7 +1817,8 @@ azx_assign_device(struct azx *chip, struct snd_pcm_substream *substream)
nums = chip->capture_streams;
}
for (i = 0; i < nums; i++, dev++)
- if (!chip->azx_dev[dev].opened) {
+ if (!chip->azx_dev[dev].opened &&
+ !dsp_is_locked(&chip->azx_dev[dev])) {
res = &chip->azx_dev[dev];
if (res->assigned_key == key)
break;
@@ -2009,6 +2033,12 @@ static int azx_pcm_hw_params(struct snd_pcm_substream *substream,
struct azx_dev *azx_dev = get_azx_dev(substream);
int ret;
+ dsp_lock(azx_dev);
+ if (dsp_is_locked(azx_dev)) {
+ ret = -EBUSY;
+ goto unlock;
+ }
+
mark_runtime_wc(chip, azx_dev, substream, false);
azx_dev->bufsize = 0;
azx_dev->period_bytes = 0;
@@ -2016,8 +2046,10 @@ static int azx_pcm_hw_params(struct snd_pcm_substream *substream,
ret = snd_pcm_lib_malloc_pages(substream,
params_buffer_bytes(hw_params));
if (ret < 0)
- return ret;
+ goto unlock;
mark_runtime_wc(chip, azx_dev, substream, true);
+ unlock:
+ dsp_unlock(azx_dev);
return ret;
}
@@ -2029,16 +2061,21 @@ static int azx_pcm_hw_free(struct snd_pcm_substream *substream)
struct hda_pcm_stream *hinfo = apcm->hinfo[substream->stream];
/* reset BDL address */
- azx_sd_writel(azx_dev, SD_BDLPL, 0);
- azx_sd_writel(azx_dev, SD_BDLPU, 0);
- azx_sd_writel(azx_dev, SD_CTL, 0);
- azx_dev->bufsize = 0;
- azx_dev->period_bytes = 0;
- azx_dev->format_val = 0;
+ dsp_lock(azx_dev);
+ if (!dsp_is_locked(azx_dev)) {
+ azx_sd_writel(azx_dev, SD_BDLPL, 0);
+ azx_sd_writel(azx_dev, SD_BDLPU, 0);
+ azx_sd_writel(azx_dev, SD_CTL, 0);
+ azx_dev->bufsize = 0;
+ azx_dev->period_bytes = 0;
+ azx_dev->format_val = 0;
+ }
snd_hda_codec_cleanup(apcm->codec, hinfo, substream);
mark_runtime_wc(chip, azx_dev, substream, false);
+ azx_dev->prepared = 0;
+ dsp_unlock(azx_dev);
return snd_pcm_lib_free_pages(substream);
}
@@ -2055,6 +2092,12 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream)
snd_hda_spdif_out_of_nid(apcm->codec, hinfo->nid);
unsigned short ctls = spdif ? spdif->ctls : 0;
+ dsp_lock(azx_dev);
+ if (dsp_is_locked(azx_dev)) {
+ err = -EBUSY;
+ goto unlock;
+ }
+
azx_stream_reset(chip, azx_dev);
format_val = snd_hda_calc_stream_format(runtime->rate,
runtime->channels,
@@ -2065,7 +2108,8 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream)
snd_printk(KERN_ERR SFX
"%s: invalid format_val, rate=%d, ch=%d, format=%d\n",
pci_name(chip->pci), runtime->rate, runtime->channels, runtime->format);
- return -EINVAL;
+ err = -EINVAL;
+ goto unlock;
}
bufsize = snd_pcm_lib_buffer_bytes(substream);
@@ -2084,7 +2128,7 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream)
azx_dev->no_period_wakeup = runtime->no_period_wakeup;
err = azx_setup_periods(chip, substream, azx_dev);
if (err < 0)
- return err;
+ goto unlock;
}
/* wallclk has 24Mhz clock source */
@@ -2101,8 +2145,14 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream)
if ((chip->driver_caps & AZX_DCAPS_CTX_WORKAROUND) &&
stream_tag > chip->capture_streams)
stream_tag -= chip->capture_streams;
- return snd_hda_codec_prepare(apcm->codec, hinfo, stream_tag,
+ err = snd_hda_codec_prepare(apcm->codec, hinfo, stream_tag,
azx_dev->format_val, substream);
+
+ unlock:
+ if (!err)
+ azx_dev->prepared = 1;
+ dsp_unlock(azx_dev);
+ return err;
}
static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
@@ -2117,6 +2167,9 @@ static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
azx_dev = get_azx_dev(substream);
trace_azx_pcm_trigger(chip, azx_dev, cmd);
+ if (!dsp_is_locked(azx_dev) || !azx_dev->prepared)
+ return -EPIPE;
+
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
rstart = 1;
@@ -2621,17 +2674,28 @@ static int azx_load_dsp_prepare(struct hda_bus *bus, unsigned int format,
struct azx_dev *azx_dev;
int err;
- if (snd_hda_lock_devices(bus))
- return -EBUSY;
+ azx_dev = azx_get_dsp_loader_dev(chip);
+
+ mutex_lock(&chip->open_mutex);
+ dsp_lock(azx_dev);
+ spin_lock_irq(&chip->reg_lock);
+ if (azx_dev->running || azx_dev->locked) {
+ spin_unlock_irq(&chip->reg_lock);
+ err = -EBUSY;
+ goto unlock;
+ }
+ azx_dev->prepared = 0;
+ chip->saved_azx_dev = *azx_dev;
+ azx_dev->locked = 1;
+ spin_unlock_irq(&chip->reg_lock);
err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG,
snd_dma_pci_data(chip->pci),
byte_size, bufp);
if (err < 0)
- goto unlock;
+ goto err_alloc;
mark_pages_wc(chip, bufp, true);
- azx_dev = azx_get_dsp_loader_dev(chip);
azx_dev->bufsize = byte_size;
azx_dev->period_bytes = byte_size;
azx_dev->format_val = format;
@@ -2649,13 +2713,21 @@ static int azx_load_dsp_prepare(struct hda_bus *bus, unsigned int format,
goto error;
azx_setup_controller(chip, azx_dev);
+ mutex_unlock(&chip->open_mutex);
return azx_dev->stream_tag;
error:
mark_pages_wc(chip, bufp, false);
snd_dma_free_pages(bufp);
-unlock:
- snd_hda_unlock_devices(bus);
+ err_alloc:
+ spin_lock_irq(&chip->reg_lock);
+ if (azx_dev->opened)
+ *azx_dev = chip->saved_azx_dev;
+ azx_dev->locked = 0;
+ spin_unlock_irq(&chip->reg_lock);
+ unlock:
+ dsp_unlock(azx_dev);
+ mutex_unlock(&chip->open_mutex);
return err;
}
@@ -2677,9 +2749,11 @@ static void azx_load_dsp_cleanup(struct hda_bus *bus,
struct azx *chip = bus->private_data;
struct azx_dev *azx_dev = azx_get_dsp_loader_dev(chip);
- if (!dmab->area)
+ if (!dmab->area || !azx_dev->locked)
return;
+ mutex_lock(&chip->open_mutex);
+ dsp_lock(azx_dev);
/* reset BDL address */
azx_sd_writel(azx_dev, SD_BDLPL, 0);
azx_sd_writel(azx_dev, SD_BDLPU, 0);
@@ -2692,7 +2766,13 @@ static void azx_load_dsp_cleanup(struct hda_bus *bus,
snd_dma_free_pages(dmab);
dmab->area = NULL;
- snd_hda_unlock_devices(bus);
+ spin_lock_irq(&chip->reg_lock);
+ if (azx_dev->opened)
+ *azx_dev = chip->saved_azx_dev;
+ azx_dev->locked = 0;
+ spin_unlock_irq(&chip->reg_lock);
+ dsp_unlock(azx_dev);
+ mutex_unlock(&chip->open_mutex);
}
#endif /* CONFIG_SND_HDA_DSP_LOADER */
@@ -3481,6 +3561,7 @@ static int azx_first_init(struct azx *chip)
}
for (i = 0; i < chip->num_streams; i++) {
+ dsp_lock_init(&chip->azx_dev[i]);
/* allocate memory for the BDL for each stream */
err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
snd_dma_pci_data(chip->pci),
--
1.8.1.4
More information about the Alsa-devel
mailing list