[PATCH] uvc: more buffers

[PATCH] uvc: more buffers

[oliver]

From: Oliver Neukum <oneukum@suse.de>

This is necessary to let the new generation of cameras
from LiteOn used in Haswell ULT notebook operate. Otherwise
the images will be truncated.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/media/usb/uvc/uvcvideo.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 9e35982..9f1930b 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -114,9 +114,9 @@
 /* Number of isochronous URBs. */
 #define UVC_URBS		5
 /* Maximum number of packets per URB. */
-#define UVC_MAX_PACKETS		32
+#define UVC_MAX_PACKETS		128	
 /* Maximum number of video buffers. */
-#define UVC_MAX_VIDEO_BUFFERS	32
+#define UVC_MAX_VIDEO_BUFFERS	128
 /* Maximum status buffer size in bytes of interrupt URB. */
 #define UVC_MAX_STATUS_SIZE	16
 
-- 
1.8.3.1

Re: [PATCH] uvc: more buffers

[Laurent Pinchart]

Hi Oliver,

Thank you for the patch.

On Friday 09 August 2013 15:11:36 oliver@neukum.org wrote:
> From: Oliver Neukum <oneukum@suse.de>
> 
> This is necessary to let the new generation of cameras from LiteOn used in
> Haswell ULT notebook operate. Otherwise the images will be truncated.

Could you please post the lsusb -v output for the device ?

Why does it need more buffers, is it a superspeed webcam ?

> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
>  drivers/media/usb/uvc/uvcvideo.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h index 9e35982..9f1930b 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -114,9 +114,9 @@
>  /* Number of isochronous URBs. */
>  #define UVC_URBS		5
>  /* Maximum number of packets per URB. */
> -#define UVC_MAX_PACKETS		32
> +#define UVC_MAX_PACKETS		128

That would mean up to 384KiB per URB. While not unreasonable, I'd like to know 
how much data your camera produces to require this.

>  /* Maximum number of video buffers. */
> -#define UVC_MAX_VIDEO_BUFFERS	32
> +#define UVC_MAX_VIDEO_BUFFERS	128

I don't think your camera really needs more than 32 V4L2 (full frame) buffers 
:-)

>  /* Maximum status buffer size in bytes of interrupt URB. */
>  #define UVC_MAX_STATUS_SIZE	16
-- 
Regards,

Laurent Pinchart

Re: [PATCH] uvc: more buffers

[Oliver Neukum]

On Fri, 2013-08-09 at 15:58 +0200, Laurent Pinchart wrote:

Hi,

> > This is necessary to let the new generation of cameras from LiteOn used in
> > Haswell ULT notebook operate. Otherwise the images will be truncated.
> 
> Could you please post the lsusb -v output for the device ?

It is attached.

> Why does it need more buffers, is it a superspeed webcam ?

No. It is HS.

> > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > ---
> >  drivers/media/usb/uvc/uvcvideo.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h
> > b/drivers/media/usb/uvc/uvcvideo.h index 9e35982..9f1930b 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -114,9 +114,9 @@
> >  /* Number of isochronous URBs. */
> >  #define UVC_URBS		5
> >  /* Maximum number of packets per URB. */
> > -#define UVC_MAX_PACKETS		32
> > +#define UVC_MAX_PACKETS		128
> 
> That would mean up to 384KiB per URB. While not unreasonable, I'd like to know 
> how much data your camera produces to require this.

How to determine that?

> >  /* Maximum number of video buffers. */
> > -#define UVC_MAX_VIDEO_BUFFERS	32
> > +#define UVC_MAX_VIDEO_BUFFERS	128
> 
> I don't think your camera really needs more than 32 V4L2 (full frame) buffers 
> :-)

Unfortunately, experimental evidence is that it does need them at
resolutions above 640x480

	Regards
		Oliver

Re: [PATCH] uvc: more buffers

[Laurent Pinchart]

Hi Oliver,

On Monday 12 August 2013 11:01:55 Oliver Neukum wrote:
> On Fri, 2013-08-09 at 15:58 +0200, Laurent Pinchart wrote:
> > > This is necessary to let the new generation of cameras from LiteOn used
> > > in Haswell ULT notebook operate. Otherwise the images will be truncated.
> > 
> > Could you please post the lsusb -v output for the device ?
> 
> It is attached.

No it isn't :-)

> > Why does it need more buffers, is it a superspeed webcam ?
> 
> No. It is HS.
> 
> > > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > > ---
> > > 
> > >  drivers/media/usb/uvc/uvcvideo.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/media/usb/uvc/uvcvideo.h
> > > b/drivers/media/usb/uvc/uvcvideo.h index 9e35982..9f1930b 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -114,9 +114,9 @@
> > > 
> > >  /* Number of isochronous URBs. */
> > >  #define UVC_URBS		5
> > >  /* Maximum number of packets per URB. */
> > > 
> > > -#define UVC_MAX_PACKETS		32
> > > +#define UVC_MAX_PACKETS		128
> > 
> > That would mean up to 384KiB per URB. While not unreasonable, I'd like to
> > know how much data your camera produces to require this.
> 
> How to determine that?

The UVC descriptors might provide some information.

Do you get errors in the kernel log with UVC_MAX_PACKETS set to 32 ?

> > >  /* Maximum number of video buffers. */
> > > 
> > > -#define UVC_MAX_VIDEO_BUFFERS	32
> > > +#define UVC_MAX_VIDEO_BUFFERS	128
> > 
> > I don't think your camera really needs more than 32 V4L2 (full frame)
> > buffers :-)
> 
> Unfortunately, experimental evidence is that it does need them at
> resolutions above 640x480

Could you please test it again with UVC_MAX_PACKETS set to 128 and 
UVC_MAX_VIDEO_BUFFERS set to 32 ? UVC_MAX_VIDEO_BUFFERS sets the maximum 
number of V4L2 full frame buffers, even 32 is probably way too high.

-- 
Regards,

Laurent Pinchart