login
Search
In certain machines, camera devices are supplied directly
by a number of regulators. This patch add the ability to drive
these regulators directly by the soc_camera driver.
What the machine code have to do to use this functionality is to:
1- Define a number of useful regulator supply descriptions such as:
static struct regulator_consumer_supply camera_reg1_consumers[] = {
...
REGULATOR_SUPPLY("camera_reg1", "soc-camera-pdrv.0"),
...
};
(Pay attention at the .N suffix of "soc-camera-pdrv" in case of
a system with multiple cameras)
2- Define the list of regulators to bind to a specific instance of
soc-camera-pdrv with their voltages:
static struct soc_camera_regulator_desc soc_camera_regs[] = {
SOCAM_REG_DESC("camera_reg1", 1300000, 1300000),
SOCAM_REG_DESC("camera_reg2", 2800000, 2800000),
...
};
3- Add the list to the corresponding soc_camera_link description:
static struct soc_camera_link iclink_my_camera = {
...
.soc_regulator_descs = soc_camera_regs,
.num_soc_regulator_descs = ARRAY_SIZE(soc_camera_regs),
};
4- And register it as usual with the platform device description:
static struct platform_device machine_my_camera = {
.name = "soc-camera-pdrv",
.id = 0,
.dev = {
.platform_data = &iclink_my_camera,
},
};
Signed-off-by: Alberto Panizzo <maramaopercheseimorto@gmail.com>
---
drivers/media/video/soc_camera.c | 135 +++++++++++++++++++++++++++++++------
include/media/soc_camera.h | 16 +++++
2 files changed, 129 insertions(+), 22 deletions(-)
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 43848a7..8fc5831 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -43,6 +43,96 @@ static LIST_HEAD(hosts);
static LIST_HEAD(devices);
static DEFINE_MUTEX(list_lock); /* Protects the list of hosts */
+static int soc_camera_setup_regulators(struct soc_camera_device *icd,
+ struct soc_camera_link *icl)
+{
+ int i, ...This patch is tested and works with the OV2640 camera that output YUV422 (UYVY) and RGB565 data. The YUV422 format is managed to be converted in IPU internal YUV444 format so this stream could be used in the future to feed directly other IPU blocks. The RGB565 format is managed as GENERIC and can be moved only from CSI to memory. Signed-off-by: Alberto Panizzo <maramaopercheseimorto@gmail.com> --- Before applying, please give me feedback if this break in some way other pixel formats! drivers/media/video/mx3_camera.c | 126 +++++++++++++++++++++++++++++++++----- 1 files changed, 110 insertions(+), 16 deletions(-) diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c index 29c5fc3..6811d6f 100644 --- a/drivers/media/video/mx3_camera.c +++ b/drivers/media/video/mx3_camera.c @@ -55,6 +55,31 @@ #define CSI_SENS_CONF_EXT_VSYNC_SHIFT 15 #define CSI_SENS_CONF_DIVRATIO_SHIFT 16 +/* + * IPU support the following data formatting (44.1.1.3 Data Flows and Formats): + * 1 YUV 4:4:4 or RGB—8 bits per color component + * 2 YUV 4:4:4 or RGB—10 bits per color component + * 3 Generic data (from sensor to the system memory only) + * The formats 1 and 2 are aligned in words of 32 bits, 3 is free and not + * recognized by IPU blocks. + * + * Taking the value of SENS_DATA_FORMAT and DATA_WIDTH, the CSI tries to + * align (or rearrange) the sampled data to fit the IPU supported formats + * as follows: + * - CSI_SENS_CONF_DATA_FMT_RGB_YUV444: It consider the pixel as a sequence of + * 3 components of width DATA_WIDTH aligning these to a 32 bit word. + * The CSI output in this case can feed other IPU blocks. + * - CSI_SENS_CONF_DATA_FMT_YUV422: It consider the pixel as a sequence of + * 2 components of width DATA_WIDTH were the first is the alternating U V + * components and the second is Y. It construct the YUV444 word repeating + * the previous U, V samples aligning the results to a 32 bit word. + * The CSI output in this case can feed other IPU ...
Signed-off-by: Alberto Panizzo <maramaopercheseimorto@gmail.com> --- drivers/media/video/Kconfig | 6 + drivers/media/video/Makefile | 1 + drivers/media/video/ov2640.c | 1153 +++++++++++++++++++++++++++++++++++++++ include/media/v4l2-chip-ident.h | 1 + 4 files changed, 1161 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov2640.c diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 0efbb29..898f76f 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -807,6 +807,12 @@ config SOC_CAMERA_OV9640 help This is a ov9640 camera driver +config SOC_CAMERA_OV2640 + tristate "ov2640 camera support" + depends on SOC_CAMERA && I2C + help + This is a ov2640 camera driver + config MX1_VIDEO bool diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index af79d47..fac185d 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -82,6 +82,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o 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_OV2640) += ov2640.o obj-$(CONFIG_SOC_CAMERA_RJ54N1) += rj54n1cb0c.o obj-$(CONFIG_SOC_CAMERA_TW9910) += tw9910.o diff --git a/drivers/media/video/ov2640.c b/drivers/media/video/ov2640.c new file mode 100644 index 0000000..5edf23e --- /dev/null +++ b/drivers/media/video/ov2640.c @@ -0,0 +1,1153 @@ +/* + * ov2640 Camera Driver + * + * Copyright (C) 2010 Alberto Panizzo <maramaopercheseimorto@gmail.com> + * + * Based on ov772x, ov9640 drivers and previous non merged implementations. + * + * Copyright 2005-2009 Freescale Semiconductor, Inc. All Rights Reserved. + * Copyright (C) 2006, OmniVision + * + * 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 ...
Well, I have no secrets, but I'm not sure everyone on the CC list is
really interested in this thread(s)... So, please consider dropping some
of them when replying, they might be grateful;)
In general looks good, just a couple of easy to fix remarks below, and,
please, fix line wrapping with the next version.
Nice, have I mentioned, how I don't find such register lists particularly
developer-friendly?:) But this is one of the cases where I don't think
requesting you to open code this one would be a wise way to spend your
time, even if it is done in emacs with something around 50 key-strokes;)
Hm, these repeated writes to unnamed registers 0x91, 0x93, 0x97, 0xa7 look
Well, you trust and don't check, that register numbers are within range,
If I understand this correctly, your sensor has just 10 data lines, and
has no configuration to switch to any other bus width. Then I wouldn#t
advertise 8 and 10 bits here. Please, have a look how this is done, e.g.,
in mt9m001.c. There we're trying to reflect the configuration more
correctly: the sensor can do 10 bits only. But the platform can override
+ for (i = 0; i <= def; i++) {
hmm, what exactly does this reset do? It probably doesn't reset register
Please, also implement at least a .cropcap, maybe also .g_crop - both
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
Hello Guennadi,
I Followed an old suggestion to add all that is showed by the
This is the effect of the real problem that I faced working with this
device: Datasheets are really hard to find and the one that I have is
lacking in half of registers description.
I've done a lot of work making sense within pure numeric arrays with
the result that you can see, but all the residual pure numeric values
have an hidden meaning also for me. As I wrote in the copyright this
driver follow two versions from OV and Freescale and in both, there
Yes, with the patch that I proposed for mx3_camera, rgb565 is working
It does a System reset that reset all registers values.
Thanks to you Guennadi!
--
Alberto!
Be Persistent!
- Greg Kroah-Hartman (FOSDEM 2010)
--
Signed-off-by: Alberto Panizzo <maramaopercheseimorto@gmail.com> --- v2 changes: - Fixed addressed errors. - Added trivial .cropcap and .g_crop functions based on the whole size of sensor. drivers/media/video/Kconfig | 6 + drivers/media/video/Makefile | 1 + drivers/media/video/ov2640.c | 1205 +++++++++++++++++++++++++++++++++++++++ include/media/v4l2-chip-ident.h | 1 + 4 files changed, 1213 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov2640.c diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 0efbb29..1fa4f1b 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -789,6 +789,12 @@ config SOC_CAMERA_PLATFORM help This is a generic SoC camera platform driver, useful for testing +config SOC_CAMERA_OV2640 + tristate "ov2640 camera support" + depends on SOC_CAMERA && I2C + help + This is a ov2640 camera driver + config SOC_CAMERA_OV6650 tristate "ov6650 sensor support" depends on SOC_CAMERA && I2C diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index af79d47..1e62c87 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -79,6 +79,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111) += mt9m111.o obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o obj-$(CONFIG_SOC_CAMERA_MT9T112) += mt9t112.o obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o +obj-$(CONFIG_SOC_CAMERA_OV2640) += ov2640.o obj-$(CONFIG_SOC_CAMERA_OV6650) += ov6650.o obj-$(CONFIG_SOC_CAMERA_OV772X) += ov772x.o obj-$(CONFIG_SOC_CAMERA_OV9640) += ov9640.o diff --git a/drivers/media/video/ov2640.c b/drivers/media/video/ov2640.c new file mode 100644 index 0000000..da48810 --- /dev/null +++ b/drivers/media/video/ov2640.c @@ -0,0 +1,1205 @@ +/* + * ov2640 Camera Driver + * + * Copyright (C) 2010 Alberto Panizzo <maramaopercheseimorto@gmail.com> + * + * Based on ov772x, ov9640 drivers and previous non merged implementations. + * + * Copyright ...
Hi Guennadi,
Have you got time to test this patch?
I think that this will solve the problems that lead to the need of
translating the 10 bits mbus codes to the corresponding 8 bit ones.
Best regards,
--
Alberto!
Be Persistent!
- Greg Kroah-Hartman (FOSDEM 2010)
--
Yes, I certainly will review and (hopefully) test this patch, just, please, give me some time. Since this is 2.6.38 material anyway, we still have a bit of time. Thanks --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ --
Ok, so far mx3_camera has only been used with mt9m022 and mt9t031 sensors
(from what I can see in the mainline), both are bayer. It can also work
with monochrome cameras, and that would be the IPU_PIX_FMT_GENERIC case
too. So, I wouldn't mind removing the rest, and only adding / fixing what
you've now tested / implemented with your omnivision sensor. If anyone is
using mx3_camera with any other formats and thinks, that they work -
please, shout now. I'll probably also post a separate mail with this
Let's just do the following:
s32 soc_mbus_samples_per_pixel(const struct soc_mbus_pixelfmt *mf)
{
switch (mf->packing) {
case SOC_MBUS_PACKING_NONE:
case SOC_MBUS_PACKING_EXTEND16:
return 1;
case SOC_MBUS_PACKING_2X8_PADHI:
case SOC_MBUS_PACKING_2X8_PADLO:
return 2;
}
return -EINVAL;
}
EXPORT_SYMBOL(soc_mbus_samples_per_pixel);
width *= soc_mbus_samples_per_pixel(fmt);
If after all the above changed your set up still works, we're cool!;)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
Alberto it would be slowly on the time to address my comments and submit updates. While at it, also, please update the subject - you probably meant "YUV422" or "YUV444" there, also below: don't know what characters are those. Please, replace with ASCII. Thanks --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ --
Ok, I'm dropping this patch Alberto, I've applied and pushed your other 2 patches from this series, but I've dropped this one. The reason is not (only), that you didn't reply to my two last mails with update-requests. But because of that I took the time today to look deeper into detail at this patch. And as a result, I don't think it is correct. Currently the mx3_camera driver transfers data from video clients (camera sensors) only in one mode - as raw data, 1-to-1. This is extablished in the way, how it creates format translation tables during the initial negotiation with client drivers in mx3_camera_get_formats(). Your patch is trying to add support for specific modes on CSI, but is only doing this in the transfer part of the driver, and not in the negotiation part. So, if you really need native support for various pixel formats, this is a wrong way to do this. If you only want to transfer data from your sensor into RAM and the current driver is failing for you, then this is a wrong way to do this, and the bug has to be found and fixed, while maintaining the present pass-through only model. Thanks --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ --
Really sorry Guennadi! My mailer didn't filtered this thread successfully and so I missed your comments! Damn! I'll go deeper on your suggestions in these days and hope to find a good solution. Thank you, --
This patch shows that IPU and CSI manage parameters in different
units. It shows that an unknown at the CSI pixel format, that require n
bytes per pixel, have to be considered generic on the IPU side and the
parameters of the DMA and CSI have to be set properly to support it.
In this way also 10-bit wide pixels formats can be managed in pass
through mode, setting properly the IPU and CSI parameters.
So, this patch shows that the CSI can manage successfully n-byte wide
pixel codes (tested with n = 1,2) without breaking the old behaviour
of providing 10-bit wide pixel formats with 8-bit wide ones.
The next step is to uniform also the pixel-code translations at this
type of management. Being able to capture real 10-bit wide samples.
This patch also, make use of a native functionality of the CSI: capture
a YUV422 format.
In this case the CSI convert this pixel format to YUV444 sent to the
BUS and the IPU re-pack the YUV444 to YUV422 into the memory.
This shows how CSI and IPU manage formats different than the generic
one and open the way to understand how to support the communication
between agents of the IPU encoding chain.
Maybe the last part is misleading you and can be dropped out from this
patch as an enhancement: the YUV422 interleaved format can be
successfully managed as CSI-BAYER/IPU-GENERIC one, the same as rgb565.
Supporting the CSI-YUV422 is a plus only to show how the CSI works.
Best Regards,
--
Alberto!
Be Persistent!
- Greg Kroah-Hartman (FOSDEM 2010)
--
Let's try slowly again: 1. The current mainline driver doesn't work for you, right? What exactly is failing and how? What fourcc format? 2. Do you think, it would be possible to fix the driver to also support your use-case with the present generic / pass-through mode? Have you tried this? Could you try? That would be a bug-fix. 3. After the first two questions are answered, then we can think about extending the driver by adding native support forvarious specific formats. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ --
Yes, does not work for me for both YUV422 interleaved and rgb565. What is captured is an image that have in the bottom half the violet color and in the upper half, half of the real image (divided Yes, this is the way I told about: dividing the geometry fixes from the Yes, sure. I'll try to explain better what the single patches are fixing, improving especially for these core functionality. Best Regards, Alberto! --
This change follows what I described upward. The CSI can manage only RGB 8 bits per component or YUV 4:4:4. Marking the stream with other types (take rgb565 as example) produce a packing-to-memory task from what the CSI is setup to transfer (RGB-8 or YUV4:4:4) to the IPU pixel format chosen. So the only way to manage pixel formats that are not native for the CSI is through: BAYER format for CSI This is a way to maintain the previous behaviour that I consider wrong and object of a second patch. SBGGR10 pixel types are managed in a strange way within the mx3_camera code I think because the previous code were not able to manage pixel formats with multiple bytes for pixel. That's why there is a first handshake of the pixel format in mx3_camera_get_formats with the needs of a "pixel format translation" to support SBGGR10 through real SBGGR8. This translation is not needed with the tools I introduce in this patch and the future is moving in this way. But what I want to do is to show first that translation is not needed (without breaking any userspace code) Yes I agree. Shall this code be pushed in a different patch? ...I think yes because of it could be useful also for other drivers.. But this code will be used only when we will get rid of the All other code-style changes are object of the version 2 of this patch :) Guennadi, how do you consider the path I've shown? Can I continue in this way or shall I present a patch that get rid of translations and manage all the pixel format in the way I understood the manual speak about? I prefer this way that maintain the usability of the whole set of pixel codes for everyone after this step and then fix together the translations. I don't hold a camera that output a grey format.. Best Regards, Alberto! --
On Mon, 3 Jan 2011, Alberto Panizzo wrote: Sorry, I'm not sure I understand what exactly you're proposing. Please, just look at my last reply, in which I explain, why I don't think this is a good way to fix things. You don't need to support various formats natively on CSI / IPU to be able to just pass data 1-to-1. If that is your only purpose, please, test it that way, and if it is broken, please, fix it. This would be good even if you actually want to support formats natively eventually, because that would also fix other formats with similar sample-per-pixel values. When this is fixed, if you want to actually support formats natively to perform some hardware-assisted format conversions, that also can be done, but then, please, explain this clearly in your patch (series). Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ --
IIRC, there has been a discussion a while ago, how to supply power to cameras by regulators. Someone has tried to provide a .power() hook in the platform code, but the problem was the order of driver loading / probing. So, how about doing the following: 1. in your platform code you register a notifier like bus_register_notifier(&soc_camera_bus_type, &cam_notifier); 2. in your notifier you verify, that the driver is already available, or request_module("my-regulator-driver"), and use another notifier to wait, until the driver is probed. 3. if the above has been successful, in your .power() method you just use the regulator as usual. Notice, that your current patch doesn't load regulator driver modules, so, it also relies on them being already available at the time of camera probing. If you can live with that restriction, you can skip loading the module and waiting for its probe above too. The reasons why I do not want to add this to the core are: (1) I do not want to have two methods for turning power on and off: a platform provided .power() hook and and a set of regulators, (2) would anyone really want to use several regulators for a camera sensor? If so, can it be the case, that, for example, the regulators have to be switched off in the reverse order to switching on? Or something else? (3) regulators can often do more, than just set one of two power levels - for on and off. What if a need arises to use other voltages? Is there any really good reason, why we _have_ to do this in soc-camera core? Thanks --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ --
Hi Guennadi, The reason I propose this implementation is because in a different subsystem (mmc) the regulator management is similar and accepted. In that case, platform code that drive regulators directly is not considered successful and it is maintained a platform .power() function that can manage other operations than regulators driving. The power of binding regulators to devices, I think, is given by the ability to fine manage these objects in terms of power management. Why? The implementation I gave, abstract these power management actions in a single layer. Of course I had to manage the way of passing Well, I'm working on the i.MX31 3 Stack board support and there are 2 regulators that powers the camera and if you consider the digital output that enable another supplier needed, the total regulators are three.. So, yes a list of regulators is needed in this case, and Yes I did not considered the order of enabling and disabling operations. Just because even the freescale drivers didn't. A practical general rule is to turn off switchers in the reverse order than the turning on one. And this can be easily implemented here. But as you rose the question, we can add priorities of turning on and Well, you are thinking at the possibility to have one camera that have different voltage levels needs during the ON time (between open and a close). IMHO I think that this could be considered only if a power management event happen and we decide that, in some way, the camera chip can be maintained powered with a different voltage level instead of turning it off and reinitializing it on resume. There are three options in regulators management: 1 Pure platform code. 2 Pure driver code. 3 subsystem code. The first is the one you are suggesting me, but other subsystems rejected it. The third IMHO is better than the first but solve the needs of the single driver. The second is the one I proposed and I think is the best way because add a general framework to manage ...
If you use the regulator bulk API it'll reverse the ordering when doing Unless you're actively varying the voltages at runtime (as Guennadi mentioned) I'd really expect the voltages to be handled by the regulator constraints. --
Great API regulator_bulk, let's get it a try! ..I was reinventing the
This is sane thinking at a static board configuration. What if a single
board have different deploying schemas where two different cameras can
be placed on the same connector and these cameras have different voltage
levels for core supply? This scenario will require two different
constraints chosen at compile time -> two different kernel binaries.
Otherwise constraints will always pick the minimum level and maybe this
will not be enough.
Best regards,
--
Alberto!
Be Persistent!
- Greg Kroah-Hartman (FOSDEM 2010)
--
The way I'd expect to see that handled is that the constraints would be chosen when the module is detected, possibly by having a platform device representing the assembly as a whole. There was someone working with plugin modules doing something along those lines. I certainly wouldn't expect the device itself to worry about this unless it was actively doing something to manage the voltage. --
FWIW I'm looking at implementing a standard regulator API feature along these lines in the background. This should hopefully mean we don't need The way MMC handled this was to provide a standard version of the hook in the core which could be used by platforms with regulators supplying the device - they just assign the appropriate function as their power() operation AIUI. That seems a fairly clean way of keeping stuff in the It does save everyone open coding stuff. --
