[alsa-devel] [PATCH 0/5] AACI: cleanups
This patch series is in preparation of adding DMA support to the AACI driver.
I'd appreciate it if ALSA people could do me the curtesy of copying me on patches they dream up on ALSA drivers maintained by me, rather than running rough-shod over work that they may have queued - especially when they say "you can also do this" "yes I know, I have a patch" and then they do it anyway.
If this is how they want to operate, I'll avoid working on any ALSA stuff in the future. Its just not worth me wasting my time.
There's no need for a specific rule; ALSA's generic AC'97 support calculates the necessary rate constraint information itself, and we can use this directly.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- sound/arm/aaci.c | 75 ++--------------------------------------------------- 1 files changed, 3 insertions(+), 72 deletions(-)
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index 1497dce..106e0b7 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -330,63 +330,6 @@ static irqreturn_t aaci_irq(int irq, void *devid) /* * ALSA support. */ - -struct aaci_stream { - unsigned char codec_idx; - unsigned char rate_idx; -}; - -static struct aaci_stream aaci_streams[] = { - [ACSTREAM_FRONT] = { - .codec_idx = 0, - .rate_idx = AC97_RATES_FRONT_DAC, - }, - [ACSTREAM_SURROUND] = { - .codec_idx = 0, - .rate_idx = AC97_RATES_SURR_DAC, - }, - [ACSTREAM_LFE] = { - .codec_idx = 0, - .rate_idx = AC97_RATES_LFE_DAC, - }, -}; - -static inline unsigned int aaci_rate_mask(struct aaci *aaci, int streamid) -{ - struct aaci_stream *s = aaci_streams + streamid; - return aaci->ac97_bus->codec[s->codec_idx]->rates[s->rate_idx]; -} - -static unsigned int rate_list[] = { - 5512, 8000, 11025, 16000, 22050, 32000, 44100, - 48000, 64000, 88200, 96000, 176400, 192000 -}; - -/* - * Double-rate rule: we can support double rate iff channels == 2 - * (unimplemented) - */ -static int -aaci_rule_rate_by_channels(struct snd_pcm_hw_params *p, struct snd_pcm_hw_rule *rule) -{ - struct aaci *aaci = rule->private; - unsigned int rate_mask = SNDRV_PCM_RATE_8000_48000|SNDRV_PCM_RATE_5512; - struct snd_interval *c = hw_param_interval(p, SNDRV_PCM_HW_PARAM_CHANNELS); - - switch (c->max) { - case 6: - rate_mask &= aaci_rate_mask(aaci, ACSTREAM_LFE); - case 4: - rate_mask &= aaci_rate_mask(aaci, ACSTREAM_SURROUND); - case 2: - rate_mask &= aaci_rate_mask(aaci, ACSTREAM_FRONT); - } - - return snd_interval_list(hw_param_interval(p, rule->var), - ARRAY_SIZE(rate_list), rate_list, - rate_mask); -} - static struct snd_pcm_hardware aaci_hw_info = { .info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | @@ -400,10 +343,7 @@ static struct snd_pcm_hardware aaci_hw_info = { */ .formats = SNDRV_PCM_FMTBIT_S16_LE,
- /* should this be continuous or knot? */ - .rates = SNDRV_PCM_RATE_CONTINUOUS, - .rate_max = 48000, - .rate_min = 4000, + /* rates are setup from the AC'97 codec */ .channels_min = 2, .channels_max = 6, .buffer_bytes_max = 64 * 1024, @@ -423,6 +363,8 @@ static int __aaci_pcm_open(struct aaci *aaci, aacirun->substream = substream; runtime->private_data = aacirun; runtime->hw = aaci_hw_info; + runtime->hw.rates = aacirun->pcm->rates; + snd_pcm_limit_hw_rates(runtime);
/* * FIXME: ALSA specifies fifo_size in bytes. If we're in normal @@ -433,17 +375,6 @@ static int __aaci_pcm_open(struct aaci *aaci, */ runtime->hw.fifo_size = aaci->fifosize * 2;
- /* - * Add rule describing hardware rate dependency - * on the number of channels. - */ - ret = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, - aaci_rule_rate_by_channels, aaci, - SNDRV_PCM_HW_PARAM_CHANNELS, - SNDRV_PCM_HW_PARAM_RATE, -1); - if (ret) - goto out; - ret = request_irq(aaci->dev->irq[0], aaci_irq, IRQF_SHARED|IRQF_DISABLED, DRIVER_NAME, aaci); if (ret)
Since the recording and playback paths are now the same, eliminate the needless conditionals.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- sound/arm/aaci.c | 18 +++++++----------- 1 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index 106e0b7..9af8ef8 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -438,18 +438,14 @@ static int aaci_pcm_hw_params(struct snd_pcm_substream *substream,
err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params)); - if (err < 0) - goto out; - - err = snd_ac97_pcm_open(aacirun->pcm, params_rate(params), - params_channels(params), - aacirun->pcm->r[0].slots); - if (err) - goto out; + if (err >= 0) { + err = snd_ac97_pcm_open(aacirun->pcm, params_rate(params), + params_channels(params), + aacirun->pcm->r[0].slots);
- aacirun->pcm_open = 1; + aacirun->pcm_open = err == 0; + }
- out: return err; }
@@ -458,7 +454,7 @@ static int aaci_pcm_prepare(struct snd_pcm_substream *substream) struct snd_pcm_runtime *runtime = substream->runtime; struct aaci_runtime *aacirun = runtime->private_data;
- aacirun->start = (void *)runtime->dma_area; + aacirun->start = runtime->dma_area; aacirun->end = aacirun->start + snd_pcm_lib_buffer_bytes(substream); aacirun->ptr = aacirun->start; aacirun->period =
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- sound/arm/aaci.c | 22 +++++++--------------- 1 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index 9af8ef8..0b88c26 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -444,6 +444,11 @@ static int aaci_pcm_hw_params(struct snd_pcm_substream *substream, aacirun->pcm->r[0].slots);
aacirun->pcm_open = err == 0; + aacirun->cr = CR_FEN | CR_COMPACT | CR_SZ16; + aacirun->fifosz = aaci->fifosize * 4; + + if (aacirun->cr & CR_COMPACT) + aacirun->fifosz >>= 1; }
return err; @@ -554,14 +559,9 @@ static int aaci_pcm_playback_hw_params(struct snd_pcm_substream *substream, * Enable FIFO, compact mode, 16 bits per sample. * FIXME: double rate slots? */ - if (ret >= 0) { - aacirun->cr = CR_FEN | CR_COMPACT | CR_SZ16; + if (ret >= 0) aacirun->cr |= channels_to_txmask[channels];
- aacirun->fifosz = aaci->fifosize * 4; - if (aacirun->cr & CR_COMPACT) - aacirun->fifosz >>= 1; - } return ret; }
@@ -648,18 +648,10 @@ static int aaci_pcm_capture_hw_params(struct snd_pcm_substream *substream, int ret;
ret = aaci_pcm_hw_params(substream, aacirun, params); - - if (ret >= 0) { - aacirun->cr = CR_FEN | CR_COMPACT | CR_SZ16; - + if (ret >= 0) /* Line in record: slot 3 and 4 */ aacirun->cr |= CR_SL3 | CR_SL4;
- aacirun->fifosz = aaci->fifosize * 4; - - if (aacirun->cr & CR_COMPACT) - aacirun->fifosz >>= 1; - } return ret; }
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- sound/arm/aaci.c | 17 +++++++++++++++-- 1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index 0b88c26..0635c62 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -366,6 +366,10 @@ static int __aaci_pcm_open(struct aaci *aaci, runtime->hw.rates = aacirun->pcm->rates; snd_pcm_limit_hw_rates(runtime);
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && + aacirun->pcm->r[1].slots) + snd_ac97_pcm_double_rate_rules(runtime); + /* * FIXME: ALSA specifies fifo_size in bytes. If we're in normal * mode, each 32-bit word contains one sample. If we're in @@ -439,9 +443,12 @@ static int aaci_pcm_hw_params(struct snd_pcm_substream *substream, err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params)); if (err >= 0) { - err = snd_ac97_pcm_open(aacirun->pcm, params_rate(params), + unsigned int rate = params_rate(params); + int dbl = rate > 48000; + + err = snd_ac97_pcm_open(aacirun->pcm, rate, params_channels(params), - aacirun->pcm->r[0].slots); + aacirun->pcm->r[dbl].slots);
aacirun->pcm_open = err == 0; aacirun->cr = CR_FEN | CR_COMPACT | CR_SZ16; @@ -808,6 +815,12 @@ static struct ac97_pcm ac97_defs[] __devinitdata = { (1 << AC97_SLOT_PCM_SRIGHT) | (1 << AC97_SLOT_LFE), }, + [1] = { + .slots = (1 << AC97_SLOT_PCM_LEFT) | + (1 << AC97_SLOT_PCM_RIGHT) | + (1 << AC97_SLOT_PCM_LEFT_0) | + (1 << AC97_SLOT_PCM_RIGHT_0), + }, }, }, [1] = { /* PCM in */
We can use finer-grained locking, which makes things easier when we gain DMA support.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- sound/arm/aaci.c | 49 +++++++++++++++++++++++++++++-------------------- sound/arm/aaci.h | 2 +- 2 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index 0635c62..e35a0fc 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -172,14 +172,15 @@ static unsigned short aaci_ac97_read(struct snd_ac97 *ac97, unsigned short reg) return v; }
-static inline void aaci_chan_wait_ready(struct aaci_runtime *aacirun) +static inline void +aaci_chan_wait_ready(struct aaci_runtime *aacirun, unsigned long mask) { u32 val; int timeout = 5000;
do { val = readl(aacirun->base + AACI_SR); - } while (val & (SR_TXB|SR_RXB) && timeout--); + } while (val & mask && timeout--); }
@@ -208,8 +209,10 @@ static void aaci_fifo_irq(struct aaci *aaci, int channel, u32 mask) writel(0, aacirun->base + AACI_IE); return; } - ptr = aacirun->ptr;
+ spin_lock(&aacirun->lock); + + ptr = aacirun->ptr; do { unsigned int len = aacirun->fifosz; u32 val; @@ -217,9 +220,9 @@ static void aaci_fifo_irq(struct aaci *aaci, int channel, u32 mask) if (aacirun->bytes <= 0) { aacirun->bytes += aacirun->period; aacirun->ptr = ptr; - spin_unlock(&aaci->lock); + spin_unlock(&aacirun->lock); snd_pcm_period_elapsed(aacirun->substream); - spin_lock(&aaci->lock); + spin_lock(&aacirun->lock); } if (!(aacirun->cr & CR_EN)) break; @@ -245,7 +248,10 @@ static void aaci_fifo_irq(struct aaci *aaci, int channel, u32 mask) ptr = aacirun->start; } } while(1); + aacirun->ptr = ptr; + + spin_unlock(&aacirun->lock); }
if (mask & ISR_URINTR) { @@ -263,6 +269,8 @@ static void aaci_fifo_irq(struct aaci *aaci, int channel, u32 mask) return; }
+ spin_lock(&aacirun->lock); + ptr = aacirun->ptr; do { unsigned int len = aacirun->fifosz; @@ -271,9 +279,9 @@ static void aaci_fifo_irq(struct aaci *aaci, int channel, u32 mask) if (aacirun->bytes <= 0) { aacirun->bytes += aacirun->period; aacirun->ptr = ptr; - spin_unlock(&aaci->lock); + spin_unlock(&aacirun->lock); snd_pcm_period_elapsed(aacirun->substream); - spin_lock(&aaci->lock); + spin_lock(&aacirun->lock); } if (!(aacirun->cr & CR_EN)) break; @@ -301,6 +309,8 @@ static void aaci_fifo_irq(struct aaci *aaci, int channel, u32 mask) } while (1);
aacirun->ptr = ptr; + + spin_unlock(&aacirun->lock); } }
@@ -310,7 +320,6 @@ static irqreturn_t aaci_irq(int irq, void *devid) u32 mask; int i;
- spin_lock(&aaci->lock); mask = readl(aaci->base + AACI_ALLINTS); if (mask) { u32 m = mask; @@ -320,7 +329,6 @@ static irqreturn_t aaci_irq(int irq, void *devid) } } } - spin_unlock(&aaci->lock);
return mask ? IRQ_HANDLED : IRQ_NONE; } @@ -580,7 +588,7 @@ static void aaci_pcm_playback_stop(struct aaci_runtime *aacirun) ie &= ~(IE_URIE|IE_TXIE); writel(ie, aacirun->base + AACI_IE); aacirun->cr &= ~CR_EN; - aaci_chan_wait_ready(aacirun); + aaci_chan_wait_ready(aacirun, SR_TXB); writel(aacirun->cr, aacirun->base + AACI_TXCR); }
@@ -588,7 +596,7 @@ static void aaci_pcm_playback_start(struct aaci_runtime *aacirun) { u32 ie;
- aaci_chan_wait_ready(aacirun); + aaci_chan_wait_ready(aacirun, SR_TXB); aacirun->cr |= CR_EN;
ie = readl(aacirun->base + AACI_IE); @@ -599,12 +607,12 @@ static void aaci_pcm_playback_start(struct aaci_runtime *aacirun)
static int aaci_pcm_playback_trigger(struct snd_pcm_substream *substream, int cmd) { - struct aaci *aaci = substream->private_data; struct aaci_runtime *aacirun = substream->runtime->private_data; unsigned long flags; int ret = 0;
- spin_lock_irqsave(&aaci->lock, flags); + spin_lock_irqsave(&aacirun->lock, flags); + switch (cmd) { case SNDRV_PCM_TRIGGER_START: aaci_pcm_playback_start(aacirun); @@ -631,7 +639,8 @@ static int aaci_pcm_playback_trigger(struct snd_pcm_substream *substream, int cm default: ret = -EINVAL; } - spin_unlock_irqrestore(&aaci->lock, flags); + + spin_unlock_irqrestore(&aacirun->lock, flags);
return ret; } @@ -666,7 +675,7 @@ static void aaci_pcm_capture_stop(struct aaci_runtime *aacirun) { u32 ie;
- aaci_chan_wait_ready(aacirun); + aaci_chan_wait_ready(aacirun, SR_RXB);
ie = readl(aacirun->base + AACI_IE); ie &= ~(IE_ORIE | IE_RXIE); @@ -681,7 +690,7 @@ static void aaci_pcm_capture_start(struct aaci_runtime *aacirun) { u32 ie;
- aaci_chan_wait_ready(aacirun); + aaci_chan_wait_ready(aacirun, SR_RXB);
#ifdef DEBUG /* RX Timeout value: bits 28:17 in RXCR */ @@ -698,12 +707,11 @@ static void aaci_pcm_capture_start(struct aaci_runtime *aacirun)
static int aaci_pcm_capture_trigger(struct snd_pcm_substream *substream, int cmd) { - struct aaci *aaci = substream->private_data; struct aaci_runtime *aacirun = substream->runtime->private_data; unsigned long flags; int ret = 0;
- spin_lock_irqsave(&aaci->lock, flags); + spin_lock_irqsave(&aacirun->lock, flags);
switch (cmd) { case SNDRV_PCM_TRIGGER_START: @@ -732,7 +740,7 @@ static int aaci_pcm_capture_trigger(struct snd_pcm_substream *substream, int cmd ret = -EINVAL; }
- spin_unlock_irqrestore(&aaci->lock, flags); + spin_unlock_irqrestore(&aacirun->lock, flags);
return ret; } @@ -933,7 +941,6 @@ static struct aaci * __devinit aaci_init_card(struct amba_device *dev)
aaci = card->private_data; mutex_init(&aaci->ac97_sem); - spin_lock_init(&aaci->lock); aaci->card = card; aaci->dev = dev;
@@ -1020,12 +1027,14 @@ static int __devinit aaci_probe(struct amba_device *dev, struct amba_id *id) /* * Playback uses AACI channel 0 */ + spin_lock_init(&aaci->playback.lock); aaci->playback.base = aaci->base + AACI_CSCH1; aaci->playback.fifo = aaci->base + AACI_DR1;
/* * Capture uses AACI channel 0 */ + spin_lock_init(&aaci->capture.lock); aaci->capture.base = aaci->base + AACI_CSCH1; aaci->capture.fifo = aaci->base + AACI_DR1;
diff --git a/sound/arm/aaci.h b/sound/arm/aaci.h index 924f69c..6a4a2ee 100644 --- a/sound/arm/aaci.h +++ b/sound/arm/aaci.h @@ -202,6 +202,7 @@ struct aaci_runtime { void __iomem *base; void __iomem *fifo; + spinlock_t lock;
struct ac97_pcm *pcm; int pcm_open; @@ -232,7 +233,6 @@ struct aaci { struct snd_ac97 *ac97;
u32 maincr; - spinlock_t lock;
struct aaci_runtime playback; struct aaci_runtime capture;
At Fri, 18 Dec 2009 17:47:31 +0000, Russell King - ARM Linux wrote:
This patch series is in preparation of adding DMA support to the AACI driver.
Thanks, applied all patches now.
I'd appreciate it if ALSA people could do me the curtesy of copying me on patches they dream up on ALSA drivers maintained by me, rather than running rough-shod over work that they may have queued - especially when they say "you can also do this" "yes I know, I have a patch" and then they do it anyway.
Oh, that was basically my misunderstanding. At that reply, you just wrote "yes it's a clean up patch". And no further mail came since then, thus I decided to fix by myself.
My fault is, of course, the change wasn't cc'ed to you, though. Hopefully this will work out in future better.
thanks,
Takashi
participants (2)
-
Russell King - ARM Linux -
Takashi Iwai