[PATCH,v2] [media] vivi: Constify structures

2012-12-28 Thread Kirill Smelkov
On Thu, Dec 27, 2012 at 12:55:11PM +0100, Hans Verkuil wrote:
 On Wed December 26 2012 16:29:43 Kirill Smelkov wrote:
  Most of *_ops and other structures in vivi.c were already declared const
  but some have not. Constify and code/data will take less space:
  
  $ size drivers/media/platform/vivi.o
textdata bss dec hex filename
  before:  12569 248   8   128253219 
  drivers/media/platform/vivi.o
  after:   12308  20   8   123363030 
  drivers/media/platform/vivi.o
  
  i.e. vivi.o is now ~500 bytes less.
  
  Signed-off-by: Kirill Smelkov k...@navytux.spb.ru
  ---
   drivers/media/platform/vivi.c | 26 +-
   1 file changed, 13 insertions(+), 13 deletions(-)
  
  diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
  index ec65089..bed04e1 100644
  --- a/drivers/media/platform/vivi.c
  +++ b/drivers/media/platform/vivi.c
  @@ -91,13 +91,13 @@ static const struct v4l2_fract
  --*/
   
   struct vivi_fmt {
  -   char  *name;
  +   const char  *name;
 
 Just use one space before '*' since it no longer lines up to anything.

[...]
  @@ -191,7 +191,7 @@ struct vivi_buffer {
  /* common v4l buffer stuff -- must be first */
  struct vb2_buffer   vb;
  struct list_headlist;
  -   struct vivi_fmt*fmt;
  +   struct vivi_fmt const  *fmt;
   };
   
   struct vivi_dmaqueue {
  @@ -250,7 +250,7 @@ struct vivi_dev {
  intinput;
   
  /* video capture */
  -   struct vivi_fmt*fmt;
  +   struct vivi_fmt const  *fmt;
 
 'const' should be before 'struct' for consistency reasons.

It's just a matter of style, and in this case I though putting const
after would not distract from the fact that fmt is `struct vivi_fmt *`
and also to align types beginning with non-const ones.

But anyway, style is style and this is not a big deal, so here you are
with corrected patch.

Thanks,
Kirill

 8 
From: Kirill Smelkov k...@navytux.spb.ru
Date: Wed, 26 Dec 2012 19:23:26 +0400
Subject: [PATCH,v2] [media] vivi: Constify structures

Most of *_ops and other structures in vivi.c were already declared const
but some have not. Constify and code/data will take less space:

$ size drivers/media/platform/vivi.o
  textdata bss dec hex filename
before:  12569 248   8   128253219 drivers/media/platform/vivi.o
after:   12308  20   8   123363030 drivers/media/platform/vivi.o

i.e. vivi.o is now ~500 bytes less.

Signed-off-by: Kirill Smelkov k...@navytux.spb.ru
---
 drivers/media/platform/vivi.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

 v2:

- put 'const' always before anything, as noted by Hans Verkuil.
- no changes otherwise.


diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index ec65089..8a33a71 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -91,13 +91,13 @@ static const struct v4l2_fract
--*/
 
 struct vivi_fmt {
-   char  *name;
+   const char *name;
u32   fourcc;  /* v4l2 format id */
u8depth;
bool  is_yuv;
 };
 
-static struct vivi_fmt formats[] = {
+static const struct vivi_fmt formats[] = {
{
.name = 4:2:2, packed, YUYV,
.fourcc   = V4L2_PIX_FMT_YUYV,
@@ -164,9 +164,9 @@ static struct vivi_fmt formats[] = {
},
 };
 
-static struct vivi_fmt *__get_format(u32 pixelformat)
+static const struct vivi_fmt *__get_format(u32 pixelformat)
 {
-   struct vivi_fmt *fmt;
+   const struct vivi_fmt *fmt;
unsigned int k;
 
for (k = 0; k  ARRAY_SIZE(formats); k++) {
@@ -181,7 +181,7 @@ static struct vivi_fmt *__get_format(u32 pixelformat)
return formats[k];
 }
 
-static struct vivi_fmt *get_format(struct v4l2_format *f)
+static const struct vivi_fmt *get_format(struct v4l2_format *f)
 {
return __get_format(f-fmt.pix.pixelformat);
 }
@@ -191,7 +191,7 @@ struct vivi_buffer {
/* common v4l buffer stuff -- must be first */
struct vb2_buffer   vb;
struct list_headlist;
-   struct vivi_fmt*fmt;
+   const struct vivi_fmt  *fmt;
 };
 
 struct vivi_dmaqueue {
@@ -250,7 +250,7 @@ struct vivi_dev {
intinput;
 
/* video capture */
-   struct vivi_fmt*fmt;
+   const struct vivi_fmt  *fmt;
struct v4l2_fract  timeperframe;
unsigned int   width, height;
struct vb2_queue   vb_vidq;
@@ -297,7 +297,7 @@ struct bar_std {
 
 /* Maximum number of bars are 10 - otherwise, the input print code
should be modified */
-static struct bar_std bars[] = {
+static const struct

[PATCH] [media] vivi: Constify structures

2012-12-26 Thread Kirill Smelkov
Most of *_ops and other structures in vivi.c were already declared const
but some have not. Constify and code/data will take less space:

$ size drivers/media/platform/vivi.o
  textdata bss dec hex filename
before:  12569 248   8   128253219 drivers/media/platform/vivi.o
after:   12308  20   8   123363030 drivers/media/platform/vivi.o

i.e. vivi.o is now ~500 bytes less.

Signed-off-by: Kirill Smelkov k...@navytux.spb.ru
---
 drivers/media/platform/vivi.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index ec65089..bed04e1 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -91,13 +91,13 @@ static const struct v4l2_fract
--*/
 
 struct vivi_fmt {
-   char  *name;
+   const char  *name;
u32   fourcc;  /* v4l2 format id */
u8depth;
bool  is_yuv;
 };
 
-static struct vivi_fmt formats[] = {
+static const struct vivi_fmt formats[] = {
{
.name = 4:2:2, packed, YUYV,
.fourcc   = V4L2_PIX_FMT_YUYV,
@@ -164,9 +164,9 @@ static struct vivi_fmt formats[] = {
},
 };
 
-static struct vivi_fmt *__get_format(u32 pixelformat)
+static const struct vivi_fmt *__get_format(u32 pixelformat)
 {
-   struct vivi_fmt *fmt;
+   const struct vivi_fmt *fmt;
unsigned int k;
 
for (k = 0; k  ARRAY_SIZE(formats); k++) {
@@ -181,7 +181,7 @@ static struct vivi_fmt *__get_format(u32 pixelformat)
return formats[k];
 }
 
-static struct vivi_fmt *get_format(struct v4l2_format *f)
+static const struct vivi_fmt *get_format(struct v4l2_format *f)
 {
return __get_format(f-fmt.pix.pixelformat);
 }
@@ -191,7 +191,7 @@ struct vivi_buffer {
/* common v4l buffer stuff -- must be first */
struct vb2_buffer   vb;
struct list_headlist;
-   struct vivi_fmt*fmt;
+   struct vivi_fmt const  *fmt;
 };
 
 struct vivi_dmaqueue {
@@ -250,7 +250,7 @@ struct vivi_dev {
intinput;
 
/* video capture */
-   struct vivi_fmt*fmt;
+   struct vivi_fmt const  *fmt;
struct v4l2_fract  timeperframe;
unsigned int   width, height;
struct vb2_queue   vb_vidq;
@@ -297,7 +297,7 @@ struct bar_std {
 
 /* Maximum number of bars are 10 - otherwise, the input print code
should be modified */
-static struct bar_std bars[] = {
+static const struct bar_std bars[] = {
{   /* Standard ITU-R color bar sequence */
{ COLOR_WHITE, COLOR_AMBER, COLOR_CYAN, COLOR_GREEN,
  COLOR_MAGENTA, COLOR_RED, COLOR_BLUE, COLOR_BLACK, 
COLOR_BLACK }
@@ -926,7 +926,7 @@ static void vivi_unlock(struct vb2_queue *vq)
 }
 
 
-static struct vb2_ops vivi_video_qops = {
+static const struct vb2_ops vivi_video_qops = {
.queue_setup= queue_setup,
.buf_prepare= buffer_prepare,
.buf_queue  = buffer_queue,
@@ -957,7 +957,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
struct v4l2_fmtdesc *f)
 {
-   struct vivi_fmt *fmt;
+   struct vivi_fmt const *fmt;
 
if (f-index = ARRAY_SIZE(formats))
return -EINVAL;
@@ -993,7 +993,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void 
*priv,
struct v4l2_format *f)
 {
struct vivi_dev *dev = video_drvdata(file);
-   struct vivi_fmt *fmt;
+   struct vivi_fmt const *fmt;
 
fmt = get_format(f);
if (!fmt) {
@@ -1102,7 +1102,7 @@ static int vidioc_s_input(struct file *file, void *priv, 
unsigned int i)
 static int vidioc_enum_frameintervals(struct file *file, void *priv,
 struct v4l2_frmivalenum *fival)
 {
-   struct vivi_fmt *fmt;
+   struct vivi_fmt const *fmt;
 
if (fival-index)
return -EINVAL;
@@ -1330,7 +1330,7 @@ static const struct v4l2_ioctl_ops vivi_ioctl_ops = {
.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
 
-static struct video_device vivi_template = {
+static const struct video_device vivi_template = {
.name   = vivi,
.fops   = vivi_fops,
.ioctl_ops  = vivi_ioctl_ops,
-- 
1.8.1.rc2.276.g82c5000
--
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 v6] [media] vivi: Teach it to tune FPS

2012-11-30 Thread Kirill Smelkov
On Mon, Nov 19, 2012 at 09:52:36AM +0400, Kirill Smelkov wrote:
 On Sun, Nov 18, 2012 at 07:25:38AM -0200, Mauro Carvalho Chehab wrote:
  Em 18-11-2012 07:24, Mauro Carvalho Chehab escreveu:
  Em 17-11-2012 08:45, Kirill Smelkov escreveu:
  On Fri, Nov 16, 2012 at 01:46:58PM -0200, Mauro Carvalho Chehab wrote:
  Em 16-11-2012 11:38, Hans Verkuil escreveu:
  On Wed November 7 2012 12:30:01 Kirill Smelkov wrote:
  [...]
  
  diff --git a/drivers/media/platform/vivi.c 
  b/drivers/media/platform/vivi.c
  index 37d0af8..5d1b374 100644
  --- a/drivers/media/platform/vivi.c
  +++ b/drivers/media/platform/vivi.c
  @@ -65,8 +65,11 @@ MODULE_PARM_DESC(vid_limit, capture memory limit 
  in megabytes);
/* Global font descriptor */
static const u8 *font8x16;
  
  -/* default to NTSC timeperframe */
  -static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
  .denominator = 3};
  +/* timeperframe: min/max and default */
  +static const struct v4l2_fract
  +tpf_min = {.numerator = 1,.denominator = UINT_MAX},  
  /* 1/infty */
  +tpf_max = {.numerator = UINT_MAX,.denominator = 1},   
/* infty */
  
  I understand your reasoning here, but I wouldn't go with UINT_MAX here. 
  Something like
  1/1000 tpf (or 1 ms) up to 86400/1 tpf (or once a day). With UINT_MAX I 
  am afraid we
  might hit application errors when they manipulate these values. The 
  shortest time
  between frames is 1 ms anyway.
  
  It's the only comment I have, it looks good otherwise.
  
  As those will be a arbitrary values, I suggest to declare a macro for it 
  at the
  beginning of vivi.c file, with some comment explaining the rationale of 
  the choose,
  and what else needs to be changed, if this changes (e. g. less than 1ms 
  would require
  changing the image generation logic, to avoid producing frames with 
  equal content).
  
  Maybe something like this? (please note, I'm not a good text writer. If
  this needs adjustment please help me shape the text up)
  
  
  diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
  index 5d1b374..45b8a81 100644
  --- a/drivers/media/platform/vivi.c
  +++ b/drivers/media/platform/vivi.c
  @@ -36,6 +36,18 @@
  
#define VIVI_MODULE_NAME vivi
  
  +/* Maximum allowed frame rate
  + *
  + * Vivi will allow setting timeperframe in [1/FPS_MAX - FPS_MAX/1] range.
  + *
  + * Ideally FPS_MAX should be infinity, i.e. practically UINT_MAX, but 
  that
  + * might hit application errors when they manipulate these values.
  + *
  + * Besides, for tpf  1ms image-generation logic should be changed, to 
  avoid
  + * producing frames with equal content.
  + */
  +#define FPS_MAX 1000
  +
#define MAX_WIDTH 1920
#define MAX_HEIGHT 1200
  
  @@ -67,8 +79,8 @@ static const u8 *font8x16;
  
/* timeperframe: min/max and default */
static const struct v4l2_fract
  -tpf_min = {.numerator = 1,.denominator = UINT_MAX},  /* 
  1/infty */
  -tpf_max = {.numerator = UINT_MAX,.denominator = 1}, 
  /* infty */
  +tpf_min = {.numerator = 1,.denominator = FPS_MAX},   /* 
  ~1/infty */
  +tpf_max = {.numerator = FPS_MAX,.denominator = 1}, 
  /* ~infty */
  
  Was too fast answering it... The comments there should also be dropped, as 
  it doesn't
  range anymore to infty.
 
 Ok, agree, let's drop those ~infty comments and be done with it.
 
 
  8 
 From: Kirill Smelkov k...@mns.spb.ru
 Date: Tue, 23 Oct 2012 16:56:59 +0400
 Subject: [PATCH v6] [media] vivi: Teach it to tune FPS
 
 I was testing my video-over-ethernet subsystem recently, and vivi
 seemed to be perfect video source for testing when one don't have lots
 of capture boards and cameras. Only its framerate was hardcoded to
 NTSC's 30fps, while in my country we usually use PAL (25 fps) and I
 needed that to precisely simulate bandwidth.
 
 That's why here is this patch with -enum_frameintervals() and
 -{g,s}_parm() implemented as suggested by Hans Verkuil which passes
 v4l2-compliance and manual testing through v4l2-ctl -P / -p fps.
 
 Regarding newly introduced __get_format(u32 pixelformat) I decided not
 to convert original get_format() to operate on fourcc codes, since = 3
 places in driver need to deal with v4l2_format and otherwise it won't be
 handy.
 
 Thanks.
 
 Signed-off-by: Kirill Smelkov k...@mns.spb.ru
 ---
  drivers/media/platform/vivi.c | 102 
 ++
  1 file changed, 94 insertions(+), 8 deletions(-)
 
 V6:
 - Moved TPF/FPS limit to beginning of vivi.c and added comment there
   why that limit is used, to avoid overlows, as suggested by Mauro
   Carvalho Chehab.
 
 V5:
 - changed  1/infty - infty/1  limits to  1/1000 - 1000/1, to avoid
   hitting aplication errors when they try to manipulato those
   values, as suggested by Hans Verkuil.
 
 V4:
 - corrected fival-stepwise setting and added its check to s_parm();
   also

Re: [PATCH v6] [media] vivi: Teach it to tune FPS

2012-11-30 Thread Kirill Smelkov
On Fri, Nov 30, 2012 at 12:10:59PM +0100, Hans Verkuil wrote:
 On Fri November 30 2012 12:02:14 Kirill Smelkov wrote:

[...]
  P.S. Hans, if this is ok for you, would you please add your ack?
  
 
 Looks good to me!
 
 Acked-by: Hans Verkuil hans.verk...@cisco.com

Thanks!
--
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 v6] [media] vivi: Teach it to tune FPS

2012-11-18 Thread Kirill Smelkov
On Sun, Nov 18, 2012 at 07:25:38AM -0200, Mauro Carvalho Chehab wrote:
 Em 18-11-2012 07:24, Mauro Carvalho Chehab escreveu:
 Em 17-11-2012 08:45, Kirill Smelkov escreveu:
 On Fri, Nov 16, 2012 at 01:46:58PM -0200, Mauro Carvalho Chehab wrote:
 Em 16-11-2012 11:38, Hans Verkuil escreveu:
 On Wed November 7 2012 12:30:01 Kirill Smelkov wrote:
 [...]
 
 diff --git a/drivers/media/platform/vivi.c 
 b/drivers/media/platform/vivi.c
 index 37d0af8..5d1b374 100644
 --- a/drivers/media/platform/vivi.c
 +++ b/drivers/media/platform/vivi.c
 @@ -65,8 +65,11 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
 megabytes);
   /* Global font descriptor */
   static const u8 *font8x16;
 
 -/* default to NTSC timeperframe */
 -static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
 .denominator = 3};
 +/* timeperframe: min/max and default */
 +static const struct v4l2_fract
 +tpf_min = {.numerator = 1,.denominator = UINT_MAX},  /* 
 1/infty */
 +tpf_max = {.numerator = UINT_MAX,.denominator = 1}, 
 /* infty */
 
 I understand your reasoning here, but I wouldn't go with UINT_MAX here. 
 Something like
 1/1000 tpf (or 1 ms) up to 86400/1 tpf (or once a day). With UINT_MAX I 
 am afraid we
 might hit application errors when they manipulate these values. The 
 shortest time
 between frames is 1 ms anyway.
 
 It's the only comment I have, it looks good otherwise.
 
 As those will be a arbitrary values, I suggest to declare a macro for it 
 at the
 beginning of vivi.c file, with some comment explaining the rationale of 
 the choose,
 and what else needs to be changed, if this changes (e. g. less than 1ms 
 would require
 changing the image generation logic, to avoid producing frames with equal 
 content).
 
 Maybe something like this? (please note, I'm not a good text writer. If
 this needs adjustment please help me shape the text up)
 
 
 diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
 index 5d1b374..45b8a81 100644
 --- a/drivers/media/platform/vivi.c
 +++ b/drivers/media/platform/vivi.c
 @@ -36,6 +36,18 @@
 
   #define VIVI_MODULE_NAME vivi
 
 +/* Maximum allowed frame rate
 + *
 + * Vivi will allow setting timeperframe in [1/FPS_MAX - FPS_MAX/1] range.
 + *
 + * Ideally FPS_MAX should be infinity, i.e. practically UINT_MAX, but that
 + * might hit application errors when they manipulate these values.
 + *
 + * Besides, for tpf  1ms image-generation logic should be changed, to 
 avoid
 + * producing frames with equal content.
 + */
 +#define FPS_MAX 1000
 +
   #define MAX_WIDTH 1920
   #define MAX_HEIGHT 1200
 
 @@ -67,8 +79,8 @@ static const u8 *font8x16;
 
   /* timeperframe: min/max and default */
   static const struct v4l2_fract
 -tpf_min = {.numerator = 1,.denominator = UINT_MAX},  /* 
 1/infty */
 -tpf_max = {.numerator = UINT_MAX,.denominator = 1}, /* 
 infty */
 +tpf_min = {.numerator = 1,.denominator = FPS_MAX},   /* 
 ~1/infty */
 +tpf_max = {.numerator = FPS_MAX,.denominator = 1}, /* 
 ~infty */
 
 Was too fast answering it... The comments there should also be dropped, as it 
 doesn't
 range anymore to infty.

Ok, agree, let's drop those ~infty comments and be done with it.


 8 
From: Kirill Smelkov k...@mns.spb.ru
Date: Tue, 23 Oct 2012 16:56:59 +0400
Subject: [PATCH v6] [media] vivi: Teach it to tune FPS

I was testing my video-over-ethernet subsystem recently, and vivi
seemed to be perfect video source for testing when one don't have lots
of capture boards and cameras. Only its framerate was hardcoded to
NTSC's 30fps, while in my country we usually use PAL (25 fps) and I
needed that to precisely simulate bandwidth.

That's why here is this patch with -enum_frameintervals() and
-{g,s}_parm() implemented as suggested by Hans Verkuil which passes
v4l2-compliance and manual testing through v4l2-ctl -P / -p fps.

Regarding newly introduced __get_format(u32 pixelformat) I decided not
to convert original get_format() to operate on fourcc codes, since = 3
places in driver need to deal with v4l2_format and otherwise it won't be
handy.

Thanks.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 102 ++
 1 file changed, 94 insertions(+), 8 deletions(-)

V6:
- Moved TPF/FPS limit to beginning of vivi.c and added comment there
  why that limit is used, to avoid overlows, as suggested by Mauro
  Carvalho Chehab.

V5:
- changed  1/infty - infty/1  limits to  1/1000 - 1000/1, to avoid
  hitting aplication errors when they try to manipulato those
  values, as suggested by Hans Verkuil.

V4:
- corrected fival-stepwise setting and added its check to s_parm();
  also cosmetics - all as per Hans Verkuil review.

V3:
- corrected issues with V4L2 spec compliance as pointed by Hans
  Verkuil.

V2:

- reworked FPS setting from module param to via -{g,s}_parm

Re: [PATCH v4] [media] vivi: Teach it to tune FPS

2012-11-17 Thread Kirill Smelkov
On Fri, Nov 16, 2012 at 01:46:58PM -0200, Mauro Carvalho Chehab wrote:
 Em 16-11-2012 11:38, Hans Verkuil escreveu:
 On Wed November 7 2012 12:30:01 Kirill Smelkov wrote:
[...]

 diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
 index 37d0af8..5d1b374 100644
 --- a/drivers/media/platform/vivi.c
 +++ b/drivers/media/platform/vivi.c
 @@ -65,8 +65,11 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
 megabytes);
   /* Global font descriptor */
   static const u8 *font8x16;
 
 -/* default to NTSC timeperframe */
 -static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
 .denominator = 3};
 +/* timeperframe: min/max and default */
 +static const struct v4l2_fract
 +   tpf_min = {.numerator = 1,  .denominator = UINT_MAX},  /* 
 1/infty */
 +   tpf_max = {.numerator = UINT_MAX,   .denominator = 1}, /* 
 infty */
 
 I understand your reasoning here, but I wouldn't go with UINT_MAX here. 
 Something like
 1/1000 tpf (or 1 ms) up to 86400/1 tpf (or once a day). With UINT_MAX I am 
 afraid we
 might hit application errors when they manipulate these values. The shortest 
 time
 between frames is 1 ms anyway.
 
 It's the only comment I have, it looks good otherwise.
 
 As those will be a arbitrary values, I suggest to declare a macro for it at 
 the
 beginning of vivi.c file, with some comment explaining the rationale of the 
 choose,
 and what else needs to be changed, if this changes (e. g. less than 1ms would 
 require
 changing the image generation logic, to avoid producing frames with equal 
 content).

Maybe something like this? (please note, I'm not a good text writer. If
this needs adjustment please help me shape the text up)


diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 5d1b374..45b8a81 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -36,6 +36,18 @@
 
 #define VIVI_MODULE_NAME vivi
 
+/* Maximum allowed frame rate
+ *
+ * Vivi will allow setting timeperframe in [1/FPS_MAX - FPS_MAX/1] range.
+ *
+ * Ideally FPS_MAX should be infinity, i.e. practically UINT_MAX, but that
+ * might hit application errors when they manipulate these values.
+ *
+ * Besides, for tpf  1ms image-generation logic should be changed, to avoid
+ * producing frames with equal content.
+ */
+#define FPS_MAX 1000
+
 #define MAX_WIDTH 1920
 #define MAX_HEIGHT 1200
 
@@ -67,8 +79,8 @@ static const u8 *font8x16;
 
 /* timeperframe: min/max and default */
 static const struct v4l2_fract
-   tpf_min = {.numerator = 1,  .denominator = UINT_MAX},  /* 
1/infty */
-   tpf_max = {.numerator = UINT_MAX,   .denominator = 1}, /* 
infty */
+   tpf_min = {.numerator = 1,  .denominator = FPS_MAX},   /* 
~1/infty */
+   tpf_max = {.numerator = FPS_MAX,.denominator = 1}, /* 
~infty */
tpf_default = {.numerator = 1001,   .denominator = 3}; /* 
NTSC */
 
 #define dprintk(dev, level, fmt, arg...) \
--
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 v5] [media] vivi: Teach it to tune FPS

2012-11-16 Thread Kirill Smelkov
On Fri, Nov 16, 2012 at 02:38:09PM +0100, Hans Verkuil wrote:
 On Wed November 7 2012 12:30:01 Kirill Smelkov wrote:
[...]

  diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
  index 37d0af8..5d1b374 100644
  --- a/drivers/media/platform/vivi.c
  +++ b/drivers/media/platform/vivi.c
  @@ -65,8 +65,11 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
  megabytes);
   /* Global font descriptor */
   static const u8 *font8x16;
   
  -/* default to NTSC timeperframe */
  -static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
  .denominator = 3};
  +/* timeperframe: min/max and default */
  +static const struct v4l2_fract
  +   tpf_min = {.numerator = 1,  .denominator = UINT_MAX},  /* 
  1/infty */
  +   tpf_max = {.numerator = UINT_MAX,   .denominator = 1}, /* 
  infty */
 
 I understand your reasoning here, but I wouldn't go with UINT_MAX here. 
 Something like
 1/1000 tpf (or 1 ms) up to 86400/1 tpf (or once a day). With UINT_MAX I am 
 afraid we
 might hit application errors when they manipulate these values. The shortest 
 time
 between frames is 1 ms anyway.
 
 It's the only comment I have, it looks good otherwise.

Thanks, Let's then merge it with 1/1000 - 1000/1 limit. Ok?

Thanks again,
Kirill


 8 
From: Kirill Smelkov k...@mns.spb.ru
Date: Tue, 23 Oct 2012 16:56:59 +0400
Subject: [PATCH v5] [media] vivi: Teach it to tune FPS

I was testing my video-over-ethernet subsystem recently, and vivi
seemed to be perfect video source for testing when one don't have lots
of capture boards and cameras. Only its framerate was hardcoded to
NTSC's 30fps, while in my country we usually use PAL (25 fps) and I
needed that to precisely simulate bandwidth.

That's why here is this patch with -enum_frameintervals() and
-{g,s}_parm() implemented as suggested by Hans Verkuil which passes
v4l2-compliance and manual testing through v4l2-ctl -P / -p fps.

Regarding newly introduced __get_format(u32 pixelformat) I decided not
to convert original get_format() to operate on fourcc codes, since = 3
places in driver need to deal with v4l2_format and otherwise it won't be
handy.

Thanks.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 92 ++-
 1 file changed, 83 insertions(+), 9 deletions(-)

V5:
- changed  1/infty - infty/1  limits to  1/1000 - 1000/1, to avoid
  hitting aplication errors when they try to manipulato those
  values, as suggested by Hans Verkuil.

V4:
- corrected fival-stepwise setting and added its check to s_parm();
  also cosmetics - all as per Hans Verkuil review.

V3:
- corrected issues with V4L2 spec compliance as pointed by Hans
  Verkuil.

V2:

- reworked FPS setting from module param to via -{g,s}_parm() as suggested
  by Hans Verkuil.

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 6e6dd25..793d7ee 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -36,10 +36,6 @@
 
 #define VIVI_MODULE_NAME vivi
 
-/* Wake up at about 30 fps */
-#define WAKE_NUMERATOR 30
-#define WAKE_DENOMINATOR 1001
-
 #define MAX_WIDTH 1920
 #define MAX_HEIGHT 1200
 
@@ -69,6 +65,12 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
megabytes);
 /* Global font descriptor */
 static const u8 *font8x16;
 
+/* timeperframe: min/max and default */
+static const struct v4l2_fract
+   tpf_min = {.numerator = 1,  .denominator = 1000},   /* 
~1/infty */
+   tpf_max = {.numerator = 1000,   .denominator = 1},  /* 
~infty */
+   tpf_default = {.numerator = 1001,   .denominator = 3};  /* NTSC 
*/
+
 #define dprintk(dev, level, fmt, arg...) \
v4l2_dbg(level, debug, dev-v4l2_dev, fmt, ## arg)
 
@@ -150,14 +152,14 @@ static struct vivi_fmt formats[] = {
},
 };
 
-static struct vivi_fmt *get_format(struct v4l2_format *f)
+static struct vivi_fmt *__get_format(u32 pixelformat)
 {
struct vivi_fmt *fmt;
unsigned int k;
 
for (k = 0; k  ARRAY_SIZE(formats); k++) {
fmt = formats[k];
-   if (fmt-fourcc == f-fmt.pix.pixelformat)
+   if (fmt-fourcc == pixelformat)
break;
}
 
@@ -167,6 +169,11 @@ static struct vivi_fmt *get_format(struct v4l2_format *f)
return formats[k];
 }
 
+static struct vivi_fmt *get_format(struct v4l2_format *f)
+{
+   return __get_format(f-fmt.pix.pixelformat);
+}
+
 /* buffer for one video frame */
 struct vivi_buffer {
/* common v4l buffer stuff -- must be first */
@@ -232,6 +239,7 @@ struct vivi_dev {
 
/* video capture */
struct vivi_fmt*fmt;
+   struct v4l2_fract  timeperframe;
unsigned int   width, height;
struct vb2_queue   vb_vidq;
unsigned int   field_count;
@@ -691,8 +699,8 @@ static void vivi_thread_tick

Re: [PATCH v5] [media] vivi: Teach it to tune FPS

2012-11-16 Thread Kirill Smelkov
On Fri, Nov 16, 2012 at 03:51:23PM +0100, Hans Verkuil wrote:
 On Fri November 16 2012 15:48:41 Kirill Smelkov wrote:
  On Fri, Nov 16, 2012 at 02:38:09PM +0100, Hans Verkuil wrote:
   On Wed November 7 2012 12:30:01 Kirill Smelkov wrote:
  [...]
  
diff --git a/drivers/media/platform/vivi.c 
b/drivers/media/platform/vivi.c
index 37d0af8..5d1b374 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -65,8 +65,11 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
megabytes);
 /* Global font descriptor */
 static const u8 *font8x16;
 
-/* default to NTSC timeperframe */
-static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
.denominator = 3};
+/* timeperframe: min/max and default */
+static const struct v4l2_fract
+   tpf_min = {.numerator = 1,  .denominator = 
UINT_MAX},  /* 1/infty */
+   tpf_max = {.numerator = UINT_MAX,   .denominator = 1},  
   /* infty */
   
   I understand your reasoning here, but I wouldn't go with UINT_MAX here. 
   Something like
   1/1000 tpf (or 1 ms) up to 86400/1 tpf (or once a day). With UINT_MAX I 
   am afraid we
   might hit application errors when they manipulate these values. The 
   shortest time
   between frames is 1 ms anyway.
   
   It's the only comment I have, it looks good otherwise.
  
  Thanks, Let's then merge it with 1/1000 - 1000/1 limit. Ok?
 
 OK.

Thanks.
--
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 v4] [media] vivi: Teach it to tune FPS

2012-11-12 Thread Kirill Smelkov
On Wed, Nov 07, 2012 at 03:30:01PM +0400, Kirill Smelkov wrote:
 On Fri, Nov 02, 2012 at 03:42:21PM +0100, Hans Verkuil wrote:
  Thanks for the ping, I forgot about this patch...
 
 Thanks for feedback and for waiting. I'm here again...
 
 
  On Tue October 23 2012 15:35:21 Kirill Smelkov wrote:
   On Tue, Oct 23, 2012 at 08:40:04AM +0200, Hans Verkuil wrote:
On Mon October 22 2012 19:01:40 Kirill Smelkov wrote:
 On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
  On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
   I was testing my video-over-ethernet subsystem today, and vivi 
   seemed to
   be perfect video source for testing when one don't have lots of 
   capture
   boards and cameras. Only its framerate was hardcoded to NTSC's 
   30fps,
   while in my country we usually use PAL (25 fps). That's why the 
   patch.
   Thanks.
  
  Rather than introducing a module option, it's much nicer if you can
  implement enum_frameintervals and g/s_parm. This can be made quite 
  flexible
  allowing you to also support 50/59.94/60 fps.
 
 Thanks for feedback. I've reworked the patch for FPS to be set via
 -{g,s}_parm(), and yes now it is more flexble, because one can set
 different FPS on different vivi devices. Only I don't know V4L2 ioctls
 details well enough and various drivers do things differently. The 
 patch
 is below. Is it ok?

Close, but it's not quite there.

You should run the v4l2-compliance tool from the v4l-utils.git 
repository
(take the master branch). That will report any errors in your 
implementation.

In this case g/s_parm doesn't set readbuffers (set it to 1) and if 
timeperframe
equals { 0, 0 }, then you should get a nominal framerate (let's stick 
to 29.97
for that). I would set the nominal framerate whenever the denominator 
== 0.

For vidioc_enum_frameintervals you need to check the IN fields and fill 
in the
stepwise struct.
   
   Thanks for pointers and info about v4l2-compliance handy-tool. I've
   tried to correct all the issues you mentioned above and here is the
   patch.
   
   (Only requirement to set stepwise.step to 1.0 for
V4L2_FRMIVAL_TYPE_CONTINUOUS seems a bit illogical to me, but anyway,
that's what the V4L2 spec requires...)
   
   Thanks,
   Kirill
   
    8 
   From: Kirill Smelkov k...@mns.spb.ru
   Date: Tue, 23 Oct 2012 16:56:59 +0400
   Subject: [PATCH v3] [media] vivi: Teach it to tune FPS
   
   I was testing my video-over-ethernet subsystem yesterday, and vivi
   seemed to be perfect video source for testing when one don't have lots
   of capture boards and cameras. Only its framerate was hardcoded to
   NTSC's 30fps, while in my country we usually use PAL (25 fps) and I
   needed that to precisely simulate bandwidth.
   
   That's why here is this patch with -enum_frameintervals() and
   -{g,s}_parm() implemented as suggested by Hans Verkuil which passes
   v4l2-compliance and manual testing through v4l2-ctl -P / -p fps.
   
   Regarding newly introduced __get_format(u32 pixelformat) I decided not
   to convert original get_format() to operate on fourcc codes, since = 3
   places in driver need to deal with v4l2_format and otherwise it won't be
   handy.
   
   Thanks.
   
   Signed-off-by: Kirill Smelkov k...@mns.spb.ru
   ---
drivers/media/platform/vivi.c | 84 
   ++-
1 file changed, 75 insertions(+), 9 deletions(-)
   
   V3:
   - corrected issues with V4L2 spec compliance as pointed by Hans
 Verkuil.
   
   V2:
   
   - reworked FPS setting from module param to via -{g,s}_parm() as 
   suggested
 by Hans Verkuil.
   
   
   diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
   index 3e6902a..3adea58 100644
   --- a/drivers/media/platform/vivi.c
   +++ b/drivers/media/platform/vivi.c
   @@ -36,10 +36,6 @@

#define VIVI_MODULE_NAME vivi

   -/* Wake up at about 30 fps */
   -#define WAKE_NUMERATOR 30
   -#define WAKE_DENOMINATOR 1001
   -
#define MAX_WIDTH 1920
#define MAX_HEIGHT 1200

   @@ -69,6 +65,9 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
   megabytes);
/* Global font descriptor */
static const u8 *font8x16;

   +/* default to NTSC timeperframe */
   +static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
   .denominator = 3};
  
  Keep the name lower case: tpf_default. Upper case is for defines only.
 
 ok
 
 [...]
 
   @@ -1049,6 +1054,63 @@ static int vidioc_s_input(struct file *file, void 
   *priv, unsigned int i)
 return 0;
}

   +/* timeperframe is arbitrary and continous */
   +static int vidioc_enum_frameintervals(struct file *file, void *priv,
   +  struct v4l2_frmivalenum *fival)
   +{
   + struct vivi_fmt *fmt;
   +
   + if (fival-index

[PATCH v4] [media] vivi: Teach it to tune FPS

2012-11-07 Thread Kirill Smelkov
On Fri, Nov 02, 2012 at 03:42:21PM +0100, Hans Verkuil wrote:
 Thanks for the ping, I forgot about this patch...

Thanks for feedback and for waiting. I'm here again...


 On Tue October 23 2012 15:35:21 Kirill Smelkov wrote:
  On Tue, Oct 23, 2012 at 08:40:04AM +0200, Hans Verkuil wrote:
   On Mon October 22 2012 19:01:40 Kirill Smelkov wrote:
On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
 On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
  I was testing my video-over-ethernet subsystem today, and vivi 
  seemed to
  be perfect video source for testing when one don't have lots of 
  capture
  boards and cameras. Only its framerate was hardcoded to NTSC's 
  30fps,
  while in my country we usually use PAL (25 fps). That's why the 
  patch.
  Thanks.
 
 Rather than introducing a module option, it's much nicer if you can
 implement enum_frameintervals and g/s_parm. This can be made quite 
 flexible
 allowing you to also support 50/59.94/60 fps.

Thanks for feedback. I've reworked the patch for FPS to be set via
-{g,s}_parm(), and yes now it is more flexble, because one can set
different FPS on different vivi devices. Only I don't know V4L2 ioctls
details well enough and various drivers do things differently. The patch
is below. Is it ok?
   
   Close, but it's not quite there.
   
   You should run the v4l2-compliance tool from the v4l-utils.git repository
   (take the master branch). That will report any errors in your 
   implementation.
   
   In this case g/s_parm doesn't set readbuffers (set it to 1) and if 
   timeperframe
   equals { 0, 0 }, then you should get a nominal framerate (let's stick to 
   29.97
   for that). I would set the nominal framerate whenever the denominator == 
   0.
   
   For vidioc_enum_frameintervals you need to check the IN fields and fill 
   in the
   stepwise struct.
  
  Thanks for pointers and info about v4l2-compliance handy-tool. I've
  tried to correct all the issues you mentioned above and here is the
  patch.
  
  (Only requirement to set stepwise.step to 1.0 for
   V4L2_FRMIVAL_TYPE_CONTINUOUS seems a bit illogical to me, but anyway,
   that's what the V4L2 spec requires...)
  
  Thanks,
  Kirill
  
   8 
  From: Kirill Smelkov k...@mns.spb.ru
  Date: Tue, 23 Oct 2012 16:56:59 +0400
  Subject: [PATCH v3] [media] vivi: Teach it to tune FPS
  
  I was testing my video-over-ethernet subsystem yesterday, and vivi
  seemed to be perfect video source for testing when one don't have lots
  of capture boards and cameras. Only its framerate was hardcoded to
  NTSC's 30fps, while in my country we usually use PAL (25 fps) and I
  needed that to precisely simulate bandwidth.
  
  That's why here is this patch with -enum_frameintervals() and
  -{g,s}_parm() implemented as suggested by Hans Verkuil which passes
  v4l2-compliance and manual testing through v4l2-ctl -P / -p fps.
  
  Regarding newly introduced __get_format(u32 pixelformat) I decided not
  to convert original get_format() to operate on fourcc codes, since = 3
  places in driver need to deal with v4l2_format and otherwise it won't be
  handy.
  
  Thanks.
  
  Signed-off-by: Kirill Smelkov k...@mns.spb.ru
  ---
   drivers/media/platform/vivi.c | 84 
  ++-
   1 file changed, 75 insertions(+), 9 deletions(-)
  
  V3:
  - corrected issues with V4L2 spec compliance as pointed by Hans
Verkuil.
  
  V2:
  
  - reworked FPS setting from module param to via -{g,s}_parm() as 
  suggested
by Hans Verkuil.
  
  
  diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
  index 3e6902a..3adea58 100644
  --- a/drivers/media/platform/vivi.c
  +++ b/drivers/media/platform/vivi.c
  @@ -36,10 +36,6 @@
   
   #define VIVI_MODULE_NAME vivi
   
  -/* Wake up at about 30 fps */
  -#define WAKE_NUMERATOR 30
  -#define WAKE_DENOMINATOR 1001
  -
   #define MAX_WIDTH 1920
   #define MAX_HEIGHT 1200
   
  @@ -69,6 +65,9 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
  megabytes);
   /* Global font descriptor */
   static const u8 *font8x16;
   
  +/* default to NTSC timeperframe */
  +static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
  .denominator = 3};
 
 Keep the name lower case: tpf_default. Upper case is for defines only.

ok

[...]

  @@ -1049,6 +1054,63 @@ static int vidioc_s_input(struct file *file, void 
  *priv, unsigned int i)
  return 0;
   }
   
  +/* timeperframe is arbitrary and continous */
  +static int vidioc_enum_frameintervals(struct file *file, void *priv,
  +struct v4l2_frmivalenum *fival)
  +{
  +   struct vivi_fmt *fmt;
  +
  +   if (fival-index)
  +   return -EINVAL;
  +
  +   fmt = __get_format(fival-pixel_format);
  +   if (!fmt)
  +   return -EINVAL;
  +
  +   /* regarding width width  height - we support any */
  +
  +   fival

[PATCH 0/4] Speedup vivi

2012-11-02 Thread Kirill Smelkov
Hello up there. I was trying to use vivi to generate multiple video streams for
my test-lab environment on atom system and noticed it wastes a lot of cpu.

Please apply some optimization patches.

Thanks,
Kirill

Kirill Smelkov (4):
  [media] vivi: Optimize gen_text()
  [media] vivi: vivi_dev-line[] was not aligned
  [media] vivi: Move computations out of vivi_fillbuf linecopy loop
  [media] vivi: Optimize precalculate_line()

 drivers/media/platform/vivi.c | 94 ++-
 1 file changed, 65 insertions(+), 29 deletions(-)

-- 
1.8.0.316.g291341c

--
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 1/4] [media] vivi: Optimize gen_text()

2012-11-02 Thread Kirill Smelkov
 ││  test   $0x8,%dl
  0,67 ││  mov%di,0x6(%eax)
  5,76 ││  mov%esi,%edi
  1,80 ││  cmove  %ecx,%edi
  4,20 ││  test   $0x4,%dl
  0,86 ││  mov%di,0x8(%eax)
  2,98 ││  mov%esi,%edi
  1,37 ││  cmove  %ecx,%edi
  4,67 ││  test   $0x2,%dl
  0,20 ││  mov%di,0xa(%eax)
  2,78 ││  mov%esi,%edi
  0,75 ││  cmove  %ecx,%edi
  3,92 ││  and$0x1,%edx
  0,75 ││  mov%esi,%edx
  2,59 ││  mov%di,0xc(%eax)
  0,59 ││  cmove  %ecx,%edx
  3,10 ││  mov%dx,0xe(%eax)
  2,39 ││  add$0x10,%eax
  0,51 ││  movzbl (%ebx),%edx
  2,86 ││  test   %dl,%dl
  2,31 │└──jneb0
  0,04 │128:   addl   $0x1,-0x10(%ebp)
  4,00 │   mov-0x1c(%ebp),%eax
  0,04 │   add%eax,-0x18(%ebp)
  0,08 │   cmpl   $0x10,-0x10(%ebp)
   │ ↑ jnea0

which almost goes away from the profile:

# cmdline : /home/kirr/local/perf/bin/perf record -g -a sleep 20
# Samples: 49K of event 'cycles'
# Event count (approx.): 16799780016
#
# Overhead  Command Shared Object   
Symbol
#   ...  
#
27.51% rawv  libc-2.13.so  [.] __memcpy_ssse3
23.77%   vivi-*  [kernel.kallsyms] [k] memcpy
 9.96% Xorg  [unknown] [.] 0xa76f5e12
 4.94%   vivi-*  [vivi][k] gen_text.constprop.6
 4.44% rawv  [vivi][k] gen_twopix
 3.17%   vivi-*  [vivi][k] vivi_fillbuff
 2.45% rawv  [vivi][k] precalculate_line
 1.20%  swapper  [kernel.kallsyms] [k] read_hpet

i.e. gen_twopix() overhead dropped from 49% to 4% and gen_text() loops
from ~8% to ~4%, and overal cycles count dropped from 31551930117 to
16799780016 which is ~1.9x whole workload speedup.

(*) for RGB24 rendering I've introduced x24, which could be thought as
synthetic u24 for simplifying the code. That's done because for
memcpy used for conditional assignment, gcc generates suboptimal code
with more indirections.

Fortunately, in C struct assignment is builtin and that's all we
need from pixeltype for font rendering.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 61 ++-
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index bfac13d..cb2337e 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -245,6 +245,7 @@ struct vivi_dev {
u8 line[MAX_WIDTH * 8];
unsigned int   pixelsize;
u8 alpha_component;
+   u32textfg, textbg;
 };
 
 /* --
@@ -525,33 +526,54 @@ static void precalculate_line(struct vivi_dev *dev)
}
 }
 
+/* need this to do rgb24 rendering */
+typedef struct { u16 __; u8 _; } __attribute__((packed)) x24;
+
 static void gen_text(struct vivi_dev *dev, char *basep,
int y, int x, char *text)
 {
int line;
+   unsigned int width = dev-width;
 
/* Checks if it is possible to show string */
-   if (y + 16 = dev-height || x + strlen(text) * 8 = dev-width)
+   if (y + 16 = dev-height || x + strlen(text) * 8 = width)
return;
 
/* Print stream time */
-   for (line = y; line  y + 16; line++) {
-   int j = 0;
-   char *pos = basep + line * dev-width * dev-pixelsize + x * 
dev-pixelsize;
-   char *s;
-
-   for (s = text; *s; s++) {
-   u8 chr = font8x16[*s * 16 + line - y];
-   int i;
-
-   for (i = 0; i  7; i++, j++) {
-   /* Draw white font on black background */
-   if (chr  (1  (7 - i)))
-   gen_twopix(dev, pos + j * 
dev-pixelsize, WHITE, (x+y)  1);
-   else
-   gen_twopix(dev, pos + j * 
dev-pixelsize, TEXT_BLACK, (x+y)  1);
-   }
-   }
+#define PRINTSTR(PIXTYPE) do { \
+   PIXTYPE fg; \
+   PIXTYPE bg; \
+   memcpy(fg, dev-textfg, sizeof(PIXTYPE)); \
+   memcpy(bg, dev-textbg, sizeof(PIXTYPE)); \
+   \
+   for (line = 0; line  16; line++) { \
+   PIXTYPE *pos = (PIXTYPE *)( basep + ((y + line) * width + x) * 
sizeof(PIXTYPE) );   \
+   u8 *s;  \
+   \
+   for (s = text; *s; s

[PATCH 2/4] [media] vivi: vivi_dev-line[] was not aligned

2012-11-02 Thread Kirill Smelkov
Though dev-line[] is u8 array we work with it as with u16, u24 or u32
pixels, and also pass it to memcpy() and it's better to align it to at
least 4.

Before the patch, on x86 offsetof(vivi_dev, line) was 1003 and after
patch it is 1004.

There is slight performance increase, but I think is is slight, only
because we start copying not from line[0]:

 8  drivers/media/platform/vivi.c
static void vivi_fillbuff(struct vivi_dev *dev, struct vivi_buffer *buf)
{
...

for (h = 0; h  hmax; h++)
memcpy(vbuf + h * wmax * dev-pixelsize,
   dev-line + (dev-mv_count % wmax) * dev-pixelsize,
   wmax * dev-pixelsize);

before:

# cmdline : /home/kirr/local/perf/bin/perf record -g -a sleep 20
#
# Samples: 49K of event 'cycles'
# Event count (approx.): 16799780016
#
# Overhead  Command Shared Object
#   ...  
#
27.51% rawv  libc-2.13.so  [.] __memcpy_ssse3
23.77%   vivi-*  [kernel.kallsyms] [k] memcpy
 9.96% Xorg  [unknown] [.] 0xa76f5e12
 4.94%   vivi-*  [vivi][k] gen_text.constprop.6
 4.44% rawv  [vivi][k] gen_twopix
 3.17%   vivi-*  [vivi][k] vivi_fillbuff
 2.45% rawv  [vivi][k] precalculate_line
 1.20%  swapper  [kernel.kallsyms] [k] read_hpet

23.77%   vivi-*  [kernel.kallsyms] [k] memcpy
 |
 --- memcpy
|
|--99.28%-- vivi_fillbuff
|  vivi_thread
|  kthread
|  ret_from_kernel_thread
 --0.72%-- [...]
after:

# cmdline : /home/kirr/local/perf/bin/perf record -g -a sleep 20
#
# Samples: 49K of event 'cycles'
# Event count (approx.): 16475832370
#
# Overhead  Command   Shared Object
#   ...  ..
#
29.07% rawv  libc-2.13.so[.] __memcpy_ssse3
20.57%   vivi-*  [kernel.kallsyms]   [k] memcpy
10.20% Xorg  [unknown]   [.] 0xa7301494
 5.16%   vivi-*  [vivi]  [k] 
gen_text.constprop.6
 4.43% rawv  [vivi]  [k] gen_twopix
 4.36%   vivi-*  [vivi]  [k] vivi_fillbuff
 2.42% rawv  [vivi]  [k] precalculate_line
 1.33%  swapper  [kernel.kallsyms]   [k] read_hpet

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index cb2337e..ddcc712 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -242,7 +242,7 @@ struct vivi_dev {
unsigned int   field_count;
 
u8 bars[9][3];
-   u8 line[MAX_WIDTH * 8];
+   u8 line[MAX_WIDTH * 8] 
__attribute__((__aligned__(4)));
unsigned int   pixelsize;
u8 alpha_component;
u32textfg, textbg;
-- 
1.8.0.316.g291341c

--
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 3/4] [media] vivi: Move computations out of vivi_fillbuf linecopy loop

2012-11-02 Thread Kirill Smelkov
The dev-mvcount % wmax thing was showing high in profiles (we do it
for each line which ~ 500 per frame)

   │ 10c0 vivi_fillbuff:
 ...
  0,39 │ 70:┌─→mov0x3ff4(%edi),%esi
  0,22 │ 76:│  mov0x2a0(%edi),%eax
  0,30 ││  mov-0x84(%ebp),%ebx
  0,35 ││  mov%eax,%edx
  0,04 ││  mov-0x7c(%ebp),%ecx
  0,35 ││  sar$0x1f,%edx
  0,44 ││  idivl  -0x7c(%ebp)
 21,68 ││  imul   %esi,%ecx
  0,70 ││  imul   %esi,%ebx
  0,52 ││  add-0x88(%ebp),%ebx
  1,65 ││  mov%ebx,%eax
  0,22 ││  imul   %edx,%esi
  0,04 ││  lea0x3f4(%edi,%esi,1),%edx
  2,18 ││→ call   vivi_fillbuff+0xa6
  0,74 ││  addl   $0x1,-0x80(%ebp)
 62,69 ││  mov-0x7c(%ebp),%edx
  1,18 ││  mov-0x80(%ebp),%ecx
  0,35 ││  add%edx,-0x84(%ebp)
  0,61 ││  cmp%ecx,-0x8c(%ebp)
  0,22 │└──jne70

so since all variables stay the same for all iterations let's move
computations out of the loop: the abovementioned division and
width*pixelsize too

before:

# cmdline : /home/kirr/local/perf/bin/perf record -g -a sleep 20
#
# Samples: 49K of event 'cycles'
# Event count (approx.): 16475832370
#
# Overhead  Command   Shared Object
#   ...  ..
#
29.07% rawv  libc-2.13.so[.] __memcpy_ssse3
20.57%   vivi-*  [kernel.kallsyms]   [k] memcpy
10.20% Xorg  [unknown]   [.] 0xa7301494
 5.16%   vivi-*  [vivi]  [k] 
gen_text.constprop.6
 4.43% rawv  [vivi]  [k] gen_twopix
 4.36%   vivi-*  [vivi]  [k] vivi_fillbuff
 2.42% rawv  [vivi]  [k] precalculate_line
 1.33%  swapper  [kernel.kallsyms]   [k] read_hpet

after:

# cmdline : /home/kirr/local/perf/bin/perf record -g -a sleep 20
#
# Samples: 46K of event 'cycles'
# Event count (approx.): 15574200568
#
# Overhead  Command Shared Object
#   ...  
#
27.99% rawv  libc-2.13.so  [.] __memcpy_ssse3
23.29%   vivi-*  [kernel.kallsyms] [k] memcpy
10.30% Xorg  [unknown] [.] 0xa75c98f8
 5.34%   vivi-*  [vivi][k] gen_text.constprop.6
 4.61% rawv  [vivi][k] gen_twopix
 2.64% rawv  [vivi][k] precalculate_line
 1.37%  swapper  [kernel.kallsyms] [k] read_hpet
 0.79% Xorg  [kernel.kallsyms] [k] read_hpet
 0.64% Xorg  [kernel.kallsyms] [k] unix_poll
 0.45% Xorg  [kernel.kallsyms] [k] fget_light
 0.43% rawv  libxcb.so.1.1.0   [.] 0xaae9
 0.40%runsv  [kernel.kallsyms] [k] ext2_try_to_allocate
 0.36% Xorg  [kernel.kallsyms] [k] 
_raw_spin_lock_irqsave
 0.31%   vivi-*  [vivi][k] vivi_fillbuff

(i.e. vivi_fillbuff own overhead is almost gone)

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index ddcc712..0272d07 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -579,22 +579,23 @@ static void gen_text(struct vivi_dev *dev, char *basep,
 
 static void vivi_fillbuff(struct vivi_dev *dev, struct vivi_buffer *buf)
 {
-   int wmax = dev-width;
+   int stride = dev-width * dev-pixelsize;
int hmax = dev-height;
struct timeval ts;
void *vbuf = vb2_plane_vaddr(buf-vb, 0);
unsigned ms;
char str[100];
int h, line = 1;
+   u8 *linestart;
s32 gain;
 
if (!vbuf)
return;
 
+   linestart = dev-line + (dev-mv_count % dev-width) * dev-pixelsize;
+
for (h = 0; h  hmax; h++)
-   memcpy(vbuf + h * wmax * dev-pixelsize,
-  dev-line + (dev-mv_count % wmax) * dev-pixelsize,
-  wmax * dev-pixelsize);
+   memcpy(vbuf + h * stride, linestart, stride);
 
/* Updates stream time */
 
-- 
1.8.0.316.g291341c

--
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 4/4] [media] vivi: Optimize precalculate_line()

2012-11-02 Thread Kirill Smelkov
precalculate_line() is not very high on profile, but it calls expensive
gen_twopix(), so let's polish it too:

call gen_twopix() only once for every color bar and then distribute
the result.

before:

# cmdline : /home/kirr/local/perf/bin/perf record -g -a sleep 20
#
# Samples: 46K of event 'cycles'
# Event count (approx.): 15574200568
#
# Overhead  Command Shared Object
#   ...  
#
27.99% rawv  libc-2.13.so  [.] __memcpy_ssse3
23.29%   vivi-*  [kernel.kallsyms] [k] memcpy
10.30% Xorg  [unknown] [.] 0xa75c98f8
 5.34%   vivi-*  [vivi][k] gen_text.constprop.6
 4.61% rawv  [vivi][k] gen_twopix
 2.64% rawv  [vivi][k] precalculate_line
 1.37%  swapper  [kernel.kallsyms] [k] read_hpet

after:

# cmdline : /home/kirr/local/perf/bin/perf record -g -a sleep 20
#
# Samples: 45K of event 'cycles'
# Event count (approx.): 15561769214
#
# Overhead  Command Shared Object
#   ...  
#
30.73% rawv  libc-2.13.so  [.] __memcpy_ssse3
26.78%   vivi-*  [kernel.kallsyms] [k] memcpy
10.68% Xorg  [unknown] [.] 0xa73015e9
 5.55%   vivi-*  [vivi][k] gen_text.constprop.6
 1.36%  swapper  [kernel.kallsyms] [k] read_hpet
 0.96% Xorg  [kernel.kallsyms] [k] read_hpet
 ...
 0.16% rawv  [vivi][k] precalculate_line
 ...
 0.14% rawv  [vivi][k] gen_twopix

(i.e. gen_twopix and precalculate_line overheads are almost gone)

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 0272d07..37d0af8 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -517,12 +517,22 @@ static void gen_twopix(struct vivi_dev *dev, u8 *buf, int 
colorpos, bool odd)
 
 static void precalculate_line(struct vivi_dev *dev)
 {
-   int w;
-
-   for (w = 0; w  dev-width * 2; w++) {
-   int colorpos = w / (dev-width / 8) % 8;
-
-   gen_twopix(dev, dev-line + w * dev-pixelsize, colorpos, w  
1);
+   unsigned pixsize  = dev-pixelsize;
+   unsigned pixsize2 = 2*pixsize;
+   int colorpos;
+   u8 *pos;
+
+   for (colorpos = 0; colorpos  16; ++colorpos) {
+   u8 pix[8];
+   int wstart =  colorpos* dev-width / 8;
+   int wend   = (colorpos+1) * dev-width / 8;
+   int w;
+
+   gen_twopix(dev, pix[0],colorpos % 8, 0);
+   gen_twopix(dev, pix[pixsize],  colorpos % 8, 1);
+
+   for (w = wstart/2*2, pos = dev-line + w*pixsize; w  wend; w 
+= 2, pos += pixsize2)
+   memcpy(pos, pix, pixsize2);
}
 }
 
-- 
1.8.0.316.g291341c

--
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 v3] [media] vivi: Teach it to tune FPS

2012-11-02 Thread Kirill Smelkov
On Tue, Oct 23, 2012 at 05:35:21PM +0400, Kirill Smelkov wrote:
 On Tue, Oct 23, 2012 at 08:40:04AM +0200, Hans Verkuil wrote:
  On Mon October 22 2012 19:01:40 Kirill Smelkov wrote:
   On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
 I was testing my video-over-ethernet subsystem today, and vivi seemed 
 to
 be perfect video source for testing when one don't have lots of 
 capture
 boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
 while in my country we usually use PAL (25 fps). That's why the patch.
 Thanks.

Rather than introducing a module option, it's much nicer if you can
implement enum_frameintervals and g/s_parm. This can be made quite 
flexible
allowing you to also support 50/59.94/60 fps.
   
   Thanks for feedback. I've reworked the patch for FPS to be set via
   -{g,s}_parm(), and yes now it is more flexble, because one can set
   different FPS on different vivi devices. Only I don't know V4L2 ioctls
   details well enough and various drivers do things differently. The patch
   is below. Is it ok?
  
  Close, but it's not quite there.
  
  You should run the v4l2-compliance tool from the v4l-utils.git repository
  (take the master branch). That will report any errors in your 
  implementation.
  
  In this case g/s_parm doesn't set readbuffers (set it to 1) and if 
  timeperframe
  equals { 0, 0 }, then you should get a nominal framerate (let's stick to 
  29.97
  for that). I would set the nominal framerate whenever the denominator == 0.
  
  For vidioc_enum_frameintervals you need to check the IN fields and fill in 
  the
  stepwise struct.
 
 Thanks for pointers and info about v4l2-compliance handy-tool. I've
 tried to correct all the issues you mentioned above and here is the
 patch.
 
 (Only requirement to set stepwise.step to 1.0 for
  V4L2_FRMIVAL_TYPE_CONTINUOUS seems a bit illogical to me, but anyway,
  that's what the V4L2 spec requires...)
 
 Thanks,
 Kirill
 
  8 
 From: Kirill Smelkov k...@mns.spb.ru
 Date: Tue, 23 Oct 2012 16:56:59 +0400
 Subject: [PATCH v3] [media] vivi: Teach it to tune FPS
 
 I was testing my video-over-ethernet subsystem yesterday, and vivi
 seemed to be perfect video source for testing when one don't have lots
 of capture boards and cameras. Only its framerate was hardcoded to
 NTSC's 30fps, while in my country we usually use PAL (25 fps) and I
 needed that to precisely simulate bandwidth.
 
 That's why here is this patch with -enum_frameintervals() and
 -{g,s}_parm() implemented as suggested by Hans Verkuil which passes
 v4l2-compliance and manual testing through v4l2-ctl -P / -p fps.
 
 Regarding newly introduced __get_format(u32 pixelformat) I decided not
 to convert original get_format() to operate on fourcc codes, since = 3
 places in driver need to deal with v4l2_format and otherwise it won't be
 handy.
 
 Thanks.
 
 Signed-off-by: Kirill Smelkov k...@mns.spb.ru
 ---
  drivers/media/platform/vivi.c | 84 
 ++-
  1 file changed, 75 insertions(+), 9 deletions(-)
 
 V3:
 - corrected issues with V4L2 spec compliance as pointed by Hans
   Verkuil.
 
 V2:
 
 - reworked FPS setting from module param to via -{g,s}_parm() as 
 suggested
   by Hans Verkuil.
 
 
 diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
 index 3e6902a..3adea58 100644
 --- a/drivers/media/platform/vivi.c
 +++ b/drivers/media/platform/vivi.c
 @@ -36,10 +36,6 @@
  
  #define VIVI_MODULE_NAME vivi
  
 -/* Wake up at about 30 fps */
 -#define WAKE_NUMERATOR 30
 -#define WAKE_DENOMINATOR 1001
 -
  #define MAX_WIDTH 1920
  #define MAX_HEIGHT 1200
  
 @@ -69,6 +65,9 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
 megabytes);
  /* Global font descriptor */
  static const u8 *font8x16;
  
 +/* default to NTSC timeperframe */
 +static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
 .denominator = 3};
 +
  #define dprintk(dev, level, fmt, arg...) \
   v4l2_dbg(level, debug, dev-v4l2_dev, fmt, ## arg)
  
 @@ -150,14 +149,14 @@ static struct vivi_fmt formats[] = {
   },
  };
  
 -static struct vivi_fmt *get_format(struct v4l2_format *f)
 +static struct vivi_fmt *__get_format(u32 pixelformat)
  {
   struct vivi_fmt *fmt;
   unsigned int k;
  
   for (k = 0; k  ARRAY_SIZE(formats); k++) {
   fmt = formats[k];
 - if (fmt-fourcc == f-fmt.pix.pixelformat)
 + if (fmt-fourcc == pixelformat)
   break;
   }
  
 @@ -167,6 +166,11 @@ static struct vivi_fmt *get_format(struct v4l2_format *f)
   return formats[k];
  }
  
 +static struct vivi_fmt *get_format(struct v4l2_format *f)
 +{
 + return __get_format(f-fmt.pix.pixelformat);
 +}
 +
  /* buffer for one video frame */
  struct vivi_buffer {
   /* common v4l buffer stuff -- must be first */
 @@ -232,6 +236,7 @@ struct

Re: [PATCH 0/4] Speedup vivi

2012-11-02 Thread Kirill Smelkov
On Fri, Nov 02, 2012 at 02:48:43PM +0100, Hans Verkuil wrote:
 On Fri November 2 2012 14:10:29 Kirill Smelkov wrote:
  Hello up there. I was trying to use vivi to generate multiple video streams 
  for
  my test-lab environment on atom system and noticed it wastes a lot of cpu.
  
  Please apply some optimization patches.
 
 Looks good!
 
 For the whole series:
 
 Acked-by: Hans Verkuil hans.verk...@cisco.com

Thanks a lot!
--
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 v3] [media] vivi: Teach it to tune FPS

2012-11-02 Thread Kirill Smelkov
On Fri, Nov 02, 2012 at 03:42:21PM +0100, Hans Verkuil wrote:
 Thanks for the ping, I forgot about this patch...

Thanks for the reply. Have to run now - will try to rework it in several
days.

Thanks again,
Kirill


 On Tue October 23 2012 15:35:21 Kirill Smelkov wrote:
  On Tue, Oct 23, 2012 at 08:40:04AM +0200, Hans Verkuil wrote:
   On Mon October 22 2012 19:01:40 Kirill Smelkov wrote:
On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
 On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
  I was testing my video-over-ethernet subsystem today, and vivi 
  seemed to
  be perfect video source for testing when one don't have lots of 
  capture
  boards and cameras. Only its framerate was hardcoded to NTSC's 
  30fps,
  while in my country we usually use PAL (25 fps). That's why the 
  patch.
  Thanks.
 
 Rather than introducing a module option, it's much nicer if you can
 implement enum_frameintervals and g/s_parm. This can be made quite 
 flexible
 allowing you to also support 50/59.94/60 fps.

Thanks for feedback. I've reworked the patch for FPS to be set via
-{g,s}_parm(), and yes now it is more flexble, because one can set
different FPS on different vivi devices. Only I don't know V4L2 ioctls
details well enough and various drivers do things differently. The patch
is below. Is it ok?
   
   Close, but it's not quite there.
   
   You should run the v4l2-compliance tool from the v4l-utils.git repository
   (take the master branch). That will report any errors in your 
   implementation.
   
   In this case g/s_parm doesn't set readbuffers (set it to 1) and if 
   timeperframe
   equals { 0, 0 }, then you should get a nominal framerate (let's stick to 
   29.97
   for that). I would set the nominal framerate whenever the denominator == 
   0.
   
   For vidioc_enum_frameintervals you need to check the IN fields and fill 
   in the
   stepwise struct.
  
  Thanks for pointers and info about v4l2-compliance handy-tool. I've
  tried to correct all the issues you mentioned above and here is the
  patch.
  
  (Only requirement to set stepwise.step to 1.0 for
   V4L2_FRMIVAL_TYPE_CONTINUOUS seems a bit illogical to me, but anyway,
   that's what the V4L2 spec requires...)
  
  Thanks,
  Kirill
  
   8 
  From: Kirill Smelkov k...@mns.spb.ru
  Date: Tue, 23 Oct 2012 16:56:59 +0400
  Subject: [PATCH v3] [media] vivi: Teach it to tune FPS
  
  I was testing my video-over-ethernet subsystem yesterday, and vivi
  seemed to be perfect video source for testing when one don't have lots
  of capture boards and cameras. Only its framerate was hardcoded to
  NTSC's 30fps, while in my country we usually use PAL (25 fps) and I
  needed that to precisely simulate bandwidth.
  
  That's why here is this patch with -enum_frameintervals() and
  -{g,s}_parm() implemented as suggested by Hans Verkuil which passes
  v4l2-compliance and manual testing through v4l2-ctl -P / -p fps.
  
  Regarding newly introduced __get_format(u32 pixelformat) I decided not
  to convert original get_format() to operate on fourcc codes, since = 3
  places in driver need to deal with v4l2_format and otherwise it won't be
  handy.
  
  Thanks.
  
  Signed-off-by: Kirill Smelkov k...@mns.spb.ru
  ---
   drivers/media/platform/vivi.c | 84 
  ++-
   1 file changed, 75 insertions(+), 9 deletions(-)
  
  V3:
  - corrected issues with V4L2 spec compliance as pointed by Hans
Verkuil.
  
  V2:
  
  - reworked FPS setting from module param to via -{g,s}_parm() as 
  suggested
by Hans Verkuil.
  
  
  diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
  index 3e6902a..3adea58 100644
  --- a/drivers/media/platform/vivi.c
  +++ b/drivers/media/platform/vivi.c
  @@ -36,10 +36,6 @@
   
   #define VIVI_MODULE_NAME vivi
   
  -/* Wake up at about 30 fps */
  -#define WAKE_NUMERATOR 30
  -#define WAKE_DENOMINATOR 1001
  -
   #define MAX_WIDTH 1920
   #define MAX_HEIGHT 1200
   
  @@ -69,6 +65,9 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
  megabytes);
   /* Global font descriptor */
   static const u8 *font8x16;
   
  +/* default to NTSC timeperframe */
  +static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, 
  .denominator = 3};
 
 Keep the name lower case: tpf_default. Upper case is for defines only.
 
  +
   #define dprintk(dev, level, fmt, arg...) \
  v4l2_dbg(level, debug, dev-v4l2_dev, fmt, ## arg)
   
  @@ -150,14 +149,14 @@ static struct vivi_fmt formats[] = {
  },
   };
   
  -static struct vivi_fmt *get_format(struct v4l2_format *f)
  +static struct vivi_fmt *__get_format(u32 pixelformat)
   {
  struct vivi_fmt *fmt;
  unsigned int k;
   
  for (k = 0; k  ARRAY_SIZE(formats); k++) {
  fmt = formats[k];
  -   if (fmt-fourcc == f-fmt.pix.pixelformat)
  +   if (fmt-fourcc == pixelformat

[PATCH v3] [media] vivi: Teach it to tune FPS

2012-10-23 Thread Kirill Smelkov
On Tue, Oct 23, 2012 at 08:40:04AM +0200, Hans Verkuil wrote:
 On Mon October 22 2012 19:01:40 Kirill Smelkov wrote:
  On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
   On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
I was testing my video-over-ethernet subsystem today, and vivi seemed to
be perfect video source for testing when one don't have lots of capture
boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
while in my country we usually use PAL (25 fps). That's why the patch.
Thanks.
   
   Rather than introducing a module option, it's much nicer if you can
   implement enum_frameintervals and g/s_parm. This can be made quite 
   flexible
   allowing you to also support 50/59.94/60 fps.
  
  Thanks for feedback. I've reworked the patch for FPS to be set via
  -{g,s}_parm(), and yes now it is more flexble, because one can set
  different FPS on different vivi devices. Only I don't know V4L2 ioctls
  details well enough and various drivers do things differently. The patch
  is below. Is it ok?
 
 Close, but it's not quite there.
 
 You should run the v4l2-compliance tool from the v4l-utils.git repository
 (take the master branch). That will report any errors in your implementation.
 
 In this case g/s_parm doesn't set readbuffers (set it to 1) and if 
 timeperframe
 equals { 0, 0 }, then you should get a nominal framerate (let's stick to 29.97
 for that). I would set the nominal framerate whenever the denominator == 0.
 
 For vidioc_enum_frameintervals you need to check the IN fields and fill in the
 stepwise struct.

Thanks for pointers and info about v4l2-compliance handy-tool. I've
tried to correct all the issues you mentioned above and here is the
patch.

(Only requirement to set stepwise.step to 1.0 for
 V4L2_FRMIVAL_TYPE_CONTINUOUS seems a bit illogical to me, but anyway,
 that's what the V4L2 spec requires...)

Thanks,
Kirill

 8 
From: Kirill Smelkov k...@mns.spb.ru
Date: Tue, 23 Oct 2012 16:56:59 +0400
Subject: [PATCH v3] [media] vivi: Teach it to tune FPS

I was testing my video-over-ethernet subsystem yesterday, and vivi
seemed to be perfect video source for testing when one don't have lots
of capture boards and cameras. Only its framerate was hardcoded to
NTSC's 30fps, while in my country we usually use PAL (25 fps) and I
needed that to precisely simulate bandwidth.

That's why here is this patch with -enum_frameintervals() and
-{g,s}_parm() implemented as suggested by Hans Verkuil which passes
v4l2-compliance and manual testing through v4l2-ctl -P / -p fps.

Regarding newly introduced __get_format(u32 pixelformat) I decided not
to convert original get_format() to operate on fourcc codes, since = 3
places in driver need to deal with v4l2_format and otherwise it won't be
handy.

Thanks.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 84 ++-
 1 file changed, 75 insertions(+), 9 deletions(-)

V3:
- corrected issues with V4L2 spec compliance as pointed by Hans
  Verkuil.

V2:

- reworked FPS setting from module param to via -{g,s}_parm() as suggested
  by Hans Verkuil.


diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 3e6902a..3adea58 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -36,10 +36,6 @@
 
 #define VIVI_MODULE_NAME vivi
 
-/* Wake up at about 30 fps */
-#define WAKE_NUMERATOR 30
-#define WAKE_DENOMINATOR 1001
-
 #define MAX_WIDTH 1920
 #define MAX_HEIGHT 1200
 
@@ -69,6 +65,9 @@ MODULE_PARM_DESC(vid_limit, capture memory limit in 
megabytes);
 /* Global font descriptor */
 static const u8 *font8x16;
 
+/* default to NTSC timeperframe */
+static const struct v4l2_fract TPF_DEFAULT = {.numerator = 1001, .denominator 
= 3};
+
 #define dprintk(dev, level, fmt, arg...) \
v4l2_dbg(level, debug, dev-v4l2_dev, fmt, ## arg)
 
@@ -150,14 +149,14 @@ static struct vivi_fmt formats[] = {
},
 };
 
-static struct vivi_fmt *get_format(struct v4l2_format *f)
+static struct vivi_fmt *__get_format(u32 pixelformat)
 {
struct vivi_fmt *fmt;
unsigned int k;
 
for (k = 0; k  ARRAY_SIZE(formats); k++) {
fmt = formats[k];
-   if (fmt-fourcc == f-fmt.pix.pixelformat)
+   if (fmt-fourcc == pixelformat)
break;
}
 
@@ -167,6 +166,11 @@ static struct vivi_fmt *get_format(struct v4l2_format *f)
return formats[k];
 }
 
+static struct vivi_fmt *get_format(struct v4l2_format *f)
+{
+   return __get_format(f-fmt.pix.pixelformat);
+}
+
 /* buffer for one video frame */
 struct vivi_buffer {
/* common v4l buffer stuff -- must be first */
@@ -232,6 +236,7 @@ struct vivi_dev {
 
/* video capture */
struct vivi_fmt*fmt;
+   struct v4l2_fract  timeperframe;
unsigned int   width, height;
struct vb2_queue

[PATCH] [media] vivi: Kill TSTAMP_* macros

2012-10-23 Thread Kirill Smelkov
Usage of TSTAMP_* macros has gone in 2010 in 730947bc (V4L/DVB: vivi:
clean up and a major overhaul) but the macros remain. Say goodbye to
them.

Cc: Hans Verkuil hverk...@xs4all.nl
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 3adea58..bfac13d 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -356,11 +356,6 @@ static void precalculate_bars(struct vivi_dev *dev)
}
 }
 
-#define TSTAMP_MIN_Y   24
-#define TSTAMP_MAX_Y   (TSTAMP_MIN_Y + 15)
-#define TSTAMP_INPUT_X 10
-#define TSTAMP_MIN_X   (54 + TSTAMP_INPUT_X)
-
 /* 'odd' is true for pixels 1, 3, 5, etc. and false for pixels 0, 2, 4, etc. */
 static void gen_twopix(struct vivi_dev *dev, u8 *buf, int colorpos, bool odd)
 {
-- 
1.8.0.rc3.331.g5b9a629

--
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 1/2] [media] vivi: Kill BUFFER_TIMEOUT macro

2012-10-22 Thread Kirill Smelkov
Usage of BUFFER_TIMEOUT has gone in 2008 in 78718e5d (V4L/DVB (7492):
vivi: Simplify the vivi driver and avoid deadlocks), but the macro
remains. Say goodbye to it.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index b366b05..3e6902a 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -39,7 +39,6 @@
 /* Wake up at about 30 fps */
 #define WAKE_NUMERATOR 30
 #define WAKE_DENOMINATOR 1001
-#define BUFFER_TIMEOUT msecs_to_jiffies(500)  /* 0.5 seconds */
 
 #define MAX_WIDTH 1920
 #define MAX_HEIGHT 1200
-- 
1.8.0.rc3.331.g5b9a629

--
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 2/2] [media] vivi: Teach it to tune FPS

2012-10-22 Thread Kirill Smelkov
I was testing my video-over-ethernet subsystem today, and vivi seemed to
be perfect video source for testing when one don't have lots of capture
boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
while in my country we usually use PAL (25 fps). That's why the patch.
Thanks.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 3e6902a..48325f4 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -36,10 +36,6 @@
 
 #define VIVI_MODULE_NAME vivi
 
-/* Wake up at about 30 fps */
-#define WAKE_NUMERATOR 30
-#define WAKE_DENOMINATOR 1001
-
 #define MAX_WIDTH 1920
 #define MAX_HEIGHT 1200
 
@@ -58,6 +54,11 @@ static unsigned n_devs = 1;
 module_param(n_devs, uint, 0644);
 MODULE_PARM_DESC(n_devs, number of video devices to create);
 
+static struct v4l2_fract fps = { 3, 1001 }; /* ~ 30 fps by default */
+static unsigned __fps[2], __nfps;
+module_param_array_named(fps, __fps, uint, __nfps, 0644);
+MODULE_PARM_DESC(fps, frames per second as ratio (e.g. 3,1001 or 25,1));
+
 static unsigned debug;
 module_param(debug, uint, 0644);
 MODULE_PARM_DESC(debug, activates debug info);
@@ -661,7 +662,7 @@ static void vivi_thread_tick(struct vivi_dev *dev)
 }
 
 #define frames_to_ms(frames)   \
-   ((frames * WAKE_NUMERATOR * 1000) / WAKE_DENOMINATOR)
+   ((frames * fps.denominator * 1000) / fps.numerator)
 
 static void vivi_sleep(struct vivi_dev *dev)
 {
@@ -1376,6 +1377,13 @@ static int __init vivi_init(void)
if (n_devs = 0)
n_devs = 1;
 
+   if (__nfps  0) {
+   fps.numerator   = __fps[0];
+   fps.denominator = (__nfps  1) ? __fps[1] : 1;
+   }
+   if (fps.numerator = 0)
+   fps.numerator = 1;
+
for (i = 0; i  n_devs; i++) {
ret = vivi_create_instance(i);
if (ret) {
-- 
1.8.0.rc3.331.g5b9a629

--
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 2/2] [media] vivi: Teach it to tune FPS

2012-10-22 Thread Kirill Smelkov
On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
 On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
  I was testing my video-over-ethernet subsystem today, and vivi seemed to
  be perfect video source for testing when one don't have lots of capture
  boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
  while in my country we usually use PAL (25 fps). That's why the patch.
  Thanks.
 
 Rather than introducing a module option, it's much nicer if you can
 implement enum_frameintervals and g/s_parm. This can be made quite flexible
 allowing you to also support 50/59.94/60 fps.

Thanks for feedback. I've reworked the patch for FPS to be set via
-{g,s}_parm(), and yes now it is more flexble, because one can set
different FPS on different vivi devices. Only I don't know V4L2 ioctls
details well enough and various drivers do things differently. The patch
is below. Is it ok?

Thanks,
Kirill


 8 
From: Kirill Smelkov k...@mns.spb.ru
Date: Mon, 22 Oct 2012 17:25:24 +0400
Subject: [PATCH v2] [media] vivi: Teach it to tune FPS

I was testing my video-over-ethernet subsystem today, and vivi seemed to
be perfect video source for testing when one don't have lots of capture
boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
while in my country we usually use PAL (25 fps).

That's why here is this patch with -enum_frameintervals and
-{g,s}_parm implemented as suggested by Hans Verkuil. Not sure I've
done -g_parm right -- some drivers clear parm memory before setting
fields, some don't. As at is at least it works for me (tested via
v4l2-ctl -P / -p fps).

Thanks.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/platform/vivi.c | 50 +--
 1 file changed, 43 insertions(+), 7 deletions(-)

V2:

- reworked FPS setting from module param to via -{g,s}_parm() as suggested
  by Hans Verkuil.

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 3e6902a..c0855a5 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -36,10 +36,6 @@
 
 #define VIVI_MODULE_NAME vivi
 
-/* Wake up at about 30 fps */
-#define WAKE_NUMERATOR 30
-#define WAKE_DENOMINATOR 1001
-
 #define MAX_WIDTH 1920
 #define MAX_HEIGHT 1200
 
@@ -232,6 +228,7 @@ struct vivi_dev {
 
/* video capture */
struct vivi_fmt*fmt;
+   struct v4l2_fract  timeperframe;
unsigned int   width, height;
struct vb2_queue   vb_vidq;
unsigned int   field_count;
@@ -660,8 +657,8 @@ static void vivi_thread_tick(struct vivi_dev *dev)
dprintk(dev, 2, [%p/%d] done\n, buf, buf-vb.v4l2_buf.index);
 }
 
-#define frames_to_ms(frames)   \
-   ((frames * WAKE_NUMERATOR * 1000) / WAKE_DENOMINATOR)
+#define frames_to_ms(dev, frames)  \
+   ((frames * dev-timeperframe.numerator * 1000) / 
dev-timeperframe.denominator)
 
 static void vivi_sleep(struct vivi_dev *dev)
 {
@@ -677,7 +674,8 @@ static void vivi_sleep(struct vivi_dev *dev)
goto stop_task;
 
/* Calculate time to wake up */
-   timeout = msecs_to_jiffies(frames_to_ms(1));
+   timeout = msecs_to_jiffies(frames_to_ms(dev, 1));
+
 
vivi_thread_tick(dev);
 
@@ -1049,6 +1047,39 @@ static int vidioc_s_input(struct file *file, void *priv, 
unsigned int i)
return 0;
 }
 
+/* timeperframe is arbitrary and continous */
+static int vidioc_enum_frameintervals(struct file *file, void *priv,
+struct v4l2_frmivalenum *fival)
+{
+   fival-type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
+   return 0;
+}
+
+static int vidioc_g_parm(struct file *file, void *priv, struct v4l2_streamparm 
*parm)
+{
+   struct vivi_dev *dev = video_drvdata(file);
+
+   if (parm-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+   return -EINVAL;
+
+   parm-parm.capture.capability   = V4L2_CAP_TIMEPERFRAME;
+   parm-parm.capture.timeperframe = dev-timeperframe;
+   return 0;
+}
+
+static int vidioc_s_parm(struct file *file, void *priv, struct v4l2_streamparm 
*parm)
+{
+   struct vivi_dev *dev = video_drvdata(file);
+
+   if (parm-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+   return -EINVAL;
+
+   dev-timeperframe.numerator = 
parm-parm.capture.timeperframe.numerator;
+   dev-timeperframe.denominator   = 
parm-parm.capture.timeperframe.denominator ?: 1;
+
+   return 0;
+}
+
 /* --- controls -- */
 
 static int vivi_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -1207,6 +1238,9 @@ static const struct v4l2_ioctl_ops vivi_ioctl_ops = {
.vidioc_enum_input= vidioc_enum_input,
.vidioc_g_input   = vidioc_g_input,
.vidioc_s_input   = vidioc_s_input,
+   .vidioc_enum_frameintervals = vidioc_enum_frameintervals

Re: [PATCH 2/2] [media] vivi: Teach it to tune FPS

2012-10-22 Thread Kirill Smelkov
On Mon, Oct 22, 2012 at 09:01:39PM +0400, Kirill Smelkov wrote:
 On Mon, Oct 22, 2012 at 04:16:14PM +0200, Hans Verkuil wrote:
  On Mon October 22 2012 15:54:44 Kirill Smelkov wrote:
   I was testing my video-over-ethernet subsystem today, and vivi seemed to
   be perfect video source for testing when one don't have lots of capture
   boards and cameras. Only its framerate was hardcoded to NTSC's 30fps,
   while in my country we usually use PAL (25 fps). That's why the patch.
   Thanks.
  
  Rather than introducing a module option, it's much nicer if you can
  implement enum_frameintervals and g/s_parm. This can be made quite flexible
  allowing you to also support 50/59.94/60 fps.
 
 Thanks for feedback. I've reworked the patch for FPS to be set via
 -{g,s}_parm(), and yes now it is more flexble, because one can set

By the way, here is what I've found while working on the abovementioned
patch:

 8 
From: Kirill Smelkov k...@mns.spb.ru
Date: Mon, 22 Oct 2012 21:14:01 +0400
Subject: [PATCH] v4l2: Fix typo in struct v4l2_captureparm description

Judging from what drivers do and from my experience temeperframe
fraction is set in seconds - look e.g. here

static int bttv_g_parm(struct file *file, void *f,
struct v4l2_streamparm *parm)
{
struct bttv_fh *fh = f;
struct bttv *btv = fh-btv;

v4l2_video_std_frame_period(bttv_tvnorms[btv-tvnorm].v4l2_id,
parm-parm.capture.timeperframe);

...

void v4l2_video_std_frame_period(int id, struct v4l2_fract *frameperiod)
{
if (id  V4L2_STD_525_60) {
frameperiod-numerator = 1001;
frameperiod-denominator = 3;
} else {
frameperiod-numerator = 1;
frameperiod-denominator = 25;
}

and also v4l2-ctl in userspace decodes this as seconds:

if (doioctl(fd, VIDIOC_G_PARM, parm, VIDIOC_G_PARM) == 0) {
const struct v4l2_fract tf = parm.parm.capture.timeperframe;

...

printf(\tFrames per second: %.3f (%d/%d)\n,
(1.0 * tf.denominator) / tf.numerator,
tf.denominator, tf.numerator);

The typo was there from day 1 - added in 2002 in e028b61b ([PATCH]
add v4l2 api)(*)

(*) found in history tree
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 include/uapi/linux/videodev2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 57bfa59..2fff7ff 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -726,29 +726,29 @@ struct v4l2_window {
__u32   field;   /* enum v4l2_field */
__u32   chromakey;
struct v4l2_clip__user *clips;
__u32   clipcount;
void__user *bitmap;
__u8global_alpha;
 };
 
 /*
  * C A P T U R E   P A R A M E T E R S
  */
 struct v4l2_captureparm {
__u32  capability;/*  Supported modes */
__u32  capturemode;   /*  Current mode */
-   struct v4l2_fract  timeperframe;  /*  Time per frame in .1us units */
+   struct v4l2_fract  timeperframe;  /*  Time per frame in seconds */
__u32  extendedmode;  /*  Driver-specific extensions */
__u32  readbuffers;   /*  # of buffers for read */
__u32  reserved[4];
 };
 
 /*  Flags for 'capability' and 'capturemode' fields */
 #define V4L2_MODE_HIGHQUALITY  0x0001  /*  High quality imaging mode */
 #define V4L2_CAP_TIMEPERFRAME  0x1000  /*  timeperframe field is supported */
 
 struct v4l2_outputparm {
__u32  capability;   /*  Supported modes */
__u32  outputmode;   /*  Current mode */
struct v4l2_fract  timeperframe; /*  Time per frame in seconds */
__u32  extendedmode; /*  Driver-specific extensions */
-- 
1.8.0.rc3.331.g5b9a629

--
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:v4l-dvb/for_v3.1] [media] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam on HP Mini 5103 netbook

2011-07-28 Thread Kirill Smelkov
On Wed, Jul 27, 2011 at 09:42:08PM +0200, Mauro Carvalho Chehab wrote:
 This is an automatic generated email to let you know that the following patch 
 were queued at the 
 http://git.linuxtv.org/media_tree.git tree:
 
 Subject: [media] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam on HP Mini 
 5103 netbook
 Author:  Kirill Smelkov k...@mns.spb.ru
 Date:Fri Jul 22 11:47:22 2011 -0300

Thanks


 The camera there identifies itself as being manufactured by Cheng Uei
 Precision Industry Co., Ltd (Foxlink), and product is titled as HP
 Webcam [2 MP Fixed].
 
 I was trying to get 2 USB video capture devices to work simultaneously,
 and noticed that the above mentioned webcam always requires packet size
 = 3072 bytes per micro frame (~= 23.4 MB/s isoc bandwidth), which is far
 more than enough to get standard NTSC 640x480x2x30 = ~17.6 MB/s isoc
 bandwidth.
 
 As there are alt interfaces with smaller MxPS
 
 T:  Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  2 Spd=480  MxCh= 0
 D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
 P:  Vendor=05c8 ProdID=0403 Rev= 1.06
 S:  Manufacturer=Foxlink
 S:  Product=HP Webcam [2 MP Fixed]
 S:  SerialNumber=200909240102
 C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
 A:  FirstIf#= 0 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
 I:* If#= 0 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 Driver=uvcvideo
 E:  Ad=83(I) Atr=03(Int.) MxPS=  16 Ivl=4ms
 I:* If#= 1 Alt= 0 #EPs= 0 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 I:  If#= 1 Alt= 1 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS= 128 Ivl=125us
 I:  If#= 1 Alt= 2 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS= 512 Ivl=125us
 I:  If#= 1 Alt= 3 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=1024 Ivl=125us
 I:  If#= 1 Alt= 4 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=1536 Ivl=125us
 I:  If#= 1 Alt= 5 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=2048 Ivl=125us
 I:  If#= 1 Alt= 6 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=2688 Ivl=125us
 I:  If#= 1 Alt= 7 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=3072 Ivl=125us
 
 UVC_QUIRK_FIX_BANDWIDTH helps here and NTSC video can be served with
 MxPS=2688 i.e. 20.5 MB/s isoc bandwidth.
 
 In terms of microframe time allocation, before the quirk NTSC video
 required 60 usecs / microframe and 53 usecs / microframe after.
 
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Signed-off-by: Kirill Smelkov k...@mns.spb.ru
 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com


May I ask, why you removed the reference to cc62a7eb? Original patch
description contained the following paragraph just before sob

Now with tweaked ehci-hcd to allow up to 90% isoc bandwidth (cc62a7eb
USB: EHCI: Allow users to override 80% max periodic bandwidth) I can
capture two video sources -- PAL 720x576 YUV422 @25fps + NTSC 640x480
YUV422 @30fps simultaneously.  Hooray!


which was removed on applying.



Thanks beforehand for answering,
Kirill
--
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:v4l-dvb/for_v3.1] [media] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam on HP Mini 5103 netbook

2011-07-28 Thread Kirill Smelkov
Mauro, thanks for answering,

On Thu, Jul 28, 2011 at 10:45:29AM -0300, Mauro Carvalho Chehab wrote:
 Em 28-07-2011 08:42, Kirill Smelkov escreveu:
  On Wed, Jul 27, 2011 at 09:42:08PM +0200, Mauro Carvalho Chehab wrote:
  This is an automatic generated email to let you know that the following 
  patch were queued at the 
  http://git.linuxtv.org/media_tree.git tree:
 
  Subject: [media] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam on HP Mini 
  5103 netbook
  Author:  Kirill Smelkov k...@mns.spb.ru
  Date:Fri Jul 22 11:47:22 2011 -0300
  
  Thanks
  
  
  The camera there identifies itself as being manufactured by Cheng Uei
  Precision Industry Co., Ltd (Foxlink), and product is titled as HP
  Webcam [2 MP Fixed].
 
  I was trying to get 2 USB video capture devices to work simultaneously,
  and noticed that the above mentioned webcam always requires packet size
  = 3072 bytes per micro frame (~= 23.4 MB/s isoc bandwidth), which is far
  more than enough to get standard NTSC 640x480x2x30 = ~17.6 MB/s isoc
  bandwidth.
 
  As there are alt interfaces with smaller MxPS
 
  T:  Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  2 Spd=480  MxCh= 0
  D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
  P:  Vendor=05c8 ProdID=0403 Rev= 1.06
  S:  Manufacturer=Foxlink
  S:  Product=HP Webcam [2 MP Fixed]
  S:  SerialNumber=200909240102
  C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
  A:  FirstIf#= 0 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
  I:* If#= 0 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 Driver=uvcvideo
  E:  Ad=83(I) Atr=03(Int.) MxPS=  16 Ivl=4ms
  I:* If#= 1 Alt= 0 #EPs= 0 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
  I:  If#= 1 Alt= 1 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS= 128 Ivl=125us
  I:  If#= 1 Alt= 2 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS= 512 Ivl=125us
  I:  If#= 1 Alt= 3 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=1024 Ivl=125us
  I:  If#= 1 Alt= 4 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=1536 Ivl=125us
  I:  If#= 1 Alt= 5 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=2048 Ivl=125us
  I:  If#= 1 Alt= 6 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=2688 Ivl=125us
  I:  If#= 1 Alt= 7 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=3072 Ivl=125us
 
  UVC_QUIRK_FIX_BANDWIDTH helps here and NTSC video can be served with
  MxPS=2688 i.e. 20.5 MB/s isoc bandwidth.
 
  In terms of microframe time allocation, before the quirk NTSC video
  required 60 usecs / microframe and 53 usecs / microframe after.
 
  Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  Signed-off-by: Kirill Smelkov k...@mns.spb.ru
  Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
  
  
  May I ask, why you removed the reference to cc62a7eb? Original patch
  description contained the following paragraph just before sob
  
  Now with tweaked ehci-hcd to allow up to 90% isoc bandwidth (cc62a7eb
  USB: EHCI: Allow users to override 80% max periodic bandwidth) I can
  capture two video sources -- PAL 720x576 YUV422 @25fps + NTSC 640x480
  YUV422 @30fps simultaneously.  Hooray!
  
  
  which was removed on applying.
 
 For a few reasons:
 1) I was not sure if the changeset reference was not changed when

And it was not changed, because when I referenced it, it was already in
Greg's usb tree, and Linus pulls from Greg and Greg does not destroy
history as far as I can tell...

merged upstream, and I was too lazy^Wbusy to double check ;)

It's just doing `git log linus/master | grep cc62a7eb`. Compared or less
time to editing...


 2) Reducing the amount of bandwidth used is good, even without trying to use
two webcams;
 3) The bandwidth override patch is independent of this one.

Yes, but coupled, and also they were motivated by each other. Could we
please merge my comment back, or is it too late?


Thanks,
Kirill
--
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:v4l-dvb/for_v3.1] [media] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam on HP Mini 5103 netbook

2011-07-28 Thread Kirill Smelkov
On Thu, Jul 28, 2011 at 11:55:04AM -0300, Mauro Carvalho Chehab wrote:
 Em 28-07-2011 10:54, Kirill Smelkov escreveu:
  Mauro, thanks for answering,
  
  On Thu, Jul 28, 2011 at 10:45:29AM -0300, Mauro Carvalho Chehab wrote:
  Em 28-07-2011 08:42, Kirill Smelkov escreveu:
  On Wed, Jul 27, 2011 at 09:42:08PM +0200, Mauro Carvalho Chehab wrote:
  This is an automatic generated email to let you know that the following 
  patch were queued at the 
  http://git.linuxtv.org/media_tree.git tree:
 
  Subject: [media] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam on HP 
  Mini 5103 netbook
  Author:  Kirill Smelkov k...@mns.spb.ru
  Date:Fri Jul 22 11:47:22 2011 -0300
 
  Thanks
 
 
  The camera there identifies itself as being manufactured by Cheng Uei
  Precision Industry Co., Ltd (Foxlink), and product is titled as HP
  Webcam [2 MP Fixed].
 
  I was trying to get 2 USB video capture devices to work simultaneously,
  and noticed that the above mentioned webcam always requires packet size
  = 3072 bytes per micro frame (~= 23.4 MB/s isoc bandwidth), which is far
  more than enough to get standard NTSC 640x480x2x30 = ~17.6 MB/s isoc
  bandwidth.
 
  As there are alt interfaces with smaller MxPS
 
  T:  Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  2 Spd=480  MxCh= 0
  D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
  P:  Vendor=05c8 ProdID=0403 Rev= 1.06
  S:  Manufacturer=Foxlink
  S:  Product=HP Webcam [2 MP Fixed]
  S:  SerialNumber=200909240102
  C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
  A:  FirstIf#= 0 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
  I:* If#= 0 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 
  Driver=uvcvideo
  E:  Ad=83(I) Atr=03(Int.) MxPS=  16 Ivl=4ms
  I:* If#= 1 Alt= 0 #EPs= 0 Cls=0e(video) Sub=02 Prot=00 
  Driver=uvcvideo
  I:  If#= 1 Alt= 1 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 
  Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS= 128 Ivl=125us
  I:  If#= 1 Alt= 2 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 
  Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS= 512 Ivl=125us
  I:  If#= 1 Alt= 3 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 
  Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=1024 Ivl=125us
  I:  If#= 1 Alt= 4 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 
  Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=1536 Ivl=125us
  I:  If#= 1 Alt= 5 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 
  Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=2048 Ivl=125us
  I:  If#= 1 Alt= 6 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 
  Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=2688 Ivl=125us
  I:  If#= 1 Alt= 7 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 
  Driver=uvcvideo
  E:  Ad=81(I) Atr=05(Isoc) MxPS=3072 Ivl=125us
 
  UVC_QUIRK_FIX_BANDWIDTH helps here and NTSC video can be served with
  MxPS=2688 i.e. 20.5 MB/s isoc bandwidth.
 
  In terms of microframe time allocation, before the quirk NTSC video
  required 60 usecs / microframe and 53 usecs / microframe after.
 
  Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  Signed-off-by: Kirill Smelkov k...@mns.spb.ru
  Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
 
 
  May I ask, why you removed the reference to cc62a7eb? Original patch
  description contained the following paragraph just before sob
 
  Now with tweaked ehci-hcd to allow up to 90% isoc bandwidth (cc62a7eb
  USB: EHCI: Allow users to override 80% max periodic bandwidth) I can
  capture two video sources -- PAL 720x576 YUV422 @25fps + NTSC 640x480
  YUV422 @30fps simultaneously.  Hooray!
 
 
  which was removed on applying.
 
  For a few reasons:
  1) I was not sure if the changeset reference was not changed when
  
  And it was not changed, because when I referenced it, it was already in
  Greg's usb tree, and Linus pulls from Greg and Greg does not destroy
  history as far as I can tell...
  
 merged upstream, and I was too lazy^Wbusy to double check ;)
  
  It's just doing `git log linus/master | grep cc62a7eb`. Compared or less
  time to editing...
  
  
  2) Reducing the amount of bandwidth used is good, even without trying to 
  use
 two webcams;
  3) The bandwidth override patch is independent of this one.
  
  Yes, but coupled, and also they were motivated by each other. Could we
  please merge my comment back, or is it too late?
 
 Too late. my tree is ready for merge upstream. Changing anything there will
 require me to rebase it and loose one day in order to wait for the patches to
 arrive at the -next tree.


Pity, but thanks anyway.
--
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, RESEND] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam found on HP Mini 5103 netbook

2011-07-23 Thread Kirill Smelkov
Hi Laurent, All,

On Sat, Jul 23, 2011 at 01:12:11AM +0200, Laurent Pinchart wrote:
 Hi Kirill,
 
 On Saturday 23 July 2011 00:25:20 Kirill Smelkov wrote:
  On Sat, Jul 23, 2011 at 12:03:57AM +0200, Laurent Pinchart wrote:
   On Friday 22 July 2011 16:47:22 Kirill Smelkov wrote:
 [ Cc'ing Andrew Morton -- Andrew, could you please pick this patch, in
 
   case there is no response from maintainers again? Thanks beforehand.
   ]

Hello up there,

My first posting was 1 month ago, and a reminder ~ 2 weeks ago. All
without a reply. v3.0 is out and they say the merge window will be
shorter this time, so in oder not to miss it, I've decided to resend my
patch on lowering USB periodic bandwidth allocation topic.
   
   I'm very very sorry for missing the patch (and worse, twice :-/).
  
  Nevermind. I'm curious though, whether I did something wrong or anything
  else?  I mean how to avoid such long delays next time?
 
 It was all my fault, mails piled up in my mailbox and for some reason I 
 marked 
 yours as processed while they were not. I certainly hope it won't happen 
 again.

I see, thanks. Yes let's hope mail won't do such surprises next time.



Could this simple patch be please applied?
   
   Yes it can. I see that Andrew already applied it to his tree. Mauro,
   should it go through there, or through your tree ? I've pushed it to my
   tree at git://linuxtv.org/pinchartl/uvcvideo.git uvcvideo-stable, so you
   can already pull.
  
  You've applied the patch from my first posting, but actually in the
  RESEND one I've added reference to EHCI-tweaking patch -- it is already
  merged into Greg's USB tree (it was not when I first posted), so could you
  please reapply? (sorry for confusion).
 
 Sure. That should now be fixed.

Thanks.


  Thanks for replying and for uvcvideo,
 
 You're more than welcome. Thank you for the patch, and thank you for keeping 
 on pushing :-)

:) Thanks again. I suppose it would be better for my patch to enter the
mainline through v4l tree then, because it's the normal path for uvc
bits.



Thanks,
Kirill


P.S.

And thanks Andrew for backing me up.
--
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, RESEND] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam found on HP Mini 5103 netbook

2011-07-22 Thread Kirill Smelkov
 [ Cc'ing Andrew Morton -- Andrew, could you please pick this patch, in
   case there is no response from maintainers again? Thanks beforehand. ]


Hello up there,

My first posting was 1 month ago, and a reminder ~ 2 weeks ago. All
without a reply. v3.0 is out and they say the merge window will be
shorter this time, so in oder not to miss it, I've decided to resend my
patch on lowering USB periodic bandwidth allocation topic. 


Could this simple patch be please applied?

Thanks,
Kirill


P.S.

Referenced in the description cc62a7eb (USB: EHCI: Allow users to
override 80% max periodic bandwidth) will be entering the mainline via
Greg's usb tree.

 8 
From: Kirill Smelkov k...@mns.spb.ru
Subject: [PATCH] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam found on HP 
Mini 5103 netbook

The camera there identifies itself as being manufactured by Cheng Uei
Precision Industry Co., Ltd (Foxlink), and product is titled as HP
Webcam [2 MP Fixed].

I was trying to get 2 USB video capture devices to work simultaneously,
and noticed that the above mentioned webcam always requires packet size
= 3072 bytes per micro frame (~= 23.4 MB/s isoc bandwidth), which is far
more than enough to get standard NTSC 640x480x2x30 = ~17.6 MB/s isoc
bandwidth.

As there are alt interfaces with smaller MxPS

T:  Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  2 Spd=480  MxCh= 0
D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=05c8 ProdID=0403 Rev= 1.06
S:  Manufacturer=Foxlink
S:  Product=HP Webcam [2 MP Fixed]
S:  SerialNumber=200909240102
C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
A:  FirstIf#= 0 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
I:* If#= 0 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 Driver=uvcvideo
E:  Ad=83(I) Atr=03(Int.) MxPS=  16 Ivl=4ms
I:* If#= 1 Alt= 0 #EPs= 0 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
I:  If#= 1 Alt= 1 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS= 128 Ivl=125us
I:  If#= 1 Alt= 2 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS= 512 Ivl=125us
I:  If#= 1 Alt= 3 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=1024 Ivl=125us
I:  If#= 1 Alt= 4 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=1536 Ivl=125us
I:  If#= 1 Alt= 5 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=2048 Ivl=125us
I:  If#= 1 Alt= 6 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=2688 Ivl=125us
I:  If#= 1 Alt= 7 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=3072 Ivl=125us

UVC_QUIRK_FIX_BANDWIDTH helps here and NTSC video can be served with
MxPS=2688 i.e. 20.5 MB/s isoc bandwidth.

In terms of microframe time allocation, before the quirk NTSC video
required 60 usecs / microframe and 53 usecs / microframe after.


P.S.

Now with tweaked ehci-hcd to allow up to 90% isoc bandwidth (cc62a7eb
USB: EHCI: Allow users to override 80% max periodic bandwidth) I can
capture two video sources -- PAL 720x576 YUV422 @25fps + NTSC 640x480
YUV422 @30fps simultaneously.  Hooray!

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/video/uvc/uvc_driver.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/uvc/uvc_driver.c 
b/drivers/media/video/uvc/uvc_driver.c
index b6eae48..f633700 100644
--- a/drivers/media/video/uvc/uvc_driver.c
+++ b/drivers/media/video/uvc/uvc_driver.c
@@ -2130,6 +2130,15 @@ static struct usb_device_id uvc_ids[] = {
  .bInterfaceProtocol   = 0,
  .driver_info  = UVC_QUIRK_PROBE_MINMAX
| UVC_QUIRK_BUILTIN_ISIGHT },
+   /* Foxlink (HP Webcam on HP Mini 5103) */
+   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
+   | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x05c8,
+ .idProduct= 0x0403,
+ .bInterfaceClass  = USB_CLASS_VIDEO,
+ .bInterfaceSubClass   = 1,
+ .bInterfaceProtocol   = 0,
+ .driver_info  = UVC_QUIRK_FIX_BANDWIDTH },
/* Genesys Logic USB 2.0 PC Camera */
{ .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
-- 
1.7.6.rc1

--
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, RESEND] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam found on HP Mini 5103 netbook

2011-07-22 Thread Kirill Smelkov
Hi Laurent, All,

On Sat, Jul 23, 2011 at 12:03:57AM +0200, Laurent Pinchart wrote:
 Hi Kirill,
 
 On Friday 22 July 2011 16:47:22 Kirill Smelkov wrote:
   [ Cc'ing Andrew Morton -- Andrew, could you please pick this patch, in
 case there is no response from maintainers again? Thanks beforehand. ]
  
  
  Hello up there,
  
  My first posting was 1 month ago, and a reminder ~ 2 weeks ago. All
  without a reply. v3.0 is out and they say the merge window will be
  shorter this time, so in oder not to miss it, I've decided to resend my
  patch on lowering USB periodic bandwidth allocation topic.
 
 I'm very very sorry for missing the patch (and worse, twice :-/).

Nevermind. I'm curious though, whether I did something wrong or anything
else?  I mean how to avoid such long delays next time?


  Could this simple patch be please applied?
 
 Yes it can. I see that Andrew already applied it to his tree. Mauro, should 
 it 
 go through there, or through your tree ? I've pushed it to my tree at 
 git://linuxtv.org/pinchartl/uvcvideo.git uvcvideo-stable, so you can already 
 pull.

You've applied the patch from my first posting, but actually in the
RESEND one I've added reference to EHCI-tweaking patch -- it is already
merged into Greg's USB tree (it was not when I first posted), so could you
please reapply? (sorry for confusion).


Thanks for replying and for uvcvideo,
Kirill
--
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] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam found on HP Mini 5103 netbook

2011-07-12 Thread Kirill Smelkov
On Tue, Jun 21, 2011 at 10:07:43PM +0400, Kirill Smelkov wrote:
 The camera there identifeis itself as being manufactured by Cheng Uei
 Precision Industry Co., Ltd (Foxlink), and product is titled as HP
 Webcam [2 MP Fixed].
 
 I was trying to get 2 USB video capture devices to work simultaneously,
 and noticed that the above mentioned webcam always requires packet size
 = 3072 bytes per micro frame (~= 23.4 MB/s isoc bandwidth), which is far
 more than enough to get standard NTSC 640x480x2x30 = ~17.6 MB/s isoc
 bandwidth.
 
 As there are alt interfaces with smaller MxPS
 
 T:  Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  2 Spd=480  MxCh= 0
 D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
 P:  Vendor=05c8 ProdID=0403 Rev= 1.06
 S:  Manufacturer=Foxlink
 S:  Product=HP Webcam [2 MP Fixed]
 S:  SerialNumber=200909240102
 C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
 A:  FirstIf#= 0 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
 I:* If#= 0 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 Driver=uvcvideo
 E:  Ad=83(I) Atr=03(Int.) MxPS=  16 Ivl=4ms
 I:* If#= 1 Alt= 0 #EPs= 0 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 I:  If#= 1 Alt= 1 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS= 128 Ivl=125us
 I:  If#= 1 Alt= 2 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS= 512 Ivl=125us
 I:  If#= 1 Alt= 3 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=1024 Ivl=125us
 I:  If#= 1 Alt= 4 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=1536 Ivl=125us
 I:  If#= 1 Alt= 5 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=2048 Ivl=125us
 I:  If#= 1 Alt= 6 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=2688 Ivl=125us
 I:  If#= 1 Alt= 7 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
 E:  Ad=81(I) Atr=05(Isoc) MxPS=3072 Ivl=125us
 
 UVC_QUIRK_FIX_BANDWIDTH helps here and NTSC video can be served with
 MxPS=2688 i.e. 20.5 MB/s isoc bandwidth.
 
 In terms of microframe time allocation, before the quirk NTSC video
 required 60 usecs / microframe and 53 usecs / microframe after.
 
 
 P.S. Now with tweaked ehci-hcd to allow up to 90% isoc bandwith (instead of
 allowed for high speed 80%) I can capture two video sources -- PAL 720x576
 YUV422 @25fps + NTSC 640x480 YUV422 @30fps simultaneously. Hooray!
 
 Signed-off-by: Kirill Smelkov k...@mns.spb.ru


Silence...  though even the above-mentioned EHCI tweak was merged already:

http://git.kernel.org/?p=linux/kernel/git/gregkh/usb-2.6.git;a=commitdiff;h=cc62a7eb6396e8be95b9a30053ed09191818b99b


Hope this patch is not lost and thanks,
Kirill
--
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 v4 0/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-07-03 Thread Kirill Smelkov
There are cases, when 80% max isochronous bandwidth is too limiting.

Let's allow knowledgeable users to override that 80% max limit explicitly for
extreme cases, like 2 high bandwidth isochronously streaming devices on the
same High Speed USB bus in order to make them work simultaneously.

See full details in PATCH 2/2.


Changes since v3:

 - as suggested by Greg KH added new entry to Documentation/ABI/;

 - for consistency also touched Documentation/usb/ehci.txt reminding we now
   have new setting for periodic transfers scheduler.

   The base text in-there requires much updating regarding isochronous
   transfers, so only TBD with brief description was put because I'm not the 
best
   person to get EHCI description into shape;

 - added ACK to PATCH 2/2 from Alan Stern based on previous feedback. (I hope
   it's still semi-ok to put it, inspite of adding new not-reviewed text into
   docs);

 - minor cosmetics in patch descriptions.


Changes since v2:

 - changed the copyright in ehci-sysfs from David Brownell to Alan Stern;

 - added ACK to PATCH 1/2 from Alan Stern;

 - inspired by concerns raised by Sarah Sharp, added more details about testing
   done on N10 chipset, system stability, and that no-harm-is-done for those,
   who do not change uframe_periodic_max from default 100us;

 - when decreasing uframe_periodic_max, compare with the maximum number of
   microseconds already allocated for any uframe, instead of stopping as soon
   as it finds something above the new limit.


Changes since v1:

 - dropped RFC status as this seems like the sort of feature somebody might
   reasonably want to use -- if they know exactly what they're doing;

 - new preparatory patch (1/2) which moves already-in-there sysfs code into
   ehci-sysfs.c;

 - moved uframe_periodic_max parameter from module option to sysfs attribute,
   so that it can be set per controller and at runtime, added validity checks;

 - clarified a bit bandwidth analysis for 96% max periodic setup as noticed by
   Alan Stern;

 - clarified patch description saying that set in stone 80% max periodic is
   specified by USB 2.0;

Kirill Smelkov (2):
  USB: EHCI: Move sysfs related bits into ehci-sysfs.c
  USB: EHCI: Allow users to override 80% max periodic bandwidth

 Documentation/ABI/testing/sysfs-module |   23 
 Documentation/usb/ehci.txt |2 +
 drivers/usb/host/ehci-hcd.c|   11 ++-
 drivers/usb/host/ehci-hub.c|   75 -
 drivers/usb/host/ehci-sched.c  |   17 +--
 drivers/usb/host/ehci-sysfs.c  |  190 
 drivers/usb/host/ehci.h|2 +
 7 files changed, 233 insertions(+), 87 deletions(-)
 create mode 100644 drivers/usb/host/ehci-sysfs.c

-- 
1.7.6.rc3

--
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 v4 1/2] USB: EHCI: Move sysfs related bits into ehci-sysfs.c

2011-07-03 Thread Kirill Smelkov
The only sysfs attr implemented so far is companion from ehci-hub.c,
but in the next patch we are going to add another sysfs file, so prior
to that let's structure things and move already-in-there sysfs code to
separate file.

NOTE: All the code I'm moving into this new file was written by Alan
Stern (in 57e06c11 EHCI: force high-speed devices to run at full
speed; Jan 16 2007), that's why I'm putting

Copyright (C) 2007 by Alan Stern

there after explicit request from the author.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Acked-by: Alan Stern st...@rowland.harvard.edu
---
 drivers/usb/host/ehci-hcd.c   |5 +-
 drivers/usb/host/ehci-hub.c   |   75 
 drivers/usb/host/ehci-sysfs.c |   94 +
 3 files changed, 97 insertions(+), 77 deletions(-)
 create mode 100644 drivers/usb/host/ehci-sysfs.c

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index e18862c..8306155 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -336,6 +336,7 @@ static void ehci_work(struct ehci_hcd *ehci);
 #include ehci-mem.c
 #include ehci-q.c
 #include ehci-sched.c
+#include ehci-sysfs.c
 
 /*-*/
 
@@ -520,7 +521,7 @@ static void ehci_stop (struct usb_hcd *hcd)
ehci_reset (ehci);
spin_unlock_irq(ehci-lock);
 
-   remove_companion_file(ehci);
+   remove_sysfs_files(ehci);
remove_debug_files (ehci);
 
/* root hub is shut down separately (first, when possible) */
@@ -754,7 +755,7 @@ static int ehci_run (struct usb_hcd *hcd)
 * since the class device isn't created that early.
 */
create_debug_files(ehci);
-   create_companion_file(ehci);
+   create_sysfs_files(ehci);
 
return 0;
 }
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index ea6184b..d9e8d71 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -471,29 +471,6 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
 
 /*-*/
 
-/* Display the ports dedicated to the companion controller */
-static ssize_t show_companion(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
-   struct ehci_hcd *ehci;
-   int nports, index, n;
-   int count = PAGE_SIZE;
-   char*ptr = buf;
-
-   ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
-   nports = HCS_N_PORTS(ehci-hcs_params);
-
-   for (index = 0; index  nports; ++index) {
-   if (test_bit(index, ehci-companion_ports)) {
-   n = scnprintf(ptr, count, %d\n, index + 1);
-   ptr += n;
-   count -= n;
-   }
-   }
-   return ptr - buf;
-}
-
 /*
  * Sets the owner of a port
  */
@@ -528,58 +505,6 @@ static void set_owner(struct ehci_hcd *ehci, int portnum, 
int new_owner)
}
 }
 
-/*
- * Dedicate or undedicate a port to the companion controller.
- * Syntax is [-]portnum, where a leading '-' sign means
- * return control of the port to the EHCI controller.
- */
-static ssize_t store_companion(struct device *dev,
-  struct device_attribute *attr,
-  const char *buf, size_t count)
-{
-   struct ehci_hcd *ehci;
-   int portnum, new_owner;
-
-   ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
-   new_owner = PORT_OWNER; /* Owned by companion */
-   if (sscanf(buf, %d, portnum) != 1)
-   return -EINVAL;
-   if (portnum  0) {
-   portnum = - portnum;
-   new_owner = 0;  /* Owned by EHCI */
-   }
-   if (portnum = 0 || portnum  HCS_N_PORTS(ehci-hcs_params))
-   return -ENOENT;
-   portnum--;
-   if (new_owner)
-   set_bit(portnum, ehci-companion_ports);
-   else
-   clear_bit(portnum, ehci-companion_ports);
-   set_owner(ehci, portnum, new_owner);
-   return count;
-}
-static DEVICE_ATTR(companion, 0644, show_companion, store_companion);
-
-static inline int create_companion_file(struct ehci_hcd *ehci)
-{
-   int i = 0;
-
-   /* with integrated TT there is no companion! */
-   if (!ehci_is_TDI(ehci))
-   i = device_create_file(ehci_to_hcd(ehci)-self.controller,
-  dev_attr_companion);
-   return i;
-}
-
-static inline void remove_companion_file(struct ehci_hcd *ehci)
-{
-   /* with integrated TT there is no companion! */
-   if (!ehci_is_TDI(ehci))
-   device_remove_file(ehci_to_hcd(ehci)-self.controller,
-  dev_attr_companion

[PATCH v4 2/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-07-03 Thread Kirill Smelkov
There are cases, when 80% max isochronous bandwidth is too limiting.

For example I have two USB video capture cards which stream uncompressed
video, and to stream full NTSC + PAL videos we'd need

NTSC 640x480 YUV422 @30fps  ~17.6 MB/s
PAL  720x576 YUV422 @25fps  ~19.7 MB/s

isoc bandwidth.

Now, due to limited alt settings in capture devices NTSC one ends up
streaming with max_pkt_size=2688  and  PAL with max_pkt_size=2892, both
with interval=1. In terms of microframe time allocation this gives

NTSC~53us
PAL ~57us

and together

~110us100us == 80% of 125us uframe time.

So those two devices can't work together simultaneously because the'd
over allocate isochronous bandwidth.

80% seemed a bit arbitrary to me, and I've tried to raise it to 90% and
both devices started to work together, so I though sometimes it would be
a good idea for users to override hardcoded default of max 80% isoc
bandwidth.

After all, isn't it a user who should decide how to load the bus? If I
can live with 10% or even 5% bulk bandwidth that should be ok. I'm a USB
newcomer, but that 80% set in stone by USB 2.0 specification seems to be
chosen pretty arbitrary to me, just to serve as a reasonable default.

NOTE 1
~~

for two streams with max_pkt_size=3072 (worst case) both time
allocation would be 60us+60us=120us which is 96% periodic bandwidth
leaving 4% for bulk and control.  Alan Stern suggested that bulk then
would be problematic (less than 300*8 bittimes left per microframe), but
I think that is still enough for control traffic.

NOTE 2
~~

Sarah Sharp expressed concern that maxing out periodic bandwidth
could lead to vendor-specific hardware bugs on host controllers, because

 It's entirely possible that you'll run into
 vendor-specific bugs if you try to pack the schedule with isochronous
 transfers.  I don't think any hardware designer would seriously test or
 validate their hardware with a schedule that is basically a violation of
 the USB bus spec (more than 80% for periodic transfers).

So far I've only tested this patch on my HP Mini 5103 with N10 chipset

kirr@mini:~$ lspci
00:00.0 Host bridge: Intel Corporation N10 Family DMI Bridge
00:02.0 VGA compatible controller: Intel Corporation N10 Family Integrated 
Graphics Controller
00:02.1 Display controller: Intel Corporation N10 Family Integrated 
Graphics Controller
00:1b.0 Audio device: Intel Corporation N10/ICH 7 Family High Definition 
Audio Controller (rev 02)
00:1c.0 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 1 
(rev 02)
00:1c.3 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 4 
(rev 02)
00:1d.0 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #1 (rev 02)
00:1d.1 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #2 (rev 02)
00:1d.2 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #3 (rev 02)
00:1d.3 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #4 (rev 02)
00:1d.7 USB Controller: Intel Corporation N10/ICH 7 Family USB2 EHCI 
Controller (rev 02)
00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2)
00:1f.0 ISA bridge: Intel Corporation NM10 Family LPC Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation N10/ICH7 Family SATA AHCI 
Controller (rev 02)
01:00.0 Network controller: Broadcom Corporation BCM4313 802.11b/g/n 
Wireless LAN Controller (rev 01)
02:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8059 PCI-E 
Gigabit Ethernet Controller (rev 11)

and the system works stable with 110us/uframe (~88%) isoc bandwith allocated for
above-mentioned isochronous transfers.

NOTE 3
~~

This feature is off by default. I mean max periodic bandwidth is set to
100us/uframe by default exactly as it was before the patch. So only those of us
who need the extreme settings are taking the risk - normal users who do not
alter uframe_periodic_max sysfs attribute should not see any change at all.

NOTE 4
~~

I've tried to update documentation in Documentation/ABI/ thoroughly, but
only TBD was put into Documentation/usb/ehci.txt -- the text there seems
to be outdated and much needing refreshing, before it could be amended.

Cc: Sarah Sharp sarah.a.sh...@linux.intel.com
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Acked-by: Alan Stern st...@rowland.harvard.edu
---
 Documentation/ABI/testing/sysfs-module |   23 +++
 Documentation/usb/ehci.txt |2 +
 drivers/usb/host/ehci-hcd.c|6 ++
 drivers/usb/host/ehci-sched.c  |   17 ++---
 drivers/usb/host/ehci-sysfs.c  |  104 ++-
 drivers/usb/host/ehci.h|2 +
 6 files changed, 140 insertions(+), 14 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-module 
b/Documentation/ABI/testing/sysfs-module
index cfcec3b..9489ea8 100644
--- a/Documentation/ABI

Re: [PATCH v3 2/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-07-02 Thread Kirill Smelkov
On Fri, Jul 01, 2011 at 02:24:27PM -0700, Greg KH wrote:
 On Fri, Jul 01, 2011 at 03:47:11PM +0400, Kirill Smelkov wrote:
   drivers/usb/host/ehci-sysfs.c |  104 
  +++--
 
 As you are adding new sysfs files, it is required that you also add new
 entries in the Documentation/ABI/ directory.
 
 Please do that for these files so that people have an idea of what they
 are, and how to use them.
 
 Care to redo this series with that change?

Sure, I'll post v4 with documentation included.

Thanks,
Kirill
--
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 v2 0/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-07-01 Thread Kirill Smelkov
On Thu, Jun 30, 2011 at 11:01:01AM -0700, Sarah Sharp wrote:
 On Fri, Jun 24, 2011 at 08:48:06PM +0400, Kirill Smelkov wrote:
  
  Changes since v1:
  
  
   - dropped RFC status as this seems like the sort of feature somebody might
 reasonably want to use -- if they know exactly what they're doing;
  
   - new preparatory patch (1/2) which moves already-in-there sysfs code into
 ehci-sysfs.c;
  
   - moved uframe_periodic_max parameter from module option to sysfs 
  attribute,
 so that it can be set per controller and at runtime, added validity 
  checks;
  
   - clarified a bit bandwith analysis for 96% max periodic setup as noticed 
  by
 Alan Stern;
  
   - clarified patch description saying that set in stone 80% max periodic is
 specified by USB 2.0;
 
 Have you tested this patch by maxing out this bandwidth on various
 types of host controllers?  It's entirely possible that you'll run into
 vendor-specific bugs if you try to pack the schedule with isochronous
 transfers.  I don't think any hardware designer would seriously test or
 validate their hardware with a schedule that is basically a violation of
 the USB bus spec (more than 80% for periodic transfers).

I've only tested it to work on my HP Mini 5103 with N10 chipset:

kirr@mini:~$ lspci 
00:00.0 Host bridge: Intel Corporation N10 Family DMI Bridge
00:02.0 VGA compatible controller: Intel Corporation N10 Family Integrated 
Graphics Controller
00:02.1 Display controller: Intel Corporation N10 Family Integrated 
Graphics Controller
00:1b.0 Audio device: Intel Corporation N10/ICH 7 Family High Definition 
Audio Controller (rev 02)
00:1c.0 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 1 
(rev 02)
00:1c.3 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 4 
(rev 02)
00:1d.0 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #1 (rev 02)
00:1d.1 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #2 (rev 02)
00:1d.2 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #3 (rev 02)
00:1d.3 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #4 (rev 02)
00:1d.7 USB Controller: Intel Corporation N10/ICH 7 Family USB2 EHCI 
Controller (rev 02)
00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2)
00:1f.0 ISA bridge: Intel Corporation NM10 Family LPC Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation N10/ICH7 Family SATA AHCI 
Controller (rev 02)
01:00.0 Network controller: Broadcom Corporation BCM4313 802.11b/g/n 
Wireless LAN Controller (rev 01)
02:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8059 PCI-E 
Gigabit Ethernet Controller (rev 11)

The system works stable with 110us/uframe (~88%) isoc bandwith allocated for
integrated UVC webcam and external EM28XX based capture board.


 But if Alan is fine with giving users a way to shoot themselves in the
 foot, and it's disabled by default, then I don't particularly mind this
 patch.

Yes, it is disabled by default, I mean max periodic bandwidth is set to
100us/uframe by default exactly as it was before the patch. So only
those of us who need the extreme settings are taking the risk - normal
users who do not alter uframe_periodic_max attribute should not see any
change at all.


Thanks for commenting. I'll extend my testing information and notes on
do-not-do-harm bahaviour in updated patch.


Kirill
--
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 v3 0/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-07-01 Thread Kirill Smelkov
There are cases, when 80% max isochronous bandwidth is too limiting.

Let's allow knowledgeable users to override that 80% max limit explicitly for
extreme cases, like 2 high bandwidth isochronously streaming devices on the
same High Speed USB bus in order to make them work simultaneously.

See full details in PATCH 2/2.


Changes since v2:

 - changed the copyright in ehci-sysfs from David Brownell to Alan Stern;

 - added ACK to PATCH 1/2 from Alan Stern;

 - inspired by concerns raised by Sarah Sharp, added more details about testing
   done on N10 chipset, system stability, and that no-harm-is-done for those,
   who do not change uframe_periodic_max from default 100us;

 - when decreasing uframe_periodic_max, compare with the maximum number of
   microseconds already allocated for any uframe, instead of stopping as soon
   as it finds something above the new limit.


Changes since v1:

 - dropped RFC status as this seems like the sort of feature somebody might
   reasonably want to use -- if they know exactly what they're doing;

 - new preparatory patch (1/2) which moves already-in-there sysfs code into
   ehci-sysfs.c;

 - moved uframe_periodic_max parameter from module option to sysfs attribute,
   so that it can be set per controller and at runtime, added validity checks;

 - clarified a bit bandwidth analysis for 96% max periodic setup as noticed by
   Alan Stern;

 - clarified patch description saying that set in stone 80% max periodic is
   specified by USB 2.0;


Kirill Smelkov (2):
  USB: EHCI: Move sysfs related bits into ehci-sysfs.c
  USB: EHCI: Allow users to override 80% max periodic bandwidth

 drivers/usb/host/ehci-hcd.c   |   11 ++-
 drivers/usb/host/ehci-hub.c   |   75 
 drivers/usb/host/ehci-sched.c |   17 ++--
 drivers/usb/host/ehci-sysfs.c |  190 +
 drivers/usb/host/ehci.h   |2 +
 5 files changed, 208 insertions(+), 87 deletions(-)
 create mode 100644 drivers/usb/host/ehci-sysfs.c

-- 
1.7.6.rc3

--
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 v3 2/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-07-01 Thread Kirill Smelkov
There are cases, when 80% max isochronous bandwidth is too limiting.

For example I have two USB video capture cards which stream uncompressed
video, and to stream full NTSC + PAL videos we'd need

NTSC 640x480 YUV422 @30fps  ~17.6 MB/s
PAL  720x576 YUV422 @25fps  ~19.7 MB/s

isoc bandwidth.

Now, due to limited alt settings in capture devices NTSC one ends up
streaming with max_pkt_size=2688  and  PAL with max_pkt_size=2892, both
with interval=1. In terms of microframe time allocation this gives

NTSC~53us
PAL ~57us

and together

~110us100us == 80% of 125us uframe time.

So those two devices can't work together simultaneously because the'd
over allocate isochronous bandwidth.

80% seemed a bit arbitrary to me, and I've tried to raise it to 90% and
both devices started to work together, so I though sometimes it would be
a good idea for users to override hardcoded default of max 80% isoc
bandwidth.

After all, isn't it a user who should decide how to load the bus? If I
can live with 10% or even 5% bulk bandwidth that should be ok. I'm a USB
newcomer, but that 80% set in stone by USB 2.0 specification seems to be
chosen pretty arbitrary to me, just to serve as a reasonable default.

NOTE 1
~~

for two streams with max_pkt_size=3072 (worst case) both time
allocation would be 60us+60us=120us which is 96% periodic bandwidth
leaving 4% for bulk and control.  Alan Stern suggested that bulk then
would be problematic (less than 300*8 bittimes left per microframe), but
I think that is still enough for control traffic.

NOTE 2
~~

Sarah Sharp expressed concern that maxing out periodic bandwidth
could lead to vendor-specific hardware bugs on host controllers, because

 It's entirely possible that you'll run into
 vendor-specific bugs if you try to pack the schedule with isochronous
 transfers.  I don't think any hardware designer would seriously test or
 validate their hardware with a schedule that is basically a violation of
 the USB bus spec (more than 80% for periodic transfers).

So far I've only tested this patch on my HP Mini 5103 with N10 chipeset

kirr@mini:~$ lspci
00:00.0 Host bridge: Intel Corporation N10 Family DMI Bridge
00:02.0 VGA compatible controller: Intel Corporation N10 Family Integrated 
Graphics Controller
00:02.1 Display controller: Intel Corporation N10 Family Integrated 
Graphics Controller
00:1b.0 Audio device: Intel Corporation N10/ICH 7 Family High Definition 
Audio Controller (rev 02)
00:1c.0 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 1 
(rev 02)
00:1c.3 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 4 
(rev 02)
00:1d.0 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #1 (rev 02)
00:1d.1 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #2 (rev 02)
00:1d.2 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #3 (rev 02)
00:1d.3 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #4 (rev 02)
00:1d.7 USB Controller: Intel Corporation N10/ICH 7 Family USB2 EHCI 
Controller (rev 02)
00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2)
00:1f.0 ISA bridge: Intel Corporation NM10 Family LPC Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation N10/ICH7 Family SATA AHCI 
Controller (rev 02)
01:00.0 Network controller: Broadcom Corporation BCM4313 802.11b/g/n 
Wireless LAN Controller (rev 01)
02:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8059 PCI-E 
Gigabit Ethernet Controller (rev 11)

and the system works stable with 110us/uframe (~88%) isoc bandwith allocated for
above-mentioned isochronous transfers.

NOTE 3
~~

This feature is off by default. I mean max periodic bandwidth is set to
100us/uframe by default exactly as it was before the patch. So only those of us
who need the extreme settings are taking the risk - normal users who do not
alter uframe_periodic_max sysfs attribute should not see any change at all.

Cc: Sarah Sharp sarah.a.sh...@linux.intel.com
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/usb/host/ehci-hcd.c   |6 ++
 drivers/usb/host/ehci-sched.c |   17 +++
 drivers/usb/host/ehci-sysfs.c |  104 +++--
 drivers/usb/host/ehci.h   |2 +
 4 files changed, 115 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 8306155..4ee62be 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -572,6 +572,12 @@ static int ehci_init(struct usb_hcd *hcd)
hcc_params = ehci_readl(ehci, ehci-caps-hcc_params);
 
/*
+* by default set standard 80% (== 100 usec/uframe) max periodic
+* bandwidth as required by USB 2.0
+*/
+   ehci-uframe_periodic_max = 100;
+
+   /*
 * hw default: 1K periodic list heads, one per frame

[PATCH v3 1/2] USB: EHCI: Move sysfs related bits into ehci-sysfs.c

2011-07-01 Thread Kirill Smelkov
The only sysfs attr implemented so far is companion from ehci-hub.c,
but in the next patch we are going to add another sysfs file, so prior
to that let's structure things and move already-in-there sysfs code to
separate file.

NOTE: All the code I'm moving into this new file was written by Alan
Stern (in 57e06c11 EHCI: force high-speed devices to run at full
speed; Jan 16 2007), that's why I'm putting

Copyright (C) 2007 by Alan Stern

there after explicit request from the author.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Acked-off-by: Alan Stern st...@rowland.harvard.edu
---
 drivers/usb/host/ehci-hcd.c   |5 +-
 drivers/usb/host/ehci-hub.c   |   75 
 drivers/usb/host/ehci-sysfs.c |   94 +
 3 files changed, 97 insertions(+), 77 deletions(-)
 create mode 100644 drivers/usb/host/ehci-sysfs.c

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index e18862c..8306155 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -336,6 +336,7 @@ static void ehci_work(struct ehci_hcd *ehci);
 #include ehci-mem.c
 #include ehci-q.c
 #include ehci-sched.c
+#include ehci-sysfs.c
 
 /*-*/
 
@@ -520,7 +521,7 @@ static void ehci_stop (struct usb_hcd *hcd)
ehci_reset (ehci);
spin_unlock_irq(ehci-lock);
 
-   remove_companion_file(ehci);
+   remove_sysfs_files(ehci);
remove_debug_files (ehci);
 
/* root hub is shut down separately (first, when possible) */
@@ -754,7 +755,7 @@ static int ehci_run (struct usb_hcd *hcd)
 * since the class device isn't created that early.
 */
create_debug_files(ehci);
-   create_companion_file(ehci);
+   create_sysfs_files(ehci);
 
return 0;
 }
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index ea6184b..d9e8d71 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -471,29 +471,6 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
 
 /*-*/
 
-/* Display the ports dedicated to the companion controller */
-static ssize_t show_companion(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
-   struct ehci_hcd *ehci;
-   int nports, index, n;
-   int count = PAGE_SIZE;
-   char*ptr = buf;
-
-   ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
-   nports = HCS_N_PORTS(ehci-hcs_params);
-
-   for (index = 0; index  nports; ++index) {
-   if (test_bit(index, ehci-companion_ports)) {
-   n = scnprintf(ptr, count, %d\n, index + 1);
-   ptr += n;
-   count -= n;
-   }
-   }
-   return ptr - buf;
-}
-
 /*
  * Sets the owner of a port
  */
@@ -528,58 +505,6 @@ static void set_owner(struct ehci_hcd *ehci, int portnum, 
int new_owner)
}
 }
 
-/*
- * Dedicate or undedicate a port to the companion controller.
- * Syntax is [-]portnum, where a leading '-' sign means
- * return control of the port to the EHCI controller.
- */
-static ssize_t store_companion(struct device *dev,
-  struct device_attribute *attr,
-  const char *buf, size_t count)
-{
-   struct ehci_hcd *ehci;
-   int portnum, new_owner;
-
-   ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
-   new_owner = PORT_OWNER; /* Owned by companion */
-   if (sscanf(buf, %d, portnum) != 1)
-   return -EINVAL;
-   if (portnum  0) {
-   portnum = - portnum;
-   new_owner = 0;  /* Owned by EHCI */
-   }
-   if (portnum = 0 || portnum  HCS_N_PORTS(ehci-hcs_params))
-   return -ENOENT;
-   portnum--;
-   if (new_owner)
-   set_bit(portnum, ehci-companion_ports);
-   else
-   clear_bit(portnum, ehci-companion_ports);
-   set_owner(ehci, portnum, new_owner);
-   return count;
-}
-static DEVICE_ATTR(companion, 0644, show_companion, store_companion);
-
-static inline int create_companion_file(struct ehci_hcd *ehci)
-{
-   int i = 0;
-
-   /* with integrated TT there is no companion! */
-   if (!ehci_is_TDI(ehci))
-   i = device_create_file(ehci_to_hcd(ehci)-self.controller,
-  dev_attr_companion);
-   return i;
-}
-
-static inline void remove_companion_file(struct ehci_hcd *ehci)
-{
-   /* with integrated TT there is no companion! */
-   if (!ehci_is_TDI(ehci))
-   device_remove_file(ehci_to_hcd(ehci)-self.controller,
-  dev_attr_companion

[PATCH 1/2] USB: EHCI: Move sysfs related bits into ehci-sysfs.c

2011-06-24 Thread Kirill Smelkov
The only sysfs attr implemented so far is companion from ehci-hub.c,
but in the next patch we are going to add another sysfs file, so prior
to that let's structure things and move already-in-there sysfs code to
separate file.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/usb/host/ehci-hcd.c   |5 +-
 drivers/usb/host/ehci-hub.c   |   75 
 drivers/usb/host/ehci-sysfs.c |   94 +
 3 files changed, 97 insertions(+), 77 deletions(-)
 create mode 100644 drivers/usb/host/ehci-sysfs.c

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index e18862c..8306155 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -336,6 +336,7 @@ static void ehci_work(struct ehci_hcd *ehci);
 #include ehci-mem.c
 #include ehci-q.c
 #include ehci-sched.c
+#include ehci-sysfs.c
 
 /*-*/
 
@@ -520,7 +521,7 @@ static void ehci_stop (struct usb_hcd *hcd)
ehci_reset (ehci);
spin_unlock_irq(ehci-lock);
 
-   remove_companion_file(ehci);
+   remove_sysfs_files(ehci);
remove_debug_files (ehci);
 
/* root hub is shut down separately (first, when possible) */
@@ -754,7 +755,7 @@ static int ehci_run (struct usb_hcd *hcd)
 * since the class device isn't created that early.
 */
create_debug_files(ehci);
-   create_companion_file(ehci);
+   create_sysfs_files(ehci);
 
return 0;
 }
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index ea6184b..d9e8d71 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -471,29 +471,6 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
 
 /*-*/
 
-/* Display the ports dedicated to the companion controller */
-static ssize_t show_companion(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
-   struct ehci_hcd *ehci;
-   int nports, index, n;
-   int count = PAGE_SIZE;
-   char*ptr = buf;
-
-   ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
-   nports = HCS_N_PORTS(ehci-hcs_params);
-
-   for (index = 0; index  nports; ++index) {
-   if (test_bit(index, ehci-companion_ports)) {
-   n = scnprintf(ptr, count, %d\n, index + 1);
-   ptr += n;
-   count -= n;
-   }
-   }
-   return ptr - buf;
-}
-
 /*
  * Sets the owner of a port
  */
@@ -528,58 +505,6 @@ static void set_owner(struct ehci_hcd *ehci, int portnum, 
int new_owner)
}
 }
 
-/*
- * Dedicate or undedicate a port to the companion controller.
- * Syntax is [-]portnum, where a leading '-' sign means
- * return control of the port to the EHCI controller.
- */
-static ssize_t store_companion(struct device *dev,
-  struct device_attribute *attr,
-  const char *buf, size_t count)
-{
-   struct ehci_hcd *ehci;
-   int portnum, new_owner;
-
-   ehci = hcd_to_ehci(bus_to_hcd(dev_get_drvdata(dev)));
-   new_owner = PORT_OWNER; /* Owned by companion */
-   if (sscanf(buf, %d, portnum) != 1)
-   return -EINVAL;
-   if (portnum  0) {
-   portnum = - portnum;
-   new_owner = 0;  /* Owned by EHCI */
-   }
-   if (portnum = 0 || portnum  HCS_N_PORTS(ehci-hcs_params))
-   return -ENOENT;
-   portnum--;
-   if (new_owner)
-   set_bit(portnum, ehci-companion_ports);
-   else
-   clear_bit(portnum, ehci-companion_ports);
-   set_owner(ehci, portnum, new_owner);
-   return count;
-}
-static DEVICE_ATTR(companion, 0644, show_companion, store_companion);
-
-static inline int create_companion_file(struct ehci_hcd *ehci)
-{
-   int i = 0;
-
-   /* with integrated TT there is no companion! */
-   if (!ehci_is_TDI(ehci))
-   i = device_create_file(ehci_to_hcd(ehci)-self.controller,
-  dev_attr_companion);
-   return i;
-}
-
-static inline void remove_companion_file(struct ehci_hcd *ehci)
-{
-   /* with integrated TT there is no companion! */
-   if (!ehci_is_TDI(ehci))
-   device_remove_file(ehci_to_hcd(ehci)-self.controller,
-  dev_attr_companion);
-}
-
-
 /*-*/
 
 static int check_reset_complete (
diff --git a/drivers/usb/host/ehci-sysfs.c b/drivers/usb/host/ehci-sysfs.c
new file mode 100644
index 000..347c8cb
--- /dev/null
+++ b/drivers/usb/host/ehci-sysfs.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2001

[PATCH v2 2/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-06-24 Thread Kirill Smelkov
There are cases, when 80% max isochronous bandwidth is too limiting.

For example I have two USB video capture cards which stream uncompressed
video, and to stream full NTSC + PAL videos we'd need

NTSC 640x480 YUV422 @30fps  ~17.6 MB/s
PAL  720x576 YUV422 @25fps  ~19.7 MB/s

isoc bandwidth.

Now, due to limited alt settings in capture devices NTSC one ends up
streaming with max_pkt_size=2688  and  PAL with max_pkt_size=2892, both
with interval=1. In terms of microframe time allocation this gives

NTSC~53us
PAL ~57us

and together

~110us100us == 80% of 125us uframe time.

So those two devices can't work together simultaneously because the'd
over allocate isochronous bandwidth.

80% seemed a bit arbitrary to me, and I've tried to raise it to 90% and
both devices started to work together, so I though sometimes it would be
a good idea for users to override hardcoded default of max 80% isoc
bandwidth.

After all, isn't it a user who should decide how to load the bus? If I
can live with 10% or even 5% bulk bandwidth that should be ok. I'm a USB
newcomer, but that 80% set in stone by USB 2.0 specification seems to be
chosen pretty arbitrary to me, just to serve as a reasonable default.

NOTE: for two streams with max_pkt_size=3072 (worst case) both time
allocation would be 60us+60us=120us which is 96% periodic bandwidth
leaving 4% for bulk and control.  Alan Stern suggested that bulk then
would be problematic (less than 300*8 bittimes left per microframe), but
I think that is still enough for control traffic.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/usb/host/ehci-hcd.c   |6 +++
 drivers/usb/host/ehci-sched.c |   17 +++
 drivers/usb/host/ehci-sysfs.c |   98 +++--
 drivers/usb/host/ehci.h   |2 +
 4 files changed, 109 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 8306155..4ee62be 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -572,6 +572,12 @@ static int ehci_init(struct usb_hcd *hcd)
hcc_params = ehci_readl(ehci, ehci-caps-hcc_params);
 
/*
+* by default set standard 80% (== 100 usec/uframe) max periodic
+* bandwidth as required by USB 2.0
+*/
+   ehci-uframe_periodic_max = 100;
+
+   /*
 * hw default: 1K periodic list heads, one per frame.
 * periodic_size can shrink by USBCMD update if hcc_params allows.
 */
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 6c9fbe3..2abf854 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -172,7 +172,7 @@ periodic_usecs (struct ehci_hcd *ehci, unsigned frame, 
unsigned uframe)
}
}
 #ifdef DEBUG
-   if (usecs  100)
+   if (usecs  ehci-uframe_periodic_max)
ehci_err (ehci, uframe %d sched overrun: %d usecs\n,
frame * 8 + uframe, usecs);
 #endif
@@ -709,11 +709,8 @@ static int check_period (
if (uframe = 8)
return 0;
 
-   /*
-* 80% periodic == 100 usec/uframe available
-* convert usecs we need to max already claimed
-*/
-   usecs = 100 - usecs;
+   /* convert usecs we need to max already claimed */
+   usecs = ehci-uframe_periodic_max - usecs;
 
/* we know 2 and 4 uframe intervals were rejected; so
 * for period 0, check _every_ microframe in the schedule.
@@ -1286,9 +1283,9 @@ itd_slot_ok (
 {
uframe %= period;
do {
-   /* can't commit more than 80% periodic == 100 usec */
+   /* can't commit more than uframe_periodic_max usec */
if (periodic_usecs (ehci, uframe  3, uframe  0x7)
-(100 - usecs))
+(ehci-uframe_periodic_max - usecs))
return 0;
 
/* we know urb-interval is 2^N uframes */
@@ -1345,7 +1342,7 @@ sitd_slot_ok (
 #endif
 
/* check starts (OUT uses more than one) */
-   max_used = 100 - stream-usecs;
+   max_used = ehci-uframe_periodic_max - stream-usecs;
for (tmp = stream-raw_mask  0xff; tmp; tmp = 1, uf++) {
if (periodic_usecs (ehci, frame, uf)  max_used)
return 0;
@@ -1354,7 +1351,7 @@ sitd_slot_ok (
/* for IN, check CSPLIT */
if (stream-c_usecs) {
uf = uframe  7;
-   max_used = 100 - stream-c_usecs;
+   max_used = ehci-uframe_periodic_max - stream-c_usecs;
do {
tmp = 1  uf;
tmp = 8;
diff --git a/drivers/usb/host/ehci-sysfs.c b/drivers/usb/host/ehci-sysfs.c
index 347c8cb..fe212ef 100644
--- a/drivers/usb/host/ehci-sysfs.c
+++ b/drivers

Re: [RFC, PATCH] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-06-24 Thread Kirill Smelkov
On Thu, Jun 23, 2011 at 01:14:04PM -0400, Alan Stern wrote:
 On Thu, 23 Jun 2011, Kirill Smelkov wrote:
 
   At 480 Mb/s, each microframe holds 7500 bytes (less if you count 
   bit-stuffing).  4% of that is 300 bytes, which is not enough for a 
   512-byte bulk packet.  I think you'd run into trouble trying to do any 
   serious bulk transfers on such a tight schedule.
  
  Yes, you seem to be right.
  
  I still think 4% is maybe enough for control traffic.
 
 It should be.

Ok then.

At least devices could be start/stopped, and frankly if someone loads
the bus with two high-bandwidth isoc streams, there is no reason to
expect any bulk transfer to happen at all.

@@ -571,6 +579,14 @@ static int ehci_init(struct usb_hcd *hcd)
hcc_params = ehci_readl(ehci, ehci-caps-hcc_params);
 
/*
+* tell user, if using non-standard (80% == 100 usec/uframe) 
bandwidth
+*/
+   if (uframe_periodic_max != 100)
+   ehci_info(ehci, using non-standard max periodic 
bandwith 
+   (%u%% == %u usec/uframe),
+   100*uframe_periodic_max/125, 
uframe_periodic_max);
+
+   /*
   
   Check for invalid values.  This should never be less than 100 or 
   greater than 125.
  
  Ok. By the way, why should we limit it to be not less than 100?
  Likewise, a user who knows exactly what he/she is doing could limit
  periodic bandwidth to be less than 80% required by USB specification.
 
 What's the point?  If you want to use less than 80% of your bandwidth 
 for periodic transfers, go ahead and do so.  You don't need to change 
 the limit.

I though it would be good for generality -- i.e. if someone wants to
limit maximum isoc bandwidth to say 50% so that would never be
overallocated by that limit that would be handy.

But I agree - it's a bit artificial, so in updated patch I've left what
you originally suggested to be 100 = uframe_periodic_max  125 (ommitting 
=125).


Thanks,
Kirill
--
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] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-06-23 Thread Kirill Smelkov
On Wed, Jun 22, 2011 at 03:22:28PM -0400, Alan Stern wrote:
 On Wed, 22 Jun 2011, Kirill Smelkov wrote:
 
  There are cases, when 80% max isochronous bandwidth is too limiting.
  
  For example I have two USB video capture cards which stream uncompressed
  video, and to stream full NTSC + PAL videos we'd need
  
  NTSC 640x480 YUV422 @30fps  ~17.6 MB/s
  PAL  720x576 YUV422 @25fps  ~19.7 MB/s
  
  isoc bandwidth.
  
  Now, due to limited alt settings in capture devices NTSC one ends up
  streaming with max_pkt_size=2688  and  PAL with max_pkt_size=2892, both
  with interval=1. In terms of microframe time allocation this gives
  
  NTSC~53us
  PAL ~57us
  
  and together
  
  ~110us100us == 80% of 125us uframe time.
  
  So those two devices can't work together simultaneously because the'd
  over allocate isochronous bandwidth.
  
  80% seemed a bit arbitrary to me, and I've tried to raise it to 90% and
  both devices started to work together, so I though sometimes it would be
  a good idea for users to override hardcoded default of max 80% isoc
  bandwidth.
  
  After all, isn't it a user who should decide how to load the bus? If I
  can live with 10% or even 5% bulk bandwidth that should be ok. I'm a USB
  newcomer, but that 80% seems to be chosen pretty arbitrary to me, just
  to serve as a reasonable default.
 
 This seems like the sort of feature somebody might reasonably want to 
 use -- if they know exactly what they're doing.

Yes, thanks, exactly my case. Now I know the idea won't be rejected it
can be polished.


  NOTE: for two streams with max_pkt_size=3072 (worst case) both time
  allocation would be 60us+60us=120us which is 96% periodic bandwidth
  leaving 4% for bulk and control. I think this should work too.
 
 At 480 Mb/s, each microframe holds 7500 bytes (less if you count 
 bit-stuffing).  4% of that is 300 bytes, which is not enough for a 
 512-byte bulk packet.  I think you'd run into trouble trying to do any 
 serious bulk transfers on such a tight schedule.

Yes, you seem to be right.

I still think 4% is maybe enough for control traffic.


  Signed-off-by: Kirill Smelkov k...@mns.spb.ru
  Cc: Alan Stern st...@rowland.harvard.edu
  ---
   drivers/usb/host/ehci-hcd.c   |   16 
   drivers/usb/host/ehci-sched.c |   17 +++--
   2 files changed, 23 insertions(+), 10 deletions(-)
  
  diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
  index c606b02..1d36e72 100644
  --- a/drivers/usb/host/ehci-hcd.c
  +++ b/drivers/usb/host/ehci-hcd.c
  @@ -112,6 +112,14 @@ static unsigned int hird;
   module_param(hird, int, S_IRUGO);
   MODULE_PARM_DESC(hird, host initiated resume duration, +1 for each 
  75us\n);
   
  +/*
  + * max periodic time per microframe
  + * (be careful, USB 2.0 requires it to be 100us = 80% of 125us)
  + */
  +static unsigned int uframe_periodic_max = 100;
  +module_param(uframe_periodic_max, uint, S_IRUGO);
  +MODULE_PARM_DESC(uframe_periodic_max, maximum allowed periodic part of a 
  microframe, us);
  +
 
 This probably should be a sysfs attribute rather than a module 
 parameter, so that it can be applied to individual buses separately.

Agree


   #defineINTR_MASK (STS_IAA | STS_FATAL | STS_PCD | STS_ERR | STS_INT)
   
   
  /*-*/
  @@ -571,6 +579,14 @@ static int ehci_init(struct usb_hcd *hcd)
  hcc_params = ehci_readl(ehci, ehci-caps-hcc_params);
   
  /*
  +* tell user, if using non-standard (80% == 100 usec/uframe) bandwidth
  +*/
  +   if (uframe_periodic_max != 100)
  +   ehci_info(ehci, using non-standard max periodic bandwith 
  +   (%u%% == %u usec/uframe),
  +   100*uframe_periodic_max/125, 
  uframe_periodic_max);
  +
  +   /*
 
 Check for invalid values.  This should never be less than 100 or 
 greater than 125.

Ok. By the way, why should we limit it to be not less than 100?
Likewise, a user who knows exactly what he/she is doing could limit
periodic bandwidth to be less than 80% required by USB specification.


   * hw default: 1K periodic list heads, one per frame.
   * periodic_size can shrink by USBCMD update if hcc_params allows.
   */
  diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
  index d12426f..fb374f2 100644
  --- a/drivers/usb/host/ehci-sched.c
  +++ b/drivers/usb/host/ehci-sched.c
  @@ -172,7 +172,7 @@ periodic_usecs (struct ehci_hcd *ehci, unsigned frame, 
  unsigned uframe)
  }
  }
   #ifdef DEBUG
  -   if (usecs  100)
  +   if (usecs  uframe_periodic_max)
 
 These changes all seem right.

Thanks. I'll try to prepare updated patch.


Kirill
--
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] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-06-23 Thread Kirill Smelkov
On Wed, Jun 22, 2011 at 10:35:44AM -0700, matt mooney wrote:
 On 10:30 Wed 22 Jun , matt mooney wrote:
  On 20:02 Wed 22 Jun , Kirill Smelkov wrote:
   There are cases, when 80% max isochronous bandwidth is too limiting.
   
   For example I have two USB video capture cards which stream uncompressed
   video, and to stream full NTSC + PAL videos we'd need
   
   NTSC 640x480 YUV422 @30fps  ~17.6 MB/s
   PAL  720x576 YUV422 @25fps  ~19.7 MB/s
   
   isoc bandwidth.
   
   Now, due to limited alt settings in capture devices NTSC one ends up
   streaming with max_pkt_size=2688  and  PAL with max_pkt_size=2892, both
   with interval=1. In terms of microframe time allocation this gives
   
   NTSC~53us
   PAL ~57us
   
   and together
   
   ~110us100us == 80% of 125us uframe time.
   
   So those two devices can't work together simultaneously because the'd
   over allocate isochronous bandwidth.
   
   80% seemed a bit arbitrary to me, and I've tried to raise it to 90% and
   both devices started to work together, so I though sometimes it would be
   a good idea for users to override hardcoded default of max 80% isoc
   bandwidth.
  
  There is nothing arbitrary about 80%. The USB 2.0 Specification constrains
  periodic transfers for high-speed endpoints to 80% of the microframe. See
  section 5.6.4.
  
 
 Looking at the patch, I see that you probably already knew that.
 
 So I digress and defer to the USB experts ;)

Yes, it was meant as 80% being arbitrary chosen by USB 2.0
specification. Notes taken - I'll clarify patch description.


Thanks for commenting,
Kirill
--
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


[RFC, PATCH] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-06-22 Thread Kirill Smelkov
There are cases, when 80% max isochronous bandwidth is too limiting.

For example I have two USB video capture cards which stream uncompressed
video, and to stream full NTSC + PAL videos we'd need

NTSC 640x480 YUV422 @30fps  ~17.6 MB/s
PAL  720x576 YUV422 @25fps  ~19.7 MB/s

isoc bandwidth.

Now, due to limited alt settings in capture devices NTSC one ends up
streaming with max_pkt_size=2688  and  PAL with max_pkt_size=2892, both
with interval=1. In terms of microframe time allocation this gives

NTSC~53us
PAL ~57us

and together

~110us100us == 80% of 125us uframe time.

So those two devices can't work together simultaneously because the'd
over allocate isochronous bandwidth.

80% seemed a bit arbitrary to me, and I've tried to raise it to 90% and
both devices started to work together, so I though sometimes it would be
a good idea for users to override hardcoded default of max 80% isoc
bandwidth.

After all, isn't it a user who should decide how to load the bus? If I
can live with 10% or even 5% bulk bandwidth that should be ok. I'm a USB
newcomer, but that 80% seems to be chosen pretty arbitrary to me, just
to serve as a reasonable default.

NOTE: for two streams with max_pkt_size=3072 (worst case) both time
allocation would be 60us+60us=120us which is 96% periodic bandwidth
leaving 4% for bulk and control. I think this should work too.

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Cc: Alan Stern st...@rowland.harvard.edu
---
 drivers/usb/host/ehci-hcd.c   |   16 
 drivers/usb/host/ehci-sched.c |   17 +++--
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index c606b02..1d36e72 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -112,6 +112,14 @@ static unsigned int hird;
 module_param(hird, int, S_IRUGO);
 MODULE_PARM_DESC(hird, host initiated resume duration, +1 for each 75us\n);
 
+/*
+ * max periodic time per microframe
+ * (be careful, USB 2.0 requires it to be 100us = 80% of 125us)
+ */
+static unsigned int uframe_periodic_max = 100;
+module_param(uframe_periodic_max, uint, S_IRUGO);
+MODULE_PARM_DESC(uframe_periodic_max, maximum allowed periodic part of a 
microframe, us);
+
 #defineINTR_MASK (STS_IAA | STS_FATAL | STS_PCD | STS_ERR | STS_INT)
 
 /*-*/
@@ -571,6 +579,14 @@ static int ehci_init(struct usb_hcd *hcd)
hcc_params = ehci_readl(ehci, ehci-caps-hcc_params);
 
/*
+* tell user, if using non-standard (80% == 100 usec/uframe) bandwidth
+*/
+   if (uframe_periodic_max != 100)
+   ehci_info(ehci, using non-standard max periodic bandwith 
+   (%u%% == %u usec/uframe),
+   100*uframe_periodic_max/125, 
uframe_periodic_max);
+
+   /*
 * hw default: 1K periodic list heads, one per frame.
 * periodic_size can shrink by USBCMD update if hcc_params allows.
 */
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index d12426f..fb374f2 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -172,7 +172,7 @@ periodic_usecs (struct ehci_hcd *ehci, unsigned frame, 
unsigned uframe)
}
}
 #ifdef DEBUG
-   if (usecs  100)
+   if (usecs  uframe_periodic_max)
ehci_err (ehci, uframe %d sched overrun: %d usecs\n,
frame * 8 + uframe, usecs);
 #endif
@@ -709,11 +709,8 @@ static int check_period (
if (uframe = 8)
return 0;
 
-   /*
-* 80% periodic == 100 usec/uframe available
-* convert usecs we need to max already claimed
-*/
-   usecs = 100 - usecs;
+   /* convert usecs we need to max already claimed */
+   usecs = uframe_periodic_max - usecs;
 
/* we know 2 and 4 uframe intervals were rejected; so
 * for period 0, check _every_ microframe in the schedule.
@@ -1286,9 +1283,9 @@ itd_slot_ok (
 {
uframe %= period;
do {
-   /* can't commit more than 80% periodic == 100 usec */
+   /* can't commit more than uframe_periodic_max usec */
if (periodic_usecs (ehci, uframe  3, uframe  0x7)
-(100 - usecs))
+(uframe_periodic_max - usecs))
return 0;
 
/* we know urb-interval is 2^N uframes */
@@ -1345,7 +1342,7 @@ sitd_slot_ok (
 #endif
 
/* check starts (OUT uses more than one) */
-   max_used = 100 - stream-usecs;
+   max_used = uframe_periodic_max - stream-usecs;
for (tmp = stream-raw_mask  0xff; tmp; tmp = 1, uf++) {
if (periodic_usecs (ehci, frame, uf)  max_used)
return 0

[PATCH] uvcvideo: Add FIX_BANDWIDTH quirk to HP Webcam found on HP Mini 5103 netbook

2011-06-21 Thread Kirill Smelkov
The camera there identifeis itself as being manufactured by Cheng Uei
Precision Industry Co., Ltd (Foxlink), and product is titled as HP
Webcam [2 MP Fixed].

I was trying to get 2 USB video capture devices to work simultaneously,
and noticed that the above mentioned webcam always requires packet size
= 3072 bytes per micro frame (~= 23.4 MB/s isoc bandwidth), which is far
more than enough to get standard NTSC 640x480x2x30 = ~17.6 MB/s isoc
bandwidth.

As there are alt interfaces with smaller MxPS

T:  Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  2 Spd=480  MxCh= 0
D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=05c8 ProdID=0403 Rev= 1.06
S:  Manufacturer=Foxlink
S:  Product=HP Webcam [2 MP Fixed]
S:  SerialNumber=200909240102
C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
A:  FirstIf#= 0 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
I:* If#= 0 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 Driver=uvcvideo
E:  Ad=83(I) Atr=03(Int.) MxPS=  16 Ivl=4ms
I:* If#= 1 Alt= 0 #EPs= 0 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
I:  If#= 1 Alt= 1 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS= 128 Ivl=125us
I:  If#= 1 Alt= 2 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS= 512 Ivl=125us
I:  If#= 1 Alt= 3 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=1024 Ivl=125us
I:  If#= 1 Alt= 4 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=1536 Ivl=125us
I:  If#= 1 Alt= 5 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=2048 Ivl=125us
I:  If#= 1 Alt= 6 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=2688 Ivl=125us
I:  If#= 1 Alt= 7 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=05(Isoc) MxPS=3072 Ivl=125us

UVC_QUIRK_FIX_BANDWIDTH helps here and NTSC video can be served with
MxPS=2688 i.e. 20.5 MB/s isoc bandwidth.

In terms of microframe time allocation, before the quirk NTSC video
required 60 usecs / microframe and 53 usecs / microframe after.


P.S. Now with tweaked ehci-hcd to allow up to 90% isoc bandwith (instead of
allowed for high speed 80%) I can capture two video sources -- PAL 720x576
YUV422 @25fps + NTSC 640x480 YUV422 @30fps simultaneously. Hooray!

Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
 drivers/media/video/uvc/uvc_driver.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/uvc/uvc_driver.c 
b/drivers/media/video/uvc/uvc_driver.c
index b6eae48..f633700 100644
--- a/drivers/media/video/uvc/uvc_driver.c
+++ b/drivers/media/video/uvc/uvc_driver.c
@@ -2130,6 +2130,15 @@ static struct usb_device_id uvc_ids[] = {
  .bInterfaceProtocol   = 0,
  .driver_info  = UVC_QUIRK_PROBE_MINMAX
| UVC_QUIRK_BUILTIN_ISIGHT },
+   /* Foxlink (HP Webcam on HP Mini 5103) */
+   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
+   | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x05c8,
+ .idProduct= 0x0403,
+ .bInterfaceClass  = USB_CLASS_VIDEO,
+ .bInterfaceSubClass   = 1,
+ .bInterfaceProtocol   = 0,
+ .driver_info  = UVC_QUIRK_FIX_BANDWIDTH },
/* Genesys Logic USB 2.0 PC Camera */
{ .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
-- 
1.7.6.rc1

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