From 05b2723387cb4086535e935ee07cca19086308fc Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Fri, 24 Aug 2007 01:06:34 -0300 Subject: [PATCH] V4L/DVB (6083): cx88-alsa: Rework buffer handling Rework the way the DMA buffer is handled and IRQs are generated. ALSA uses a ring-buffer of multiple periods. Each period is supposed to corrispond to one IRQ. The existing driver was generating one interrupt per ring-buffer, as opposed to per period. This meant that as soon as the IRQ was generated, the hardware was already starting to re-write the beginning of the buffer. Since the DMA happens on a per-line basis, there was only a narrow window to copy the data out before the buffer was overwritten. The cx88 core RISC program generator is modified so that it can set the IRQ and counter flags to count every X lines of DMA transfer. This way we can generate an interrupt every period instead of every full ring-buffer. Right now only period of one line are supported, but it should be possible to support longer periods. Note that a WRITE instruction generates an IRQ when it starts, not when the transfer is finished. Thus to generate an IRQ when line X is done, one must set the IRQ flag on the instruction that starts line X+1, not the one that ends line X. Change the line size so that there are four lines in the SRAM FIFO. If there are not four lines, the analog output from the cx88's internal DACs is full of clicks and pops. Try to handle FIFO sync errors. Sometimes the chip generates many of these errors before audio data starts. Up to 50 sync errors will be ignored and the counter reset. Have the IRQ handler save the RISC counter to the chip struct, and then have the pointer callback use this to calculate the pointer position. We could read the counter from the pointer callback, but sometimes the sync errors on start up cause the counter to go crazy. ALSA sees this and thinks there has been an overrun. The IRQ hander can avoid saving the counter position on sync errors. The chip "opened" flag wasn't necessary. ALSA won't try to open the same substream multiple times. Probably this code was cut&pasted from the bt87x driver, which has multiple sub-streams for one chip. Do error checking for the videobuf mapping functions. snd_card_cx88_runtime_free() is useless and can be deleted. Signed-off-by: Trent Piepho Signed-off-by: Mauro Carvalho Chehab --- drivers/media/video/cx88/cx88-alsa.c | 168 ++++++++++++--------------- drivers/media/video/cx88/cx88-core.c | 22 ++-- drivers/media/video/cx88/cx88-mpeg.c | 2 +- drivers/media/video/cx88/cx88.h | 2 +- 4 files changed, 89 insertions(+), 105 deletions(-) diff --git a/drivers/media/video/cx88/cx88-alsa.c b/drivers/media/video/cx88/cx88-alsa.c index 8d19d33c8b..33dd4cb5e8 100644 --- a/drivers/media/video/cx88/cx88-alsa.c +++ b/drivers/media/video/cx88/cx88-alsa.c @@ -3,6 +3,7 @@ * Support for audio capture * PCI function #1 of the cx2388x. * + * (c) 2007 Trent Piepho * (c) 2005,2006 Ricardo Cerqueira * (c) 2005 Mauro Carvalho Chehab * Based on a dummy cx88 module by Gerd Knorr @@ -47,7 +48,6 @@ #define dprintk_core(level,fmt, arg...) if (debug >= level) \ printk(KERN_DEBUG "%s/1: " fmt, chip->core->name , ## arg) - /**************************************************************************** Data type declarations - Can be moded to a header file later ****************************************************************************/ @@ -58,6 +58,7 @@ struct cx88_audio_dev { struct cx88_core *core; struct cx88_dmaqueue q; + u64 starttime; /* pci i/o */ struct pci_dev *pci; @@ -68,24 +69,20 @@ struct cx88_audio_dev { struct snd_card *card; spinlock_t reg_lock; + atomic_t count; unsigned int dma_size; unsigned int period_size; unsigned int num_periods; - struct videobuf_dmabuf dma_risc; + struct videobuf_dmabuf dma_risc; int mixer_volume[MIXER_ADDR_LAST+1][2]; int capture_source[MIXER_ADDR_LAST+1][2]; - long int read_count; - long int read_offset; - - struct cx88_buffer *buf; - - long opened; - struct snd_pcm_substream *substream; + struct cx88_buffer *buf; + struct snd_pcm_substream *substream; }; typedef struct cx88_audio_dev snd_cx88_card_t; @@ -146,16 +143,13 @@ static int _cx88_start_audio_dma(snd_cx88_card_t *chip) cx_write(MO_AUDD_LNGTH, buf->bpl); /* reset counter */ - cx_write(MO_AUDD_GPCNTRL,GP_COUNT_CONTROL_RESET); + cx_write(MO_AUDD_GPCNTRL, GP_COUNT_CONTROL_RESET); + atomic_set(&chip->count, 0); - dprintk(1, "Start audio DMA, %d B/line, %d lines/FIFO, %d lines/irq, " - "%d B/irq\n", buf->bpl, cx_read(audio_ch->cmds_start + 8)>>1, + dprintk(1, "Start audio DMA, %d B/line, %d lines/FIFO, %d periods, %d " + "byte buffer\n", buf->bpl, cx_read(audio_ch->cmds_start + 8)>>1, chip->num_periods, buf->bpl * chip->num_periods); - dprintk(1, "Enabling IRQ, setting mask from 0x%x to 0x%x\n", - chip->core->pci_irqmask, - chip->core->pci_irqmask | PCI_INT_AUDINT); - /* Enables corresponding bits at AUD_INT_STAT */ cx_write(MO_AUD_INTMSK, AUD_INT_OPC_ERR | AUD_INT_DN_SYNC | AUD_INT_DN_RISCI2 | AUD_INT_DN_RISCI1); @@ -198,7 +192,7 @@ static int _cx88_stop_audio_dma(snd_cx88_card_t *chip) return 0; } -#define MAX_IRQ_LOOP 10 +#define MAX_IRQ_LOOP 50 /* * BOARD Specific: IRQ dma bits @@ -223,14 +217,11 @@ static void cx8801_aud_irq(snd_cx88_card_t *chip) { struct cx88_core *core = chip->core; u32 status, mask; - u32 count; status = cx_read(MO_AUD_INTSTAT); mask = cx_read(MO_AUD_INTMSK); - if (0 == (status & mask)) { - spin_unlock(&chip->reg_lock); + if (0 == (status & mask)) return; - } cx_write(MO_AUD_INTSTAT, status); if (debug > 1 || (status & mask & ~0xff)) cx88_print_irqbits(core->name, "irq aud", @@ -238,27 +229,20 @@ static void cx8801_aud_irq(snd_cx88_card_t *chip) status, mask); /* risc op code error */ if (status & AUD_INT_OPC_ERR) { - printk(KERN_WARNING "%s/0: audio risc op code error\n",core->name); + printk(KERN_WARNING "%s/1: Audio risc op code error\n",core->name); cx_clear(MO_AUD_DMACNTRL, 0x11); cx88_sram_channel_dump(core, &cx88_sram_channels[SRAM_CH25]); } - + if (status & AUD_INT_DN_SYNC) { + dprintk(1, "Downstream sync error\n"); + cx_write(MO_AUDD_GPCNTRL, GP_COUNT_CONTROL_RESET); + return; + } /* risc1 downstream */ if (status & AUD_INT_DN_RISCI1) { - spin_lock(&chip->reg_lock); - count = cx_read(MO_AUDD_GPCNT); - spin_unlock(&chip->reg_lock); - if (chip->read_count == 0) - chip->read_count += chip->dma_size; - } - - if (chip->read_count >= chip->period_size) { - dprintk(2, "Elapsing period\n"); + atomic_set(&chip->count, cx_read(MO_AUDD_GPCNT)); snd_pcm_period_elapsed(chip->substream); } - - dprintk(3,"Leaving audio IRQ handler...\n"); - /* FIXME: Any other status should deserve a special handling? */ } @@ -277,23 +261,20 @@ static irqreturn_t cx8801_irq(int irq, void *dev_id) (core->pci_irqmask | PCI_INT_AUDINT); if (0 == status) goto out; - dprintk( 3, "cx8801_irq\n" ); - dprintk( 3, " loop: %d/%d\n", loop, MAX_IRQ_LOOP ); - dprintk( 3, " status: %d\n", status ); + dprintk(3, "cx8801_irq loop %d/%d, status %x\n", + loop, MAX_IRQ_LOOP, status); handled = 1; cx_write(MO_PCI_INTSTAT, status); if (status & core->pci_irqmask) cx88_core_irq(core, status); - if (status & PCI_INT_AUDINT) { - dprintk( 2, " ALSA IRQ handling\n" ); + if (status & PCI_INT_AUDINT) cx8801_aud_irq(chip); - } - }; + } if (MAX_IRQ_LOOP == loop) { - dprintk( 0, "clearing mask\n" ); - dprintk(1,"%s/0: irq loop -- clearing mask\n", + printk(KERN_ERR + "%s/1: IRQ loop detected, disabling interrupts\n", core->name); cx_clear(MO_PCI_INTMSK, PCI_INT_AUDINT); } @@ -315,7 +296,7 @@ static int dsp_buffer_free(snd_cx88_card_t *chip) chip->dma_size = 0; - return 0; + return 0; } /**************************************************************************** @@ -325,6 +306,7 @@ static int dsp_buffer_free(snd_cx88_card_t *chip) /* * Digital hardware definition */ +#define DEFAULT_FIFO_SIZE 4096 static struct snd_pcm_hardware snd_cx88_digital_hw = { .info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_INTERLEAVED | @@ -337,19 +319,15 @@ static struct snd_pcm_hardware snd_cx88_digital_hw = { .rate_max = 48000, .channels_min = 2, .channels_max = 2, - .buffer_bytes_max = (2*2048), - .period_bytes_min = 2048, - .period_bytes_max = 2048, - .periods_min = 2, - .periods_max = 2, + /* Analog audio output will be full of clicks and pops if there + are not exactly four lines in the SRAM FIFO buffer. */ + .period_bytes_min = DEFAULT_FIFO_SIZE/4, + .period_bytes_max = DEFAULT_FIFO_SIZE/4, + .periods_min = 1, + .periods_max = 1024, + .buffer_bytes_max = (1024*1024), }; -/* - * audio pcm capture runtime free - */ -static void snd_card_cx88_runtime_free(struct snd_pcm_runtime *runtime) -{ -} /* * audio pcm capture open callback */ @@ -359,26 +337,24 @@ static int snd_cx88_pcm_open(struct snd_pcm_substream *substream) struct snd_pcm_runtime *runtime = substream->runtime; int err; - if (test_and_set_bit(0, &chip->opened)) - return -EBUSY; - - err = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); + err = snd_pcm_hw_constraint_pow2(runtime, 0, SNDRV_PCM_HW_PARAM_PERIODS); if (err < 0) goto _error; chip->substream = substream; - chip->read_count = 0; - chip->read_offset = 0; - - runtime->private_free = snd_card_cx88_runtime_free; runtime->hw = snd_cx88_digital_hw; + if (cx88_sram_channels[SRAM_CH25].fifo_size != DEFAULT_FIFO_SIZE) { + unsigned int bpl = cx88_sram_channels[SRAM_CH25].fifo_size / 4; + bpl &= ~7; /* must be multiple of 8 */ + runtime->hw.period_bytes_min = bpl; + runtime->hw.period_bytes_max = bpl; + } + return 0; _error: dprintk(1,"Error opening PCM!\n"); - clear_bit(0, &chip->opened); - smp_mb__after_clear_bit(); return err; } @@ -387,11 +363,6 @@ _error: */ static int snd_cx88_close(struct snd_pcm_substream *substream) { - snd_cx88_card_t *chip = snd_pcm_substream_chip(substream); - - clear_bit(0, &chip->opened); - smp_mb__after_clear_bit(); - return 0; } @@ -403,54 +374,61 @@ static int snd_cx88_hw_params(struct snd_pcm_substream * substream, { snd_cx88_card_t *chip = snd_pcm_substream_chip(substream); struct cx88_buffer *buf; + int ret; if (substream->runtime->dma_area) { dsp_buffer_free(chip); substream->runtime->dma_area = NULL; } - chip->period_size = params_period_bytes(hw_params); chip->num_periods = params_periods(hw_params); chip->dma_size = chip->period_size * params_periods(hw_params); BUG_ON(!chip->dma_size); + BUG_ON(chip->num_periods & (chip->num_periods-1)); - dprintk(1,"Setting buffer\n"); - - buf = kzalloc(sizeof(*buf),GFP_KERNEL); + buf = kzalloc(sizeof(*buf), GFP_KERNEL); if (NULL == buf) return -ENOMEM; buf->vb.memory = V4L2_MEMORY_MMAP; + buf->vb.field = V4L2_FIELD_NONE; buf->vb.width = chip->period_size; + buf->bpl = chip->period_size; buf->vb.height = chip->num_periods; buf->vb.size = chip->dma_size; - buf->vb.field = V4L2_FIELD_NONE; videobuf_dma_init(&buf->vb.dma); - videobuf_dma_init_kernel(&buf->vb.dma,PCI_DMA_FROMDEVICE, + ret = videobuf_dma_init_kernel(&buf->vb.dma, PCI_DMA_FROMDEVICE, (PAGE_ALIGN(buf->vb.size) >> PAGE_SHIFT)); + if (ret < 0) + goto error; - videobuf_pci_dma_map(chip->pci,&buf->vb.dma); - + ret = videobuf_pci_dma_map(chip->pci,&buf->vb.dma); + if (ret < 0) + goto error; - cx88_risc_databuffer(chip->pci, &buf->risc, - buf->vb.dma.sglist, - buf->vb.width, buf->vb.height); + ret = cx88_risc_databuffer(chip->pci, &buf->risc, buf->vb.dma.sglist, + buf->vb.width, buf->vb.height, 1); + if (ret < 0) + goto error; - buf->risc.jmp[0] = cpu_to_le32(RISC_JUMP | RISC_IRQ1 | RISC_CNT_INC); + /* Loop back to start of program */ + buf->risc.jmp[0] = cpu_to_le32(RISC_JUMP|RISC_IRQ1|RISC_CNT_INC); buf->risc.jmp[1] = cpu_to_le32(buf->risc.dma); buf->vb.state = STATE_PREPARED; - buf->bpl = chip->period_size; chip->buf = buf; chip->dma_risc = buf->vb.dma; - dprintk(1,"Buffer ready at %u\n",chip->dma_risc.nr_pages); substream->runtime->dma_area = chip->dma_risc.vmalloc; return 0; + +error: + kfree(buf); + return ret; } /* @@ -477,7 +455,6 @@ static int snd_cx88_prepare(struct snd_pcm_substream *substream) return 0; } - /* * trigger callback */ @@ -486,6 +463,7 @@ static int snd_cx88_card_trigger(struct snd_pcm_substream *substream, int cmd) snd_cx88_card_t *chip = snd_pcm_substream_chip(substream); int err; + /* Local interrupts are already disabled by ALSA */ spin_lock(&chip->reg_lock); switch (cmd) { @@ -512,17 +490,14 @@ static snd_pcm_uframes_t snd_cx88_pointer(struct snd_pcm_substream *substream) { snd_cx88_card_t *chip = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; + u16 count; - if (chip->read_count) { - chip->read_count -= snd_pcm_lib_period_bytes(substream); - chip->read_offset += snd_pcm_lib_period_bytes(substream); - if (chip->read_offset == chip->dma_size) - chip->read_offset = 0; - } - - dprintk(2, "Pointer time, will return %li, read %li\n",chip->read_offset,chip->read_count); - return bytes_to_frames(runtime, chip->read_offset); + count = atomic_read(&chip->count); +// dprintk(2, "%s - count %d (+%u), period %d, frame %lu\n", __FUNCTION__, +// count, new, count & (runtime->periods-1), +// runtime->period_size * (count & (runtime->periods-1))); + return runtime->period_size * (count & (runtime->periods-1)); } /* @@ -592,10 +567,13 @@ static int snd_cx88_capture_volume_put(struct snd_kcontrol *kcontrol, int v; u32 old_control; + /* Do we really know this will always be called with IRQs on? */ spin_lock_irq(&chip->reg_lock); + old_control = 0x3f - (cx_read(AUD_VOL_CTL) & 0x3f); v = 0x3f - (value->value.integer.value[0] & 0x3f); cx_andor(AUD_VOL_CTL, 0x3f, v); + spin_unlock_irq(&chip->reg_lock); return v != old_control; diff --git a/drivers/media/video/cx88/cx88-core.c b/drivers/media/video/cx88/cx88-core.c index ce7f1f0ae0..41f7f374c1 100644 --- a/drivers/media/video/cx88/cx88-core.c +++ b/drivers/media/video/cx88/cx88-core.c @@ -68,13 +68,15 @@ static DEFINE_MUTEX(devlist); #define NO_SYNC_LINE (-1U) +/* @lpi: lines per IRQ, or 0 to not generate irqs. Note: IRQ to be + generated _after_ lpi lines are transferred. */ static u32* cx88_risc_field(u32 *rp, struct scatterlist *sglist, unsigned int offset, u32 sync_line, unsigned int bpl, unsigned int padding, - unsigned int lines) + unsigned int lines, unsigned int lpi) { struct scatterlist *sg; - unsigned int line,todo; + unsigned int line,todo,sol; /* sync instruction */ if (sync_line != NO_SYNC_LINE) @@ -87,15 +89,19 @@ static u32* cx88_risc_field(u32 *rp, struct scatterlist *sglist, offset -= sg_dma_len(sg); sg++; } + if (lpi && line>0 && !(line % lpi)) + sol = RISC_SOL | RISC_IRQ1 | RISC_CNT_INC; + else + sol = RISC_SOL; if (bpl <= sg_dma_len(sg)-offset) { /* fits into current chunk */ - *(rp++)=cpu_to_le32(RISC_WRITE|RISC_SOL|RISC_EOL|bpl); + *(rp++)=cpu_to_le32(RISC_WRITE|sol|RISC_EOL|bpl); *(rp++)=cpu_to_le32(sg_dma_address(sg)+offset); offset+=bpl; } else { /* scanline needs to be split */ todo = bpl; - *(rp++)=cpu_to_le32(RISC_WRITE|RISC_SOL| + *(rp++)=cpu_to_le32(RISC_WRITE|sol| (sg_dma_len(sg)-offset)); *(rp++)=cpu_to_le32(sg_dma_address(sg)+offset); todo -= (sg_dma_len(sg)-offset); @@ -146,10 +152,10 @@ int cx88_risc_buffer(struct pci_dev *pci, struct btcx_riscmem *risc, rp = risc->cpu; if (UNSET != top_offset) rp = cx88_risc_field(rp, sglist, top_offset, 0, - bpl, padding, lines); + bpl, padding, lines, 0); if (UNSET != bottom_offset) rp = cx88_risc_field(rp, sglist, bottom_offset, 0x200, - bpl, padding, lines); + bpl, padding, lines, 0); /* save pointer to jmp instruction address */ risc->jmp = rp; @@ -159,7 +165,7 @@ int cx88_risc_buffer(struct pci_dev *pci, struct btcx_riscmem *risc, int cx88_risc_databuffer(struct pci_dev *pci, struct btcx_riscmem *risc, struct scatterlist *sglist, unsigned int bpl, - unsigned int lines) + unsigned int lines, unsigned int lpi) { u32 instructions; u32 *rp; @@ -176,7 +182,7 @@ int cx88_risc_databuffer(struct pci_dev *pci, struct btcx_riscmem *risc, /* write risc instructions */ rp = risc->cpu; - rp = cx88_risc_field(rp, sglist, 0, NO_SYNC_LINE, bpl, 0, lines); + rp = cx88_risc_field(rp, sglist, 0, NO_SYNC_LINE, bpl, 0, lines, lpi); /* save pointer to jmp instruction address */ risc->jmp = rp; diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c index 6ece35b5fc..4e6a84f15d 100644 --- a/drivers/media/video/cx88/cx88-mpeg.c +++ b/drivers/media/video/cx88/cx88-mpeg.c @@ -253,7 +253,7 @@ int cx8802_buf_prepare(struct videobuf_queue *q, struct cx8802_dev *dev, goto fail; cx88_risc_databuffer(dev->pci, &buf->risc, buf->vb.dma.sglist, - buf->vb.width, buf->vb.height); + buf->vb.width, buf->vb.height, 0); } buf->vb.state = STATE_PREPARED; return 0; diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h index 80e49f986f..e436a37b44 100644 --- a/drivers/media/video/cx88/cx88.h +++ b/drivers/media/video/cx88/cx88.h @@ -519,7 +519,7 @@ cx88_risc_buffer(struct pci_dev *pci, struct btcx_riscmem *risc, extern int cx88_risc_databuffer(struct pci_dev *pci, struct btcx_riscmem *risc, struct scatterlist *sglist, unsigned int bpl, - unsigned int lines); + unsigned int lines, unsigned int lpi); extern int cx88_risc_stopper(struct pci_dev *pci, struct btcx_riscmem *risc, u32 reg, u32 mask, u32 value); -- 2.39.5