Re: [PATCH 2/5] media: ov7670: make try_fmt() consistent with 'min_height' and 'min_width'.

2012-09-27 Thread javier Martin
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.

2012-09-27 Thread javier Martin
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.

2012-09-27 Thread javier Martin
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.

2012-09-27 Thread javier Martin
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.

2012-09-27 Thread Javier Martin
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.

2012-09-27 Thread Javier Martin
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.

2012-09-27 Thread Javier Martin
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'.

2012-09-27 Thread Javier Martin
'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.

2012-09-27 Thread Javier Martin
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.

2012-09-27 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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

2012-09-28 Thread Javier Martin
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

2012-09-28 Thread javier Martin
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.

2012-09-28 Thread Javier Martin
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

2012-09-28 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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

2012-09-28 Thread javier Martin
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.

2012-09-28 Thread Javier Martin
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

2012-09-28 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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.

2012-09-28 Thread Javier Martin
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.

2012-10-08 Thread javier Martin
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

2012-10-08 Thread javier Martin
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

2012-10-09 Thread javier Martin
 *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

2012-10-09 Thread javier Martin
 __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

2012-10-09 Thread javier Martin
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()

2012-10-10 Thread javier Martin
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

2012-10-24 Thread javier Martin
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

2015-06-23 Thread Javier Martin

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.

2015-07-29 Thread Javier Martin

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.

2015-07-29 Thread Javier Martin

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.

2015-08-07 Thread Javier Martin

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.

2015-08-07 Thread Javier Martin

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.

2015-09-30 Thread Javier Martin
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.

2015-09-30 Thread Javier Martin

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.

2015-09-30 Thread Javier Martin
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.

2015-09-21 Thread Javier Martin

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

2018-03-12 Thread Javier Martin

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

2018-03-14 Thread Javier Martin
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

2018-03-14 Thread Javier Martin

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

2018-03-14 Thread Javier Martin


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

2018-04-23 Thread Javier Martin
 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?


<    1   2   3   4   5