[PATCH 0/3] drm/exynos: Add device tree support for IPP driver

[PATCH 0/3] drm/exynos: Add device tree support for IPP driver

[Sylwester Nawrocki]

Hi Inki,

This small patch series adds device tree support for the DRM FIMC driver.
The binding documentation can be found in -next at Documentation/devicetree/
bindings/media/samsung-fimc.txt.
It will make the driver dependent on OF. This patch series is needed in
3.10 to ensure simultaneous operation of the DRM FIMC and the camera ISP
on Exynos4x12.

Thanks,
Sylwester

Sylwester Nawrocki (3):
  drm/exynos: Remove redundant devm_kfree()
  drm/exynos: Rework fimc clocks handling
  drm/exynos: Add device tree support for fimc ipp driver

 drivers/gpu/drm/exynos/Kconfig           |    2 +-
 drivers/gpu/drm/exynos/exynos_drm_fimc.c |  263 ++++++++++++++++--------------
 drivers/gpu/drm/exynos/regs-fimc.h       |    7 +-
 3 files changed, 147 insertions(+), 125 deletions(-)

--
1.7.9.5

[PATCH 1/3] drm/exynos: Remove redundant devm_kfree()

[Sylwester Nawrocki]

There is no need for explicit calls of devm_kfree(), as
the allocated memory will be freed during driver's detach.
Remove the redundant devm_kfree() calls from probe() and
remove() callbacks.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimc.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 411f69b7..d812c57 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1843,7 +1843,6 @@ static int fimc_probe(struct platform_device *pdev)
 	return 0;
 
 err_ippdrv_register:
-	devm_kfree(dev, ippdrv->prop_list);
 	pm_runtime_disable(dev);
 err_get_irq:
 	free_irq(ctx->irq, ctx);
@@ -1857,7 +1856,6 @@ static int fimc_remove(struct platform_device *pdev)
 	struct fimc_context *ctx = get_fimc_context(dev);
 	struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
 
-	devm_kfree(dev, ippdrv->prop_list);
 	exynos_drm_ippdrv_unregister(ippdrv);
 	mutex_destroy(&ctx->lock);
 
-- 
1.7.9.5

[PATCH 2/3] drm/exynos: Rework fimc clocks handling

[Sylwester Nawrocki]

The clocks handling is refactored and a "mux" clock handling is
added to account for changes in the clocks driver. After switching
to the common clock framework the sclk_fimc clock is now split
into two clocks: a gate and a mux clock. In order to retain the
exisiting functionality two additional consumer clocks are passed
to the driver from device tree: "mux" and "parent". Then the driver
sets "parent" clock as a parent clock of the "mux" clock. These two
additional clocks are optional, and should go away when there is a
standard way of setting up parent clocks on DT platforms.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimc.c |  168 +++++++++++++++++-------------
 1 file changed, 98 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index d812c57..9bea585 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -76,6 +76,27 @@ enum fimc_wb {
 	FIMC_WB_B,
 };
 
+enum {
+	FIMC_CLK_LCLK,
+	FIMC_CLK_GATE,
+	FIMC_CLK_WB_A,
+	FIMC_CLK_WB_B,
+	FIMC_CLK_MUX,
+	FIMC_CLK_PARENT,
+	FIMC_CLKS_MAX
+};
+
+static const char * fimc_clock_names[] = {
+	[FIMC_CLK_LCLK]   = "sclk_fimc",
+	[FIMC_CLK_GATE]   = "fimc",
+	[FIMC_CLK_WB_A]   = "pxl_async0",
+	[FIMC_CLK_WB_B]   = "pxl_async1",
+	[FIMC_CLK_MUX]    = "mux",
+	[FIMC_CLK_PARENT] = "parent",
+};
+
+#define FIMC_DEFAULT_LCLK_FREQUENCY 133000000UL
+
 /*
  * A structure of scaler.
  *
@@ -132,15 +153,12 @@ struct fimc_driverdata {
  *
  * @ippdrv: prepare initialization using ippdrv.
  * @regs_res: register resources.
+ * @dev: pointer to the fimc device structure.
  * @regs: memory mapped io registers.
  * @lock: locking of operations.
- * @sclk_fimc_clk: fimc source clock.
- * @fimc_clk: fimc clock.
- * @wb_clk: writeback a clock.
- * @wb_b_clk: writeback b clock.
+ * @clocks: fimc clocks.
+ * @clk_frequency: LCLK clock frequency.
  * @sc: scaler infomations.
- * @odr: ordering of YUV.
- * @ver: fimc version.
  * @pol: porarity of writeback.
  * @id: fimc id.
  * @irq: irq number.
@@ -148,13 +166,12 @@ struct fimc_driverdata {
  */
 struct fimc_context {
 	struct exynos_drm_ippdrv	ippdrv;
+	struct device	*dev;
 	struct resource	*regs_res;
 	void __iomem	*regs;
 	struct mutex	lock;
-	struct clk	*sclk_fimc_clk;
-	struct clk	*fimc_clk;
-	struct clk	*wb_clk;
-	struct clk	*wb_b_clk;
+	struct clk	*clocks[FIMC_CLKS_MAX];
+	u32		clk_frequency;
 	struct fimc_scaler	sc;
 	struct fimc_driverdata	*ddata;
 	struct exynos_drm_ipp_pol	pol;
@@ -1301,14 +1318,12 @@ static int fimc_clk_ctrl(struct fimc_context *ctx, bool enable)
 	DRM_DEBUG_KMS("%s:enable[%d]\n", __func__, enable);
 
 	if (enable) {
-		clk_enable(ctx->sclk_fimc_clk);
-		clk_enable(ctx->fimc_clk);
-		clk_enable(ctx->wb_clk);
+		clk_prepare_enable(ctx->clocks[FIMC_CLK_GATE]);
+		clk_prepare_enable(ctx->clocks[FIMC_CLK_WB_A]);
 		ctx->suspended = false;
 	} else {
-		clk_disable(ctx->sclk_fimc_clk);
-		clk_disable(ctx->fimc_clk);
-		clk_disable(ctx->wb_clk);
+		clk_disable_unprepare(ctx->clocks[FIMC_CLK_GATE]);
+		clk_disable_unprepare(ctx->clocks[FIMC_CLK_WB_A]);
 		ctx->suspended = true;
 	}
 
@@ -1713,11 +1728,66 @@ static void fimc_ippdrv_stop(struct device *dev, enum drm_exynos_ipp_cmd cmd)
 	fimc_write(cfg, EXYNOS_CIGCTRL);
 }
 
+static void fimc_put_clocks(struct fimc_context *ctx)
+{
+	int i;
+
+	for (i = 0; i < FIMC_CLKS_MAX; i++) {
+		if (IS_ERR(ctx->clocks[i]))
+			continue;
+		clk_put(ctx->clocks[i]);
+		ctx->clocks[i] = ERR_PTR(-EINVAL);
+	}
+}
+
+static int fimc_setup_clocks(struct fimc_context *ctx)
+{
+	struct device *dev;
+	int ret, i;
+
+	for (i = 0; i < FIMC_CLKS_MAX; i++)
+		ctx->clocks[i] = ERR_PTR(-EINVAL);
+
+	for (i = 0; i < FIMC_CLKS_MAX; i++) {
+		if (i == FIMC_CLK_WB_A || i == FIMC_CLK_WB_B)
+			dev = ctx->dev->parent;
+		else
+			dev = ctx->dev;
+
+		ctx->clocks[i] = clk_get(dev, fimc_clock_names[i]);
+		if (IS_ERR(ctx->clocks[i])) {
+			if (i >= FIMC_CLK_MUX)
+				break;
+			ret = PTR_ERR(ctx->clocks[i]);
+			goto e_clk_free;
+		}
+	}
+
+	if (!IS_ERR(ctx->clocks[FIMC_CLK_PARENT])) {
+		ret = clk_set_parent(ctx->clocks[FIMC_CLK_MUX],
+				     ctx->clocks[FIMC_CLK_PARENT]);
+		if (ret < 0) {
+			dev_err(ctx->dev, "failed to set parent: %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = clk_set_rate(ctx->clocks[FIMC_CLK_LCLK], ctx->clk_frequency);
+	if (ret < 0)
+		return ret;
+
+	return clk_prepare_enable(ctx->clocks[FIMC_CLK_LCLK]);
+
+e_clk_free:
+	dev_err(ctx->dev, "failed to get clock: %s\n", fimc_clock_names[i]);
+	fimc_put_clocks(ctx);
+	return ret;
+}
+
 static int fimc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fimc_context *ctx;
-	struct clk	*parent_clk;
 	struct resource *res;
 	struct exynos_drm_ippdrv *ippdrv;
 	struct exynos_drm_fimc_pdata *pdata;
@@ -1734,55 +1804,6 @@ static int fimc_probe(struct platform_device *pdev)
 	if (!ctx)
 		return -ENOMEM;
 
-	ddata = (struct fimc_driverdata *)
-		platform_get_device_id(pdev)->driver_data;
-
-	/* clock control */
-	ctx->sclk_fimc_clk = devm_clk_get(dev, "sclk_fimc");
-	if (IS_ERR(ctx->sclk_fimc_clk)) {
-		dev_err(dev, "failed to get src fimc clock.\n");
-		return PTR_ERR(ctx->sclk_fimc_clk);
-	}
-	clk_enable(ctx->sclk_fimc_clk);
-
-	ctx->fimc_clk = devm_clk_get(dev, "fimc");
-	if (IS_ERR(ctx->fimc_clk)) {
-		dev_err(dev, "failed to get fimc clock.\n");
-		clk_disable(ctx->sclk_fimc_clk);
-		return PTR_ERR(ctx->fimc_clk);
-	}
-
-	ctx->wb_clk = devm_clk_get(dev, "pxl_async0");
-	if (IS_ERR(ctx->wb_clk)) {
-		dev_err(dev, "failed to get writeback a clock.\n");
-		clk_disable(ctx->sclk_fimc_clk);
-		return PTR_ERR(ctx->wb_clk);
-	}
-
-	ctx->wb_b_clk = devm_clk_get(dev, "pxl_async1");
-	if (IS_ERR(ctx->wb_b_clk)) {
-		dev_err(dev, "failed to get writeback b clock.\n");
-		clk_disable(ctx->sclk_fimc_clk);
-		return PTR_ERR(ctx->wb_b_clk);
-	}
-
-	parent_clk = devm_clk_get(dev, ddata->parent_clk);
-
-	if (IS_ERR(parent_clk)) {
-		dev_err(dev, "failed to get parent clock.\n");
-		clk_disable(ctx->sclk_fimc_clk);
-		return PTR_ERR(parent_clk);
-	}
-
-	if (clk_set_parent(ctx->sclk_fimc_clk, parent_clk)) {
-		dev_err(dev, "failed to set parent.\n");
-		clk_disable(ctx->sclk_fimc_clk);
-		return -EINVAL;
-	}
-
-	devm_clk_put(dev, parent_clk);
-	clk_set_rate(ctx->sclk_fimc_clk, pdata->clk_rate);
-
 	/* resource memory */
 	ctx->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ctx->regs = devm_ioremap_resource(dev, ctx->regs_res);
@@ -1804,6 +1825,9 @@ static int fimc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	ret = fimc_setup_clocks(ctx);
+	if (ret < 0)
+		goto err_free_irq;
 	/* context initailization */
 	ctx->id = pdev->id;
 	ctx->pol = pdata->pol;
@@ -1820,7 +1844,7 @@ static int fimc_probe(struct platform_device *pdev)
 	ret = fimc_init_prop_list(ippdrv);
 	if (ret < 0) {
 		dev_err(dev, "failed to init property list.\n");
-		goto err_get_irq;
+		goto err_put_clk;
 	}
 
 	DRM_DEBUG_KMS("%s:id[%d]ippdrv[0x%x]\n", __func__, ctx->id,
@@ -1835,16 +1859,19 @@ static int fimc_probe(struct platform_device *pdev)
 	ret = exynos_drm_ippdrv_register(ippdrv);
 	if (ret < 0) {
 		dev_err(dev, "failed to register drm fimc device.\n");
-		goto err_ippdrv_register;
+		goto err_pm_dis;
 	}
 
 	dev_info(&pdev->dev, "drm fimc registered successfully.\n");
 
 	return 0;
 
-err_ippdrv_register:
+err_pm_dis:
+	devm_kfree(dev, ippdrv->prop_list);
 	pm_runtime_disable(dev);
-err_get_irq:
+err_put_clk:
+	fimc_put_clocks(ctx);
+err_free_irq:
 	free_irq(ctx->irq, ctx);
 
 	return ret;
@@ -1859,6 +1886,7 @@ static int fimc_remove(struct platform_device *pdev)
 	exynos_drm_ippdrv_unregister(ippdrv);
 	mutex_destroy(&ctx->lock);
 
+	fimc_put_clocks(ctx);
 	pm_runtime_set_suspended(dev);
 	pm_runtime_disable(dev);
 
-- 
1.7.9.5

Re: [PATCH 2/3] drm/exynos: Rework fimc clocks handling

[Sachin Kamat]

Hi Sylwester,

On 16 April 2013 23:01, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> @@ -1835,16 +1859,19 @@ static int fimc_probe(struct platform_device *pdev)
>         ret = exynos_drm_ippdrv_register(ippdrv);
>         if (ret < 0) {
>                 dev_err(dev, "failed to register drm fimc device.\n");
> -               goto err_ippdrv_register;
> +               goto err_pm_dis;
>         }
>
>         dev_info(&pdev->dev, "drm fimc registered successfully.\n");
>
>         return 0;
>
> -err_ippdrv_register:
> +err_pm_dis:
> +       devm_kfree(dev, ippdrv->prop_list);

devm_kfree was removed in patch1 of this series. Do we need it back?


-- 
With warm regards,
Sachin

Re: [PATCH 2/3] drm/exynos: Rework fimc clocks handling

[Sylwester Nawrocki]

On 04/17/2013 06:02 AM, Sachin Kamat wrote:
> Hi Sylwester,
> 
> On 16 April 2013 23:01, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>> @@ -1835,16 +1859,19 @@ static int fimc_probe(struct platform_device *pdev)
>>         ret = exynos_drm_ippdrv_register(ippdrv);
>>         if (ret < 0) {
>>                 dev_err(dev, "failed to register drm fimc device.\n");
>> -               goto err_ippdrv_register;
>> +               goto err_pm_dis;
>>         }
>>
>>         dev_info(&pdev->dev, "drm fimc registered successfully.\n");
>>
>>         return 0;
>>
>> -err_ippdrv_register:
>> +err_pm_dis:
>> +       devm_kfree(dev, ippdrv->prop_list);
> 
> devm_kfree was removed in patch1 of this series. Do we need it back?

Certainly not, that's a rebase error. I'll resend it fixed.
Thanks for your review.

Regards,
Sylwester

[PATCH 3/3] drm/exynos: Add device tree support for fimc ipp driver

[Sylwester Nawrocki]

This patch adds OF initialization support for the FIMC driver.
The binding documentation can be found at Documentation/devicetree/
bindings/media/samsung-fimc.txt.

The syscon regmap interface is used to serialize access to the
shared CAMBLK registers from within the V4L2 FIMC-IS and the DRM
FIMC drivers. The DRM driver uses this interface for setting up
the FIFO data link between FIMD and FIMC IP blocks, while the V4L2
one for setting up a data link between the camera ISP and FIMC for
camera capture. The CAMBLK registers are not accessed any more
through a statically mapped IO. Synchronized access to these
registers is required for simultaneous operation of the camera
ISP and the DRM IPP on Exynos4x12.

Exynos4 is going to be a dt-only platform since 3.10, thus the
driver data and driver_ids static data structures are removed.

Camera input signal polarities are not currently parsed from the
device tree.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/Kconfig           |    2 +-
 drivers/gpu/drm/exynos/exynos_drm_fimc.c |  101 +++++++++++++++---------------
 drivers/gpu/drm/exynos/regs-fimc.h       |    7 +--
 3 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 406f32a..5c4be2a 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -56,7 +56,7 @@ config DRM_EXYNOS_IPP
 
 config DRM_EXYNOS_FIMC
 	bool "Exynos DRM FIMC"
-	depends on DRM_EXYNOS_IPP
+	depends on DRM_EXYNOS_IPP && MFD_SYSCON && OF
 	help
 	  Choose this option if you want to use Exynos FIMC for DRM.
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 9bea585..f25801e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -12,11 +12,12 @@
  *
  */
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
-#include <plat/map-base.h>
 
 #include <drm/drmP.h>
 #include <drm/exynos_drm.h>
@@ -140,15 +141,6 @@ struct fimc_capability {
 };
 
 /*
- * A structure of fimc driver data.
- *
- * @parent_clk: name of parent clock.
- */
-struct fimc_driverdata {
-	char	*parent_clk;
-};
-
-/*
  * A structure of fimc context.
  *
  * @ippdrv: prepare initialization using ippdrv.
@@ -158,6 +150,7 @@ struct fimc_driverdata {
  * @lock: locking of operations.
  * @clocks: fimc clocks.
  * @clk_frequency: LCLK clock frequency.
+ * @sysreg: handle to SYSREG block regmap.
  * @sc: scaler infomations.
  * @pol: porarity of writeback.
  * @id: fimc id.
@@ -172,8 +165,8 @@ struct fimc_context {
 	struct mutex	lock;
 	struct clk	*clocks[FIMC_CLKS_MAX];
 	u32		clk_frequency;
+	struct regmap	*sysreg;
 	struct fimc_scaler	sc;
-	struct fimc_driverdata	*ddata;
 	struct exynos_drm_ipp_pol	pol;
 	int	id;
 	int	irq;
@@ -217,17 +210,13 @@ static void fimc_sw_reset(struct fimc_context *ctx)
 	fimc_write(0x0, EXYNOS_CIFCNTSEQ);
 }
 
-static void fimc_set_camblk_fimd0_wb(struct fimc_context *ctx)
+static int fimc_set_camblk_fimd0_wb(struct fimc_context *ctx)
 {
-	u32 camblk_cfg;
-
 	DRM_DEBUG_KMS("%s\n", __func__);
 
-	camblk_cfg = readl(SYSREG_CAMERA_BLK);
-	camblk_cfg &= ~(SYSREG_FIMD0WB_DEST_MASK);
-	camblk_cfg |= ctx->id << (SYSREG_FIMD0WB_DEST_SHIFT);
-
-	writel(camblk_cfg, SYSREG_CAMERA_BLK);
+	return regmap_update_bits(ctx->sysreg, SYSREG_CAMERA_BLK,
+				  SYSREG_FIMD0WB_DEST_MASK,
+				  ctx->id << SYSREG_FIMD0WB_DEST_SHIFT);
 }
 
 static void fimc_set_type_ctrl(struct fimc_context *ctx, enum fimc_wb wb)
@@ -1628,7 +1617,9 @@ static int fimc_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd)
 		fimc_handle_lastend(ctx, true);
 
 		/* setup FIMD */
-		fimc_set_camblk_fimd0_wb(ctx);
+		ret = fimc_set_camblk_fimd0_wb(ctx);
+		if (ret < 0)
+			return ret;
 
 		set_wb.enable = 1;
 		set_wb.refresh = property->refresh_rate;
@@ -1784,26 +1775,50 @@ e_clk_free:
 	return ret;
 }
 
+static int fimc_parse_dt(struct fimc_context *ctx)
+{
+	struct device_node *node = ctx->dev->of_node;
+
+	if (!of_property_read_bool(node, "samsung,lcd-wb"))
+		return -ENODEV;
+
+	if (of_property_read_u32(node, "clock-frequency",
+					&ctx->clk_frequency))
+		ctx->clk_frequency = FIMC_DEFAULT_LCLK_FREQUENCY;
+
+	ctx->id = of_alias_get_id(node, "fimc");
+	if (ctx->id < 0)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int fimc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fimc_context *ctx;
 	struct resource *res;
 	struct exynos_drm_ippdrv *ippdrv;
-	struct exynos_drm_fimc_pdata *pdata;
-	struct fimc_driverdata *ddata;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata) {
-		dev_err(dev, "no platform data specified.\n");
-		return -EINVAL;
-	}
+	if (!dev->of_node)
+		return -ENODEV;
 
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
+	ctx->dev = dev;
+
+	ret = fimc_parse_dt(ctx);
+	if (ret < 0)
+		return ret;
+
+	ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
+						"samsung,sysreg");
+	if (IS_ERR(ctx->sysreg))
+		return PTR_ERR(ctx->sysreg);
+
 	/* resource memory */
 	ctx->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ctx->regs = devm_ioremap_resource(dev, ctx->regs_res);
@@ -1828,10 +1843,6 @@ static int fimc_probe(struct platform_device *pdev)
 	ret = fimc_setup_clocks(ctx);
 	if (ret < 0)
 		goto err_free_irq;
-	/* context initailization */
-	ctx->id = pdev->id;
-	ctx->pol = pdata->pol;
-	ctx->ddata = ddata;
 
 	ippdrv = &ctx->ippdrv;
 	ippdrv->dev = dev;
@@ -1941,36 +1952,22 @@ static int fimc_runtime_resume(struct device *dev)
 }
 #endif
 
-static struct fimc_driverdata exynos4210_fimc_data = {
-	.parent_clk = "mout_mpll",
-};
-
-static struct fimc_driverdata exynos4410_fimc_data = {
-	.parent_clk = "mout_mpll_user",
-};
-
-static struct platform_device_id fimc_driver_ids[] = {
-	{
-		.name		= "exynos4210-fimc",
-		.driver_data	= (unsigned long)&exynos4210_fimc_data,
-	}, {
-		.name		= "exynos4412-fimc",
-		.driver_data	= (unsigned long)&exynos4410_fimc_data,
-	},
-	{},
-};
-MODULE_DEVICE_TABLE(platform, fimc_driver_ids);
-
 static const struct dev_pm_ops fimc_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(fimc_suspend, fimc_resume)
 	SET_RUNTIME_PM_OPS(fimc_runtime_suspend, fimc_runtime_resume, NULL)
 };
 
+static const struct of_device_id fimc_of_match[] = {
+	{ .compatible = "samsung,exynos4210-fimc" },
+	{ .compatible = "samsung,exynos4212-fimc" },
+	{ },
+};
+
 struct platform_driver fimc_driver = {
 	.probe		= fimc_probe,
 	.remove		= fimc_remove,
-	.id_table	= fimc_driver_ids,
 	.driver		= {
+		.of_match_table = fimc_of_match,
 		.name	= "exynos-drm-fimc",
 		.owner	= THIS_MODULE,
 		.pm	= &fimc_pm_ops,
diff --git a/drivers/gpu/drm/exynos/regs-fimc.h b/drivers/gpu/drm/exynos/regs-fimc.h
index b4f9ca1..3049613 100644
--- a/drivers/gpu/drm/exynos/regs-fimc.h
+++ b/drivers/gpu/drm/exynos/regs-fimc.h
@@ -661,9 +661,8 @@
 #define EXYNOS_CLKSRC_SCLK				(1 << 1)
 
 /* SYSREG for FIMC writeback */
-#define SYSREG_CAMERA_BLK			(S3C_VA_SYS + 0x0218)
-#define SYSREG_ISP_BLK				(S3C_VA_SYS + 0x020c)
-#define SYSREG_FIMD0WB_DEST_MASK	(0x3 << 23)
-#define SYSREG_FIMD0WB_DEST_SHIFT	23
+#define SYSREG_CAMERA_BLK			(0x0218)
+#define SYSREG_FIMD0WB_DEST_MASK		(0x3 << 23)
+#define SYSREG_FIMD0WB_DEST_SHIFT		23
 
 #endif /* EXYNOS_REGS_FIMC_H */
-- 
1.7.9.5

Re: [PATCH 3/3] drm/exynos: Add device tree support for fimc ipp driver

[Eunchul Kim]

Dear Sylwester Nawrocki.

Sorry. I didn't check your third patch. I understand the "mux", "parent" 
meaning. please ignore my comment of "mux", "parent"

and please check below comments.

Thank's
BR
Eunchul Kim.

On 04/17/2013 02:31 AM, Sylwester Nawrocki wrote:
> This patch adds OF initialization support for the FIMC driver.
> The binding documentation can be found at Documentation/devicetree/
> bindings/media/samsung-fimc.txt.
>
> The syscon regmap interface is used to serialize access to the
> shared CAMBLK registers from within the V4L2 FIMC-IS and the DRM
> FIMC drivers. The DRM driver uses this interface for setting up
> the FIFO data link between FIMD and FIMC IP blocks, while the V4L2
> one for setting up a data link between the camera ISP and FIMC for
> camera capture. The CAMBLK registers are not accessed any more
> through a statically mapped IO. Synchronized access to these
> registers is required for simultaneous operation of the camera
> ISP and the DRM IPP on Exynos4x12.
>
> Exynos4 is going to be a dt-only platform since 3.10, thus the
> driver data and driver_ids static data structures are removed.
>
> Camera input signal polarities are not currently parsed from the
> device tree.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>   drivers/gpu/drm/exynos/Kconfig           |    2 +-
>   drivers/gpu/drm/exynos/exynos_drm_fimc.c |  101 +++++++++++++++---------------
>   drivers/gpu/drm/exynos/regs-fimc.h       |    7 +--
>   3 files changed, 53 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
> index 406f32a..5c4be2a 100644
> --- a/drivers/gpu/drm/exynos/Kconfig
> +++ b/drivers/gpu/drm/exynos/Kconfig
> @@ -56,7 +56,7 @@ config DRM_EXYNOS_IPP
>
>   config DRM_EXYNOS_FIMC
>   	bool "Exynos DRM FIMC"
> -	depends on DRM_EXYNOS_IPP
> +	depends on DRM_EXYNOS_IPP && MFD_SYSCON && OF
>   	help
>   	  Choose this option if you want to use Exynos FIMC for DRM.
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index 9bea585..f25801e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -12,11 +12,12 @@
>    *
>    */
>   #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>   #include <linux/module.h>
>   #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>   #include <linux/clk.h>
>   #include <linux/pm_runtime.h>
> -#include <plat/map-base.h>
>
>   #include <drm/drmP.h>
>   #include <drm/exynos_drm.h>
> @@ -140,15 +141,6 @@ struct fimc_capability {
>   };
>
>   /*
> - * A structure of fimc driver data.
> - *
> - * @parent_clk: name of parent clock.
> - */
> -struct fimc_driverdata {
> -	char	*parent_clk;
> -};
> -
> -/*
>    * A structure of fimc context.
>    *
>    * @ippdrv: prepare initialization using ippdrv.
> @@ -158,6 +150,7 @@ struct fimc_driverdata {
>    * @lock: locking of operations.
>    * @clocks: fimc clocks.
>    * @clk_frequency: LCLK clock frequency.
> + * @sysreg: handle to SYSREG block regmap.
>    * @sc: scaler infomations.
>    * @pol: porarity of writeback.
>    * @id: fimc id.
> @@ -172,8 +165,8 @@ struct fimc_context {
>   	struct mutex	lock;
>   	struct clk	*clocks[FIMC_CLKS_MAX];
>   	u32		clk_frequency;
> +	struct regmap	*sysreg;
>   	struct fimc_scaler	sc;
> -	struct fimc_driverdata	*ddata;
>   	struct exynos_drm_ipp_pol	pol;
>   	int	id;
>   	int	irq;
> @@ -217,17 +210,13 @@ static void fimc_sw_reset(struct fimc_context *ctx)
>   	fimc_write(0x0, EXYNOS_CIFCNTSEQ);
>   }
>
> -static void fimc_set_camblk_fimd0_wb(struct fimc_context *ctx)
> +static int fimc_set_camblk_fimd0_wb(struct fimc_context *ctx)
>   {
> -	u32 camblk_cfg;
> -
>   	DRM_DEBUG_KMS("%s\n", __func__);
>
> -	camblk_cfg = readl(SYSREG_CAMERA_BLK);
> -	camblk_cfg &= ~(SYSREG_FIMD0WB_DEST_MASK);
> -	camblk_cfg |= ctx->id << (SYSREG_FIMD0WB_DEST_SHIFT);
> -
> -	writel(camblk_cfg, SYSREG_CAMERA_BLK);
> +	return regmap_update_bits(ctx->sysreg, SYSREG_CAMERA_BLK,
> +				  SYSREG_FIMD0WB_DEST_MASK,
> +				  ctx->id << SYSREG_FIMD0WB_DEST_SHIFT);
>   }
>
>   static void fimc_set_type_ctrl(struct fimc_context *ctx, enum fimc_wb wb)
> @@ -1628,7 +1617,9 @@ static int fimc_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd)
>   		fimc_handle_lastend(ctx, true);
>
>   		/* setup FIMD */
> -		fimc_set_camblk_fimd0_wb(ctx);
> +		ret = fimc_set_camblk_fimd0_wb(ctx);
> +		if (ret < 0)

- please adds error log information for indicating error.

> +			return ret;
>
>   		set_wb.enable = 1;
>   		set_wb.refresh = property->refresh_rate;
> @@ -1784,26 +1775,50 @@ e_clk_free:
>   	return ret;
>   }
>
> +static int fimc_parse_dt(struct fimc_context *ctx)
> +{
> +	struct device_node *node = ctx->dev->of_node;
> +
> +	if (!of_property_read_bool(node, "samsung,lcd-wb"))

- It also need error log ?

> +		return -ENODEV;
> +
> +	if (of_property_read_u32(node, "clock-frequency",
> +					&ctx->clk_frequency))
> +		ctx->clk_frequency = FIMC_DEFAULT_LCLK_FREQUENCY;

- We have many kind of H/W version of FIMC device. I think we need to 
use pdata instead of static value.

> +
> +	ctx->id = of_alias_get_id(node, "fimc");
> +	if (ctx->id < 0)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>   static int fimc_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct fimc_context *ctx;
>   	struct resource *res;
>   	struct exynos_drm_ippdrv *ippdrv;
> -	struct exynos_drm_fimc_pdata *pdata;
> -	struct fimc_driverdata *ddata;
>   	int ret;
>
> -	pdata = pdev->dev.platform_data;
> -	if (!pdata) {
> -		dev_err(dev, "no platform data specified.\n");
> -		return -EINVAL;
> -	}
> +	if (!dev->of_node)

- ditto.(error log)

> +		return -ENODEV;
>
>   	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>   	if (!ctx)
>   		return -ENOMEM;
>
> +	ctx->dev = dev;
> +
> +	ret = fimc_parse_dt(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> +						"samsung,sysreg");
> +	if (IS_ERR(ctx->sysreg))

- ditto.(error log)

> +		return PTR_ERR(ctx->sysreg);
> +
>   	/* resource memory */
>   	ctx->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	ctx->regs = devm_ioremap_resource(dev, ctx->regs_res);
> @@ -1828,10 +1843,6 @@ static int fimc_probe(struct platform_device *pdev)
>   	ret = fimc_setup_clocks(ctx);
>   	if (ret < 0)
>   		goto err_free_irq;
> -	/* context initailization */
> -	ctx->id = pdev->id;
> -	ctx->pol = pdata->pol;
> -	ctx->ddata = ddata;
>
>   	ippdrv = &ctx->ippdrv;
>   	ippdrv->dev = dev;
> @@ -1941,36 +1952,22 @@ static int fimc_runtime_resume(struct device *dev)
>   }
>   #endif
>
> -static struct fimc_driverdata exynos4210_fimc_data = {
> -	.parent_clk = "mout_mpll",
> -};
> -
> -static struct fimc_driverdata exynos4410_fimc_data = {
> -	.parent_clk = "mout_mpll_user",
> -};
> -
> -static struct platform_device_id fimc_driver_ids[] = {
> -	{
> -		.name		= "exynos4210-fimc",
> -		.driver_data	= (unsigned long)&exynos4210_fimc_data,
> -	}, {
> -		.name		= "exynos4412-fimc",
> -		.driver_data	= (unsigned long)&exynos4410_fimc_data,
> -	},
> -	{},
> -};
> -MODULE_DEVICE_TABLE(platform, fimc_driver_ids);
> -
>   static const struct dev_pm_ops fimc_pm_ops = {
>   	SET_SYSTEM_SLEEP_PM_OPS(fimc_suspend, fimc_resume)
>   	SET_RUNTIME_PM_OPS(fimc_runtime_suspend, fimc_runtime_resume, NULL)
>   };
>
> +static const struct of_device_id fimc_of_match[] = {
> +	{ .compatible = "samsung,exynos4210-fimc" },
> +	{ .compatible = "samsung,exynos4212-fimc" },
> +	{ },
> +};
> +
>   struct platform_driver fimc_driver = {
>   	.probe		= fimc_probe,
>   	.remove		= fimc_remove,
> -	.id_table	= fimc_driver_ids,
>   	.driver		= {
> +		.of_match_table = fimc_of_match,
>   		.name	= "exynos-drm-fimc",
>   		.owner	= THIS_MODULE,
>   		.pm	= &fimc_pm_ops,
> diff --git a/drivers/gpu/drm/exynos/regs-fimc.h b/drivers/gpu/drm/exynos/regs-fimc.h
> index b4f9ca1..3049613 100644
> --- a/drivers/gpu/drm/exynos/regs-fimc.h
> +++ b/drivers/gpu/drm/exynos/regs-fimc.h
> @@ -661,9 +661,8 @@
>   #define EXYNOS_CLKSRC_SCLK				(1 << 1)
>
>   /* SYSREG for FIMC writeback */
> -#define SYSREG_CAMERA_BLK			(S3C_VA_SYS + 0x0218)
> -#define SYSREG_ISP_BLK				(S3C_VA_SYS + 0x020c)
> -#define SYSREG_FIMD0WB_DEST_MASK	(0x3 << 23)
> -#define SYSREG_FIMD0WB_DEST_SHIFT	23
> +#define SYSREG_CAMERA_BLK			(0x0218)
> +#define SYSREG_FIMD0WB_DEST_MASK		(0x3 << 23)
> +#define SYSREG_FIMD0WB_DEST_SHIFT		23
>
>   #endif /* EXYNOS_REGS_FIMC_H */
>

Re: [PATCH 3/3] drm/exynos: Add device tree support for fimc ipp driver

[Sylwester Nawrocki]

On 04/19/2013 01:38 PM, Eunchul Kim wrote:
>>   static void fimc_set_type_ctrl(struct fimc_context *ctx, enum fimc_wb wb)
>> @@ -1628,7 +1617,9 @@ static int fimc_ippdrv_start(struct device *dev, enum
>> drm_exynos_ipp_cmd cmd)
>>           fimc_handle_lastend(ctx, true);
>>
>>           /* setup FIMD */
>> -        fimc_set_camblk_fimd0_wb(ctx);
>> +        ret = fimc_set_camblk_fimd0_wb(ctx);
>> +        if (ret < 0)
> 
> - please adds error log information for indicating error.

OK, I'll add it.

>> +            return ret;
>>
>>           set_wb.enable = 1;
>>           set_wb.refresh = property->refresh_rate;
>> @@ -1784,26 +1775,50 @@ e_clk_free:
>>       return ret;
>>   }
>>
>> +static int fimc_parse_dt(struct fimc_context *ctx)
>> +{
>> +    struct device_node *node = ctx->dev->of_node;
>> +
>> +    if (!of_property_read_bool(node, "samsung,lcd-wb"))
> 
> - It also need error log ?

No, but it definitely deserves some comment why it is done like this.
Please see my other reply to Inki.

>> +        return -ENODEV;
>> +
>> +    if (of_property_read_u32(node, "clock-frequency",
>> +                    &ctx->clk_frequency))
>> +        ctx->clk_frequency = FIMC_DEFAULT_LCLK_FREQUENCY;
> 
> - We have many kind of H/W version of FIMC device. I think we need to use pdata
> instead of static value.

This is just a fallback value that will be used when "clock-frequency"
property is not found in fimc node in the device tree.

>> +    ctx->id = of_alias_get_id(node, "fimc");
>> +    if (ctx->id < 0)
>> +        return -EINVAL;
>> +
>> +    return 0;
>> +}
>> +
>>   static int fimc_probe(struct platform_device *pdev)
>>   {
>>       struct device *dev = &pdev->dev;
>>       struct fimc_context *ctx;
>>       struct resource *res;
>>       struct exynos_drm_ippdrv *ippdrv;
>> -    struct exynos_drm_fimc_pdata *pdata;
>> -    struct fimc_driverdata *ddata;
>>       int ret;
>>
>> -    pdata = pdev->dev.platform_data;
>> -    if (!pdata) {
>> -        dev_err(dev, "no platform data specified.\n");
>> -        return -EINVAL;
>> -    }
>> +    if (!dev->of_node)
> 
> - ditto.(error log)

Probably won't hurt, I'll add it.

>> +        return -ENODEV;
>>
>>       ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>       if (!ctx)
>>           return -ENOMEM;
>>
>> +    ctx->dev = dev;
>> +
>> +    ret = fimc_parse_dt(ctx);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
>> +                        "samsung,sysreg");
>> +    if (IS_ERR(ctx->sysreg))
> 
> - ditto.(error log)

OK. Will add here as well.

Re: [PATCH 3/3] drm/exynos: Add device tree support for fimc ipp driver

[Inki Dae]

[-- Attachment #1.1: Type: text/plain, Size: 9813 bytes --]

2013/4/17 Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

> This patch adds OF initialization support for the FIMC driver.
> The binding documentation can be found at Documentation/devicetree/
> bindings/media/samsung-fimc.txt.
>
> The syscon regmap interface is used to serialize access to the
> shared CAMBLK registers from within the V4L2 FIMC-IS and the DRM
> FIMC drivers. The DRM driver uses this interface for setting up
> the FIFO data link between FIMD and FIMC IP blocks, while the V4L2
> one for setting up a data link between the camera ISP and FIMC for
> camera capture. The CAMBLK registers are not accessed any more
> through a statically mapped IO. Synchronized access to these
> registers is required for simultaneous operation of the camera
> ISP and the DRM IPP on Exynos4x12.
>
> Exynos4 is going to be a dt-only platform since 3.10, thus the
> driver data and driver_ids static data structures are removed.
>
> Camera input signal polarities are not currently parsed from the
> device tree.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/gpu/drm/exynos/Kconfig           |    2 +-
>  drivers/gpu/drm/exynos/exynos_drm_fimc.c |  101
> +++++++++++++++---------------
>  drivers/gpu/drm/exynos/regs-fimc.h       |    7 +--
>  3 files changed, 53 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/Kconfig
> b/drivers/gpu/drm/exynos/Kconfig
> index 406f32a..5c4be2a 100644
> --- a/drivers/gpu/drm/exynos/Kconfig
> +++ b/drivers/gpu/drm/exynos/Kconfig
> @@ -56,7 +56,7 @@ config DRM_EXYNOS_IPP
>
>  config DRM_EXYNOS_FIMC
>         bool "Exynos DRM FIMC"
> -       depends on DRM_EXYNOS_IPP
> +       depends on DRM_EXYNOS_IPP && MFD_SYSCON && OF
>         help
>           Choose this option if you want to use Exynos FIMC for DRM.
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index 9bea585..f25801e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -12,11 +12,12 @@
>   *
>   */
>  #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/clk.h>
>  #include <linux/pm_runtime.h>
> -#include <plat/map-base.h>
>
>  #include <drm/drmP.h>
>  #include <drm/exynos_drm.h>
> @@ -140,15 +141,6 @@ struct fimc_capability {
>  };
>
>  /*
> - * A structure of fimc driver data.
> - *
> - * @parent_clk: name of parent clock.
> - */
> -struct fimc_driverdata {
> -       char    *parent_clk;
> -};
> -
> -/*
>   * A structure of fimc context.
>   *
>   * @ippdrv: prepare initialization using ippdrv.
> @@ -158,6 +150,7 @@ struct fimc_driverdata {
>   * @lock: locking of operations.
>   * @clocks: fimc clocks.
>   * @clk_frequency: LCLK clock frequency.
> + * @sysreg: handle to SYSREG block regmap.
>   * @sc: scaler infomations.
>   * @pol: porarity of writeback.
>   * @id: fimc id.
> @@ -172,8 +165,8 @@ struct fimc_context {
>         struct mutex    lock;
>         struct clk      *clocks[FIMC_CLKS_MAX];
>         u32             clk_frequency;
> +       struct regmap   *sysreg;
>         struct fimc_scaler      sc;
> -       struct fimc_driverdata  *ddata;
>         struct exynos_drm_ipp_pol       pol;
>         int     id;
>         int     irq;
> @@ -217,17 +210,13 @@ static void fimc_sw_reset(struct fimc_context *ctx)
>         fimc_write(0x0, EXYNOS_CIFCNTSEQ);
>  }
>
> -static void fimc_set_camblk_fimd0_wb(struct fimc_context *ctx)
> +static int fimc_set_camblk_fimd0_wb(struct fimc_context *ctx)
>  {
> -       u32 camblk_cfg;
> -
>         DRM_DEBUG_KMS("%s\n", __func__);
>
> -       camblk_cfg = readl(SYSREG_CAMERA_BLK);
> -       camblk_cfg &= ~(SYSREG_FIMD0WB_DEST_MASK);
> -       camblk_cfg |= ctx->id << (SYSREG_FIMD0WB_DEST_SHIFT);
> -
> -       writel(camblk_cfg, SYSREG_CAMERA_BLK);
> +       return regmap_update_bits(ctx->sysreg, SYSREG_CAMERA_BLK,
> +                                 SYSREG_FIMD0WB_DEST_MASK,
> +                                 ctx->id << SYSREG_FIMD0WB_DEST_SHIFT);
>  }
>
>  static void fimc_set_type_ctrl(struct fimc_context *ctx, enum fimc_wb wb)
> @@ -1628,7 +1617,9 @@ static int fimc_ippdrv_start(struct device *dev,
> enum drm_exynos_ipp_cmd cmd)
>                 fimc_handle_lastend(ctx, true);
>
>                 /* setup FIMD */
> -               fimc_set_camblk_fimd0_wb(ctx);
> +               ret = fimc_set_camblk_fimd0_wb(ctx);
> +               if (ret < 0)
> +                       return ret;
>
>                 set_wb.enable = 1;
>                 set_wb.refresh = property->refresh_rate;
> @@ -1784,26 +1775,50 @@ e_clk_free:
>         return ret;
>  }
>
> +static int fimc_parse_dt(struct fimc_context *ctx)
> +{
> +       struct device_node *node = ctx->dev->of_node;
> +
> +       if (!of_property_read_bool(node, "samsung,lcd-wb"))
> +               return -ENODEV;
>


Isn't the above property really needed? This makes the use of write-back
feature to be forced. I think some machines might not need this feature so
their dts files have no that property.
 So in this case, drm fimc driver probing will be failed. is that correct
action and your intention?


> +
> +       if (of_property_read_u32(node, "clock-frequency",
> +                                       &ctx->clk_frequency))
> +               ctx->clk_frequency = FIMC_DEFAULT_LCLK_FREQUENCY;
> +
> +       ctx->id = of_alias_get_id(node, "fimc");
> +       if (ctx->id < 0)
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
>  static int fimc_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct fimc_context *ctx;
>         struct resource *res;
>         struct exynos_drm_ippdrv *ippdrv;
> -       struct exynos_drm_fimc_pdata *pdata;
> -       struct fimc_driverdata *ddata;
>         int ret;
>
> -       pdata = pdev->dev.platform_data;
> -       if (!pdata) {
> -               dev_err(dev, "no platform data specified.\n");
> -               return -EINVAL;
> -       }
> +       if (!dev->of_node)
> +               return -ENODEV;
>
>         ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>         if (!ctx)
>                 return -ENOMEM;
>
> +       ctx->dev = dev;
> +
> +       ret = fimc_parse_dt(ctx);
> +       if (ret < 0)
> +               return ret;
> +
> +       ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> +                                               "samsung,sysreg");
> +       if (IS_ERR(ctx->sysreg))
> +               return PTR_ERR(ctx->sysreg);
> +
>         /* resource memory */
>         ctx->regs_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         ctx->regs = devm_ioremap_resource(dev, ctx->regs_res);
> @@ -1828,10 +1843,6 @@ static int fimc_probe(struct platform_device *pdev)
>         ret = fimc_setup_clocks(ctx);
>         if (ret < 0)
>                 goto err_free_irq;
> -       /* context initailization */
> -       ctx->id = pdev->id;
> -       ctx->pol = pdata->pol;
> -       ctx->ddata = ddata;
>
>         ippdrv = &ctx->ippdrv;
>         ippdrv->dev = dev;
> @@ -1941,36 +1952,22 @@ static int fimc_runtime_resume(struct device *dev)
>  }
>  #endif
>
> -static struct fimc_driverdata exynos4210_fimc_data = {
> -       .parent_clk = "mout_mpll",
> -};
> -
> -static struct fimc_driverdata exynos4410_fimc_data = {
> -       .parent_clk = "mout_mpll_user",
> -};
> -
> -static struct platform_device_id fimc_driver_ids[] = {
> -       {
> -               .name           = "exynos4210-fimc",
> -               .driver_data    = (unsigned long)&exynos4210_fimc_data,
> -       }, {
> -               .name           = "exynos4412-fimc",
> -               .driver_data    = (unsigned long)&exynos4410_fimc_data,
> -       },
> -       {},
> -};
> -MODULE_DEVICE_TABLE(platform, fimc_driver_ids);
> -
>  static const struct dev_pm_ops fimc_pm_ops = {
>         SET_SYSTEM_SLEEP_PM_OPS(fimc_suspend, fimc_resume)
>         SET_RUNTIME_PM_OPS(fimc_runtime_suspend, fimc_runtime_resume, NULL)
>  };
>
> +static const struct of_device_id fimc_of_match[] = {
> +       { .compatible = "samsung,exynos4210-fimc" },
> +       { .compatible = "samsung,exynos4212-fimc" },
> +       { },
> +};
> +
>  struct platform_driver fimc_driver = {
>         .probe          = fimc_probe,
>         .remove         = fimc_remove,
> -       .id_table       = fimc_driver_ids,
>         .driver         = {
> +               .of_match_table = fimc_of_match,
>                 .name   = "exynos-drm-fimc",
>                 .owner  = THIS_MODULE,
>                 .pm     = &fimc_pm_ops,
> diff --git a/drivers/gpu/drm/exynos/regs-fimc.h
> b/drivers/gpu/drm/exynos/regs-fimc.h
> index b4f9ca1..3049613 100644
> --- a/drivers/gpu/drm/exynos/regs-fimc.h
> +++ b/drivers/gpu/drm/exynos/regs-fimc.h
> @@ -661,9 +661,8 @@
>  #define EXYNOS_CLKSRC_SCLK                             (1 << 1)
>
>  /* SYSREG for FIMC writeback */
> -#define SYSREG_CAMERA_BLK                      (S3C_VA_SYS + 0x0218)
> -#define SYSREG_ISP_BLK                         (S3C_VA_SYS + 0x020c)
> -#define SYSREG_FIMD0WB_DEST_MASK       (0x3 << 23)
> -#define SYSREG_FIMD0WB_DEST_SHIFT      23
> +#define SYSREG_CAMERA_BLK                      (0x0218)
> +#define SYSREG_FIMD0WB_DEST_MASK               (0x3 << 23)
> +#define SYSREG_FIMD0WB_DEST_SHIFT              23
>
>  #endif /* EXYNOS_REGS_FIMC_H */
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 11839 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 3/3] drm/exynos: Add device tree support for fimc ipp driver

[Sylwester Nawrocki]

On 04/20/2013 06:21 PM, Inki Dae wrote:
>     +static int fimc_parse_dt(struct fimc_context *ctx)
>     +{
>     +       struct device_node *node = ctx->dev->of_node;
>     +
>     +       if (!of_property_read_bool(node, "samsung,lcd-wb"))
>     +               return -ENODEV;
> 
> 
> 
> Isn't the above property really needed? This makes the use of write-back
> feature to be forced. I think some machines might not need this feature so
> their dts files have no that property.
>  So in this case, drm fimc driver probing will be failed. is that correct
> action and your intention?

Sorry, I should have added a comment explaining why it is done like this.

"samsung,lcd-wb" is the hardware property, it should be in DT node of each
FIMC that has data path from LCD to its Writeback input. See [1] for full
binding description.
So for Exynos4, FIMC2, FIMC3 will be now bound to the DRM driver. And FIMC0,
FIMC1 will be bound to the V4L2 camera driver. There is complementary check
there:

if (of_property_read_bool(node, "samsung,lcd-wb"))
	return -ENODEV;

Ideally there should be just one core driver for FIMC IP block, which is
associated with the fimc DT nodes. And switching between DRM and V4L2
should be possible at run-time. I think Media Controller API could be
helpful in that (note it is not V4L2 specific at all).
But this is all not trivial, especially WRT power management.

Before we have that I assume we could stick with using the "samsung,lcd-wb"
property.

>     +       if (of_property_read_u32(node, "clock-frequency",
>     +                                       &ctx->clk_frequency))
>     +               ctx->clk_frequency = FIMC_DEFAULT_LCLK_FREQUENCY;
>     +
>     +       ctx->id = of_alias_get_id(node, "fimc");
>     +       if (ctx->id < 0)
>     +               return -EINVAL;
>     +
>     +       return 0;
>     +}


P.S. Html is frowned upon on the mailing lists, can please fix you mailer
to use plain text ?

[1]
http://git.linuxtv.org/media_tree.git/blob/master:/Documentation/devicetree/bindings/media/samsung-fimc.txt