Re: [PATCH] saa7134: Add pm_qos_request to fix video corruption

2012-12-04 Thread Mauro Carvalho Chehab
Em Mon, 29 Oct 2012 14:11 +
Simon Farnsworth simon.farnswo...@onelan.co.uk escreveu:

 On Monday 29 October 2012 09:32:27 Andy Walls wrote:
  On Mon, 2012-10-29 at 13:02 +, Simon Farnsworth wrote:
   It will affect other drivers as well; the basic cause is that modern chips
   can enter a package deep sleep state that affects both CPU speed and 
   latency
   to start of DMA. On older systems, this couldn't happen - the Northbridge
   kept running at all times, and DMA latencies were low.
   
   However, on the Intel Sandybridge system I'm testing with, the maximum 
   wake
   latency from deep sleep is 109 microseconds; the SAA7134's internal FIFO 
   can't
   hold onto data for that long, and overflows, resulting in the corruption 
   I'm
   seeing. The pm QoS request fixes this for me, by telling the PM subsystem
   that the SAA7134 cannot cope with a long latency on the first write of a 
   DMA
   transfer.
   
   Now, some media bridges (like the ones driven by the cx18 driver) can cope
   with very high latency before the beginning of a DMA burst. Andy Walls has
   worked on the cx18 driver to cope in this situation, so it doesn't fail 
   even
   with the 109 microsecond DMA latency we have on Sandybridge.
  
  Well if brdige wake-up DMA latency is the problem, it is alos the case
  that the CX23418 has a *lot* of on board memory with which to collect
  video and compress it.  (And lets face it, the CX23418 is an SoC with
  two ARM cores and a lot of dedicated external memory, as opposed to the
  SAA7134 with 1 kB of FIFO.)   That hardware helps quite a bit, if the
  PCI bus is slow to wake up.
  
  I found a SAA7134 sheet for you:
  
  http://www.nxp.com/documents/data_sheet/SAA7134HL.pdf
  
  Section 6.4.3 has a short description of the behaviour when the FIFO
  overflows.
 
 That's a good description of what I'm seeing, so fits with the idea that it's
 FIFO underflow.
  
  But this sheet (close enough):
  
  http://www.nxp.com/documents/data_sheet/SAA7133HL.pdf
  
  Has much nicer examples of the programmed levels of the FIFO in section
  6.4.3.  That 1 kB is for everything: raw VBI, Y, U, V, MPEG, and Audio.
  So you're lucky if one full line of video fits in the FIFO.
  
 And that datasheet suggests that my 31 usec request is too high; in the
 fastidious configuration, I'd need a latency of 22 usec, not 31.
 
 Does anyone have register-level documentation for the SAA7134, to confirm the
 maximum tolerated latency with the FIFO configuration Linux uses?
 
   Others, like the SAA7134, just don't have that much buffering, and we need
   to ask the pm core to cope with it. I suspect that most old drivers will 
   need
   updating if anyone wants to use them with modern systems; either they'll 
   have
   an architecture like the cx18 series, and the type of work Andy has done 
   will
   fix the problem, or they'll behave like the saa7134, and need a pm_qos 
   request
   to ensure that the CPU package (and thus memory controller) stay awake.
  
  Unless the chip has a lot of internal memory and processing resources, I
  suspect a pm_qos solution is needed to ensure the PCI bus responds in
  time.
  
  This is a system level issue though.  Having the drivers decide what QoS
  they need in the absences of total system requirements, is the right
  thing for the home user.  It might not be very friendly for a
  professional solution where someone is trying to tune the use of the
  system IO bandwidth and CPU resources.
  
  The ivtv driver and cx18 driver unconditionally bumping up their PCI
  latency timer to 64 cycles minimum always bugged me: drivers shouldn't
  be deciding QoS in a vaccuum.  But, then again, user complaints went
  away and the 64 PCI cycles seemed to be a minimum QoS that everyone
  needed.  Also both drivers have a ivtv/cx18_pci_latency module option to
  override the behaviour anyway.
  
 So, one trick that the pm_qos request infrastructure gives us is that I only
 request reduced latency when we are actually streaming. It's up to the pm core
 to determine what that means - e.g. keep CPU awake, program chipset registers,
 ignore the request completely.
 
 The other part of this is that I'm flagging my QoS requirements; if the wakeup
 latency is higher than I'm requesting, I'll fail - that is clearly wrong. On
 the other hand, if you have reasons to want lower wakeup latencies, the core
 will combine all requests (both userspace and kernelspace), and choose the
 minimum. If you're sophisticated enough to accept the problems involved in not
 waking up in time to service DMA requests, you're probably also up to the task
 of changing the kernel slightly to not request lower latencies.

Simon,

If I understood your above comments well, you'll be submitting us a version 2
of this patch changing the latency to a value closer to 22 usec, after testing 
it.

So, I'll mark this patch as changes-requested and I'll wait for your next one.


Cheers,
Mauro
--
To unsubscribe from this 

Re: [PATCH] saa7134: Add pm_qos_request to fix video corruption

2012-10-30 Thread Alan Stern
On Mon, 29 Oct 2012, Simon Farnsworth wrote:

 On Monday 29 October 2012 13:44:45 Mauro Carvalho Chehab wrote:
  Thanks for digging into it and getting more data. Do you know if this change
  it also needed with USB devices that do DMA (isoc and/or bulk)? Or the USB
  core already handles that?
  
 I'm not a huge expert - the linux-pm list (cc'd) will have people around who
 know more.
 
 If I've understood correctly, though, the USB core should take care of pm_qos
 requests if they're needed for the hardware; remember that if the HCD has
 enough buffering, there's no need for a pm_qos request.

The USB core is not PM-QOS aware.  It relies on the PM core to tell it 
when devices may safely be runtime-suspended.

Alan Stern

--
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] saa7134: Add pm_qos_request to fix video corruption

2012-10-29 Thread Simon Farnsworth
On Monday 22 October 2012 12:50:11 Simon Farnsworth wrote:
 The SAA7134 appears to have trouble buffering more than one line of video
 when doing DMA. Rather than try to fix the driver to cope (as has been done
 by Andy Walls for the cx18 driver), put in a pm_qos_request to limit deep
 sleep exit latencies.
 
 The visible effect of not having this is that seemingly random lines are
 only partly transferred - if you feed in a static image, you see a portion
 of the image flicker into place.
 
 Signed-off-by: Simon Farnsworth simon.farnswo...@onelan.co.uk

Hello Mauro,

I've just noticed that I forgot to CC you in on this patch I sent last week - 
Patchwork grabbed it at https://patchwork.kernel.org/patch/1625311/ but if you 
want me to resend it so that you've got it in a mailbox for consideration, 
just let me know.
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH] saa7134: Add pm_qos_request to fix video corruption

2012-10-29 Thread Mauro Carvalho Chehab
Em Mon, 29 Oct 2012 11:25:38 +
Simon Farnsworth simon.farnswo...@onelan.co.uk escreveu:

 On Monday 22 October 2012 12:50:11 Simon Farnsworth wrote:
  The SAA7134 appears to have trouble buffering more than one line of video
  when doing DMA. Rather than try to fix the driver to cope (as has been done
  by Andy Walls for the cx18 driver), put in a pm_qos_request to limit deep
  sleep exit latencies.
  
  The visible effect of not having this is that seemingly random lines are
  only partly transferred - if you feed in a static image, you see a portion
  of the image flicker into place.
  
  Signed-off-by: Simon Farnsworth simon.farnswo...@onelan.co.uk
 
 Hello Mauro,
 
 I've just noticed that I forgot to CC you in on this patch I sent last week - 
 Patchwork grabbed it at https://patchwork.kernel.org/patch/1625311/ but if 
 you 
 want me to resend it so that you've got it in a mailbox for consideration, 
 just let me know.

I prefer if you don't c/c me on that ;) Patchwork is the main source that I use
on my patch reviews.

Btw, I saw your patch yesterday (and skipped it, for now), as I never played
with those pm QoS stuff before, nor I ever noticed anything like what you
reported on saa7134 (but I can't even remember the last time I tested something
on a saa7134 board ;) - so, it can be some new bug).

So, I'll postpone its review to when I have some time to actually test it
especially as the same issue might also be happening on other drivers.

Cheers,
Mauro
--
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] saa7134: Add pm_qos_request to fix video corruption

2012-10-29 Thread Simon Farnsworth
On Monday 29 October 2012 09:58:17 Mauro Carvalho Chehab wrote:
 I prefer if you don't c/c me on that ;) Patchwork is the main source that I 
 use
 on my patch reviews.
 
Noted.

 Btw, I saw your patch yesterday (and skipped it, for now), as I never played
 with those pm QoS stuff before, nor I ever noticed anything like what you
 reported on saa7134 (but I can't even remember the last time I tested 
 something
 on a saa7134 board ;) - so, it can be some new bug).
 
 So, I'll postpone its review to when I have some time to actually test it
 especially as the same issue might also be happening on other drivers.
 
It will affect other drivers as well; the basic cause is that modern chips
can enter a package deep sleep state that affects both CPU speed and latency
to start of DMA. On older systems, this couldn't happen - the Northbridge
kept running at all times, and DMA latencies were low.

However, on the Intel Sandybridge system I'm testing with, the maximum wake
latency from deep sleep is 109 microseconds; the SAA7134's internal FIFO can't
hold onto data for that long, and overflows, resulting in the corruption I'm
seeing. The pm QoS request fixes this for me, by telling the PM subsystem
that the SAA7134 cannot cope with a long latency on the first write of a DMA
transfer.

Now, some media bridges (like the ones driven by the cx18 driver) can cope
with very high latency before the beginning of a DMA burst. Andy Walls has
worked on the cx18 driver to cope in this situation, so it doesn't fail even
with the 109 microsecond DMA latency we have on Sandybridge.

Others, like the SAA7134, just don't have that much buffering, and we need
to ask the pm core to cope with it. I suspect that most old drivers will need
updating if anyone wants to use them with modern systems; either they'll have
an architecture like the cx18 series, and the type of work Andy has done will
fix the problem, or they'll behave like the saa7134, and need a pm_qos request
to ensure that the CPU package (and thus memory controller) stay awake.
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH] saa7134: Add pm_qos_request to fix video corruption

2012-10-29 Thread Andy Walls
On Mon, 2012-10-29 at 13:02 +, Simon Farnsworth wrote:
 On Monday 29 October 2012 09:58:17 Mauro Carvalho Chehab wrote:
  I prefer if you don't c/c me on that ;) Patchwork is the main source that I 
  use
  on my patch reviews.
  
 Noted.
 
  Btw, I saw your patch yesterday (and skipped it, for now), as I never played
  with those pm QoS stuff before, nor I ever noticed anything like what you
  reported on saa7134 (but I can't even remember the last time I tested 
  something
  on a saa7134 board ;) - so, it can be some new bug).
  
  So, I'll postpone its review to when I have some time to actually test it
  especially as the same issue might also be happening on other drivers.
  
 It will affect other drivers as well; the basic cause is that modern chips
 can enter a package deep sleep state that affects both CPU speed and latency
 to start of DMA. On older systems, this couldn't happen - the Northbridge
 kept running at all times, and DMA latencies were low.
 
 However, on the Intel Sandybridge system I'm testing with, the maximum wake
 latency from deep sleep is 109 microseconds; the SAA7134's internal FIFO can't
 hold onto data for that long, and overflows, resulting in the corruption I'm
 seeing. The pm QoS request fixes this for me, by telling the PM subsystem
 that the SAA7134 cannot cope with a long latency on the first write of a DMA
 transfer.
 
 Now, some media bridges (like the ones driven by the cx18 driver) can cope
 with very high latency before the beginning of a DMA burst. Andy Walls has
 worked on the cx18 driver to cope in this situation, so it doesn't fail even
 with the 109 microsecond DMA latency we have on Sandybridge.

Well if brdige wake-up DMA latency is the problem, it is alos the case
that the CX23418 has a *lot* of on board memory with which to collect
video and compress it.  (And lets face it, the CX23418 is an SoC with
two ARM cores and a lot of dedicated external memory, as opposed to the
SAA7134 with 1 kB of FIFO.)   That hardware helps quite a bit, if the
PCI bus is slow to wake up.

I found a SAA7134 sheet for you:

http://www.nxp.com/documents/data_sheet/SAA7134HL.pdf

Section 6.4.3 has a short description of the behaviour when the FIFO
overflows.

But this sheet (close enough):

http://www.nxp.com/documents/data_sheet/SAA7133HL.pdf

Has much nicer examples of the programmed levels of the FIFO in section
6.4.3.  That 1 kB is for everything: raw VBI, Y, U, V, MPEG, and Audio.
So you're lucky if one full line of video fits in the FIFO.

 Others, like the SAA7134, just don't have that much buffering, and we need
 to ask the pm core to cope with it. I suspect that most old drivers will need
 updating if anyone wants to use them with modern systems; either they'll have
 an architecture like the cx18 series, and the type of work Andy has done will
 fix the problem, or they'll behave like the saa7134, and need a pm_qos request
 to ensure that the CPU package (and thus memory controller) stay awake.

Unless the chip has a lot of internal memory and processing resources, I
suspect a pm_qos solution is needed to ensure the PCI bus responds in
time.

This is a system level issue though.  Having the drivers decide what QoS
they need in the absences of total system requirements, is the right
thing for the home user.  It might not be very friendly for a
professional solution where someone is trying to tune the use of the
system IO bandwidth and CPU resources.

The ivtv driver and cx18 driver unconditionally bumping up their PCI
latency timer to 64 cycles minimum always bugged me: drivers shouldn't
be deciding QoS in a vaccuum.  But, then again, user complaints went
away and the 64 PCI cycles seemed to be a minimum QoS that everyone
needed.  Also both drivers have a ivtv/cx18_pci_latency module option to
override the behaviour anyway.


-Andy

--
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] saa7134: Add pm_qos_request to fix video corruption

2012-10-29 Thread Simon Farnsworth
On Monday 29 October 2012 09:32:27 Andy Walls wrote:
 On Mon, 2012-10-29 at 13:02 +, Simon Farnsworth wrote:
  It will affect other drivers as well; the basic cause is that modern chips
  can enter a package deep sleep state that affects both CPU speed and latency
  to start of DMA. On older systems, this couldn't happen - the Northbridge
  kept running at all times, and DMA latencies were low.
  
  However, on the Intel Sandybridge system I'm testing with, the maximum wake
  latency from deep sleep is 109 microseconds; the SAA7134's internal FIFO 
  can't
  hold onto data for that long, and overflows, resulting in the corruption I'm
  seeing. The pm QoS request fixes this for me, by telling the PM subsystem
  that the SAA7134 cannot cope with a long latency on the first write of a DMA
  transfer.
  
  Now, some media bridges (like the ones driven by the cx18 driver) can cope
  with very high latency before the beginning of a DMA burst. Andy Walls has
  worked on the cx18 driver to cope in this situation, so it doesn't fail even
  with the 109 microsecond DMA latency we have on Sandybridge.
 
 Well if brdige wake-up DMA latency is the problem, it is alos the case
 that the CX23418 has a *lot* of on board memory with which to collect
 video and compress it.  (And lets face it, the CX23418 is an SoC with
 two ARM cores and a lot of dedicated external memory, as opposed to the
 SAA7134 with 1 kB of FIFO.)   That hardware helps quite a bit, if the
 PCI bus is slow to wake up.
 
 I found a SAA7134 sheet for you:
 
 http://www.nxp.com/documents/data_sheet/SAA7134HL.pdf
 
 Section 6.4.3 has a short description of the behaviour when the FIFO
 overflows.

That's a good description of what I'm seeing, so fits with the idea that it's
FIFO underflow.
 
 But this sheet (close enough):
 
 http://www.nxp.com/documents/data_sheet/SAA7133HL.pdf
 
 Has much nicer examples of the programmed levels of the FIFO in section
 6.4.3.  That 1 kB is for everything: raw VBI, Y, U, V, MPEG, and Audio.
 So you're lucky if one full line of video fits in the FIFO.
 
And that datasheet suggests that my 31 usec request is too high; in the
fastidious configuration, I'd need a latency of 22 usec, not 31.

Does anyone have register-level documentation for the SAA7134, to confirm the
maximum tolerated latency with the FIFO configuration Linux uses?

  Others, like the SAA7134, just don't have that much buffering, and we need
  to ask the pm core to cope with it. I suspect that most old drivers will 
  need
  updating if anyone wants to use them with modern systems; either they'll 
  have
  an architecture like the cx18 series, and the type of work Andy has done 
  will
  fix the problem, or they'll behave like the saa7134, and need a pm_qos 
  request
  to ensure that the CPU package (and thus memory controller) stay awake.
 
 Unless the chip has a lot of internal memory and processing resources, I
 suspect a pm_qos solution is needed to ensure the PCI bus responds in
 time.
 
 This is a system level issue though.  Having the drivers decide what QoS
 they need in the absences of total system requirements, is the right
 thing for the home user.  It might not be very friendly for a
 professional solution where someone is trying to tune the use of the
 system IO bandwidth and CPU resources.
 
 The ivtv driver and cx18 driver unconditionally bumping up their PCI
 latency timer to 64 cycles minimum always bugged me: drivers shouldn't
 be deciding QoS in a vaccuum.  But, then again, user complaints went
 away and the 64 PCI cycles seemed to be a minimum QoS that everyone
 needed.  Also both drivers have a ivtv/cx18_pci_latency module option to
 override the behaviour anyway.
 
So, one trick that the pm_qos request infrastructure gives us is that I only
request reduced latency when we are actually streaming. It's up to the pm core
to determine what that means - e.g. keep CPU awake, program chipset registers,
ignore the request completely.

The other part of this is that I'm flagging my QoS requirements; if the wakeup
latency is higher than I'm requesting, I'll fail - that is clearly wrong. On
the other hand, if you have reasons to want lower wakeup latencies, the core
will combine all requests (both userspace and kernelspace), and choose the
minimum. If you're sophisticated enough to accept the problems involved in not
waking up in time to service DMA requests, you're probably also up to the task
of changing the kernel slightly to not request lower latencies.
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH] saa7134: Add pm_qos_request to fix video corruption

2012-10-29 Thread Andy Walls
On Mon, 2012-10-29 at 13:02 +, Simon Farnsworth wrote:
 On Monday 29 October 2012 09:58:17 Mauro Carvalho Chehab wrote:
  I prefer if you don't c/c me on that ;) Patchwork is the main source that I 
  use
  on my patch reviews.
  
 Noted.
 
  Btw, I saw your patch yesterday (and skipped it, for now), as I never played
  with those pm QoS stuff before, nor I ever noticed anything like what you
  reported on saa7134 (but I can't even remember the last time I tested 
  something
  on a saa7134 board ;) - so, it can be some new bug).
  
  So, I'll postpone its review to when I have some time to actually test it
  especially as the same issue might also be happening on other drivers.
  
 It will affect other drivers as well; the basic cause is that modern chips
 can enter a package deep sleep state that affects both CPU speed and latency
 to start of DMA. On older systems, this couldn't happen - the Northbridge
 kept running at all times, and DMA latencies were low.
 
 However, on the Intel Sandybridge system I'm testing with, the maximum wake
 latency from deep sleep is 109 microseconds; the SAA7134's internal FIFO can't
 hold onto data for that long, and overflows,

BTW

For Y:U:Y:V or raw VBI with a PAL line rate
109 usecs * 15,625 lines/sec ~= 1.7 lines

1.7 lines * 1440 samples/line ~= 2452 samples

2452 samples / 1024 samples/FIFO ~= 2.4 FIFOs

So 109 usecs fully overruns the FIFO by about 1.4 FIFO depths.


  resulting in the corruption I'm
 seeing. The pm QoS request fixes this for me, by telling the PM subsystem
 that the SAA7134 cannot cope with a long latency on the first write of a DMA
 transfer.

Regards,
Andy

--
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] saa7134: Add pm_qos_request to fix video corruption

2012-10-29 Thread Mauro Carvalho Chehab
Em Mon, 29 Oct 2012 14:11 +
Simon Farnsworth simon.farnswo...@onelan.co.uk escreveu:

 On Monday 29 October 2012 09:32:27 Andy Walls wrote:
  On Mon, 2012-10-29 at 13:02 +, Simon Farnsworth wrote:
   It will affect other drivers as well; the basic cause is that modern chips
   can enter a package deep sleep state that affects both CPU speed and 
   latency
   to start of DMA. On older systems, this couldn't happen - the Northbridge
   kept running at all times, and DMA latencies were low.
   
   However, on the Intel Sandybridge system I'm testing with, the maximum 
   wake
   latency from deep sleep is 109 microseconds; the SAA7134's internal FIFO 
   can't
   hold onto data for that long, and overflows, resulting in the corruption 
   I'm
   seeing. The pm QoS request fixes this for me, by telling the PM subsystem
   that the SAA7134 cannot cope with a long latency on the first write of a 
   DMA
   transfer.
   
   Now, some media bridges (like the ones driven by the cx18 driver) can cope
   with very high latency before the beginning of a DMA burst. Andy Walls has
   worked on the cx18 driver to cope in this situation, so it doesn't fail 
   even
   with the 109 microsecond DMA latency we have on Sandybridge.
  
  Well if brdige wake-up DMA latency is the problem, it is alos the case
  that the CX23418 has a *lot* of on board memory with which to collect
  video and compress it.  (And lets face it, the CX23418 is an SoC with
  two ARM cores and a lot of dedicated external memory, as opposed to the
  SAA7134 with 1 kB of FIFO.)   That hardware helps quite a bit, if the
  PCI bus is slow to wake up.
  
  I found a SAA7134 sheet for you:
  
  http://www.nxp.com/documents/data_sheet/SAA7134HL.pdf
  
  Section 6.4.3 has a short description of the behaviour when the FIFO
  overflows.
 
 That's a good description of what I'm seeing, so fits with the idea that it's
 FIFO underflow.
  
  But this sheet (close enough):
  
  http://www.nxp.com/documents/data_sheet/SAA7133HL.pdf
  
  Has much nicer examples of the programmed levels of the FIFO in section
  6.4.3.  That 1 kB is for everything: raw VBI, Y, U, V, MPEG, and Audio.
  So you're lucky if one full line of video fits in the FIFO.
  
 And that datasheet suggests that my 31 usec request is too high; in the
 fastidious configuration, I'd need a latency of 22 usec, not 31.
 
 Does anyone have register-level documentation for the SAA7134, to confirm the
 maximum tolerated latency with the FIFO configuration Linux uses?
 
   Others, like the SAA7134, just don't have that much buffering, and we need
   to ask the pm core to cope with it. I suspect that most old drivers will 
   need
   updating if anyone wants to use them with modern systems; either they'll 
   have
   an architecture like the cx18 series, and the type of work Andy has done 
   will
   fix the problem, or they'll behave like the saa7134, and need a pm_qos 
   request
   to ensure that the CPU package (and thus memory controller) stay awake.
  
  Unless the chip has a lot of internal memory and processing resources, I
  suspect a pm_qos solution is needed to ensure the PCI bus responds in
  time.
  
  This is a system level issue though.  Having the drivers decide what QoS
  they need in the absences of total system requirements, is the right
  thing for the home user.  It might not be very friendly for a
  professional solution where someone is trying to tune the use of the
  system IO bandwidth and CPU resources.
  
  The ivtv driver and cx18 driver unconditionally bumping up their PCI
  latency timer to 64 cycles minimum always bugged me: drivers shouldn't
  be deciding QoS in a vaccuum.  But, then again, user complaints went
  away and the 64 PCI cycles seemed to be a minimum QoS that everyone
  needed.  Also both drivers have a ivtv/cx18_pci_latency module option to
  override the behaviour anyway.
  
 So, one trick that the pm_qos request infrastructure gives us is that I only
 request reduced latency when we are actually streaming. It's up to the pm core
 to determine what that means - e.g. keep CPU awake, program chipset registers,
 ignore the request completely.
 
 The other part of this is that I'm flagging my QoS requirements; if the wakeup
 latency is higher than I'm requesting, I'll fail - that is clearly wrong. On
 the other hand, if you have reasons to want lower wakeup latencies, the core
 will combine all requests (both userspace and kernelspace), and choose the
 minimum. If you're sophisticated enough to accept the problems involved in not
 waking up in time to service DMA requests, you're probably also up to the task
 of changing the kernel slightly to not request lower latencies.

Andy/Simon,

Thanks for digging into it and getting more data. Do you know if this change
it also needed with USB devices that do DMA (isoc and/or bulk)? Or the USB
core already handles that?


Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body 

Re: [PATCH] saa7134: Add pm_qos_request to fix video corruption

2012-10-29 Thread Simon Farnsworth
On Monday 29 October 2012 13:44:45 Mauro Carvalho Chehab wrote:
 Thanks for digging into it and getting more data. Do you know if this change
 it also needed with USB devices that do DMA (isoc and/or bulk)? Or the USB
 core already handles that?
 
I'm not a huge expert - the linux-pm list (cc'd) will have people around who
know more.

If I've understood correctly, though, the USB core should take care of pm_qos
requests if they're needed for the hardware; remember that if the HCD has
enough buffering, there's no need for a pm_qos request. It's only needed for
devices like the SAA7134 where the buffer is small (1K split into pieces)
compared to the sample data rate (27 megabytes/second raw video).

For the benefit of the linux-pm list; this all starts with me providing a
patch to have the saa7134 driver request reduced cpu_dma_latency when
streaming, as I've seen buffer exhaustion. We've got far enough to know that
the value I chose was wrong for the saa7134, but Mauro also wants guidance on
whether USB devices (not host controllers) also need to request reduced
latency.
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com


signature.asc
Description: This is a digitally signed message part.


[PATCH] saa7134: Add pm_qos_request to fix video corruption

2012-10-22 Thread Simon Farnsworth
The SAA7134 appears to have trouble buffering more than one line of video
when doing DMA. Rather than try to fix the driver to cope (as has been done
by Andy Walls for the cx18 driver), put in a pm_qos_request to limit deep
sleep exit latencies.

The visible effect of not having this is that seemingly random lines are
only partly transferred - if you feed in a static image, you see a portion
of the image flicker into place.

Signed-off-by: Simon Farnsworth simon.farnswo...@onelan.co.uk
---

As per the comment, note that I've not been able to nail down the maximum
latency the SAA7134 can cope with. I know that the chip has a 1KiB FIFO
buffer, so I'm assuming that it can store half a line of video at a time, on
the basis of 720 luma, 360 Cb, 360 Cr samples, totalling 1440 bytes per
line. If this is a bad assumption (I've not been able to find register-level
documentation for the chip, so I don't know what
saa_writel(SAA7134_FIFO_SIZE, 0x08070503) does in saa7134_hw_enable1() in
saa7134-core.c), that value will need adjusting to match the real FIFO
latency.

 drivers/media/pci/saa7134/saa7134-video.c | 12 
 drivers/media/pci/saa7134/saa7134.h   |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/drivers/media/pci/saa7134/saa7134-video.c 
b/drivers/media/pci/saa7134/saa7134-video.c
index 4a77124..dbc0b5d 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2248,6 +2248,15 @@ static int saa7134_streamon(struct file *file, void 
*priv,
if (!res_get(dev, fh, res))
return -EBUSY;
 
+   /* The SAA7134 has a 1K FIFO; the assumption here is that that's
+* enough for half a line of video in the configuration Linux uses.
+* If it isn't, reduce the 31 usec down to the maximum FIFO time
+* allowance.
+*/
+   pm_qos_add_request(fh-qos_request,
+  PM_QOS_CPU_DMA_LATENCY,
+  31);
+
return videobuf_streamon(saa7134_queue(fh));
 }
 
@@ -2259,6 +2269,8 @@ static int saa7134_streamoff(struct file *file, void 
*priv,
struct saa7134_dev *dev = fh-dev;
int res = saa7134_resource(fh);
 
+   pm_qos_remove_request(fh-qos_request);
+
err = videobuf_streamoff(saa7134_queue(fh));
if (err  0)
return err;
diff --git a/drivers/media/pci/saa7134/saa7134.h 
b/drivers/media/pci/saa7134/saa7134.h
index c24b651..d09393b 100644
--- a/drivers/media/pci/saa7134/saa7134.h
+++ b/drivers/media/pci/saa7134/saa7134.h
@@ -29,6 +29,7 @@
 #include linux/notifier.h
 #include linux/delay.h
 #include linux/mutex.h
+#include linux/pm_qos_params.h
 
 #include asm/io.h
 
@@ -469,6 +470,7 @@ struct saa7134_fh {
enum v4l2_buf_type type;
unsigned int   resources;
enum v4l2_priority prio;
+   struct pm_qos_request_list qos_request;
 
/* video overlay */
struct v4l2_window win;
-- 
1.7.11.2

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