[PATCH,v2] [media] vivi: Constify structures
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
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
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
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
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
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
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
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
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
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
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()
││ 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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
[ 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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