[PATCH v2] media: soc_camera: rcar_vin: Add support for 10-bit YUV cameras
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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