From c156c54d796f1b926a72a308dc084eec6eaad1c6 Mon Sep 17 00:00:00 2001 From: Ezequiel Garcia Date: Sat, 4 Jun 2016 20:47:18 -0300 Subject: [media] tw686x: audio: Implement non-memcpy capture Now that we've introduced the dma_mode parameter to pick the DMA operation, let's use it to also select the audio DMA operation. When dma_mode != memcpy, the driver will avoid using memcpy in the audio capture path, and the DMA hardware operation will act directly on the ALSA buffers. Signed-off-by: Ezequiel Garcia Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/tw686x/tw686x-audio.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) (limited to 'drivers/media/pci/tw686x/tw686x-audio.c') diff --git a/drivers/media/pci/tw686x/tw686x-audio.c b/drivers/media/pci/tw686x/tw686x-audio.c index 91459ab715b2..a14d1b07edec 100644 --- a/drivers/media/pci/tw686x/tw686x-audio.c +++ b/drivers/media/pci/tw686x/tw686x-audio.c @@ -62,12 +62,22 @@ void tw686x_audio_irq(struct tw686x_dev *dev, unsigned long requests, } spin_unlock_irqrestore(&ac->lock, flags); + if (!done || !next) + continue; + /* + * Checking for a non-nil dma_desc[pb]->virt buffer is + * the same as checking for memcpy DMA mode. + */ desc = &ac->dma_descs[pb]; - if (done && next && desc->virt) { - memcpy(done->virt, desc->virt, desc->size); - ac->ptr = done->dma - ac->buf[0].dma; - snd_pcm_period_elapsed(ac->ss); + if (desc->virt) { + memcpy(done->virt, desc->virt, + desc->size); + } else { + u32 reg = pb ? ADMA_B_ADDR[ch] : ADMA_P_ADDR[ch]; + reg_write(dev, reg, next->dma); } + ac->ptr = done->dma - ac->buf[0].dma; + snd_pcm_period_elapsed(ac->ss); } } @@ -181,6 +191,12 @@ static int tw686x_pcm_prepare(struct snd_pcm_substream *ss) ac->curr_bufs[0] = p_buf; ac->curr_bufs[1] = b_buf; ac->ptr = 0; + + if (dev->dma_mode != TW686X_DMA_MODE_MEMCPY) { + reg_write(dev, ADMA_P_ADDR[ac->ch], p_buf->dma); + reg_write(dev, ADMA_B_ADDR[ac->ch], b_buf->dma); + } + spin_unlock_irqrestore(&ac->lock, flags); return 0; @@ -290,6 +306,14 @@ static int tw686x_audio_dma_alloc(struct tw686x_dev *dev, { int pb; + /* + * In the memcpy DMA mode we allocate a consistent buffer + * and use it for the DMA capture. Otherwise, DMA + * acts on the ALSA buffers as received in pcm_prepare. + */ + if (dev->dma_mode != TW686X_DMA_MODE_MEMCPY) + return 0; + for (pb = 0; pb < 2; pb++) { u32 reg = pb ? ADMA_B_ADDR[ac->ch] : ADMA_P_ADDR[ac->ch]; void *virt; -- cgit v1.2.3 From 447d7c329145989e96cd0a89970a6e009407bad9 Mon Sep 17 00:00:00 2001 From: Ezequiel Garcia Date: Sat, 4 Jun 2016 20:47:19 -0300 Subject: [media] tw686x: audio: Allow to configure the period size Currently, the driver has a fixed period size of 4096 bytes (2048 frames). Since this hardware can configure the audio capture size, this commit allows a period size range of [512-4096]. This is very useful to reduce the audio latency. Signed-off-by: Ezequiel Garcia Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/tw686x/tw686x-audio.c | 48 ++++++++++++++++++--------------- drivers/media/pci/tw686x/tw686x-regs.h | 4 +++ drivers/media/pci/tw686x/tw686x.h | 5 ++-- 3 files changed, 34 insertions(+), 23 deletions(-) (limited to 'drivers/media/pci/tw686x/tw686x-audio.c') diff --git a/drivers/media/pci/tw686x/tw686x-audio.c b/drivers/media/pci/tw686x/tw686x-audio.c index a14d1b07edec..987a22663525 100644 --- a/drivers/media/pci/tw686x/tw686x-audio.c +++ b/drivers/media/pci/tw686x/tw686x-audio.c @@ -71,7 +71,7 @@ void tw686x_audio_irq(struct tw686x_dev *dev, unsigned long requests, desc = &ac->dma_descs[pb]; if (desc->virt) { memcpy(done->virt, desc->virt, - desc->size); + dev->period_size); } else { u32 reg = pb ? ADMA_B_ADDR[ch] : ADMA_P_ADDR[ch]; reg_write(dev, reg, next->dma); @@ -93,10 +93,11 @@ static int tw686x_pcm_hw_free(struct snd_pcm_substream *ss) } /* - * The audio device rate is global and shared among all + * Audio parameters are global and shared among all * capture channels. The driver makes no effort to prevent - * rate modifications. User is free change the rate, but it - * means changing the rate for all capture sub-devices. + * any modifications. User is free change the audio rate, + * or period size, thus changing parameters for all capture + * sub-devices. */ static const struct snd_pcm_hardware tw686x_capture_hw = { .info = (SNDRV_PCM_INFO_MMAP | @@ -109,9 +110,9 @@ static const struct snd_pcm_hardware tw686x_capture_hw = { .rate_max = 48000, .channels_min = 1, .channels_max = 1, - .buffer_bytes_max = TW686X_AUDIO_PAGE_MAX * TW686X_AUDIO_PAGE_SZ, - .period_bytes_min = TW686X_AUDIO_PAGE_SZ, - .period_bytes_max = TW686X_AUDIO_PAGE_SZ, + .buffer_bytes_max = TW686X_AUDIO_PAGE_MAX * AUDIO_DMA_SIZE_MAX, + .period_bytes_min = AUDIO_DMA_SIZE_MIN, + .period_bytes_max = AUDIO_DMA_SIZE_MAX, .periods_min = TW686X_AUDIO_PERIODS_MIN, .periods_max = TW686X_AUDIO_PERIODS_MAX, }; @@ -166,12 +167,21 @@ static int tw686x_pcm_prepare(struct snd_pcm_substream *ss) reg_write(dev, AUDIO_CONTROL2, reg); } - if (period_size != TW686X_AUDIO_PAGE_SZ || - rt->periods < TW686X_AUDIO_PERIODS_MIN || - rt->periods > TW686X_AUDIO_PERIODS_MAX) { - return -EINVAL; + if (dev->period_size != period_size) { + u32 reg; + + dev->period_size = period_size; + reg = reg_read(dev, AUDIO_CONTROL1); + reg &= ~(AUDIO_DMA_SIZE_MASK << AUDIO_DMA_SIZE_SHIFT); + reg |= period_size << AUDIO_DMA_SIZE_SHIFT; + + reg_write(dev, AUDIO_CONTROL1, reg); } + if (rt->periods < TW686X_AUDIO_PERIODS_MIN || + rt->periods > TW686X_AUDIO_PERIODS_MAX) + return -EINVAL; + spin_lock_irqsave(&ac->lock, flags); INIT_LIST_HEAD(&ac->buf_list); @@ -282,8 +292,8 @@ static int tw686x_snd_pcm_init(struct tw686x_dev *dev) return snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV, snd_dma_pci_data(dev->pci_dev), - TW686X_AUDIO_PAGE_MAX * TW686X_AUDIO_PAGE_SZ, - TW686X_AUDIO_PAGE_MAX * TW686X_AUDIO_PAGE_SZ); + TW686X_AUDIO_PAGE_MAX * AUDIO_DMA_SIZE_MAX, + TW686X_AUDIO_PAGE_MAX * AUDIO_DMA_SIZE_MAX); } static void tw686x_audio_dma_free(struct tw686x_dev *dev, @@ -318,7 +328,7 @@ static int tw686x_audio_dma_alloc(struct tw686x_dev *dev, u32 reg = pb ? ADMA_B_ADDR[ac->ch] : ADMA_P_ADDR[ac->ch]; void *virt; - virt = pci_alloc_consistent(dev->pci_dev, TW686X_AUDIO_PAGE_SZ, + virt = pci_alloc_consistent(dev->pci_dev, AUDIO_DMA_SIZE_MAX, &ac->dma_descs[pb].phys); if (!virt) { dev_err(&dev->pci_dev->dev, @@ -327,7 +337,7 @@ static int tw686x_audio_dma_alloc(struct tw686x_dev *dev, return -ENOMEM; } ac->dma_descs[pb].virt = virt; - ac->dma_descs[pb].size = TW686X_AUDIO_PAGE_SZ; + ac->dma_descs[pb].size = AUDIO_DMA_SIZE_MAX; reg_write(dev, reg, ac->dma_descs[pb].phys); } return 0; @@ -358,12 +368,8 @@ int tw686x_audio_init(struct tw686x_dev *dev) struct snd_card *card; int err, ch; - /* - * AUDIO_CONTROL1 - * DMA byte length [31:19] = 4096 (i.e. ALSA period) - * External audio enable [0] = enabled - */ - reg_write(dev, AUDIO_CONTROL1, 0x80000001); + /* Enable external audio */ + reg_write(dev, AUDIO_CONTROL1, BIT(0)); err = snd_card_new(&pci_dev->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, diff --git a/drivers/media/pci/tw686x/tw686x-regs.h b/drivers/media/pci/tw686x/tw686x-regs.h index 37c39bcd7572..15a956642ef4 100644 --- a/drivers/media/pci/tw686x/tw686x-regs.h +++ b/drivers/media/pci/tw686x/tw686x-regs.h @@ -105,6 +105,10 @@ 0x2d0, 0x2d1, 0x2d2, 0x2d3 }) #define SYS_MODE_DMA_SHIFT 13 +#define AUDIO_DMA_SIZE_SHIFT 19 +#define AUDIO_DMA_SIZE_MIN SZ_512 +#define AUDIO_DMA_SIZE_MAX SZ_4K +#define AUDIO_DMA_SIZE_MASK (SZ_8K - 1) #define DMA_CMD_ENABLE BIT(31) #define INT_STATUS_DMA_TOUT BIT(17) diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h index 25e3c84331aa..4acb90543174 100644 --- a/drivers/media/pci/tw686x/tw686x.h +++ b/drivers/media/pci/tw686x/tw686x.h @@ -27,7 +27,6 @@ #define TYPE_SECOND_GEN 0x10 #define TW686X_DEF_PHASE_REF 0x1518 -#define TW686X_AUDIO_PAGE_SZ 4096 #define TW686X_AUDIO_PAGE_MAX 16 #define TW686X_AUDIO_PERIODS_MIN 2 #define TW686X_AUDIO_PERIODS_MAX TW686X_AUDIO_PAGE_MAX @@ -139,7 +138,9 @@ struct tw686x_dev { struct tw686x_video_channel *video_channels; struct tw686x_audio_channel *audio_channels; - int audio_rate; /* per-device value */ + /* Per-device audio parameters */ + int audio_rate; + int period_size; struct timer_list dma_delay_timer; u32 pending_dma_en; /* must be protected by lock */ -- cgit v1.2.3 From 6deab6fec81fb0deeca676c3ce77aa61faee892c Mon Sep 17 00:00:00 2001 From: Ezequiel Garcia Date: Sat, 4 Jun 2016 20:47:20 -0300 Subject: [media] tw686x: audio: Prevent hw param changes while busy Audio hw params are shared across all DMA channels, so if the user changes any of these while any DMA channel is enabled, it will impact the enabled channels, potentially causing serious instability issues. This commit avoids such situation, by preventing any hw param change (on any DMA channel) if any other DMA audio channel is capturing. Signed-off-by: Ezequiel Garcia Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/tw686x/tw686x-audio.c | 20 ++++++++++++++++---- drivers/media/pci/tw686x/tw686x.h | 1 + 2 files changed, 17 insertions(+), 4 deletions(-) (limited to 'drivers/media/pci/tw686x/tw686x-audio.c') diff --git a/drivers/media/pci/tw686x/tw686x-audio.c b/drivers/media/pci/tw686x/tw686x-audio.c index 987a22663525..96e444c49173 100644 --- a/drivers/media/pci/tw686x/tw686x-audio.c +++ b/drivers/media/pci/tw686x/tw686x-audio.c @@ -94,10 +94,8 @@ static int tw686x_pcm_hw_free(struct snd_pcm_substream *ss) /* * Audio parameters are global and shared among all - * capture channels. The driver makes no effort to prevent - * any modifications. User is free change the audio rate, - * or period size, thus changing parameters for all capture - * sub-devices. + * capture channels. The driver prevents changes to + * the parameters if any audio channel is capturing. */ static const struct snd_pcm_hardware tw686x_capture_hw = { .info = (SNDRV_PCM_INFO_MMAP | @@ -154,6 +152,14 @@ static int tw686x_pcm_prepare(struct snd_pcm_substream *ss) int i; spin_lock_irqsave(&dev->lock, flags); + /* + * Given the audio parameters are global (i.e. shared across + * DMA channels), we need to check new params are allowed. + */ + if (((dev->audio_rate != rt->rate) || + (dev->period_size != period_size)) && dev->audio_enabled) + goto err_audio_busy; + tw686x_disable_channel(dev, AUDIO_CHANNEL_OFFSET + ac->ch); spin_unlock_irqrestore(&dev->lock, flags); @@ -210,6 +216,10 @@ static int tw686x_pcm_prepare(struct snd_pcm_substream *ss) spin_unlock_irqrestore(&ac->lock, flags); return 0; + +err_audio_busy: + spin_unlock_irqrestore(&dev->lock, flags); + return -EBUSY; } static int tw686x_pcm_trigger(struct snd_pcm_substream *ss, int cmd) @@ -223,6 +233,7 @@ static int tw686x_pcm_trigger(struct snd_pcm_substream *ss, int cmd) case SNDRV_PCM_TRIGGER_START: if (ac->curr_bufs[0] && ac->curr_bufs[1]) { spin_lock_irqsave(&dev->lock, flags); + dev->audio_enabled = 1; tw686x_enable_channel(dev, AUDIO_CHANNEL_OFFSET + ac->ch); spin_unlock_irqrestore(&dev->lock, flags); @@ -235,6 +246,7 @@ static int tw686x_pcm_trigger(struct snd_pcm_substream *ss, int cmd) break; case SNDRV_PCM_TRIGGER_STOP: spin_lock_irqsave(&dev->lock, flags); + dev->audio_enabled = 0; tw686x_disable_channel(dev, AUDIO_CHANNEL_OFFSET + ac->ch); spin_unlock_irqrestore(&dev->lock, flags); diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h index 4acb90543174..35d7bc94f78f 100644 --- a/drivers/media/pci/tw686x/tw686x.h +++ b/drivers/media/pci/tw686x/tw686x.h @@ -141,6 +141,7 @@ struct tw686x_dev { /* Per-device audio parameters */ int audio_rate; int period_size; + int audio_enabled; struct timer_list dma_delay_timer; u32 pending_dma_en; /* must be protected by lock */ -- cgit v1.2.3