Re: DVB core enhancements - comments please?
Moikka Antti.
On 07/01/2012 02:11 PM, Antti Palosaari wrote:
Moikka Marko,
-- snip --
Hmm, I did some initial suspend / resume changes for DVB USB when I rewrote it
recently. On suspend, it just kills all ongoing urbs used for streaming. And on
resume it resubmit those urbs in order to resume streaming. It just works as it
doesn't hang computer anymore. What I tested applications continued to show
same television channels on resume.
The problem for that solution is that it does not have any power management as
power management is DVB-core responsibility. So it continues eating current
because chips are not put sleep and due to that those DVB-core changes are
required.
I think that runtime (RT) frontend power saving is a different thing.
It isn't necessarily suspend/resume thing.
I implemented runtime Frontend power saving in 2007 on that patch I referenced.
I used dvb-core's existing functionality. Maybe this concept is applicable for
you too.
I added into Mantis bridge device initialization following functions:
+ mantis-fe-ops.tuner_ops.sleep =
mantis_dvb_tuner_sleep;
+ mantis-fe-ops.tuner_ops.init = mantis_dvb_tuner_init;
tuner_ops.{sleep,init} modification had to be the last one.
I maintained in mantis-has_power the frontend's power status.
Maybe I could have read the active status from PCI context too.
The concept was something like:
- dvb-core has responsibility to call tuner_ops.sleep() and tuner_ops.init()
when applicable.
- Mantis PCI Bridge driver (or specific USB driver) has responsibility
to provide sleep and init implementations for the specific device.
- Mantis bridge device will do the whole task of frontend power management, by
calling Frontend's
tear down / initialization functions when necessary.
- What changes encrypted channels need?
I think none?
regards
Antti
Regards,
Marko
--
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
Re: DVB core enhancements - comments please?
Hi Antti. My suspend / resume patch implemented Kaffeine continues viewing channel after resume. Some of the ideas could be still useful: http://www.spinics.net/lists/linux-dvb/msg19651.html Rest of this email has a more thorough description. Regards, Marko Ristola On 06/28/2012 03:33 AM, Antti Palosaari wrote: Here is my list of needed DVB core related changes. Feel free to comment - what are not needed or what you would like to see instead. I will try to implement what I can (and what I like most interesting :). ... suspend / resume support -- * support is currently quite missing, all what is done is on interface drivers * needs power management * streaming makes it hard * quite a lot work to get it working in case of straming is ongoing I've implemented Suspend/Resume for Mantis cu1216 in 2007 (PCI DVB-C device): Kaffeine continued viewing the channel after resume. When Tuner was idle too long, it was powered off too. According to Manu Abraham at that time, somewhat smaller patch would have sufficed. That patch contais nonrelated fixes too, and won't compile now. Here is the reference (with Manu's answer): Start of the thread: http://www.spinics.net/lists/linux-dvb/msg19532.html The patch: http://www.spinics.net/lists/linux-dvb/msg19651.html Manu's answer: http://www.spinics.net/lists/linux-dvb/msg19668.html Thoughts about up-to-date implementation - Bridge (PCI) device must implement suspend/resume callbacks. - Frontend might need some change (power off / power on callbacks)? - save Tuner / DMA transfer state to memory might be addable to dvb_core. - Bridge device supporting suspend/resume needs to have a (non-regression) fallback for (frontend) devices that don't have a full tested Kaffeine works suspend/resume implementation yet. - What changes encrypted channels need? Marko -- 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
Re: [PATCH] [media] drxk: change it to use request_firmware_nowait()
Hi Mauro
I think that your __i2c_transfer() solution is elegant for DVB-C frontend tuner
reprogramming sleep cases:
TDA6650 programming manual, page 9 states:
Main divider data are valid only if no new I2C-bus transmission
is started (START condition) during the computation
period of 50 μs.
http://www.datasheetcatalog.org/datasheet/philips/TDA6650.pdf
It states that after TDA6650 frontend programming, the whole I2C bus
(for any I2C transfer into any I2C device) must be quiet at least 50μs.
My main point for __i2c_transfer() elegance can be seen easilly
from the code examples below.
-
An elegant way to solve only the 50μs TDA6650 sleep problem (not gate control
issue),
is to solve it in tda665x.c's tda665x_set_state():
static int tda665x_set_state(struct dvb_frontend *fe,
enum tuner_param param,
struct tuner_state *tstate)
{
...
i2c_lock_adapter(state-i2c);
/* Set params without taking I2C lock ( uses __i2c_transfer() ) */
err = __tda665x_write(state, buf, 5);
usleep(50);
i2c_unlock_adapter(state-i2c);
...
}
Ugly robust way to solveonly the 50μs TDA6650 sleep problem (not gate control
issue,
is to teach I2C transfer code TDA6650's requirements:
-- warning: ugly --
saa7146_i2c_transfer(..) {
...
mutex_lock_interruptible(dev-i2c_lock);
...
send_i2c_messages();
if (num == 5 is_tda665())
usleep(50);
mutex_unlock(dev-i2c_lock);
}
Here is a robust version with __i2c_transfer(). Main idea is to
protect the whole communicating with I2C Frontend device with I2C lock:
- Open I2C Gate so that you can talk to DVB Frontend Tuner.
- Send 5 bytes into DVB-C Frontend tuner.
- Quiesce I2C bus by sleeping 50μs to protect the Frontend Tuner message.
- Close I2C Gate so that DVB Frontend Tuner isn't reachable by I2C bus.
static int tda665x_set_state(struct dvb_frontend *fe,
enum tuner_param param,
struct tuner_state *tstate)
{
...
i2c_lock_adapter(state-i2c);
__open_i2c_gate(state);
/* Set params without taking I2C lock ( uses __i2c_transfer() ) */
err = __tda665x_write(state, buf, 5);
usleep(50);
__close_i2c_gate(state);
i2c_unlock_adapter(state-i2c);
...
}
-- Ugly solution for the same full problem in SAA7146 I2C transfer function
itself (code with some unsolved problems) --
saa7146_i2c_transfer(..) {
...
mutex_lock_interruptible(dev-i2c_lock);
if (is_tda665(addr))
send_msg_tda665_gate_open(dev);
...
send_i2c_messages();
if (num == 5 is_tda665())
usleep(50);
if (is_tda665(addr))
send_msg_tda665_gate_close(dev);
mutex_unlock(dev-i2c_lock);
}
Regards,
Marko Ristola
On 06/21/2012 10:10 PM, Mauro Carvalho Chehab wrote:
Em 21-06-2012 10:36, Mauro Carvalho Chehab escreveu:
The firmware blob may not be available when the driver probes.
Instead of blocking the whole kernel use request_firmware_nowait() and
continue without firmware.
This shouldn't be that bad on drx-k devices, as they all seem to have an
internal firmware. So, only the firmware update will take a little longer
to happen.
While thinking on converting another DVB frontend driver, I just realized
that a patch like that won't work fine.
As most of you know, there are _several_ I2C chips that don't tolerate any
usage of the I2C bus while a firmware is being loaded (I dunno if this is the
case of drx-k, but I won't doubt).
The current approach makes the device probe() logic is serialized. So, there's
no chance that two different I2C drivers to try to access the bus at the same
time, if the bridge driver is properly implemented.
However, now that firmware is loaded asynchronously, several other I2C drivers
may be trying to use the bus at the same time. So, events like IR (and CI)
polling,
tuner get_status, etc can happen during a firmware transfer, causing the
firmware
to not load properly.
A fix for that will require to lock the firmware load I2C traffic into a single
transaction.
So, the code that sends the firmware to the board need to be changed to
something like:
static int DownloadMicrocode(struct drxk_state *state,
const u8 pMCImage[], u32 Length)
{
...
i2c_lock_adapter(state-i2c);
...
for (i = 0; i nBlocks; i += 1) {
...
status = __i2c_transfer(state-i2c, ...);
...
}
i2c_unlock_adapter(state-i2c);
return status;
}
where __i2c_transfer() would be a variant of i2c_transfer() that won't take the
I2C lock.
Other drivers that load firmware at I2C and use request_firmware_nowait() will
also
need a similar approach.
Jean,
What do you think?
Regards,
Mauro
Cc: Antti Palosaari cr...@iki.fi
Cc: Kay Sievers k...@redhat.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
drivers/media/dvb
Re: [PATCH 04/11] Show timeouts on I2C transfers.
I had a plan to rework I2C handling a lot more,
than log the changes.
I wrote a patch that uses I2C IRQ.
I had a feeling that it worked well (with old single CPU desktop computer):
framerate with HDTV was low, but it was glitchless.
Do you want to have the patch to be sent for you?
I think that I solved in the patch robust I2C command + exact IRQ response for
that command,
so that those two will not get out of sync.
I tried to rework the patch to make a bit smaller patch,
but I couldn't test the new one: hardware is too broken.
Regards,
Marko
01.04.2012 18:53, Steinar H. Gunderson kirjoitti:
From: Steinar H. Gunderson se...@samfundet.no
On I2C reads and writes, show if we had any timeouts in the debug output.
Signed-off-by: Steinar H. Gunderson se...@samfundet.no
---
drivers/media/dvb/mantis/mantis_i2c.c | 26 --
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/mantis/mantis_i2c.c
b/drivers/media/dvb/mantis/mantis_i2c.c
index e779451..ddd1922 100644
--- a/drivers/media/dvb/mantis/mantis_i2c.c
+++ b/drivers/media/dvb/mantis/mantis_i2c.c
@@ -38,6 +38,7 @@
static int mantis_i2c_read(struct mantis_pci *mantis, const struct i2c_msg
*msg)
{
u32 rxd, i, stat, trials;
+ u32 timeouts = 0;
dprintk(MANTIS_INFO, 0, %s: Address=[0x%02x] R[ ,
__func__, msg-addr);
@@ -60,6 +61,9 @@ static int mantis_i2c_read(struct mantis_pci *mantis, const
struct i2c_msg *msg)
if (stat MANTIS_INT_I2CDONE)
break;
}
+ if (trials == TRIALS) {
+ ++timeouts;
+ }
dprintk(MANTIS_TMG, 0, I2CDONE: trials=%d\n, trials);
@@ -69,6 +73,9 @@ static int mantis_i2c_read(struct mantis_pci *mantis, const
struct i2c_msg *msg)
if (stat MANTIS_INT_I2CRACK)
break;
}
+ if (trials == TRIALS) {
+ ++timeouts;
+ }
dprintk(MANTIS_TMG, 0, I2CRACK: trials=%d\n, trials);
@@ -76,7 +83,11 @@ static int mantis_i2c_read(struct mantis_pci *mantis,
const struct i2c_msg *msg)
msg-buf[i] = (u8)((rxd 8) 0xFF);
dprintk(MANTIS_INFO, 0, %02x , msg-buf[i]);
}
- dprintk(MANTIS_INFO, 0, ]\n);
+ if (timeouts) {
+ dprintk(MANTIS_INFO, 0, ] %d timeouts\n, timeouts);
+ } else {
+ dprintk(MANTIS_INFO, 0, ]\n);
+ }
return 0;
}
@@ -85,6 +96,7 @@ static int mantis_i2c_write(struct mantis_pci *mantis,
const struct i2c_msg *msg
{
int i;
u32 txd = 0, stat, trials;
+ u32 timeouts = 0;
dprintk(MANTIS_INFO, 0, %s: Address=[0x%02x] W[ ,
__func__, msg-addr);
@@ -108,6 +120,9 @@ static int mantis_i2c_write(struct mantis_pci *mantis,
const struct i2c_msg *msg
if (stat MANTIS_INT_I2CDONE)
break;
}
+ if (trials == TRIALS) {
+ ++timeouts;
+ }
dprintk(MANTIS_TMG, 0, I2CDONE: trials=%d\n, trials);
@@ -117,10 +132,17 @@ static int mantis_i2c_write(struct mantis_pci *mantis,
const struct i2c_msg *msg
if (stat MANTIS_INT_I2CRACK)
break;
}
+ if (trials == TRIALS) {
+ ++timeouts;
+ }
dprintk(MANTIS_TMG, 0, I2CRACK: trials=%d\n, trials);
}
- dprintk(MANTIS_INFO, 0, ]\n);
+ if (timeouts) {
+ dprintk(MANTIS_INFO, 0, ] %d timeouts\n, timeouts);
+ } else {
+ dprintk(MANTIS_INFO, 0, ]\n);
+ }
return 0;
}
--
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
Re: [PATCH] Various nits, fixes and hacks for mantis CA support on SMP
03.03.2012 01:21, Steinar H. Gunderson kirjoitti: On Sat, Mar 03, 2012 at 12:42:19AM +0200, Marko Ristola wrote: I'm not happy with I2CDONE busy looping either. I've tried twice lately to swith into I2C IRQ, but those patches have caused I2CDONE timeouts. Note that there are already timeouts with the current polling code, but they are ignored (my patch makes them at least be printed with verbose=5). I can't immediately recall if it's on RACK, DONE or both. I think that occasional timeouts belong to the picture (RACK missing means: packet lost). If I2CDONE comes immediately when I2C command has been emitted, it is a driver software bug (some earlier I2C command). Do my following I2C logic thoughts make any sense? Well, note first of all that I know next to nothing about I2C, and I've never seen any hardware documentation on the Mantis card (is there any?). But generally it makes sense to me, except that I've never heard of the demand of radio silence for 10 ms before. Ok. Radio silence for 10 ms isn't your problem: when you have a FE_LOCK, 10 ms requirement is no more relevant. There might be race conditions, that the driver possibly manages: 1. If two threads talk into DVB frontend, one could turn off the I2C gate, while the other is talking to DVB frontend. This would case lack of I2CRACK: only way to recover would be to turn the I2C gate on, and then redo the I2C transfer. Note that I've tried putting mutexes around the I2C functions, and it didn't help on the I2C timeouts; however, that was largely on a single-character level, so it might not be enough. (You can see these mutexes being commented out in my patch.) I have now a new idea for what to try with I2C interrupts. It all depends, whether I understand the driver coding well enough. /* Steinar */ I have also a low interest for Mantis coding: Ideas come fast, and if the new code doesn't work right away, I back up the code somewhere and drop it. Some years ago during Summer Holiday time I could get something done and even patches got applied into Linux kernel. Marko Ristola -- 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
Re: [PATCH] Various nits, fixes and hacks for mantis CA support on SMP
Hi Nice work. I looked your code very shortly. I'm not happy with I2CDONE busy looping either. I've tried twice lately to swith into I2C IRQ, but those patches have caused I2CDONE timeouts. Do my following I2C logic thoughts make any sense? I2C bus might have some underlying requirements: 1. When driver wants to talk to a DVB frontend, it first opens up an I2C gate, then talks, and finally closes the I2C gate. This is ok. 2. After setting up DVB frontend frequency etc. I2C bus should be quiet 10ms (radio silence, to fasten taking DVB TS lock). This is ok. There might be race conditions, that the driver possibly manages: 1. If two threads talk into DVB frontend, one could turn off the I2C gate, while the other is talking to DVB frontend. This would case lack of I2CRACK: only way to recover would be to turn the I2C gate on, and then redo the I2C transfer. 2. Thread 1 turns I2C gate into DVB frontend and sets up frequency. It waits 10ms. Thread 2 will talk to some I2C device, thus breaking the silence. This case would not be so bad, because it would just delay taking the wanted DVB TS lock. Regards, Marko Ristola 28.02.2012 03:03, Steinar H. Gunderson kirjoitti: Hi, This patch, against 3.3-rc4, is basically a conglomerate of patches that together seem to make CA support on mantis working and stable, even on SMP systems. (I'm using a Terratec Cinergy DVB-S2 card with a Conax CAM, with mumudvb as userspace.) There are a few fixes from this mailing list and some of my own; the end result is too ugly to include, and there are still things I don't understand at all, but I hope it can be useful for some. Below is the list of what the patch does: - I've followed the instructions from some post on this mailing list to enable CAM support in the first place (mantis_set_direction move to mantis_pci.c, uncomment mantis_ca_init). - The MANTIS_GPIF_STATUS fix from http://patchwork.linuxtv.org/patch/8776/. Not that it seems to change a lot for me, but it makes sense. - I've fixed a ton of SMP-related bugs. Basically a lot of the members of mantis_ca were accessed from several threads without a mutex, which is a big no-no; I've mostly changed to using atomic operations here, although I also added some locks were it made sense (e.g. when resetting the CAM). The ca_lock is replaced by a more general int_stat_lock, which ideally is held when banging on MANTIS_INT_STAT. (I have no hardware documentation, so I'm afraid I don't really know the specifics here.) - mantis_hif_write_wait() would never clear MANTIS_SBUF_OPDONE_BIT, leading to a lot of operations never actually waiting for the callback. I've added many such fixes, as well as debugging output when the bit is in a surprising state (e.g., MANTIS_SBUF_OPDONE_BIT set before the beginning of an operation, where it really should be cleared). - Some operations check for timeout by testing if wait_event_timeout() return -ERESTARTSYS. However, wait_event_timeout() can can never do this; the return value for timeout is zero. I've fixed this (well, I seemingly forgot one; have to do that in the next version :-) ). Unfortunately, this make the problems in the next point a _lot_ worse, since timeouts are now actually percolated up the stack. - As others have noticed, sometimes, especially during DMA transfers, the IRQ0 flag is never properly set and thus reads never return. (The typical case for this is when we've just done a write and the en50221 thread is waiting for the CAM status word to signal STATUSREG_DA; if this doesn't happen in a reasonable amount of time, the upstream libdvben50221.so will report errors back to mumudvb.) I have no idea why this happens more often on SMP systems than on UMP systems, but they really seem to do. I haven't found any reasonable workaround for reliable polling either, so I'm making a hack -- if there's nothing returned in two milliseconds, the read is simply assumed to have completed. This is an unfortunate hack, but in practice it's identical to the previous behavior except with a shorter timeout. - A hack to fix a mutex issue in the DVB layer; dvb_usercopy(), which is called on all ioctls, not only copies data to and from userspace, but also takes a lock on the file descriptor, which means that only one ioctl can run at a time. This means that if one thread of mumudvb is busy trying to get, say, the SNR from the frontend (which can hang due to the issue above), the CAM thread's ioctl(fd, CA_GET_SLOT_INFO, ...) will hang, even though it doesn't need to communicate with the hardware at all. This obviously requires a better fix, but I don't know the generic DVB layer well enough to say what it is. Maybe it's some sort of remnant of from when all ioctl()s took the BKL. Note that on UMP kernels without preemption, mutex_lock
[PATCH] Mantis and Hopper: Fix CAM hangup caused by losing GPIF status
Mantis and Hopper drivers: Fix CAM hangup problem,
where interrupt handler clears GPIF status, even though
GPIF interrupt isn't active.
Manuel reported that this patch fixes his problem:
http://www.spinics.net/lists/linux-media/msg41473.html
(CAM hangs up about once per 20 minutes, each hangup takes about 3-5s.)
Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi
diff --git a/drivers/media/dvb/mantis/hopper_cards.c
b/drivers/media/dvb/mantis/hopper_cards.c
index 71622f6..c2084e9 100644
--- a/drivers/media/dvb/mantis/hopper_cards.c
+++ b/drivers/media/dvb/mantis/hopper_cards.c
@@ -84,15 +84,6 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
if (!(stat mask))
return IRQ_NONE;
- rst_mask = MANTIS_GPIF_WRACK |
- MANTIS_GPIF_OTHERR |
- MANTIS_SBUF_WSTO |
- MANTIS_GPIF_EXTIRQ;
-
- rst_stat = mmread(MANTIS_GPIF_STATUS);
- rst_stat = rst_mask;
- mmwrite(rst_stat, MANTIS_GPIF_STATUS);
-
mantis-mantis_int_stat = stat;
mantis-mantis_int_mask = mask;
dprintk(MANTIS_DEBUG, 0, \n-- Stat=%02x Mask=%02x --, stat, mask);
@@ -101,6 +92,16 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
}
if (stat MANTIS_INT_IRQ0) {
dprintk(MANTIS_DEBUG, 0, %s, label[1]);
+
+ rst_mask = MANTIS_GPIF_WRACK |
+ MANTIS_GPIF_OTHERR |
+ MANTIS_SBUF_WSTO |
+ MANTIS_GPIF_EXTIRQ;
+
+ rst_stat = mmread(MANTIS_GPIF_STATUS);
+ rst_stat = rst_mask;
+ mmwrite(rst_stat, MANTIS_GPIF_STATUS);
+
mantis-gpif_status = rst_stat;
wake_up(ca-hif_write_wq);
schedule_work(ca-hif_evm_work);
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index c2bb90b..109a5fb 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -92,15 +92,6 @@ static irqreturn_t mantis_irq_handler(int irq, void *dev_id)
if (!(stat mask))
return IRQ_NONE;
- rst_mask = MANTIS_GPIF_WRACK |
- MANTIS_GPIF_OTHERR |
- MANTIS_SBUF_WSTO |
- MANTIS_GPIF_EXTIRQ;
-
- rst_stat = mmread(MANTIS_GPIF_STATUS);
- rst_stat = rst_mask;
- mmwrite(rst_stat, MANTIS_GPIF_STATUS);
-
mantis-mantis_int_stat = stat;
mantis-mantis_int_mask = mask;
dprintk(MANTIS_DEBUG, 0, \n-- Stat=%02x Mask=%02x --, stat, mask);
@@ -109,6 +100,15 @@ static irqreturn_t mantis_irq_handler(int irq, void
*dev_id)
}
if (stat MANTIS_INT_IRQ0) {
dprintk(MANTIS_DEBUG, 0, %s, label[1]);
+ rst_mask = MANTIS_GPIF_WRACK |
+ MANTIS_GPIF_OTHERR |
+ MANTIS_SBUF_WSTO |
+ MANTIS_GPIF_EXTIRQ;
+
+ rst_stat = mmread(MANTIS_GPIF_STATUS);
+ rst_stat = rst_mask;
+ mmwrite(rst_stat, MANTIS_GPIF_STATUS);
+
mantis-gpif_status = rst_stat;
wake_up(ca-hif_write_wq);
schedule_work(ca-hif_evm_work);
--
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
Re: Multiple Mantis devices gives me glitches
13.12.2011 20:11, Vidar Tyldum kirjoitti: 13.12.2011 08:31, Marko Ristola: Hi Here is a patch that went into Linus GIT this year. It reduces the number of DMA transfer interrupts into one third. Linus released 2.6.38.8 doesn't seem to have this patch yet Good news, combining this patch with IRQ management fixes the problem for me. Status: * Stock 2.6.38.8 mantis, no IRQ management: glitches * Stock 2.6.38.8 mantis, with IRQ management: glitches * Patched 2.6.38.8 mantis, no IRQ management: glitches (less) * Patched 2.6.38.8 mantis, with IRQ management: very few glitches * Same as above, but latency_timer set to 0xff: no glitches in one hour The patch was applied to 2.6.38.8 (in Ubuntu terms: 2.6.38-13-generic-pae). Tests involved having VDR record on three different transponders at the same time, which means lots of IO and all three cards active at once. For IRQ management I tried both 'irqbalancer' and manual setting with IRQ affinity (which is basicly what irqbalancer does). I tried playing with the latency timer on the other scenarios, not only the last one, but all had glitches anyways. Not sure what to conclude with: - Unfortunate IRQ handling on this CPU (two and two share IRQ handling)? - Too many interrupts generated by driver (design issues)? - Driver not handling SMP gracefully? ...or a combination? Your patch is definately needed. Is there anything I can do to help getting it in? Here is a reference for Linu's master GIT tree for the patch that went in: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=79d06d4dff733ee472e1f8933a33317a18195c0c It will be in Linux 3.2 kernel release. Ubuntu will take it in some day when they grab a kernel with number at least 3.2. If you describe your problem into Ubuntu bug report system with There is a fix already accepted upstream. Here is a reference for the patch., there is a chance that you get it backported too. I tested the patch originally very thoroughly. It improves things, but doesn't break any. VDR wants data from the DVB card in large chunks to avoid unnecessary CPU context switching. DVB 16KB patch is designed so, that DVB card has always some data to be delivered for VDR, but at the same time it avoids doing unnecessary many interrupts. Here is another patch that I wrote for my DVB HDTV network delivery glitches problem. It might help you a bit also: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=38e009aac9e02d2c30fd9a5e979ab31433e7d578 It tries to copy DMA buffer data as fast as possible into DVB-CORE's internal 128K buffer. Thus it prevents from Mantis DMA 64K buffer from being overrun. It helps lots if the original dvb_dmx_swfilter(_204) is very slow, which I saw with perf top tool. Later I detected that it helps with some computers a lot, and with some other computers the patch doesn't help so much. That patch doesn't guarantee glitch free data delivery either. Next question is, which buffer overruns now? Mantis 64K DMA buffer? DVB-CORE's 128K buffer? VDR reads from DVB-CORE's 128K buffer. dvb_dmx_swfilter(_204) in media/dvb-core/dvb_demux.c is designed to drop data if DVB-CORE's 128K buffer becomes full. Maybe we could add into mantis_dma.c some logic: Mantis DMA transfer delivers data into DVB-CORE's 128K buffer in 16K chunks, avoiding the 128K buffer overrun. If DVB-CORE's buffer would overrun, Mantis DMA transfer sends data, if also Mantis 64K DMA buffer is becoming full. I suspect I2C latency problem to be a root cause for these glitches, but there isn't a patch to test it yet. I2C latency should not be a problem with quad core CPU: it should affect most with a single CPU computer. perf top shows easilly some CPU hogs. Regards, Marko Ristola -- 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
Re: Mantis CAM not SMP safe / Activating CAM on Technisat Skystar HD2 (DVB-S2)
On 12/10/2011 01:57 AM, Ninja wrote:
Hi,
has anyone an idea how the SMP problems could be fixed?
You could turn on Mantis Kernel module's debug messages.
It could tell you the emitted interrupts.
One risky thing with the Interrupt handler code is that
MANTIS_GPIF_STATUS is cleared, even though IRQ0 isn't active yet.
This could lead to a rare starvation of the wait queue you described.
I supplied a patch below. Does it help?
I did some further investigation. When comparing the number of interrupts with
all cores enabled and the interrupts with only one core enabled it seems like
only the IRQ0 changed, the other IRQs and the total number stays quite the same:
4 Cores:
All IRQ/sec: 493
Masked IRQ/sec: 400
Unknown IRQ/sec: 0
DMA/sec: 400
IRQ-0/sec: 143
IRQ-1/sec: 0
OCERR/sec: 0
PABRT/sec: 0
RIPRR/sec: 0
PPERR/sec: 0
FTRGT/sec: 0
RISCI/sec: 258
RACK/sec: 0
1 Core:
All IRQ/sec: 518
Masked IRQ/sec: 504
Unknown IRQ/sec: 0
DMA/sec: 504
IRQ-0/sec: 246
IRQ-1/sec: 0
OCERR/sec: 0
PABRT/sec: 0
RIPRR/sec: 0
PPERR/sec: 0
FTRGT/sec: 0
RISCI/sec: 258
RACK/sec: 0
So, where might be the problem?
Turning on Mantis debug messages, might tell the difference between these
interrupts.
I hope somebody can help, because I think we are very close to a fully
functional CAM here.
I ran out of things to test to get closer to the solution :(
Btw: Is there any documentation available for the mantis PCI bridge?
Not that I know.
Manuel
--
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
Regards,
Marko Ristola
--- PATCH --
Mantis/Hopper: Check and clear GPIF status bits only when IRQ0 bit is active.
Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi
diff --git a/drivers/media/dvb/mantis/hopper_cards.c
b/drivers/media/dvb/mantis/hopper_cards.c
index 71622f6..c2084e9 100644
--- a/drivers/media/dvb/mantis/hopper_cards.c
+++ b/drivers/media/dvb/mantis/hopper_cards.c
@@ -84,15 +84,6 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
if (!(stat mask))
return IRQ_NONE;
- rst_mask = MANTIS_GPIF_WRACK |
- MANTIS_GPIF_OTHERR |
- MANTIS_SBUF_WSTO |
- MANTIS_GPIF_EXTIRQ;
-
- rst_stat = mmread(MANTIS_GPIF_STATUS);
- rst_stat = rst_mask;
- mmwrite(rst_stat, MANTIS_GPIF_STATUS);
-
mantis-mantis_int_stat = stat;
mantis-mantis_int_mask = mask;
dprintk(MANTIS_DEBUG, 0, \n-- Stat=%02x Mask=%02x --, stat, mask);
@@ -101,6 +92,16 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
}
if (stat MANTIS_INT_IRQ0) {
dprintk(MANTIS_DEBUG, 0, %s, label[1]);
+
+ rst_mask = MANTIS_GPIF_WRACK |
+ MANTIS_GPIF_OTHERR |
+ MANTIS_SBUF_WSTO |
+ MANTIS_GPIF_EXTIRQ;
+
+ rst_stat = mmread(MANTIS_GPIF_STATUS);
+ rst_stat = rst_mask;
+ mmwrite(rst_stat, MANTIS_GPIF_STATUS);
+
mantis-gpif_status = rst_stat;
wake_up(ca-hif_write_wq);
schedule_work(ca-hif_evm_work);
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index c2bb90b..109a5fb 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -92,15 +92,6 @@ static irqreturn_t mantis_irq_handler(int irq, void *dev_id)
if (!(stat mask))
return IRQ_NONE;
- rst_mask = MANTIS_GPIF_WRACK |
- MANTIS_GPIF_OTHERR |
- MANTIS_SBUF_WSTO |
- MANTIS_GPIF_EXTIRQ;
-
- rst_stat = mmread(MANTIS_GPIF_STATUS);
- rst_stat = rst_mask;
- mmwrite(rst_stat, MANTIS_GPIF_STATUS);
-
mantis-mantis_int_stat = stat;
mantis-mantis_int_mask = mask;
dprintk(MANTIS_DEBUG, 0, \n-- Stat=%02x Mask=%02x --, stat, mask);
@@ -109,6 +100,15 @@ static irqreturn_t mantis_irq_handler(int irq, void
*dev_id)
}
if (stat MANTIS_INT_IRQ0) {
dprintk(MANTIS_DEBUG, 0, %s, label[1]);
+ rst_mask = MANTIS_GPIF_WRACK |
+ MANTIS_GPIF_OTHERR |
+ MANTIS_SBUF_WSTO |
+ MANTIS_GPIF_EXTIRQ;
+
+ rst_stat = mmread(MANTIS_GPIF_STATUS);
+ rst_stat = rst_mask;
+ mmwrite(rst_stat, MANTIS_GPIF_STATUS);
+
mantis-gpif_status = rst_stat;
wake_up(ca-hif_write_wq);
schedule_work(ca-hif_evm_work);
--
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
Re: Multiple Mantis devices gives me glitches
Hi
Here is a patch that went into Linus GIT this year.
It reduces the number of DMA transfer interrupts into one third.
Linus released 2.6.38.8 doesn't seem to have this patch yet.
I had a single Mantis card and a single CPU.
I used VDR to deliver HDTV channel via network
into a faster computer for viewing.
One known latency point with Mantis is I2C transfer.
With my measure, latency is about 0.33ms per I2C transfer byte.
Basic read / write (address+value) minimal latency is thus 0.66ms.
Latency amount depends on I2C bus speed on the card.
I don't have a working patch to fix this yet though.
Regards, Marko Ristola
---
Refactor Mantis DMA transfer to deliver 16Kb TS data per interrupt
https://patchwork.kernel.org/patch/107036/
With VDR streaming HDTV into network, generating an interrupt once per 16kb,
implemented in this patch, seems to support more robust throughput with HDTV.
Fix leaking almost 64kb data from the previous TS after changing the TS.
One effect of the old version was, that the DMA transfer and driver's
DMA buffer access might happen at the same time - a race condition.
Signed-off-by: Marko M. Ristola marko.rist...@kolumbus.fi
Regards,
Marko Ristola
diff --git a/drivers/media/dvb/mantis/hopper_cards.c
b/drivers/media/dvb/mantis/hopper_cards.c
index 09e9fc7..3b7e376 100644
--- a/drivers/media/dvb/mantis/hopper_cards.c
+++ b/drivers/media/dvb/mantis/hopper_cards.c
@@ -126,7 +126,7 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
}
if (stat MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, %s, label[8]);
- mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
+ mantis-busy_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
if (stat MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index cf4b39f..8f048d5 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -134,7 +134,7 @@ static irqreturn_t mantis_irq_handler(int irq, void *dev_id)
}
if (stat MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, %s, label[8]);
- mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
+ mantis-busy_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
if (stat MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_common.h
b/drivers/media/dvb/mantis/mantis_common.h
index d0b645a..23b23b7 100644
--- a/drivers/media/dvb/mantis/mantis_common.h
+++ b/drivers/media/dvb/mantis/mantis_common.h
@@ -122,11 +122,8 @@ struct mantis_pci {
unsigned intnum;
/* RISC Core */
- u32 finished_block;
+ u32 busy_block;
u32 last_block;
- u32 line_bytes;
- u32 line_count;
- u32 risc_pos;
u8 *buf_cpu;
dma_addr_t buf_dma;
u32 *risc_cpu;
diff --git a/drivers/media/dvb/mantis/mantis_dma.c
b/drivers/media/dvb/mantis/mantis_dma.c
index 46202a4..c61ca7d 100644
--- a/drivers/media/dvb/mantis/mantis_dma.c
+++ b/drivers/media/dvb/mantis/mantis_dma.c
@@ -43,13 +43,17 @@
#define RISC_IRQ (0x01 24)
#define RISC_STATUS(status) ~status) 0x0f) 20) | ((status 0x0f) 16))
-#define RISC_FLUSH() (mantis-risc_pos = 0)
-#define RISC_INSTR(opcode) (mantis-risc_cpu[mantis-risc_pos++] =
cpu_to_le32(opcode))
+#define RISC_FLUSH(risc_pos) (risc_pos = 0)
+#define RISC_INSTR(risc_pos, opcode) (mantis-risc_cpu[risc_pos++] =
cpu_to_le32(opcode))
#define MANTIS_BUF_SIZE (64 * 1024)
-#define MANTIS_BLOCK_BYTES (MANTIS_BUF_SIZE 4)
-#define MANTIS_BLOCK_COUNT (1 4)
-#define MANTIS_RISC_SIZE PAGE_SIZE
+#define MANTIS_BLOCK_BYTES (MANTIS_BUF_SIZE / 4)
+#define MANTIS_DMA_TR_BYTES (2 * 1024) /* upper limit: 4095 bytes. */
+#define MANTIS_BLOCK_COUNT (MANTIS_BUF_SIZE / MANTIS_BLOCK_BYTES)
+
+#define MANTIS_DMA_TR_UNITS (MANTIS_BLOCK_BYTES / MANTIS_DMA_TR_BYTES)
+/* MANTIS_BUF_SIZE / MANTIS_DMA_TR_UNITS must not exceed MANTIS_RISC_SIZE (4k
RISC cmd buffer) */
+#define MANTIS_RISC_SIZE PAGE_SIZE /* RISC program must fit here. */
int mantis_dma_exit(struct mantis_pci *mantis)
{
@@ -124,27 +128,6 @@ err:
return -ENOMEM;
}
-static inline int mantis_calc_lines(struct mantis_pci *mantis)
-{
- mantis-line_bytes = MANTIS_BLOCK_BYTES;
- mantis-line_count = MANTIS_BLOCK_COUNT;
-
- while (mantis-line_bytes 4095) {
- mantis-line_bytes = 1;
- mantis-line_count = 1;
- }
-
- dprintk(MANTIS_DEBUG, 1, Mantis
Re: USB mini-summit at LinuxCon Vancouver
Hi I've been thinking about the Kernel driver side. Mauro and others emailed requirements on Jun or July. I'm sorry for this spam: maybe you have thought this already. A linked list of read/write locks as the solution for these protections could be a base for the general solution. Locks could be accessed either by name string name or by an integer identifier. The bridge driver would be the container of the lock list. Lock take / release handles would be changeable by Bridge driver at driver init. This way bridge could tune the lock taking actions. Sub devices would not have to know the details of the bridge device. When implemented as a library .ko module, this could be used by all related kernel drivers. The locking code would be very general. Maybe adding a lock list into each PCI bus device would solve the device export problem to KVM too. If some module wouldn't handle the proper locking yet, it would not deliver the protection, but it would work as before: no regressions. The library could also be called so that a driver would ask three locks at the same time. If the driver would get all three locks, it would return success. If the driver would not get all three locks, it would not lock any of them (with _trylock case). I don't have time to implement this feature. Happy meeting for all of you, Marko Ristola -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
Hi. IMO, your thoughts about core resource locking mechanism sound good. I say here some thoughts and examples how the resource locking mechanism might work. IMHO, It's good enough now if we are heading to a solution. At least I would not alone find a proper concept. 07.07.2011 18:14, Mauro Carvalho Chehab kirjoitti: Em 06-07-2011 16:59, Marko Ristola escreveu: 06.07.2011 21:17, Devin Heitmueller kirjoitti: On Wed, Jul 6, 2011 at 1:53 PM, Marko Ristola marko.rist...@kolumbus.fi wrote: IMO, the proper solution is to provide a core resource locking mechanism, that will keep track about what device resources are in usage (tuner, analog audio/video demods, digital demods, sec, radio reception, i2c buses, etc), and some mechanisms like the above that will power the subdevice off when it is not being used for a given amount of time (a Kconfig option can be added so set the time to X minutes or to disable such mechanism, in order to preserve back compatibility). One special case for the locking mechanisms that may or may not be covered by such core resource locking is the access to the I2C buses. Currently, the DVB, V4L and remote controller stacks may try to use resources behind I2C at the same time. The I2C core has a locking schema, but it works only if a transaction is atomically commanded. So, if it requires multiple I2C transfers, all of them need to be grouped into one i2c xfer call. Complex operations like firmware transfers are protected by the I2C locking, so one driver might be generating RC polling events while a firmware is being transferring, causing it to fail. A generic locking schema could have shared/exclusive locks by name (device name, pointer ?). A generic resource set of named locks could contain these locks. Firmware load would take an exclusive lock on I2C master for the whole transfer. RC polling would take a shared lock on I2C master and an exclusive lock on RC UART device. Or if there is no need to share I2C device, RC polling would just take exclusive lock on I2C Master. If I2C bus must be quiesced for 10ms after frontend's tuning action, I2C master exclusive lock could be held 10ms after last I2C transfer. i2c/bridge1 state should be protected if the frontend is behind an I2C bridge. The existing I2C xfer mutex would be as it is now: it is a lower level lock, no regressions to come. Taking a shared lock of tuner_power_switch would mark that the device must not be turned off. While holding the shared lock, no deferred watch would be needed. While releasing tuner_power_switch lock, usage timestamp on that name should be updated. If there are no more tuner_power_switch holders, delayed task could be activated and run after 3 minutes to power the device off if needed. Bridge drivers that don't have full runtime power saving support, would not introduce a callback which a delayed task would run to turn power off / on. Regards, Mauro IMO, suspend / resume code must be a separate thing. We might want to suspend the laptop while watching a DVB channel. We don't want to have runtime power management active while watching a DVB channel. Suspend quiesces the device possibly in one phase. It needs to have the device in a good state before suspending: take an exclusive I2C Master lock before going to suspend. While resuming, release I2C Master lock. So even though these details are incomplete, suspend/resume could perhaps be integrated with the generic advanced locking scheme. Regards, Marko -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
Hi. I think that you could reuse lots of code with smart suspend / resume. What do you think about this DVB power saving case (about the concept, don't look at details, please): - One device has responsibility to do the power off when it can be done (mantis_core.ko) - In my case there is only one frontend tda10021.ko to take care of. - dvb_frontend.c would call fe-sleep(fe). The callback goes into mantis_core.ko. - mantis_core.ko will traverse all devices on it's responsibility, all (tda10021.ko) are idle. = suspend tda10021.ko by calling tda10021-sleep() and do additional power off things. - When dvb_frontend.c wants tuner to be woken up, mantis_core.ko does hardware resets and power on first and then resumes tda10021-init(fe). I implemented something that worked a few years ago with suspend / resume. In mantis_dvb.c's driver probe function I modified tuner_ops to enable these runtime powersaving features: + mantis-fe-ops.tuner_ops.sleep = mantis_dvb_tuner_sleep; + mantis-fe-ops.tuner_ops.init = mantis_dvb_tuner_init; That way mantis_core.ko had all needed details to do any advanced power savings I wanted. Suspend / resume worked well: During resume there was only a glitch at the picture and sound and the TV channel watching continued. tda10021 (was cu1216 at that time) restored the original TV channel. It took DVB FE_LOCK during resume. Suspended DMA transfer was recovered before returning into userspace. So I think that you need a single device (mantis_core.ko) that can see the whole picture, in what states the subdevices are: can you touch the power button?. With DVB this is easy because dvb_frontend.c tells when a frontend is idle and when it is not. The similar idea of some kind of watchdog that is able to track when a single device (frontend) is used and when it is not used, would be sufficient. The topmost driver (mantis_core.ko in my case) would then be responsible to track multiple frontends (subdevices), if they all are idle or not, with suitable mutex protection. Then this driver could easilly suspend/resume these subdevices and press power switch when necessary. So the clash between DVB and V4L devices would be solved: Both DVB and V4L calls a (different) sleep() function on mantis_core.ko mantis_core.ko will turn power off when both frontends are sleeping. If only one sleeps, the one can be put to sleep or suspended, but power button can't be touched. What do you think? I did this easy case mantis_core.ko solution in about Summer 2007. It needs a rewrite and testing, if I take it from the dust. Regards, Marko Ristola 16.06.2011 15:51, Devin Heitmueller wrote: On Thu, Jun 16, 2011 at 7:14 AM, Mauro Carvalho Chehab mche...@redhat.com wrote: One possible logic that would solve the scripting would be to use a watchdog to monitor ioctl activities. If not used for a while, it could send a s_power to put the device to sleep, but this may not solve all our problems. So, I agree with Devin: we need to add an option to explicitly control the power management logic of the device, having 3 modes of operation: POWER_AUTO - use the watchdogs to poweroff POWER_ON - explicitly powers on whatever subdevices are needed in order to make the V4L ready to stream; POWER_OFF - Put all subdevices to power-off if they support it. After implementing such logic, and keeping the default as POWER_ON, we may announce that the default will change to POWER_AUTO, and give some time for userspace apps/scripts that need to use a different mode to change their behaviour. That means that, for example, radio -qf will need to change to POWER_ON mode, and radio -m should call POWER_OFF. I've considered this idea before, and it's not bad in theory. The one thing you will definitely have to watch out for is causing a race between DVB and V4L for hybrid tuners. In other words, you can have a user switching from analog to digital and you don't want the tuner to get powered down a few seconds after they started streaming video from DVB. Any such solution would have to take the above into account. We've got a history of race conditions like this and I definitely don't want to see a new one introduced. Devin -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
06.07.2011 21:17, Devin Heitmueller kirjoitti: On Wed, Jul 6, 2011 at 1:53 PM, Marko Ristola marko.rist...@kolumbus.fi wrote: All that said, I believe that you are correct in that the business logic needs to ultimately be decided by the bridge driver, rather than having the dvb/tuner core blindly calling the sleep routines against the tuner and demod drivers without a full understanding of what impact it has on the board as a whole. You wrote it nicely and compactly. What do you think about tracking coarse last busy time rather than figuring out accurate idle time? dvb_frontend.c and V4L side would just poll the device: bridge-wake(). wake() will just store current busy timestamp to the bridge device with coarse accuracy, if subdevices are already at active state. If subdevices are powered off, it will first power them on and resume them, and then store busy timestamp. Bridge device would have a delayed task: Check after 3 minutes: If I haven't been busy for three minutes, I'll go to sleep. I'll suspend the subdevices and power them off. The delayed task would refresh itself: check again after last awake time + 3 minutes. Delayed task could be further developed to support multiple suspend states. Cheers, Devin Marko -- 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
Re: [PATCH] Refactor Mantis DMA transfer to deliver 16Kb TS data per interrupt
Hi Mauro and Manu I think that the patch is robust enough to be accepted. Please Mauro accept this patch at appropriate time. Would you Manu, please, read my lengthy detailed description of this patch when you have time? Then you know what it does and why. I provided a descriptive table of the garbage generating problem, if you want to have a quick look. Regards, Marko Ristola When I wrote this patch, I took lots of time to understand the RISC processor's commands from Manu's existing code and from Manu's old Internet references found by Google. I have verified thoroughly that the code is correct. It doesn't do any new assumptions on the RISC processor or DMA transfer in addition to Manu's existing assumptions. Users know by experience that this patch work. Why this increases performance so much? Mantis I2C transfer (and thus Hopper too) does busy looping, reserving the CPU for the entire I2C transfer time (under 1ms, big single latency point). So having a faster CPU doesn't solve this latency problem, but having more than one CPU helps. Easiest way to reduce these IRQ activated latencies is to generate the IRQ less often. Reduce interrupts by emitting IRQ less often I wanted to reduce IRQs into one third (one per 16K instead of one per 4K). So instead of RISC processor doing two 2048 byte DMA transfers per IRQ I do 16384 / 2048 = 8 DMA transfers and then emit IRQ. The RISC processor can do DMA transfers up to 4095 bytes per RISC instruction. That's why Manu's code does single DMA transfer per 2048 bytes and emits IRQ on every second instruction. DMA transfer garbage problem DMA transfer garbage problem is another thing that this patch fixes. When Mantis/Hopper tunes into a TS, first DMA transfer will deliver almost the whole 64K DMA buffer into dvb_dmx_swfilter(_204). This has caused application crashes and delays on tuning into the channel and occurrences of wrong audio/video on application. Over these years application robustness has improved. Here is original buggy code's DMA transfer variables as a table: Table: Garbage generating block iteration without my patch linebuf_pos finished_block_by_IRQ original_last_block last_block_after_driver_processing 0 0 15 015 ( driver processes garbage 4K blocks indexed 0 - 14). 1 2048 2 40960 15 0 ( driver processes garbage 4K block at index 15). 3 6144 4 81921 01 (first valid 4K data block at index 0). snip - line goes from 0 to 31: one RISC CPU DMA transfer instruction per line. - buf_postracks pointer where RISC CPU will copy 2048 bytes with the DMA transfer. - finished_block_by_IRQ RISC CPU reports via IRQ: I have finished block 15, driver can process it. - original_last_blockDriver will copy this 4K block first, until just before finished_block_by_IRQ. - last_block_after_driver_processing The value of last_block after driver has processed 4K DMA buffer blocks. I fix this by thinking it in another way: RISC CPU doesn't report what is the previous acceptable block. It reports: I'm writing into block XXX. Don't touch it! The fix is here: + RISC_INSTR(risc_pos, RISC_WRITE | +RISC_IRQ | +RISC_STATUS(line) | +MANTIS_DMA_TR_BYTES); RISC CPU informs Mantis/Hopper driver that 16K buffer at line will be overwritten. Table: Working block iteration after my patch linestepbuf_pos busy_block original_last_block last_block_after_driver_processing 0 0 0 0 0 0 (kernel driver wakened, block 0 is busy, do nothing) 0 1 2048 snip 0 6 12288 0 7 14336 1 0 16384 1 0 1 (kernel driver writes valid 16K block at index 0, block 1 is busy) 1 1 18432 snip 1 7 30720 2 0 32768 2 1 2 (kernel driver writes valid 16K block at index 1, block 2 is busy) 2 1 34816 snip - line goes from 0 to 3. It counts 16K blocks. - step goes from 0 to 8. It counts 2K blocks within a 16K block. - buf_pos goes from 0 to under 64K, in 2K byte steps. - busy_blockRISC IRQ reports via IRQ: I will write to block X. Don't touch it. (index is between 0 and 3). - original_last_block Driver will copy this 16K block first, until the block before busy block
Re: AW: Remote control not working for Terratec Cinergy C (2.6.37 Mantis driver)
I noticed that too on the C code: If keypress comes from the remote control, driver does both push down and release immediately. Some years ago I made a version that did something like this: I measured that a remote control sends key pressed in about 20ms cycles. Thus I decided that the driver can do following: Whe key '1' is pressed initially, send key 1 pressed to input layer. If within 30ms a '1 pressed' comes from the remote control, driver keeps '1' as pressed (do nothing for input layer). If there won't come a '1 pressed' from remote within 30ms, then driver sends key 1 unpressed to input layer. I don't know if there is any reusable algorithm (easilly usable code) for remote control drivers for this. Regards, Marko Ristola 21.05.2011 10:23, Adrian C. kirjoitti: Haven't noticed earlier that every button press is executed twice, until I did some testing with Oxine. Not sure how much Lirc is to blame for this, and for button 0 not working. I will move to the Lirc list. Thanks again for the patch. -- 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
Re: Remote control not working for Terratec Cinergy C (2.6.37 Mantis driver)
04.05.2011 01:42, Adrian C. kirjoitti: On Wed, 2 Mar 2011, Marko Ristola wrote: So this means, that my remote control works, pressing key with hex value 0x26 works. It works. Unfortunately mantis_uart.c doesn't have IR input initialization at all But it does not work. How can it work and not work at the same time? The hardware device is active (it is enabled, messages are sent from the remote to the Kernel Mantis software driver. The bytes can be logged into /var/log/messages file. That's all the driver is designed to do at this point. I have the Skystar HD2 (b), subsystem: 1ae4:0003 now, same chipset as Cinergy S2 and some others, VP-1041. Lirc failed with my old COM receiver so I tried to use the cards IR as fall-back, and of course I failed again. This was on 2.6.38.4. Only information I found is 1 year old[1]. IR was in flux but it still doesn't work even though mantis pulls in all those ir-* modules, no input device is created. Can someone fill us in, please, will it be supported this year? Would you please ask from Manu Abraham. Maybe he gets paid for doing it. I have experimented with my remote control too a few years ago, but I think it is good if Manu gets it as a job. Regards, Marko Ristola 1. http://www.mail-archive.com/linux-media@vger.kernel.org/msg14641.html -- 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 -- 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
[PATCH] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer
Avoid unnecessary DVB TS 188 sized packet copying from DMA buffer into stack.
Backtrack one 188 sized packet just after some garbage bytes when possible.
This obsoletes patch https://patchwork.kernel.org/patch/118147/
Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c
b/drivers/media/dvb/dvb-core/dvb_demux.c
index 4a88a3e..faa3671 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -478,97 +478,94 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux,
const u8 *buf,
EXPORT_SYMBOL(dvb_dmx_swfilter_packets);
-void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
+static inline int find_next_packet(const u8 *buf, int pos, size_t count,
+ const int pktsize)
{
- int p = 0, i, j;
+ int start = pos, lost;
- spin_lock(demux-lock);
-
- if (demux-tsbufp) {
- i = demux-tsbufp;
- j = 188 - i;
- if (count j) {
- memcpy(demux-tsbuf[i], buf, count);
- demux-tsbufp += count;
- goto bailout;
- }
- memcpy(demux-tsbuf[i], buf, j);
- if (demux-tsbuf[0] == 0x47)
- dvb_dmx_swfilter_packet(demux, demux-tsbuf);
- demux-tsbufp = 0;
- p += j;
+ while (pos count) {
+ if (buf[pos] == 0x47 ||
+ (pktsize == 204 buf[pos] == 0xB8))
+ break;
+ pos++;
}
- while (p count) {
- if (buf[p] == 0x47) {
- if (count - p = 188) {
- dvb_dmx_swfilter_packet(demux, buf[p]);
- p += 188;
- } else {
- i = count - p;
- memcpy(demux-tsbuf, buf[p], i);
- demux-tsbufp = i;
- goto bailout;
- }
- } else
- p++;
+ lost = pos - start;
+ if (lost) {
+ /* This garbage is part of a valid packet? */
+ int backtrack = pos - pktsize;
+ if (backtrack = 0 (buf[backtrack] == 0x47 ||
+ (pktsize == 204 buf[backtrack] == 0xB8)))
+ return backtrack;
}
-bailout:
- spin_unlock(demux-lock);
+ return pos;
}
-EXPORT_SYMBOL(dvb_dmx_swfilter);
-
-void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf, size_t count)
+/* Filter all pktsize= 188 or 204 sized packets and skip garbage. */
+static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf,
+ size_t count, const int pktsize)
{
int p = 0, i, j;
- u8 tmppack[188];
+ const u8 *q;
spin_lock(demux-lock);
- if (demux-tsbufp) {
+ if (demux-tsbufp) { /* tsbuf[0] is now 0x47. */
i = demux-tsbufp;
- j = 204 - i;
+ j = pktsize - i;
if (count j) {
memcpy(demux-tsbuf[i], buf, count);
demux-tsbufp += count;
goto bailout;
}
memcpy(demux-tsbuf[i], buf, j);
- if ((demux-tsbuf[0] == 0x47) || (demux-tsbuf[0] == 0xB8)) {
- memcpy(tmppack, demux-tsbuf, 188);
- if (tmppack[0] == 0xB8)
- tmppack[0] = 0x47;
- dvb_dmx_swfilter_packet(demux, tmppack);
- }
+ if (demux-tsbuf[0] == 0x47) /* double check */
+ dvb_dmx_swfilter_packet(demux, demux-tsbuf);
demux-tsbufp = 0;
p += j;
}
- while (p count) {
- if ((buf[p] == 0x47) || (buf[p] == 0xB8)) {
- if (count - p = 204) {
- memcpy(tmppack, buf[p], 188);
- if (tmppack[0] == 0xB8)
- tmppack[0] = 0x47;
- dvb_dmx_swfilter_packet(demux, tmppack);
- p += 204;
- } else {
- i = count - p;
- memcpy(demux-tsbuf, buf[p], i);
- demux-tsbufp = i;
- goto bailout;
- }
- } else {
- p++;
+ while (1) {
+ p = find_next_packet(buf, p, count, pktsize);
+ if (p = count)
+ break;
+ if (count - p pktsize)
+ break;
+
+ q = buf[p];
+
+ if (pktsize == 204 (*q == 0xB8
Re: [PATCH] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer
Here is some statistics without likely and with likely functions. It seems that using likely() gives better performance with Phenom I too. % Plain knl % No likely % With likely % dvb_ringbuffer_write5,9 62,88,7 81,35,7 79,2 dvb_dmx_swfilter_packet 1,2 12,80,7 6,5 0,8 11,1 dvb_dmx_swfilter_2042,3 24,51,3 12,10,7 9,7 Here Plain knl % is perf top -d 30 percentage. 24,5 12,1 and 9,7 are percentages without a patch, with basic patch and last is with likely functions using patch. Regards, Marko Ristola 08.04.2011 18:40, Marko Ristola kirjoitti: Avoid unnecessary DVB TS 188 sized packet copying from DMA buffer into stack. Backtrack one 188 sized packet just after some garbage bytes when possible. This obsoletes patch https://patchwork.kernel.org/patch/118147/ Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c b/drivers/media/dvb/dvb-core/dvb_demux.c index 4a88a3e..faa3671 100644 --- a/drivers/media/dvb/dvb-core/dvb_demux.c +++ b/drivers/media/dvb/dvb-core/dvb_demux.c @@ -478,97 +478,94 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux, const u8 *buf, EXPORT_SYMBOL(dvb_dmx_swfilter_packets); -- 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
Re: Pending dvb_dmx_swfilter(_204)() patch tested enough
Hello Andreas and Mauro
Thank you for looking at the patch.
Than you Mauro for giving the likely() unlikely() GCC assembly example
and remind of scripts/checkpatch.pl.
28.03.2011 15:07, Andreas Oberritter wrote:
Hello Marko,
On 03/26/2011 09:20 PM, Marko Ristola wrote:
Following patch has been tested enough since last Summer 2010:
Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function
https://patchwork.kernel.org/patch/118147/
It modifies both dvb_dmx_swfilter_204() and dvb_dmx_swfilter() functions.
sorry, I didn't know about your patch. Can you please resubmit it with
the following changes?
- Don't use camelCase (findNextPacket)
I'll rename it as find_next_packet().
- Remove disabled printk() calls.
I'll do that.
- Only one statement per line.
if (unlikely(lost = pos - start)) {
while (likely((p = findNextPacket(buf, p, count, pktsize)) count)) {
I'll alter while() line as:
while (1) {
p = find_next_packet(buf, p, count, pktsize);
if (p = count)
break;
if (count - p pktsize)
break;
- Add white space between while and the opening brace.
while(likely(pos count)) {
Thanks.
- Use unsigned data types for pos and pktsize:
static inline int findNextPacket(const u8 *buf, int pos, size_t count,
const int pktsize)
I'll change them.
The CodingStyle[1] document can serve as a guideline on how to properly
format kernel code.
Thanks. I have read it, but reading again is always good for learning.
Does the excessive use of likely() and unlikely() really improve the
performance or is it just a guess?
I'll try to measure performance difference next weekend: I haven't tested the
effect
on likely/unlikely operations. I have tested the effect of the whole patch only.
I selected likely and unlikely() sentences very carefully, so they should be
correct.
I'm not sure if recovering a packet by backtracking is worth it on my patch:
If recovered packets are typically discarded by dvb_dmx_swfilter_packet()
later, I'll drop the code.
Here is a description of the problem as I saw it last Summer:
My DVB card seemed to lose a few packets somewhere in the DMA transfer buffer
and then had a short packet
(less than 204 garbage bytes) and then normal good packets.
Backtracking use case (deliver 16K bytes of data for dvb_dmx_swfilter_204() per
call):
1. DVB card loses a few packets.
2. DVB card delivers less than 204 bytes garbage, containing packet start byte
in the middle.
3. dvb_dmx_swfilter_204() finds start byte from garbage. Deliver 204 sized
garbage packet into dvb_dmx_swfilter_packet().
4. dvb_dmx_swfilter_204() detects that packet at (3) was garbage. One good
packet can be restored by buffer backtracking.
5. dvb_dmx_swfilter_204() delivers restored packet into
dvb_dmx_swfilter_packet().
6. dvb_dmx_swfilter_204() continues to deliver 204 sized packets into
dvb_dmx_swfilter_packet().
I suspect that on (5), the restored packet is discarded by
dvb_dmx_swfilter_packet() because of
packet counter sequencing error. There is no benefit with recovering the packet
in this (typical) case.
dvb_dmx_swfilter_packet() will discard packets until it finds a next group of
packets with group
start identifier. Is this correct?
Regards,
Marko Ristola
Regards,
Andreas
[1]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/CodingStyle
--
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
--
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
Pending dvb_dmx_swfilter(_204)() patch tested enough
Hi Mauro. Following patch has been tested enough since last Summer 2010: Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function https://patchwork.kernel.org/patch/118147/ It modifies both dvb_dmx_swfilter_204() and dvb_dmx_swfilter() functions. I Myself tested dvb_dmx_swfilter_204() with terrestrial DVB-C, using 204 sized packets. Some other Mantis users have satellite receiver equipment, using 188 sized packets, thus using dvb_dmx_swfilter(). The patch has been in yaVDR/testing branch and used there. So both functions are tested enough during one half year period. I was asked by a person this week that if the patch could go forward. Regards, Marko Ristola -- 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
Re: Remote control not working for Terratec Cinergy C (2.6.37 Mantis driver)
28.02.2011 19:26, Jonas Hanschke kirjoitti: Hi, despite lots of time spent tinkering around and looking for help on the web, I've had no success in getting to work the remote control of my DVB-C card. It is a Terratec Cinergy C: http://linuxtv.org/wiki/index.php/TerraTec_Cinergy_C_DVB-C and am using the Mantis driver. Since it was merged into the kernel tree in 2.6.33, watching TV works without patches, but the remote control does not, although it is supposed to be supported, according to the link above. Kernel is a vanilla 2.6.37.2 with custom configuration on an old AMD Athlon XP machine, running debian Squeeze. When I modprobe the Mantis driver, the following IR-modules are pulled in automagically: ir_lirc_codec lirc_dev ir_core However, no input device is created during module loading. dmesg output: Mantis :01:0a.0: PCI INT A - Link[APC1] - GSI 16 (level, high) - IRQ 16 DVB: registering new adapter (Mantis DVB adapter) IR LIRC bridge handler initialized DVB: registering adapter 0 frontend 0 (Philips TDA10023 DVB-C)... Am I missing some additional modules? Are there any dependencies on other kernel config options that are not handled automatically by make menuconf? If additional information is needed, I will be happy to provide it. However, I am not sure what is useful and what is not and did not want to bloat this message. Before merging into v4l-dvb, doing modprobe mantis was enough. I don't know how it should work with recent kernels. I haven't seen remote control working lately. Turning mantis module debug options on gives some information what is happening into /var/log/messages. Regards, Marko Ristola Thanks in advance, Jonas -- 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 -- 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
Re: Summary of the pending patches up to Dec, 31 (26 patches)
Dear Developers, Happy New Year. I'll try to describe my patches so that they would be more understandable. First comment is for all dvb_dmx_swfilter() and dvb_dmx_swfilter_204() users: please test the performance improvement. The second comment is mostly to convince Manu Abraham, but the concept might be useful for others too. Please read. 31.12.2010 14:41, Mauro Carvalho Chehab wrote: Dear Developers, == Need more tests/acks from DVB users == Aug, 7 2010: Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function http://patchwork.kernel.org/patch/118147 Marko Ristola marko.rist...@kolumbus.fi This patch affects equally for both dvb_dmx_swfilter() and dvb_dmx_swfilter_204(). The resulting performance and functionality is similar with dvb_dmx_swfilter_packets(). dvb_dmx_swfilter(_204)() have robustness checks. Devices without such need can still use dvb_dmx_swfilter_packets(). My performance patch helps especially with high volume 256-QAM TS streams containing for example HDTV content by removing one unnecessary stream copy. With perf top, dvb_dmx_swfilter_204()'s CPU consumption reduced in the following way: Without the patch dvb_dmx_swfilter_204() and dvb_dmx_swfilter_packet() took about equal amounts of CPU. With the patch dvb_dmx_swfilter_204()'s CPU consumption went into 1/5 or 1/10 of the original (resulting minor CPU consumption). dvb_dmx_swfilter_packet()'s CPU time was about as before. Test environment was Fedora with VDR streaming HDTV into network. I have tested the patch thoroughly (see for example https://patchwork.kernel.org/patch/108274). The patch assumes that dvb_dmx_swfilter_packet() can eat data stream fast enough so that the DVB card's DMA engine will not replace buffer content underneath of dvb_dmx_swfilter_204(). It would be helpful if someone using dvb_dmx_swfilter() could try the patch too: maybe nobody has tried it. The benefit would be, that dvb_dmx_swfilter() is almost equally fast as the existing blatantly fast dvb_dmx_swfilter_packets() :) * I want to see people testing the above patch, as it seems to improve * * DVB performance by avoiding data copy. * == mantis patches - Waiting for Manu Abraham abraham.m...@gmail.com == Jun,20 2010: [2/2] DVB/V4L: mantis: remove unused files http://patchwork.kernel.org/patch/107062 Bjørn Mork bj...@mork.no Jul,19 2010: Twinhan DTV Ter-CI (3030 mantis) http://patchwork.kernel.org/patch/112708 Niklas Claesson nicke.claes...@gmail.com Aug, 7 2010: Refactor Mantis DMA transfer to deliver 16Kb TS data per interrupt http://patchwork.kernel.org/patch/118173 Marko Ristola marko.rist...@kolumbus.fi 16Kb TS stream improvement: What do you think about this, Manu Abraham? If high volume TS stream (256-QAM) with 50Mbits/s generates one interrupt once per 4096 bytes, you have a problem. Mantis driver copies 4096 bytes with one interrupt, into dvb_core's 128K buffer. VDR reads data in big pieces (as much data as you get). Network layer receives data in about 100K - 300K pieces from VDR. So I think that it is unnecessary to interrupt other processes once per 4096 bytes, for 56Mbit/s streams. With 16K / interrupt you reduce the number of interrupts into one quarter, without affecting VDR at all, lessening unnecessary interrupts and giving CPU time to other processes. The CPU might have some other things to do without wanting to get interrupted once per (displaying HDTV, delivering HDTV stream to network). Mantis/Hopper kernel driver must process the whole high volume TS stream, although it might deliver only one radio channel forward. I saw glitches with HDTV (followed by stream halt) even though the Mantis DVB card only delivered the data into the local network via VDR. I don't have such single CPU computer anymore though. 16K comes from the fact that it is not good to try to overload dvb_core's 128K buffer. 3/4 of the interrupts are gone. DVB driver does at least twice the number of interrupts compared to VDR reading the stream: dvb_core's buffer is never empty when VDR asks more data. Best regards, Marko Ristola -- 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
Re: Thoughts about suspending DVB C PCI device transparently
Hi.
Thank you for noticing my old email.
My observation of working suspend/resume implementation was,
that the needed amount of code for suspend and resume functions
was very small and easy. I reorganized mantis_pci.c so that both
module initialization and suspend/resume could use the same
functions for DVB device initialization.
Mantis structure needed maintenance of status before suspend
so that the status could be restored afterwards:
- power state (had feature to turn tuner off when idle).
- get_frontend(fe, param) content.
- number of feeds before suspend.
Each sub-device (C-file) needed it's own suspend() and resume() function
and these functions had to be called in the right order, starting
from mantis_pci.c's functions, some maintained IRQ states. After powering
and initializing all hardware, frontend was tuned back to state before suspend,
FE lock waited and DMA transfer turned back on.
Other kernel parts (the framework) took care that
no user space program will interfere during the resume process.
Media/dvb-core didn't need any changes for this to work.
Regards,
Marko Ristola
20.11.2010 14:57, Richard Zidlicky wrote:
Hi,
found this old email when searching for suspend issues, seems like a good idea.
Just wondering - how many eg dvb drivers are there with working
suspend/hibernate
(all buses, not just PCI)? Me thinks only a small fraction, the rest will crash
unless blacklisted by pm-utils?
I don't know any that works, but maybe there are a few of them.
Would it be worth to code a generic approach working around drivers that need
to be
blacklisted? It seems that because of eg firmware loading this might be the
only way
to get dvb drivers behave?
Sharing the approach would work, but code sharing might not be worth it.
Maybe putting some knowledge into small functions to ease up bus initialization
would help: driver developer would need as few knowledge from the bus as
possible.
A document and a reference implementation would help others to add
suspend/resume
into their simple drivers. Maybe users and developers want nicer suspend/resume
implementation now compared to the old times. Is the demand high enough now?
Richard
Once in a time I wrote into Mantis driver Suspend / resume
code. The idea was, that bridge driver (mantis_dvb.c) will
handle the suspend / resume transparently to the application.
With a PCI device this was rather easy to achieve.
With xine, there was just a glitch with video and audio
after resume.
So after suspend, frontend was tuned into the original
frequency, and the DMA transfer state was restored.
Suspend:
1. Turn off possible DMA transfer if active (feeds 0)
2. Remember tuner power on state.
3. Do tuner and fronted power off.
Resume:
1. Restore frontend and tuner power.
2. (feeds 0)? Set frequency for the tuner.
3. (feeds 0)? Restore DMA transfer into previous state.
What do you think about this?
I need some feedback: is it worth coding?
Other needed code is usual suspend / resume stuff.
Is it worth powering off the tuner, if it isn't
used?
For my current usage, powering off the unused tuner
gives more power savings than implementing suspend/resume.
Marko Ristola
--
// suspend to standby, ram or disk.
int mantis_dvb_suspend(struct mantis_pci *mantis, pm_message_t
prevState, pm_message_t mesg)
{
if (mantis-feeds 0)
mantis_dma_stop(mantis);
if (mantis-has_power)
mantis_fe_powerdown(mantis); // power off tuner.
return 0;
}
void mantis_dvb_resume(struct mantis_pci *mantis, pm_message_t prevMesg)
{
// power on frontend and tuner.
mantis_frontend_tuner_init(mantis);
if (mantis-feeds 0 mantis-fe-ops.tuner_ops.init)
(mantis-fe-ops.init)(mantis-fe);
if (mantis-feeds 0) {
(mantis-fe-ops.set_frontend)(mantis-fe, NULL);
mantis_dma_start(mantis);
}
}
--
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
--
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
--
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
Re: [GIT PATCHES FOR 2.6.38] mantis for_2.6.38
Hi Bjørn Mork and Mauro Carvalho Chehab. The following patch was an experiment and wasn't actually meant for inclusion into the Kernel. Jul,10 2010: Mantis driver patch: use interrupt for I2C traffic instead of busy reg http://patchwork.kernel.org/patch/111245 Marko Ristola marko.rist...@kolumbus.fi Would you please drop it from the Patchwork? Would you Bjørn remove it from your Git repository? The patch is ugly and it doesn't work, the broken part is active only with vp2033. I'm sorry for the hassle. Cheers, Marko Ristola 13.11.2010 16:26, Mauro Carvalho Chehab wrote: Em 12-11-2010 12:43, Bjørn Mork escreveu: Hello, I've been waiting for this list of patchwork patches to be included for quite a while, and have now taken the liberty to clean them up as necessary and add them to a git tree, based on the current media_tree for_v2.6.38 branch, with exceptions as noted below: == mantis patches - Waiting for Manu Abraham abraham.m...@gmail.com == Jul,10 2010: Mantis driver patch: use interrupt for I2C traffic instead of busy reg http://patchwork.kernel.org/patch/111245 Marko Ristola marko.rist...@kolumbus.fi The following changes since commit af9f14f7fc31f0d7b7cdf8f7f7f15a3c3794aea3[media] IR: add tv power scancode to rc6 mce keymap are available in the git repository at: git://git.mork.no/mantis.git for_2.6.38 -- 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
Re: [PATCH] Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function
Hi.
This patch is like the original, but without mangled spaces.
I found Documentation/email-clients.txt to be useful for tuning Thunderbird.
DVB-S2 users with high volume stream data are interested in trying this patch
too.
Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c
b/drivers/media/dvb/dvb-core/dvb_demux.c
index 977ddba..b71d77d 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -478,97 +478,96 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux,
const u8 *buf,
EXPORT_SYMBOL(dvb_dmx_swfilter_packets);
-void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
+static inline int findNextPacket(const u8 *buf, int pos, size_t count,
+ const int pktsize)
{
- int p = 0, i, j;
+ int start = pos, lost;
- spin_lock(demux-lock);
-
- if (demux-tsbufp) {
- i = demux-tsbufp;
- j = 188 - i;
- if (count j) {
- memcpy(demux-tsbuf[i], buf, count);
- demux-tsbufp += count;
- goto bailout;
- }
- memcpy(demux-tsbuf[i], buf, j);
- if (demux-tsbuf[0] == 0x47)
- dvb_dmx_swfilter_packet(demux, demux-tsbuf);
- demux-tsbufp = 0;
- p += j;
+ while(likely(pos count)) {
+ if (likely(buf[pos] == 0x47 ||
+ (pktsize == 204 buf[pos] == 0xB8)))
+ break;
+ pos++;
}
- while (p count) {
- if (buf[p] == 0x47) {
- if (count - p = 188) {
- dvb_dmx_swfilter_packet(demux, buf[p]);
- p += 188;
- } else {
- i = count - p;
- memcpy(demux-tsbuf, buf[p], i);
- demux-tsbufp = i;
- goto bailout;
- }
- } else
- p++;
+ if (unlikely(lost = pos - start)) {
+ /* This garbage is part of a valid packet? */
+ int backtrack = pos - pktsize;
+ if (backtrack = 0 (buf[backtrack] == 0x47 ||
+ (pktsize == 204 buf[backtrack] == 0xB8))) {
+ /* printk(demux: backtracked %d bytes
+* \n, start - backtrack); */
+ return backtrack;
+ }
+ /*printk(demux: skipped %d bytes at %d\n, lost, start); */
}
-bailout:
- spin_unlock(demux-lock);
+ return pos;
}
-EXPORT_SYMBOL(dvb_dmx_swfilter);
-
-void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf, size_t count)
+/* Filter all pktsize= 188 or 204 sized packets and skip garbage. */
+static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf,
+ size_t count, const int pktsize)
{
int p = 0, i, j;
- u8 tmppack[188];
+ const u8 *q;
spin_lock(demux-lock);
- if (demux-tsbufp) {
+ if (likely(demux-tsbufp)) { /* tsbuf[0] is now 0x47. */
i = demux-tsbufp;
- j = 204 - i;
- if (count j) {
+ j = pktsize - i;
+ if (unlikely(count j)) {
memcpy(demux-tsbuf[i], buf, count);
demux-tsbufp += count;
goto bailout;
}
memcpy(demux-tsbuf[i], buf, j);
- if ((demux-tsbuf[0] == 0x47) || (demux-tsbuf[0] == 0xB8)) {
- memcpy(tmppack, demux-tsbuf, 188);
- if (tmppack[0] == 0xB8)
- tmppack[0] = 0x47;
- dvb_dmx_swfilter_packet(demux, tmppack);
- }
+ if (likely(demux-tsbuf[0] == 0x47)) /* double check */
+ dvb_dmx_swfilter_packet(demux, demux-tsbuf);
demux-tsbufp = 0;
p += j;
}
- while (p count) {
- if ((buf[p] == 0x47) || (buf[p] == 0xB8)) {
- if (count - p = 204) {
- memcpy(tmppack, buf[p], 188);
- if (tmppack[0] == 0xB8)
- tmppack[0] = 0x47;
- dvb_dmx_swfilter_packet(demux, tmppack);
- p += 204;
- } else {
- i = count - p;
- memcpy(demux-tsbuf, buf[p], i);
- demux-tsbufp = i;
- goto bailout;
- }
- } else {
- p++;
+ while (likely((p
Re: [PATCH] Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function
Hi The old patch with broken spaces is https://patchwork.kernel.org/patch/108274/ The one with good patch and bad introduction is https://patchwork.kernel.org/patch/118147/ Regards, Marko Ristola 07.08.2010 12:47, Marko Ristola wrote: Hi. This patch is like the original, but without mangled spaces. I found Documentation/email-clients.txt to be useful for tuning Thunderbird. DVB-S2 users with high volume stream data are interested in trying this patch too. Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi [ snip ] -- 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
[PATCH] Refactor Mantis DMA transfer to deliver 16Kb TS data per interrupt
This patch obsoletes patch with broken spaces
https://patchwork.kernel.org/patch/107036/
With VDR streaming HDTV into network, generating an interrupt once per 16kb,
implemented in this patch, seems to support more robust throughput with HDTV.
Fix leaking almost 64kb data from the previous TS after changing the TS.
One effect of the old version was, that the DMA transfer and driver's
DMA buffer access might happen at the same time - a race condition.
Signed-off-by: Marko M. Ristola marko.rist...@kolumbus.fi
Regards,
Marko Ristola
diff --git a/drivers/media/dvb/mantis/hopper_cards.c
b/drivers/media/dvb/mantis/hopper_cards.c
index 09e9fc7..3b7e376 100644
--- a/drivers/media/dvb/mantis/hopper_cards.c
+++ b/drivers/media/dvb/mantis/hopper_cards.c
@@ -126,7 +126,7 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
}
if (stat MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, %s, label[8]);
- mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
+ mantis-busy_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
if (stat MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index cf4b39f..8f048d5 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -134,7 +134,7 @@ static irqreturn_t mantis_irq_handler(int irq, void *dev_id)
}
if (stat MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, %s, label[8]);
- mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
+ mantis-busy_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
if (stat MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_common.h
b/drivers/media/dvb/mantis/mantis_common.h
index d0b645a..23b23b7 100644
--- a/drivers/media/dvb/mantis/mantis_common.h
+++ b/drivers/media/dvb/mantis/mantis_common.h
@@ -122,11 +122,8 @@ struct mantis_pci {
unsigned intnum;
/* RISC Core */
- u32 finished_block;
+ u32 busy_block;
u32 last_block;
- u32 line_bytes;
- u32 line_count;
- u32 risc_pos;
u8 *buf_cpu;
dma_addr_t buf_dma;
u32 *risc_cpu;
diff --git a/drivers/media/dvb/mantis/mantis_dma.c
b/drivers/media/dvb/mantis/mantis_dma.c
index 46202a4..c61ca7d 100644
--- a/drivers/media/dvb/mantis/mantis_dma.c
+++ b/drivers/media/dvb/mantis/mantis_dma.c
@@ -43,13 +43,17 @@
#define RISC_IRQ (0x01 24)
#define RISC_STATUS(status)~status) 0x0f) 20) | ((status 0x0f)
16))
-#define RISC_FLUSH() (mantis-risc_pos = 0)
-#define RISC_INSTR(opcode) (mantis-risc_cpu[mantis-risc_pos++] =
cpu_to_le32(opcode))
+#define RISC_FLUSH(risc_pos) (risc_pos = 0)
+#define RISC_INSTR(risc_pos, opcode) (mantis-risc_cpu[risc_pos++] =
cpu_to_le32(opcode))
#define MANTIS_BUF_SIZE(64 * 1024)
-#define MANTIS_BLOCK_BYTES (MANTIS_BUF_SIZE 4)
-#define MANTIS_BLOCK_COUNT (1 4)
-#define MANTIS_RISC_SIZE PAGE_SIZE
+#define MANTIS_BLOCK_BYTES (MANTIS_BUF_SIZE / 4)
+#define MANTIS_DMA_TR_BYTES (2 * 1024) /* upper limit: 4095 bytes. */
+#define MANTIS_BLOCK_COUNT (MANTIS_BUF_SIZE / MANTIS_BLOCK_BYTES)
+
+#define MANTIS_DMA_TR_UNITS (MANTIS_BLOCK_BYTES / MANTIS_DMA_TR_BYTES)
+/* MANTIS_BUF_SIZE / MANTIS_DMA_TR_UNITS must not exceed MANTIS_RISC_SIZE (4k
RISC cmd buffer) */
+#define MANTIS_RISC_SIZE PAGE_SIZE /* RISC program must fit here. */
int mantis_dma_exit(struct mantis_pci *mantis)
{
@@ -124,27 +128,6 @@ err:
return -ENOMEM;
}
-static inline int mantis_calc_lines(struct mantis_pci *mantis)
-{
- mantis-line_bytes = MANTIS_BLOCK_BYTES;
- mantis-line_count = MANTIS_BLOCK_COUNT;
-
- while (mantis-line_bytes 4095) {
- mantis-line_bytes = 1;
- mantis-line_count = 1;
- }
-
- dprintk(MANTIS_DEBUG, 1, Mantis RISC block bytes=[%d], line
bytes=[%d], line count=[%d],
- MANTIS_BLOCK_BYTES, mantis-line_bytes, mantis-line_count);
-
- if (mantis-line_count 255) {
- dprintk(MANTIS_ERROR, 1, Buffer size error);
- return -EINVAL;
- }
-
- return 0;
-}
-
int mantis_dma_init(struct mantis_pci *mantis)
{
int err = 0;
@@ -158,12 +141,6 @@ int mantis_dma_init(struct mantis_pci *mantis)
goto err;
}
- err = mantis_calc_lines(mantis);
- if (err 0) {
- dprintk(MANTIS_ERROR, 1, Mantis calc lines failed);
-
- goto err;
- }
return 0;
err:
@@ -174,31
Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration
05.08.2010 12:38, Laurent Pinchart wrote: Hi Marko, On Wednesday 04 August 2010 21:28:22 Marko Ristola wrote: Hi Hans and Laurent. I hope my thoughts help you further. Thank you for sharing them. Thank you for taking time to answer for me. [ snip ] There's probably some confusion here. The media controller aims at giving userspace the ability to discover the internal device topology. This includes various information about the entities, such as a name, a version number, ... The I2C data you mention are low-level information required by the kernel to talk to the I2C device, but I don't think they're useful to userspace at all. That kind of information come either from platform data or from the I2C device driver and get used internally in the kernel. Do you see any need to expose such information to userspace ? I don't see any needs to expose such information into userspace. I have some thoughts about I2C hardware problems, but that is of course off topic. I wrote them here only because I tried to draw some bigger picture how this entity, pads and links thing relates to the whole driver. [ snip ] With I2C, an array of I2C slave devices that are reachable via I2C bus would work for controlling the device rather nicely. Higher abstraction level So detailed descriptions and bus knowledge is needed for controlling each entity and pad. It's needed to control them inside the kernel, but I don't think it's needed in userspace. I agree: The detailed information needs to be internal to the driver, and userspace needs only the information that is needed for being able to decide how to configure the streams over many drivers, and how each driver must be asked to do the configuration for it, without driver needing knowledge about other related drivers. Maybe a pad could have a list of pads that it can connect to. Each destination pad reference could have a link as a property if the link is mandatory for binding the pads together. A list of open links without pads might be usable too, enabling binding different drivers to pass data to each other with pad-link-pad binding. Driver could be a property of a pad for being able to do binding over drivers. You have of course a better picture and understanding about these, and you will need to come up with the best solution together on your own. Maybe I just need to do something else and go and buy another motherboard from Helsinki Ruoholahti to replace my overheated one, walking by some Nokia Research Center buildings. Best regards from the summerish Finland. Marko Ristola -- 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
Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration
Hi Hans and Laurent. I hope my thoughts help you further. 03.08.2010 12:22, Laurent Pinchart wrote: Hi Hans, On Monday 02 August 2010 23:01:55 Hans Verkuil wrote: On Monday 02 August 2010 16:35:54 Laurent Pinchart wrote: On Sunday 01 August 2010 13:58:20 Hans Verkuil wrote: On Thursday 29 July 2010 18:06:39 Laurent Pinchart wrote: [snip] [snip] It's a possibility, but it's always a bit of a hassle in an application to work with group IDs. I wonder if there is a more elegant method. The problem is a bit broader than just showing relationships between video nodes and ALSA devices. We also need to show relationships between lens/flash controllers and sensors for instance. Group IDs sound easy, but I'm open to suggestions. Low level example DVB I2C bus is easy: get all I2C devices from an entity (DVB demuxer). Some external chip (entity, the tuner) might be behind some I2C bridge device. With I2C you need to know the characteristics, how you talk with the destination device via the bus (extra sleeps, clock speed, quiesce the whole bus for 50ms after talking to the slave device). I'd like that each device would describe how it should be talked to via the bus. On i2c_transfer you could hide opening and closing the I2C bridge, and hide the callbacks for extra sleeps so that the main driver and core framework code is free from such ugly details. By storing entity's special requirements inside of it, you could reuse the callbacks with another product variant. With I2C, an array of I2C slave devices that are reachable via I2C bus would work for controlling the device rather nicely. Higher abstraction level So detailed descriptions and bus knowledge is needed for controlling each entity and pad. That hierarchy is a bit different than optimal hierarchy of how the streams can flow into, within and out from the entity (the driver). Buses are the gateways for the data stream flows, shared by two or more entities/pads by links. Thus I'd suggest to separate these two hierarchies (initialization time hierarchy and stream flow capability hierarchy) at necessary points, and use buses to bind the entities/pads by links to each other. A single wire with just two end points can also be thought like a bus. Regards, Marko Ristola [ snip ] -- 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
Re: [Q]: any DVB-S2 card which is 45MS/s capable?
Hi. I have written some Mantis bandwidth related DMA transfer optimizations on June/July this year. They are now waiting for approval by Manu Abraham. Those reduce CPU pressure, increasing the bandwidth that can be received from the DVB card. 45MS/s bandwidth support will surely benefit from those patches. Main features: 1. Do one CPU interrupt per 16KB data instead per 4KB data. My implementation benefits only Mantis cards. https://patchwork.kernel.org/patch/107036/ 2. Remove unnecessary big CPU overhead, when data is delivered from the DVB card's DMA buffer into Linux kernel's DVB subsystem. Number 2 reduces the CPU pressure by almost 50%. This implementation benefits many other Linux supported DVB cards too. http://patchwork.kernel.org/patch/108274/ Those helped with my older single CPU Core computer with 256-QAM, delivering HDTV channel into the network and watching the HDTV channel with a faster computer. The performance bottlenecks could be seen on the command line with perf top. I had to increase PCI bus latency setting into 64 too from the BIOS. Moving DVB device into separate IRQ line with Ethernet card helped too, because Ethernet card did an interrupt per ethernet packet. So if the hardware can deliver 45MS/s data fast enough, software side can be optimized up to some point. My patches contain the most easy major optimizations that I found. If you can test some of those patches, it might help to get them into Linux kernel faster. Best regards, Marko Ristola 27.07.2010 18:11, VDR User wrote: On Tue, Jul 27, 2010 at 5:52 AM, Emmanueleall...@gmail.com wrote: VDR User a écrit : Look at the vp-1041 I think. From what I gathered it is not able to do 45MS/s for DVB-S2. Thanks anyway, You may want to ask Manu Abraham (author of the mantis driver) about that to be sure. It seems I recall him telling me it could quite some time ago. -- 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 -- 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
Thoughts about suspending DVB C PCI device transparently
Hi all.
Once in a time I wrote into Mantis driver Suspend / resume
code. The idea was, that bridge driver (mantis_dvb.c) will
handle the suspend / resume transparently to the application.
With a PCI device this was rather easy to achieve.
With xine, there was just a glitch with video and audio
after resume.
So after suspend, frontend was tuned into the original
frequency, and the DMA transfer state was restored.
Suspend:
1. Turn off possible DMA transfer if active (feeds 0)
2. Remember tuner power on state.
3. Do tuner and fronted power off.
Resume:
1. Restore frontend and tuner power.
2. (feeds 0)? Set frequency for the tuner.
3. (feeds 0)? Restore DMA transfer into previous state.
What do you think about this?
I need some feedback: is it worth coding?
Other needed code is usual suspend / resume stuff.
Is it worth powering off the tuner, if it isn't
used?
For my current usage, powering off the unused tuner
gives more power savings than implementing suspend/resume.
Marko Ristola
--
// suspend to standby, ram or disk.
int mantis_dvb_suspend(struct mantis_pci *mantis, pm_message_t
prevState, pm_message_t mesg)
{
if (mantis-feeds 0)
mantis_dma_stop(mantis);
if (mantis-has_power)
mantis_fe_powerdown(mantis); // power off tuner.
return 0;
}
void mantis_dvb_resume(struct mantis_pci *mantis, pm_message_t prevMesg)
{
// power on frontend and tuner.
mantis_frontend_tuner_init(mantis);
if (mantis-feeds 0 mantis-fe-ops.tuner_ops.init)
(mantis-fe-ops.init)(mantis-fe);
if (mantis-feeds 0) {
(mantis-fe-ops.set_frontend)(mantis-fe, NULL);
mantis_dma_start(mantis);
}
}
--
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
Mantis driver patch: use interrupt for I2C traffic instead of busy registry polling
Hi
Here is a maybe too ugly patch for direct inclusion:
The code should be reworked to be nicer first.
This seems to work though.
What do you think the idea?
Problem:
I figured out that Mantis I2C read and write functions do busy waiting,
by reading via PCI bus a 32 bit word about 350 times in a tight loop.
I suspected that PCI DMA transfer from Mantis card to
CPU memory might get disturbed (FRTG DMA error message by Mantis)
because this heavy polling might cause the RISC processor's
DMA transfer to slow down so much, that there
comes glitches into the DVB stream (some internal FIFO gets full).
So that's why I wrote this patch: by reducing PCI bus contention,
the RISC DMA transfer should be more robust.
The heavy I2C command seems to be that dvb_core checks whether FE_LOCK
still holds.
I think that DVB card's I2C transfers aren't so performance critical,
that the heavy PCI contention by polling is justified. Or is it?
I looked at saa7146_i2c.c from Linux kernel, how to use
wait_event_interruptible_timeout.
Signed-off-by: Marko M Ristola marko.rist...@kolumbus.fi
Regards,
Marko Ristola
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index cf4b39f..56c5c30 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -58,7 +58,7 @@ static int devs;
#define DRIVER_NAMEMantis
-static char *label[10] = {
+static char *label[11] = {
DMA,
IRQ-0,
IRQ-1,
@@ -68,6 +68,7 @@ static char *label[10] = {
PPERR,
FTRGT,
RISCI,
+I2CDONE,
RACK
};
@@ -137,8 +138,15 @@ static irqreturn_t mantis_irq_handler(int irq, void
*dev_id)
mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
-if (stat MANTIS_INT_I2CDONE) {
-dprintk(MANTIS_DEBUG, 0, %s, label[9]);
+if (stat (MANTIS_INT_I2CDONE | MANTIS_INT_I2CRACK)) {
+if (stat MANTIS_INT_I2CDONE) {
+dprintk(MANTIS_DEBUG, 0, %s, label[9]);
+mantis-i2c_status |= MANTIS_INT_I2CDONE;
+}
+if (stat MANTIS_INT_I2CRACK) {
+dprintk(MANTIS_DEBUG, 0, %s, label[10]);
+mantis-i2c_status |= MANTIS_INT_I2CRACK;
+}
wake_up(mantis-i2c_wq);
}
mmwrite(stat, MANTIS_INT_STAT);
diff --git a/drivers/media/dvb/mantis/mantis_common.h
b/drivers/media/dvb/mantis/mantis_common.h
index d0b645a..3773ad0 100644
--- a/drivers/media/dvb/mantis/mantis_common.h
+++ b/drivers/media/dvb/mantis/mantis_common.h
@@ -77,7 +77,8 @@
enum mantis_i2c_mode {
MANTIS_PAGE_MODE = 0,
-MANTIS_BYTE_MODE,
+MANTIS_BYTE_MODE = 1,
+MANTIS_IRQ_PAGE_MODE = 2,
};
struct mantis_pci;
@@ -136,6 +137,7 @@ struct mantis_pci {
struct i2c_adapteradapter;
inti2c_rc;
+inti2c_status;
wait_queue_head_ti2c_wq;
struct mutexi2c_lock;
diff --git a/drivers/media/dvb/mantis/mantis_i2c.c
b/drivers/media/dvb/mantis/mantis_i2c.c
index 7870bcf..76de8f8 100644
--- a/drivers/media/dvb/mantis/mantis_i2c.c
+++ b/drivers/media/dvb/mantis/mantis_i2c.c
@@ -38,6 +38,8 @@
static int mantis_i2c_read(struct mantis_pci *mantis, const struct
i2c_msg *msg)
{
u32 rxd, i, stat, trials;
+u32 intstat;
+long timeout;
dprintk(MANTIS_INFO, 0, %s: Address=[0x%02x] R[ ,
__func__, msg-addr);
@@ -51,27 +53,53 @@ static int mantis_i2c_read(struct mantis_pci
*mantis, const struct i2c_msg *msg)
if (i == (msg-len - 1))
rxd = ~MANTIS_I2C_STOP;
-mmwrite(MANTIS_INT_I2CDONE, MANTIS_INT_STAT);
+mantis-i2c_status = 0;
+intstat = MANTIS_INT_I2CDONE | MANTIS_INT_I2CRACK;
+mmwrite(intstat, MANTIS_INT_STAT);
mmwrite(rxd, MANTIS_I2CDATA_CTL);
/* wait for xfer completion */
-for (trials = 0; trials TRIALS; trials++) {
-stat = mmread(MANTIS_INT_STAT);
-if (stat MANTIS_INT_I2CDONE)
-break;
+if (mantis-hwconfig-i2c_mode == MANTIS_IRQ_PAGE_MODE) {
+timeout = HZ / 64;
+timeout = wait_event_interruptible_timeout(mantis-i2c_wq,
mantis-i2c_status MANTIS_INT_I2CDONE, timeout);
+if (timeout == -ERESTARTSYS) {
+dprintk(MANTIS_DEBUG, 1, Mantis I2C read: got
-ERESTARTSYS.\n);
+return -ERESTARTSYS;
+} if (timeout == 0L) {
+dprintk(MANTIS_DEBUG, 1, Mantis I2C read: waiting
I2CDONE failed.\n);
+return -EIO;
+}
+dprintk(MANTIS_TMG, 0, I2CDONE: time remained %ld/%d
jiffies\n, timeout, HZ / 64);
+} else {
+for (trials = 0; trials TRIALS; trials++) {
+stat = mmread(MANTIS_INT_STAT);
+if (stat MANTIS_INT_I2CDONE)
+break;
+}
+dprintk(MANTIS_TMG, 0, I2CDONE: trials=%d\n, trials
Re: [PATCH] Mantis: append tasklet maintenance for DVB stream delivery
Resending into linux-media, for confirming authorship:
I have personally done this patch.
Acked-by: Marko M Ristola marko.rist...@kolumbus.fi
Regards,
Marko
20.06.2010 16:51, Bjørn Mork kirjoitti:
Note that mantis_core_exit() is never called. Unless I've missed
something, the drivers/media/dvb/mantis/mantis_core.{h,c} files can
just be deleted. They look like some development leftovers?
I see. mantis_core.ko kernel module exists though.
Maybe the mantis/Makefile references for mantis_core.c, mantis.c and hopper.c
are just some leftovers too.
I moved tasklet_enable/disable calls into mantis_dvb.c where almost all other
tasklet code is located.
So the following reasoning still holds:
1. dvb_dmxdev_filter_stop() calls mantis_dvb_stop_feed: mantis_dma_stop()
2. dvb_dmxdev_filter_stop() calls release_ts_feed() or some other filter
freeing function.
3. tasklet: mantis_dma_xfer calls dvb_dmx_swfilter to copy DMA buffer's content
into freed memory, accessing freed spinlocks.
This case might occur while tuning into another frequency.
Perhaps cdurrhau has found some version from this bug at
http://www.linuxtv.org/pipermail/linux-dvb/2010-June/032688.html:
This is what I get on the remote console via IPMI:
40849.442492] BUG: soft lockup - CPU#2 stuck for 61s! [section
handler:4617]
New reasoning for the patch (same as the one above, but from higher level):
After dvb-core has called mantis-fe-stop_feed(dvbdmxfeed) the last time (count
to zero),
no data should ever be copied with dvb_dmx_swfilter() by a tasklet: the target
structure might be in an unusable state.
Caller of mantis_fe-stop_feed() assumes that feeding is stopped after
stop_feed() has been called, ie. dvb_dmx_swfilter()
isn't running, and won't be called.
There is a risk that dvb_dmx_swfilter() references freed resources (memory or
spinlocks or ???) causing instabilities.
Thus tasklet_disable(mantis-tasklet) must be called inside of
mantis-fe-stop_feed(dvbdmxfeed) when necessary.
Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi
Marko
diff --git a/drivers/media/dvb/mantis/mantis_dvb.c
b/drivers/media/dvb/mantis/mantis_dvb.c
index 99d82ee..a9864f7 100644
--- a/drivers/media/dvb/mantis/mantis_dvb.c
+++ b/drivers/media/dvb/mantis/mantis_dvb.c
@@ -117,6 +117,7 @@ static int mantis_dvb_start_feed(struct dvb_demux_feed
*dvbdmxfeed)
if (mantis-feeds == 1) {
dprintk(MANTIS_DEBUG, 1, mantis start feed dma);
mantis_dma_start(mantis);
+tasklet_enable(mantis-tasklet);
}
return mantis-feeds;
@@ -136,6 +137,7 @@ static int mantis_dvb_stop_feed(struct dvb_demux_feed
*dvbdmxfeed)
mantis-feeds--;
if (mantis-feeds == 0) {
dprintk(MANTIS_DEBUG, 1, mantis stop feed and dma);
+tasklet_disable(mantis-tasklet);
mantis_dma_stop(mantis);
}
@@ -216,6 +218,7 @@ int __devinit mantis_dvb_init(struct mantis_pci *mantis)
dvb_net_init(mantis-dvb_adapter, mantis-dvbnet, mantis-demux.dmx);
tasklet_init(mantis-tasklet, mantis_dma_xfer, (unsigned long) mantis);
+tasklet_disable(mantis-tasklet);
if (mantis-hwconfig) {
result = config-frontend_init(mantis, mantis-fe);
if (result 0) {
--
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
[PATCH] Mantis DMA transfer cleanup, fixes data corruption and a race, improves performance. (signed-off this time)
Hi
This is a resend of the exactly same patch
than I sent 2010-06-20, to sign off it.
Signed-off-by: Marko M Ristola marko.rist...@kolumbus.fi
Regards,
Marko Ristola
-
Here is another version of the DMA transfer fix.
Please try it. Comments?
The current DMA code with drivers/media/dvb/mantis/mantis_dma.c has user
visible problems:
- about 1500 interrupts per second. One CPU can't copy h.264 data over
the network for me (vdr, streamdev).
- 64K garbage at start of the data stream, part of which goes into User
land application.
The garbage data is partly from the same stream (given twice), and
partly from previous tuned frequency (buffer
uninitialized at DMA start).
- Race condition: Memory copying from DMA buffer is done at the same
time as there is DMA transfer going on to the same bytes.
Can this race cause harware problems?
Current DMA code is not clear:
Current DMA RISC coding creates one DMA transfer per 2048 bytes.
Risc command generating code has mantis-line_count=32.
mantis-last_block and mantis-finished_block iterates over [0 to 16].
These counters are confusing. It takes lots of time to look and debug to
understand
what is happening there even for me :(
Basic example how the 64K garbage generation happens:
At mantis_dma_start, mantis-last_block = mantis-finished_block = 0
1st DMA transfer (2048 bytes) generates interrupt, sets
mantis-finished_block=15.
Tasklet will call mantis_dma_xfer(), which iterates from
mantis-last_block = 0 to mantis-finished_block = 14. Set last_block=15.
2nd DMA transfer of 2048 bytes goes quietly (no interrupt generated),
race with the tasklet above here.
3rd DMA transfer sets mantis-finished_block = 0, mantis_dma_xfer()
copies mantis-finished_block = 15. Set last_block=0.
After this copying continues from block 0, so the content is valid,
although block 0 was already partly copied.
Because the current looping implementation is too hard to understand,
I decided to rewrite it and not give you any small patch that fixes the
issue but nobody else understands it.
This doesn't have the alignment stuff at all that I mentioned in earlier
emails last week.
Basic idea:
Keep DMA buffer of size 64k, but generate interrupts four times, thus
one interrupt per 16k.
Rename mantis-finished_block to be mantis-busy_block, because that
keeps mantis_dma_xfer() simple:
while(mantis-last_block != mantis-busy_block) { do copy, last_block =
(last_block + 1) mod 4.
last_block is thus incremented until last_block == busy_block, which
can't be copied yet.
DMA RISC code generation: outer loop iterates over blocks from 0 to 4.
Inner loop iterates over DMA transfer units from 0 to
MANTIS_DMA_TR_UNITS, each DMA transfer is 2048 bytes.
The interrupt is generated at block[0], DMA unit 0: the block 0 is now
busy :)
mantis-line_bytes, mantis-line_count and mantis-risc_pos
were used only for DMA risc code generation.
Removed them from the structure.
Benefits of this code:
- Removes the 64k garbage issue.
- Remove race condition with concurrent DMA transfer and memory copy.
- Lessen interrupts to about 350 per second (seen by powertop) by
moving 16k bytes per interrupt, instead of 4k per interrupt.
The number of interrupts gets much smaller, and it becomes possible
with single core AMD cpu to
deliver h.264 data over the network from vdr via streamdev.
- Mantis DMA code is more understandable for reviewers and others.
My patch is GPLv2.
The patch is made against GIT linuxtv/master, applies cleanly to
Mercurial v4l-dvb too.
Best regards,
Marko Ristola
diff --git a/drivers/media/dvb/mantis/hopper_cards.c
b/drivers/media/dvb/mantis/hopper_cards.c
index 09e9fc7..3b7e376 100644
--- a/drivers/media/dvb/mantis/hopper_cards.c
+++ b/drivers/media/dvb/mantis/hopper_cards.c
@@ -126,7 +126,7 @@ static irqreturn_t hopper_irq_handler(int irq, void
*dev_id)
}
if (stat MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, %s, label[8]);
-mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
+mantis-busy_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
if (stat MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index cf4b39f..8f048d5 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -134,7 +134,7 @@ static irqreturn_t mantis_irq_handler(int irq, void
*dev_id)
}
if (stat MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, %s, label[8]);
-mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
+mantis-busy_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
if (stat MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_common.h
b/drivers/media/dvb/mantis/mantis_common.h
index d0b645a..23b23b7 100644
--- a/drivers/media/dvb/mantis/mantis_common.h
+++ b/drivers/media/dvb/mantis
Re: buffer management
23.06.2010 15:43, Steven Toth kirjoitti:
Now, on each video interrupt, I know which SG list i need to read
from. At this stage i do need to copy the
buffers associated with each of the SG lists at once. In this
scenario, I don't see how videobuf could be used,
while I keep getting this feeling that a simple copy_to_user of the
entire buffer could do the whole job in a
better way, since the buffers themselves are already managed and
initialized already. Am I correct in thinking
so, or is it that I am overlooking something ?
I've thought this a bit. I didn't find any good easy solution for
copying directly into users pace.
Here are the easiest trivial speed improvements I found:
dvb_core: dvb_dmx_swfilter(_204) functions:
- avoid unnecessary packet copying.
- I emailed those patches. You can see the difference with perf top
easilly
(Fedora at least has perf).
So with this the copying into ringbuffer remains as an unnecessary thing.
Mantis: Don't do busy looping on mantis_i2c.c
Harder algorithm that does only copy_to_user(), but avoids ring buffer
copying too:
Here is an algorithm that how you could do just the copy_to_user() from
the DMA buffer directly rather safely.
The algorithm is for 204 sized packets, but is
trivial to convert for 188 sized packets too.
I don't know if this is worth it though.
I found with Mantis that packets from DMA aren't always sized as 204
bytes (random sized garbage).
Even with this, you could do 204 - 188 conversion, without moving data:
1. Reserve first 256 bytes of each 4K DMA buffer page outside of the DMA
transfer.
2. DMA of buf[0][256 - 4095].
3. Do 204 - 188 conversion for buf[0][256 - 4095].
4. Copy remaining overflowed 45 bytes into buf[1][256 - 45], just
before second DMA start.
5. Deliver an array of pointers for 188 sized packets for
dvb_dmx_swfilter_nocopy_packets().
6. DMA of buf[1][256 - 4095].
7. Do 204 - 188 conversion for buf[1][(256 - 45) - 4095].
8. Copy remaining overflowed 25 bytes into buf[2][256 - 25], just
before third DMA start.
9. Deliver a second array of pointers for 188 sized packets for
dvb_dmx_swfilter_nocopy_packets().
dvb_dmx_swfilter_nocopy_packets would then have to deliver just the
pointers with sizes
into a new style ring buffer.
Then you would wake up the waiting ringbuffer reader, and it would
do the copy_to_user() as is done now.
Some or most of the DMA buffer management could be in dvb_core/ side,
because it is so generic.
On Mantis side, you must grab DMA interrupt and call a function like
dvb_dmabuf_set_busy_buf(bufno).
Other thing is, that you must traverse DMA buffers for RISC programming,
like:
for (i=0; i dvb_dmabuf_count(dmabuf); i++) {
RISC_DMA(dvb_dmabuf_dma_pos(dmabuf, i) | MAKE_IRQ);
}
RISC_JUMP(risc_dma);
Other buffer management code isn't driver specific.
One possible problem that remains with this approach, is that what if
the DMA buffer gets overwritten?
Then the pointers in the ringbuffer might become garbage.
Another possible (cache coherency) problem is, that in this scenario
you both read and write to the DMA buffer.
Maybe with x86 computers this isn't a problem:
you never have to modify the same cache line
from both the DMA side and tasklet side at the same time.
Best regards,
Marko Ristola
--
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
Mantis DMA interrupt with FTRGT flag?
Hi Manu. Here is Mantis debug messages at verbose level 7: Jun 28 20:01:53 koivu kernel: -- Stat=3c0a Mask=802 --DMAFTRGTRISCIUnknown Stat=3000 Mask=802 Jun 28 20:01:53 koivu kernel: mantis_dma_xfer (0): last block=[2] finished block=[3] Jun 28 20:01:53 koivu kernel: demux: skipped 148 bytes at 9497: 00 .. Jun 28 20:01:53 koivu kernel: TS packet counter mismatch. PID=0x2b2 expected 0xa got 0xb What FTRGT message means? Those errors come seemingly randomly at blocks 0, 1, 2 and 3: 56 Stat=2000 68 71 Stat=1000 73 Stat=3000 So above, Stat= is hidden. With bttv, FTRGT means something like FIFO overflow. How can I make the error more rare? Regards, Marko Ristola -- 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
Re: [PATCH] Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function
Hi
Here is an improved version of the original patch:
The original patch removed unnecessary copying for 204 sized packets only.
This patch improves performance for 188 sized packets too.
Unnecessary copying means: if dvb_dmx_swfilter(_204)() doesn't have to
modify
the source packet, the source packet is delivered for
dvb_dmx_swfilter_packet()
without copying.
This assumes, that a DMA transfer won't modify the accepted 188/204 sized
packet underneath while dvb_dmx_swfilter_packet() processes it.
The assumption is already in dvb_dmx_swfilter_packets().
With tasklets the risk for breaking the assumption is low. If there
would be
a normal thread instead of a tasklet, copying from the DMA buffer might
come too late.
Could someone test this patch who uses the dvb_dmx_swfilter() function
(188 sized)?
So _dvb_dmx_swfilter is now common for both 188 and 204 sized packet
parsing.
The measure was done during recording of H.264 steram under VDR,
using perf top -d 10
PerfTop: 62 irqs/sec kernel:80.6% [1000Hz cycles], (all, 1 CPUs)
samples pcnt
functionDSO
339.00 18.1%
dvb_dmx_swfilter_packet
[dvb_core]
315.00 16.9%
acpi_pm_read
[kernel.kallsyms]
70.00 3.7%
_ZN2SI5CRC325crc32EPKcij
/usr/sbin/vdr
53.00 2.8%
mantis_i2c_xfer
[mantis_core]
53.00 2.8%
__mktime_internal
/lib64/libc-2.12.so
39.00 2.1%
_ZN14cAudioRepacker6RepackEP17cRingBufferLinearPKhi
/usr/sbin/vdr
33.00 1.8%
delay_tsc
[kernel.kallsyms]
30.00 1.6%
dvb_dmx_memcopy
[dvb_core]
28.00 1.5%
dvb_ringbuffer_write
[dvb_core]
28.00 1.5%
_dvb_dmx_swfilter
[dvb_core]
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c
b/drivers/media/dvb/dvb-core/dvb_demux.c
index 977ddba..2dd 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -478,95 +478,78 @@ void dvb_dmx_swfilter_packets(struct dvb_demux
*demux, const u8 *buf,
EXPORT_SYMBOL(dvb_dmx_swfilter_packets);
-void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
+static inline int findNextSyncByte(const u8 *buf, int pos, size_t
count, const int pktsize)
+{
+while(likely(pos count)) {
+if (likely(buf[pos] == 0x47 || (pktsize == 204 buf[pos] ==
0xB8)))
+break;
+pos++;
+}
+
+return pos;
+}
+
+/* pktsize must be either 204 or 188. If pktsize is 204, 0xB8 must be
accepted for SYNC byte too, but then convert it into 0x47.
+ * Designed pktsize so, that compiler would remove 204 related code
during inlining. */
+static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8
*buf, size_t count, const int pktsize)
{
int p = 0, i, j;
+const u8 *q;
spin_lock(demux-lock);
-if (demux-tsbufp) {
+if (likely(demux-tsbufp)) { /* tsbuf[0] is now 0x47. */
i = demux-tsbufp;
-j = 188 - i;
-if (count j) {
+j = pktsize - i;
+if (unlikely(count j)) {
memcpy(demux-tsbuf[i], buf, count);
demux-tsbufp += count;
goto bailout;
}
memcpy(demux-tsbuf[i], buf, j);
-if (demux-tsbuf[0] == 0x47)
+if (likely(demux-tsbuf[0] == 0x47)) /* double check */
dvb_dmx_swfilter_packet(demux, demux-tsbuf);
demux-tsbufp = 0;
p += j;
}
-while (p count) {
-if (buf[p] == 0x47) {
-if (count - p = 188) {
-dvb_dmx_swfilter_packet(demux, buf[p]);
-p += 188;
-} else {
-i = count - p;
-memcpy(demux-tsbuf, buf[p], i);
-demux-tsbufp = i;
-goto bailout;
-}
-} else
-p++;
+while (likely((p = findNextSyncByte(buf, p, count, pktsize))
count)) {
+if (unlikely(count - p pktsize))
+break;
+
+q = buf[p];
+
+if (unlikely(pktsize == 204 (*q == 0xB8))) {
+memcpy(demux-tsbuf, q, 188);
+demux-tsbuf[0] = 0x47;
+q = demux-tsbuf;
+}
+
Re: TS discontinuity with TT S-2300
27.06.2010 15:37, Oliver Endriss wrote: Hi, On Sunday 27 June 2010 01:05:57 Jaroslav Klaus wrote: Hi, I'm loosing TS packets in my dual CAM premium TT S-2300 card (av7110+saa7146). I use dvblast to select 4 TV channels (~ 16 PIDs) from multiplex, descramble them and stream them to network. Dvblast reports TS discontinuity across all video PIDs only (no audio) usually every 1-3 minutes ~80 packets. But sometimes it goes well for tens of minutes (up to 1-2hours). Everything seems to be ok with 3 TV channels. The full-featured cards are not able to deliver the full bandwidth of a transponder. It is a limitaion of the board design, not a firmware or driver issue. I noticed that saa7146 uses dvb_dmx_swfilter_packets(). I planned using of that function too for Mantis 16K buffer delivery, but I found out that hardware delivers sometimes additional bytes (corrupted partially lost packets?) between the full sized 204 byte packets: Jun 26 16:20:37 koivu kernel: demux: skipped 49 bytes at position 3379 Jun 26 16:20:37 koivu kernel: demux: skipped 18 bytes at position 9868 Jun 26 16:20:37 koivu kernel: demux: skipped 30 bytes at position 10090 Jun 26 16:20:38 koivu kernel: demux: skipped 14 bytes at position 7208 Jun 26 16:20:38 koivu kernel: demux: skipped 114 bytes at position 7426 So dvb_dmx_swfilter(_204)() is needed to skip these unwanted bytes. With simple usage of dvb_dmx_swfilter_packets() the rest of the buffer would have been lost. I wrote a faster version of these functions, also for 188 sized packets today: Re: [PATCH] Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function CU Marko -- 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
Re: [PATCH] Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function
Hi
dvb_dmx_swfilter(_204) performance improvement, with packet recovery.
Remove unnecessary copying of 188 and 204 sized packets within
dvb_dmx_swfilter() and dvb_dmx_swfilter_204() functions.
Recover one packet by backtracking, when there is a short packet
with a correct SYNC byte in the middle.
This patch tries to recover with backtracking one 188 / 204
sized packet if some garbage is found.
The backtracking recovery case, DVB data:
Packet Num, Packet data
10x47 + 99 bytes garbage
20x47 + 187 bytes data
30x47 + 187 bytes data
Recovery algorithm:
First packet 1's 0x47 is found. Process it, advance 188 bytes.
Find next SYNC byte, packet 3 found.
Check packet 3 position - 188: is there a packet?
yes, return packet 2.
Advance 188 bytes, return packet 3.
Jun 27 20:44:59 koivu kernel: demux: backtracked 137 bytes into position 442
Jun 27 20:44:59 koivu kernel: demux: backtracked 28 bytes into position 2250
Jun 27 20:45:02 koivu kernel: demux: backtracked 35 bytes into position 2634
Jun 27 20:45:02 koivu kernel: demux: backtracked 12 bytes into position 1398
Jun 27 20:45:03 koivu kernel: demux: skipped 146 bytes at position 378:
09 93 ...
Jun 27 20:45:03 koivu kernel: demux: backtracked 177 bytes into position 551
Jun 27 20:45:03 koivu kernel: demux: skipped 14 bytes at position 959:
6c 1e ...
Jun 27 20:45:03 koivu kernel: demux: backtracked 191 bytes into position 986
Regards,
Marko Ristola
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c
b/drivers/media/dvb/dvb-core/dvb_demux.c
index 977ddba..b71d77d 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -478,97 +478,96 @@ void dvb_dmx_swfilter_packets(struct dvb_demux
*demux, const u8 *buf,
EXPORT_SYMBOL(dvb_dmx_swfilter_packets);
-void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
+static inline int findNextPacket(const u8 *buf, int pos, size_t count,
+ const int pktsize)
{
-int p = 0, i, j;
+int start = pos, lost;
-spin_lock(demux-lock);
-
-if (demux-tsbufp) {
-i = demux-tsbufp;
-j = 188 - i;
-if (count j) {
-memcpy(demux-tsbuf[i], buf, count);
-demux-tsbufp += count;
-goto bailout;
-}
-memcpy(demux-tsbuf[i], buf, j);
-if (demux-tsbuf[0] == 0x47)
-dvb_dmx_swfilter_packet(demux, demux-tsbuf);
-demux-tsbufp = 0;
-p += j;
+while(likely(pos count)) {
+if (likely(buf[pos] == 0x47 ||
+(pktsize == 204 buf[pos] == 0xB8)))
+break;
+pos++;
}
-while (p count) {
-if (buf[p] == 0x47) {
-if (count - p = 188) {
-dvb_dmx_swfilter_packet(demux, buf[p]);
-p += 188;
-} else {
-i = count - p;
-memcpy(demux-tsbuf, buf[p], i);
-demux-tsbufp = i;
-goto bailout;
-}
-} else
-p++;
+if (unlikely(lost = pos - start)) {
+/* This garbage is part of a valid packet? */
+int backtrack = pos - pktsize;
+if (backtrack = 0 (buf[backtrack] == 0x47 ||
+(pktsize == 204 buf[backtrack] == 0xB8))) {
+/* printk(demux: backtracked %d bytes
+ * \n, start - backtrack); */
+return backtrack;
+}
+/*printk(demux: skipped %d bytes at %d\n, lost, start); */
}
-bailout:
-spin_unlock(demux-lock);
+return pos;
}
-EXPORT_SYMBOL(dvb_dmx_swfilter);
-
-void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf,
size_t count)
+/* Filter all pktsize= 188 or 204 sized packets and skip garbage. */
+static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8
*buf,
+size_t count, const int pktsize)
{
int p = 0, i, j;
-u8 tmppack[188];
+const u8 *q;
spin_lock(demux-lock);
-if (demux-tsbufp) {
+if (likely(demux-tsbufp)) { /* tsbuf[0] is now 0x47. */
i = demux-tsbufp;
-j = 204 - i;
-if (count j) {
+j = pktsize - i;
+if (unlikely(count j)) {
memcpy(demux-tsbuf[i], buf, count);
demux-tsbufp += count;
goto bailout;
}
memcpy(demux-tsbuf[i], buf, j);
-if ((demux-tsbuf[0] == 0x47) || (demux-tsbuf[0] == 0xB8)) {
-memcpy(tmppack, demux-tsbuf, 188);
-if (tmppack[0] == 0xB8)
-tmppack[0] = 0x47;
-dvb_dmx_swfilter_packet(demux, tmppack);
-}
+if (likely(demux-tsbuf[0] == 0x47)) /* double check */
+dvb_dmx_swfilter_packet(demux, demux-tsbuf);
demux-tsbufp = 0;
p += j;
}
-while (p count) {
-if ((buf[p] == 0x47) || (buf[p] == 0xB8)) {
-if (count - p = 204
Re: TS discontinuity with TT S-2300
27.06.2010 21:25, Oliver Endriss wrote: Are you sure that Mantis driver delivers garbage, not partial packets? Please note that the dvb_dmx_swfilter[_204]() routines must accept partial packets, i.e. the rest of the packet will be dellivered with the next call. Yes, I'm sure that the garbage comes from the PCI device itself: garbage should be found at less than 204 byte position otherwise. My latest mailed patch that I use makes sure 0x47 is found at demux-tsbufp[0], if there is at least one spared byte. Otherwise the resulting 188 sized packet would be discarded eventually. It would be good if Mauro would accept my patch for dvb_dmx_swfilter(_204) functions some day. It increases robustness with garbage input, and performance by avoiding unnecessary packet copying. dvb_dmx_swfilter_packets() expects complete TS packets from the driver. The saa7146 does so. It does not deliver garbage data. (Otherwise this would have been noticed a long time ago. The ttpci drivers are the oldest DVB drivers and are very well tested.) I'm very sorry I even questioned ttpci's and the hardware's robustness. I'm delighted to hear that ttpci is so well implemented and tested and so long :) I was very impressed in the quality of the code with dvb_dmx_swfilter_packets(): so simple and efficient. Unfortunately I have had lots of problems with Mantis. But because of that, I've learned DVB driver programming. H.264 works now mostly in one TV channel without crashing Xine with remote vdr. I had to modify streamdev plugin too for vdr. CU Marko -- 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
[PATCH] Avoid unnecessary data copying inside dvb_dmx_swfilter_204() function
Hi Mauro Carvalho Chehab.
I'm sorry for sending this twice. I Don't know yet how to disable HTML
formatting permanently.
This patch does a significant performance improvement (for the function
involved)
by avoiding move of 188 sized packets during 204 to 188 packet conversion.
Also this has a robustness fix at discontinuity points of DMA transfer
blocks.
I have tested that this code works.
While using perf top, I noticed that there were two functions that
used CPU:
One was dvb_dmx_swfilter_packet and the other was dvb_dmx_swfilter_204.
dvb_dmx_swfilter_204 converts the packets from 204 byte format into 188
format.
It does so by copying the packet before modifications to it. I noticed
that it was a very rare case
(usually at most once in 16K data) when the SYNC byte had to be modified
from 0xB8 to 0x47.
So my modification does following things:
* Take a copy of the 188 or 204 bytes only if the packet must be
modified or backed up.
* Backing up last 204 bytes: store only data with the 1st byte as the
SYNC byte (0xB8 or 0x47).
This is the robustness fix: back up a beginning of a real packet, not
some unknown data block.
During my testing, this loses some bytes occassionally because of some
garbage in DMA transferred data:
I measured the number of lost bytes in the inline function that searches
the next SYNC byte.
So the algorithm seems to work, usually there aren't many bursts of
skipped bytes after the stream has stabilized.
I wanted to know how many bytes are skipped and in which positions in
16K block:
Jun 26 16:20:37 koivu kernel: demux: skipped 49 bytes at position 3379
Jun 26 16:20:37 koivu kernel: demux: skipped 18 bytes at position 9868
Jun 26 16:20:37 koivu kernel: demux: skipped 30 bytes at position 10090
Jun 26 16:20:38 koivu kernel: demux: skipped 14 bytes at position 7208
Jun 26 16:20:38 koivu kernel: demux: skipped 114 bytes at position 7426
Jun 26 16:20:38 koivu kernel: demux: skipped 103 bytes at position 12920
Jun 26 16:20:38 koivu kernel: demux: skipped 72 bytes at position 13431
Jun 26 16:20:38 koivu kernel: demux: skipped 1 bytes at position 13707
Jun 26 16:20:45 koivu kernel: demux: skipped 140 bytes at position 8476
Jun 26 16:20:46 koivu kernel: demux: skipped 70 bytes at position 11396
Jun 26 16:20:46 koivu kernel: demux: skipped 8 bytes at position 11670
Jun 26 16:20:46 koivu kernel: demux: skipped 2 bytes at position 11882
I measured performance improvement with Perf top. The CPU was probably
mostly at 1Ghz and sometimes at 2Ghz,
so the actual number of samples changed randomly, but the relative
performance change between
unaltered dvb_dmx_swfilter_packet() and dvb_dmx_swfilter_204() is
significant (unrelated rows removed):
Testing was done with H.264 channel recording, and with Mercurial
version of v4l-dvb branch.
The patch below is done against v4l-dvb GIT version.
Original performance:
samples pcnt
functionDSO
66.00 14.1%
dvb_dmx_swfilter_packet
[dvb_core]
59.00 12.6%
dvb_dmx_swfilter_204
[dvb_core]
11.00 2.4%
mantis_i2c_xfer
[mantis_core]
Performance after patching:
samples pcnt
functionDSO
81.00 29.0%
dvb_dmx_swfilter_packet
[dvb_core]
14.00 5.0%
mantis_i2c_xfer
[mantis_core]
8.00 2.9%
dvb_dmx_swfilter_204
[dvb_core]
So the relative performance has increased by a mangditude for
dvb_dmx_swfilter_204().
This patch is a modified version of the original dvb_dmx_swfilter_204().
I can give this patch for the Linux Kernel with license:
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
* as published by the Free Software Foundation; either version 2.1
* of the License, or (at your option) any later version.
as the original license in that file.
Patch written by: Marko Ristola marko.rist...@kolumbus.fi
Regards,
Marko Ristola
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c
b/drivers/media/dvb/dvb-core/dvb_demux.c
index 977ddba..627d103 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -520,49 +520,60 @@ bailout:
EXPORT_SYMBOL(dvb_dmx_swfilter);
+static inline int find204Sync(const u8 *buf, int pos, size_t count)
+{
+while(likely(pos count)) {
+if (likely(buf[pos] == 0x47 || buf[pos] == 0xB8))
+break;
+pos
Re: [linux-dvb] Cinergy C PCI HD with current v4l-dvb - PATCH for review/pull included
22.06.2010 22:27, Yannick K wrote: On 6/19/10 15:56 , Marko Ristola wrote: Hello all interested in a robust Mantis driver. hi Marko, is there a hg/git branch where your (and possibly Bjorn Morks) recent patches are already in? since i had difficulties to apply some of them. Unfortunately there isn't a private Git tree. I've now figured out that Manu is the maintainer who could take the patches in from me first. Manu has some ongoing work to modify the DMA transfer by using different buffering scheme. So he will do the rework, and the changes will go into jusst.de. It seems that my version won't go in. What hg/Git tree you tried to use to apply our patches? I can send you another patch against that. further more, is there a way/patch for the current tree which gives us a working remote control? Maybe Manu's jusst.de HG tree? I have been trying to get a working H.264 picture, and the remote control isn't so urgent for me. Regards, Marko -- 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
Re: buffer management
23.06.2010 15:43, Steven Toth kirjoitti: Now, on each video interrupt, I know which SG list i need to read from. At this stage i do need to copy the buffers associated with each of the SG lists at once. In this scenario, I don't see how videobuf could be used, while I keep getting this feeling that a simple copy_to_user of the entire buffer could do the whole job in a better way, since the buffers themselves are already managed and initialized already. Am I correct in thinking so, or is it that I am overlooking something ? How to activate DMA transfers only if there is empty space for the DMA transfer? Regards, Marko Manu, SAA7164 suffers from this. If you find a solution I'd love to hear it. Regards, - Steve -- 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
Re: [PATCH] Mantis, hopper: use MODULE_DEVICE_TABLE use the macro to make modules auto-loadable
21.06.2010 19:51, Bjørn Mork wrote: VDR Useruser@gmail.com writes: Instead of copypaste patches from Manu's tree, maybe it's better to just wait for him to push all the changes into v4l. I'm Manu sorry about trying to put patches directly into v4-dvb, if those should go onto your branch first. So, what Manu do you think about my DMA patch or other patches I sent into linux-media mailing list this weekend? Is it okay to generate one interrupt once per 16k bytes, or are the interrupts too rare? At least VDR reads the DVB stream rarely, so I think that it is enough if the DVB card has always something to be delivered when VDR or MythTV asks more data, so if the number of DMA transfer IRQs is twice than the number of times VDR or MythTV asks more data per second, then the context switches are in balance. Understandable DMA RISC programming should be easier to maintain, and it removes the initial garbage from the stream too. How about the tasklet enable/disable patch I wrote? What needs to be done for these patches to be accepted? Best regards, Marko -- 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
[PATCH] Mantis DMA transfer cleanup, fixes data corruption and a race, improves performance.
Hi
Here is another version of the DMA transfer fix.
Please try it. Comments?
The current DMA code with drivers/media/dvb/mantis/mantis_dma.c has user
visible problems:
- about 1500 interrupts per second. One CPU can't copy h.264 data over
the network for me (vdr, streamdev).
- 64K garbage at start of the data stream, part of which goes into User
land application.
The garbage data is partly from the same stream (given twice), and
partly from previous tuned frequency (buffer
uninitialized at DMA start).
- Race condition: Memory copying from DMA buffer is done at the same
time as there is DMA transfer going on to the same bytes.
Can this race cause harware problems?
Current DMA code is not clear:
Current DMA RISC coding creates one DMA transfer per 2048 bytes.
Risc command generating code has mantis-line_count=32.
mantis-last_block and mantis-finished_block iterates over [0 to 16].
These counters are confusing. It takes lots of time to look and debug to
understand
what is happening there even for me :(
Basic example how the 64K garbage generation happens:
At mantis_dma_start, mantis-last_block = mantis-finished_block = 0
1st DMA transfer (2048 bytes) generates interrupt, sets
mantis-finished_block=15.
Tasklet will call mantis_dma_xfer(), which iterates from
mantis-last_block = 0 to mantis-finished_block = 14. Set last_block=15.
2nd DMA transfer of 2048 bytes goes quietly (no interrupt generated),
race with the tasklet above here.
3rd DMA transfer sets mantis-finished_block = 0, mantis_dma_xfer()
copies mantis-finished_block = 15. Set last_block=0.
After this copying continues from block 0, so the content is valid,
although block 0 was already partly copied.
Because the current looping implementation is too hard to understand,
I decided to rewrite it and not give you any small patch that fixes the
issue but nobody else understands it.
This doesn't have the alignment stuff at all that I mentioned in earlier
emails last week.
Basic idea:
Keep DMA buffer of size 64k, but generate interrupts four times, thus
one interrupt per 16k.
Rename mantis-finished_block to be mantis-busy_block, because that
keeps mantis_dma_xfer() simple:
while(mantis-last_block != mantis-busy_block) { do copy, last_block =
(last_block + 1) mod 4.
last_block is thus incremented until last_block == busy_block, which
can't be copied yet.
DMA RISC code generation: outer loop iterates over blocks from 0 to 4.
Inner loop iterates over DMA transfer units from 0 to
MANTIS_DMA_TR_UNITS, each DMA transfer is 2048 bytes.
The interrupt is generated at block[0], DMA unit 0: the block 0 is now
busy :)
mantis-line_bytes, mantis-line_count and mantis-risc_pos
were used only for DMA risc code generation.
Removed them from the structure.
Benefits of this code:
- Removes the 64k garbage issue.
- Remove race condition with concurrent DMA transfer and memory copy.
- Lessen interrupts to about 350 per second (seen by powertop) by
moving 16k bytes per interrupt, instead of 4k per interrupt.
The number of interrupts gets much smaller, and it becomes possible
with single core AMD cpu to
deliver h.264 data over the network from vdr via streamdev.
- Mantis DMA code is more understandable for reviewers and others.
My patch is GPLv2.
The patch is made against GIT linuxtv/master, applies cleanly to
Mercurial v4l-dvb too.
Best regards,
Marko Ristola
diff --git a/drivers/media/dvb/mantis/hopper_cards.c
b/drivers/media/dvb/mantis/hopper_cards.c
index 09e9fc7..3b7e376 100644
--- a/drivers/media/dvb/mantis/hopper_cards.c
+++ b/drivers/media/dvb/mantis/hopper_cards.c
@@ -126,7 +126,7 @@ static irqreturn_t hopper_irq_handler(int irq, void
*dev_id)
}
if (stat MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, %s, label[8]);
-mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
+mantis-busy_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
if (stat MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index cf4b39f..8f048d5 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -134,7 +134,7 @@ static irqreturn_t mantis_irq_handler(int irq, void
*dev_id)
}
if (stat MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, %s, label[8]);
-mantis-finished_block = (stat MANTIS_INT_RISCSTAT) 28;
+mantis-busy_block = (stat MANTIS_INT_RISCSTAT) 28;
tasklet_schedule(mantis-tasklet);
}
if (stat MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_common.h
b/drivers/media/dvb/mantis/mantis_common.h
index d0b645a..23b23b7 100644
--- a/drivers/media/dvb/mantis/mantis_common.h
+++ b/drivers/media/dvb/mantis/mantis_common.h
@@ -122,11 +122,8 @@ struct mantis_pci {
unsigned intnum;
/*RISC Core*/
-u32finished_block;
+u32
[PATCH] Mantis: append tasklet maintenance for DVB stream delivery
Hi
I have a patch that should fix possible memory corruption problems in
Mantis drivers
with tasklets after DMA transfer has been stopped.
In the patch tasklet is enabled only for DVB stream delivery, at end of
DVB stream delivery tasklet is disabled again.
The lack of tasklet maintenance might cause problems with following
schedulings:
1. dvb_dmxdev_filter_stop() calls mantis_dvb_stop_feed: mantis_dma_stop()
2. dvb_dmxdev_filter_stop() calls release_ts_feed() or some other filter
freeing function.
3. tasklet: mantis_dma_xfer calls dvb_dmx_swfilter to copy DMA buffer's
content into freed memory, accessing freed spinlocks.
This case might occur while tuning into another frequency.
Perhaps cdurrhau has found some version from this bug at
http://www.linuxtv.org/pipermail/linux-dvb/2010-June/032688.html:
This is what I get on the remote console via IPMI:
40849.442492] BUG: soft lockup - CPU#2 stuck for 61s! [section
handler:4617]
The following schedule might also be a problem:
1. mantis_core_exit: mantis_dma_stop()
2. mantis_core_exit: mantis_dma_exit().
3. run tasklet (with another CPU?), accessing memory freed by
mantis_dma_exit().
This case might occur with rmmod.
The following patch tries to deactivate the tasklet in mantis_dma_stop
and activate it in mantis_dma_start, thus avoiding these cases.
Marko Ristola
diff --git a/drivers/media/dvb/mantis/mantis_dma.c
b/drivers/media/dvb/mantis/mantis_dma.c
index 46202a4..cf502a6 100644
--- a/drivers/media/dvb/mantis/mantis_dma.c
+++ b/drivers/media/dvb/mantis/mantis_dma.c
@@ -217,12 +217,14 @@ void mantis_dma_start(struct mantis_pci *mantis)
mmwrite(MANTIS_FIFO_EN | MANTIS_DCAP_EN
| MANTIS_RISC_EN, MANTIS_DMA_CTL);
+tasklet_enable(mantis-tasklet);
}
void mantis_dma_stop(struct mantis_pci *mantis)
{
u32 stat = 0, mask = 0;
+tasklet_disable(mantis-tasklet);
stat = mmread(MANTIS_INT_STAT);
mask = mmread(MANTIS_INT_MASK);
dprintk(MANTIS_DEBUG, 1, Mantis Stop DMA engine);
diff --git a/drivers/media/dvb/mantis/mantis_dvb.c
b/drivers/media/dvb/mantis/mantis_dvb.c
index 99d82ee..0c29f01 100644
--- a/drivers/media/dvb/mantis/mantis_dvb.c
+++ b/drivers/media/dvb/mantis/mantis_dvb.c
@@ -216,6 +216,7 @@ int __devinit mantis_dvb_init(struct mantis_pci *mantis)
dvb_net_init(mantis-dvb_adapter, mantis-dvbnet,
mantis-demux.dmx);
tasklet_init(mantis-tasklet, mantis_dma_xfer, (unsigned long)
mantis);
+tasklet_disable_nosync(mantis-tasklet);
if (mantis-hwconfig) {
result = config-frontend_init(mantis, mantis-fe);
if (result 0) {
--
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
Re: [PATCH] Mantis: append tasklet maintenance for DVB stream delivery
20.06.2010 16:51, Bjørn Mork kirjoitti:
Note that mantis_core_exit() is never called. Unless I've missed
something, the drivers/media/dvb/mantis/mantis_core.{h,c} files can
just be deleted. They look like some development leftovers?
I see. mantis_core.ko kernel module exists though.
Maybe the mantis/Makefile references for mantis_core.c, mantis.c and
hopper.c are just some leftovers too.
I moved tasklet_enable/disable calls into mantis_dvb.c where almost all
other tasklet code is located.
So the following reasoning still holds:
1. dvb_dmxdev_filter_stop() calls mantis_dvb_stop_feed: mantis_dma_stop()
2. dvb_dmxdev_filter_stop() calls release_ts_feed() or some other filter
freeing function.
3. tasklet: mantis_dma_xfer calls dvb_dmx_swfilter to copy DMA buffer's
content into freed memory, accessing freed spinlocks.
This case might occur while tuning into another frequency.
Perhaps cdurrhau has found some version from this bug at
http://www.linuxtv.org/pipermail/linux-dvb/2010-June/032688.html:
This is what I get on the remote console via IPMI:
40849.442492] BUG: soft lockup - CPU#2 stuck for 61s! [section
handler:4617]
New reasoning for the patch (same as the one above, but from higher level):
After dvb-core has called mantis-fe-stop_feed(dvbdmxfeed) the last time
(count to zero),
no data should ever be copied with dvb_dmx_swfilter() by a tasklet: the
target structure might be in an unusable state.
Caller of mantis_fe-stop_feed() assumes that feeding is stopped after
stop_feed() has been called, ie. dvb_dmx_swfilter()
isn't running, and won't be called.
There is a risk that dvb_dmx_swfilter() references freed resources
(memory or spinlocks or ???) causing instabilities.
Thus tasklet_disable(mantis-tasklet) must be called inside of
mantis-fe-stop_feed(dvbdmxfeed) when necessary.
Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi
Marko
diff --git a/drivers/media/dvb/mantis/mantis_dvb.c
b/drivers/media/dvb/mantis/mantis_dvb.c
index 99d82ee..a9864f7 100644
--- a/drivers/media/dvb/mantis/mantis_dvb.c
+++ b/drivers/media/dvb/mantis/mantis_dvb.c
@@ -117,6 +117,7 @@ static int mantis_dvb_start_feed(struct
dvb_demux_feed *dvbdmxfeed)
if (mantis-feeds == 1) {
dprintk(MANTIS_DEBUG, 1, mantis start feed dma);
mantis_dma_start(mantis);
+tasklet_enable(mantis-tasklet);
}
return mantis-feeds;
@@ -136,6 +137,7 @@ static int mantis_dvb_stop_feed(struct
dvb_demux_feed *dvbdmxfeed)
mantis-feeds--;
if (mantis-feeds == 0) {
dprintk(MANTIS_DEBUG, 1, mantis stop feed and dma);
+tasklet_disable(mantis-tasklet);
mantis_dma_stop(mantis);
}
@@ -216,6 +218,7 @@ int __devinit mantis_dvb_init(struct mantis_pci *mantis)
dvb_net_init(mantis-dvb_adapter, mantis-dvbnet,
mantis-demux.dmx);
tasklet_init(mantis-tasklet, mantis_dma_xfer, (unsigned long)
mantis);
+tasklet_disable(mantis-tasklet);
if (mantis-hwconfig) {
result = config-frontend_init(mantis, mantis-fe);
if (result 0) {
--
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
Re: Cinergy C PCI HD with current v4l-dvb - PATCH for review/pull included
Hi Bjørn, Thank you for your answer. 19.06.2010 18:34, Bjørn Mork kirjoitti: [ I think this should have been posted to linux-media@vger.kernel.org instead ] Marko Ristolamarko.rist...@kolumbus.fi writes: I am now subscribed into that list too, but maybe I start with the small patches instead of the big patch. Hello all interested in a robust Mantis driver. I have done some Mantis DMA fixes about two to four years ago into v4l-dvb/linux/drivers/dvb/media/mantis/mantis_dma.c, but they were discarded then, but the problems remained. Those fixes helped at least one individual with Cinergy C PCI HD card too by lessening the number of artifacts and making the card usable. I sent the patches for many Finnish people at those years. I find this very interesting. I have two DVB-C cards, where one of them is a Terratec Cinergy C, and I do see the occasional stuck CPUs. Never realized that the mantis driver might be the cause of that. I can't see the spin lock bug because I have now only one CPU. tasklet disable/enable is an easy thing to start bug exclusion. For Mauro and other maintainers - the patch in this email is now against current Mercurial http://linuxtv.org/hg/v4l-dvb branch. What do you think about this patch? Could you apply it? I accept that this code may be added to Linux Kernel, or somewhere else, licensed as GPLv2. This code is fully written by me (Marko Ristola). Signed-off-by: Marko Ristolamarko.rist...@kolumbus.fi I am not Mauro (or other maintainers :-), but I guess you will have a much better chance getting this properly reviewed if you move to git, split up the patch according to your very good 4 point description, and use that description as a commit message for the patches. This will also allow the rest of us to test the effect of each issue independently. Probably Mauro doesn't even be on the old list any more. The patch does two things (other two things are just thoughts, related to robustness): 1. Don't flush 64K garbage at stream start, occurs if mantis_dma_start is run more than once. This fix could be done with altering only about two lines of code by fixing the broken DMA RISC program code. (Bug description: At line[0] RISC IRQ will store line=15 on current code, causing copying of lines between 0 and 14, and next time 15 from previous DVB TS stream (total 64K garbage). After that line=0 contains current stream, no garbage. Bug fix description: RISC program must store at interrupt the active line variable's value, instead of ( (line - 1) mod 16)). This sounds like a real bug. Maybe a candidate for stable as well? Yes, this is a bug and it is easy to fix. On my patch, it seems, that risc_step=0 point could be used for the interrupt instead of risc_step=1, thus making the code a bit easier to understand. 2. Alter DMA transfer so, that each DMA transfer copies only complete packets (either 188 or 204 bytes) with each DMA transfer. The hypothesis is that if a DMA transfer transfers only a part of the 188/204 sized packet, that packet gets corrupted (last bytes of 2048 sized block). My DMA transfers are aligned by 64 bytes in CPU memory (DMA buffer start + multiple of 16x(188 or 204)). I cannot really understand how such corruption could happen, unless there are other bugs here? And if there are, then anything hiding them would be bad... Mantis PCI card receives by radio 188/204 byte sized packets. If there are CRC errors with those, they are counted by the PCI card. If there are zero fatal CRC errors counted, but even then some packets are missing from a recording, there is a serious problem between the radio and the disk storage. The DMA transfer from PCI to CPU DMA buffer and the copying from CPU DMA buffer to generic v4l-dvb code are the only Mantis specific tasks. All other is used so much, that they should be seen by everyone. I'll try to proof it. If I can't prove it, no code is needed to fix it. 3. tasklet_enable/ tasklet_disable for DMA start/stop is commented out on my patch. I think that tasklet enabling maintenance should be done at these points, but I don't know whether the lack of those might cause spin lock failures. Lack of these might cause problems while switching channels (EPG search switches frequency). Sounds reasonable. Did you try it? I didn't have time yet, thus I commented it out. 4. There is some problem with rmmod mantis, do lsmod: Module Size Used by tda100215486 4294967291 modprobe mantis tda100215486 4294967292 mantis So the reference count from mantis to tda10021 gets corrupted at rmmod. I have Fedora 13 kernel, 2.6.33.5-124.fc13.x86_64 FWIW, I do not see this running a Debian kernel (2.6.32-5-amd64) with a backported mantis driver (from 2.6.35-rc1). Module Size Used by tda100214774 1 mantis # rmmod mantis tda100214774 0 # modprobe
Re: Cinergy C PCI HD with current v4l-dvb - PATCH for review/pull included
Proving the DMA transfer alignment problem existence isn't easy. So maybe it doesn't exist. I saw only very rare packet losses on picture quality (didn't test h264 this time). I couldn't do the record test though. Too much work. Marko Ristola -- 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
