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" <liplia...@tut.by> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
