Re: [RFC/PATCH 05/13] media: s5p-fimc: Add device tree support for FIMC devices
Hi Laurent, On 07/26/2012 04:54 PM, Laurent Pinchart wrote: On Wednesday 18 July 2012 21:53:34 Sylwester Nawrocki wrote: I think we need a one combined RFC and continue discussions in one thread. Agreed. Still, our proposals are quite different, but I believe we need something in between. I presume we should focus more to have common bindings for subdevs that are reused among different host/ISP devices, i.e. sensors and encoders. For simple host interfaces we can likely come up with common binding patterns, but more complex processing pipelines may require a sort of individual approach. The suspend/resume handling is still something I don't have an idea on how the solution for might look like.. Instantiating all devices from a top level driver could help, but it is only going to work when platforms are converted to the common clock framework and have their clocks instantiated from device tree. This week I'm out of office, and next one or two I have some pending assignments. So there might be some delay before I can dedicate some reasonable amount of time to carry on with that topic. I unfortunately won't be attending KS this time. That's bad news :-( I still think this topic should be discussed during KS, I Yeah, shit happens.. :) I guess -ENOBUDGET this time... I didn't really plan early to attend KS, I might be coming to ELCE though. However it's a rather distant event and we'll probably get most things settled by that time. expect several developers to be interested. The media workshop might not be the best venue though, as we might need quite a lot of time. Until KS let's continue the discussion by e-mail. OK, thank you for taking time to review the RFCs. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 05/13] media: s5p-fimc: Add device tree support for FIMC devices
Hi Sylwester, On Wednesday 18 July 2012 21:53:34 Sylwester Nawrocki wrote: On 07/18/2012 10:17 AM, Guennadi Liakhovetski wrote: On Tue, 17 Jul 2012, Sylwester Nawrocki wrote: On 07/16/2012 11:13 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Karol Lewandowskik.lewando...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com From the documentation below I think, I understand what it does, but why is it needed? It doesn't describe your video subsystem topology, right? How various subdevices are connected. It just lists them all in one node... A description for this patch would be very welcome IMHO and, maybe, such a node can be completely avoided? Sorry, I'll provide better description in next iteration. It's true it doesn't describe the topology in detail, as there are multiple one-to many possible connections between sub-devices. An exact topology is coded in the driver and can be changed through MC API. The samsung,camif-mux-id and video-bus-type properties at sensor nodes just specify to which physical SoC camera port an image sensor is connected. So, don't you think my media-link child nodes are a good solution for this? Not quite yet ;) It would be good to see some example implementation and how it actually works. Originally the all camera devices were supposed to land under common 'camera' node. And a top level driver would be registering all platform devices. With this approach it would be possible to better control PM handling (which currently depends on an order of registering devices to the driver core). But then we discovered that we couldn't use OF_DEV_AUXDATA in such case, which was required to preserve platform device names, in order for the clock API to work. So I've moved some sub-devices out of 'camera' node and have added only a list of phandles to them in that node. This is rather a cheap workaround.. I think all camera sub-devices should be placed under common node, as there are some properties that don't belong to any sub-node: GPIO config, clocks, to name a few. Of course simpler devices might not need such a composite node. I think we can treat the sub-device interdependencies as an issue separate from a need for a common node. If some devices need to reflect the topology better, we probably could include in some nodes (a list of) phandles to other nodes. This could ease parsing the topology at the drivers, by using existing OF infrastructure. Ok, I think you have some good ideas in your RFC's, an interesting question now is - how to proceed. Do you think we'd be able to work out a combined RFC? Or would you prefer to make two versions and then see what others think? In either case it would be nice, I think, if you could try to separate what you see as common V4L DT bindings, then we could discuss that separately. Whereas what you think is private to your hardware, we can also look at for common ideas, or maybe even some of those properties we'll wake to make common too. I think we need a one combined RFC and continue discussions in one thread. Agreed. Still, our proposals are quite different, but I believe we need something in between. I presume we should focus more to have common bindings for subdevs that are reused among different host/ISP devices, i.e. sensors and encoders. For simple host interfaces we can likely come up with common binding patterns, but more complex processing pipelines may require a sort of individual approach. The suspend/resume handling is still something I don't have an idea on how the solution for might look like.. Instantiating all devices from a top level driver could help, but it is only going to work when platforms are converted to the common clock framework and have their clocks instantiated from device tree. This week I'm out of office, and next one or two I have some pending assignments. So there might be some delay before I can dedicate some reasonable amount of time to carry on with that topic. I unfortunately won't be attending KS this time. That's bad news :-( I still think this topic should be discussed during KS, I expect several developers to be interested. The media workshop might not be the best venue though, as we might need quite a lot of time. Until KS let's continue the discussion by e-mail. I'll try to prepare some summary with topics that appear common. And also to restructure my RFC series to better separate new common features and specific H/W support. In the meantime we could possibly continue discussions in your RFC thread. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [RFC/PATCH 05/13] media: s5p-fimc: Add device tree support for FIMC devices
On 07/18/2012 10:17 AM, Guennadi Liakhovetski wrote: On Tue, 17 Jul 2012, Sylwester Nawrocki wrote: On 07/16/2012 11:13 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Karol Lewandowskik.lewando...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com From the documentation below I think, I understand what it does, but why is it needed? It doesn't describe your video subsystem topology, right? How various subdevices are connected. It just lists them all in one node... A description for this patch would be very welcome IMHO and, maybe, such a node can be completely avoided? Sorry, I'll provide better description in next iteration. It's true it doesn't describe the topology in detail, as there are multiple one-to many possible connections between sub-devices. An exact topology is coded in the driver and can be changed through MC API. The samsung,camif-mux-id and video-bus-type properties at sensor nodes just specify to which physical SoC camera port an image sensor is connected. So, don't you think my media-link child nodes are a good solution for this? Not quite yet ;) It would be good to see some example implementation and how it actually works. Originally the all camera devices were supposed to land under common 'camera' node. And a top level driver would be registering all platform devices. With this approach it would be possible to better control PM handling (which currently depends on an order of registering devices to the driver core). But then we discovered that we couldn't use OF_DEV_AUXDATA in such case, which was required to preserve platform device names, in order for the clock API to work. So I've moved some sub-devices out of 'camera' node and have added only a list of phandles to them in that node. This is rather a cheap workaround.. I think all camera sub-devices should be placed under common node, as there are some properties that don't belong to any sub-node: GPIO config, clocks, to name a few. Of course simpler devices might not need such a composite node. I think we can treat the sub-device interdependencies as an issue separate from a need for a common node. If some devices need to reflect the topology better, we probably could include in some nodes (a list of) phandles to other nodes. This could ease parsing the topology at the drivers, by using existing OF infrastructure. Ok, I think you have some good ideas in your RFC's, an interesting question now is - how to proceed. Do you think we'd be able to work out a combined RFC? Or would you prefer to make two versions and then see what others think? In either case it would be nice, I think, if you could try to separate what you see as common V4L DT bindings, then we could discuss that separately. Whereas what you think is private to your hardware, we can also look at for common ideas, or maybe even some of those properties we'll wake to make common too. I think we need a one combined RFC and continue discussions in one thread. Still, our proposals are quite different, but I believe we need something in between. I presume we should focus more to have common bindings for subdevs that are reused among different host/ISP devices, i.e. sensors and encoders. For simple host interfaces we can likely come up with common binding patterns, but more complex processing pipelines may require a sort of individual approach. The suspend/resume handling is still something I don't have an idea on how the solution for might look like.. Instantiating all devices from a top level driver could help, but it is only going to work when platforms are converted to the common clock framework and have their clocks instantiated from device tree. This week I'm out of office, and next one or two I have some pending assignments. So there might be some delay before I can dedicate some reasonable amount of time to carry on with that topic. I unfortunately won't be attending KS this time. I'll try to prepare some summary with topics that appear common. And also to restructure my RFC series to better separate new common features and specific H/W support. In the meantime we could possibly continue discussions in your RFC thread. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 05/13] media: s5p-fimc: Add device tree support for FIMC devices
On 07/16/2012 11:13 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Karol Lewandowskik.lewando...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com From the documentation below I think, I understand what it does, but why is it needed? It doesn't describe your video subsystem topology, right? How various subdevices are connected. It just lists them all in one node... A description for this patch would be very welcome IMHO and, maybe, such a node can be completely avoided? Sorry, I'll provide better description in next iteration. It's true it doesn't describe the topology in detail, as there are multiple one-to many possible connections between sub-devices. An exact topology is coded in the driver and can be changed through MC API. The samsung,camif-mux-id and video-bus-type properties at sensor nodes just specify to which physical SoC camera port an image sensor is connected. Originally the all camera devices were supposed to land under common 'camera' node. And a top level driver would be registering all platform devices. With this approach it would be possible to better control PM handling (which currently depends on an order of registering devices to the driver core). But then we discovered that we couldn't use OF_DEV_AUXDATA in such case, which was required to preserve platform device names, in order for the clock API to work. So I've moved some sub-devices out of 'camera' node and have added only a list of phandles to them in that node. This is rather a cheap workaround.. I think all camera sub-devices should be placed under common node, as there are some properties that don't belong to any sub-node: GPIO config, clocks, to name a few. Of course simpler devices might not need such a composite node. I think we can treat the sub-device interdependencies as an issue separate from a need for a common node. If some devices need to reflect the topology better, we probably could include in some nodes (a list of) phandles to other nodes. This could ease parsing the topology at the drivers, by using existing OF infrastructure. -- Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 05/13] media: s5p-fimc: Add device tree support for FIMC devices
On Fri, 25 May 2012, Sylwester Nawrocki wrote:
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Karol Lewandowski k.lewando...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
From the documentation below I think, I understand what it does, but why
is it needed? It doesn't describe your video subsystem topology, right?
How various subdevices are connected. It just lists them all in one
node... A description for this patch would be very welcome IMHO and,
maybe, such a node can be completely avoided?
Thanks
Guennadi
---
.../bindings/camera/soc/samsung-fimc.txt | 66
drivers/media/video/s5p-fimc/fimc-capture.c|2 +-
drivers/media/video/s5p-fimc/fimc-core.c | 410
+++-
drivers/media/video/s5p-fimc/fimc-core.h |2 -
drivers/media/video/s5p-fimc/fimc-mdevice.c|8 +-
5 files changed, 291 insertions(+), 197 deletions(-)
create mode 100644
Documentation/devicetree/bindings/camera/soc/samsung-fimc.txt
diff --git a/Documentation/devicetree/bindings/camera/soc/samsung-fimc.txt
b/Documentation/devicetree/bindings/camera/soc/samsung-fimc.txt
new file mode 100644
index 000..1ec48e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/camera/soc/samsung-fimc.txt
@@ -0,0 +1,66 @@
+Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
+--
+
+The Exynos Camera subsystem uses a dedicated device node associated with
+top level device driver that manages common properties of the whole
subsystem,
+like common camera port pins or clocks for external image sensors. This
+aggregate node references related platform sub-devices - FIMC, FIMC-LITE,
+MIPI-CSIS [1], and it also contains nodes describing image sensors wired to
+the host SoC's video port and using I2C or SPI as the control bus.
+
+
+Common 'camera' node
+
+
+Required properties:
+
+- compatible: must be samsung,fimc
+- fimc-controllers : an array of phandles to 'fimc' device nodes,
+ size of this array must be at least 1;
+
+Optional properties:
+
+- csi-rx-controllers : an array of phandles to 'csis' device nodes,
+it is required for sensors with MIPI-CSI2 bus;
+
+'fimc' device node
+--
+
+Required properties:
+
+- compatible : should be one of:
+ samsung,s5pv210-fimc
+ samsung,exynos4210-fimc;
+ samsung,exynos4412-fimc;
+- reg : physical base address and size of the device memory
mapped
+registers;
+- interrupts : FIMC interrupt to the CPU should be described here;
+- cell-index : FIMC IP instance index, the number of available instances
+depends on the SoC revision. For S5PV210 valid values are:
+0...2, for Exynos4x1x: 0...3.
+
+Example:
+
+ fimc0: fimc@1180 {
+ compatible = samsung,exynos4210-fimc;
+ reg = 0x1180 0x1000;
+ interrupts = 0 85 0;
+ cell-index = 0;
+ };
+
+ csis0: csis@1188 {
+ compatible = samsung,exynos4210-csis;
+ reg = 0x1188 0x1000;
+ interrupts = 0 78 0;
+ cell-index = 0;
+ };
+
+ camera {
+ compatible = samsung,fimc;
+ #address-cells = 1;
+ #size-cells = 1;
+ fimc-controllers = fimc0;
+ csi-rx-controllers = csis0;
+ };
+
+[1] Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 05/13] media: s5p-fimc: Add device tree support for FIMC devices
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Karol Lewandowski k.lewando...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
.../bindings/camera/soc/samsung-fimc.txt | 66
drivers/media/video/s5p-fimc/fimc-capture.c|2 +-
drivers/media/video/s5p-fimc/fimc-core.c | 410 +++-
drivers/media/video/s5p-fimc/fimc-core.h |2 -
drivers/media/video/s5p-fimc/fimc-mdevice.c|8 +-
5 files changed, 291 insertions(+), 197 deletions(-)
create mode 100644
Documentation/devicetree/bindings/camera/soc/samsung-fimc.txt
diff --git a/Documentation/devicetree/bindings/camera/soc/samsung-fimc.txt
b/Documentation/devicetree/bindings/camera/soc/samsung-fimc.txt
new file mode 100644
index 000..1ec48e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/camera/soc/samsung-fimc.txt
@@ -0,0 +1,66 @@
+Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
+--
+
+The Exynos Camera subsystem uses a dedicated device node associated with
+top level device driver that manages common properties of the whole subsystem,
+like common camera port pins or clocks for external image sensors. This
+aggregate node references related platform sub-devices - FIMC, FIMC-LITE,
+MIPI-CSIS [1], and it also contains nodes describing image sensors wired to
+the host SoC's video port and using I2C or SPI as the control bus.
+
+
+Common 'camera' node
+
+
+Required properties:
+
+- compatible : must be samsung,fimc
+- fimc-controllers : an array of phandles to 'fimc' device nodes,
+size of this array must be at least 1;
+
+Optional properties:
+
+- csi-rx-controllers : an array of phandles to 'csis' device nodes,
+ it is required for sensors with MIPI-CSI2 bus;
+
+'fimc' device node
+--
+
+Required properties:
+
+- compatible : should be one of:
+ samsung,s5pv210-fimc
+ samsung,exynos4210-fimc;
+ samsung,exynos4412-fimc;
+- reg : physical base address and size of the device memory mapped
+ registers;
+- interrupts : FIMC interrupt to the CPU should be described here;
+- cell-index : FIMC IP instance index, the number of available instances
+ depends on the SoC revision. For S5PV210 valid values are:
+ 0...2, for Exynos4x1x: 0...3.
+
+Example:
+
+ fimc0: fimc@1180 {
+ compatible = samsung,exynos4210-fimc;
+ reg = 0x1180 0x1000;
+ interrupts = 0 85 0;
+ cell-index = 0;
+ };
+
+ csis0: csis@1188 {
+ compatible = samsung,exynos4210-csis;
+ reg = 0x1188 0x1000;
+ interrupts = 0 78 0;
+ cell-index = 0;
+ };
+
+ camera {
+ compatible = samsung,fimc;
+ #address-cells = 1;
+ #size-cells = 1;
+ fimc-controllers = fimc0;
+ csi-rx-controllers = csis0;
+ };
+
+[1] Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c
b/drivers/media/video/s5p-fimc/fimc-capture.c
index 0549451..7585b2f 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -1684,7 +1684,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc)
v4l2_subdev_init(sd, fimc_subdev_ops);
sd-flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
- snprintf(sd-name, sizeof(sd-name), FIMC.%d, fimc-pdev-id);
+ snprintf(sd-name, sizeof(sd-name), FIMC.%d, fimc-id);
fimc-vid_cap.sd_pads[FIMC_SD_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
fimc-vid_cap.sd_pads[FIMC_SD_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
b/drivers/media/video/s5p-fimc/fimc-core.c
index fedcd56..30c6365 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -21,6 +21,8 @@
#include linux/pm_runtime.h
#include linux/list.h
#include linux/io.h
+#include linux/of.h
+#include linux/of_device.h
#include linux/slab.h
#include linux/clk.h
#include media/v4l2-ioctl.h
@@ -188,6 +190,198 @@ static struct fimc_fmt fimc_formats[] = {
},
};
+/* Image pixel limits, similar across several FIMC HW revisions. */
+static struct fimc_pix_limit s5p_pix_limit[4] = {
+ [0] = {
+ .scaler_en_w= 3264,
+ .scaler_dis_w = 8192,
+ .in_rot_en_h= 1920,
+ .in_rot_dis_w = 8192,
+ .out_rot_en_w = 1920,
+ .out_rot_dis_w = 4224,
+ },
+ [1] = {
+ .scaler_en_w= 4224,
+ .scaler_dis_w = 8192,
+ .in_rot_en_h= 1920,
+ .in_rot_dis_w = 8192,
+ .out_rot_en_w = 1920,
+
