[PATCH v2] media: soc_camera: rcar_vin: Add support for 10-bit YUV cameras

2014-02-25 Thread Phil Edworthy
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
v2:
  - Fix silly mistake with missing break.

 drivers/media/platform/soc_camera/rcar_vin.c |9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c 
b/drivers/media/platform/soc_camera/rcar_vin.c
index 3b1c05a..702dc47 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -68,6 +68,8 @@
 #define VNMC_YCAL  (1  19)
 #define VNMC_INF_YUV8_BT656(0  16)
 #define VNMC_INF_YUV8_BT601(1  16)
+#define VNMC_INF_YUV10_BT656   (2  16)
+#define VNMC_INF_YUV10_BT601   (3  16)
 #define VNMC_INF_YUV16 (5  16)
 #define VNMC_VUP   (1  10)
 #define VNMC_IM_ODD(0  3)
@@ -275,6 +277,12 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
vnmc |= priv-pdata-flags  RCAR_VIN_BT656 ?
VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
+   break;
+   case V4L2_MBUS_FMT_YUYV10_2X10:
+   /* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
+   vnmc |= priv-pdata-flags  RCAR_VIN_BT656 ?
+   VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
+   break;
default:
break;
}
@@ -1003,6 +1011,7 @@ static int rcar_vin_get_formats(struct soc_camera_device 
*icd, unsigned int idx,
switch (code) {
case V4L2_MBUS_FMT_YUYV8_1X16:
case V4L2_MBUS_FMT_YUYV8_2X8:
+   case V4L2_MBUS_FMT_YUYV10_2X10:
if (cam-extra_fmt)
break;
 
-- 
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] media: soc_camera: rcar_vin: Add support for 10-bit YUV cameras

2014-02-24 Thread Phil Edworthy
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
 drivers/media/platform/soc_camera/rcar_vin.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c 
b/drivers/media/platform/soc_camera/rcar_vin.c
index 3b1c05a..9929375 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -68,6 +68,8 @@
 #define VNMC_YCAL  (1  19)
 #define VNMC_INF_YUV8_BT656(0  16)
 #define VNMC_INF_YUV8_BT601(1  16)
+#define VNMC_INF_YUV10_BT656   (2  16)
+#define VNMC_INF_YUV10_BT601   (3  16)
 #define VNMC_INF_YUV16 (5  16)
 #define VNMC_VUP   (1  10)
 #define VNMC_IM_ODD(0  3)
@@ -275,6 +277,10 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
vnmc |= priv-pdata-flags  RCAR_VIN_BT656 ?
VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
+   case V4L2_MBUS_FMT_YUYV10_2X10:
+   /* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
+   vnmc |= priv-pdata-flags  RCAR_VIN_BT656 ?
+   VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
default:
break;
}
@@ -1003,6 +1009,7 @@ static int rcar_vin_get_formats(struct soc_camera_device 
*icd, unsigned int idx,
switch (code) {
case V4L2_MBUS_FMT_YUYV8_1X16:
case V4L2_MBUS_FMT_YUYV8_2X8:
+   case V4L2_MBUS_FMT_YUYV10_2X10:
if (cam-extra_fmt)
break;
 
-- 
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 v3] ov10635: Add OmniVision ov10635 SoC camera driver

2013-07-29 Thread phil . edworthy
Hi Guennadi,

  Ok, now I see. My comment about the sensor output size changing is 
wrong. 
  The sensor doesn't do any scaling, so we are cropping it.
 
 Ah, ok, then you shouldn't change video sizes in your .s_fmt(), just 
 return the current cropping rectangle.

I'm reworking the code but realised that the sensor _does_ do both scaling 
and cropping. Though scaling is only possible by dropping every other scan 
line when required height is = 400 pixels. So does that mean .s_fmt() 
should select the appropriate mode?

Thanks
Phil
--
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] ov10635: Add OmniVision ov10635 SoC camera driver

2013-07-18 Thread phil . edworthy
Hi Guennadi,

  +{
  +   struct i2c_client *client = v4l2_get_subdevdata(sd);
  +   struct ov10635_priv *priv = to_ov10635(client);
  +   struct v4l2_captureparm *cp = parms-parm.capture;
  +   enum v4l2_mbus_pixelcode code;
  +   int ret;
  +
  +   if (parms-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
  +  return -EINVAL;
  +   if (cp-extendedmode != 0)
  +  return -EINVAL;
  +
  +   /* FIXME Check we can handle the requested framerate */
  +   priv-fps_denominator = cp-timeperframe.numerator;
  +   priv-fps_numerator = cp-timeperframe.denominator;
 
 Yes, fixing this could be a good idea :) Just add one parameter to your 
 set_params() and use NULL elsewhere.

There is one issue with setting the camera to achieve different framerate. 
The camera can work at up to 60fps with lower resolutions, i.e. when 
vertical sub-sampling is used. However, the API uses separate functions 
for changing resolution and framerate. So, userspace could use a low 
resolution, high framerate setting, then attempt to use a high resolution, 
low framerate setting. Clearly, it's possible for userspace to call s_fmt 
and s_parm in a way that attempts to set high resolution with the old 
(high) framerate. In this case, a check for valid settings will fail.

Is this a generally known issue and userspace works round it?

Thanks
Phil
--
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] ov10635: Add OmniVision ov10635 SoC camera driver

2013-07-18 Thread phil . edworthy
Hi Guennadi,

  There is one issue with setting the camera to achieve different 
framerate. 
  The camera can work at up to 60fps with lower resolutions, i.e. when 
  vertical sub-sampling is used. However, the API uses separate 
functions 
  for changing resolution and framerate. So, userspace could use a low 
  resolution, high framerate setting, then attempt to use a high 
resolution, 
  low framerate setting. Clearly, it's possible for userspace to call 
s_fmt 
  and s_parm in a way that attempts to set high resolution with the old 
  (high) framerate. In this case, a check for valid settings will fail.
  
  Is this a generally known issue and userspace works round it?
 
 It is generally known, that not all ioctl() settings can be combined, 
yes. 
 E.g. a driver can support a range of cropping values and multiple 
formats, 
 but not every format can be used with every cropping rectangle. So, if 
you 
 first set a format and then an incompatible cropping or vice versa, one 
of 
 ioctl()s will either fail or adjust parameters as close to the original 
 request as possible. This has been discussed multiple times, ideas were 
 expressed to create a recommended or even a compulsory ioctl() order, 
but 
 I'm not sure how far this has come. I'm sure other developers on the 
list 
 will have more info to this topic.

Thanks for the info.

On a similar note, cameras often need quite long periods after setting 
registers before they take hold. Currently this driver will change the 
registers, and delay, for both calls to s_parm and s_fmt. Is there a way 
to avoid this?

Thanks
Phil
--
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] ov10635: Add OmniVision ov10635 SoC camera driver

2013-07-16 Thread phil . edworthy
Hi Laurent,

 From: Laurent Pinchart laurent.pinch...@ideasonboard.com
 To: Phil Edworthy phil.edwor...@renesas.com, 
 Cc: linux-media@vger.kernel.org, Jean-Philippe Francois 
 jp.franc...@cynove.com, Hans Verkuil hverk...@xs4all.nl, Guennadi 
 Liakhovetski g.liakhovet...@gmx.de, Mauro Carvalho Chehab 
mche...@redhat.com
 Date: 16/07/2013 13:05
 Subject: Re: [PATCH v3] ov10635: Add OmniVision ov10635 SoC camera 
driver
 
 Hi Phil,
 
 Thank you for the patch.
 
 On Wednesday 05 June 2013 10:11:35 Phil Edworthy wrote:
  Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
  ---
  v3:
   - Removed dupplicated writes to reg 0x3042
   - Program all the standard registers after checking the ID
  
  v2:
   - Simplified flow in ov10635_s_ctrl.
   - Removed chip ident code - build tested only
  
   drivers/media/i2c/soc_camera/Kconfig   |6 +
   drivers/media/i2c/soc_camera/Makefile  |1 +
   drivers/media/i2c/soc_camera/ov10635.c | 1134 
+
   3 files changed, 1141 insertions(+)
   create mode 100644 drivers/media/i2c/soc_camera/ov10635.c
  
  diff --git a/drivers/media/i2c/soc_camera/Kconfig
  b/drivers/media/i2c/soc_camera/Kconfig index 23d352f..db97ee6 100644
  --- a/drivers/media/i2c/soc_camera/Kconfig
  +++ b/drivers/media/i2c/soc_camera/Kconfig
  @@ -74,6 +74,12 @@ config SOC_CAMERA_OV9740
  help
This is a ov9740 camera driver
  
  +config SOC_CAMERA_OV10635
  +   tristate ov10635 camera support
  +   depends on SOC_CAMERA  I2C
  +   help
  + This is an OmniVision ov10635 camera driver
  +
   config SOC_CAMERA_RJ54N1
  tristate rj54n1cb0c support
  depends on SOC_CAMERA  I2C
  diff --git a/drivers/media/i2c/soc_camera/Makefile
  b/drivers/media/i2c/soc_camera/Makefile index d0421fe..f3d3403 100644
  --- a/drivers/media/i2c/soc_camera/Makefile
  +++ b/drivers/media/i2c/soc_camera/Makefile
  @@ -10,5 +10,6 @@ obj-$(CONFIG_SOC_CAMERA_OV6650)  += ov6650.o
   obj-$(CONFIG_SOC_CAMERA_OV772X)  += ov772x.o
   obj-$(CONFIG_SOC_CAMERA_OV9640)  += ov9640.o
   obj-$(CONFIG_SOC_CAMERA_OV9740)  += ov9740.o
  +obj-$(CONFIG_SOC_CAMERA_OV10635)   += ov10635.o
   obj-$(CONFIG_SOC_CAMERA_RJ54N1)  += rj54n1cb0c.o
   obj-$(CONFIG_SOC_CAMERA_TW9910)  += tw9910.o
  diff --git a/drivers/media/i2c/soc_camera/ov10635.c
  b/drivers/media/i2c/soc_camera/ov10635.c new file mode 100644
  index 000..064cc7b
  --- /dev/null
  +++ b/drivers/media/i2c/soc_camera/ov10635.c
  @@ -0,0 +1,1134 @@
  +/*
  + * OmniVision OV10635 Camera Driver
  + *
  + * Copyright (C) 2013 Phil Edworthy
  + * Copyright (C) 2013 Renesas Electronics
  + *
  + * This driver has been tested at QVGA, VGA and 720p, and 1280x800 at 
up to
  + * 30fps and it should work at any resolution in between and any 
frame
  rate + * up to 30fps.
  + *
  + * FIXME:
  + *  Horizontal flip (mirroring) does not work correctly. The image is
  flipped, + *  but the colors are wrong.
  + *
  + * This program is free software; you can redistribute it and/or 
modify
  + * it under the terms of the GNU General Public License version 2 as
  + * published by the Free Software Foundation.
  + *
  + */
  +
  +#include linux/delay.h
  +#include linux/i2c.h
  +#include linux/init.h
  +#include linux/module.h
  +#include linux/slab.h
  +#include linux/v4l2-mediabus.h
  +#include linux/videodev2.h
  +
  +#include media/soc_camera.h
  +#include media/v4l2-common.h
  +#include media/v4l2-ctrls.h
  +
  +/* Register definitions */
  +#define   OV10635_VFLIP 0x381c
  +#defineOV10635_VFLIP_ON  (0x3  6)
  +#defineOV10635_VFLIP_SUBSAMPLE   0x1
  +#define   OV10635_HMIRROR 0x381d
  +#defineOV10635_HMIRROR_ON  0x3
  +#define OV10635_PID 0x300a
  +#define OV10635_VER 0x300b
  +
  +/* IDs */
  +#define OV10635_VERSION_REG  0xa635
  +#define OV10635_VERSION(pid, ver)   (((pid)  8) | ((ver)  0xff))
  +
  +#define OV10635_SENSOR_WIDTH  1312
  +#define OV10635_SENSOR_HEIGHT  814
  +
  +#define OV10635_MAX_WIDTH  1280
  +#define OV10635_MAX_HEIGHT  800
  +
  +struct ov10635_color_format {
  +   enum v4l2_mbus_pixelcode code;
  +   enum v4l2_colorspace colorspace;
  +};
  +
  +struct ov10635_reg {
  +   u16   reg;
  +   u8   val;
  +};
  +
  +struct ov10635_priv {
  +   struct v4l2_subdev   subdev;
  +   struct v4l2_ctrl_handler   hdl;
  +   int xvclk;
  +   int fps_numerator;
  +   int fps_denominator;
  +   const struct ov10635_color_format   *cfmt;
  +   int width;
  +   int height;
  +};
  +
  +/* default register setup */
  +static const struct ov10635_reg ov10635_regs_default[] = {
  +   { 0x0103, 0x01 }, { 0x301b, 0xff }, { 0x301c, 0xff }, { 0x301a, 
0xff },
snip

  +   { 0xc35c, 0x00 }, { 0xc4bc, 0x01 }, { 0xc4bd, 0x60 }, { 0x5608, 
0x0d },
  +};
  +
  +static const struct ov10635_reg ov10635_regs_change_mode[] = {
  +   { 0x301b, 0xff }, { 0x301c, 0xff }, { 0x301a, 0xff }, { 0x5005, 
0x08 },
  +   { 0x3007

Re: [PATCH v3] ov10635: Add OmniVision ov10635 SoC camera driver

2013-07-15 Thread phil . edworthy
Hi Guennadi,

Thanks for the review.

 I'll comment to this version, although the driver has to be updated to 
the 
 V4L2 clock API at least, preferably also to asynchronous probing.
Ok, I'll have to look into that.
 
snip
  + * FIXME:
  + *  Horizontal flip (mirroring) does not work correctly. The image is 
flipped,
  + *  but the colors are wrong.
 
 Then maybe just remove it, if you cannot fix it? You could post an 
 incremental patch / rfc later just to have it on the ML for the future, 
in 
 case someone manages to fix it.
Ok, I'll remove it.
 
snip
  +/* Register definitions */
  +#define   OV10635_VFLIP 0x381c
  +#defineOV10635_VFLIP_ON  (0x3  6)
  +#defineOV10635_VFLIP_SUBSAMPLE   0x1
  +#define   OV10635_HMIRROR 0x381d
  +#defineOV10635_HMIRROR_ON  0x3
  +#define OV10635_PID 0x300a
  +#define OV10635_VER 0x300b
 
 Please, don't mix spaces and TABs for the same pattern, I'd just use 
 spaces between #define and the macro name above
Oops, missed those.
 
snip
  +struct ov10635_priv {
  +   struct v4l2_subdev   subdev;
  +   struct v4l2_ctrl_handler   hdl;
  +   int xvclk;
  +   int fps_numerator;
  +   int fps_denominator;
  +   const struct ov10635_color_format   *cfmt;
  +   int width;
  +   int height;
  +};
 
 Uhm, may I suggest to either align all the lines, or to leave all 
 unaligned :) Either variant would look better than the above imho :)
Ok

snip
  +/* read a register */
  +static int ov10635_reg_read(struct i2c_client *client, u16 reg, u8 
*val)
  +{
  +   int ret;
  +   u8 data[2] = { reg  8, reg  0xff };
  +   struct i2c_msg msg = {
  +  .addr   = client-addr,
  +  .flags   = 0,
  +  .len   = 2,
  +  .buf   = data,
  +   };
  +
  +   ret = i2c_transfer(client-adapter, msg, 1);
  +   if (ret  0)
  +  goto err;
  +
  +   msg.flags = I2C_M_RD;
  +   msg.len   = 1;
  +   msg.buf   = data,
  +   ret = i2c_transfer(client-adapter, msg, 1);
  +   if (ret  0)
  +  goto err;
  +
  +   *val = data[0];
 
 I think, you can do this in one I2C transfer with 2 messages. Look e.g. 
 imx074.c. Although, now looking at it, I'm not sure why it has .len = 2 
in 
 the second message...
Ok, I'll change this to one i2c transfer. As you sauy, no idea why the imx 
code is reading 2 bytes though...

snip
 Also here: wouldn't it be possible and better to do this as one I2C 
 transfer with 2 messages?
Ok

snip
  +/* Read a register, alter its bits, write it back */
  +static int ov10635_reg_rmw(struct i2c_client *client, u16 reg, u8 
set, u8 unset)
  +{
  +   u8 val;
  +   int ret;
  +
  +   ret = ov10635_reg_read(client, reg, val);
  +   if (ret) {
  +  dev_err(client-dev,
  + [Read]-Modify-Write of register %04x failed!\n, reg);
  +  return ret;
  +   }
  +
  +   val |= set;
  +   val = ~unset;
  +
  +   ret = ov10635_reg_write(client, reg, val);
  +   if (ret)
  +  dev_err(client-dev,
  + Read-Modify-[Write] of register %04x failed!\n, reg);
  +
  +   return ret;
  +}
  +
  +
 
 Are these two empty lines above on purpose?
No, I'll remove the extra one

snip
  +/* Start/Stop streaming from the device */
  +static int ov10635_s_stream(struct v4l2_subdev *sd, int enable)
  +{
  +   struct i2c_client *client = v4l2_get_subdevdata(sd);
  +
  +   ov10635_reg_write(client, 0x0100, enable);
  +   return 0;
 
 Don't you want to return the possible error from reg_write()?
 
return ov10635_reg_write(...);
Yes, I will do so. 

snip
  + continue;
  +
  +  /* Mult = reg 0x3003, bits 5:0 */
 
 You could also define macros for 0x3003, 0x3004 and others, where you 
know 
 the role of those registers, even if not their official names.
Do you mean macros for the bit fields?

snip
 superfluous parenthesis
and
 Coding style - please, add spaces around arithmetic operations 
throughout 
 the code.
I'll check them all.

snip
  +/* 2 clock cycles for every YUV422 pixel */
  +if (pclk  (((hts * *vtsmin)/fps_denominator)
  +   * fps_numerator * 2))
 
 Actually just
 
 +if (pclk  hts * *vtsmin / fps_denominator
 +   * fps_numerator * 2)
 
 would do just fine
It would, but I think we should use parenthesis here to ensure the  divide 
by the denominator happens before multiplying by the numerator. This is to 
ensure the value doesn't overflow.
 
snip
  +   *r3003 = (u8)best_j;
  +   *r3004 = ((u8)best_i  4) | (u8)best_k;
 
 You don't need those casts: i  8, j  32, k  8.
Ok.
 
snip
  +
  +   /* Did we get a valid PCLK? */
  +   if (best_pclk == INT_MAX)
  +  return -1;
 
 But you could move this check above *r3003 and *r3004 assignments and 
 please, return a proper error code, not -1 (-EPERM).
Ok

snip
  +static int ov10635_set_regs(struct i2c_client *client,
  +   const struct ov10635_reg *regs, int nr_regs)
 
 You might consider defining a macro like
 
 #define ov10635_set_reg_array(client, array) 

Re: [PATCH v3] ov10635: Add OmniVision ov10635 SoC camera driver

2013-07-15 Thread phil . edworthy
Hi Guennadi,

+/* read a register */
+static int ov10635_reg_read(struct i2c_client *client, u16 reg, 
u8 
  *val)
+{
+   int ret;
+   u8 data[2] = { reg  8, reg  0xff };
+   struct i2c_msg msg = {
+  .addr   = client-addr,
+  .flags   = 0,
+  .len   = 2,
+  .buf   = data,
+   };
+
+   ret = i2c_transfer(client-adapter, msg, 1);
+   if (ret  0)
+  goto err;
+
+   msg.flags = I2C_M_RD;
+   msg.len   = 1;
+   msg.buf   = data,
+   ret = i2c_transfer(client-adapter, msg, 1);
+   if (ret  0)
+  goto err;
+
+   *val = data[0];
   
   I think, you can do this in one I2C transfer with 2 messages. Look 
e.g. 
   imx074.c. Although, now looking at it, I'm not sure why it has .len 
= 2 
  in 
   the second message...
  Ok, I'll change this to one i2c transfer. As you sauy, no idea why the 
imx 
  code is reading 2 bytes though...
 
 But I don't have any way to test it anymore, anyway: the only user - 
 ap4evb - is gone now. So, that driver doesn't matter much anymore. We 
can 
 just fix that blindly without testing, or we can leave it as is and mark 

 the driver broken, or we can remove it completely.
ok

 [snip]
 
+ continue;
+
+  /* Mult = reg 0x3003, bits 5:0 */
   
   You could also define macros for 0x3003, 0x3004 and others, where 
you 
  know 
   the role of those registers, even if not their official names.
  Do you mean macros for the bit fields?
 
 No, primarily I mean macros for register addresses.
ok

 [snip]
 
+/* 2 clock cycles for every YUV422 pixel */
+if (pclk  (((hts * *vtsmin)/fps_denominator)
+   * fps_numerator * 2))
   
   Actually just
   
   +if (pclk  hts * *vtsmin / fps_denominator
   +   * fps_numerator * 2)
   
   would do just fine
  It would, but I think we should use parenthesis here to ensure the 
divide 
  by the denominator happens before multiplying by the numerator. This 
is to 
  ensure the value doesn't overflow.
 
 I think the C standard already guarantees that. You only need 
parenthesis 
 to enforce a non-standard calculation order.
ok, but I think I'll add a comment about being careful to avoid overflow.

 [snip]
 
+static int ov10635_g_crop(struct v4l2_subdev *sd, struct 
v4l2_crop *a)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
+   struct ov10635_priv *priv = to_ov10635(client);
+
+   if (priv) {
+  a-c.width = priv-width;
+  a-c.height = priv-height;
   
   Wait, what is priv-width and priv-height? Are they sensor output 
sizes 
   or crop sizes?
  Sensor output sizes. Ah, I guess this is one of the few 
cameras/drivers 
  that can support setup the sensor for any size (except restrictions 
for 
  4:2:2 format). So maybe I should not implement these functions? 
Looking at 
  the CEU SoC camera host driver, it would then use the defrect cropcap. 
I 
  am not sure what that will be though.
 
 Cropping and scaling are two different functions. Cropping selects an 
area 
 on the sensor matrix to use for data sampling. Scaling configures to 
which 
 output rectangle to scale that area. So, since your camera can do both 
and 
 your driver supports both, you shouldn't return the same sizes in 
.g_fmt() 
 and .g_crop() unless, of course, a 1:1 scale has been set. And currently 

 you do exactly that - return priv-width and priv-height in both.
Ok, now I see. My comment about the sensor output size changing is wrong. 
The sensor doesn't do any scaling, so we are cropping it.

Thanks
Phil
--
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] ov10635: Add OmniVision ov10635 SoC camera driver

2013-07-12 Thread phil . edworthy
Hi Guennadi,

snip
 Well, I think, both of you will agree, that these register value lists 
 look horrible and actually have little to do with open-source software, 
 but I don't know what to do about them either. We could just reject them 

 and only accept drivers, properly describing what they do with the 
 hardware, or request to move this into one of firmware loading 
options... 
 But I'm not sure any of these options would actually do us any good. 
Just 
 sayin'.
Any further thoughts on what to do about this driver?

Based on what I have seen, we aren't going to get the info to properly 
describe _all_ of the registers. It's a shame the camera's registers don't 
default to usable settings, but that's what we have.

In the case of this driver, there are a number of registers that are 
programmed based on the required resolution, framerate and format. 
Extracting that from the material I had was rather painful and the result 
is useful for anyone wanting to use this camera. Any register written to 
outside ov10635_regs_default[] should be programmed in some meaningful 
way, whereas those in ov10635_regs_default[] should be treated like 
firmware. However, even those registers accessed outside of 
ov10635_regs_default[] could still do with better descriptions. I should 
note that some of the values written to registers are empirically derived 
since they are based on device timing. I don't have any details about the 
device timing, just the values that were written for a number of different 
modes (resolution and framerate).

Maybe the pragmatic approach is to use firmware for all those in 
ov10635_regs_default[], and the rest of the registers have to be well 
documented. I was asked to remove some comments about register 
names/functionality by OmniVision, but I can ask again if it helps.

Thanks
Phil
--
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] ov10635: Add OmniVision ov10635 SoC camera driver

2013-06-05 Thread Phil Edworthy
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
v3:
 - Removed dupplicated writes to reg 0x3042
 - Program all the standard registers after checking the ID

v2:
 - Simplified flow in ov10635_s_ctrl.
 - Removed chip ident code - build tested only

 drivers/media/i2c/soc_camera/Kconfig   |6 +
 drivers/media/i2c/soc_camera/Makefile  |1 +
 drivers/media/i2c/soc_camera/ov10635.c | 1134 
 3 files changed, 1141 insertions(+)
 create mode 100644 drivers/media/i2c/soc_camera/ov10635.c

diff --git a/drivers/media/i2c/soc_camera/Kconfig 
b/drivers/media/i2c/soc_camera/Kconfig
index 23d352f..db97ee6 100644
--- a/drivers/media/i2c/soc_camera/Kconfig
+++ b/drivers/media/i2c/soc_camera/Kconfig
@@ -74,6 +74,12 @@ config SOC_CAMERA_OV9740
help
  This is a ov9740 camera driver
 
+config SOC_CAMERA_OV10635
+   tristate ov10635 camera support
+   depends on SOC_CAMERA  I2C
+   help
+ This is an OmniVision ov10635 camera driver
+
 config SOC_CAMERA_RJ54N1
tristate rj54n1cb0c support
depends on SOC_CAMERA  I2C
diff --git a/drivers/media/i2c/soc_camera/Makefile 
b/drivers/media/i2c/soc_camera/Makefile
index d0421fe..f3d3403 100644
--- a/drivers/media/i2c/soc_camera/Makefile
+++ b/drivers/media/i2c/soc_camera/Makefile
@@ -10,5 +10,6 @@ obj-$(CONFIG_SOC_CAMERA_OV6650)   += ov6650.o
 obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o
 obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o
 obj-$(CONFIG_SOC_CAMERA_OV9740)+= ov9740.o
+obj-$(CONFIG_SOC_CAMERA_OV10635)   += ov10635.o
 obj-$(CONFIG_SOC_CAMERA_RJ54N1)+= rj54n1cb0c.o
 obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o
diff --git a/drivers/media/i2c/soc_camera/ov10635.c 
b/drivers/media/i2c/soc_camera/ov10635.c
new file mode 100644
index 000..064cc7b
--- /dev/null
+++ b/drivers/media/i2c/soc_camera/ov10635.c
@@ -0,0 +1,1134 @@
+/*
+ * OmniVision OV10635 Camera Driver
+ *
+ * Copyright (C) 2013 Phil Edworthy
+ * Copyright (C) 2013 Renesas Electronics
+ *
+ * This driver has been tested at QVGA, VGA and 720p, and 1280x800 at up to
+ * 30fps and it should work at any resolution in between and any frame rate
+ * up to 30fps.
+ *
+ * FIXME:
+ *  Horizontal flip (mirroring) does not work correctly. The image is flipped,
+ *  but the colors are wrong.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include linux/delay.h
+#include linux/i2c.h
+#include linux/init.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/v4l2-mediabus.h
+#include linux/videodev2.h
+
+#include media/soc_camera.h
+#include media/v4l2-common.h
+#include media/v4l2-ctrls.h
+
+/* Register definitions */
+#defineOV10635_VFLIP   0x381c
+#define OV10635_VFLIP_ON   (0x3  6)
+#define OV10635_VFLIP_SUBSAMPLE0x1
+#defineOV10635_HMIRROR 0x381d
+#define OV10635_HMIRROR_ON 0x3
+#define OV10635_PID0x300a
+#define OV10635_VER0x300b
+
+/* IDs */
+#define OV10635_VERSION_REG0xa635
+#define OV10635_VERSION(pid, ver)  (((pid)  8) | ((ver)  0xff))
+
+#define OV10635_SENSOR_WIDTH   1312
+#define OV10635_SENSOR_HEIGHT  814
+
+#define OV10635_MAX_WIDTH  1280
+#define OV10635_MAX_HEIGHT 800
+
+struct ov10635_color_format {
+   enum v4l2_mbus_pixelcode code;
+   enum v4l2_colorspace colorspace;
+};
+
+struct ov10635_reg {
+   u16 reg;
+   u8  val;
+};
+
+struct ov10635_priv {
+   struct v4l2_subdev  subdev;
+   struct v4l2_ctrl_handlerhdl;
+   int xvclk;
+   int fps_numerator;
+   int fps_denominator;
+   const struct ov10635_color_format   *cfmt;
+   int width;
+   int height;
+};
+
+/* default register setup */
+static const struct ov10635_reg ov10635_regs_default[] = {
+   { 0x0103, 0x01 }, { 0x301b, 0xff }, { 0x301c, 0xff }, { 0x301a, 0xff },
+   { 0x3011, 0x02 }, /* drive strength reduced to x1 */
+   { 0x6900, 0x0c }, { 0x6901, 0x11 }, { 0x3503, 0x10 }, { 0x3025, 0x03 },
+   { 0x3005, 0x20 }, { 0x3006, 0x91 }, { 0x3600, 0x74 }, { 0x3601, 0x2b },
+   { 0x3612, 0x00 }, { 0x3611, 0x67 }, { 0x3633, 0xca }, { 0x3602, 0x2f },
+   { 0x3603, 0x00 }, { 0x3630, 0x28 }, { 0x3631, 0x16 }, { 0x3714, 0x10 },
+   { 0x371d, 0x01 }, { 0x4300, 0x38 }, { 0x3007, 0x01 }, { 0x3024, 0x01 },
+   { 0x3020, 0x0b }, { 0x3702, 0x0d }, { 0x3703, 0x20 }, { 0x3704, 0x15 },
+   { 0x3709, 0xa8 }, { 0x370c, 0xc7 }, { 0x370d, 0x80 }, { 0x3712, 0x00 },
+   { 0x3713, 0x20 }, { 0x3715, 0x04 }, { 0x381d, 0x40

Re: [PATCH v2] ov10635: Add OmniVision ov10635 SoC camera driver

2013-06-05 Thread phil . edworthy
Hi Guennadi,

 Thanks for the patch. I'll look at it in more detail hopefully soon 
 enough... One remark so far to Jean-Philippe's comment:
 
[snip]
  Register 0x3042 is only touched by the enable part, not by the change
  mode part
  I think you could move the {0x3042, 0xf0} sequence in the
  standard_regs array, and keep
  only the 0x301b, 0x301c, 0x301a registers.
  
  By the way, did you test with a single write ? There is the same
  sequence in ov5642
  init, so I believe it is copy pasted in every omnivision init. Is it
  actually useful ?
 
 If this is indeed the case and all OV sensor camera drivers use the same 

 initialisation sequence, maybe it's time to consider making one firmware 

 file for them all and loading it in user-space?

After looking at other OV drivers, the sequences are very different. I 
think Jean-Philippe's comment was specific to data files that OV provide 
for their cameras.

Regards
Phil
--
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] ov10635: Add OmniVision ov10635 SoC camera driver

2013-06-04 Thread phil . edworthy
Hi Jean-Philippe,

Thanks for the review.

snip
  +static const struct ov10635_reg ov10635_regs_enable[] = {
  +   { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 
 0x3042, 0xf0 },
  +   { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 
 0x3042, 0xf0 },
  +   { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 
 0x3042, 0xf0 },
  +   { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 
 0x3042, 0xf0 },
  +   { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 
 0x3042, 0xf0 },
  +   { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 
 0x3042, 0xf0 },
  +   { 0x3042, 0xf0 }, { 0x3042, 0xf0 }, { 0x301b, 0xf0 }, { 
 0x301c, 0xf0 },
  +   { 0x301a, 0xf0 },
  +};
 
 Register 0x3042 is only touched by the enable part, not by the change
 mode part
 I think you could move the {0x3042, 0xf0} sequence in the
 standard_regs array, and keep
 only the 0x301b, 0x301c, 0x301a registers.
I tried this, but it doesn't work. You have to write to the 0x3042 
register after other setup writes.

 By the way, did you test with a single write ? There is the same
 sequence in ov5642
 init, so I believe it is copy pasted in every omnivision init. Is it
 actually useful ?
I tried this  a single write works so I'll change this. iirc, it was 
taken from a reference set of writes from OmniVision. 

snip
  +static int ov10635_video_probe(struct i2c_client *client)
  +{
  +   struct ov10635_priv *priv = to_ov10635(client);
  +   u8 pid, ver;
  +   int ret;
  +
  +   /* Program all the 'standard' registers */
  +   ret = ov10635_set_regs(client, ov10635_regs_default,
  +   ARRAY_SIZE(ov10635_regs_default));
  +   if (ret)
  +   return ret;
  +
  +   /* check and show product ID and manufacturer ID */
  +   ret = ov10635_reg_read(client, OV10635_PID, pid);
  +   if (ret)
  +   return ret;
  +   ret = ov10635_reg_read(client, OV10635_VER, ver);
  +   if (ret)
  +   return ret;
  +
  +   if (OV10635_VERSION(pid, ver) != OV10635_VERSION_REG) {
  +   dev_err(client-dev, Product ID error %x:%x\n, pid, 
ver);
  +   return -ENODEV;
  +   }
 
 Shouldn't the order be reversed here ?
 iow, first chek chip id register, then proceed with the register init ?
Good point! I'll fix this.

Thanks
Phil
--
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] ov10635: Add OmniVision ov10635 SoC camera driver

2013-06-03 Thread Phil Edworthy
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
 drivers/media/i2c/soc_camera/Kconfig   |6 +
 drivers/media/i2c/soc_camera/Makefile  |1 +
 drivers/media/i2c/soc_camera/ov10635.c | 1169 
 include/media/v4l2-chip-ident.h|1 +
 4 files changed, 1177 insertions(+)
 create mode 100644 drivers/media/i2c/soc_camera/ov10635.c

diff --git a/drivers/media/i2c/soc_camera/Kconfig 
b/drivers/media/i2c/soc_camera/Kconfig
index 6dff2b7..0b17296 100644
--- a/drivers/media/i2c/soc_camera/Kconfig
+++ b/drivers/media/i2c/soc_camera/Kconfig
@@ -76,6 +76,12 @@ config SOC_CAMERA_OV9740
help
  This is a ov9740 camera driver
 
+config SOC_CAMERA_OV10635
+   tristate ov10635 camera support
+   depends on SOC_CAMERA  I2C
+   help
+ This is an OmniVision ov10635 camera driver
+
 config SOC_CAMERA_RJ54N1
tristate rj54n1cb0c support
depends on SOC_CAMERA  I2C
diff --git a/drivers/media/i2c/soc_camera/Makefile 
b/drivers/media/i2c/soc_camera/Makefile
index d0421fe..f3d3403 100644
--- a/drivers/media/i2c/soc_camera/Makefile
+++ b/drivers/media/i2c/soc_camera/Makefile
@@ -10,5 +10,6 @@ obj-$(CONFIG_SOC_CAMERA_OV6650)   += ov6650.o
 obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o
 obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o
 obj-$(CONFIG_SOC_CAMERA_OV9740)+= ov9740.o
+obj-$(CONFIG_SOC_CAMERA_OV10635)   += ov10635.o
 obj-$(CONFIG_SOC_CAMERA_RJ54N1)+= rj54n1cb0c.o
 obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o
diff --git a/drivers/media/i2c/soc_camera/ov10635.c 
b/drivers/media/i2c/soc_camera/ov10635.c
new file mode 100644
index 000..82d01b8
--- /dev/null
+++ b/drivers/media/i2c/soc_camera/ov10635.c
@@ -0,0 +1,1169 @@
+/*
+ * OmniVision OV10635 Camera Driver
+ *
+ * Copyright (C) 2013 Phil Edworthy
+ * Copyright (C) 2013 Renesas Electronics
+ *
+ * This driver has been tested at QVGA, VGA and 720p, and 1280x800 at up to
+ * 30fps and it should work at any resolution in between and any frame rate
+ * up to 30fps.
+ *
+ * FIXME:
+ *  Horizontal flip (mirroring) does not work correctly. The image is flipped,
+ *  but the colors are wrong.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include linux/delay.h
+#include linux/i2c.h
+#include linux/init.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/v4l2-mediabus.h
+#include linux/videodev2.h
+
+#include media/soc_camera.h
+#include media/v4l2-chip-ident.h
+#include media/v4l2-common.h
+#include media/v4l2-ctrls.h
+
+/* Register definitions */
+#defineOV10635_VFLIP   0x381c
+#define OV10635_VFLIP_ON   (0x3  6)
+#define OV10635_VFLIP_SUBSAMPLE0x1
+#defineOV10635_HMIRROR 0x381d
+#define OV10635_HMIRROR_ON 0x3
+#define OV10635_PID0x300a
+#define OV10635_VER0x300b
+
+/* IDs */
+#define OV10635_VERSION_REG0xa635
+#define OV10635_VERSION(pid, ver)  (((pid)  8) | ((ver)  0xff))
+
+#define OV10635_SENSOR_WIDTH   1312
+#define OV10635_SENSOR_HEIGHT  814
+
+#define OV10635_MAX_WIDTH  1280
+#define OV10635_MAX_HEIGHT 800
+
+struct ov10635_color_format {
+   enum v4l2_mbus_pixelcode code;
+   enum v4l2_colorspace colorspace;
+};
+
+struct ov10635_reg {
+   u16 reg;
+   u8  val;
+};
+
+struct ov10635_priv {
+   struct v4l2_subdev  subdev;
+   struct v4l2_ctrl_handlerhdl;
+   int model;
+   int revision;
+   int xvclk;
+   int fps_numerator;
+   int fps_denominator;
+   const struct ov10635_color_format   *cfmt;
+   int width;
+   int height;
+};
+
+/* default register setup */
+static const struct ov10635_reg ov10635_regs_default[] = {
+   { 0x0103, 0x01 }, { 0x301b, 0xff }, { 0x301c, 0xff }, { 0x301a, 0xff },
+   { 0x3011, 0x02 }, /* drive strength reduced to x1 */
+   { 0x6900, 0x0c }, { 0x6901, 0x11 }, { 0x3503, 0x10 }, { 0x3025, 0x03 },
+   { 0x3005, 0x20 }, { 0x3006, 0x91 }, { 0x3600, 0x74 }, { 0x3601, 0x2b },
+   { 0x3612, 0x00 }, { 0x3611, 0x67 }, { 0x3633, 0xca }, { 0x3602, 0x2f },
+   { 0x3603, 0x00 }, { 0x3630, 0x28 }, { 0x3631, 0x16 }, { 0x3714, 0x10 },
+   { 0x371d, 0x01 }, { 0x4300, 0x38 }, { 0x3007, 0x01 }, { 0x3024, 0x01 },
+   { 0x3020, 0x0b }, { 0x3702, 0x0d }, { 0x3703, 0x20 }, { 0x3704, 0x15 },
+   { 0x3709, 0xa8 }, { 0x370c, 0xc7 }, { 0x370d, 0x80 }, { 0x3712, 0x00 },
+   { 0x3713, 0x20 }, { 0x3715, 0x04 }, { 0x381d, 0x40 }, { 0x381c, 0x00 },
+   { 0x3822

Re: [PATCH] ov10635: Add OmniVision ov10635 SoC camera driver

2013-06-03 Thread phil . edworthy
Hi Hans,

 Subject: Re: [PATCH] ov10635: Add OmniVision ov10635 SoC camera driver
snip
  +#include media/v4l2-chip-ident.h
 
 Don't implement chip_ident or use this header: it's going to be 
 removed in 3.11.

snip
  +/* Set status of additional camera capabilities */
  +static int ov10635_s_ctrl(struct v4l2_ctrl *ctrl)
  +{
  +   struct ov10635_priv *priv = container_of(ctrl-handler,
  +   struct ov10635_priv, hdl);
  +   struct i2c_client *client = v4l2_get_subdevdata(priv-subdev);
  +
  +   switch (ctrl-id) {
  +   case V4L2_CID_VFLIP:
  +  if (ctrl-val)
  + return ov10635_reg_rmw(client, OV10635_VFLIP,
  +OV10635_VFLIP_ON, 0);
  +  else
 
 No need for 'else'.
Ok.

  + return ov10635_reg_rmw(client, OV10635_VFLIP,
  +0, OV10635_VFLIP_ON);
  +  break;
 
 No need for 'break'. Ditto for HFLIP.
Ok.

snip
  +/* Get chip identification */
  +static int ov10635_g_chip_ident(struct v4l2_subdev *sd,
  +struct v4l2_dbg_chip_ident *id)
  +{
  +   struct i2c_client *client = v4l2_get_subdevdata(sd);
  +   struct ov10635_priv *priv = to_ov10635(client);
  +
  +   id-ident   = priv-model;
  +   id-revision   = priv-revision;
  +
  +   return 0;
  +}
 
 Drop this function, no longer needed.

snip
  diff --git a/include/media/v4l2-chip-ident.h b/include/media/v4l2-
 chip-ident.h
  index 4ee125b..8fc3303 100644
  --- a/include/media/v4l2-chip-ident.h
  +++ b/include/media/v4l2-chip-ident.h
  @@ -77,6 +77,7 @@ enum {
  V4L2_IDENT_OV2640 = 259,
  V4L2_IDENT_OV9740 = 260,
  V4L2_IDENT_OV5642 = 261,
  +   V4L2_IDENT_OV10635 = 262,
  
  /* module saa7146: reserved range 300-309 */
  V4L2_IDENT_SAA7146 = 300,
  
 
 This can be dropped as well, because this header will go away.

Thanks for the very quick review! I'll have a look at the media tree to 
see what's changing wrt the chip ident.

Phil
--
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] ov10635: Add OmniVision ov10635 SoC camera driver

2013-06-03 Thread Phil Edworthy
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
v2:
 - Simplified flow in ov10635_s_ctrl.
 - Removed chip ident code - build tested only

 drivers/media/i2c/soc_camera/Kconfig   |6 +
 drivers/media/i2c/soc_camera/Makefile  |1 +
 drivers/media/i2c/soc_camera/ov10635.c | 1141 
 3 files changed, 1148 insertions(+)
 create mode 100644 drivers/media/i2c/soc_camera/ov10635.c

diff --git a/drivers/media/i2c/soc_camera/Kconfig 
b/drivers/media/i2c/soc_camera/Kconfig
index 23d352f..db97ee6 100644
--- a/drivers/media/i2c/soc_camera/Kconfig
+++ b/drivers/media/i2c/soc_camera/Kconfig
@@ -74,6 +74,12 @@ config SOC_CAMERA_OV9740
help
  This is a ov9740 camera driver
 
+config SOC_CAMERA_OV10635
+   tristate ov10635 camera support
+   depends on SOC_CAMERA  I2C
+   help
+ This is an OmniVision ov10635 camera driver
+
 config SOC_CAMERA_RJ54N1
tristate rj54n1cb0c support
depends on SOC_CAMERA  I2C
diff --git a/drivers/media/i2c/soc_camera/Makefile 
b/drivers/media/i2c/soc_camera/Makefile
index d0421fe..f3d3403 100644
--- a/drivers/media/i2c/soc_camera/Makefile
+++ b/drivers/media/i2c/soc_camera/Makefile
@@ -10,5 +10,6 @@ obj-$(CONFIG_SOC_CAMERA_OV6650)   += ov6650.o
 obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o
 obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o
 obj-$(CONFIG_SOC_CAMERA_OV9740)+= ov9740.o
+obj-$(CONFIG_SOC_CAMERA_OV10635)   += ov10635.o
 obj-$(CONFIG_SOC_CAMERA_RJ54N1)+= rj54n1cb0c.o
 obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o
diff --git a/drivers/media/i2c/soc_camera/ov10635.c 
b/drivers/media/i2c/soc_camera/ov10635.c
new file mode 100644
index 000..bf08aae
--- /dev/null
+++ b/drivers/media/i2c/soc_camera/ov10635.c
@@ -0,0 +1,1141 @@
+/*
+ * OmniVision OV10635 Camera Driver
+ *
+ * Copyright (C) 2013 Phil Edworthy
+ * Copyright (C) 2013 Renesas Electronics
+ *
+ * This driver has been tested at QVGA, VGA and 720p, and 1280x800 at up to
+ * 30fps and it should work at any resolution in between and any frame rate
+ * up to 30fps.
+ *
+ * FIXME:
+ *  Horizontal flip (mirroring) does not work correctly. The image is flipped,
+ *  but the colors are wrong.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include linux/delay.h
+#include linux/i2c.h
+#include linux/init.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/v4l2-mediabus.h
+#include linux/videodev2.h
+
+#include media/soc_camera.h
+#include media/v4l2-common.h
+#include media/v4l2-ctrls.h
+
+/* Register definitions */
+#defineOV10635_VFLIP   0x381c
+#define OV10635_VFLIP_ON   (0x3  6)
+#define OV10635_VFLIP_SUBSAMPLE0x1
+#defineOV10635_HMIRROR 0x381d
+#define OV10635_HMIRROR_ON 0x3
+#define OV10635_PID0x300a
+#define OV10635_VER0x300b
+
+/* IDs */
+#define OV10635_VERSION_REG0xa635
+#define OV10635_VERSION(pid, ver)  (((pid)  8) | ((ver)  0xff))
+
+#define OV10635_SENSOR_WIDTH   1312
+#define OV10635_SENSOR_HEIGHT  814
+
+#define OV10635_MAX_WIDTH  1280
+#define OV10635_MAX_HEIGHT 800
+
+struct ov10635_color_format {
+   enum v4l2_mbus_pixelcode code;
+   enum v4l2_colorspace colorspace;
+};
+
+struct ov10635_reg {
+   u16 reg;
+   u8  val;
+};
+
+struct ov10635_priv {
+   struct v4l2_subdev  subdev;
+   struct v4l2_ctrl_handlerhdl;
+   int xvclk;
+   int fps_numerator;
+   int fps_denominator;
+   const struct ov10635_color_format   *cfmt;
+   int width;
+   int height;
+};
+
+/* default register setup */
+static const struct ov10635_reg ov10635_regs_default[] = {
+   { 0x0103, 0x01 }, { 0x301b, 0xff }, { 0x301c, 0xff }, { 0x301a, 0xff },
+   { 0x3011, 0x02 }, /* drive strength reduced to x1 */
+   { 0x6900, 0x0c }, { 0x6901, 0x11 }, { 0x3503, 0x10 }, { 0x3025, 0x03 },
+   { 0x3005, 0x20 }, { 0x3006, 0x91 }, { 0x3600, 0x74 }, { 0x3601, 0x2b },
+   { 0x3612, 0x00 }, { 0x3611, 0x67 }, { 0x3633, 0xca }, { 0x3602, 0x2f },
+   { 0x3603, 0x00 }, { 0x3630, 0x28 }, { 0x3631, 0x16 }, { 0x3714, 0x10 },
+   { 0x371d, 0x01 }, { 0x4300, 0x38 }, { 0x3007, 0x01 }, { 0x3024, 0x01 },
+   { 0x3020, 0x0b }, { 0x3702, 0x0d }, { 0x3703, 0x20 }, { 0x3704, 0x15 },
+   { 0x3709, 0xa8 }, { 0x370c, 0xc7 }, { 0x370d, 0x80 }, { 0x3712, 0x00 },
+   { 0x3713, 0x20 }, { 0x3715, 0x04 }, { 0x381d, 0x40 }, { 0x381c, 0x00 },
+   { 0x3822, 0x50 }, { 0x3824, 0x10 }, { 0x3815, 0x8c }, { 0x3804, 0x05

Re: [PATCH v5] V4L2: soc_camera: Renesas R-Car VIN driver

2013-05-28 Thread phil . edworthy
Hi Sergei, Vladimir,

  Oops, the comments about the captured image contents are my fault.
  However, the unhandled irq after stopping capture is still an issue.
 
 Thanks for letting us know.
The good news is that your driver works fine. 

The problem I saw only occurs when your patches were integrated into 
Simon's next branch (which was renesas-next-20130515v2+v3.10-rc1), so I 
guess something in the rc1 caused problems. Once I tested the driver 
against 3.9, it works fine.

Thanks
Phil
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] V4L2: soc_camera: Renesas R-Car VIN driver

2013-05-21 Thread phil . edworthy
Hi Sergei, Vladimir,

  Subject: [PATCH v5] V4L2: soc_camera: Renesas R-Car VIN driver
 
  From: Vladimir Barinov vladimir.bari...@cogentembedded.com
 
  Add Renesas R-Car VIN (Video In) V4L2 driver.
 
  Based on the patch by Phil Edworthy phil.edwor...@renesas.com.
 
  I've seen old patches that add VIN to the Marzen board, do you have an
  updated version?
 
 The last version of that patchset is 4, here it is archived:
 
 http://marc.info/?l=linux-shm=136865481429756
 http://marc.info/?l=linux-shm=136865499029807
 http://marc.info/?l=linux-shm=136865509129843
 http://marc.info/?l=linux-shm=136865520029900

First of all, thank you for your work on this driver.

I have tried your patches on the Marzen board using an Expansion Board 
with an OmniVision 10635 camera (progressive BT656), using an out-of-tree 
driver. There appears to be an issue with the interrupt handling compared 
to my original driver.

Using a simple test app I wrote, I get an unhandled irq if the app does 
some work after stopping the capture. In this case, the work after capture 
is storing a captured image to a file. As a dirty hack to see what's 
actually being captured, I just commented out the code that checks the 
interrupt status:
//  if (!int_status)
//  goto done;
This allows me to save the captured image. However, this shows about 
2/3rds valid picture data (though it looks vertically shifted), with the 
rest black. Also, a couple of other lines are black.

I realise that you don't have the Marzen Expansion Board  don't have an 
ov10635 camera. However, unfortunately, I don't have much time that I can 
spend on this. Do any of the boards you have use a progressive camera?

Regards
Phil
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] V4L2: soc_camera: Renesas R-Car VIN driver

2013-05-21 Thread phil . edworthy
Hi Sergei, Vladimir,

   Subject: [PATCH v5] V4L2: soc_camera: Renesas R-Car VIN driver
  
   From: Vladimir Barinov vladimir.bari...@cogentembedded.com
  
   Add Renesas R-Car VIN (Video In) V4L2 driver.
  
   Based on the patch by Phil Edworthy phil.edwor...@renesas.com.
  
   I've seen old patches that add VIN to the Marzen board, do you have 
an
   updated version?
  
  The last version of that patchset is 4, here it is archived:
  
  http://marc.info/?l=linux-shm=136865481429756
  http://marc.info/?l=linux-shm=136865499029807
  http://marc.info/?l=linux-shm=136865509129843
  http://marc.info/?l=linux-shm=136865520029900

 First of all, thank you for your work on this driver.
 
 I have tried your patches on the Marzen board using an Expansion 
 Board with an OmniVision 10635 camera (progressive BT656), using an 
 out-of-tree driver. There appears to be an issue with the interrupt 
 handling compared to my original driver.
 
 Using a simple test app I wrote, I get an unhandled irq if the app 
 does some work after stopping the capture. In this case, the work 
 after capture is storing a captured image to a file. As a dirty hack
 to see what's actually being captured, I just commented out the code
 that checks the interrupt status:
 // if (!int_status)
 //  goto done;
 This allows me to save the captured image. However, this shows about
 2/3rds valid picture data (though it looks vertically shifted), with
 the rest black. Also, a couple of other lines are black.
 
 I realise that you don't have the Marzen Expansion Board  don't 
 have an ov10635 camera. However, unfortunately, I don't have much 
 time that I can spend on this. Do any of the boards you have use a 
 progressive camera?

Oops, the comments about the captured image contents are my fault. 
However, the unhandled irq after stopping capture is still an issue.

Regards
Phil

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] V4L2: soc_camera: Renesas R-Car VIN driver

2013-05-20 Thread phil . edworthy
Hi Sergei, Vladimir,

 Subject: [PATCH v5] V4L2: soc_camera: Renesas R-Car VIN driver
 
 From: Vladimir Barinov vladimir.bari...@cogentembedded.com
 
 Add Renesas R-Car VIN (Video In) V4L2 driver.
 
 Based on the patch by Phil Edworthy phil.edwor...@renesas.com.

I've seen old patches that add VIN to the Marzen board, do you have an 
updated version?

Thanks
Phil
--
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] soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format

2013-05-01 Thread phil . edworthy
Hi Guennadi,

 From: Guennadi Liakhovetski g.liakhovet...@gmx.de
 To: phil.edwor...@renesas.com, 
 Cc: linux-media@vger.kernel.org, Mauro Carvalho Chehab 
mche...@redhat.com
 Date: 26/04/2013 22:00
 Subject: Re: [PATCH] soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format
 
 Hi Phil
 
 On Fri, 26 Apr 2013, phil.edwor...@renesas.com wrote:
 
  Hi Guennadi,
  
  snip
   Wow, what kind of host can pack two 10-bit samples into 3 
bytes 
  and 
  write 
   3-byte pixels to memory?
  I think I might have misunderstood how this is used. From my 
  understanding, the MBUS formats are used to describe the 
hardware 
  interfaces to cameras, i.e. 2 samples of 10 bits. I guess that 
the 
  
fourcc 
  field also determines what v4l2 format is required to capture 
  this. 
 
 No, not quite. This table describes default pass-through 
capture 
  of 
 video data on a media bus to memory. E.g. the first entry in the 

  table 
 means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the 

  bus, 
you 
 sample 8 bits at a time, and store the samples 1-to-1 into RAM, 
you 
  get 
 the V4L2_PIX_FMT_YUYV format in your buffer. It can also 
describe 
  some 
 standard operations with the sampled data, like swapping the 
order, 
 filling missing high bits (e.g. if you sample 10 bits but store 
16 
  bits 
 per sample with high 6 bits nullified). The table also specifies 

  which 
 bits are used for padding in the original data, e.g. 
 V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has 
SOC_MBUS_PACKING_2X8_PADLO, 
whereas 
 V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has 
SOC_MBUS_PACKING_2X8_PADHI, 
  which 

 means, that out of 16 bits of data, that you get when you sample 
an 
8-bit 
 bus twice, either low or high 6 bits are invalid and should be 
discarded.

Ok, I see. However, is it necessary to provide a default 
pass-through 
  v4l2 
format?
   
   No, it's not. If no (soc-camera) host camera driver is willing to 
use 
  this 
   pass-through conversion, then it's not required.
  Ok, I'll look at that when I get a moment!
  
I can't see a suitable v4l2 format! For the hardware I have been 
working on, there is always the option of converting the data to 
  another 
format, so this is not really needed. I doubt that it makes sense 
to 
  add 
yet another v4l2 format for userspace, when typical uses would 
involve 
  the 
host hardware converting the format to something else, e.g. 
V4L2_PIX_FMT_RGB32.
   
   Up to you, really. If you don't need this default conversion, don't 
add 
   it.
  Ok, it seems like it would be a bad idea to provide a default 
conversion 
  that my not be supported by other hosts.
 
 Right, that table collects natural conversions, mostly just 
 straightforward bus-to-buffer. In your case of 2 10-bit samples such a 
 natural transfer would produce one 16-bit word per sample, of which only 

 10 bits are useful information. So, your 20 bits of pixel data would be 
 located in bits 25:16 and 9:0 of each 32-bit (long)word. I don't think 
 there is a fourcc code, describing such a buffer layout... It probably 
 would be useless without a special conversion software.

So, if there is not a natural conversion  I do not populate the fourcc 
field, doesn't the other information in the soc_mbus_lookup struct becomes 
moot? Nothing can allocate a buffer for it, so nothing should be using the 
bits_per_sample, packing or order fields. Similarly, nothing should call 
soc_mbus_samples_per_pixel() with this format. By extension, there is no 
need to add SOC_MBUS_PACKING_2X10_PADHI to soc_mbus_bytes_per_line().

Since V4L2_MBUS_FMT_YUYV10_2X10 already exists without the above 
additional info, I guess there is no need for this patch at all.

Thanks
Phil
--
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] soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format

2013-04-26 Thread phil . edworthy
Hi Guennadi,

snip
 Wow, what kind of host can pack two 10-bit samples into 3 bytes 
and 
write 
 3-byte pixels to memory?
I think I might have misunderstood how this is used. From my 
understanding, the MBUS formats are used to describe the hardware 
interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the 

  fourcc 
field also determines what v4l2 format is required to capture 
this. 
   
   No, not quite. This table describes default pass-through capture 
of 
   video data on a media bus to memory. E.g. the first entry in the 
table 
   means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the 
bus, 
  you 
   sample 8 bits at a time, and store the samples 1-to-1 into RAM, you 
get 
   the V4L2_PIX_FMT_YUYV format in your buffer. It can also describe 
some 
   standard operations with the sampled data, like swapping the order, 
   filling missing high bits (e.g. if you sample 10 bits but store 16 
bits 
   per sample with high 6 bits nullified). The table also specifies 
which 
   bits are used for padding in the original data, e.g. 
   V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has SOC_MBUS_PACKING_2X8_PADLO, 
  whereas 
   V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has SOC_MBUS_PACKING_2X8_PADHI, 
which 
  
   means, that out of 16 bits of data, that you get when you sample an 
  8-bit 
   bus twice, either low or high 6 bits are invalid and should be 
  discarded.
  
  Ok, I see. However, is it necessary to provide a default pass-through 
v4l2 
  format?
 
 No, it's not. If no (soc-camera) host camera driver is willing to use 
this 
 pass-through conversion, then it's not required.
Ok, I'll look at that when I get a moment!

  I can't see a suitable v4l2 format! For the hardware I have been 
  working on, there is always the option of converting the data to 
another 
  format, so this is not really needed. I doubt that it makes sense to 
add 
  yet another v4l2 format for userspace, when typical uses would involve 
the 
  host hardware converting the format to something else, e.g. 
  V4L2_PIX_FMT_RGB32.
 
 Up to you, really. If you don't need this default conversion, don't add 
 it.
Ok, it seems like it would be a bad idea to provide a default conversion 
that my not be supported by other hosts.

However, I am not sure how the two relate to each other. How does 
the 
above code imply 3 bytes?
   
   Not the above code, but your entry in the soc_mbus_bytes_per_line() 
   function below, where you multiply width * 3.
  
  It looks like hosts use soc_mbus_bytes_per_line() to report the size 
of 
  video buffers needed. Shouldn't the hosts report the buffer metrics 
for 
  the v4l2 format, since that is what will be output? What has this to 
do 
  with the MBUS specifics?
 
 struct soc_mbus_pixelfmt describes a conversion from an MBUS code to a 
 pixel format in memory. Camera host drivers call that function with a 
 _suitable_ conversion descriptor (either a standard or a special one) 
and 
 the function calculates the number of bytes.
Right, I think I understand!

Thanks
Phil
--
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] soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format

2013-04-25 Thread phil . edworthy
Hi Guennadi,

Thanks for the review!

 On Wed, 17 Apr 2013, Phil Edworthy wrote:
 
  The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to 
mediabus, so
  this patch just adds SoC camera support.
  
  Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
  ---
   drivers/media/platform/soc_camera/soc_mediabus.c |   15 
+++
   include/media/soc_mediabus.h |3 +++
   2 files changed, 18 insertions(+), 0 deletions(-)
  
  diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
 drivers/media/platform/soc_camera/soc_mediabus.c
  index 7569e77..be47d41 100644
  --- a/drivers/media/platform/soc_camera/soc_mediabus.c
  +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
  @@ -57,6 +57,15 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 .layout = SOC_MBUS_LAYOUT_PACKED,
  },
   }, {
  +   .code = V4L2_MBUS_FMT_YUYV10_2X10,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_YUYV,
  +  .name = YUYV,
  +  .bits_per_sample   = 10,
  +  .packing  = SOC_MBUS_PACKING_2X10_PADHI,
 
 Wow, what kind of host can pack two 10-bit samples into 3 bytes and 
write 
 3-byte pixels to memory?
I think I might have misunderstood how this is used. From my 
understanding, the MBUS formats are used to describe the hardware 
interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the fourcc 
field also determines what v4l2 format is required to capture this. 
However, I am not sure how the two relate to each other. How does the 
above code imply 3 bytes?

  +  .order = SOC_MBUS_ORDER_LE,
  +   },
  +}, {
  .code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
  .fmt = {
 .fourcc = V4L2_PIX_FMT_RGB555,
  @@ -403,6 +412,10 @@ int soc_mbus_samples_per_pixel(const struct 
 soc_mbus_pixelfmt *mf,
 *numerator = 2;
 *denominator = 1;
 return 0;
  +   case SOC_MBUS_PACKING_2X10_PADHI:
  +  *numerator = 3;
  +  *denominator = 1;
 
 Why 3? it's 2 samples per pixel, right? Should be *numerator = 2 above?
Not sure why I thought it should be 3, I think I had it in my head that 
this was the number of bytes needed per pixel. Clearly this is not the 
case!

  +  return 0;
  case SOC_MBUS_PACKING_1_5X8:
 *numerator = 3;
 *denominator = 2;
  @@ -428,6 +441,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
 struct soc_mbus_pixelfmt *mf)
  case SOC_MBUS_PACKING_2X8_PADLO:
  case SOC_MBUS_PACKING_EXTEND16:
 return width * 2;
  +   case SOC_MBUS_PACKING_2X10_PADHI:
  +  return width * 3;
  case SOC_MBUS_PACKING_1_5X8:
 return width * 3 / 2;
  case SOC_MBUS_PACKING_VARIABLE:
  diff --git a/include/media/soc_mediabus.h 
b/include/media/soc_mediabus.h
  index d33f6d0..b131a47 100644
  --- a/include/media/soc_mediabus.h
  +++ b/include/media/soc_mediabus.h
  @@ -21,6 +21,8 @@
* @SOC_MBUS_PACKING_2X8_PADHI:   16 bits transferred in 2 8-bit 
 samples, in the
*possibly incomplete byte high bits are padding
* @SOC_MBUS_PACKING_2X8_PADLO:   as above, but low bits are padding
  + * @SOC_MBUS_PACKING_2X10_PADHI:20 bits transferred in 2 10-bit 
 samples. The
 
 A TAB is missing after :?
Ok.
 
  + *high bits are padding
* @SOC_MBUS_PACKING_EXTEND16:   sample width (e.g., 10 bits) has
 to be extended
*to 16 bits
* @SOC_MBUS_PACKING_VARIABLE:   compressed formats with variable 
packing
  @@ -33,6 +35,7 @@ enum soc_mbus_packing {
  SOC_MBUS_PACKING_NONE,
  SOC_MBUS_PACKING_2X8_PADHI,
  SOC_MBUS_PACKING_2X8_PADLO,
  +   SOC_MBUS_PACKING_2X10_PADHI,
  SOC_MBUS_PACKING_EXTEND16,
  SOC_MBUS_PACKING_VARIABLE,
  SOC_MBUS_PACKING_1_5X8,
  -- 
  1.7.5.4

Thanks
Phil
--
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] soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format

2013-04-25 Thread phil . edworthy
Hi Guennadi,

   On Wed, 17 Apr 2013, Phil Edworthy wrote:
   
The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to 
  mediabus, so
this patch just adds SoC camera support.

Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
 drivers/media/platform/soc_camera/soc_mediabus.c |   15 
  +++
 include/media/soc_mediabus.h |3 +++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
   drivers/media/platform/soc_camera/soc_mediabus.c
index 7569e77..be47d41 100644
--- a/drivers/media/platform/soc_camera/soc_mediabus.c
+++ b/drivers/media/platform/soc_camera/soc_mediabus.c
@@ -57,6 +57,15 @@ static const struct soc_mbus_lookup mbus_fmt[] 
= {
   .layout = SOC_MBUS_LAYOUT_PACKED,
},
 }, {
+   .code = V4L2_MBUS_FMT_YUYV10_2X10,
+   .fmt = {
+  .fourcc = V4L2_PIX_FMT_YUYV,
+  .name = YUYV,
+  .bits_per_sample   = 10,
+  .packing  = SOC_MBUS_PACKING_2X10_PADHI,
   
   Wow, what kind of host can pack two 10-bit samples into 3 bytes and 
  write 
   3-byte pixels to memory?
  I think I might have misunderstood how this is used. From my 
  understanding, the MBUS formats are used to describe the hardware 
  interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the 
fourcc 
  field also determines what v4l2 format is required to capture this. 
 
 No, not quite. This table describes default pass-through capture of 
 video data on a media bus to memory. E.g. the first entry in the table 
 means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the bus, 
you 
 sample 8 bits at a time, and store the samples 1-to-1 into RAM, you get 
 the V4L2_PIX_FMT_YUYV format in your buffer. It can also describe some 
 standard operations with the sampled data, like swapping the order, 
 filling missing high bits (e.g. if you sample 10 bits but store 16 bits 
 per sample with high 6 bits nullified). The table also specifies which 
 bits are used for padding in the original data, e.g. 
 V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has SOC_MBUS_PACKING_2X8_PADLO, 
whereas 
 V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has SOC_MBUS_PACKING_2X8_PADHI, which 

 means, that out of 16 bits of data, that you get when you sample an 
8-bit 
 bus twice, either low or high 6 bits are invalid and should be 
discarded.

Ok, I see. However, is it necessary to provide a default pass-through v4l2 
format? I can't see a suitable v4l2 format! For the hardware I have been 
working on, there is always the option of converting the data to another 
format, so this is not really needed. I doubt that it makes sense to add 
yet another v4l2 format for userspace, when typical uses would involve the 
host hardware converting the format to something else, e.g. 
V4L2_PIX_FMT_RGB32.

  However, I am not sure how the two relate to each other. How does the 
  above code imply 3 bytes?
 
 Not the above code, but your entry in the soc_mbus_bytes_per_line() 
 function below, where you multiply width * 3.

It looks like hosts use soc_mbus_bytes_per_line() to report the size of 
video buffers needed. Shouldn't the hosts report the buffer metrics for 
the v4l2 format, since that is what will be output? What has this to do 
with the MBUS specifics?

+  .order = SOC_MBUS_ORDER_LE,
+   },
+}, {
.code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
.fmt = {
   .fourcc = V4L2_PIX_FMT_RGB555,
@@ -403,6 +412,10 @@ int soc_mbus_samples_per_pixel(const struct 
   soc_mbus_pixelfmt *mf,
   *numerator = 2;
   *denominator = 1;
   return 0;
+   case SOC_MBUS_PACKING_2X10_PADHI:
+  *numerator = 3;
+  *denominator = 1;
   
   Why 3? it's 2 samples per pixel, right? Should be *numerator = 2 
above?
  Not sure why I thought it should be 3, I think I had it in my head 
that 
  this was the number of bytes needed per pixel. Clearly this is not the 

  case!
  
+  return 0;
case SOC_MBUS_PACKING_1_5X8:
   *numerator = 3;
   *denominator = 2;
@@ -428,6 +441,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
   struct soc_mbus_pixelfmt *mf)
case SOC_MBUS_PACKING_2X8_PADLO:
case SOC_MBUS_PACKING_EXTEND16:
   return width * 2;
+   case SOC_MBUS_PACKING_2X10_PADHI:
+  return width * 3;
case SOC_MBUS_PACKING_1_5X8:
   return width * 3 / 2;
case SOC_MBUS_PACKING_VARIABLE:
diff --git a/include/media/soc_mediabus.h 
  b/include/media/soc_mediabus.h
index d33f6d0..b131a47 100644
--- a/include/media/soc_mediabus.h
+++ b/include/media/soc_mediabus.h
@@ -21,6 +21,8 @@
  * @SOC_MBUS_PACKING_2X8_PADHI:   16 bits transferred in 2 8-bit 
   samples, in the
  *possibly incomplete byte high bits are padding

[PATCH] soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format

2013-04-17 Thread Phil Edworthy
The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to mediabus, so
this patch just adds SoC camera support.

Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
 drivers/media/platform/soc_camera/soc_mediabus.c |   15 +++
 include/media/soc_mediabus.h |3 +++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c 
b/drivers/media/platform/soc_camera/soc_mediabus.c
index 7569e77..be47d41 100644
--- a/drivers/media/platform/soc_camera/soc_mediabus.c
+++ b/drivers/media/platform/soc_camera/soc_mediabus.c
@@ -57,6 +57,15 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
.layout = SOC_MBUS_LAYOUT_PACKED,
},
 }, {
+   .code = V4L2_MBUS_FMT_YUYV10_2X10,
+   .fmt = {
+   .fourcc = V4L2_PIX_FMT_YUYV,
+   .name   = YUYV,
+   .bits_per_sample= 10,
+   .packing= SOC_MBUS_PACKING_2X10_PADHI,
+   .order  = SOC_MBUS_ORDER_LE,
+   },
+}, {
.code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
.fmt = {
.fourcc = V4L2_PIX_FMT_RGB555,
@@ -403,6 +412,10 @@ int soc_mbus_samples_per_pixel(const struct 
soc_mbus_pixelfmt *mf,
*numerator = 2;
*denominator = 1;
return 0;
+   case SOC_MBUS_PACKING_2X10_PADHI:
+   *numerator = 3;
+   *denominator = 1;
+   return 0;
case SOC_MBUS_PACKING_1_5X8:
*numerator = 3;
*denominator = 2;
@@ -428,6 +441,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const struct 
soc_mbus_pixelfmt *mf)
case SOC_MBUS_PACKING_2X8_PADLO:
case SOC_MBUS_PACKING_EXTEND16:
return width * 2;
+   case SOC_MBUS_PACKING_2X10_PADHI:
+   return width * 3;
case SOC_MBUS_PACKING_1_5X8:
return width * 3 / 2;
case SOC_MBUS_PACKING_VARIABLE:
diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h
index d33f6d0..b131a47 100644
--- a/include/media/soc_mediabus.h
+++ b/include/media/soc_mediabus.h
@@ -21,6 +21,8 @@
  * @SOC_MBUS_PACKING_2X8_PADHI:16 bits transferred in 2 8-bit samples, 
in the
  * possibly incomplete byte high bits are padding
  * @SOC_MBUS_PACKING_2X8_PADLO:as above, but low bits are padding
+ * @SOC_MBUS_PACKING_2X10_PADHI:20 bits transferred in 2 10-bit samples. The
+ * high bits are padding
  * @SOC_MBUS_PACKING_EXTEND16: sample width (e.g., 10 bits) has to be extended
  * to 16 bits
  * @SOC_MBUS_PACKING_VARIABLE: compressed formats with variable packing
@@ -33,6 +35,7 @@ enum soc_mbus_packing {
SOC_MBUS_PACKING_NONE,
SOC_MBUS_PACKING_2X8_PADHI,
SOC_MBUS_PACKING_2X8_PADLO,
+   SOC_MBUS_PACKING_2X10_PADHI,
SOC_MBUS_PACKING_EXTEND16,
SOC_MBUS_PACKING_VARIABLE,
SOC_MBUS_PACKING_1_5X8,
-- 
1.7.5.4

--
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] soc_camera: Add RGB666 RGB888 formats

2013-03-18 Thread Phil Edworthy
Based on work done by Katsuya Matsubara.

Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
v2:
 - Added documentation.
 - Added SOCAM_DATAWIDTH_12 definition.

 Documentation/DocBook/media/v4l/subdev-formats.xml |  206 +++-
 Documentation/DocBook/media_api.tmpl   |1 +
 drivers/media/platform/soc_camera/soc_mediabus.c   |   42 
 include/media/soc_camera.h |7 +-
 include/media/soc_mediabus.h   |3 +
 include/uapi/linux/v4l2-mediabus.h |6 +-
 6 files changed, 253 insertions(+), 12 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml 
b/Documentation/DocBook/media/v4l/subdev-formats.xml
index cc51372..adc6198 100644
--- a/Documentation/DocBook/media/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
@@ -93,19 +93,35 @@
 
   table pgwide=0 frame=none id=v4l2-mbus-pixelcode-rgb
titleRGB formats/title
-   tgroup cols=11
+   tgroup cols=27
  colspec colname=id align=left /
  colspec colname=code align=center/
  colspec colname=bit /
- colspec colnum=4 colname=b07 align=center /
- colspec colnum=5 colname=b06 align=center /
- colspec colnum=6 colname=b05 align=center /
- colspec colnum=7 colname=b04 align=center /
- colspec colnum=8 colname=b03 align=center /
- colspec colnum=9 colname=b02 align=center /
- colspec colnum=10 colname=b01 align=center /
- colspec colnum=11 colname=b00 align=center /
- spanspec namest=b07 nameend=b00 spanname=b0 /
+ colspec colnum=4 colname=b23 align=center /
+ colspec colnum=5 colname=b22 align=center /
+ colspec colnum=6 colname=b21 align=center /
+ colspec colnum=7 colname=b20 align=center /
+ colspec colnum=8 colname=b19 align=center /
+ colspec colnum=9 colname=b18 align=center /
+ colspec colnum=10 colname=b17 align=center /
+ colspec colnum=11 colname=b16 align=center /
+ colspec colnum=12 colname=b15 align=center /
+ colspec colnum=13 colname=b14 align=center /
+ colspec colnum=14 colname=b13 align=center /
+ colspec colnum=15 colname=b12 align=center /
+ colspec colnum=16 colname=b11 align=center /
+ colspec colnum=17 colname=b10 align=center /
+ colspec colnum=18 colname=b09 align=center /
+ colspec colnum=19 colname=b08 align=center /
+ colspec colnum=20 colname=b07 align=center /
+ colspec colnum=21 colname=b06 align=center /
+ colspec colnum=22 colname=b05 align=center /
+ colspec colnum=23 colname=b04 align=center /
+ colspec colnum=24 colname=b03 align=center /
+ colspec colnum=25 colname=b02 align=center /
+ colspec colnum=26 colname=b01 align=center /
+ colspec colnum=27 colname=b00 align=center /
+ spanspec namest=b23 nameend=b00 spanname=b0 /
  thead
row
  entryIdentifier/entry
@@ -117,6 +133,22 @@
  entry/entry
  entry/entry
  entryBit/entry
+ entry23/entry
+ entry22/entry
+ entry21/entry
+ entry20/entry
+ entry19/entry
+ entry18/entry
+ entry17/entry
+ entry16/entry
+ entry15/entry
+ entry14/entry
+ entry13/entry
+ entry12/entry
+ entry11/entry
+ entry10/entry
+ entry9/entry
+ entry8/entry
  entry7/entry
  entry6/entry
  entry5/entry
@@ -132,6 +164,7 @@
  entryV4L2_MBUS_FMT_RGB444_2X8_PADHI_BE/entry
  entry0x1001/entry
  entry/entry
+ dash-ent-16;
  entry0/entry
  entry0/entry
  entry0/entry
@@ -145,6 +178,7 @@
  entry/entry
  entry/entry
  entry/entry
+ dash-ent-16;
  entrygsubscript3/subscript/entry
  entrygsubscript2/subscript/entry
  entrygsubscript1/subscript/entry
@@ -158,6 +192,7 @@
  entryV4L2_MBUS_FMT_RGB444_2X8_PADHI_LE/entry
  entry0x1002/entry
  entry/entry
+ dash-ent-16;
  entrygsubscript3/subscript/entry
  entrygsubscript2/subscript/entry
  entrygsubscript1/subscript/entry
@@ -171,6 +206,7 @@
  entry/entry
  entry/entry
  entry/entry
+ dash-ent-16;
  entry0/entry
  entry0/entry
  entry0/entry
@@ -184,6 +220,7 @@
  entryV4L2_MBUS_FMT_RGB555_2X8_PADHI_BE/entry
  entry0x1003/entry
  entry/entry
+ dash-ent-16;
  entry0/entry
  entryrsubscript4/subscript/entry

Re: [PATCH] soc_camera: Add RGB666 RGB888 formats

2013-03-14 Thread phil . edworthy
Hi Guennadi,

Thanks for the review, I'll try to find some time to fix  send a new 
version.

Phil

 On Thu, 14 Feb 2013, Phil Edworthy wrote:
 
  Based on work done by Katsuya Matsubara.
  
  Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
 
 Looks mostly good to me, but please also provide format descriptions for 

 Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml, also see a couple 

 of minor notes below
 
  ---
   drivers/media/platform/soc_camera/soc_mediabus.c |   42 +
 +
   include/media/soc_camera.h   |6 +++-
   include/media/soc_mediabus.h |3 ++
   include/uapi/linux/v4l2-mediabus.h   |6 +++-
   4 files changed, 55 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
 drivers/media/platform/soc_camera/soc_mediabus.c
  index a397812..d8acfd3 100644
  --- a/drivers/media/platform/soc_camera/soc_mediabus.c
  +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
  @@ -97,6 +97,42 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 .layout = SOC_MBUS_LAYOUT_PACKED,
  },
   }, {
  +   .code = V4L2_MBUS_FMT_RGB666_1X18,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_RGB32,
  +  .name = RGB666/32bpp,
  +  .bits_per_sample   = 18,
  +  .packing  = SOC_MBUS_PACKING_EXTEND32,
  +  .order = SOC_MBUS_ORDER_LE,
  +   },
  +}, {
  +   .code = V4L2_MBUS_FMT_RGB888_1X24,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_RGB32,
  +  .name = RGB888/32bpp,
  +  .bits_per_sample   = 24,
  +  .packing  = SOC_MBUS_PACKING_EXTEND32,
  +  .order = SOC_MBUS_ORDER_LE,
  +   },
  +}, {
  +   .code = V4L2_MBUS_FMT_RGB888_2X12_BE,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_RGB32,
  +  .name = RGB888/32bpp,
  +  .bits_per_sample   = 12,
  +  .packing  = SOC_MBUS_PACKING_EXTEND32,
  +  .order = SOC_MBUS_ORDER_BE,
  +   },
  +}, {
  +   .code = V4L2_MBUS_FMT_RGB888_2X12_LE,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_RGB32,
  +  .name = RGB888/32bpp,
  +  .bits_per_sample   = 12,
  +  .packing  = SOC_MBUS_PACKING_EXTEND32,
  +  .order = SOC_MBUS_ORDER_LE,
  +   },
  +}, {
  .code = V4L2_MBUS_FMT_SBGGR8_1X8,
  .fmt = {
 .fourcc = V4L2_PIX_FMT_SBGGR8,
  @@ -358,6 +394,10 @@ int soc_mbus_samples_per_pixel(const struct 
 soc_mbus_pixelfmt *mf,
 *numerator = 1;
 *denominator = 1;
 return 0;
  +   case SOC_MBUS_PACKING_EXTEND32:
  +  *numerator = 1;
  +  *denominator = 1;
  +  return 0;
  case SOC_MBUS_PACKING_2X8_PADHI:
  case SOC_MBUS_PACKING_2X8_PADLO:
 *numerator = 2;
  @@ -395,6 +435,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
 struct soc_mbus_pixelfmt *mf)
 return width * 3 / 2;
  case SOC_MBUS_PACKING_VARIABLE:
 return 0;
  +   case SOC_MBUS_PACKING_EXTEND32:
  +  return width * 4;
  }
  return -EINVAL;
   }
  diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
  index 6442edc..c820be2 100644
  --- a/include/media/soc_camera.h
  +++ b/include/media/soc_camera.h
  @@ -231,10 +231,14 @@ struct soc_camera_sense {
   #define SOCAM_DATAWIDTH_10   SOCAM_DATAWIDTH(10)
 
 Didn't you forget to define SOCAM_DATAWIDTH_12 here?
 
   #define SOCAM_DATAWIDTH_15   SOCAM_DATAWIDTH(15)
   #define SOCAM_DATAWIDTH_16   SOCAM_DATAWIDTH(16)
  +#define SOCAM_DATAWIDTH_18   SOCAM_DATAWIDTH(18)
  +#define SOCAM_DATAWIDTH_24   SOCAM_DATAWIDTH(24)
  
   #define SOCAM_DATAWIDTH_MASK (SOCAM_DATAWIDTH_4 | SOCAM_DATAWIDTH_8 | 
\
  SOCAM_DATAWIDTH_9 | SOCAM_DATAWIDTH_10 | \
  -   SOCAM_DATAWIDTH_15 | SOCAM_DATAWIDTH_16)
  +   SOCAM_DATAWIDTH_12 | SOCAM_DATAWIDTH_15 | \
  +   SOCAM_DATAWIDTH_16 | SOCAM_DATAWIDTH_18 | \
  +   SOCAM_DATAWIDTH_24)
  
   static inline void soc_camera_limit_side(int *start, int *length,
 unsigned int start_min,
  diff --git a/include/media/soc_mediabus.h 
b/include/media/soc_mediabus.h
  index 0dc6f46..eea98d1 100644
  --- a/include/media/soc_mediabus.h
  +++ b/include/media/soc_mediabus.h
  @@ -26,6 +26,8 @@
* @SOC_MBUS_PACKING_VARIABLE:   compressed formats with variable 
packing
* @SOC_MBUS_PACKING_1_5X8:   used for packed YUV 4:2:0 formats, 
where 4
*pixels occupy 6 bytes in RAM
  + * @SOC_MBUS_PACKING_EXTEND32:  sample width (e.g., 24 bits) has 
 to be extended
 
 Please, use a TAB above
 
 Thanks
 Guennadi
 
  + *to 32 bits
*/
   enum soc_mbus_packing {
  SOC_MBUS_PACKING_NONE,
  @@ -34,6 +36,7 @@ enum soc_mbus_packing {
  SOC_MBUS_PACKING_EXTEND16,
  SOC_MBUS_PACKING_VARIABLE,
  SOC_MBUS_PACKING_1_5X8,
  +   SOC_MBUS_PACKING_EXTEND32,
   };
  
   /**
  diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/
 linux/v4l2

Re: [PATCH] soc_camera: Add RGB666 RGB888 formats

2013-03-14 Thread phil . edworthy
Hi Guennadi,

 From: Guennadi Liakhovetski g.liakhovet...@gmx.de
 To: Phil Edworthy phil.edwor...@renesas.com, 
 Cc: Mauro Carvalho Chehab mche...@redhat.com, 
linux-media@vger.kernel.org
 Date: 08/03/2013 13:30
 Subject: Re: [PATCH] soc_camera: Add RGB666  RGB888 formats
 
 Hi Phil
 
 On Thu, 14 Feb 2013, Phil Edworthy wrote:
 
  Based on work done by Katsuya Matsubara.
  
  Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
 
 Looks mostly good to me, but please also provide format descriptions for 

 Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml, also see a couple 

 of minor notes below

I had a look at the doc you pointed to, but it appears to only describe 
formats that can be accessed by userspace. I take it you meant 
Documentation/DocBook/media/v4l/subdev-formats.xml?

  ---
   drivers/media/platform/soc_camera/soc_mediabus.c |   42 +
 +
   include/media/soc_camera.h   |6 +++-
   include/media/soc_mediabus.h |3 ++
   include/uapi/linux/v4l2-mediabus.h   |6 +++-
   4 files changed, 55 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
 drivers/media/platform/soc_camera/soc_mediabus.c
  index a397812..d8acfd3 100644
  --- a/drivers/media/platform/soc_camera/soc_mediabus.c
  +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
  @@ -97,6 +97,42 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 .layout = SOC_MBUS_LAYOUT_PACKED,
  },
   }, {
  +   .code = V4L2_MBUS_FMT_RGB666_1X18,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_RGB32,
  +  .name = RGB666/32bpp,
  +  .bits_per_sample   = 18,
  +  .packing  = SOC_MBUS_PACKING_EXTEND32,
  +  .order = SOC_MBUS_ORDER_LE,
  +   },
  +}, {
  +   .code = V4L2_MBUS_FMT_RGB888_1X24,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_RGB32,
  +  .name = RGB888/32bpp,
  +  .bits_per_sample   = 24,
  +  .packing  = SOC_MBUS_PACKING_EXTEND32,
  +  .order = SOC_MBUS_ORDER_LE,
  +   },
  +}, {
  +   .code = V4L2_MBUS_FMT_RGB888_2X12_BE,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_RGB32,
  +  .name = RGB888/32bpp,
  +  .bits_per_sample   = 12,
  +  .packing  = SOC_MBUS_PACKING_EXTEND32,
  +  .order = SOC_MBUS_ORDER_BE,
  +   },
  +}, {
  +   .code = V4L2_MBUS_FMT_RGB888_2X12_LE,
  +   .fmt = {
  +  .fourcc = V4L2_PIX_FMT_RGB32,
  +  .name = RGB888/32bpp,
  +  .bits_per_sample   = 12,
  +  .packing  = SOC_MBUS_PACKING_EXTEND32,
  +  .order = SOC_MBUS_ORDER_LE,
  +   },
  +}, {
  .code = V4L2_MBUS_FMT_SBGGR8_1X8,
  .fmt = {
 .fourcc = V4L2_PIX_FMT_SBGGR8,
  @@ -358,6 +394,10 @@ int soc_mbus_samples_per_pixel(const struct 
 soc_mbus_pixelfmt *mf,
 *numerator = 1;
 *denominator = 1;
 return 0;
  +   case SOC_MBUS_PACKING_EXTEND32:
  +  *numerator = 1;
  +  *denominator = 1;
  +  return 0;
  case SOC_MBUS_PACKING_2X8_PADHI:
  case SOC_MBUS_PACKING_2X8_PADLO:
 *numerator = 2;
  @@ -395,6 +435,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
 struct soc_mbus_pixelfmt *mf)
 return width * 3 / 2;
  case SOC_MBUS_PACKING_VARIABLE:
 return 0;
  +   case SOC_MBUS_PACKING_EXTEND32:
  +  return width * 4;
  }
  return -EINVAL;
   }
  diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
  index 6442edc..c820be2 100644
  --- a/include/media/soc_camera.h
  +++ b/include/media/soc_camera.h
  @@ -231,10 +231,14 @@ struct soc_camera_sense {
   #define SOCAM_DATAWIDTH_10   SOCAM_DATAWIDTH(10)
 
 Didn't you forget to define SOCAM_DATAWIDTH_12 here?
Yes, I forgot this.

   #define SOCAM_DATAWIDTH_15   SOCAM_DATAWIDTH(15)
   #define SOCAM_DATAWIDTH_16   SOCAM_DATAWIDTH(16)
  +#define SOCAM_DATAWIDTH_18   SOCAM_DATAWIDTH(18)
  +#define SOCAM_DATAWIDTH_24   SOCAM_DATAWIDTH(24)
  
   #define SOCAM_DATAWIDTH_MASK (SOCAM_DATAWIDTH_4 | SOCAM_DATAWIDTH_8 | 
\
  SOCAM_DATAWIDTH_9 | SOCAM_DATAWIDTH_10 | \
  -   SOCAM_DATAWIDTH_15 | SOCAM_DATAWIDTH_16)
  +   SOCAM_DATAWIDTH_12 | SOCAM_DATAWIDTH_15 | \
  +   SOCAM_DATAWIDTH_16 | SOCAM_DATAWIDTH_18 | \
  +   SOCAM_DATAWIDTH_24)
  
   static inline void soc_camera_limit_side(int *start, int *length,
 unsigned int start_min,
  diff --git a/include/media/soc_mediabus.h 
b/include/media/soc_mediabus.h
  index 0dc6f46..eea98d1 100644
  --- a/include/media/soc_mediabus.h
  +++ b/include/media/soc_mediabus.h
  @@ -26,6 +26,8 @@
* @SOC_MBUS_PACKING_VARIABLE:   compressed formats with variable 
packing
* @SOC_MBUS_PACKING_1_5X8:   used for packed YUV 4:2:0 formats, 
where 4
*pixels occupy 6 bytes in RAM
  + * @SOC_MBUS_PACKING_EXTEND32:  sample width (e.g., 24 bits) has 
 to be extended
 
 Please

[PATCH] soc_camera: Add RGB666 RGB888 formats

2013-02-14 Thread Phil Edworthy
Based on work done by Katsuya Matsubara.

Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
 drivers/media/platform/soc_camera/soc_mediabus.c |   42 ++
 include/media/soc_camera.h   |6 +++-
 include/media/soc_mediabus.h |3 ++
 include/uapi/linux/v4l2-mediabus.h   |6 +++-
 4 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c 
b/drivers/media/platform/soc_camera/soc_mediabus.c
index a397812..d8acfd3 100644
--- a/drivers/media/platform/soc_camera/soc_mediabus.c
+++ b/drivers/media/platform/soc_camera/soc_mediabus.c
@@ -97,6 +97,42 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
.layout = SOC_MBUS_LAYOUT_PACKED,
},
 }, {
+   .code = V4L2_MBUS_FMT_RGB666_1X18,
+   .fmt = {
+   .fourcc = V4L2_PIX_FMT_RGB32,
+   .name   = RGB666/32bpp,
+   .bits_per_sample= 18,
+   .packing= SOC_MBUS_PACKING_EXTEND32,
+   .order  = SOC_MBUS_ORDER_LE,
+   },
+}, {
+   .code = V4L2_MBUS_FMT_RGB888_1X24,
+   .fmt = {
+   .fourcc = V4L2_PIX_FMT_RGB32,
+   .name   = RGB888/32bpp,
+   .bits_per_sample= 24,
+   .packing= SOC_MBUS_PACKING_EXTEND32,
+   .order  = SOC_MBUS_ORDER_LE,
+   },
+}, {
+   .code = V4L2_MBUS_FMT_RGB888_2X12_BE,
+   .fmt = {
+   .fourcc = V4L2_PIX_FMT_RGB32,
+   .name   = RGB888/32bpp,
+   .bits_per_sample= 12,
+   .packing= SOC_MBUS_PACKING_EXTEND32,
+   .order  = SOC_MBUS_ORDER_BE,
+   },
+}, {
+   .code = V4L2_MBUS_FMT_RGB888_2X12_LE,
+   .fmt = {
+   .fourcc = V4L2_PIX_FMT_RGB32,
+   .name   = RGB888/32bpp,
+   .bits_per_sample= 12,
+   .packing= SOC_MBUS_PACKING_EXTEND32,
+   .order  = SOC_MBUS_ORDER_LE,
+   },
+}, {
.code = V4L2_MBUS_FMT_SBGGR8_1X8,
.fmt = {
.fourcc = V4L2_PIX_FMT_SBGGR8,
@@ -358,6 +394,10 @@ int soc_mbus_samples_per_pixel(const struct 
soc_mbus_pixelfmt *mf,
*numerator = 1;
*denominator = 1;
return 0;
+   case SOC_MBUS_PACKING_EXTEND32:
+   *numerator = 1;
+   *denominator = 1;
+   return 0;
case SOC_MBUS_PACKING_2X8_PADHI:
case SOC_MBUS_PACKING_2X8_PADLO:
*numerator = 2;
@@ -395,6 +435,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const struct 
soc_mbus_pixelfmt *mf)
return width * 3 / 2;
case SOC_MBUS_PACKING_VARIABLE:
return 0;
+   case SOC_MBUS_PACKING_EXTEND32:
+   return width * 4;
}
return -EINVAL;
 }
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 6442edc..c820be2 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -231,10 +231,14 @@ struct soc_camera_sense {
 #define SOCAM_DATAWIDTH_10 SOCAM_DATAWIDTH(10)
 #define SOCAM_DATAWIDTH_15 SOCAM_DATAWIDTH(15)
 #define SOCAM_DATAWIDTH_16 SOCAM_DATAWIDTH(16)
+#define SOCAM_DATAWIDTH_18 SOCAM_DATAWIDTH(18)
+#define SOCAM_DATAWIDTH_24 SOCAM_DATAWIDTH(24)
 
 #define SOCAM_DATAWIDTH_MASK (SOCAM_DATAWIDTH_4 | SOCAM_DATAWIDTH_8 | \
  SOCAM_DATAWIDTH_9 | SOCAM_DATAWIDTH_10 | \
- SOCAM_DATAWIDTH_15 | SOCAM_DATAWIDTH_16)
+ SOCAM_DATAWIDTH_12 | SOCAM_DATAWIDTH_15 | \
+ SOCAM_DATAWIDTH_16 | SOCAM_DATAWIDTH_18 | \
+ SOCAM_DATAWIDTH_24)
 
 static inline void soc_camera_limit_side(int *start, int *length,
unsigned int start_min,
diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h
index 0dc6f46..eea98d1 100644
--- a/include/media/soc_mediabus.h
+++ b/include/media/soc_mediabus.h
@@ -26,6 +26,8 @@
  * @SOC_MBUS_PACKING_VARIABLE: compressed formats with variable packing
  * @SOC_MBUS_PACKING_1_5X8:used for packed YUV 4:2:0 formats, where 4
  * pixels occupy 6 bytes in RAM
+ * @SOC_MBUS_PACKING_EXTEND32:  sample width (e.g., 24 bits) has to be extended
+ * to 32 bits
  */
 enum soc_mbus_packing {
SOC_MBUS_PACKING_NONE,
@@ -34,6 +36,7 @@ enum soc_mbus_packing {
SOC_MBUS_PACKING_EXTEND16,
SOC_MBUS_PACKING_VARIABLE,
SOC_MBUS_PACKING_1_5X8,
+   SOC_MBUS_PACKING_EXTEND32,
 };
 
 /**
diff --git a/include/uapi/linux/v4l2-mediabus.h 
b

Re: [PATCH 2/3] soc-camera: mt9t112: modify delay time after initialize

2011-10-19 Thread phil . edworthy
Hi Guennadi, Morimoto-san,

   Both are needed.
   These are bug fix patches
  
  I tried to capture several frames beginning with the very first one 
(as 
  much as performance allowed), and I do see several black or wrongly 
  coloured framed in the beginning, but none of those patches, including 
the 
  proposed 300ms at the end of .s_stream() fixes the problem reliably. 
So, 
  either this problems, that these patches fix, are specific to the 
Solution 
  Engine board (is it the one, where the problems have been observed?), 
or 
  one needs a different testing method. If they are SE-specific - I 
don't 
  think, getting those fixes in the driver is very important, because 
  mt9t112 data for SE is not in the mainline. If I was testing wrongly, 
  please, tell me how exactly to reproduce those problems and see, how 
one 
  or another patch fixes them.
 
 I guess mt9t112 camera is used in SE (with local circuit ?)
 and Ecovec.
 But I forgot detail of this issue (I have no mt9t112 for now).
 
 I think Phil is the person who wanted this patch.

There are capture issues on the Ecovec board with this camera. iirc, these 
patches made the situation better but still didn't completely fix all 
issues. Morimoto-san has made comments previously that the mt9t112 is a 
little difficult to setup and we don't have the relevant information from 
the manufacturer.

Thanks
Phil

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