From cc9429bcb6e36e9f2c51e4e47b95740e472c4c2d Mon Sep 17 00:00:00 2001 From: Pieter Palmers Date: Wed, 19 Mar 2008 22:10:59 +0100 Subject: ieee1394: rawiso: requeue packet for transmission after skipped cycle As it seems, some host controllers have issues that can cause them to skip cycles now and then when using large packets. I suspect that this is due to DMA not succeeding in time. If the transmit fifo can't contain more than one packet (big packets), the DMA should provide a new packet each cycle (125us). I am under the impression that my current PCI express test system can't guarantee this. In any case, the patch tries to provide a workaround as follows: The DMA program descriptors are modified such that when an error occurs, the DMA engine retries the descriptor the next cycle instead of stalling. This way no data is lost. The side effect of this is that packets are sent with one cycle delay. This however might not be that much of a problem for certain protocols (e.g. AM824). If they use padding packets for e.g. rate matching they can drop one of those to resync the streams. The amount of skips between two userspace wakeups is counted. This number is then propagated to userspace through the upper 16 bits of the 'dropped' parameter. This allows unmodified userspace applications due to the following: 1) libraw simply passes this dropped parameter to the user application 2) the meaning of the dropped parameter is: if it's nonzero, something bad has happened. The actual value of the parameter at this moment does not have a specific meaning. A libraw client can then retrieve the number of skipped cycles and account for them if needed. Signed-off-by: Pieter Palmers Signed-off-by: Stefan Richter --- drivers/ieee1394/iso.h | 2 ++ drivers/ieee1394/ohci1394.c | 34 ++++++++++++++++++++++++++++++++++ drivers/ieee1394/raw1394.c | 7 ++++++- 3 files changed, 42 insertions(+), 1 deletion(-) (limited to 'drivers/ieee1394') diff --git a/drivers/ieee1394/iso.h b/drivers/ieee1394/iso.h index b94e55e6eaa5..b5de5f21ef78 100644 --- a/drivers/ieee1394/iso.h +++ b/drivers/ieee1394/iso.h @@ -123,6 +123,8 @@ struct hpsb_iso { /* how many times the buffer has overflowed or underflowed */ atomic_t overflows; + /* how many cycles were skipped for a given context */ + atomic_t skips; /* Current number of bytes lost in discarded packets */ int bytes_discarded; diff --git a/drivers/ieee1394/ohci1394.c b/drivers/ieee1394/ohci1394.c index 0690469fcecf..e509e13cb7a7 100644 --- a/drivers/ieee1394/ohci1394.c +++ b/drivers/ieee1394/ohci1394.c @@ -1723,6 +1723,8 @@ struct ohci_iso_xmit { struct dma_prog_region prog; struct ohci1394_iso_tasklet task; int task_active; + int last_cycle; + atomic_t skips; u32 ContextControlSet; u32 ContextControlClear; @@ -1759,6 +1761,8 @@ static int ohci_iso_xmit_init(struct hpsb_iso *iso) iso->hostdata = xmit; xmit->ohci = iso->host->hostdata; xmit->task_active = 0; + xmit->last_cycle = -1; + atomic_set(&iso->skips, 0); dma_prog_region_init(&xmit->prog); @@ -1856,6 +1860,26 @@ static void ohci_iso_xmit_task(unsigned long data) /* parse cycle */ cycle = le32_to_cpu(cmd->output_last.status) & 0x1FFF; + if (xmit->last_cycle > -1) { + int cycle_diff = cycle - xmit->last_cycle; + int skip; + + /* unwrap */ + if (cycle_diff < 0) { + cycle_diff += 8000; + if (cycle_diff < 0) + PRINT(KERN_ERR, "bogus cycle diff %d\n", + cycle_diff); + } + + skip = cycle_diff - 1; + if (skip > 0) { + DBGMSG("skipped %d cycles without packet loss", skip); + atomic_add(skip, &iso->skips); + } + } + xmit->last_cycle = cycle; + /* tell the subsystem the packet has gone out */ hpsb_iso_packet_sent(iso, cycle, event != 0x11); @@ -1943,6 +1967,16 @@ static int ohci_iso_xmit_queue(struct hpsb_iso *iso, struct hpsb_iso_packet_info prev->output_last.branchAddress = cpu_to_le32( dma_prog_region_offset_to_bus(&xmit->prog, sizeof(struct iso_xmit_cmd) * next_i) | 3); + /* + * Link the skip address to this descriptor itself. This causes a + * context to skip a cycle whenever lost cycles or FIFO overruns occur, + * without dropping the data at that point the application should then + * decide whether this is an error condition or not. Some protocols + * can deal with this by dropping some rate-matching padding packets. + */ + next->output_more_immediate.branchAddress = + prev->output_last.branchAddress; + /* disable interrupt, unless required by the IRQ interval */ if (prev_i % iso->irq_interval) { prev->output_last.control &= cpu_to_le32(~(3 << 20)); /* no interrupt */ diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c index 04e96ba56e09..567dafc2369d 100644 --- a/drivers/ieee1394/raw1394.c +++ b/drivers/ieee1394/raw1394.c @@ -2356,13 +2356,16 @@ static void rawiso_activity_cb(struct hpsb_iso *iso) static void raw1394_iso_fill_status(struct hpsb_iso *iso, struct raw1394_iso_status *stat) { + int overflows = atomic_read(&iso->overflows); + int skips = atomic_read(&iso->skips); + stat->config.data_buf_size = iso->buf_size; stat->config.buf_packets = iso->buf_packets; stat->config.channel = iso->channel; stat->config.speed = iso->speed; stat->config.irq_interval = iso->irq_interval; stat->n_packets = hpsb_iso_n_ready(iso); - stat->overflows = atomic_read(&iso->overflows); + stat->overflows = ((skips & 0xFFFF) << 16) | ((overflows & 0xFFFF)); stat->xmit_cycle = iso->xmit_cycle; } @@ -2437,6 +2440,8 @@ static int raw1394_iso_get_status(struct file_info *fi, void __user * uaddr) /* reset overflow counter */ atomic_set(&iso->overflows, 0); + /* reset skip counter */ + atomic_set(&iso->skips, 0); return 0; } -- cgit v1.2.3 From e38649702ea64fdbbab3dd459bf8558142dd3bc4 Mon Sep 17 00:00:00 2001 From: Tony Breeds Date: Thu, 24 Apr 2008 09:02:04 +0200 Subject: ieee1394: silence defined but not used warning in non-modular builds Currently the kernel will issue the following warning: drivers/ieee1394/raw1394.c:2938: warning: 'raw1394_id_table' defined but not used Add #ifdef MODULE guards around the declaration. Signed-off-by: Tony Breeds Ditto with dv1394_id_table and video1394_id_table. Signed-off-by: Stefan Richter --- drivers/ieee1394/dv1394.c | 2 ++ drivers/ieee1394/raw1394.c | 2 ++ drivers/ieee1394/video1394.c | 2 ++ 3 files changed, 6 insertions(+) (limited to 'drivers/ieee1394') diff --git a/drivers/ieee1394/dv1394.c b/drivers/ieee1394/dv1394.c index 6228fadacd38..9d19aec5820a 100644 --- a/drivers/ieee1394/dv1394.c +++ b/drivers/ieee1394/dv1394.c @@ -2167,6 +2167,7 @@ static const struct file_operations dv1394_fops= /* * Export information about protocols/devices supported by this driver. */ +#ifdef MODULE static struct ieee1394_device_id dv1394_id_table[] = { { .match_flags = IEEE1394_MATCH_SPECIFIER_ID | IEEE1394_MATCH_VERSION, @@ -2177,6 +2178,7 @@ static struct ieee1394_device_id dv1394_id_table[] = { }; MODULE_DEVICE_TABLE(ieee1394, dv1394_id_table); +#endif /* MODULE */ static struct hpsb_protocol_driver dv1394_driver = { .name = "dv1394", diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c index 567dafc2369d..ec2a0adbedb2 100644 --- a/drivers/ieee1394/raw1394.c +++ b/drivers/ieee1394/raw1394.c @@ -2940,6 +2940,7 @@ static int raw1394_release(struct inode *inode, struct file *file) /* * Export information about protocols/devices supported by this driver. */ +#ifdef MODULE static struct ieee1394_device_id raw1394_id_table[] = { { .match_flags = IEEE1394_MATCH_SPECIFIER_ID | IEEE1394_MATCH_VERSION, @@ -2961,6 +2962,7 @@ static struct ieee1394_device_id raw1394_id_table[] = { }; MODULE_DEVICE_TABLE(ieee1394, raw1394_id_table); +#endif /* MODULE */ static struct hpsb_protocol_driver raw1394_driver = { .name = "raw1394", diff --git a/drivers/ieee1394/video1394.c b/drivers/ieee1394/video1394.c index e03024eeeac1..e24772d336e1 100644 --- a/drivers/ieee1394/video1394.c +++ b/drivers/ieee1394/video1394.c @@ -1293,6 +1293,7 @@ static const struct file_operations video1394_fops= /* * Export information about protocols/devices supported by this driver. */ +#ifdef MODULE static struct ieee1394_device_id video1394_id_table[] = { { .match_flags = IEEE1394_MATCH_SPECIFIER_ID | IEEE1394_MATCH_VERSION, @@ -1313,6 +1314,7 @@ static struct ieee1394_device_id video1394_id_table[] = { }; MODULE_DEVICE_TABLE(ieee1394, video1394_id_table); +#endif /* MODULE */ static struct hpsb_protocol_driver video1394_driver = { .name = VIDEO1394_DRIVER_NAME, -- cgit v1.2.3