Re: [linux-dvb] Cinergy C PCI HD with current v4l-dvb - PATCH for review/pull included

2010-06-24 Thread Marko Ristola
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: [linux-dvb] Cinergy C PCI HD with current v4l-dvb - PATCH for review/pull included

2010-06-22 Thread Yannick K

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.

further more, is there a way/patch for the current tree which gives us a 
working remote control?


regards
yannick
--
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

2010-06-20 Thread Bjørn Mork
Bjørn Mork  writes:
> Marko Ristola  writes:
>
>> 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 mantis
> tda100214774  1 mantis

Sorry, you are of course quite right, Marko: There is such a bug, and it
will in fact cause an oops if this double dereferencing causes a
frontend driver which is in use to be unloaded.  But it will only affect
the frontend drivers actually in use, which is why I didn't see it above
(my card uses tda10023).

The cause of this (and the unwanted loading of a gazillion unused
frontend drivers), is that the mantis driver doesn't use dvb_attach.

I will follow up with a patch to fix this



Bjørn

--
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

2010-06-19 Thread Marko Ristola


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


Re: Cinergy C PCI HD with current v4l-dvb - PATCH for review/pull included

2010-06-19 Thread Marko Ristola

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 Ristola  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 Ristola
 

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 mantis
tda100214774  1 mantis

   
Good 

Re: Cinergy C PCI HD with current v4l-dvb - PATCH for review/pull included

2010-06-19 Thread Bjørn Mork
[ I think this should have been posted to linux-media@vger.kernel.org instead ]

Marko Ristola  writes:

> 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.

> 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 Ristola 

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.

> 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?

> 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...

> 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?


> 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 mantis
tda100214774  1 mantis

> The second modification is the reason for the big number of changes.
> This is also the patch, that isn't easily acceptable, because it is hard
> to proof that the packets do get corrupted at DMA transfer cutting
> points, or that the hard lockups/reboots are caused
> by the DMA transfer misalignment by 188/204 bytes.
> The reasoning for the fix is, that without this alignment change there
> were too many artefacts (VDR DVB
> recordings lost sound after a while, thus VDR recording feature was
> unusable).

This sounds a bit like magic to me.  Any idea why such a change would
make any difference?

> But with these modifications on recordings the sound was
> usable. Picture artefacts weren't so fatal at real time, even
> though on Windows side there were much less picture artefacts.

I haven't really thought much about artefacts, but I do notice a sharp
high pitched sound now and then.  Might be a symptom?

> For historical reasons, I need still to modprobe mantis by hand,
> and then the drivers get probed and loaded. Usua