Hi,
Thanks for the update. I'd like to possibly queue it for 3.12
once the review comments are addressed and the DT maintainers
are OK with that.
W dniu 2013-08-14 06:46, Arun Kumar K pisze:
From: Shaik Ameer Basha <[email protected]>
This patch adds support for media device for EXYNOS5 SoCs.
The current media device supports the following ips to connect
through the media controller framework.
* MIPI-CSIS
Support interconnection(subdev interface) between devices
* FIMC-LITE
Support capture interface from device(Sensor, MIPI-CSIS) to memory
Support interconnection(subdev interface) between devices
* FIMC-IS
Camera post-processing IP having multiple sub-nodes.
G-Scaler will be added later to the current media device.
The media device creates two kinds of pipelines for connecting
the above mentioned IPs.
The pipeline0 is uses Sensor, MIPI-CSIS and FIMC-LITE which captures
image data and dumps to memory.
Pipeline1 uses FIMC-IS components for doing post-processing
operations on the captured image and give scaled YUV output.
Pipeline0
+--------+ +-----------+ +-----------+ +--------+
| Sensor | --> | MIPI-CSIS | --> | FIMC-LITE | --> | Memory |
+--------+ +-----------+ +-----------+ +--------+
Pipeline1
+--------+ +--------+ +-----------+ +-----------+
| Memory | --> | ISP | --> | SCC | --> | SCP |
+--------+ +--------+ +-----------+ +-----------+
Signed-off-by: Shaik Ameer Basha <[email protected]>
Signed-off-by: Arun Kumar K <[email protected]>
---
.../devicetree/bindings/media/exynos5-mdev.txt | 130 +++
drivers/media/platform/exynos5-is/exynos5-mdev.c | 1218 ++++++++++++++++++++
drivers/media/platform/exynos5-is/exynos5-mdev.h | 160 +++
3 files changed, 1508 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/exynos5-mdev.txt
create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.c
create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.h
diff --git a/Documentation/devicetree/bindings/media/exynos5-mdev.txt
b/Documentation/devicetree/bindings/media/exynos5-mdev.txt
new file mode 100644
index 0000000..007ce21
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/exynos5-mdev.txt
@@ -0,0 +1,130 @@
+Samsung EXYNOS5 SoC Camera Subsystem
+------------------------------------
+
+The Exynos5 SoC Camera subsystem comprises of multiple sub-devices
+represented by separate device tree nodes. Currently this includes: FIMC-LITE,
+MIPI CSIS and FIMC-IS.
+
+The sub-subdevices are defined as child nodes of the common 'camera' node which
That statement is not true any more, the sub-devices are now referenced
by phandles.
+also includes common properties of the whole subsystem not really specific to
+any single sub-device, like common camera port pins or the CAMCLK clock outputs
+for external image sensors attached to an SoC.
+
+Common 'camera' node
+--------------------
+
+Required properties:
+
+- compatible : must be "samsung,exynos5-fimc", "simple-bus"
s/exynos5/exynos5250. And "simple-bus" is not needed.
+- clocks : list of clock specifiers, corresponding to entries in
+ the clock-names property;
+- clock-names : must contain "sclk_bayer" entry
+- csis : list of phandles to the mipi-csis device nodes
+- fimc-lite : list of phandles to the fimc-lite device nodes
+- fimc-is : phandle to the fimc-is device node
These properties should be prefixed with "samsung,".
+The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used
+to define a required pinctrl state named "default".
+
+'parallel-ports' node
+---------------------
+
+This node should contain child 'port' nodes specifying active parallel video
+input ports. It includes camera A, camera B, MIPI-CSI2 and RGB bay inputs.
MIPI CSI-2 is a serial bus, it doesn't belong to the parallel-ports
node. Thus only 1,2,5 should be listed in this section. My apologies
if I wasn't clear enough in my previous comments.
+'reg' property in the port nodes specifies the input type:
+ 1 - parallel camport A
+ 2 - parallel camport B
+ 3 - serial MIPI CSI-2 port 0
+ 4 - serial MIPI CSI-2 port 1
+ 5 - RGB camera bay
+
+3, 4 are already described in samsung-mipi-csis.txt
+
+Image sensor nodes
+------------------
+
+The sensor device nodes should be added to their control bus controller (e.g.
+I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
+using the common video interfaces bindings, defined in video-interfaces.txt.
+The implementation of this bindings requires clock-frequency property to be
+present in the sensor device nodes.
That statement is not valid any more. The clock is now handled by
the sensor drivers and if clock-frequency property is not present
a default value will be used. So you need to drop this sentence.
+Example:
+
+ aliases {
+ fimc-lite0 = &fimc_lite_0
+ };
+
+ /* Parallel bus IF sensor */
+ i2c_0: i2c@13860000 {
+ s5k6aa: sensor@3c {
+ compatible = "samsung,s5k6aafx";
+ reg = <0x3c>;
+ vddio-supply = <...>;
+
+ clock-frequency = <24000000>;
+ clocks = <...>;
+ clock-names = "mclk";
+
+ port {
+ s5k6aa_ep: endpoint {
+ remote-endpoint = <&fimc0_ep>;
+ bus-width = <8>;
+ hsync-active = <0>;
+ vsync-active = <1>;
+ pclk-sample = <1>;
+ };
+ };
+ };
+ };
+
+ /* MIPI CSI-2 bus IF sensor */
+ s5c73m3: sensor@1a {
+ compatible = "samsung,s5c73m3";
+ reg = <0x1a>;
+ vddio-supply = <...>;
+
+ clock-frequency = <24000000>;
+ clocks = <...>;
+ clock-names = "mclk";
+
+ port {
+ s5c73m3_1: endpoint {
+ data-lanes = <1 2 3 4>;
+ remote-endpoint = <&csis0_ep>;
+ };
+ };
+ };
+
+ camera {
+ compatible = "samsung,exynos5-fimc", "simple-bus";
s/"samsung,exynos5-fimc"/"samsung,exynos5250-fimc"
+ #address-cells = <1>;
+ #size-cells = <1>;
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&cam_port_a_clk_active>;
+
+ csis = <&csis_0>, <&csis_1>;
s/csis =/samsung,csis =
+ fimc-lite = <&fimc_lite_0>, <&fimc_lite_1>, <&fimc_lite_2>;
s/fimc-lite =/samsung,fimc-lite =
nit: Superfluos space after "<&fimc_lite_0>," ?
+ fimc-is = <&fimc_is>;
s/fimc-is =/samsung,fimc-is =
+ /* parallel camera ports */
+ parallel-ports {
+ /* camera A input */
+ port@0 {
+ reg = <0>;
+ fimc0_ep: endpoint {
fimc0_ep seems an incorrect name, perhaps cam_a_ep or camport_a_ep ?
+ remote-endpoint = <&s5k6aa_ep>;
+ bus-width = <8>;
+ hsync-active = <0>;
+ vsync-active = <1>;
+ pclk-sample = <1>;
+ };
+ };
+ };
+ };
+
+MIPI-CSIS device binding is defined in samsung-mipi-csis.txt, FIMC-LITE
+device binding is defined in exynos-fimc-lite.txt and FIMC-IS binding
+is defined in exynos5-fimc-is.txt.
diff --git a/drivers/media/platform/exynos5-is/exynos5-mdev.c
b/drivers/media/platform/exynos5-is/exynos5-mdev.c
new file mode 100644
index 0000000..0def7c4
--- /dev/null
+++ b/drivers/media/platform/exynos5-is/exynos5-mdev.c
@@ -0,0 +1,1218 @@
+/*
+ * EXYNOS5 SoC series camera host interface media device driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Shaik Ameer Basha <[email protected]>
+ * Arun Kumar K <[email protected]>
+ *
+ * This driver is based on exynos4-is media device driver written by
+ * Sylwester Nawrocki <[email protected]>.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation, either version 2 of the License,
+ * or (at your option) any later version.
+ */
+
+#include <linux/bug.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/of_i2c.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/types.h>
+#include <linux/slab.h>
nit: slab.h should be before types.h
+#include <media/v4l2-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-of.h>
+#include <media/media-device.h>
+#include <media/s5p_fimc.h>
nit: not in alphabetical order
+#include "exynos5-mdev.h"
+#include "fimc-is.h"
+
+#define dbg(fmt, args...) \
+ pr_debug("%s:%d: " fmt "\n", __func__, __LINE__, ##args)
Please remove this unused macro.
+#define BAYER_CLK_NAME "sclk_bayer"
+
+/**
+ * fimc_pipeline_prepare - update pipeline information with subdevice pointers
+ * @me: media entity terminating the pipeline
+ *
+ * Caller holds the graph mutex.
+ */
+static void fimc_pipeline_prepare(struct fimc_pipeline *p,
+ struct media_entity *me)
+{
+ struct v4l2_subdev *sd;
+ int i;
+
+ for (i = 0; i < IDX_MAX; i++)
+ p->subdevs[i] = NULL;
+
+ while (1) {
+ struct media_pad *pad = NULL;
+
+ /* Find remote source pad */
+ for (i = 0; i < me->num_pads; i++) {
+ struct media_pad *spad = &me->pads[i];
+ if (!(spad->flags & MEDIA_PAD_FL_SINK))
+ continue;
+ pad = media_entity_remote_pad(spad);
+ if (pad)
+ break;
+ }
+
+ if (pad == NULL ||
+ media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV) {
+ break;
+ }
+ sd = media_entity_to_v4l2_subdev(pad->entity);
+
+ switch (sd->grp_id) {
+ case GRP_ID_FIMC_IS_SENSOR:
+ case GRP_ID_SENSOR:
+ p->subdevs[IDX_SENSOR] = sd;
+ break;
+ case GRP_ID_CSIS:
+ p->subdevs[IDX_CSIS] = sd;
+ break;
+ case GRP_ID_FLITE:
+ p->subdevs[IDX_FLITE] = sd;
+ break;
+ default:
+ pr_warn("%s: Unknown subdev grp_id: %#x\n",
+ __func__, sd->grp_id);
+ }
+ me = &sd->entity;
+ if (me->num_pads == 1)
+ break;
+ }
+
+ /*
+ * For using FIMC-IS firmware controlled sensors, ISP subdev
+ * has to be initialized along with pipeline0 devices.
+ * So an ISP subdev from a free ISP pipeline is assigned to
+ * this pipeline
nit: missing dot.
+ */
+ if (p->subdevs[IDX_SENSOR]->grp_id == GRP_ID_FIMC_IS_SENSOR) {
+ struct fimc_pipeline_isp *p_isp;
+
+ list_for_each_entry(p_isp, p->isp_pipelines, list) {
+ if (!p_isp->in_use) {
+ p->subdevs[IDX_FIMC_IS] =
+ p_isp->subdevs[IDX_ISP];
+ p_isp->in_use = true;
+ break;
+ }
+ }
+ }
+}
+
[...]
+/* Register FIMC-LITE, CSIS and FIMC-IS media entities */
+static int fimc_md_register_of_platform_entities(struct fimc_md *fmd,
+ struct device_node *camera)
+{
+ struct device_node *node;
+ struct platform_device *pdev;
+ int ret = 0;
+ int i;
+
+ /* Register MIPI-CSIS entities */
+ for (i = 0; i < FIMC_NUM_MIPI_CSIS; i++) {
+
+ node = of_parse_phandle(camera, "csis", i);
+ if (!node || !of_device_is_compatible(node,
+ CSIS_OF_COMPATIBLE_NAME))
Not sure if you need checking against the value of the compatible
property. Perhaps it is enough to say in the binding what compatible
values are supported for each phandle ? If you want to keep that
than I would say it is an error in the device tree structure
to have an incorrect device pointed by this phandle, which deserves
to be signalled with, e.g. WARN_ON().
For sure inactive device should be skipped, so you should also
have of_device_is_available() check here.
+ continue;
+
+ pdev = of_find_device_by_node(node);
+ if (!pdev)
+ continue;
+
+ ret = fimc_md_register_platform_entity(fmd, pdev, IDX_CSIS);
+ put_device(&pdev->dev);
+ if (ret < 0)
+ break;
+ }
+
+ /* Register FIMC-LITE entities */
+ for (i = 0; i < FIMC_NUM_FIMC_LITE; i++) {
+
+ node = of_parse_phandle(camera, "fimc-lite", i);
+ if (!node || !of_device_is_compatible(node,
+ FIMC_LITE_OF_COMPATIBLE_NAME))
Ditto.
+ continue;
+
+ pdev = of_find_device_by_node(node);
+ if (!pdev)
+ continue;
+
+ ret = fimc_md_register_platform_entity(fmd, pdev, IDX_FLITE);
+ put_device(&pdev->dev);
+ if (ret < 0)
+ break;
+ }
+
+ /* Register fimc-is entity */
+ node = of_parse_phandle(camera, "fimc-is", 0);
+ if (!node || !of_device_is_compatible(node,
+ FIMC_IS_OF_COMPATIBLE_NAME))
Ditto.
+ goto exit;
+
+ pdev = of_find_device_by_node(node);
+ if (!pdev)
+ goto exit;
+
+ ret = fimc_md_register_platform_entity(fmd, pdev, IDX_FIMC_IS);
+
+ put_device(&pdev->dev);
+exit:
+ return ret;
+}
[...]
+static int fimc_md_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct v4l2_device *v4l2_dev;
+ struct fimc_md *fmd;
+ int ret;
+
+ fmd = devm_kzalloc(dev, sizeof(*fmd), GFP_KERNEL);
+ if (!fmd)
+ return -ENOMEM;
+
+ spin_lock_init(&fmd->slock);
+ fmd->pdev = pdev;
+ INIT_LIST_HEAD(&fmd->pipelines);
+ INIT_LIST_HEAD(&fmd->isp_pipelines);
+
+ strlcpy(fmd->media_dev.model, "SAMSUNG EXYNOS5 IS",
+ sizeof(fmd->media_dev.model));
+ fmd->media_dev.link_notify = fimc_md_link_notify;
+ fmd->media_dev.dev = dev;
+
+ v4l2_dev = &fmd->v4l2_dev;
+ v4l2_dev->mdev = &fmd->media_dev;
+ strlcpy(v4l2_dev->name, "exynos5-fimc-md", sizeof(v4l2_dev->name));
+
+ ret = v4l2_device_register(dev, &fmd->v4l2_dev);
+ if (ret < 0) {
+ v4l2_err(v4l2_dev, "Failed to register v4l2_device: %d\n", ret);
+ return ret;
+ }
+
+ ret = media_device_register(&fmd->media_dev);
+ if (ret < 0) {
+ v4l2_err(v4l2_dev, "Failed to register media dev: %d\n", ret);
+ goto err_md;
+ }
+
+ fmd->clk_bayer = ERR_PTR(-EINVAL);
This assignment is meaningless, could be just removed.
+ fmd->clk_bayer = clk_get(dev, BAYER_CLK_NAME);
+ if (IS_ERR(fmd->clk_bayer)) {
+ v4l2_err(v4l2_dev, "Failed to get clk: %s\n", BAYER_CLK_NAME);
+ goto err_md;
+ }
+
+ platform_set_drvdata(pdev, fmd);
+
+ /* Protect the media graph while we're registering entities */
+ mutex_lock(&fmd->media_dev.graph_mutex);
+
+ ret = fimc_md_register_of_platform_entities(fmd, dev->of_node);
+ if (ret)
+ goto err_unlock;
+
+ fmd->num_sensors = 0;
+ ret = fimc_md_of_sensors_register(fmd, dev->of_node);
+ if (ret)
+ goto err_unlock;
+
+ mutex_unlock(&fmd->media_dev.graph_mutex);
+
+ fmd->subdev_notifier.subdevs = fmd->async_subdevs;
+ fmd->subdev_notifier.num_subdevs = fmd->num_sensors;
+ fmd->subdev_notifier.bound = subdev_notifier_bound;
+ fmd->subdev_notifier.complete = subdev_notifier_complete;
+ fmd->num_sensors = 0;
+
+ ret = v4l2_async_notifier_register(&fmd->v4l2_dev,
+ &fmd->subdev_notifier);
+ if (ret)
+ goto err_clk;
+
+ return 0;
+
+err_unlock:
+ mutex_unlock(&fmd->media_dev.graph_mutex);
+err_clk:
+ clk_put(fmd->clk_bayer);
+ fimc_md_unregister_entities(fmd);
+ media_device_unregister(&fmd->media_dev);
+err_md:
+ v4l2_device_unregister(&fmd->v4l2_dev);
+ return ret;
+}
[...]
+MODULE_AUTHOR("Shaik Ameer Basha <[email protected]>");
+MODULE_DESCRIPTION("EXYNOS5 FIMC media device driver");
s/FIMC/camera subsystem ?
+MODULE_LICENSE("GPL");
s/GPL/GPL v2 ?
diff --git a/drivers/media/platform/exynos5-is/exynos5-mdev.h
b/drivers/media/platform/exynos5-is/exynos5-mdev.h
new file mode 100644
index 0000000..8563a21
--- /dev/null
+++ b/drivers/media/platform/exynos5-is/exynos5-mdev.h
@@ -0,0 +1,160 @@
[...]
+/**
+ * struct fimc_md - fimc media device information
+ * @csis: MIPI CSIS subdevs data
+ * @sensor: array of registered sensor subdevs
+ * @num_sensors: actual number of registered sensors
+ * @bayer_clk: bus clk for external sensors
s/bayer_clk/clk_bayer
+ * @fimc: array of registered fimc devices
s/fimc/fimc_lite
+ * @is: fimc-is data structure
+ * @media_dev: top level media device
+ * @v4l2_dev: top level v4l2_device holding up the subdevs
+ * @pdev: platform device this media device is hooked up into
+ * @user_subdev_api: true if subdevs are not configured by the host driver
+ * @slock: spinlock protecting @sensor array
+ */
+struct fimc_md {
+ struct fimc_csis_info csis[CSIS_MAX_ENTITIES];
+ struct fimc_sensor_info sensor[FIMC_MAX_SENSORS];
+ int num_sensors;
+ struct clk *clk_bayer;
+ struct fimc_lite *fimc_lite[FIMC_LITE_MAX_DEVS];
+ struct fimc_is *is;
+ struct media_device media_dev;
+ struct v4l2_device v4l2_dev;
+ struct platform_device *pdev;
+ struct v4l2_async_notifier subdev_notifier;
+ struct v4l2_async_subdev *async_subdevs[FIMC_MAX_SENSORS];
+
+ bool user_subdev_api;
This field is unused, please remove.
+ spinlock_t slock;
+ struct list_head pipelines;
+ struct list_head isp_pipelines;
+};
+#define is_subdev_pad(pad) (pad == NULL || \
+ media_entity_type(pad->entity) == MEDIA_ENT_T_V4L2_SUBDEV)
+
+#define me_subtype(me) \
+ ((me->type) & (MEDIA_ENT_TYPE_MASK | MEDIA_ENT_SUBTYPE_MASK))
+
+#define subdev_has_devnode(__sd) (__sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE)
And these three macros as well.
+#define to_fimc_pipeline(_ep) container_of(_ep, struct fimc_pipeline, ep)
+#define to_fimc_isp_pipeline(_ep) \
+ container_of(_ep, struct fimc_pipeline_isp, ep)
+
+static inline struct fimc_md *entity_to_fimc_mdev(struct media_entity *me)
+{
+ return me->parent == NULL ? NULL :
+ container_of(me->parent, struct fimc_md, media_dev);
+}
+
+static inline struct fimc_md *notifier_to_fimc_md(struct v4l2_async_notifier
*n)
+{
+ return container_of(n, struct fimc_md, subdev_notifier);
+}
+static inline void fimc_md_graph_lock(struct exynos_video_entity *ve)
+{
+ mutex_lock(&ve->vdev.entity.parent->graph_mutex);
+}
+
+static inline void fimc_md_graph_unlock(struct exynos_video_entity *ve)
+{
+ mutex_unlock(&ve->vdev.entity.parent->graph_mutex);
+}
These two functions are unused, should be safe to remove them.
+static inline struct v4l2_subdev *__fimc_md_get_subdev(
+ struct exynos_media_pipeline *ep,
+ unsigned int index)
+{
+ struct fimc_pipeline *p = to_fimc_pipeline(ep);
+
+ if (!p || index >= IDX_MAX)
+ return NULL;
+ else
+ return p->subdevs[index];
+}
Unused function ?
--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html