On Sat, 28 Feb 2009, Andy Walls wrote: > On Fri, 2009-02-27 at 21:05 +0200, Igor M. Liplianin wrote: > > On 27 февраля 2009, "Igor M. Liplianin" <liplianin@xxxxxx> wrote: > > > On Fri, 27 Feb 2009, Igor M. Liplianin wrote: > > > > 01/02: dm1105: not demuxing from interrupt context. > > > > http://mercurial.intuxication.org/hg/v4l-dvb-commits?cmd=changeset;node=6 > > > >faf0753950b > > > > > > I'm not sure if you considered this, but the default work queue is > > > multi-threaded with a kernel thread for each CPU. This means that if the > > > IRQ handler calls schedule_work() while your work function is running then > > > you can get a second copy running of your work function running at the same > > > time. It doesn't look like your work function uses atomit_t or locking, so > > > I'm guessing it's not safe to run concurrently. > > > > > > For the pluto2 patch, I created a single threaded work queue. This avoids > > > the concurrency problem and it's not like the demuxer can run in parallel > > > anyway. Having your own work queue also means that you don't have to worry > > > about latency from whatever other tasks might be in the default work queue. > > > > > > Also consider that the work function is queued mutliple times before it > > > runs, it will only run once. I.e. queuing a work func that's already in > > > the queue does nothing (one the function starts to run, it's no longer in > > > the queue and can be added again before it's finished). The pluto2 patch I > > > posted didn't take this into account, but I've since fixed it. > > > > I'll return to this :) > > But it complicates things more and more :( > > Yes it does complicate things. Here are some excerpts from what I had > to do for cx18. Perhaps it will help you. (Or perhaps it will not help > at all. The CX23418 chip put alot of requirements on me that drove my > solution.) Here's the latest patch for pluto2. It's a much simpler chip than cx18. I've used atomic operations to design a lockless system. If the driver runs out of DMA buffers it will set a "stalled" flag and not re-enable DMA by acking the IRQ. When the work function has run and freed up some buffers, it will clear the stalled flag and re-enable DMA. The device's internal buffer will have to do while the driver is stalled to avoid an overrun. diff -r 960985ba30c6 -r 92fadfbb8880 linux/drivers/media/dvb/pluto2/pluto2.c --- a/linux/drivers/media/dvb/pluto2/pluto2.c Tue Feb 10 21:31:59 2009 +0100 +++ b/linux/drivers/media/dvb/pluto2/pluto2.c Fri Feb 27 11:10:56 2009 -0800 @@ -23,6 +23,25 @@ * */ +/* + * Internal Buffer: + * The device has an internal 4kB buffer than can hold up to 21 TS packets. If + * MSKO is cleared, the device will set OVR and generate an irq if the internal + * buffer overflows. + * + * DMA: + * Data will be DMAed to the address in PCAR. The register will auto-increment + * after each transfer and should not be written to during DMA. NBPACKETS + * stores the number of packets transferred during the last DMA operation. If + * DEM is cleared an irq will be generated after DMA and DE (and NBPACKETS) + * will be set. A new DMA transfer will not start until IACK is set, so this + * is the time to set PCAR if needed. + * + * The AFULL "almost full" function is not supported. ADEF * 2 is the almost + * full trigger point. Maybe this is also the point at which a DMA operation + * is triggered? + */ + #include <linux/i2c.h> #include <linux/i2c-algo-bit.h> #include <linux/init.h> @@ -87,13 +106,28 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr #define SLCS_SWC (0x0001 << 0) #define TS_DMA_PACKETS (8) -#define TS_DMA_BYTES (188 * TS_DMA_PACKETS) +#define TS_DMA_BYTES (4096) +#define TS_DMA_BUFFERS (2) #define I2C_ADDR_TDA10046 0x10 #define I2C_ADDR_TUA6034 0xc2 #define NHWFILTERS 8 struct pluto { + /* DMA buffer, at start so it's page aligned for streaming DMA. Each + * buf gets its own page, as streaming DMA operates on page + * granularity. We only need 4k (which is usually what page size is) + * to hold the maximum DMA size of 21 packets. */ + u8 dma_buf[TS_DMA_BUFFERS][PAGE_SIZE]; + + dma_addr_t dma_addr[TS_DMA_BUFFERS]; + atomic_t npackets[TS_DMA_BUFFERS]; + atomic_t dma_buf_num, dmx_buf_num; + unsigned long stalled; + struct work_struct work; + struct workqueue_struct *dmaq; + char wqname[16]; + /* pci */ struct pci_dev *pdev; u8 __iomem *io_mem; @@ -116,11 +150,6 @@ struct pluto { /* irq */ unsigned int overflow; - - /* dma */ - dma_addr_t dma_addr; - u8 dma_buf[TS_DMA_BYTES]; - u8 dummy[4096]; }; static inline struct pluto *feed_to_pluto(struct dvb_demux_feed *feed) @@ -232,26 +261,36 @@ static void pluto_reset_ts(struct pluto } } -static void pluto_set_dma_addr(struct pluto *pluto) -{ - pluto_writereg(pluto, REG_PCAR, pluto->dma_addr); +static void pluto_set_dma_addr(struct pluto *pluto, unsigned int num) +{ + pluto_writereg(pluto, REG_PCAR, pluto->dma_addr[num]); } static int __devinit pluto_dma_map(struct pluto *pluto) { - pluto->dma_addr = pci_map_single(pluto->pdev, pluto->dma_buf, + int r; + + pluto->dma_addr[0] = pci_map_single(pluto->pdev, pluto->dma_buf[0], TS_DMA_BYTES, PCI_DMA_FROMDEVICE); - -#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 27) - return pci_dma_mapping_error(pluto->dma_addr); -#else - return pci_dma_mapping_error(pluto->pdev, pluto->dma_addr); -#endif + r = pci_dma_mapping_error(pluto->pdev, pluto->dma_addr[0]); + if (r) + return r; + + pluto->dma_addr[1] = pci_map_single(pluto->pdev, pluto->dma_buf[1], + TS_DMA_BYTES, PCI_DMA_FROMDEVICE); + r = pci_dma_mapping_error(pluto->pdev, pluto->dma_addr[1]); + if (r) + pci_unmap_single(pluto->pdev, pluto->dma_addr[0], + TS_DMA_BYTES, PCI_DMA_FROMDEVICE); + + return r; } static void pluto_dma_unmap(struct pluto *pluto) { - pci_unmap_single(pluto->pdev, pluto->dma_addr, + pci_unmap_single(pluto->pdev, pluto->dma_addr[0], + TS_DMA_BYTES, PCI_DMA_FROMDEVICE); + pci_unmap_single(pluto->pdev, pluto->dma_addr[1], TS_DMA_BYTES, PCI_DMA_FROMDEVICE); } @@ -287,11 +326,18 @@ static int pluto_stop_feed(struct dvb_de return 0; } -static void pluto_dma_end(struct pluto *pluto, unsigned int nbpackets) -{ - /* synchronize the DMA transfer with the CPU - * first so that we see updated contents. */ - pci_dma_sync_single_for_cpu(pluto->pdev, pluto->dma_addr, +static int pluto_dma_end(struct pluto *pluto, unsigned int nbpackets) +{ + unsigned int bnum = atomic_read(&pluto->dma_buf_num) % TS_DMA_BUFFERS; + unsigned int newbnum; + int ret = 1; + + if (test_bit(0, &pluto->stalled)) + dev_err(&pluto->pdev->dev, "DMA while driver stalled?\n"); + + /* synchronize the DMA transfer with the CPU first so that we see + * updated contents. */ + pci_dma_sync_single_for_cpu(pluto->pdev, pluto->dma_addr[bnum], TS_DMA_BYTES, PCI_DMA_FROMDEVICE); /* Workaround for broken hardware: @@ -305,28 +351,105 @@ static void pluto_dma_end(struct pluto * * has been transfered. Only a reset seems to solve this */ if ((nbpackets == 0) || (nbpackets > TS_DMA_PACKETS)) { - unsigned int i = 0; - while (pluto->dma_buf[i] == 0x47) - i += 188; - nbpackets = i / 188; - if (i == 0) { + unsigned int i; + + for (i = 0, nbpackets = 0; + i < sizeof(pluto->dma_buf[bnum]) && pluto->dma_buf[bnum][i] == 0x47; + i += 188, nbpackets++) + ; + + if (nbpackets == 0) { pluto_reset_ts(pluto, 1); - dev_printk(KERN_DEBUG, &pluto->pdev->dev, "resetting TS because of invalid packet counter\n"); + dev_dbg(&pluto->pdev->dev, + "resetting TS because of invalid packet counter\n"); + + /* Just re-use old buffer */ + pci_dma_sync_single_for_device(pluto->pdev, + pluto->dma_addr[bnum], + TS_DMA_BYTES, + PCI_DMA_FROMDEVICE); + pluto_set_dma_addr(pluto, bnum); + return 1; } } - dvb_dmx_swfilter_packets(&pluto->demux, pluto->dma_buf, nbpackets); - - /* clear the dma buffer. this is needed to be able to identify - * new valid ts packets above */ - memset(pluto->dma_buf, 0, nbpackets * 188); + /* Mark buffer as used */ + atomic_set(&pluto->npackets[bnum], nbpackets); + + /* Switch to the next buffer */ + newbnum = atomic_inc_return(&pluto->dma_buf_num) % TS_DMA_BUFFERS; /* reset the dma address */ - pluto_set_dma_addr(pluto); - - /* sync the buffer and give it back to the card */ - pci_dma_sync_single_for_device(pluto->pdev, pluto->dma_addr, - TS_DMA_BYTES, PCI_DMA_FROMDEVICE); + pluto_set_dma_addr(pluto, newbnum); + + if (atomic_read(&pluto->npackets[newbnum])) { + dev_dbg(&pluto->pdev->dev, "buffer not ready\n"); + /* Buffer isn't empty. We want to hold off setting IACK until + * it is, so that it won't get DMAed into. */ + ret = 0; + + /* Setting stalled will tell the demux work function that it + * needs to do the IACK when it has freed up a buffer. */ + set_bit(0, &pluto->stalled); + mb(); + } + + /* queue demuxer work function to processes buffer(s) */ + queue_work(pluto->dmaq, &pluto->work); + + return ret; +} + +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) +static void pluto_dmx_buffers(void *_pluto) +#else +static void pluto_dmx_buffers(struct work_struct *work) +#endif +{ +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) + struct pluto *pluto = _pluto; +#else + struct pluto *pluto = container_of(work, struct pluto, work); +#endif + + /* If this function is queued multiple times by multiple IRQs before it + * runs, it will still only be run once. So we must process all buffers + * available and not just one. It also possible that it will get queued + * again while it is already running, in which it will run again but + * there will be no buffers to process, but we still must check if the + * driver is stalled. */ + while (atomic_read(&pluto->dmx_buf_num) != + atomic_read(&pluto->dma_buf_num)) { + unsigned int bnum = atomic_read(&pluto->dmx_buf_num) % TS_DMA_BUFFERS; + unsigned int npackets = atomic_read(&pluto->npackets[bnum]); + unsigned int i; + + atomic_inc(&pluto->dmx_buf_num); + + if (!npackets) { + dev_warn(&pluto->pdev->dev, + "Empty buffer queued to demuxer?\n"); + continue; + } + + dvb_dmx_swfilter_packets(&pluto->demux, pluto->dma_buf[bnum], + npackets); + + /* Clear the MPEG start codes in the DMA buffer. This is + * needed to be able to identify new valid TS packets. */ + for (i = 0; npackets; npackets--, i += 188) + pluto->dma_buf[bnum][i] = 0; + + /* Give buffer back to device */ + pci_dma_sync_single_for_device(pluto->pdev, pluto->dma_addr[bnum], + TS_DMA_BYTES, PCI_DMA_FROMDEVICE); + + /* Mark buffer as empty */ + atomic_set(&pluto->npackets[bnum], 0); + } + /* Need IACK to enable DMA again? */ + if (test_and_clear_bit(0, &pluto->stalled)) + pluto_write_tscr(pluto, TSCR_IACK); } #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,19) @@ -337,6 +460,7 @@ static irqreturn_t pluto_irq(int irq, vo { struct pluto *pluto = dev_id; u32 tscr; + int ack = 1; /* check whether an interrupt occured on this device */ tscr = pluto_readreg(pluto, REG_TSCR); @@ -349,24 +473,24 @@ static irqreturn_t pluto_irq(int irq, vo return IRQ_HANDLED; } + /* overflow interrupt */ + if (tscr & TSCR_OVR) + pluto->overflow++; + /* dma end interrupt */ if (tscr & TSCR_DE) { - pluto_dma_end(pluto, (tscr & TSCR_NBPACKETS) >> 24); - /* overflow interrupt */ - if (tscr & TSCR_OVR) - pluto->overflow++; + ack = pluto_dma_end(pluto, (tscr & TSCR_NBPACKETS) >> 24); if (pluto->overflow) { dev_err(&pluto->pdev->dev, "overflow irq (%d)\n", pluto->overflow); pluto_reset_ts(pluto, 1); pluto->overflow = 0; } - } else if (tscr & TSCR_OVR) { - pluto->overflow++; } /* ACK the interrupt */ - pluto_write_tscr(pluto, tscr | TSCR_IACK); + if (ack) + pluto_write_tscr(pluto, tscr | TSCR_IACK); return IRQ_HANDLED; } @@ -412,7 +536,7 @@ static int __devinit pluto_hw_init(struc #endif /* map DMA and set address */ pluto_dma_map(pluto); - pluto_set_dma_addr(pluto); + pluto_set_dma_addr(pluto, 0); /* enable interrupts */ pluto_enable_irqs(pluto); @@ -610,7 +734,7 @@ static int __devinit pluto2_probe(struct pluto = kzalloc(sizeof(struct pluto), GFP_KERNEL); if (!pluto) - goto out; + return -ENOMEM; pluto->pdev = pdev; @@ -639,13 +763,9 @@ static int __devinit pluto2_probe(struct pci_set_drvdata(pdev, pluto); - ret = request_irq(pdev->irq, pluto_irq, IRQF_SHARED, DRIVER_NAME, pluto); + ret = pluto_hw_init(pluto); if (ret < 0) goto err_pci_iounmap; - - ret = pluto_hw_init(pluto); - if (ret < 0) - goto err_free_irq; /* i2c */ i2c_set_adapdata(&pluto->i2c_adap, pluto); @@ -721,9 +841,27 @@ static int __devinit pluto2_probe(struct goto err_disconnect_frontend; dvb_net_init(dvb_adapter, &pluto->dvbnet, dmx); -out: - return ret; - + +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) + INIT_WORK(&pluto->work, pluto_dmx_buffers, pluto); +#else + INIT_WORK(&pluto->work, pluto_dmx_buffers); +#endif + sprintf(pluto->wqname, "%s[%d] dvb", dvb_adapter->name, dvb_adapter->num); + pluto->dmaq = create_singlethread_workqueue(pluto->wqname); + if (!pluto->dmaq) + goto err_dvb_net; + + ret = request_irq(pdev->irq, pluto_irq, IRQF_SHARED, DRIVER_NAME, pluto); + if (ret < 0) + goto err_workqueue; + + return 0; + +err_workqueue: + destroy_workqueue(pluto->dmaq); +err_dvb_net: + dvb_net_release(&pluto->dvbnet); err_disconnect_frontend: dmx->disconnect_frontend(dmx); err_remove_mem_frontend: @@ -740,8 +878,6 @@ err_i2c_del_adapter: i2c_del_adapter(&pluto->i2c_adap); err_pluto_hw_exit: pluto_hw_exit(pluto); -err_free_irq: - free_irq(pdev->irq, pluto); err_pci_iounmap: pci_iounmap(pdev, pluto->io_mem); err_pci_release_regions: @@ -751,7 +887,7 @@ err_kfree: err_kfree: pci_set_drvdata(pdev, NULL); kfree(pluto); - goto out; + return ret; } static void __devexit pluto2_remove(struct pci_dev *pdev) diff -r 960985ba30c6 -r 92fadfbb8880 v4l/compat.h --- a/v4l/compat.h Tue Feb 10 21:31:59 2009 +0100 +++ b/v4l/compat.h Fri Feb 27 11:10:56 2009 -0800 @@ -421,4 +421,8 @@ static inline int usb_endpoint_type(cons } while (0) #endif -#endif +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 27) +#define pci_dma_mapping_error(dev, addr) pci_dma_mapping_error(addr) +#endif + +#endif -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
