Re: [PATCH 2/5] media: ov7670: make try_fmt() consistent with 'min_height' and 'min_width'.
On 26 September 2012 18:42, Jonathan Corbet cor...@lwn.net wrote: On Wed, 26 Sep 2012 11:47:54 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: 'min_height' and 'min_width' are variables that allow to specify the minimum resolution that the sensor will achieve. This patch make v4l2 fmt callbacks consider this parameters in order to return valid data to user space. I'd have preferred to do this differently, perhaps backing toward larger sizes if the minimum turns out to be violated. But so be it... Acked-by: Jonathan Corbet cor...@lwn.net jon Thank you. I will have to modify this patch slightly when I fix the previous one though. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 3/5] media: ov7670: calculate framerate properly for ov7675.
On 26 September 2012 18:50, Jonathan Corbet cor...@lwn.net wrote: On Wed, 26 Sep 2012 11:47:55 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: According to the datasheet ov7675 uses a formula to achieve the desired framerate that is different from the operations done in the current code. In fact, this formula should apply to ov7670 too. This would mean that current code is wrong but, in order to preserve compatibility, the new formula will be used for ov7675 only. At this point I couldn't tell you what the real situation is; it's been a while and there's always a fair amount of black magic involved with ov7670 configuration. I do appreciate attention to not breaking existing users. Indeed, this sensor is the quirkier I've dealt with, with those magic values in non documented registers... +static void ov7670_get_framerate(struct v4l2_subdev *sd, + struct v4l2_fract *tpf) This bugs me, though. It's called ov7670_get_framerate() but it's getting the rate for the ov7675 - confusing. Meanwhile the real ov7670 code remains inline while ov7675 has its own function. Actually, I did this on purpose because I wanted to remark that this function should be valid for both models and because I expected that the old behaviour was removed sometime in the future. Please make two functions, one of which is ov7675_get_framerate(), and call the right one for the model. Same for the set functions, obviously. Maybe what's really needed is a structure full of sensor-specific operations? The get_wsizes() function could go there too. That would take a lot of if statements out of the code. The idea of a structure of sensor-specific operations seems reasonable. Furthermore, I think we should encourage users to use the right formula in the future. For this reason we could define 4 functions ov7670_set_framerate_legacy() ov7670_get_framerate_legacy() ov7675_set_framerate() ov7675_get_framerate() + /* + * The datasheet claims that clkrc = 0 will divide the input clock by 1 + * but we've checked with an oscilloscope that it divides by 2 instead. + * So, if clkrc = 0 just bypass the divider. + */ Thanks for documenting this kind of thing. You are welcome. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 4/5] media: ov7670: add possibility to bypass pll for ov7675.
On 26 September 2012 18:52, Jonathan Corbet cor...@lwn.net wrote: On Wed, 26 Sep 2012 11:47:56 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: Signed-off-by: Javier Martin javier.mar...@vista-silicon.com This one needs a changelog - what does bypassing the PLL do and why might you want to do it? Otherwise: As I stated in a previous patch, frame rate depends on the pixclk. Moreover: pixclk = xvclk / clkrc * PLLfactor Bypassing the PLL means that the PLL gets out of the way so, in practice, PLLfactor = 1 pixclk = xvclk / clkrc For a frame rate of 30 fps a pixclk of 24MHz is needed. Since we have a clean clock signal we want pixclk = xvclk. If one applies the formula in ov7670_set_framerate() with PLLfactor = 1 and clock_speed = 24 MHz the resulting clkrc = 1 which means that: pixclk = xvclk which is what we want Acked-by: Jonathan Corbet cor...@lwn.net Thank you. I will add a changelog when I send v2 of the series. Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 5/5] media: ov7670: Add possibility to disable pixclk during hblank.
On 26 September 2012 18:52, Jonathan Corbet cor...@lwn.net wrote: On Wed, 26 Sep 2012 11:47:57 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c |8 include/media/ov7670.h |1 + 2 files changed, 9 insertions(+) Again, needs a changelog. Otherwise Our soc-camera host driver captures pixels during blanking periods if pixclk is enabled. In order to avoid capturing bogus data we need to disable pixclk during those blanking periods I'll add it to v2. Acked-by: Jonathan Corbet cor...@lwn.net Thank you. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 v2 0/5] media: ov7670: driver cleanup and support for ov7674.
The following series includes all the changes discussed in [1] that don't affect either bridge drivers that use ov7670 or soc-camera framework For this reason they are considered non controversial and sent separately. At least 1 more series will follow in order to implement all features described in [1]. This v2 include changes requested by Jonathan Corbet. See each separate patch. [PATCH v2 1/5] media: ov7670: add support for ov7675. [PATCH v2 2/5] media: ov7670: make try_fmt() consistent with [PATCH v2 3/5] media: ov7670: calculate framerate properly for ov7675. [PATCH v2 4/5] media: ov7670: add possibility to bypass pll for ov7675. [PATCH v2 5/5] media: ov7670: Add possibility to disable pixclk during -- 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 v2 1/5] media: ov7670: add support for ov7675.
ov7675 and ov7670 share the same registers but there is no way to distinguish them at runtime. However, they require different tweaks to achieve the desired resolution. For this reason this patch adds a new ov7675 entry to the ov7670_id table. Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- Changes since v1: - Use separate arrays for supported resolutions. - Don't support not tested resolutions in ov7675. --- drivers/media/i2c/ov7670.c | 101 +++- 1 file changed, 72 insertions(+), 29 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index e7c82b2..17eb5d7 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -183,6 +183,27 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); #define REG_HAECC7 0xaa/* Hist AEC/AGC control 7 */ #define REG_BD60MAX0xab/* 60hz banding step limit */ +enum ov7670_model { + MODEL_OV7670 = 0, + MODEL_OV7675, +}; + +struct ov7670_win_size { + int width; + int height; + unsigned char com7_bit; + int hstart; /* Start/stop values for the camera. Note */ + int hstop; /* that they do not always make complete */ + int vstart; /* sense to humans, but evidently the sensor */ + int vstop; /* will do the right thing... */ + struct regval_list *regs; /* Regs to tweak */ +}; + +struct ov7670_devtype { + /* formats supported for each model */ + struct ov7670_win_size *win_sizes; + unsigned int n_win_sizes; +}; /* * Information we maintain about a known sensor. @@ -198,6 +219,7 @@ struct ov7670_info { int clock_speed;/* External clock speed (MHz) */ u8 clkrc; /* Clock divider value */ bool use_smbus; /* Use smbus I/O instead of I2C */ + const struct ov7670_devtype *devtype; /* Device specifics */ }; static inline struct ov7670_info *to_state(struct v4l2_subdev *sd) @@ -652,65 +674,70 @@ static struct regval_list ov7670_qcif_regs[] = { { 0xff, 0xff }, }; -static struct ov7670_win_size { - int width; - int height; - unsigned char com7_bit; - int hstart; /* Start/stop values for the camera. Note */ - int hstop; /* that they do not always make complete */ - int vstart; /* sense to humans, but evidently the sensor */ - int vstop; /* will do the right thing... */ - struct regval_list *regs; /* Regs to tweak */ -/* h/vref stuff */ -} ov7670_win_sizes[] = { +static struct ov7670_win_size ov7670_win_sizes[] = { /* VGA */ { .width = VGA_WIDTH, .height = VGA_HEIGHT, .com7_bit = COM7_FMT_VGA, - .hstart = 158, /* These values from */ - .hstop = 14, /* Omnivision */ + .hstart = 158, /* These values from */ + .hstop = 14, /* Omnivision */ .vstart = 10, .vstop = 490, - .regs = NULL, + .regs = NULL, }, /* CIF */ { .width = CIF_WIDTH, .height = CIF_HEIGHT, .com7_bit = COM7_FMT_CIF, - .hstart = 170, /* Empirically determined */ + .hstart = 170, /* Empirically determined */ .hstop = 90, .vstart = 14, .vstop = 494, - .regs = NULL, + .regs = NULL, }, /* QVGA */ { .width = QVGA_WIDTH, .height = QVGA_HEIGHT, .com7_bit = COM7_FMT_QVGA, - .hstart = 168, /* Empirically determined */ + .hstart = 168, /* Empirically determined */ .hstop = 24, .vstart = 12, .vstop = 492, - .regs = NULL, + .regs = NULL, }, /* QCIF */ { .width = QCIF_WIDTH, .height = QCIF_HEIGHT, .com7_bit = COM7_FMT_VGA, /* see comment above */ - .hstart = 456, /* Empirically determined */ + .hstart = 456, /* Empirically determined */ .hstop = 24, .vstart = 14, .vstop = 494, - .regs = ov7670_qcif_regs, - }, + .regs = ov7670_qcif_regs, + } }; -#define N_WIN_SIZES (ARRAY_SIZE
[PATCH v2 3/5] media: ov7670: calculate framerate properly for ov7675.
According to the datasheet ov7675 uses a formula to achieve the desired framerate that is different from the operations done in the current code. In fact, this formula should apply to ov7670 too. This would mean that current code is wrong but, in order to preserve compatibility, the new formula will be used for ov7675 only. Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- Changes since v1: - Create separate functions for frame rate control. --- drivers/media/i2c/ov7670.c | 136 ++-- 1 file changed, 118 insertions(+), 18 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 3eaa06c..cb62d08 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -47,6 +47,8 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); */ #define OV7670_I2C_ADDR 0x42 +#define PLL_FACTOR 4 + /* Registers */ #define REG_GAIN 0x00/* Gain lower 8 bits (rest in vref) */ #define REG_BLUE 0x01/* blue gain */ @@ -164,6 +166,12 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); #define REG_GFIX 0x69/* Fix gain control */ +#define REG_DBLV 0x6b/* PLL control an debugging */ +#define DBLV_BYPASS0x00/* Bypass PLL */ +#define DBLV_X40x01/* clock x4 */ +#define DBLV_X60x10/* clock x6 */ +#define DBLV_X80x11/* clock x8 */ + #define REG_REG76 0x76/* OV's name */ #define R76_BLKPCOR0x80/* Black pixel correction enable */ #define R76_WHTPCOR0x40/* White pixel correction enable */ @@ -203,6 +211,9 @@ struct ov7670_devtype { /* formats supported for each model */ struct ov7670_win_size *win_sizes; unsigned int n_win_sizes; + /* callbacks for frame rate control */ + int (*set_framerate)(struct v4l2_subdev *, struct v4l2_fract *); + void (*get_framerate)(struct v4l2_subdev *, struct v4l2_fract *); }; /* @@ -739,6 +750,98 @@ static struct ov7670_win_size ov7675_win_sizes[] = { } }; +static void ov7675_get_framerate(struct v4l2_subdev *sd, +struct v4l2_fract *tpf) +{ + struct ov7670_info *info = to_state(sd); + u32 clkrc = info-clkrc; + u32 pll_factor = PLL_FACTOR; + + clkrc++; + if (info-fmt-mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8) + clkrc = (clkrc 1); + + tpf-numerator = 1; + tpf-denominator = (5 * pll_factor * info-clock_speed) / + (4 * clkrc); +} + +static int ov7675_set_framerate(struct v4l2_subdev *sd, +struct v4l2_fract *tpf) +{ + struct ov7670_info *info = to_state(sd); + u32 clkrc; + u32 pll_factor = PLL_FACTOR; + int ret; + + /* +* The formula is fps = 5/4*pixclk for YUV/RGB and +* fps = 5/2*pixclk for RAW. +* +* pixclk = clock_speed / (clkrc + 1) * PLLfactor +* +*/ + if (tpf-numerator == 0 || tpf-denominator == 0) { + clkrc = 0; + } else { + clkrc = (5 * pll_factor * info-clock_speed * tpf-numerator) / + (4 * tpf-denominator); + if (info-fmt-mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8) + clkrc = (clkrc 1); + clkrc--; + } + + /* +* The datasheet claims that clkrc = 0 will divide the input clock by 1 +* but we've checked with an oscilloscope that it divides by 2 instead. +* So, if clkrc = 0 just bypass the divider. +*/ + if (clkrc = 0) + clkrc = CLK_EXT; + else if (clkrc CLK_SCALE) + clkrc = CLK_SCALE; + info-clkrc = clkrc; + + /* Recalculate frame rate */ + ov7675_get_framerate(sd, tpf); + + ret = ov7670_write(sd, REG_CLKRC, info-clkrc); + if (ret 0) + return ret; + return ov7670_write(sd, REG_DBLV, DBLV_X4); +} + +static void ov7670_get_framerate_legacy(struct v4l2_subdev *sd, +struct v4l2_fract *tpf) +{ + struct ov7670_info *info = to_state(sd); + + tpf-numerator = 1; + tpf-denominator = info-clock_speed; + if ((info-clkrc CLK_EXT) == 0 (info-clkrc CLK_SCALE) 1) + tpf-denominator /= (info-clkrc CLK_SCALE); +} + +static int ov7670_set_framerate_legacy(struct v4l2_subdev *sd, + struct v4l2_fract *tpf) +{ + struct ov7670_info *info = to_state(sd); + int div; + + if (tpf-numerator == 0 || tpf-denominator == 0) + div = 1; /* Reset to full rate */ + else + div = (tpf-numerator * info-clock_speed) / tpf-denominator; + if (div == 0) + div = 1; + else if (div CLK_SCALE) + div = CLK_SCALE; + info-clkrc = (info-clkrc 0x80) | div; + tpf-numerator = 1; + tpf-denominator = info
[PATCH v2 2/5] media: ov7670: make try_fmt() consistent with 'min_height' and 'min_width'.
'min_height' and 'min_width' are variables that allow to specify the minimum resolution that the sensor will achieve. This patch make v4l2 fmt callbacks consider this parameters in order to return valid data to user space. Acked-by: Jonathan Corbet cor...@lwn.net Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c | 22 +++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 17eb5d7..3eaa06c 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -786,10 +786,11 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd, struct ov7670_format_struct **ret_fmt, struct ov7670_win_size **ret_wsize) { - int index; + int index, i; struct ov7670_win_size *wsize; struct ov7670_info *info = to_state(sd); unsigned int n_win_sizes = info-devtype-n_win_sizes; + unsigned int win_sizes_limit = n_win_sizes; for (index = 0; index N_OV7670_FMTS; index++) if (ov7670_formats[index].mbus_code == fmt-code) @@ -805,15 +806,30 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd, * Fields: the OV devices claim to be progressive. */ fmt-field = V4L2_FIELD_NONE; + + /* +* Don't consider values that don't match min_height and min_width +* constraints. +*/ + if (info-min_width || info-min_height) + for (i = 0; i n_win_sizes; i++) { + wsize = info-devtype-win_sizes + i; + + if (wsize-width info-min_width || + wsize-height info-min_height) { + win_sizes_limit = i; + break; + } + } /* * Round requested image size down to the nearest * we support, but not below the smallest. */ for (wsize = info-devtype-win_sizes; -wsize info-devtype-win_sizes + n_win_sizes; wsize++) +wsize info-devtype-win_sizes + win_sizes_limit; wsize++) if (fmt-width = wsize-width fmt-height = wsize-height) break; - if (wsize = info-devtype-win_sizes + n_win_sizes) + if (wsize = info-devtype-win_sizes + win_sizes_limit) wsize--; /* Take the smallest one */ if (ret_wsize != NULL) *ret_wsize = wsize; -- 1.7.9.5 -- 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 v2 4/5] media: ov7670: add possibility to bypass pll for ov7675.
For a frame rate of 30 fps a pixclk of 24MHz is needed. For those cases where the ov7670 has a clean 24MHz input (xvclk) the PLL can be bypassed. This will result in a value of clkrc of 1, which means that in practice pixclk = xvclk (input clock) Acked-by: Jonathan Corbet cor...@lwn.net Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- Changes since v1: - Added changelog. --- drivers/media/i2c/ov7670.c | 28 ++-- include/media/ov7670.h |1 + 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index cb62d08..559c23e 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -230,6 +230,7 @@ struct ov7670_info { int clock_speed;/* External clock speed (MHz) */ u8 clkrc; /* Clock divider value */ bool use_smbus; /* Use smbus I/O instead of I2C */ + bool pll_bypass; const struct ov7670_devtype *devtype; /* Device specifics */ }; @@ -755,7 +756,12 @@ static void ov7675_get_framerate(struct v4l2_subdev *sd, { struct ov7670_info *info = to_state(sd); u32 clkrc = info-clkrc; - u32 pll_factor = PLL_FACTOR; + int pll_factor; + + if (info-pll_bypass) + pll_factor = 1; + else + pll_factor = PLL_FACTOR; clkrc++; if (info-fmt-mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8) @@ -771,7 +777,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd, { struct ov7670_info *info = to_state(sd); u32 clkrc; - u32 pll_factor = PLL_FACTOR; + int pll_factor; int ret; /* @@ -781,6 +787,16 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd, * pixclk = clock_speed / (clkrc + 1) * PLLfactor * */ + if (info-pll_bypass) { + pll_factor = 1; + ret = ov7670_write(sd, REG_DBLV, DBLV_BYPASS); + } else { + pll_factor = PLL_FACTOR; + ret = ov7670_write(sd, REG_DBLV, DBLV_X4); + } + if (ret 0) + return ret; + if (tpf-numerator == 0 || tpf-denominator == 0) { clkrc = 0; } else { @@ -808,6 +824,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd, ret = ov7670_write(sd, REG_CLKRC, info-clkrc); if (ret 0) return ret; + return ov7670_write(sd, REG_DBLV, DBLV_X4); } @@ -1688,6 +1705,13 @@ static int ov7670_probe(struct i2c_client *client, if (config-clock_speed) info-clock_speed = config-clock_speed; + + /* +* It should be allowed for ov7670 too when it is migrated to +* the new frame rate formula. +*/ + if (config-pll_bypass id-driver_data != MODEL_OV7670) + info-pll_bypass = true; } /* Make sure it's an ov7670 */ diff --git a/include/media/ov7670.h b/include/media/ov7670.h index b133bc1..a68c8bb 100644 --- a/include/media/ov7670.h +++ b/include/media/ov7670.h @@ -15,6 +15,7 @@ struct ov7670_config { int min_height; /* Filter out smaller sizes */ int clock_speed;/* External clock speed (MHz) */ bool use_smbus; /* Use smbus I/O instead of I2C */ + bool pll_bypass;/* Choose whether to bypass the PLL */ }; #endif -- 1.7.9.5 -- 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 v2 5/5] media: ov7670: Add possibility to disable pixclk during hblank.
Some bridge drivers captures pixels during blanking periods if pixclk is enabled. In order to avoid capturing bogus data we need to disable pixclk in the sensor during those blanking periods. Acked-by: Jonathan Corbet cor...@lwn.net Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- Changes since v1: - Added changelog. --- drivers/media/i2c/ov7670.c |7 +++ include/media/ov7670.h |1 + 2 files changed, 8 insertions(+) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 559c23e..9d7a92e 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -231,6 +231,7 @@ struct ov7670_info { u8 clkrc; /* Clock divider value */ bool use_smbus; /* Use smbus I/O instead of I2C */ bool pll_bypass; + bool pclk_hb_disable; const struct ov7670_devtype *devtype; /* Device specifics */ }; @@ -1712,6 +1713,9 @@ static int ov7670_probe(struct i2c_client *client, */ if (config-pll_bypass id-driver_data != MODEL_OV7670) info-pll_bypass = true; + + if (config-pclk_hb_disable) + info-pclk_hb_disable = true; } /* Make sure it's an ov7670 */ @@ -1736,6 +1740,9 @@ static int ov7670_probe(struct i2c_client *client, tpf.denominator = 30; info-devtype-set_framerate(sd, tpf); + if (info-pclk_hb_disable) + ov7670_write(sd, REG_COM10, COM10_PCLK_HB); + return 0; } diff --git a/include/media/ov7670.h b/include/media/ov7670.h index a68c8bb..1913d51 100644 --- a/include/media/ov7670.h +++ b/include/media/ov7670.h @@ -16,6 +16,7 @@ struct ov7670_config { int clock_speed;/* External clock speed (MHz) */ bool use_smbus; /* Use smbus I/O instead of I2C */ bool pll_bypass;/* Choose whether to bypass the PLL */ + bool pclk_hb_disable; /* Disable toggling pixclk during horizontal blanking */ }; #endif -- 1.7.9.5 -- 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 0/3] ov7670: migrate this sensor and its users to ctrl framework.
The following series migrate ov7670 sensor and current users to ctrl framework as discussed in [1]. This has been tested against mx2_camera soc-camera bridge, so tests or acks will be required from people using cam-core and via-camera out there. This will have to be applied on top of my previous series: [PATCH v2 0/5] media: ov7670: driver cleanup and support for ov7674. All submitted patches have been obtained from Hans' tree and slightly modified to apply on current media_tree for-v3.7 branch: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/cafe-ctrl [PATCH 1/3] ov7670: use the control framework [PATCH 2/3] mcam-core: implement the control framework. [PATCH 3/3] via-camera: implement the control framework. [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg51778.html -- 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/3] mcam-core: implement the control framework.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 54 --- drivers/media/platform/marvell-ccic/mcam-core.h |2 + 2 files changed, 11 insertions(+), 45 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index ce2b7b4..568f8a5 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -22,6 +22,7 @@ #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-ioctl.h +#include media/v4l2-ctrls.h #include media/v4l2-chip-ident.h #include media/ov7670.h #include media/videobuf2-vmalloc.h @@ -1232,47 +1233,6 @@ static int mcam_vidioc_dqbuf(struct file *filp, void *priv, return ret; } - - -static int mcam_vidioc_queryctrl(struct file *filp, void *priv, - struct v4l2_queryctrl *qc) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, queryctrl, qc); - mutex_unlock(cam-s_mutex); - return ret; -} - - -static int mcam_vidioc_g_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, g_ctrl, ctrl); - mutex_unlock(cam-s_mutex); - return ret; -} - - -static int mcam_vidioc_s_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, s_ctrl, ctrl); - mutex_unlock(cam-s_mutex); - return ret; -} - - static int mcam_vidioc_querycap(struct file *file, void *priv, struct v4l2_capability *cap) { @@ -1520,9 +1480,6 @@ static const struct v4l2_ioctl_ops mcam_v4l_ioctl_ops = { .vidioc_dqbuf = mcam_vidioc_dqbuf, .vidioc_streamon= mcam_vidioc_streamon, .vidioc_streamoff = mcam_vidioc_streamoff, - .vidioc_queryctrl = mcam_vidioc_queryctrl, - .vidioc_g_ctrl = mcam_vidioc_g_ctrl, - .vidioc_s_ctrl = mcam_vidioc_s_ctrl, .vidioc_g_parm = mcam_vidioc_g_parm, .vidioc_s_parm = mcam_vidioc_s_parm, .vidioc_enum_framesizes = mcam_vidioc_enum_framesizes, @@ -1786,14 +1743,19 @@ int mccic_register(struct mcam_camera *cam) /* * Get the v4l2 setup done. */ + ret = v4l2_ctrl_handler_init(cam-ctrl_handler, 10); + if (ret) + goto out_unregister; + cam-v4l2_dev.ctrl_handler = cam-ctrl_handler; + mutex_lock(cam-s_mutex); cam-vdev = mcam_v4l_template; cam-vdev.debug = 0; cam-vdev.v4l2_dev = cam-v4l2_dev; + video_set_drvdata(cam-vdev, cam); ret = video_register_device(cam-vdev, VFL_TYPE_GRABBER, -1); if (ret) goto out; - video_set_drvdata(cam-vdev, cam); /* * If so requested, try to get our DMA buffers now. @@ -1805,6 +1767,7 @@ int mccic_register(struct mcam_camera *cam) } out: + v4l2_ctrl_handler_free(cam-ctrl_handler); mutex_unlock(cam-s_mutex); return ret; out_unregister: @@ -1829,6 +1792,7 @@ void mccic_shutdown(struct mcam_camera *cam) if (cam-buffer_mode == B_vmalloc) mcam_free_dma_bufs(cam); video_unregister_device(cam-vdev); + v4l2_ctrl_handler_free(cam-ctrl_handler); v4l2_device_unregister(cam-v4l2_dev); } diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index bd6acba..f7c34ab 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -8,6 +8,7 @@ #include linux/list.h #include media/v4l2-common.h +#include media/v4l2-ctrls.h #include media/v4l2-dev.h #include media/videobuf2-core.h @@ -104,6 +105,7 @@ struct mcam_camera { * should not be touched by the platform code. */ struct v4l2_device v4l2_dev; + struct v4l2_ctrl_handler ctrl_handler; enum mcam_state state; unsigned long flags;/* Buffer status, mainly (dev_lock) */ int users; /* How many open FDs */ -- 1.7.9.5 -- 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/3] via-camera: implement the control framework.
And added a missing kfree to clean up the via_camera struct. Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/platform/via-camera.c | 60 --- 1 file changed, 14 insertions(+), 46 deletions(-) diff --git a/drivers/media/platform/via-camera.c b/drivers/media/platform/via-camera.c index eb404c2..1b5f97d 100644 --- a/drivers/media/platform/via-camera.c +++ b/drivers/media/platform/via-camera.c @@ -18,6 +18,7 @@ #include media/v4l2-device.h #include media/v4l2-ioctl.h #include media/v4l2-chip-ident.h +#include media/v4l2-ctrls.h #include media/ov7670.h #include media/videobuf-dma-sg.h #include linux/delay.h @@ -63,6 +64,7 @@ enum viacam_opstate { S_IDLE = 0, S_RUNNING = 1 }; struct via_camera { struct v4l2_device v4l2_dev; + struct v4l2_ctrl_handler ctrl_handler; struct video_device vdev; struct v4l2_subdev *sensor; struct platform_device *platdev; @@ -818,47 +820,6 @@ static int viacam_g_chip_ident(struct file *file, void *priv, } /* - * Control ops are passed through to the sensor. - */ -static int viacam_queryctrl(struct file *filp, void *priv, - struct v4l2_queryctrl *qc) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, queryctrl, qc); - mutex_unlock(cam-lock); - return ret; -} - - -static int viacam_g_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, g_ctrl, ctrl); - mutex_unlock(cam-lock); - return ret; -} - - -static int viacam_s_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, s_ctrl, ctrl); - mutex_unlock(cam-lock); - return ret; -} - -/* * Only one input. */ static int viacam_enum_input(struct file *filp, void *priv, @@ -1214,9 +1175,6 @@ static int viacam_enum_frameintervals(struct file *filp, void *priv, static const struct v4l2_ioctl_ops viacam_ioctl_ops = { .vidioc_g_chip_ident= viacam_g_chip_ident, - .vidioc_queryctrl = viacam_queryctrl, - .vidioc_g_ctrl = viacam_g_ctrl, - .vidioc_s_ctrl = viacam_s_ctrl, .vidioc_enum_input = viacam_enum_input, .vidioc_g_input = viacam_g_input, .vidioc_s_input = viacam_s_input, @@ -1418,8 +1376,12 @@ static __devinit int viacam_probe(struct platform_device *pdev) ret = v4l2_device_register(pdev-dev, cam-v4l2_dev); if (ret) { dev_err(pdev-dev, Unable to register v4l2 device\n); - return ret; + goto out_free; } + ret = v4l2_ctrl_handler_init(cam-ctrl_handler, 10); + if (ret) + goto out_unregister; + cam-v4l2_dev.ctrl_handler = cam-ctrl_handler; /* * Convince the system that we can do DMA. */ @@ -1436,7 +1398,7 @@ static __devinit int viacam_probe(struct platform_device *pdev) */ ret = via_sensor_power_setup(cam); if (ret) - goto out_unregister; + goto out_ctrl_hdl_free; via_sensor_power_up(cam); /* @@ -1485,8 +1447,12 @@ out_irq: free_irq(viadev-pdev-irq, cam); out_power_down: via_sensor_power_release(cam); +out_ctrl_hdl_free: + v4l2_ctrl_handler_free(cam-ctrl_handler); out_unregister: v4l2_device_unregister(cam-v4l2_dev); +out_free: + kfree(cam); return ret; } @@ -1499,6 +1465,8 @@ static __devexit int viacam_remove(struct platform_device *pdev) v4l2_device_unregister(cam-v4l2_dev); free_irq(viadev-pdev-irq, cam); via_sensor_power_release(cam); + v4l2_ctrl_handler_free(cam-ctrl_handler); + kfree(cam); via_cam_info = NULL; return 0; } -- 1.7.9.5 -- 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/3] ov7670: use the control framework
Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c | 295 +--- 1 file changed, 115 insertions(+), 180 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 9d7a92e..eead1b4 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -18,6 +18,7 @@ #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-chip-ident.h +#include media/v4l2-ctrls.h #include media/v4l2-mediabus.h #include media/ov7670.h @@ -222,9 +223,23 @@ struct ov7670_devtype { struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; + struct { + /* gain cluster */ + struct v4l2_ctrl *auto_gain; + struct v4l2_ctrl *gain; + }; + struct { + /* exposure cluster */ + struct v4l2_ctrl *auto_exposure; + struct v4l2_ctrl *exposure; + }; + struct { + /* saturation/hue cluster */ + struct v4l2_ctrl *saturation; + struct v4l2_ctrl *hue; + }; struct ov7670_format_struct *fmt; /* Current format */ - unsigned char sat; /* Saturation value */ - int hue;/* Hue value */ int min_width; /* Filter out smaller sizes */ int min_height; /* Filter out smaller sizes */ int clock_speed;/* External clock speed (MHz) */ @@ -240,6 +255,11 @@ static inline struct ov7670_info *to_state(struct v4l2_subdev *sd) return container_of(sd, struct ov7670_info, sd); } +static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl) +{ + return container_of(ctrl-handler, struct ov7670_info, hdl)-sd; +} + /* @@ -1195,23 +1215,23 @@ static int ov7670_cosine(int theta) static void ov7670_calc_cmatrix(struct ov7670_info *info, - int matrix[CMATRIX_LEN]) + int matrix[CMATRIX_LEN], int sat, int hue) { int i; /* * Apply the current saturation setting first. */ for (i = 0; i CMATRIX_LEN; i++) - matrix[i] = (info-fmt-cmatrix[i]*info-sat) 7; + matrix[i] = (info-fmt-cmatrix[i] * sat) 7; /* * Then, if need be, rotate the hue value. */ - if (info-hue != 0) { + if (hue != 0) { int sinth, costh, tmpmatrix[CMATRIX_LEN]; memcpy(tmpmatrix, matrix, CMATRIX_LEN*sizeof(int)); - sinth = ov7670_sine(info-hue); - costh = ov7670_cosine(info-hue); + sinth = ov7670_sine(hue); + costh = ov7670_cosine(hue); matrix[0] = (matrix[3]*sinth + matrix[0]*costh)/1000; matrix[1] = (matrix[4]*sinth + matrix[1]*costh)/1000; @@ -1224,60 +1244,21 @@ static void ov7670_calc_cmatrix(struct ov7670_info *info, -static int ov7670_s_sat(struct v4l2_subdev *sd, int value) -{ - struct ov7670_info *info = to_state(sd); - int matrix[CMATRIX_LEN]; - int ret; - - info-sat = value; - ov7670_calc_cmatrix(info, matrix); - ret = ov7670_store_cmatrix(sd, matrix); - return ret; -} - -static int ov7670_g_sat(struct v4l2_subdev *sd, __s32 *value) -{ - struct ov7670_info *info = to_state(sd); - - *value = info-sat; - return 0; -} - -static int ov7670_s_hue(struct v4l2_subdev *sd, int value) +static int ov7670_s_sat_hue(struct v4l2_subdev *sd, int sat, int hue) { struct ov7670_info *info = to_state(sd); int matrix[CMATRIX_LEN]; int ret; - if (value -180 || value 180) - return -EINVAL; - info-hue = value; - ov7670_calc_cmatrix(info, matrix); + ov7670_calc_cmatrix(info, matrix, sat, hue); ret = ov7670_store_cmatrix(sd, matrix); return ret; } -static int ov7670_g_hue(struct v4l2_subdev *sd, __s32 *value) -{ - struct ov7670_info *info = to_state(sd); - - *value = info-hue; - return 0; -} - - /* * Some weird registers seem to store values in a sign/magnitude format! */ -static unsigned char ov7670_sm_to_abs(unsigned char v) -{ - if ((v 0x80) == 0) - return v + 128; - return 128 - (v 0x7f); -} - static unsigned char ov7670_abs_to_sm(unsigned char v) { @@ -1299,40 +1280,11 @@ static int ov7670_s_brightness(struct v4l2_subdev *sd, int value) return ret; } -static int ov7670_g_brightness(struct v4l2_subdev *sd, __s32 *value) -{ - unsigned char v = 0; - int ret = ov7670_read(sd, REG_BRIGHT, v); - - *value = ov7670_sm_to_abs(v); - return ret; -} - static int ov7670_s_contrast(struct v4l2_subdev *sd, int value) { return ov7670_write(sd
Re: [PATCH 1/3] ov7670: use the control framework
Hi Hans, On 28 September 2012 10:23, Hans Verkuil hverk...@xs4all.nl wrote: On Fri September 28 2012 09:48:01 Javier Martin wrote: static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ctrl = ov7670_g_ctrl, - .s_ctrl = ov7670_s_ctrl, - .queryctrl = ov7670_queryctrl, + .g_ext_ctrls = v4l2_subdev_g_ext_ctrls, + .try_ext_ctrls = v4l2_subdev_try_ext_ctrls, + .s_ext_ctrls = v4l2_subdev_s_ext_ctrls, + .g_ctrl = v4l2_subdev_g_ctrl, + .s_ctrl = v4l2_subdev_s_ctrl, + .queryctrl = v4l2_subdev_queryctrl, + .querymenu = v4l2_subdev_querymenu, Can you make a fourth patch removing these lines again after mcam-core and via-camera are converted to the control framework? These control ops are only needed if there are bridge drivers using this sensor driver that are not yet converted to the control framework. Since that limitation is now lifted, these ops can be removed as well. Fine, I will do that. .reset = ov7670_reset, .init = ov7670_init, #ifdef CONFIG_VIDEO_ADV_DEBUG @@ -1732,7 +1630,6 @@ static int ov7670_probe(struct i2c_client *client, info-devtype = ov7670_devdata[id-driver_data]; info-fmt = ov7670_formats[0]; - info-sat = 128;/* Review this */ info-clkrc = 0; /* Set default frame rate to 30 fps */ @@ -1743,6 +1640,42 @@ static int ov7670_probe(struct i2c_client *client, if (info-pclk_hb_disable) ov7670_write(sd, REG_COM10, COM10_PCLK_HB); + v4l2_ctrl_handler_init(info-hdl, 10); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_BRIGHTNESS, 0, 255, 1, 128); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_CONTRAST, 0, 127, 1, 64); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_VFLIP, 0, 1, 1, 0); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_HFLIP, 0, 1, 1, 0); + info-saturation = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_SATURATION, 0, 256, 1, 128); + info-hue = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_HUE, -180, 180, 5, 0); + info-gain = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_GAIN, 0, 255, 1, 128); + info-auto_gain = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_AUTOGAIN, 0, 1, 1, 1); + info-exposure = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, + V4L2_CID_EXPOSURE, 0, 65535, 1, 500); + info-auto_exposure = v4l2_ctrl_new_std_menu(info-hdl, ov7670_ctrl_ops, + V4L2_CID_EXPOSURE_AUTO, 1, 0, 0); + sd-ctrl_handler = info-hdl; + if (info-hdl.error) { + int err = info-hdl.error; + + v4l2_ctrl_handler_free(info-hdl); + kfree(info); + return err; + } + info-gain-flags |= V4L2_CTRL_FLAG_VOLATILE; + info-exposure-flags |= V4L2_CTRL_FLAG_VOLATILE; + v4l2_ctrl_cluster(2, info-auto_gain); + v4l2_ctrl_cluster(2, info-auto_exposure); You need to use v4l2_ctrl_auto_cluster for these two. Not having that function at the time was the reason my work on this driver stalled. The auto cluster functionality takes care of most of the nitty gritty details of handling these situations. OK, let me take a look. Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 v2 0/4] ov7670: migrate this sensor and its users to ctrl framework.
The following series migrate ov7670 sensor and current users to ctrl framework as discussed in [1]. This has been tested against mx2_camera soc-camera bridge, so tests or acks will be required from people using cam-core and via-camera out there. This will have to be applied on top of my previous series: [PATCH v2 0/5] media: ov7670: driver cleanup and support for ov7674. All submitted patches have been obtained from Hans' tree and slightly modified to apply on current media_tree for-v3.7 branch: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/cafe-ctrl Changes since v1: - A 4th patch has been added as requested by Hans. - See changelog in patch 1. [PATCH v2 1/4] ov7670: use the control framework [PATCH v2 2/4] mcam-core: implement the control framework. [PATCH v2 3/4] via-camera: implement the control framework. [PATCH v2 4/4] ov7670: remove legacy ctrl callbacks. [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg51778.html -- 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 v2 1/4] ov7670: use the control framework
Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- Changes since v1: - Use v4l2_ctrl_auto_cluster() for auto_gain and auto_exp. --- drivers/media/i2c/ov7670.c | 310 1 file changed, 112 insertions(+), 198 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 9d7a92e..babd55c 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -18,6 +18,7 @@ #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-chip-ident.h +#include media/v4l2-ctrls.h #include media/v4l2-mediabus.h #include media/ov7670.h @@ -222,9 +223,23 @@ struct ov7670_devtype { struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; + struct { + /* gain cluster */ + struct v4l2_ctrl *auto_gain; + struct v4l2_ctrl *gain; + }; + struct { + /* exposure cluster */ + struct v4l2_ctrl *auto_exposure; + struct v4l2_ctrl *exposure; + }; + struct { + /* saturation/hue cluster */ + struct v4l2_ctrl *saturation; + struct v4l2_ctrl *hue; + }; struct ov7670_format_struct *fmt; /* Current format */ - unsigned char sat; /* Saturation value */ - int hue;/* Hue value */ int min_width; /* Filter out smaller sizes */ int min_height; /* Filter out smaller sizes */ int clock_speed;/* External clock speed (MHz) */ @@ -240,6 +255,11 @@ static inline struct ov7670_info *to_state(struct v4l2_subdev *sd) return container_of(sd, struct ov7670_info, sd); } +static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl) +{ + return container_of(ctrl-handler, struct ov7670_info, hdl)-sd; +} + /* @@ -1195,23 +1215,23 @@ static int ov7670_cosine(int theta) static void ov7670_calc_cmatrix(struct ov7670_info *info, - int matrix[CMATRIX_LEN]) + int matrix[CMATRIX_LEN], int sat, int hue) { int i; /* * Apply the current saturation setting first. */ for (i = 0; i CMATRIX_LEN; i++) - matrix[i] = (info-fmt-cmatrix[i]*info-sat) 7; + matrix[i] = (info-fmt-cmatrix[i] * sat) 7; /* * Then, if need be, rotate the hue value. */ - if (info-hue != 0) { + if (hue != 0) { int sinth, costh, tmpmatrix[CMATRIX_LEN]; memcpy(tmpmatrix, matrix, CMATRIX_LEN*sizeof(int)); - sinth = ov7670_sine(info-hue); - costh = ov7670_cosine(info-hue); + sinth = ov7670_sine(hue); + costh = ov7670_cosine(hue); matrix[0] = (matrix[3]*sinth + matrix[0]*costh)/1000; matrix[1] = (matrix[4]*sinth + matrix[1]*costh)/1000; @@ -1224,60 +1244,21 @@ static void ov7670_calc_cmatrix(struct ov7670_info *info, -static int ov7670_s_sat(struct v4l2_subdev *sd, int value) +static int ov7670_s_sat_hue(struct v4l2_subdev *sd, int sat, int hue) { struct ov7670_info *info = to_state(sd); int matrix[CMATRIX_LEN]; int ret; - info-sat = value; - ov7670_calc_cmatrix(info, matrix); + ov7670_calc_cmatrix(info, matrix, sat, hue); ret = ov7670_store_cmatrix(sd, matrix); return ret; } -static int ov7670_g_sat(struct v4l2_subdev *sd, __s32 *value) -{ - struct ov7670_info *info = to_state(sd); - - *value = info-sat; - return 0; -} - -static int ov7670_s_hue(struct v4l2_subdev *sd, int value) -{ - struct ov7670_info *info = to_state(sd); - int matrix[CMATRIX_LEN]; - int ret; - - if (value -180 || value 180) - return -EINVAL; - info-hue = value; - ov7670_calc_cmatrix(info, matrix); - ret = ov7670_store_cmatrix(sd, matrix); - return ret; -} - - -static int ov7670_g_hue(struct v4l2_subdev *sd, __s32 *value) -{ - struct ov7670_info *info = to_state(sd); - - *value = info-hue; - return 0; -} - /* * Some weird registers seem to store values in a sign/magnitude format! */ -static unsigned char ov7670_sm_to_abs(unsigned char v) -{ - if ((v 0x80) == 0) - return v + 128; - return 128 - (v 0x7f); -} - static unsigned char ov7670_abs_to_sm(unsigned char v) { @@ -1299,40 +1280,11 @@ static int ov7670_s_brightness(struct v4l2_subdev *sd, int value) return ret; } -static int ov7670_g_brightness(struct v4l2_subdev *sd, __s32 *value) -{ - unsigned char v = 0; - int ret = ov7670_read(sd, REG_BRIGHT, v); - - *value = ov7670_sm_to_abs(v); - return ret; -} - static int
[PATCH v2 2/4] mcam-core: implement the control framework.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 54 --- drivers/media/platform/marvell-ccic/mcam-core.h |2 + 2 files changed, 11 insertions(+), 45 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index ce2b7b4..568f8a5 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -22,6 +22,7 @@ #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-ioctl.h +#include media/v4l2-ctrls.h #include media/v4l2-chip-ident.h #include media/ov7670.h #include media/videobuf2-vmalloc.h @@ -1232,47 +1233,6 @@ static int mcam_vidioc_dqbuf(struct file *filp, void *priv, return ret; } - - -static int mcam_vidioc_queryctrl(struct file *filp, void *priv, - struct v4l2_queryctrl *qc) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, queryctrl, qc); - mutex_unlock(cam-s_mutex); - return ret; -} - - -static int mcam_vidioc_g_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, g_ctrl, ctrl); - mutex_unlock(cam-s_mutex); - return ret; -} - - -static int mcam_vidioc_s_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, s_ctrl, ctrl); - mutex_unlock(cam-s_mutex); - return ret; -} - - static int mcam_vidioc_querycap(struct file *file, void *priv, struct v4l2_capability *cap) { @@ -1520,9 +1480,6 @@ static const struct v4l2_ioctl_ops mcam_v4l_ioctl_ops = { .vidioc_dqbuf = mcam_vidioc_dqbuf, .vidioc_streamon= mcam_vidioc_streamon, .vidioc_streamoff = mcam_vidioc_streamoff, - .vidioc_queryctrl = mcam_vidioc_queryctrl, - .vidioc_g_ctrl = mcam_vidioc_g_ctrl, - .vidioc_s_ctrl = mcam_vidioc_s_ctrl, .vidioc_g_parm = mcam_vidioc_g_parm, .vidioc_s_parm = mcam_vidioc_s_parm, .vidioc_enum_framesizes = mcam_vidioc_enum_framesizes, @@ -1786,14 +1743,19 @@ int mccic_register(struct mcam_camera *cam) /* * Get the v4l2 setup done. */ + ret = v4l2_ctrl_handler_init(cam-ctrl_handler, 10); + if (ret) + goto out_unregister; + cam-v4l2_dev.ctrl_handler = cam-ctrl_handler; + mutex_lock(cam-s_mutex); cam-vdev = mcam_v4l_template; cam-vdev.debug = 0; cam-vdev.v4l2_dev = cam-v4l2_dev; + video_set_drvdata(cam-vdev, cam); ret = video_register_device(cam-vdev, VFL_TYPE_GRABBER, -1); if (ret) goto out; - video_set_drvdata(cam-vdev, cam); /* * If so requested, try to get our DMA buffers now. @@ -1805,6 +1767,7 @@ int mccic_register(struct mcam_camera *cam) } out: + v4l2_ctrl_handler_free(cam-ctrl_handler); mutex_unlock(cam-s_mutex); return ret; out_unregister: @@ -1829,6 +1792,7 @@ void mccic_shutdown(struct mcam_camera *cam) if (cam-buffer_mode == B_vmalloc) mcam_free_dma_bufs(cam); video_unregister_device(cam-vdev); + v4l2_ctrl_handler_free(cam-ctrl_handler); v4l2_device_unregister(cam-v4l2_dev); } diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index bd6acba..f7c34ab 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -8,6 +8,7 @@ #include linux/list.h #include media/v4l2-common.h +#include media/v4l2-ctrls.h #include media/v4l2-dev.h #include media/videobuf2-core.h @@ -104,6 +105,7 @@ struct mcam_camera { * should not be touched by the platform code. */ struct v4l2_device v4l2_dev; + struct v4l2_ctrl_handler ctrl_handler; enum mcam_state state; unsigned long flags;/* Buffer status, mainly (dev_lock) */ int users; /* How many open FDs */ -- 1.7.9.5 -- 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 v2 3/4] via-camera: implement the control framework.
And added a missing kfree to clean up the via_camera struct. Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/platform/via-camera.c | 60 --- 1 file changed, 14 insertions(+), 46 deletions(-) diff --git a/drivers/media/platform/via-camera.c b/drivers/media/platform/via-camera.c index eb404c2..1b5f97d 100644 --- a/drivers/media/platform/via-camera.c +++ b/drivers/media/platform/via-camera.c @@ -18,6 +18,7 @@ #include media/v4l2-device.h #include media/v4l2-ioctl.h #include media/v4l2-chip-ident.h +#include media/v4l2-ctrls.h #include media/ov7670.h #include media/videobuf-dma-sg.h #include linux/delay.h @@ -63,6 +64,7 @@ enum viacam_opstate { S_IDLE = 0, S_RUNNING = 1 }; struct via_camera { struct v4l2_device v4l2_dev; + struct v4l2_ctrl_handler ctrl_handler; struct video_device vdev; struct v4l2_subdev *sensor; struct platform_device *platdev; @@ -818,47 +820,6 @@ static int viacam_g_chip_ident(struct file *file, void *priv, } /* - * Control ops are passed through to the sensor. - */ -static int viacam_queryctrl(struct file *filp, void *priv, - struct v4l2_queryctrl *qc) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, queryctrl, qc); - mutex_unlock(cam-lock); - return ret; -} - - -static int viacam_g_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, g_ctrl, ctrl); - mutex_unlock(cam-lock); - return ret; -} - - -static int viacam_s_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, s_ctrl, ctrl); - mutex_unlock(cam-lock); - return ret; -} - -/* * Only one input. */ static int viacam_enum_input(struct file *filp, void *priv, @@ -1214,9 +1175,6 @@ static int viacam_enum_frameintervals(struct file *filp, void *priv, static const struct v4l2_ioctl_ops viacam_ioctl_ops = { .vidioc_g_chip_ident= viacam_g_chip_ident, - .vidioc_queryctrl = viacam_queryctrl, - .vidioc_g_ctrl = viacam_g_ctrl, - .vidioc_s_ctrl = viacam_s_ctrl, .vidioc_enum_input = viacam_enum_input, .vidioc_g_input = viacam_g_input, .vidioc_s_input = viacam_s_input, @@ -1418,8 +1376,12 @@ static __devinit int viacam_probe(struct platform_device *pdev) ret = v4l2_device_register(pdev-dev, cam-v4l2_dev); if (ret) { dev_err(pdev-dev, Unable to register v4l2 device\n); - return ret; + goto out_free; } + ret = v4l2_ctrl_handler_init(cam-ctrl_handler, 10); + if (ret) + goto out_unregister; + cam-v4l2_dev.ctrl_handler = cam-ctrl_handler; /* * Convince the system that we can do DMA. */ @@ -1436,7 +1398,7 @@ static __devinit int viacam_probe(struct platform_device *pdev) */ ret = via_sensor_power_setup(cam); if (ret) - goto out_unregister; + goto out_ctrl_hdl_free; via_sensor_power_up(cam); /* @@ -1485,8 +1447,12 @@ out_irq: free_irq(viadev-pdev-irq, cam); out_power_down: via_sensor_power_release(cam); +out_ctrl_hdl_free: + v4l2_ctrl_handler_free(cam-ctrl_handler); out_unregister: v4l2_device_unregister(cam-v4l2_dev); +out_free: + kfree(cam); return ret; } @@ -1499,6 +1465,8 @@ static __devexit int viacam_remove(struct platform_device *pdev) v4l2_device_unregister(cam-v4l2_dev); free_irq(viadev-pdev-irq, cam); via_sensor_power_release(cam); + v4l2_ctrl_handler_free(cam-ctrl_handler); + kfree(cam); via_cam_info = NULL; return 0; } -- 1.7.9.5 -- 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 v2 4/4] ov7670: remove legacy ctrl callbacks.
via-camera and mcam-core were the only bridge drivers that used ov7670. Since now they have been moved to use the ctrl framework, the old legacy callbacks in the ov7670 can be removed. Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c |7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index babd55c..113a4f3 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -1504,13 +1504,6 @@ static int ov7670_s_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *r static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ext_ctrls = v4l2_subdev_g_ext_ctrls, - .try_ext_ctrls = v4l2_subdev_try_ext_ctrls, - .s_ext_ctrls = v4l2_subdev_s_ext_ctrls, - .g_ctrl = v4l2_subdev_g_ctrl, - .s_ctrl = v4l2_subdev_s_ctrl, - .queryctrl = v4l2_subdev_queryctrl, - .querymenu = v4l2_subdev_querymenu, .reset = ov7670_reset, .init = ov7670_init, #ifdef CONFIG_VIDEO_ADV_DEBUG -- 1.7.9.5 -- 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 1/4] ov7670: use the control framework
Hi Hans, On 28 September 2012 13:05, Hans Verkuil hverk...@xs4all.nl wrote: On Fri September 28 2012 12:50:55 Javier Martin wrote: Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- Changes since v1: - Use v4l2_ctrl_auto_cluster() for auto_gain and auto_exp. --- drivers/media/i2c/ov7670.c | 310 1 file changed, 112 insertions(+), 198 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 9d7a92e..babd55c 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -18,6 +18,7 @@ #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-chip-ident.h +#include media/v4l2-ctrls.h #include media/v4l2-mediabus.h #include media/ov7670.h @@ -222,9 +223,23 @@ struct ov7670_devtype { struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; + struct { + /* gain cluster */ + struct v4l2_ctrl *auto_gain; + struct v4l2_ctrl *gain; + }; + struct { + /* exposure cluster */ + struct v4l2_ctrl *auto_exposure; + struct v4l2_ctrl *exposure; + }; + struct { + /* saturation/hue cluster */ + struct v4l2_ctrl *saturation; + struct v4l2_ctrl *hue; + }; struct ov7670_format_struct *fmt; /* Current format */ - unsigned char sat; /* Saturation value */ - int hue;/* Hue value */ int min_width; /* Filter out smaller sizes */ int min_height; /* Filter out smaller sizes */ int clock_speed;/* External clock speed (MHz) */ @@ -240,6 +255,11 @@ static inline struct ov7670_info *to_state(struct v4l2_subdev *sd) return container_of(sd, struct ov7670_info, sd); } +static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl) +{ + return container_of(ctrl-handler, struct ov7670_info, hdl)-sd; +} + /* @@ -1195,23 +1215,23 @@ static int ov7670_cosine(int theta) static void ov7670_calc_cmatrix(struct ov7670_info *info, - int matrix[CMATRIX_LEN]) + int matrix[CMATRIX_LEN], int sat, int hue) { int i; /* * Apply the current saturation setting first. */ for (i = 0; i CMATRIX_LEN; i++) - matrix[i] = (info-fmt-cmatrix[i]*info-sat) 7; + matrix[i] = (info-fmt-cmatrix[i] * sat) 7; /* * Then, if need be, rotate the hue value. */ - if (info-hue != 0) { + if (hue != 0) { int sinth, costh, tmpmatrix[CMATRIX_LEN]; memcpy(tmpmatrix, matrix, CMATRIX_LEN*sizeof(int)); - sinth = ov7670_sine(info-hue); - costh = ov7670_cosine(info-hue); + sinth = ov7670_sine(hue); + costh = ov7670_cosine(hue); matrix[0] = (matrix[3]*sinth + matrix[0]*costh)/1000; matrix[1] = (matrix[4]*sinth + matrix[1]*costh)/1000; @@ -1224,60 +1244,21 @@ static void ov7670_calc_cmatrix(struct ov7670_info *info, -static int ov7670_s_sat(struct v4l2_subdev *sd, int value) +static int ov7670_s_sat_hue(struct v4l2_subdev *sd, int sat, int hue) { struct ov7670_info *info = to_state(sd); int matrix[CMATRIX_LEN]; int ret; - info-sat = value; - ov7670_calc_cmatrix(info, matrix); + ov7670_calc_cmatrix(info, matrix, sat, hue); ret = ov7670_store_cmatrix(sd, matrix); return ret; } -static int ov7670_g_sat(struct v4l2_subdev *sd, __s32 *value) -{ - struct ov7670_info *info = to_state(sd); - - *value = info-sat; - return 0; -} - -static int ov7670_s_hue(struct v4l2_subdev *sd, int value) -{ - struct ov7670_info *info = to_state(sd); - int matrix[CMATRIX_LEN]; - int ret; - - if (value -180 || value 180) - return -EINVAL; - info-hue = value; - ov7670_calc_cmatrix(info, matrix); - ret = ov7670_store_cmatrix(sd, matrix); - return ret; -} - - -static int ov7670_g_hue(struct v4l2_subdev *sd, __s32 *value) -{ - struct ov7670_info *info = to_state(sd); - - *value = info-hue; - return 0; -} - /* * Some weird registers seem to store values in a sign/magnitude format! */ -static unsigned char ov7670_sm_to_abs(unsigned char v) -{ - if ((v 0x80) == 0) - return v + 128; - return 128 - (v 0x7f); -} - static unsigned char ov7670_abs_to_sm(unsigned char v) { @@ -1299,40 +1280,11 @@ static int ov7670_s_brightness(struct v4l2_subdev *sd, int value) return ret; } -static int ov7670_g_brightness(struct v4l2_subdev *sd, __s32 *value) -{ - unsigned char v = 0; - int ret
[PATCH v3 0/4] ov7670: migrate this sensor and its users to ctrl framework.
The following series migrate ov7670 sensor and current users to ctrl framework as discussed in [1]. This has been tested against mx2_camera soc-camera bridge, so tests or acks will be required from people using cam-core and via-camera out there. This will have to be applied on top of my previous series: [PATCH v2 0/5] media: ov7670: driver cleanup and support for ov7674. All submitted patches have been obtained from Hans' tree and slightly modified to apply on current media_tree for-v3.7 branch: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/cafe-ctrl Changes since v2: - See changelog in patch 1. [PATCH v3 1/4] ov7670: use the control framework [PATCH v3 2/4] mcam-core: implement the control framework. [PATCH v3 3/4] via-camera: implement the control framework. [PATCH v3 4/4] ov7670: remove legacy ctrl callbacks. [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg51778.html -- 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 1/4] ov7670: use the control framework
Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- Changes since v2: - Do not use 'cur.val' to get gain value. --- drivers/media/i2c/ov7670.c | 310 1 file changed, 112 insertions(+), 198 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 9d7a92e..2f6470a 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -18,6 +18,7 @@ #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-chip-ident.h +#include media/v4l2-ctrls.h #include media/v4l2-mediabus.h #include media/ov7670.h @@ -222,9 +223,23 @@ struct ov7670_devtype { struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; + struct { + /* gain cluster */ + struct v4l2_ctrl *auto_gain; + struct v4l2_ctrl *gain; + }; + struct { + /* exposure cluster */ + struct v4l2_ctrl *auto_exposure; + struct v4l2_ctrl *exposure; + }; + struct { + /* saturation/hue cluster */ + struct v4l2_ctrl *saturation; + struct v4l2_ctrl *hue; + }; struct ov7670_format_struct *fmt; /* Current format */ - unsigned char sat; /* Saturation value */ - int hue;/* Hue value */ int min_width; /* Filter out smaller sizes */ int min_height; /* Filter out smaller sizes */ int clock_speed;/* External clock speed (MHz) */ @@ -240,6 +255,11 @@ static inline struct ov7670_info *to_state(struct v4l2_subdev *sd) return container_of(sd, struct ov7670_info, sd); } +static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl) +{ + return container_of(ctrl-handler, struct ov7670_info, hdl)-sd; +} + /* @@ -1195,23 +1215,23 @@ static int ov7670_cosine(int theta) static void ov7670_calc_cmatrix(struct ov7670_info *info, - int matrix[CMATRIX_LEN]) + int matrix[CMATRIX_LEN], int sat, int hue) { int i; /* * Apply the current saturation setting first. */ for (i = 0; i CMATRIX_LEN; i++) - matrix[i] = (info-fmt-cmatrix[i]*info-sat) 7; + matrix[i] = (info-fmt-cmatrix[i] * sat) 7; /* * Then, if need be, rotate the hue value. */ - if (info-hue != 0) { + if (hue != 0) { int sinth, costh, tmpmatrix[CMATRIX_LEN]; memcpy(tmpmatrix, matrix, CMATRIX_LEN*sizeof(int)); - sinth = ov7670_sine(info-hue); - costh = ov7670_cosine(info-hue); + sinth = ov7670_sine(hue); + costh = ov7670_cosine(hue); matrix[0] = (matrix[3]*sinth + matrix[0]*costh)/1000; matrix[1] = (matrix[4]*sinth + matrix[1]*costh)/1000; @@ -1224,60 +1244,21 @@ static void ov7670_calc_cmatrix(struct ov7670_info *info, -static int ov7670_s_sat(struct v4l2_subdev *sd, int value) +static int ov7670_s_sat_hue(struct v4l2_subdev *sd, int sat, int hue) { struct ov7670_info *info = to_state(sd); int matrix[CMATRIX_LEN]; int ret; - info-sat = value; - ov7670_calc_cmatrix(info, matrix); + ov7670_calc_cmatrix(info, matrix, sat, hue); ret = ov7670_store_cmatrix(sd, matrix); return ret; } -static int ov7670_g_sat(struct v4l2_subdev *sd, __s32 *value) -{ - struct ov7670_info *info = to_state(sd); - - *value = info-sat; - return 0; -} - -static int ov7670_s_hue(struct v4l2_subdev *sd, int value) -{ - struct ov7670_info *info = to_state(sd); - int matrix[CMATRIX_LEN]; - int ret; - - if (value -180 || value 180) - return -EINVAL; - info-hue = value; - ov7670_calc_cmatrix(info, matrix); - ret = ov7670_store_cmatrix(sd, matrix); - return ret; -} - - -static int ov7670_g_hue(struct v4l2_subdev *sd, __s32 *value) -{ - struct ov7670_info *info = to_state(sd); - - *value = info-hue; - return 0; -} - /* * Some weird registers seem to store values in a sign/magnitude format! */ -static unsigned char ov7670_sm_to_abs(unsigned char v) -{ - if ((v 0x80) == 0) - return v + 128; - return 128 - (v 0x7f); -} - static unsigned char ov7670_abs_to_sm(unsigned char v) { @@ -1299,40 +1280,11 @@ static int ov7670_s_brightness(struct v4l2_subdev *sd, int value) return ret; } -static int ov7670_g_brightness(struct v4l2_subdev *sd, __s32 *value) -{ - unsigned char v = 0; - int ret = ov7670_read(sd, REG_BRIGHT, v); - - *value = ov7670_sm_to_abs(v); - return ret; -} - static int ov7670_s_contrast(struct
[PATCH v3 4/4] ov7670: remove legacy ctrl callbacks.
via-camera and mcam-core were the only bridge drivers that used ov7670. Since now they have been moved to use the ctrl framework, the old legacy callbacks in the ov7670 can be removed. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c |7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 2f6470a..361b101 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -1504,13 +1504,6 @@ static int ov7670_s_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *r static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ext_ctrls = v4l2_subdev_g_ext_ctrls, - .try_ext_ctrls = v4l2_subdev_try_ext_ctrls, - .s_ext_ctrls = v4l2_subdev_s_ext_ctrls, - .g_ctrl = v4l2_subdev_g_ctrl, - .s_ctrl = v4l2_subdev_s_ctrl, - .queryctrl = v4l2_subdev_queryctrl, - .querymenu = v4l2_subdev_querymenu, .reset = ov7670_reset, .init = ov7670_init, #ifdef CONFIG_VIDEO_ADV_DEBUG -- 1.7.9.5 -- 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 3/4] via-camera: implement the control framework.
And added a missing kfree to clean up the via_camera struct. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/platform/via-camera.c | 60 --- 1 file changed, 14 insertions(+), 46 deletions(-) diff --git a/drivers/media/platform/via-camera.c b/drivers/media/platform/via-camera.c index eb404c2..1b5f97d 100644 --- a/drivers/media/platform/via-camera.c +++ b/drivers/media/platform/via-camera.c @@ -18,6 +18,7 @@ #include media/v4l2-device.h #include media/v4l2-ioctl.h #include media/v4l2-chip-ident.h +#include media/v4l2-ctrls.h #include media/ov7670.h #include media/videobuf-dma-sg.h #include linux/delay.h @@ -63,6 +64,7 @@ enum viacam_opstate { S_IDLE = 0, S_RUNNING = 1 }; struct via_camera { struct v4l2_device v4l2_dev; + struct v4l2_ctrl_handler ctrl_handler; struct video_device vdev; struct v4l2_subdev *sensor; struct platform_device *platdev; @@ -818,47 +820,6 @@ static int viacam_g_chip_ident(struct file *file, void *priv, } /* - * Control ops are passed through to the sensor. - */ -static int viacam_queryctrl(struct file *filp, void *priv, - struct v4l2_queryctrl *qc) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, queryctrl, qc); - mutex_unlock(cam-lock); - return ret; -} - - -static int viacam_g_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, g_ctrl, ctrl); - mutex_unlock(cam-lock); - return ret; -} - - -static int viacam_s_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct via_camera *cam = priv; - int ret; - - mutex_lock(cam-lock); - ret = sensor_call(cam, core, s_ctrl, ctrl); - mutex_unlock(cam-lock); - return ret; -} - -/* * Only one input. */ static int viacam_enum_input(struct file *filp, void *priv, @@ -1214,9 +1175,6 @@ static int viacam_enum_frameintervals(struct file *filp, void *priv, static const struct v4l2_ioctl_ops viacam_ioctl_ops = { .vidioc_g_chip_ident= viacam_g_chip_ident, - .vidioc_queryctrl = viacam_queryctrl, - .vidioc_g_ctrl = viacam_g_ctrl, - .vidioc_s_ctrl = viacam_s_ctrl, .vidioc_enum_input = viacam_enum_input, .vidioc_g_input = viacam_g_input, .vidioc_s_input = viacam_s_input, @@ -1418,8 +1376,12 @@ static __devinit int viacam_probe(struct platform_device *pdev) ret = v4l2_device_register(pdev-dev, cam-v4l2_dev); if (ret) { dev_err(pdev-dev, Unable to register v4l2 device\n); - return ret; + goto out_free; } + ret = v4l2_ctrl_handler_init(cam-ctrl_handler, 10); + if (ret) + goto out_unregister; + cam-v4l2_dev.ctrl_handler = cam-ctrl_handler; /* * Convince the system that we can do DMA. */ @@ -1436,7 +1398,7 @@ static __devinit int viacam_probe(struct platform_device *pdev) */ ret = via_sensor_power_setup(cam); if (ret) - goto out_unregister; + goto out_ctrl_hdl_free; via_sensor_power_up(cam); /* @@ -1485,8 +1447,12 @@ out_irq: free_irq(viadev-pdev-irq, cam); out_power_down: via_sensor_power_release(cam); +out_ctrl_hdl_free: + v4l2_ctrl_handler_free(cam-ctrl_handler); out_unregister: v4l2_device_unregister(cam-v4l2_dev); +out_free: + kfree(cam); return ret; } @@ -1499,6 +1465,8 @@ static __devexit int viacam_remove(struct platform_device *pdev) v4l2_device_unregister(cam-v4l2_dev); free_irq(viadev-pdev-irq, cam); via_sensor_power_release(cam); + v4l2_ctrl_handler_free(cam-ctrl_handler); + kfree(cam); via_cam_info = NULL; return 0; } -- 1.7.9.5 -- 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/4] mcam-core: implement the control framework.
Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 54 --- drivers/media/platform/marvell-ccic/mcam-core.h |2 + 2 files changed, 11 insertions(+), 45 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index ce2b7b4..568f8a5 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -22,6 +22,7 @@ #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-ioctl.h +#include media/v4l2-ctrls.h #include media/v4l2-chip-ident.h #include media/ov7670.h #include media/videobuf2-vmalloc.h @@ -1232,47 +1233,6 @@ static int mcam_vidioc_dqbuf(struct file *filp, void *priv, return ret; } - - -static int mcam_vidioc_queryctrl(struct file *filp, void *priv, - struct v4l2_queryctrl *qc) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, queryctrl, qc); - mutex_unlock(cam-s_mutex); - return ret; -} - - -static int mcam_vidioc_g_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, g_ctrl, ctrl); - mutex_unlock(cam-s_mutex); - return ret; -} - - -static int mcam_vidioc_s_ctrl(struct file *filp, void *priv, - struct v4l2_control *ctrl) -{ - struct mcam_camera *cam = priv; - int ret; - - mutex_lock(cam-s_mutex); - ret = sensor_call(cam, core, s_ctrl, ctrl); - mutex_unlock(cam-s_mutex); - return ret; -} - - static int mcam_vidioc_querycap(struct file *file, void *priv, struct v4l2_capability *cap) { @@ -1520,9 +1480,6 @@ static const struct v4l2_ioctl_ops mcam_v4l_ioctl_ops = { .vidioc_dqbuf = mcam_vidioc_dqbuf, .vidioc_streamon= mcam_vidioc_streamon, .vidioc_streamoff = mcam_vidioc_streamoff, - .vidioc_queryctrl = mcam_vidioc_queryctrl, - .vidioc_g_ctrl = mcam_vidioc_g_ctrl, - .vidioc_s_ctrl = mcam_vidioc_s_ctrl, .vidioc_g_parm = mcam_vidioc_g_parm, .vidioc_s_parm = mcam_vidioc_s_parm, .vidioc_enum_framesizes = mcam_vidioc_enum_framesizes, @@ -1786,14 +1743,19 @@ int mccic_register(struct mcam_camera *cam) /* * Get the v4l2 setup done. */ + ret = v4l2_ctrl_handler_init(cam-ctrl_handler, 10); + if (ret) + goto out_unregister; + cam-v4l2_dev.ctrl_handler = cam-ctrl_handler; + mutex_lock(cam-s_mutex); cam-vdev = mcam_v4l_template; cam-vdev.debug = 0; cam-vdev.v4l2_dev = cam-v4l2_dev; + video_set_drvdata(cam-vdev, cam); ret = video_register_device(cam-vdev, VFL_TYPE_GRABBER, -1); if (ret) goto out; - video_set_drvdata(cam-vdev, cam); /* * If so requested, try to get our DMA buffers now. @@ -1805,6 +1767,7 @@ int mccic_register(struct mcam_camera *cam) } out: + v4l2_ctrl_handler_free(cam-ctrl_handler); mutex_unlock(cam-s_mutex); return ret; out_unregister: @@ -1829,6 +1792,7 @@ void mccic_shutdown(struct mcam_camera *cam) if (cam-buffer_mode == B_vmalloc) mcam_free_dma_bufs(cam); video_unregister_device(cam-vdev); + v4l2_ctrl_handler_free(cam-ctrl_handler); v4l2_device_unregister(cam-v4l2_dev); } diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index bd6acba..f7c34ab 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -8,6 +8,7 @@ #include linux/list.h #include media/v4l2-common.h +#include media/v4l2-ctrls.h #include media/v4l2-dev.h #include media/videobuf2-core.h @@ -104,6 +105,7 @@ struct mcam_camera { * should not be touched by the platform code. */ struct v4l2_device v4l2_dev; + struct v4l2_ctrl_handler ctrl_handler; enum mcam_state state; unsigned long flags;/* Buffer status, mainly (dev_lock) */ int users; /* How many open FDs */ -- 1.7.9.5 -- 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 1/5] media: ov7670: add support for ov7675.
On 6 October 2012 17:19, Mauro Carvalho Chehab mche...@infradead.org wrote: Em Thu, 27 Sep 2012 08:58:33 +0200 javier Martin javier.mar...@vista-silicon.com escreveu: Hi Jonathan, thank you for your time. On 26 September 2012 18:40, Jonathan Corbet cor...@lwn.net wrote: This is going to have to be quick, sorry... On Wed, 26 Sep 2012 11:47:53 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: +static struct ov7670_win_size ov7670_win_sizes[2][4] = { + /* ov7670 */ I must confess I don't like this; now we've got constants in an array that was automatically sized before and ov7670_win_sizes[info-model] everywhere. I'd suggest a separate array for each device and an ov7670_get_wsizes(model) function. + /* CIF - WARNING: not tested for ov7675 */ + { ...and this is part of why I don't like it. My experience with this particular sensor says that, if it's not tested, it hasn't yet seen the magic-number tweaking required to actually make it work. Please don't claim to support formats that you don't know actually work, or I'll get stuck with the bug reports :) Your concern makes a lot of sense. In fact, that was one of my doubts whether to 'support' not tested formats or not. Let me fix that in a new version. Hi Javier, I'm assuming that you'll be sending a new version of this entire changeset. So, I'll just mark this entire series as changes_requested. Hi Mauro, v2 of this changeset has already been sent with Jon Corbet's ack: http://www.mail-archive.com/linux-media@vger.kernel.org/msg52767.html https://patchwork.kernel.org/patch/1515001/ https://patchwork.kernel.org/patch/1515021/ https://patchwork.kernel.org/patch/1515011/ https://patchwork.kernel.org/patch/1515031/ https://patchwork.kernel.org/patch/1515041/ Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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]: mx2_camera: Fix regression caused by clock conversion
On 8 October 2012 11:09, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Fabio On Fri, 5 Oct 2012, Fabio Estevam wrote: Since mx27 transitioned to the commmon clock framework in 3.5, the correct way to acquire the csi clock is to get csi_ahb and csi_per clocks separately. By not doing so the camera sensor does not probe correctly: soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 mx2-camera mx2-camera.0: Camera driver attached to camera 0 ov2640 0-0030: Product ID error fb:fb mx2-camera mx2-camera.0: Camera driver detached from camera 0 mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 6650 Adapt the mx2_camera driver to the new clock framework and make it functional again. Do I understand it right, that since the driver is currently broken, it doesn't matter any more in which order these two patches get applied, so, we can push them via different trees - ARM and media? Thanks Guennadi Please, hold on a couple of days before merging this one. This driver is currently working in our Visstrim M10 platform without this patch and I need to test it to confirm whether it breaks something or not. Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 2/2] [media]: mx2_camera: Fix regression caused by clock conversion
*pdev) clk_disable_unprepare(pcdev-clk_emma_ahb); } + clk_disable_unprepare(pcdev-clk_csi_per); + clk_disable_unprepare(pcdev-clk_csi_ahb); + dev_info(pdev-dev, MX2 Camera driver unloaded\n); return 0; -- 1.7.9.5 This patch doesn't fix the BUG it claims to, since I have it working properly in our Visstrim M10 platform without it. Look: soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 mx2-camera mx2-camera.0: Camera driver attached to camera 0 ov7670 0-0021: chip found @ 0x42 (imx-i2c) [..] mx2-camera mx2-camera.0: Camera driver detached from camera 0 mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 6650 Furthermore, it's not correct, since there isn't such per clock for the CSI in 3.5 [1], 3.6 [2], linux-next-20121008[3], or next-20121009[4]. [1] http://lxr.linux.no/#linux+v3.5/arch/arm/mach-imx/clk-imx27.c#L226 [2] http://lxr.linux.no/#linux+v3.6/arch/arm/mach-imx/clk-imx27.c#L226 [3] http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/mach-imx/clk-imx27.c;h=3b6b640eed247ea1b7848c7a7fa01801f0190cde;hb=cc925138a0dd9ae388135bb3cf11ee1729f9c4e9#l226 [4] http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/mach-imx/clk-imx27.c;h=3b6b640eed247ea1b7848c7a7fa01801f0190cde;hb=b066f61482c7eac44e656499426a3c56d29c32ed#l226 Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 2/2] [media]: mx2_camera: Fix regression caused by clock conversion
__devexit mx2_camera_remove(struct platform_device *pdev) clk_disable_unprepare(pcdev-clk_emma_ahb); } + clk_disable_unprepare(pcdev-clk_csi_per); + clk_disable_unprepare(pcdev-clk_csi_ahb); + dev_info(pdev-dev, MX2 Camera driver unloaded\n); return 0; I only test detection at kernel boot not streaming using Gstreamer due to lack of time. On imx27_3ds board with ov2640 sensor: ov2640 0-0030: ov2640 Product ID 26:42 Manufacturer ID 7f:a2 mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 4433 On clone imx27_3ds board with mt9v111 sensor (draft driver): mt9v111 0-0048: Detected a MT9V111 chip ID 823a mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 4433 Tested-by: Gaƫtan Carlier gcem...@gmail.com Sorry I missed patch 1/2. Both patches are correct: Tested-by: Javier Martin javier.mar...@vista-silicon.com -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 1/2] ARM: clk-imx27: Add missing clock for mx2-camera
On 5 October 2012 23:53, Fabio Estevam fabio.este...@freescale.com wrote: During the clock conversion for mx27 the per4_gate clock was missed to get registered as a dependency of mx2-camera driver. In the old mx27 clock driver we used to have: DEFINE_CLOCK1(csi_clk, 0, NULL, 0, parent, csi_clk1, per4_clk); ,so does the same in the new clock driver. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- arch/arm/mach-imx/clk-imx27.c |1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c index 3b6b640..5ef0f08 100644 --- a/arch/arm/mach-imx/clk-imx27.c +++ b/arch/arm/mach-imx/clk-imx27.c @@ -224,6 +224,7 @@ int __init mx27_clocks_init(unsigned long fref) clk_register_clkdev(clk[lcdc_ipg_gate], ipg, imx-fb.0); clk_register_clkdev(clk[lcdc_ahb_gate], ahb, imx-fb.0); clk_register_clkdev(clk[csi_ahb_gate], ahb, mx2-camera.0); + clk_register_clkdev(clk[per4_gate], per, mx2-camera.0); clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); clk_register_clkdev(clk[usb_ipg_gate], ipg, fsl-usb2-udc); clk_register_clkdev(clk[usb_ahb_gate], ahb, fsl-usb2-udc); -- 1.7.9.5 Tested-by: Javier Martin javier.mar...@vista-silicon.com -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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] [media] coda: Do not use __cancel_delayed_work()
On 10 October 2012 05:43, Tejun Heo t...@kernel.org wrote: On Wed, Oct 10, 2012 at 12:33:29AM -0300, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com commit 136b5721d (workqueue: deprecate __cancel_delayed_work()) made __cancel_delayed_work deprecated. Use cancel_delayed_work instead and get rid of the following warning: drivers/media/platform/coda.c:1543: warning: '__cancel_delayed_work' is deprecated (declared at include/linux/workqueue.h:437) Signed-off-by: Fabio Estevam fabio.este...@freescale.com Acked-by: Tejun Heo t...@kernel.org Thanks! -- tejun Thanks Fabio. Acked-by: Javier Martin javier.mar...@vista-silicon.com -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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 2/2] [media]: mx2_camera: Fix regression caused by clock conversion
On 23 October 2012 00:17, Fabio Estevam feste...@gmail.com wrote: Hi Guennadi On Mon, Oct 22, 2012 at 7:07 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: ? I don't find a clock named per and associated with mx2-camera.0 in arch/arm/mach-imx/clk-imx27.c. I only find it in clk-imx25.c. Does this mean, that this your patch is for i.MX25? But you're saying it's for i.MX27. Confused... I provide this mx27 clock in the first patch of the series: http://patchwork.linuxtv.org/patch/14915/ Yes, I made the same mistake. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- 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
i.MX6 video capture support in mainline
Hello, we have an BD-SL-i.MX6 platform (compatible with the Nitrogen6X) where we are currently running the BSP from Freescale, which is based on kernel 3.10 if I recall properly. We are aware that those drivers have some issues, specially when it comes to compliance with the V4L2 frameworks like the media controller API, stability, etc... Furthermore, we need to use the mainline kernel because some of the drivers that we need to use are not available in the Freescale kernel. The biggest problem that we have found so far for switching to the mainline kernel is the video capture support in the I.MX6 IPU. I've been following some old e-mail threads (from 2014) and I eventually found Philipp Zabel's repository branch 'nitrogen6x-ipu-media' which has what seems to be an early version of an i.MX6 IPU capture driver via the CSI interface. We've got here the same setup with an ov5642 sensor connected to the CSI interface and we have been giving a try to the driver. This is what we have tried so far: cat /dev/video0 # This is needed so that open gets called and the csi links are created media-ctl -l 'ov5642 1-003c:0-mipi_ipu1_mux:1[1], /soc/ipu@0240/port@0:1-IPU0 SMFC0:0[1]' media-ctl -l 'IPU0 SMFC0:1-imx-ipuv3-camera.2:0[1]' The last command will fail like this: imx-ipuv3 240.ipu: invalid link 'IPU0 SMFC0'(5):1 - 'imx-ipuv3-camera.2'(2):0 Unable to parse link: Invalid argument (22) The reason it fails, apparently, is that the links that have been created when opening /dev/video0 are not included in the ipu_links[] static table defined in drivers/gpio/ipu-v3/ipu-media.c which is where the ipu_smfc_link_setup() function tries to find a valid link. I've got some questions regarding this driver and iMX6 video capture support in general that someone here may gladly answer: a) Is anyone currently working on mainlining iMX6 video capture support? I know about Steve's and Philipp's work but I haven't seen any progress since September 2014. b) Does anyone know whether it's possible to capture YUV420P video using the driver in Philipp's repository? If so could you please provide the pipeline setup that you used with media-ctl? c) If we were willing to help with mainline submission of this driver what issues should we focus on? Regards, Javier. [1] git://git.pengutronix.de/git/pza/linux.git -- 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: coda: Problems with encoding in i.MX6DL.
Hi Philipp, thanks for your fast answer. Apparently, the firmware is being loaded properly although it complains about that version not being supported. After queuing some YUV420 buffers with a simple application I perform a VIDIOC_STREAMON in both the CAPTURE and the OUTPUT interfaces but I get the following error: coda 204.vpu: coda is not initialized. ... but then suddenly it's not. (coda_is_initialized just checks whether PC != 0) Could this have something to do with the PU power domain? Do all coda registers read 0x0 ? Do you have CONFIG_PM disabled? Check if d438462c20a3 (ARM: imx6: gpc: always enable PU domain if CONFIG_PM is not set) makes a difference. I think that patch hasn't made it into stable yet. Indeed, I was having problems with the runtime PM from the beginning and hacked up the code in the gpmc a bit to make sure the coda was always enabled but somehow I forgot to comment the poweroff callback and the codas was being powered off and never turned on again. Just in case it is useful for someone else these are the functions in arch/arm/mach-imx/gpc.c whose code I completely commented out: _imx6q_pm_pu_power_off imx6q_pm_pu_power_off Anyway, it looks like power management for the coda is a bit broken in the i.MX6D. I'll leave it disabled for now so that I continue with my development but I plan to have a look at it later on to see if I can fix it properly. [ cut here ] WARNING: CPU: 0 PID: 91 at drivers/media/v4l2-core/videobuf2-core.c:1792 vb2_start_streaming+0xe0/0x15c() That is because after copying buffers to the bitstream, the driver currently marks them as done. When start_streaming fails, videobuf2 expects drivers to re-queue them. So we'd have to flush the bitstream and re-queue the buffers so they can be copied to the bitstream all over during the next try. This warning is a result of incomplete error handling in the coda start_streaming implementation. I see, I might look into this if I manage to get some spare time. Regards, Javier. -- 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
coda: Problems with encoding in i.MX6DL.
Hello, I am running kernel 4.1 in a var-dvk-solo-linux evaluation board from Variscite. This is what I get at system start-up: coda 204.vpu: Firmware code revision: 34588 coda 204.vpu: Initialized CODA960. coda 204.vpu: Unsupported firmware version: 2.1.8 coda 204.vpu: codec registered as /dev/video[0-1] Apparently, the firmware is being loaded properly although it complains about that version not being supported. After queuing some YUV420 buffers with a simple application I perform a VIDIOC_STREAMON in both the CAPTURE and the OUTPUT interfaces but I get the following error: coda 204.vpu: coda is not initialized. [ cut here ] WARNING: CPU: 0 PID: 91 at drivers/media/v4l2-core/videobuf2-core.c:1792 vb2_start_streaming+0xe0/0x15c() Modules linked in: CPU: 0 PID: 91 Comm: wmip_bsp_tests Tainted: GW 4.1.0-4-g192a113-dirty #96 Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) Backtrace: [c0012cb4] (dump_backtrace) from [c0012ecc] (show_stack+0x18/0x1c) r6:c0815888 r5: r4:c08d3764 r3: [c0012eb4] (show_stack) from [c061ab8c] (dump_stack+0x8c/0x9c) [c061ab00] (dump_stack) from [c002775c] (warn_slowpath_common+0x88/0xb8) r5:0700 r4: [c00276d4] (warn_slowpath_common) from [c0027830] (warn_slowpath_null+0x24/0x2c) r8:edc35000 r7: r6:ee1ee408 r5:ee1ee4d8 r4:fff2 [c002780c] (warn_slowpath_null) from [c0489734] (vb2_start_streaming+0xe0/0x15c) [c0489654] (vb2_start_streaming) from [c048bc50] (vb2_internal_streamon+0x118/0x164) r7: r6:edc1614c r5:ee1ee400 r4:ee1ee408 [c048bb38] (vb2_internal_streamon) from [c048bcd4] (vb2_streamon+0x38/0x58) r5:ee1ee400 r4:0001 [c048bc9c] (vb2_streamon) from [c0485670] (v4l2_m2m_streamon+0x38/0x54) [c0485638] (v4l2_m2m_streamon) from [c04856a4] (v4l2_m2m_ioctl_streamon+0x18/0x1c) r5:ee82f068 r4:40045612 [c048568c] (v4l2_m2m_ioctl_streamon) from [c0474ea0] (v4l_streamon+0x20/0x24) [c0474e80] (v4l_streamon) from [c0477810] (__video_do_ioctl+0x264/0x2cc) [c04775ac] (__video_do_ioctl) from [c0477294] (video_usercopy+0x190/0x48c) r10:ee1ebe20 r9:0001 r8:be916b74 r7: r6: r5:c04775ac r4:40045612 [c0477104] (video_usercopy) from [c04775a8] (video_ioctl2+0x18/0x1c) r10:eea7dd88 r9:be916b74 r8:ee82fcf0 r7:40045612 r6:be916b74 r5:edc35000 r4:ee82f068 [c0477590] (video_ioctl2) from [c0473a4c] (v4l2_ioctl+0xac/0xc8) [c04739a0] (v4l2_ioctl) from [c00f8334] (do_vfs_ioctl+0x430/0x624) r8:0003 r7:be916b74 r6:0003 r5:edc35000 r4:edc35000 r3:c04739a0 [c00f7f04] (do_vfs_ioctl) from [c00f8564] (SyS_ioctl+0x3c/0x64) r10: r9:ee1ea000 r8:0003 r7:be916b74 r6:40045612 r5:edc35000 r4:edc35000 [c00f8528] (SyS_ioctl) from [c000f720] (ret_fast_syscall+0x0/0x3c) r8:c000f8c4 r7:0036 r6:8aa8 r5: r4: r3:be916b74 ---[ end trace 2b0ba71bfb12fec4 ]--- As anyone seen the same issue? Could be related to the Unsupported firmware version complaint? Do you know where to get the 2.1.5 firmware for the i.MX6D? Regards, Javier. -- 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
imx-drm: Color issues scanning out YUV420 frames through the overlay plane.
Hi, I am using mainline kernel 4.1 and I was writing a small application that uses double buffering to read YUV420 frames from a file at 30fps and displays them using the overlay plane in the imx-drm driver. The first issue I noticed is that the image was green so I had to apply the following patches to make the U and V components be scanned out properly: http://lists.freedesktop.org/archives/dri-devel/2014-October/071052.html http://lists.freedesktop.org/archives/dri-devel/2014-October/071025.html http://lists.freedesktop.org/archives/dri-devel/2014-October/071048.html The thing is that, even after applying the 3 patches above, colors are a bit strange. They seem about right but there are some artifacts, like a saturation effect that spoils the image. You can see some snapshots here to see what I am talking about: https://imageshack.com/i/f0nAM5Xbj https://imageshack.com/i/hl7bZMNjj https://imageshack.com/i/eyRjURxRj And the original video is the first one in this page: http://media.xiph.org/video/derf/ On the other hand, colors in the primary plane using the fbdev interface and RGB look correct. Has anyone seen something similar or is YUV420 working fine for you? Regards, Javier. -- 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: imx-drm: Color issues scanning out YUV420 frames through the overlay plane.
Sorry for sending this to the wrong list. On 07/08/15 09:25, Javier Martin wrote: Hi, I am using mainline kernel 4.1 and I was writing a small application that uses double buffering to read YUV420 frames from a file at 30fps and displays them using the overlay plane in the imx-drm driver. The first issue I noticed is that the image was green so I had to apply the following patches to make the U and V components be scanned out properly: http://lists.freedesktop.org/archives/dri-devel/2014-October/071052.html http://lists.freedesktop.org/archives/dri-devel/2014-October/071025.html http://lists.freedesktop.org/archives/dri-devel/2014-October/071048.html The thing is that, even after applying the 3 patches above, colors are a bit strange. They seem about right but there are some artifacts, like a saturation effect that spoils the image. You can see some snapshots here to see what I am talking about: https://imageshack.com/i/f0nAM5Xbj https://imageshack.com/i/hl7bZMNjj https://imageshack.com/i/eyRjURxRj And the original video is the first one in this page: http://media.xiph.org/video/derf/ On the other hand, colors in the primary plane using the fbdev interface and RGB look correct. Has anyone seen something similar or is YUV420 working fine for you? Regards, Javier. -- 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 -- 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] media: Add a driver for the ov5640 sensor.
The ov5640 sensor from Omnivision supports up to 2592x1944 and both CSI and MIPI interfaces. The following driver adds support for the CSI interface only and VGA, 720p resolutions at 30fps. Signed-off-by: Javier Martin <javiermar...@by.com.es> --- .../devicetree/bindings/media/i2c/ov5640.txt | 47 + arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts | 50 - arch/arm/boot/dts/imx6qdl-var-som.dtsi | 422 ++ drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/ov5640.c | 1403 drivers/media/i2c/ov5642.c | 129 +- drivers/media/v4l2-core/v4l2-ctrls.c | 30 +- 8 files changed, 1702 insertions(+), 391 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt create mode 100644 drivers/media/i2c/ov5640.c diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt new file mode 100644 index 000..2e93e97 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt @@ -0,0 +1,47 @@ +* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor + +The Omnivision OV5640 is a 1/4-Inch CMOS active pixel digital image sensor with +an active array size of 2592H x 1932V. It is programmable through a simple +two-wire serial interface. + +Required Properties: +- compatible: value should be "ovti,ov5640" +- clocks: reference to the xclk clock +- clock-names: should be "xclk" +- clock-rates: the xclk clock frequency + +Optional Properties: +- reset-gpio: Chip reset GPIO +- pwdn-gpio: Chip power down GPIO + +The device node must contain one 'port' child node for its digital output +video port, in accordance with the video interface bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. + +Example: + +{ + ... + + ov5640: ov5640@3c { + compatible = "ovti,ov5640"; + pinctrl-names = "default"; + pinctrl-0 = <_ov5640 _csi0>; + reg = <0x3c>; + + clocks = < 200>; + clock-names = "xclk"; + clock-rates = <2400>; + + reset-gpio = < 20 GPIO_ACTIVE_LOW>; + pwdn-gpio = < 6 GPIO_ACTIVE_HIGH>; + + port { + ov5640_to_csi0: endpoint { + remote-endpoint = <_from_ov5640>; + hsync-active = <1>; + vsync-active = <1>; + }; + }; + }; + }; diff --git a/arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts b/arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts index 5431e90..f92f68c 100644 --- a/arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts +++ b/arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts @@ -23,53 +23,3 @@ }; - { - status = "okay"; - - lvds-channel@0 { - fsl,data-mapping = "spwg"; - fsl,data-width = <24>; - crtc = "ipu1-di0"; - status = "okay"; - - display-timings { - native-mode = <>; - timingr0: hsd100pxn1 { - clock-frequency = <7110>; - hactive = <1280>; - vactive = <800>; - hback-porch = <50>; - hfront-porch = <50>; - vback-porch = <6>; - vfront-porch = <6>; - hsync-len = <60>; - vsync-len = <11>; - }; - }; - }; - - lvds-channel@1 { - fsl,data-mapping = "spwg"; - fsl,data-width = <24>; - crtc = "ipu1-di1"; - primary; - status = "okay"; - - display-timings { - native-mode = <>; - timing1: hsd100pxn1 { - clock-frequency = <7110>; - hactive = <1280>; - vactive = <800>; - hback-porch = <50>; - hfront-porch = <50>; - vback-porch = <6>; - vfront-porch = <6>; - h
Re: [PATCH] media: Add a driver for the ov5640 sensor.
Sorry for the unrelated patches, I will submit this again. On 30/09/15 09:34, Javier Martin wrote: The ov5640 sensor from Omnivision supports up to 2592x1944 and both CSI and MIPI interfaces. The following driver adds support for the CSI interface only and VGA, 720p resolutions at 30fps. Signed-off-by: Javier Martin <javiermar...@by.com.es> --- .../devicetree/bindings/media/i2c/ov5640.txt | 47 + arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts | 50 - arch/arm/boot/dts/imx6qdl-var-som.dtsi | 422 ++ drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/ov5640.c | 1403 drivers/media/i2c/ov5642.c | 129 +- drivers/media/v4l2-core/v4l2-ctrls.c | 30 +- 8 files changed, 1702 insertions(+), 391 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt create mode 100644 drivers/media/i2c/ov5640.c diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt new file mode 100644 index 000..2e93e97 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt @@ -0,0 +1,47 @@ +* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor + +The Omnivision OV5640 is a 1/4-Inch CMOS active pixel digital image sensor with +an active array size of 2592H x 1932V. It is programmable through a simple +two-wire serial interface. + +Required Properties: +- compatible: value should be "ovti,ov5640" +- clocks: reference to the xclk clock +- clock-names: should be "xclk" +- clock-rates: the xclk clock frequency + +Optional Properties: +- reset-gpio: Chip reset GPIO +- pwdn-gpio: Chip power down GPIO + +The device node must contain one 'port' child node for its digital output +video port, in accordance with the video interface bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. + +Example: + +{ + ... + + ov5640: ov5640@3c { + compatible = "ovti,ov5640"; + pinctrl-names = "default"; + pinctrl-0 = <_ov5640 _csi0>; + reg = <0x3c>; + + clocks = < 200>; + clock-names = "xclk"; + clock-rates = <2400>; + + reset-gpio = < 20 GPIO_ACTIVE_LOW>; + pwdn-gpio = < 6 GPIO_ACTIVE_HIGH>; + + port { + ov5640_to_csi0: endpoint { + remote-endpoint = <_from_ov5640>; + hsync-active = <1>; + vsync-active = <1>; + }; + }; + }; + }; diff --git a/arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts b/arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts index 5431e90..f92f68c 100644 --- a/arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts +++ b/arch/arm/boot/dts/imx6dl-var-som-solo-vsc.dts @@ -23,53 +23,3 @@ }; - { - status = "okay"; - - lvds-channel@0 { - fsl,data-mapping = "spwg"; - fsl,data-width = <24>; - crtc = "ipu1-di0"; - status = "okay"; - - display-timings { - native-mode = <>; - timingr0: hsd100pxn1 { - clock-frequency = <7110>; - hactive = <1280>; - vactive = <800>; - hback-porch = <50>; - hfront-porch = <50>; - vback-porch = <6>; - vfront-porch = <6>; - hsync-len = <60>; - vsync-len = <11>; - }; - }; - }; - - lvds-channel@1 { - fsl,data-mapping = "spwg"; - fsl,data-width = <24>; - crtc = "ipu1-di1"; - primary; - status = "okay"; - - display-timings { - native-mode = <>; - timing1: hsd100pxn1 { - clock-frequency = <7110>; - hactive = <1280>; - vactive = <800>; - hback-porch = <50>; - hfront-porch = <50>; - vb
[PATCH v2] media: Add a driver for the ov5640 sensor.
The ov5640 sensor from Omnivision supports up to 2592x1944 and both CSI and MIPI interfaces. The following driver adds support for the CSI interface only and VGA, 720p resolutions at 30fps. Signed-off-by: Javier Martin <javiermar...@by.com.es> --- .../devicetree/bindings/media/i2c/ov5640.txt | 47 + drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/ov5640.c | 1403 4 files changed, 1462 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt create mode 100644 drivers/media/i2c/ov5640.c diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt new file mode 100644 index 000..2e93e97 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt @@ -0,0 +1,47 @@ +* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor + +The Omnivision OV5640 is a 1/4-Inch CMOS active pixel digital image sensor with +an active array size of 2592H x 1932V. It is programmable through a simple +two-wire serial interface. + +Required Properties: +- compatible: value should be "ovti,ov5640" +- clocks: reference to the xclk clock +- clock-names: should be "xclk" +- clock-rates: the xclk clock frequency + +Optional Properties: +- reset-gpio: Chip reset GPIO +- pwdn-gpio: Chip power down GPIO + +The device node must contain one 'port' child node for its digital output +video port, in accordance with the video interface bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. + +Example: + +{ + ... + + ov5640: ov5640@3c { + compatible = "ovti,ov5640"; + pinctrl-names = "default"; + pinctrl-0 = <_ov5640 _csi0>; + reg = <0x3c>; + + clocks = < 200>; + clock-names = "xclk"; + clock-rates = <2400>; + + reset-gpio = < 20 GPIO_ACTIVE_LOW>; + pwdn-gpio = < 6 GPIO_ACTIVE_HIGH>; + + port { + ov5640_to_csi0: endpoint { + remote-endpoint = <_from_ov5640>; + hsync-active = <1>; + vsync-active = <1>; + }; + }; + }; + }; diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 6f30ea7..8c6689b 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -488,6 +488,17 @@ config VIDEO_OV7640 To compile this driver as a module, choose M here: the module will be called ov7640. +config VIDEO_OV5640 + tristate "OmniVision OV5640 sensor support" + depends on I2C && VIDEO_V4L2 + depends on MEDIA_CAMERA_SUPPORT + ---help--- + This is a Video4Linux2 sensor-level driver for the OmniVision + OV5640 camera. + + To compile this driver as a module, choose M here: the + module will be called ov5640. + config VIDEO_OV7670 tristate "OmniVision OV7670 sensor support" depends on I2C && VIDEO_V4L2 diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 34e7da2..65b224f 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -54,6 +54,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o +obj-$(CONFIG_VIDEO_OV5640) += ov5640.o obj-$(CONFIG_VIDEO_OV7640) += ov7640.o obj-$(CONFIG_VIDEO_OV7670) += ov7670.o obj-$(CONFIG_VIDEO_OV9650) += ov9650.o diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c new file mode 100644 index 000..e06b812 --- /dev/null +++ b/drivers/media/i2c/ov5640.c @@ -0,0 +1,1403 @@ +/* + * Driver for the OV5640 sensor from Omnivision CSI interface only. + * + * Copyright (C) 2015 By Tech Design S.L. All Rights Reserved. + * Copyright (C) 2012-2013 Freescale Semiconductor, Inc. All Rights Reserved. + * + * Based on the MT9P031 driver and an out of tree ov5640 driver by Freescale: + * https://github.com/varigit/linux-2.6-imx/blob/imx_3.14.38_6qp_beta-var02/ + * drivers/media/platform/mxc/capture/ov5640.c + * + */ + +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + + * This program is distributed in the hope that it will be us
RFC: ov5640 kernel driver.
Hi, we want to a v4l2 driver for the ov5640 sensor from Omnivision. AFAIK, there was an attempt in the past to mainline that driver [1] but it didn't make it in the end. Some people were asking for the code for the ov5640 and the ov5642 to be merged [2] as well but IMHO both sensors are not that similar so that it's worth a common driver. The approach we had in mind so far was creating a new, independent, v4l2-subdev driver for the ov5640 with mbus support. I've found several sources out there with code for the ov5640 but, surprisingly, few attempts to mainline it. I would whether it is just people didn't take the effort or there was something wrong with the code. Has anyone got some comments/advices on this before we start coding? Is anyone already working on this and maybe we can collaborate instead of having two forks of the same driver? Regards, Javier. [1] https://lwn.net/Articles/470643/ [2] http://www.spinics.net/lists/linux-omap/msg69611.html -- 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
coda: i.MX6 decoding performance issues for multi-streaming
Hi, we have an i.MX6 Solo based board running the latest mainline kernel (4.15.3). As part of our development we were measuring the decoding performance of the i.MX6 coda chip. For that purpose we are feeding the decoder with 640x368 @ 30fps H.264 streams that have been generated by another i.MX6 coda encoder configured with fixed qp = 25 and gopsize = 16. For 1-2 streams it works smoothly. However, when adding the 3rd stream the first decoder instance starts to output these kind of errors: DEC_PIC_SUCCESS = 2097153 -> 0x21 DEC_PIC_SUCCESS = 2621441 -> 0x280001 Every time one of these errors appears we can observe a weird artifact in the decoded video (pixelated macroblocks and/or jumps back in time). I tried looking at the original VPU lib implementation by Freescale [1] but they don't seem to handle these errors either. As I don't have access to any kind of Coda IP documentation it's quite hard to me to perform any additional debugging. Has anyone experienced these kind of performance issues too? I'm open to any suggestions and willing to perform extra tests to get to the bottom of this. Regards, Javier. [1] https://github.com/genesi/imx-lib/blob/master/vpu/vpu_lib.c#L2926
Re: [DE] Re: coda: i.MX6 decoding performance issues for multi-streaming
Sorry everyone about my previous e-mail with all the HTML garbage. Here is the plain text answer instead. Hi Philipp, thanks for your answer. On 13/03/18 12:20, Philipp Zabel wrote: > Hi Javier, > > On Mon, 2018-03-12 at 17:54 +0100, Javier Martin wrote: >> Hi, >> we have an i.MX6 Solo based board running the latest mainline kernel >> (4.15.3). >> >> As part of our development we were measuring the decoding performance of >> the i.MX6 coda chip. >> >> For that purpose we are feeding the decoder with 640x368 @ 30fps H.264 >> streams that have been generated by another i.MX6 coda encoder >> configured with fixed qp = 25 and gopsize = 16. >> >> For 1-2 streams it works smoothly. However, when adding the 3rd stream >> the first decoder instance starts to output these kind of errors: >> >> DEC_PIC_SUCCESS = 2097153 -> 0x21 >> DEC_PIC_SUCCESS = 2621441 -> 0x280001 > I think these might be (recoverable?) error flags, but so far I have > never seen them myself. > I've had reports of those occurring occasionally with certain streams > (not encoded by coda, regardless of the number of running decoder > instances) though. > > What is the coda firmware version you are using? I'm currently using 3.1.1 both for encoding and decoding. I think I got it from the latest BSP provided by NXP. Now that you mention it the driver is printing these messages at probe time which I had ignored so far: coda 204.vpu: Firmware code revision: 46056 coda 204.vpu: Initialized CODA960. coda 204.vpu: Unsupported firmware version: 3.1.1 coda 204.vpu: codec registered as /dev/video[3-4] Do you think I should use an older version instead? Also, do you think it would be worth trying different parameters in the encoder to see how the decoder responds in those cases? Regards, Javier.
Re: [CN] Re: [DE] Re: coda: i.MX6 decoding performance issues for multi-streaming
Hello, On 14/03/18 14:57, Philipp Zabel wrote: On Wed, 2018-03-14 at 13:05 +0100, Javier Martin wrote: Sorry everyone about my previous e-mail with all the HTML garbage. Here is the plain text answer instead. Hi Philipp, thanks for your answer. On 13/03/18 12:20, Philipp Zabel wrote: > Hi Javier, > > On Mon, 2018-03-12 at 17:54 +0100, Javier Martin wrote: >> Hi, >> we have an i.MX6 Solo based board running the latest mainline kernel >> (4.15.3). >> >> As part of our development we were measuring the decoding performance of >> the i.MX6 coda chip. >> >> For that purpose we are feeding the decoder with 640x368 @ 30fps H.264 >> streams that have been generated by another i.MX6 coda encoder >> configured with fixed qp = 25 and gopsize = 16. Those are the defaults. Is the encoder running on the same system, at the same time? Or are you decoding a previously encoded stream (multiple previously encoded streams)? The encoder is running on a different system with an older 4.1.0 kernel. Altough the firmware version in the code is 3.1.1 as well. Do you think I should try updating the system in the encoder to kernel 4.15 too and see if that makes any difference? [...] I'm currently using 3.1.1 both for encoding and decoding. I think I got it from the latest BSP provided by NXP. Now that you mention it the driver is printing these messages at probe time which I had ignored so far: coda 204.vpu: Firmware code revision: 46056 coda 204.vpu: Initialized CODA960. coda 204.vpu: Unsupported firmware version: 3.1.1 coda 204.vpu: codec registered as /dev/video[3-4] That is strange, commit be7f1ab26f42 ("media: coda: mark CODA960 firmware versions 2.3.10 and 3.1.1 as supported") was merged in v4.14. You are right, those messages where taken from an old 4.1 kernel and not from the latest 4.15 where they don't appear any longer. Sorry for the noise. Do you think I should use an older version instead? Unfortunately I have no indication that this would help. Also, do you think it would be worth trying different parameters in the encoder to see how the decoder responds in those cases? Possibly. It would be interesting to know if this happens more often for low resolutions / low quality / static frames than high resolutions / high quality / high movement. I can easily prepare a test matrix with several resolutions, QPs and content and let you know the results. Although first I'd like to know your opinion on whether I should update the encoder to kernel 4.15 too. Regards, Javier.
Re: [DE] Re: [CN] Re: [DE] Re: coda: i.MX6 decoding performance issues for multi-streaming
Hello Philipp, On 14/03/18 16:11, Philipp Zabel wrote: Hi Javier, On Wed, 2018-03-14 at 15:35 +0100, Javier Martin wrote: [...] The encoder is running on a different system with an older 4.1.0 kernel. Altough the firmware version in the code is 3.1.1 as well. Do you think I should try updating the system in the encoder to kernel 4.15 too and see if that makes any difference? I don't think that should matter. It'd be more interesting if GOP size has a significant influence. Does the Problem also appear in I-frame only streams? OK, I've performed some tests with several resolutions and gop sizes, here is the table with the results: Always playing 3 streams | Resolution | QP | GopSize | Kind of content | Result | | 640x368 | 25 |16 | Waving hands | time shifts, no DEC_PIC_SUCCESS | | 640x368 | 25 |0 | Waving hands | time shifts, no DEC_PIC_SUCCESS | | 320x192 | 25 |0 | Waving hands | time shifts, no DEC_PIC_SUCCESS | | 320x192 | 25 |16 | Waving hands | time shifts, no DEC_PIC_SUCCESS | | 1280x720 | 25 |16 | Waving hands | macroblock artifacts and lots of DEC_PIC_SUCCESS messages | | 1280x720 | 25 |0 | Waving hands | Surprisingly smooth, no artifacts, time shifts nor DEC_PIC_SUCCESS| * The issues always happens in the first stream, the other 2 streams are fine. * With GopSize = 0 I can even decode 4 720p streams with no artifacts It looks like for small resolutions it suffers from time shifts when multi-streaming, always affecting the first stream for some reason. In this case gop size doesn't seem to make any difference. For higher resolutions like 720p using GopSize = 0 seems to improve things a lot. Regards, Javier.
Re: [DE] Re: [CN] Re: [DE] Re: coda: i.MX6 decoding performance issues for multi-streaming
Sorry for resurrecting this thread but I'm still quite interested on making this scenario work: > OK, I've performed some tests with several resolutions and gop sizes, here is the table with the results: > > Always playing 3 streams > > | Resolution | QP | GopSize | Kind of content | Result | > | 640x368 | 25 |16 | Waving hands | time shifts, no DEC_PIC_SUCCESS | > | 640x368 | 25 |0 | Waving hands | time shifts, no DEC_PIC_SUCCESS| > | 320x192 | 25 |0 | Waving hands | time shifts, no DEC_PIC_SUCCESS | > | 320x192 | 25 |16 | Waving hands | time shifts, no DEC_PIC_SUCCESS | > | 1280x720 | 25 |16 | Waving hands | macroblock artifacts and lots of DEC_PIC_SUCCESS messages | > | 1280x720 | 25 |0 | Waving hands | Surprisingly smooth, no artifacts, time shifts nor DEC_PIC_SUCCESS| > > * The issues always happens in the first stream, the other 2 streams are fine. > * With GopSize = 0 I can even decode 4 720p streams with no artifacts > > It looks like for small resolutions it suffers from time shifts when multi-streaming, always affecting the first stream for some reason. In this case gop size doesn't seem to make any difference. > > For higher resolutions like 720p using GopSize = 0 seems to improve things a lot. > Philipp, you mentioned some possible issue with context switches in a previous e-mail: > I fear this may be some interaction between coda context switches and > bitstream reader unit state. Philipp, do these results confirm your theory? Are there any more tests I could prepare to help get to the bottom of this or this is something that belongs entirely to the coda firmware domain? Does anyone know if the official BSP from NXP is able to decode 4 flows without issues?