All of lore.kernel.org
 help / color / mirror / Atom feed
* cx18: Reprise of YUV frame alignment improvements
@ 2009-11-11  4:31 Andy Walls
  2009-11-12  0:38 ` Brandon Jenkins
  2009-11-23  3:04 ` Devin Heitmueller
  0 siblings, 2 replies; 12+ messages in thread
From: Andy Walls @ 2009-11-11  4:31 UTC (permalink / raw)
  To: ivtv-devel, linux-media

OK, here's my second attempt at getting rid of cx18 YUV frame alignment
and tearing issues.

	http://linuxtv.org/hg/~awalls/cx18-yuv2

This change primarily implements full scatter-gather buffer handling
between the cx18 driver and the CX23418 firmware.  That in turn allows
me to set the MDL size to have exactly one YUV frame per MDL transfer
from the encoder to eliminate frame alignment issues, while using very
small buffers that should not have anyone's machine go into a panic.  (I
also tweaked the VBI transfer size while I was at it.)

I'm pretty happy with the results.  I can run this set of streams
simultaneously from one HVR-1600 and have witnessed no new cx18 driver
issues on my machine:

YUV:  mplayer /dev/video32 -demuxer rawvideo -rawvideo w=720:h=480:format=hm12:ntsc
PCM:  aplay -f dat < /dev/video24
VBI:  ~/build/zvbi-0.2.30/test/osc -2
MPEG: mplayer /dev/video0 -cache 8192
ATSC: mplayer dvb://WTTG\ DT -cache 8192

(ALSA or my soundcard couldn't mix together 3 streams of audio out to my
speakers though.  Only 2 streams, PCM and MPEG audio, were audible).

The cx18 default YUV buffer size is now 3 * 33.75 kB = 3 full HM12
macroblock sets that cover 32 screen lines for each macroblock set.  A
full NTSC frame requires 15 * 33.75 kB and a full PAL frame requires 18
* 33.75 kB which is why I picked 3 * 33.75 kB.  I don't anticipate
anyone having problems with this new default YUV buffer size of about
~102 kB, since the current default YUV buffer size is 128 kB.

(BTW the cx18 driver restricts YUV captures to sizes which are a
multiple of 32 lines in height.  I believe the reasoning is that the
software HM12 decoders may not gracefully handle a partial macroblock
set when not a multiple of 32 lines.  This changeset is robust enough to
handle lifting that restriction, if someone has a smart HM12 decoder
that can handle partial macroblocks sensibly.)



Could folks give this cx18 code a test to make sure their primary use
cases didn't break?


Regards,
Andy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-11  4:31 cx18: Reprise of YUV frame alignment improvements Andy Walls
@ 2009-11-12  0:38 ` Brandon Jenkins
  2009-11-12  1:06   ` Andy Walls
  2009-11-23  3:04 ` Devin Heitmueller
  1 sibling, 1 reply; 12+ messages in thread
From: Brandon Jenkins @ 2009-11-12  0:38 UTC (permalink / raw)
  To: Andy Walls; +Cc: ivtv-devel, linux-media

On Tue, Nov 10, 2009 at 11:31 PM, Andy Walls <awalls@radix.net> wrote:
>
> Could folks give this cx18 code a test to make sure their primary use
> cases didn't break?
>
>
> Regards,
> Andy
>
Andy,

I have loaded the drivers (also pulling in Devin's change request too)
and am running with 3 cards and SageTV. I'll let you know if something
pops up, but nothing thus far.

Brandon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-12  0:38 ` Brandon Jenkins
@ 2009-11-12  1:06   ` Andy Walls
  2009-11-12  1:23     ` Brandon Jenkins
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Walls @ 2009-11-12  1:06 UTC (permalink / raw)
  To: Brandon Jenkins; +Cc: ivtv-devel, linux-media

On Wed, 2009-11-11 at 19:38 -0500, Brandon Jenkins wrote:
> On Tue, Nov 10, 2009 at 11:31 PM, Andy Walls <awalls@radix.net> wrote:
> >
> > Could folks give this cx18 code a test to make sure their primary use
> > cases didn't break?
> >
> >
> > Regards,
> > Andy
> >
> Andy,
> 
> I have loaded the drivers (also pulling in Devin's change request too)
> and am running with 3 cards and SageTV. I'll let you know if something
> pops up, but nothing thus far.
> 
> Brandon


Thanks a bunch.  It's nice to have someone with a system capable of
being highly loaded to try to break things. :)

For myself, after 1 day I haven't noticed anything.  Though my normal
use case is simple live Digital TV viewing with MythTV - not terribly
stressing.

I assume you mean the mxl5005s and s5h1409 changes for clear QAM when
you say Devin's change.

Regards,
Andy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-12  1:06   ` Andy Walls
@ 2009-11-12  1:23     ` Brandon Jenkins
  0 siblings, 0 replies; 12+ messages in thread
From: Brandon Jenkins @ 2009-11-12  1:23 UTC (permalink / raw)
  To: Andy Walls; +Cc: ivtv-devel, linux-media

On Wed, Nov 11, 2009 at 8:06 PM, Andy Walls <awalls@radix.net> wrote:
> On Wed, 2009-11-11 at 19:38 -0500, Brandon Jenkins wrote:
>> On Tue, Nov 10, 2009 at 11:31 PM, Andy Walls <awalls@radix.net> wrote:

>
> I assume you mean the mxl5005s and s5h1409 changes for clear QAM when
> you say Devin's change.
>
> Regards,
> Andy
>
>

You are correct sir

Brandon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-11  4:31 cx18: Reprise of YUV frame alignment improvements Andy Walls
  2009-11-12  0:38 ` Brandon Jenkins
@ 2009-11-23  3:04 ` Devin Heitmueller
  2009-11-23 12:12   ` Andy Walls
  1 sibling, 1 reply; 12+ messages in thread
From: Devin Heitmueller @ 2009-11-23  3:04 UTC (permalink / raw)
  To: Andy Walls; +Cc: ivtv-devel, linux-media

On Tue, Nov 10, 2009 at 11:31 PM, Andy Walls <awalls@radix.net> wrote:
> OK, here's my second attempt at getting rid of cx18 YUV frame alignment
> and tearing issues.
>
>        http://linuxtv.org/hg/~awalls/cx18-yuv2

Hi Andy,

I did some testing of your tree, using the following command

mplayer /dev/video32 -demuxer rawvideo -rawvideo w=720:h=480:format=hm12:ntsc

and then in parallel run a series of make commands of the v4l-dvb tree

make -j2 && make unload && make -j2 && make unload && make -j2 && make
unload && make -j2 && make unload

I was definitely seeing the corruption by doing this test before your
patches (both frame alignment and colorspace problems as PCI frames
were being dropped).  After your change, I no longer see those
problems.  The picture never became misaligned.  However, it would
appear that some sort of regression may have been introduced with the
buffer handling.

I was seeing a continuous reporting of the following in dmesg, even
*after* I stopped generating the load by running the make commands.

[ 5175.703811] cx18-0: Could not find MDL 106 for stream encoder YUV
[ 5175.737380] cx18-0: Could not find MDL 111 for stream encoder YUV
[ 5175.804317] cx18-0: Skipped encoder YUV, MDL 96, 3 times - it must
have dropped out of rotation
[ 5175.804324] cx18-0: Skipped encoder YUV, MDL 101, 3 times - it must
have dropped out of rotation
[ 5175.904500] cx18-0: Skipped encoder YUV, MDL 96, 2 times - it must
have dropped out of rotation
[ 5176.204507] cx18-0: Skipped encoder YUV, MDL 101, 1 times - it must
have dropped out of rotation
[ 5176.204513] cx18-0: Skipped encoder YUV, MDL 96, 1 times - it must
have dropped out of rotation
[ 5176.204518] cx18-0: Could not find MDL 111 for stream encoder YUV

I would expect to see frame drops while the system was under high
load, but I would expect that the errors would stop once the load fell
back to something reasonable.  However, they continue to accumulate
even after the make commands stop and the only thing running on the
system is mplayer (with a CPU load of around 10%).

I think this tree is definitely on the right track, but it looks like
some edge case has been missed.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-23  3:04 ` Devin Heitmueller
@ 2009-11-23 12:12   ` Andy Walls
  2009-11-23 12:32     ` Andy Walls
  2009-11-23 17:09     ` Devin Heitmueller
  0 siblings, 2 replies; 12+ messages in thread
From: Andy Walls @ 2009-11-23 12:12 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: ivtv-devel, linux-media

On Sun, 2009-11-22 at 22:04 -0500, Devin Heitmueller wrote:
> On Tue, Nov 10, 2009 at 11:31 PM, Andy Walls <awalls@radix.net> wrote:
> > OK, here's my second attempt at getting rid of cx18 YUV frame alignment
> > and tearing issues.
> >
> >        http://linuxtv.org/hg/~awalls/cx18-yuv2
> 
> Hi Andy,
> 
> I did some testing of your tree, using the following command
> 
> mplayer /dev/video32 -demuxer rawvideo -rawvideo w=720:h=480:format=hm12:ntsc
> 
> and then in parallel run a series of make commands of the v4l-dvb tree
> 
> make -j2 && make unload && make -j2 && make unload && make -j2 && make
> unload && make -j2 && make unload
> 
> I was definitely seeing the corruption by doing this test before your
> patches (both frame alignment and colorspace problems as PCI frames
> were being dropped).  After your change, I no longer see those
> problems.  The picture never became misaligned.

Great.  Thanks for the test.


>  However, it would
> appear that some sort of regression may have been introduced with the
> buffer handling.
> 
> I was seeing a continuous reporting of the following in dmesg, even
> *after* I stopped generating the load by running the make commands.
> 
> [ 5175.703811] cx18-0: Could not find MDL 106 for stream encoder YUV
> [ 5175.737380] cx18-0: Could not find MDL 111 for stream encoder YUV
> [ 5175.804317] cx18-0: Skipped encoder YUV, MDL 96, 3 times - it must
> have dropped out of rotation
> [ 5175.804324] cx18-0: Skipped encoder YUV, MDL 101, 3 times - it must
> have dropped out of rotation
> [ 5175.904500] cx18-0: Skipped encoder YUV, MDL 96, 2 times - it must
> have dropped out of rotation
> [ 5176.204507] cx18-0: Skipped encoder YUV, MDL 101, 1 times - it must
> have dropped out of rotation
> [ 5176.204513] cx18-0: Skipped encoder YUV, MDL 96, 1 times - it must
> have dropped out of rotation
> [ 5176.204518] cx18-0: Could not find MDL 111 for stream encoder YUV

Congratulations, you're seeing my buffer notification consistency check
and sweep-up code in action.

In the early days of cx18 maintenance by me, the driver would stop
"capturing" a stream after anywhere from an hour to an hour and a half -
black screen in MythTV.  The original (current?) problem had a few
components:

1. There is only *one* CPU2EPU mailbox and all DMA_DONE notifications
come through it.

2. The CX23418 firmware does not wait long, at all, for you to pick up
and acknowledge the CPU2EPU mailbox.  It is a shorter window when you
have multiple streams running.

3. If you cleanly miss an MDL notification, you don't know which MDL you
missed and you don't know how many bytes were used in it.  You drop it.

4. If you get a half written mailbox, like in your MDL 111 message
above, then you have a mailbox consistency problem which is logged, but
you also drop the MDL.

5. If you don't give an MDL back to the firmware, it never uses it
again.  That's why you see the sweep-up log messages.  As soon as an MDL
is skipped *on the order of the depth* of q_busy times, when looking for
the currently DMA_DONE'd MDL, that skipped MDL must have been dropped.
It is picked up and put back into rotation then.


I will note that skipping an MDL 1 time and sweeping it up indicates the
CX23418 firmware (q_busy) doesn't have a lot of MDLs to work with for
that stream.  You need to devote more memory to that stream or have the
application read them off faster (so the MDL goes from q_full to q_free
to q_busy).



> I would expect to see frame drops while the system was under high
> load, but I would expect that the errors would stop once the load fell
> back to something reasonable.  However, they continue to accumulate
> even after the make commands stop and the only thing running on the
> system is mplayer (with a CPU load of around 10%).

You likely have:

1. a system-level interrupt handler latency problem

and/or

2. the cx18-NN-out/M workrer threads aren't being woken up often enough
to give MDL's back to the CX23418 firmware fast enough.


For #1, if there is a linux driver sharing the CX23418 interrupt line
(as shown by cat /proc/interrupts) then try unloading that driver,
moving the CX23418 to another PCI slot, or somehow else keeping some
other linux device driver from masking the CX23418's IRQ line for too
long.  The ahci disk controller driver is a known culprit with a time
consuming error path in the top half of its IRQ handler.

The easy solution to #2 is give enough memory for a few more MDLs for
that stream with module parameters.


> I think this tree is definitely on the right track, but it looks like
> some edge case has been missed.

What you see is normal.  I can take a look at things, but it's generally
a system level issue.  One thing that can be done in the cx18 driver is
to optimize the paths called by the out_work_handler, so that MDLs get
back to the firmware with a minimum of delay. 

It's never been a big deal, with lots of MDLs for a stream, to have one
or two MDLs tied up.  With YUV only having very few MDLs, having an MDL
tied up, not being given back to the firmware promptly, could ba a
problem.

Regards,
Andy

> Devin
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-23 12:12   ` Andy Walls
@ 2009-11-23 12:32     ` Andy Walls
  2009-11-23 17:09     ` Devin Heitmueller
  1 sibling, 0 replies; 12+ messages in thread
From: Andy Walls @ 2009-11-23 12:32 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: ivtv-devel, linux-media

On Mon, 2009-11-23 at 07:12 -0500, Andy Walls wrote:
> On Sun, 2009-11-22 at 22:04 -0500, Devin Heitmueller wrote:
> > On Tue, Nov 10, 2009 at 11:31 PM, Andy Walls <awalls@radix.net> wrote:
> > > OK, here's my second attempt at getting rid of cx18 YUV frame alignment
> > > and tearing issues.
> > >
> > >        http://linuxtv.org/hg/~awalls/cx18-yuv2
> > 
> > Hi Andy,
> > 
> > I did some testing of your tree, using the following command
> > 
> > mplayer /dev/video32 -demuxer rawvideo -rawvideo w=720:h=480:format=hm12:ntsc
> > 
> > and then in parallel run a series of make commands of the v4l-dvb tree
> > 
> > make -j2 && make unload && make -j2 && make unload && make -j2 && make
> > unload && make -j2 && make unload
> > 
> > I was definitely seeing the corruption by doing this test before your
> > patches (both frame alignment and colorspace problems as PCI frames
> > were being dropped).  After your change, I no longer see those
> > problems.  The picture never became misaligned.
> 
> Great.  Thanks for the test.
> 
> 
> >  However, it would
> > appear that some sort of regression may have been introduced with the
> > buffer handling.
> > 
> > I was seeing a continuous reporting of the following in dmesg, even
> > *after* I stopped generating the load by running the make commands.
> > 
> > [ 5175.703811] cx18-0: Could not find MDL 106 for stream encoder YUV
> > [ 5175.737380] cx18-0: Could not find MDL 111 for stream encoder YUV
> > [ 5175.804317] cx18-0: Skipped encoder YUV, MDL 96, 3 times - it must
> > have dropped out of rotation
> > [ 5175.804324] cx18-0: Skipped encoder YUV, MDL 101, 3 times - it must
> > have dropped out of rotation
> > [ 5175.904500] cx18-0: Skipped encoder YUV, MDL 96, 2 times - it must
> > have dropped out of rotation
> > [ 5176.204507] cx18-0: Skipped encoder YUV, MDL 101, 1 times - it must
> > have dropped out of rotation
> > [ 5176.204513] cx18-0: Skipped encoder YUV, MDL 96, 1 times - it must
> > have dropped out of rotation
> > [ 5176.204518] cx18-0: Could not find MDL 111 for stream encoder YUV
> 
> Congratulations, you're seeing my buffer notification consistency check
> and sweep-up code in action.
> 
> In the early days of cx18 maintenance by me, the driver would stop
> "capturing" a stream after anywhere from an hour to an hour and a half -
> black screen in MythTV.  The original (current?) problem had a few
> components:
> 
> 1. There is only *one* CPU2EPU mailbox and all DMA_DONE notifications
> come through it.
> 
> 2. The CX23418 firmware does not wait long, at all, for you to pick up
> and acknowledge the CPU2EPU mailbox.  It is a shorter window when you
> have multiple streams running.
> 
> 3. If you cleanly miss an MDL notification, you don't know which MDL you
> missed and you don't know how many bytes were used in it.  You drop it.
> 
> 4. If you get a half written mailbox, like in your MDL 111 message
> above, then you have a mailbox consistency problem which is logged, but
> you also drop the MDL.
> 
> 5. If you don't give an MDL back to the firmware, it never uses it
> again.  That's why you see the sweep-up log messages.  As soon as an MDL
> is skipped *on the order of the depth* of q_busy times, when looking for
> the currently DMA_DONE'd MDL, that skipped MDL must have been dropped.
> It is picked up and put back into rotation then.
> 
> 
> I will note that skipping an MDL 1 time and sweeping it up indicates the
> CX23418 firmware (q_busy) doesn't have a lot of MDLs to work with for
> that stream.  You need to devote more memory to that stream or have the
> application read them off faster (so the MDL goes from q_full to q_free
> to q_busy).
> 
> 
> 
> > I would expect to see frame drops while the system was under high
> > load, but I would expect that the errors would stop once the load fell
> > back to something reasonable.  However, they continue to accumulate
> > even after the make commands stop and the only thing running on the
> > system is mplayer (with a CPU load of around 10%).
> 
> You likely have:
> 
> 1. a system-level interrupt handler latency problem
> 
> and/or
> 
> 2. the cx18-NN-out/M workrer threads aren't being woken up often enough
> to give MDL's back to the CX23418 firmware fast enough.

One other possibility:

3. Once mplayer got behind, it stayed behind to render frames on a
smooth timeline.  That means more MDLs are intentionally being held on
q_full by the application.


> For #1, if there is a linux driver sharing the CX23418 interrupt line
> (as shown by cat /proc/interrupts) then try unloading that driver,
> moving the CX23418 to another PCI slot, or somehow else keeping some
> other linux device driver from masking the CX23418's IRQ line for too
> long.  The ahci disk controller driver is a known culprit with a time
> consuming error path in the top half of its IRQ handler.
> 
> The easy solution to #2 is give enough memory for a few more MDLs for
> that stream with module parameters.


For number 3, for the YUV stream, we can make the assumption we can
steal the 2nd MDL from the front of q_full and give it back to q_busy in
low depth of q_busy situations.  That will result in a forceably dropped
old frame for the application.

Regards,
Andy

> > I think this tree is definitely on the right track, but it looks like
> > some edge case has been missed.
> 
> What you see is normal.  I can take a look at things, but it's generally
> a system level issue.  One thing that can be done in the cx18 driver is
> to optimize the paths called by the out_work_handler, so that MDLs get
> back to the firmware with a minimum of delay. 
> 
> It's never been a big deal, with lots of MDLs for a stream, to have one
> or two MDLs tied up.  With YUV only having very few MDLs, having an MDL
> tied up, not being given back to the firmware promptly, could ba a
> problem.
> 
> Regards,
> Andy
> 
> > Devin
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-23 12:12   ` Andy Walls
  2009-11-23 12:32     ` Andy Walls
@ 2009-11-23 17:09     ` Devin Heitmueller
  2009-11-24  1:49       ` Andy Walls
  1 sibling, 1 reply; 12+ messages in thread
From: Devin Heitmueller @ 2009-11-23 17:09 UTC (permalink / raw)
  To: Andy Walls; +Cc: ivtv-devel, linux-media

On Mon, Nov 23, 2009 at 7:12 AM, Andy Walls <awalls@radix.net> wrote:
> 5. If you don't give an MDL back to the firmware, it never uses it
> again.  That's why you see the sweep-up log messages.  As soon as an MDL
> is skipped *on the order of the depth* of q_busy times, when looking for
> the currently DMA_DONE'd MDL, that skipped MDL must have been dropped.
> It is picked up and put back into rotation then.

Perhaps I am misinterpreting the definition of "sweep-up" in this
context.  Don't the buffers get forcefully returned to the pool at
that point?  If so, why would I see the same error over and over long
after the CPU utilization has dropped back to a reasonable level.

I feel like I must be missing something here.

1.  CPU load goes up (ok)
2.  Packets start to get dropped (expected)
3.  CPU load goes back down (ok)
4.  Packets continue to get dropped and never recycled, even after
minutes of virtually no CPU load?

I can totally appreciate the notion that the video would look choppy
when the system is otherwise under high load, but my expectation would
be that once the load drops back to 0, the video should look fine
(perhaps with some small window of time where it is still recovering).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-23 17:09     ` Devin Heitmueller
@ 2009-11-24  1:49       ` Andy Walls
  2009-11-24 17:57         ` Devin Heitmueller
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Walls @ 2009-11-24  1:49 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: ivtv-devel, linux-media

On Mon, 2009-11-23 at 12:09 -0500, Devin Heitmueller wrote:
> On Mon, Nov 23, 2009 at 7:12 AM, Andy Walls <awalls@radix.net> wrote:
> > 5. If you don't give an MDL back to the firmware, it never uses it
> > again.  That's why you see the sweep-up log messages.  As soon as an MDL
> > is skipped *on the order of the depth* of q_busy times, when looking for
> > the currently DMA_DONE'd MDL, that skipped MDL must have been dropped.
> > It is picked up and put back into rotation then.
> 
> Perhaps I am misinterpreting the definition of "sweep-up" in this
> context.  Don't the buffers get forcefully returned to the pool at
> that point?  

You've got it right.


> If so, why would I see the same error over and over long
> after the CPU utilization has dropped back to a reasonable level.
> 
> I feel like I must be missing something here.
> 
> 1.  CPU load goes up (ok)
> 2.  Packets start to get dropped (expected)
> 3.  CPU load goes back down (ok)
> 4.  Packets continue to get dropped and never recycled, even after
> minutes of virtually no CPU load?
> 
> I can totally appreciate the notion that the video would look choppy
> when the system is otherwise under high load, but my expectation would
> be that once the load drops back to 0, the video should look fine
> (perhaps with some small window of time where it is still recovering).


OK the messages are coming from the sweep up implementation, let's
assume it's not working right (which is reasonable to me).

The sweep up algorithm relies on an assumption.

Assumption: The firmware uses MDL on a FIFO basis based on the order in
which we submitted the MDLs to the firmware.  Thus, the order of MDLs in
the q_busy linked list should match how the firmware returns them.

Given that the decision to perform sweep up is based on the absolute
current depth of q_busy (was the buffer skipped q_busy.depth - 1 times
or more?), it turns out that

a. For large numbers of MDLs on q_busy, the assumption needs to only be
approximately true.

b. For very small numbers of MDLs on q_busy (i.e. 2), the assumption
needs to be strictly true or errant sweep-ups happen.

So I suspect for the case of the CX23418 firmware only having 1 MDL and
use giving it another MDL, the CX23418 might use the one we just gave it
first - violating the assumption amd causing errant sweep ups.


The fix is simple: don't sweep up a skipped buffer that meets the
current 

	skipped > = (q_busy.depth -1)

criteria in the case of

	q_full.depth > q_free.depth + qbusy.depth

Which says if we've got a lot of MDLs tied up waiting for the
application to read them, don't both sweeping up potentially lost
buffers until the q_busy.depth increases.  Since "lost" MDLs stay on
q_busy and are counted in q_busy.depth, this will always end up
returning MDLs to the firmware as the application reads data and returns
MDLs.



Of course that's all speculation about the problem.  If you could
reproduce the condition and then

# echo 271 > /sys/modules/cx18/parameters/debug

to record the sequence of CX18_DE_SET_MDLs and DMA_DONE sequence numbers
for the YUIV stream, it might provide conifrmation of what is going on.
271 is high volume messages for info, warning, mailbox, and dma debug
messages.  It will write a lot to your logs.

Regards,
Andy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-24  1:49       ` Andy Walls
@ 2009-11-24 17:57         ` Devin Heitmueller
  2009-11-24 23:39           ` Andy Walls
  0 siblings, 1 reply; 12+ messages in thread
From: Devin Heitmueller @ 2009-11-24 17:57 UTC (permalink / raw)
  To: Andy Walls; +Cc: ivtv-devel, linux-media

On Mon, Nov 23, 2009 at 8:49 PM, Andy Walls <awalls@radix.net> wrote:
> Of course that's all speculation about the problem.  If you could
> reproduce the condition and then
>
> # echo 271 > /sys/modules/cx18/parameters/debug

Hi Andy,

Thanks for the additional info.  I had to tear down my HVR-1600 test
rig to finish the em28xx PAL support (using a PVR-350 and the CD of
PAL VBI samples you were very kind in sending me), but I should be
able to get back to this early next week.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-24 17:57         ` Devin Heitmueller
@ 2009-11-24 23:39           ` Andy Walls
  2009-11-25 16:43             ` Devin Heitmueller
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Walls @ 2009-11-24 23:39 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: ivtv-devel, linux-media

On Tue, 2009-11-24 at 12:57 -0500, Devin Heitmueller wrote:
> On Mon, Nov 23, 2009 at 8:49 PM, Andy Walls <awalls@radix.net> wrote:
> > Of course that's all speculation about the problem.  If you could
> > reproduce the condition and then
> >
> > # echo 271 > /sys/modules/cx18/parameters/debug
> 
> Hi Andy,
> 
> Thanks for the additional info.  I had to tear down my HVR-1600 test
> rig to finish the em28xx PAL support

So I read -- Looks good!

>  (using a PVR-350 and the CD of
> PAL VBI samples you were very kind in sending me), but I should be
> able to get back to this early next week.

Take your time.  I've got plenty of other problems. ;)


BTW, I did a quick skim of your cx18-alsa stuff.  I noticed two things:

1.  A memory leak in an error path:

http://www.kernellabs.com/hg/~dheitmueller/hvr-1600-alsa-2/rev/cb267593943f#l85


2.  Technically open_id should probably be changed to an atomic type and
atomic_inc() used:

http://www.kernellabs.com/hg/~dheitmueller/hvr-1600-alsa-2/rev/cb267593943f#l80

Under normal use it will likely never matter though, but perhaps someone
could use it as a possible exploit.



I'll try and give the code a good review and test sometime this weekend.
I just wanted to let you know about those minor bugs before I forgot.

Regards,
Andy

> Devin



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: cx18: Reprise of YUV frame alignment improvements
  2009-11-24 23:39           ` Andy Walls
@ 2009-11-25 16:43             ` Devin Heitmueller
  0 siblings, 0 replies; 12+ messages in thread
From: Devin Heitmueller @ 2009-11-25 16:43 UTC (permalink / raw)
  To: Andy Walls; +Cc: ivtv-devel, linux-media

On Tue, Nov 24, 2009 at 6:39 PM, Andy Walls <awalls@radix.net> wrote:
> BTW, I did a quick skim of your cx18-alsa stuff.  I noticed two things:
>
> 1.  A memory leak in an error path:
>
> http://www.kernellabs.com/hg/~dheitmueller/hvr-1600-alsa-2/rev/cb267593943f#l85
>
>
> 2.  Technically open_id should probably be changed to an atomic type and
> atomic_inc() used:
>
> http://www.kernellabs.com/hg/~dheitmueller/hvr-1600-alsa-2/rev/cb267593943f#l80
>
> Under normal use it will likely never matter though, but perhaps someone
> could use it as a possible exploit.
>
>
>
> I'll try and give the code a good review and test sometime this weekend.
> I just wanted to let you know about those minor bugs before I forgot.

Thanks for taking the time to review.  I will make both of those
changes early next week.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2009-11-25 16:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-11  4:31 cx18: Reprise of YUV frame alignment improvements Andy Walls
2009-11-12  0:38 ` Brandon Jenkins
2009-11-12  1:06   ` Andy Walls
2009-11-12  1:23     ` Brandon Jenkins
2009-11-23  3:04 ` Devin Heitmueller
2009-11-23 12:12   ` Andy Walls
2009-11-23 12:32     ` Andy Walls
2009-11-23 17:09     ` Devin Heitmueller
2009-11-24  1:49       ` Andy Walls
2009-11-24 17:57         ` Devin Heitmueller
2009-11-24 23:39           ` Andy Walls
2009-11-25 16:43             ` Devin Heitmueller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.