[PATCH V3 0/2] video: drm: Add Device tree support to DRM-FIMD

[PATCH V3 0/2] video: drm: Add Device tree support to DRM-FIMD

[Leela Krishna Amudala]

This patch set adds device tree support for DRM-FIMD for Samsung's Exynos5250.
It includes parsing platform data from dts file. Also, adds the driver data
for exynos5 device.

This patchset is based and tested on top of v3.6-rc1
Also depends on below patchset 
http://lists.freedesktop.org/archives/dri-devel/2012-August/026076.html

Changes since V2:
	- Added driver data to exynos5-drm-fimd as per Marek Szyprowski suggestion

Changes since V1:
        - Corrected typo errors and changed compatibility string

Leela Krishna Amudala (2):
  drm/exynos: add platform_device_id table and driver data for exynos5
    drm fimd
  video: drm: exynos: Add device tree support

 Documentation/devicetree/bindings/fb/drm-fimd.txt |   80 +++++++++++
 drivers/gpu/drm/exynos/exynos_drm_fimd.c          |  151 ++++++++++++++++++++-
 2 files changed, 224 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt

[PATCH V3 1/2] drm/exynos: add platform_device_id table and driver data for exynos5 drm fimd

[Leela Krishna Amudala]

The name of the exynos drm fimd device is renamed to exynos-drm-fimd
and two device ids are created for exynos4-fb and exynos5-drm-fimd.
Also, added driver data for exynos5 to pick the fimd version at runtime and
to choose the VIDTCON register offsets accordingly.

Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c |   56 +++++++++++++++++++++++++++---
 1 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 24c0bd4..8379c59 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -57,6 +57,18 @@
 
 #define get_fimd_context(dev)	platform_get_drvdata(to_platform_device(dev))
 
+enum fimd_version_type {
+	VERSION_8, /* FIMD_VERSION8 */
+};
+
+struct drm_fimd_driver_data {
+	enum fimd_version_type fimd_ver;
+};
+
+struct drm_fimd_driver_data exynos5_drm_fimd_driver_data = {
+	.fimd_ver = VERSION_8,
+};
+
 struct fimd_win_data {
 	unsigned int		offset_x;
 	unsigned int		offset_y;
@@ -91,6 +103,13 @@ struct fimd_context {
 	struct exynos_drm_panel_info *panel;
 };
 
+static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
+	struct platform_device *pdev)
+{
+	return (struct drm_fimd_driver_data *)
+		platform_get_device_id(pdev)->driver_data;
+}
+
 static bool fimd_display_is_connected(struct device *dev)
 {
 	DRM_DEBUG_KMS("%s\n", __FILE__);
@@ -194,32 +213,47 @@ static void fimd_commit(struct device *dev)
 	struct fimd_context *ctx = get_fimd_context(dev);
 	struct exynos_drm_panel_info *panel = ctx->panel;
 	struct fb_videomode *timing = &panel->timing;
+	struct drm_fimd_driver_data *driver_data;
+	struct platform_device *pdev = to_platform_device(dev);
 	u32 val;
 
+	driver_data = drm_fimd_get_driver_data(pdev);
 	if (ctx->suspended)
 		return;
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
 	/* setup polarity values from machine code. */
-	writel(ctx->vidcon1, ctx->regs + VIDCON1);
+	if (driver_data->fimd_ver == VERSION_8)
+		writel(ctx->vidcon1, ctx->regs + FIMD_V8_VIDCON1);
+	else
+		writel(ctx->vidcon1, ctx->regs + VIDCON1);
 
 	/* setup vertical timing values. */
 	val = VIDTCON0_VBPD(timing->upper_margin - 1) |
 	       VIDTCON0_VFPD(timing->lower_margin - 1) |
 	       VIDTCON0_VSPW(timing->vsync_len - 1);
-	writel(val, ctx->regs + VIDTCON0);
+	if (driver_data->fimd_ver == VERSION_8)
+		writel(val, ctx->regs + FIMD_V8_VIDTCON0);
+	else
+		writel(val, ctx->regs + VIDTCON0);
 
 	/* setup horizontal timing values.  */
 	val = VIDTCON1_HBPD(timing->left_margin - 1) |
 	       VIDTCON1_HFPD(timing->right_margin - 1) |
 	       VIDTCON1_HSPW(timing->hsync_len - 1);
-	writel(val, ctx->regs + VIDTCON1);
+	if (driver_data->fimd_ver == VERSION_8)
+		writel(val, ctx->regs + FIMD_V8_VIDTCON1);
+	else
+		writel(val, ctx->regs + VIDTCON1);
 
 	/* setup horizontal and vertical display size. */
 	val = VIDTCON2_LINEVAL(timing->yres - 1) |
 	       VIDTCON2_HOZVAL(timing->xres - 1);
-	writel(val, ctx->regs + VIDTCON2);
+	if (driver_data->fimd_ver == VERSION_8)
+		writel(val, ctx->regs + FIMD_V8_VIDTCON2);
+	else
+		writel(val, ctx->regs + VIDTCON2);
 
 	/* setup clock source, clock divider, enable dma. */
 	val = ctx->vidcon0;
@@ -982,6 +1016,17 @@ static int fimd_runtime_resume(struct device *dev)
 }
 #endif
 
+static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
+	{
+		.name		= "exynos4-fb",
+	}, {
+		.name		= "exynos5-drm-fimd",
+		.driver_data	= (unsigned long)&exynos5_drm_fimd_driver_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
+
 static const struct dev_pm_ops fimd_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
 	SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
@@ -990,8 +1035,9 @@ static const struct dev_pm_ops fimd_pm_ops = {
 struct platform_driver fimd_driver = {
 	.probe		= fimd_probe,
 	.remove		= __devexit_p(fimd_remove),
+	.id_table       = exynos_drm_fimd_driver_ids,
 	.driver		= {
-		.name	= "exynos4-fb",
+		.name	= "exynos-drm-fimd",
 		.owner	= THIS_MODULE,
 		.pm	= &fimd_pm_ops,
 	},
-- 
1.7.0.4

[PATCH V3 2/2] video: drm: exynos: Add device tree support

[Leela Krishna Amudala]

Add device tree based discovery support for DRM-FIMD driver.

Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/fb/drm-fimd.txt |   80 +++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_fimd.c          |   95 ++++++++++++++++++++-
 2 files changed, 173 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt

diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt b/Documentation/devicetree/bindings/fb/drm-fimd.txt
new file mode 100644
index 0000000..8ad8814
--- /dev/null
+++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
@@ -0,0 +1,80 @@
+* Samsung Display Controller using DRM frame work
+
+The display controller is used to transfer image data from memory to an
+external LCD driver interface. It supports various color formats such as
+rgb and yuv.
+
+Required properties:
+ - compatible: Should be "samsung,exynos5-drm" for fimd using DRM frame work.
+ - reg: physical base address of the controller and length of memory
+   mapped region.
+ - interrupts: Three interrupts should be specified. The interrupts should be
+   specified in the following order.
+   - VSYNC interrupt
+   - FIFO level interrupt
+   - FIMD System Interrupt
+
+ - samsung,fimd-display: This property should specify the phandle of the
+   display device node which holds the video interface timing with the
+   below mentioned properties.
+
+   - lcd-htiming: Specifies the horizontal timing for the overlay. The
+     horizontal timing includes four parameters in the following order.
+
+     - horizontal back porch (in number of lcd clocks)
+     - horizontal front porch (in number of lcd clocks)
+     - hsync pulse width (in number of lcd clocks)
+     - Display panels X resolution.
+
+   - lcd-vtiming: Specifies the vertical timing for the overlay. The
+     vertical timing includes four parameters in the following order.
+
+     - vertical back porch (in number of lcd lines)
+     - vertical front porch (in number of lcd lines)
+     - vsync pulse width (in number of lcd clocks)
+     - Display panels Y resolution.
+
+
+ - samsung,default-window: Specifies the default window number of the fimd controller.
+
+ - samsung,fimd-win-bpp: Specifies the bits per pixel.
+
+Optional properties:
+ - supports-mipi-panel: Specifies the lcd is mipi panel type
+ - samsung,fimd-vidout-rgb: Video output format is RGB.
+ - samsung,fimd-inv-vclk: invert video clock polarity.
+ - samsung,fimd-frame-rate: Number of video frames per second.
+
+Example:
+
+	The following is an example for the fimd controller is split into
+	two portions. The SoC specific portion can be specified in the SoC
+	specific dts file. The board specific portion can be specified in the
+	board specific dts file.
+
+	- SoC Specific portion
+
+	fimd {
+		compatible = "samsung,exynos5-drm";
+		interrupt-parent = <&combiner>;
+		reg = <0x14400000 0x40000>;
+		interrupts = <18 5>, <18 4>, <18 6>;
+	};
+
+	- Board Specific portion
+
+	lcd_fimd0: lcd_panel0 {
+			lcd-htiming = <4 4 4 480>;
+			lcd-vtiming = <4 4 4 320>;
+			supports-mipi-panel;
+	};
+
+	fimd {
+		samsung,fimd-display = <&lcd_fimd0>;
+		samsung,fimd-vidout-rgb;
+		samsung,fimd-inv-vclk;
+		samsung,fimd-frame-rate = <60>;
+		samsung,default-window = <0>;
+		samsung,fimd-win-bpp = <32>;
+	};
+
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 8379c59..1753846 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
 
 #include <video/samsung_fimd.h>
 #include <drm/exynos_drm.h>
@@ -103,9 +104,18 @@ struct fimd_context {
 	struct exynos_drm_panel_info *panel;
 };
 
+static const struct of_device_id drm_fimd_dt_match[];
+
 static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
 	struct platform_device *pdev)
 {
+#ifdef CONFIG_OF
+	if (pdev->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(drm_fimd_dt_match, pdev->dev.of_node);
+		return (struct drm_fimd_driver_data *)match->data;
+	}
+#endif
 	return (struct drm_fimd_driver_data *)
 		platform_get_device_id(pdev)->driver_data;
 }
@@ -821,12 +831,79 @@ static int fimd_power_on(struct fimd_context *ctx, bool enable)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *disp_np;
+	struct exynos_drm_fimd_pdata *pd;
+	u32 data[4];
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd) {
+		dev_err(dev, "memory allocation for pdata failed\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
+		pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
+	if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
+		pd->vidcon0 |= VIDCON0_VIDOUT_TV;
+	if (of_get_property(np, "samsung,fimd-inv-hsync", NULL))
+		pd->vidcon1 |= VIDCON1_INV_HSYNC;
+	if (of_get_property(np, "samsung,fimd-inv-vsync", NULL))
+		pd->vidcon1 |= VIDCON1_INV_VSYNC;
+	if (of_get_property(np, "samsung,fimd-inv-vclk", NULL))
+		pd->vidcon1 |= VIDCON1_INV_VCLK;
+	if (of_get_property(np, "samsung,fimd-inv-vden", NULL))
+		pd->vidcon1 |= VIDCON1_INV_VDEN;
+
+	disp_np = of_parse_phandle(np, "samsung,fimd-display", 0);
+	if (!disp_np) {
+		dev_err(dev, "unable to find display panel info\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32_array(disp_np, "lcd-htiming", data, 4)) {
+		dev_err(dev, "invalid horizontal timing\n");
+		return ERR_PTR(-EINVAL);
+	}
+	pd->panel.timing.left_margin = data[0];
+	pd->panel.timing.right_margin = data[1];
+	pd->panel.timing.hsync_len = data[2];
+	pd->panel.timing.xres = data[3];
+
+	if (of_property_read_u32_array(disp_np, "lcd-vtiming", data, 4)) {
+		dev_err(dev, "invalid vertical timing\n");
+		return ERR_PTR(-EINVAL);
+	}
+	pd->panel.timing.upper_margin = data[0];
+	pd->panel.timing.lower_margin = data[1];
+	pd->panel.timing.vsync_len = data[2];
+	pd->panel.timing.yres = data[3];
+
+	of_property_read_u32(np, "samsung,fimd-frame-rate",
+				&pd->panel.timing.refresh);
+
+	of_property_read_u32(np, "samsung,default-window", &pd->default_win);
+
+	of_property_read_u32(np, "samsung,fimd-win-bpp", &pd->bpp);
+
+	return pd;
+}
+#else
+static int exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
 static int __devinit fimd_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fimd_context *ctx;
 	struct exynos_drm_subdrv *subdrv;
-	struct exynos_drm_fimd_pdata *pdata;
+	struct exynos_drm_fimd_pdata *pdata = pdev->dev.platform_data;
 	struct exynos_drm_panel_info *panel;
 	struct resource *res;
 	int win;
@@ -834,7 +911,11 @@ static int __devinit fimd_probe(struct platform_device *pdev)
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
-	pdata = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		pdata = drm_fimd_dt_parse_pdata(&pdev->dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
 	if (!pdata) {
 		dev_err(dev, "no platform data specified\n");
 		return -EINVAL;
@@ -1027,6 +1108,15 @@ static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
 
+#ifdef CONFIG_OF
+static const struct of_device_id drm_fimd_dt_match[] = {
+	{ .compatible = "samsung,exynos5-drm",
+	  .data	= &exynos5_drm_fimd_driver_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, drm_fimd_dt_match);
+#endif
+
 static const struct dev_pm_ops fimd_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
 	SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
@@ -1040,5 +1130,6 @@ struct platform_driver fimd_driver = {
 		.name	= "exynos-drm-fimd",
 		.owner	= THIS_MODULE,
 		.pm	= &fimd_pm_ops,
+		.of_match_table = of_match_ptr(drm_fimd_dt_match),
 	},
 };
-- 
1.7.0.4

Re: [PATCH V3 1/2] drm/exynos: add platform_device_id table and driver data for exynos5 drm fimd

[Joonyoung Shim]

Hi Leela.

2012/8/16 Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>:
> The name of the exynos drm fimd device is renamed to exynos-drm-fimd
> and two device ids are created for exynos4-fb and exynos5-drm-fimd.
> Also, added driver data for exynos5 to pick the fimd version at runtime and
> to choose the VIDTCON register offsets accordingly.
>
> Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   56 +++++++++++++++++++++++++++---
>  1 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 24c0bd4..8379c59 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -57,6 +57,18 @@
>
>  #define get_fimd_context(dev)  platform_get_drvdata(to_platform_device(dev))
>
> +enum fimd_version_type {
> +       VERSION_8, /* FIMD_VERSION8 */
> +};
> +
> +struct drm_fimd_driver_data {

Don't use drm_ prefix for code consistency.

> +       enum fimd_version_type fimd_ver;

I don't prefer to check using version, it needs many if statement.
Refer s3c-fb driver and how about use changed base address?

struct fimd_driverdata {
       unsigned int    timing_base;
};

static struct fimd_driverdata exynos5_fimd_data = {
       .timing_base    = 0x20000,
};

> +};
> +
> +struct drm_fimd_driver_data exynos5_drm_fimd_driver_data = {
> +       .fimd_ver = VERSION_8,
> +};
> +
>  struct fimd_win_data {
>         unsigned int            offset_x;
>         unsigned int            offset_y;
> @@ -91,6 +103,13 @@ struct fimd_context {
>         struct exynos_drm_panel_info *panel;
>  };
>
> +static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
> +       struct platform_device *pdev)
> +{
> +       return (struct drm_fimd_driver_data *)
> +               platform_get_device_id(pdev)->driver_data;
> +}
> +
>  static bool fimd_display_is_connected(struct device *dev)
>  {
>         DRM_DEBUG_KMS("%s\n", __FILE__);
> @@ -194,32 +213,47 @@ static void fimd_commit(struct device *dev)
>         struct fimd_context *ctx = get_fimd_context(dev);
>         struct exynos_drm_panel_info *panel = ctx->panel;
>         struct fb_videomode *timing = &panel->timing;
> +       struct drm_fimd_driver_data *driver_data;
> +       struct platform_device *pdev = to_platform_device(dev);
>         u32 val;
>
> +       driver_data = drm_fimd_get_driver_data(pdev);
>         if (ctx->suspended)
>                 return;
>
>         DRM_DEBUG_KMS("%s\n", __FILE__);
>
>         /* setup polarity values from machine code. */
> -       writel(ctx->vidcon1, ctx->regs + VIDCON1);
> +       if (driver_data->fimd_ver == VERSION_8)
> +               writel(ctx->vidcon1, ctx->regs + FIMD_V8_VIDCON1);
> +       else
> +               writel(ctx->vidcon1, ctx->regs + VIDCON1);
>
>         /* setup vertical timing values. */
>         val = VIDTCON0_VBPD(timing->upper_margin - 1) |
>                VIDTCON0_VFPD(timing->lower_margin - 1) |
>                VIDTCON0_VSPW(timing->vsync_len - 1);
> -       writel(val, ctx->regs + VIDTCON0);
> +       if (driver_data->fimd_ver == VERSION_8)
> +               writel(val, ctx->regs + FIMD_V8_VIDTCON0);
> +       else
> +               writel(val, ctx->regs + VIDTCON0);
>
>         /* setup horizontal timing values.  */
>         val = VIDTCON1_HBPD(timing->left_margin - 1) |
>                VIDTCON1_HFPD(timing->right_margin - 1) |
>                VIDTCON1_HSPW(timing->hsync_len - 1);
> -       writel(val, ctx->regs + VIDTCON1);
> +       if (driver_data->fimd_ver == VERSION_8)
> +               writel(val, ctx->regs + FIMD_V8_VIDTCON1);
> +       else
> +               writel(val, ctx->regs + VIDTCON1);
>
>         /* setup horizontal and vertical display size. */
>         val = VIDTCON2_LINEVAL(timing->yres - 1) |
>                VIDTCON2_HOZVAL(timing->xres - 1);
> -       writel(val, ctx->regs + VIDTCON2);
> +       if (driver_data->fimd_ver == VERSION_8)
> +               writel(val, ctx->regs + FIMD_V8_VIDTCON2);
> +       else
> +               writel(val, ctx->regs + VIDTCON2);
>
>         /* setup clock source, clock divider, enable dma. */
>         val = ctx->vidcon0;
> @@ -982,6 +1016,17 @@ static int fimd_runtime_resume(struct device *dev)
>  }
>  #endif
>
> +static struct platform_device_id exynos_drm_fimd_driver_ids[] = {

Just use "fimd_driver_ids".

> +       {
> +               .name           = "exynos4-fb",
> +       }, {
> +               .name           = "exynos5-drm-fimd",

I think this name should be "exynos5-fb" because exynos5 fb driver and
clock-exynos5 also use "exynos5-fb".

> +               .driver_data    = (unsigned long)&exynos5_drm_fimd_driver_data,
> +       },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
> +
>  static const struct dev_pm_ops fimd_pm_ops = {
>         SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
>         SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
> @@ -990,8 +1035,9 @@ static const struct dev_pm_ops fimd_pm_ops = {
>  struct platform_driver fimd_driver = {
>         .probe          = fimd_probe,
>         .remove         = __devexit_p(fimd_remove),
> +       .id_table       = exynos_drm_fimd_driver_ids,
>         .driver         = {
> -               .name   = "exynos4-fb",
> +               .name   = "exynos-drm-fimd",
>                 .owner  = THIS_MODULE,
>                 .pm     = &fimd_pm_ops,
>         },
> --
> 1.7.0.4
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Thanks.

-- 
- Joonyoung Shim

Re: [PATCH V3 2/2] video: drm: exynos: Add device tree support

[Joonyoung Shim]

Hi,

2012/8/16 Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>:
> Add device tree based discovery support for DRM-FIMD driver.
>
> Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/fb/drm-fimd.txt |   80 +++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c          |   95 ++++++++++++++++++++-
>  2 files changed, 173 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
>
> diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> new file mode 100644
> index 0000000..8ad8814
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> @@ -0,0 +1,80 @@
> +* Samsung Display Controller using DRM frame work
> +
> +The display controller is used to transfer image data from memory to an
> +external LCD driver interface. It supports various color formats such as
> +rgb and yuv.
> +
> +Required properties:
> + - compatible: Should be "samsung,exynos5-drm" for fimd using DRM frame work.

Just use "samsung,exynos5-fb" and add to support exynos4-fb

> + - reg: physical base address of the controller and length of memory
> +   mapped region.
> + - interrupts: Three interrupts should be specified. The interrupts should be
> +   specified in the following order.
> +   - VSYNC interrupt
> +   - FIFO level interrupt
> +   - FIMD System Interrupt
> +
> + - samsung,fimd-display: This property should specify the phandle of the
> +   display device node which holds the video interface timing with the
> +   below mentioned properties.
> +
> +   - lcd-htiming: Specifies the horizontal timing for the overlay. The
> +     horizontal timing includes four parameters in the following order.
> +
> +     - horizontal back porch (in number of lcd clocks)
> +     - horizontal front porch (in number of lcd clocks)
> +     - hsync pulse width (in number of lcd clocks)
> +     - Display panels X resolution.
> +
> +   - lcd-vtiming: Specifies the vertical timing for the overlay. The
> +     vertical timing includes four parameters in the following order.
> +
> +     - vertical back porch (in number of lcd lines)
> +     - vertical front porch (in number of lcd lines)
> +     - vsync pulse width (in number of lcd clocks)
> +     - Display panels Y resolution.
> +
> +
> + - samsung,default-window: Specifies the default window number of the fimd controller.
> +
> + - samsung,fimd-win-bpp: Specifies the bits per pixel.
> +
> +Optional properties:
> + - supports-mipi-panel: Specifies the lcd is mipi panel type

How is this property used?

> + - samsung,fimd-vidout-rgb: Video output format is RGB.
> + - samsung,fimd-inv-vclk: invert video clock polarity.
> + - samsung,fimd-frame-rate: Number of video frames per second.
> +
> +Example:
> +
> +       The following is an example for the fimd controller is split into
> +       two portions. The SoC specific portion can be specified in the SoC
> +       specific dts file. The board specific portion can be specified in the
> +       board specific dts file.
> +
> +       - SoC Specific portion
> +
> +       fimd {
> +               compatible = "samsung,exynos5-drm";
> +               interrupt-parent = <&combiner>;
> +               reg = <0x14400000 0x40000>;
> +               interrupts = <18 5>, <18 4>, <18 6>;
> +       };
> +
> +       - Board Specific portion
> +
> +       lcd_fimd0: lcd_panel0 {
> +                       lcd-htiming = <4 4 4 480>;
> +                       lcd-vtiming = <4 4 4 320>;
> +                       supports-mipi-panel;
> +       };
> +
> +       fimd {
> +               samsung,fimd-display = <&lcd_fimd0>;
> +               samsung,fimd-vidout-rgb;
> +               samsung,fimd-inv-vclk;
> +               samsung,fimd-frame-rate = <60>;
> +               samsung,default-window = <0>;
> +               samsung,fimd-win-bpp = <32>;
> +       };
> +
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 8379c59..1753846 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -18,6 +18,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of.h>
>
>  #include <video/samsung_fimd.h>
>  #include <drm/exynos_drm.h>
> @@ -103,9 +104,18 @@ struct fimd_context {
>         struct exynos_drm_panel_info *panel;
>  };
>
> +static const struct of_device_id drm_fimd_dt_match[];
> +

Please remove drm_ prefix over all.

>  static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
>         struct platform_device *pdev)
>  {
> +#ifdef CONFIG_OF
> +       if (pdev->dev.of_node) {
> +               const struct of_device_id *match;
> +               match = of_match_node(drm_fimd_dt_match, pdev->dev.of_node);

Use of_match_ptr().

> +               return (struct drm_fimd_driver_data *)match->data;
> +       }
> +#endif
>         return (struct drm_fimd_driver_data *)
>                 platform_get_device_id(pdev)->driver_data;
>  }
> @@ -821,12 +831,79 @@ static int fimd_power_on(struct fimd_context *ctx, bool enable)
>         return 0;
>  }
>
> +#ifdef CONFIG_OF
> +static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
> +{
> +       struct device_node *np = dev->of_node;
> +       struct device_node *disp_np;
> +       struct exynos_drm_fimd_pdata *pd;
> +       u32 data[4];
> +
> +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> +       if (!pd) {
> +               dev_err(dev, "memory allocation for pdata failed\n");
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
> +               pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
> +       if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
> +               pd->vidcon0 |= VIDCON0_VIDOUT_TV;

What is this?

> +       if (of_get_property(np, "samsung,fimd-inv-hsync", NULL))
> +               pd->vidcon1 |= VIDCON1_INV_HSYNC;
> +       if (of_get_property(np, "samsung,fimd-inv-vsync", NULL))
> +               pd->vidcon1 |= VIDCON1_INV_VSYNC;
> +       if (of_get_property(np, "samsung,fimd-inv-vclk", NULL))
> +               pd->vidcon1 |= VIDCON1_INV_VCLK;
> +       if (of_get_property(np, "samsung,fimd-inv-vden", NULL))
> +               pd->vidcon1 |= VIDCON1_INV_VDEN;
> +
> +       disp_np = of_parse_phandle(np, "samsung,fimd-display", 0);
> +       if (!disp_np) {
> +               dev_err(dev, "unable to find display panel info\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (of_property_read_u32_array(disp_np, "lcd-htiming", data, 4)) {
> +               dev_err(dev, "invalid horizontal timing\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +       pd->panel.timing.left_margin = data[0];
> +       pd->panel.timing.right_margin = data[1];
> +       pd->panel.timing.hsync_len = data[2];
> +       pd->panel.timing.xres = data[3];
> +
> +       if (of_property_read_u32_array(disp_np, "lcd-vtiming", data, 4)) {
> +               dev_err(dev, "invalid vertical timing\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +       pd->panel.timing.upper_margin = data[0];
> +       pd->panel.timing.lower_margin = data[1];
> +       pd->panel.timing.vsync_len = data[2];
> +       pd->panel.timing.yres = data[3];
> +
> +       of_property_read_u32(np, "samsung,fimd-frame-rate",
> +                               &pd->panel.timing.refresh);
> +
> +       of_property_read_u32(np, "samsung,default-window", &pd->default_win);
> +
> +       of_property_read_u32(np, "samsung,fimd-win-bpp", &pd->bpp);
> +
> +       return pd;
> +}
> +#else
> +static int exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
> +{
> +       return NULL;
> +}
> +#endif /* CONFIG_OF */
> +
>  static int __devinit fimd_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct fimd_context *ctx;
>         struct exynos_drm_subdrv *subdrv;
> -       struct exynos_drm_fimd_pdata *pdata;
> +       struct exynos_drm_fimd_pdata *pdata = pdev->dev.platform_data;
>         struct exynos_drm_panel_info *panel;
>         struct resource *res;
>         int win;
> @@ -834,7 +911,11 @@ static int __devinit fimd_probe(struct platform_device *pdev)
>
>         DRM_DEBUG_KMS("%s\n", __FILE__);
>
> -       pdata = pdev->dev.platform_data;
> +       if (pdev->dev.of_node) {
> +               pdata = drm_fimd_dt_parse_pdata(&pdev->dev);
> +               if (IS_ERR(pdata))
> +                       return PTR_ERR(pdata);
> +       }
>         if (!pdata) {
>                 dev_err(dev, "no platform data specified\n");
>                 return -EINVAL;
> @@ -1027,6 +1108,15 @@ static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id drm_fimd_dt_match[] = {
> +       { .compatible = "samsung,exynos5-drm",
> +         .data = &exynos5_drm_fimd_driver_data },

Add also samsung,exynos4-fb.

> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, drm_fimd_dt_match);
> +#endif
> +
>  static const struct dev_pm_ops fimd_pm_ops = {
>         SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
>         SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
> @@ -1040,5 +1130,6 @@ struct platform_driver fimd_driver = {
>                 .name   = "exynos-drm-fimd",
>                 .owner  = THIS_MODULE,
>                 .pm     = &fimd_pm_ops,
> +               .of_match_table = of_match_ptr(drm_fimd_dt_match),
>         },
>  };
> --
> 1.7.0.4
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Thanks.

-- 
- Joonyoung Shim

Re: [PATCH V3 1/2] drm/exynos: add platform_device_id table and driver data for exynos5 drm fimd

[Leela Krishna Amudala]

Hello,

On Fri, Aug 17, 2012 at 6:18 AM, Joonyoung Shim <dofmind-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> Hi Leela.
>
> 2012/8/16 Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>:
> > The name of the exynos drm fimd device is renamed to exynos-drm-fimd
> > and two device ids are created for exynos4-fb and exynos5-drm-fimd.
> > Also, added driver data for exynos5 to pick the fimd version at runtime
> > and
> > to choose the VIDTCON register offsets accordingly.
> >
> > Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   56
> > +++++++++++++++++++++++++++---
> >  1 files changed, 51 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > index 24c0bd4..8379c59 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > @@ -57,6 +57,18 @@
> >
> >  #define get_fimd_context(dev)
> > platform_get_drvdata(to_platform_device(dev))
> >
> > +enum fimd_version_type {
> > +       VERSION_8, /* FIMD_VERSION8 */
> > +};
> > +
> > +struct drm_fimd_driver_data {
>
> Don't use drm_ prefix for code consistency.
>

Ok, I'll remove the drm_prefix

> > +       enum fimd_version_type fimd_ver;
>
> I don't prefer to check using version, it needs many if statement.
> Refer s3c-fb driver and how about use changed base address?
>
> struct fimd_driverdata {
>        unsigned int    timing_base;
> };
>
> static struct fimd_driverdata exynos5_fimd_data = {
>        .timing_base    = 0x20000,
> };
>

Ok, If I use the above structure as driver data for exynos5, then the
register write calls look this way

writel(val, ctx->regs + driver_data->timing_base + VIDTCON1);
[driver_data->timing_base = 0x20000 in case of exynos5 and 0 in case
of exynos4]
and only one statement is enough for both exynos4 and exynos5.

I had put some effort for the patch set
http://lists.freedesktop.org/archives/dri-devel/2012-August/026076.html
that contains macro definitions for FIMD_V8_VIDCONX and is already
merged to kgene's for-next branch.
If I go with the approach that you suggested then these macros will not be used.

If other reviewers also accepted to use timing_base as driver data,
then will go for it other wise will keep the code same :) .

> > +};
> > +
> > +struct drm_fimd_driver_data exynos5_drm_fimd_driver_data = {
> > +       .fimd_ver = VERSION_8,
> > +};
> > +
> >  struct fimd_win_data {
> >         unsigned int            offset_x;
> >         unsigned int            offset_y;
> > @@ -91,6 +103,13 @@ struct fimd_context {
> >         struct exynos_drm_panel_info *panel;
> >  };
> >
> > +static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
> > +       struct platform_device *pdev)
> > +{
> > +       return (struct drm_fimd_driver_data *)
> > +               platform_get_device_id(pdev)->driver_data;
> > +}
> > +
> >  static bool fimd_display_is_connected(struct device *dev)
> >  {
> >         DRM_DEBUG_KMS("%s\n", __FILE__);
> > @@ -194,32 +213,47 @@ static void fimd_commit(struct device *dev)
> >         struct fimd_context *ctx = get_fimd_context(dev);
> >         struct exynos_drm_panel_info *panel = ctx->panel;
> >         struct fb_videomode *timing = &panel->timing;
> > +       struct drm_fimd_driver_data *driver_data;
> > +       struct platform_device *pdev = to_platform_device(dev);
> >         u32 val;
> >
> > +       driver_data = drm_fimd_get_driver_data(pdev);
> >         if (ctx->suspended)
> >                 return;
> >
> >         DRM_DEBUG_KMS("%s\n", __FILE__);
> >
> >         /* setup polarity values from machine code. */
> > -       writel(ctx->vidcon1, ctx->regs + VIDCON1);
> > +       if (driver_data->fimd_ver == VERSION_8)
> > +               writel(ctx->vidcon1, ctx->regs + FIMD_V8_VIDCON1);
> > +       else
> > +               writel(ctx->vidcon1, ctx->regs + VIDCON1);
> >
> >         /* setup vertical timing values. */
> >         val = VIDTCON0_VBPD(timing->upper_margin - 1) |
> >                VIDTCON0_VFPD(timing->lower_margin - 1) |
> >                VIDTCON0_VSPW(timing->vsync_len - 1);
> > -       writel(val, ctx->regs + VIDTCON0);
> > +       if (driver_data->fimd_ver == VERSION_8)
> > +               writel(val, ctx->regs + FIMD_V8_VIDTCON0);
> > +       else
> > +               writel(val, ctx->regs + VIDTCON0);
> >
> >         /* setup horizontal timing values.  */
> >         val = VIDTCON1_HBPD(timing->left_margin - 1) |
> >                VIDTCON1_HFPD(timing->right_margin - 1) |
> >                VIDTCON1_HSPW(timing->hsync_len - 1);
> > -       writel(val, ctx->regs + VIDTCON1);
> > +       if (driver_data->fimd_ver == VERSION_8)
> > +               writel(val, ctx->regs + FIMD_V8_VIDTCON1);
> > +       else
> > +               writel(val, ctx->regs + VIDTCON1);
> >
> >         /* setup horizontal and vertical display size. */
> >         val = VIDTCON2_LINEVAL(timing->yres - 1) |
> >                VIDTCON2_HOZVAL(timing->xres - 1);
> > -       writel(val, ctx->regs + VIDTCON2);
> > +       if (driver_data->fimd_ver == VERSION_8)
> > +               writel(val, ctx->regs + FIMD_V8_VIDTCON2);
> > +       else
> > +               writel(val, ctx->regs + VIDTCON2);
> >
> >         /* setup clock source, clock divider, enable dma. */
> >         val = ctx->vidcon0;
> > @@ -982,6 +1016,17 @@ static int fimd_runtime_resume(struct device *dev)
> >  }
> >  #endif
> >
> > +static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
>
> Just use "fimd_driver_ids".
>

Ok, will take care of it.

> > +       {
> > +               .name           = "exynos4-fb",
> > +       }, {
> > +               .name           = "exynos5-drm-fimd",
>
> I think this name should be "exynos5-fb" because exynos5 fb driver and
> clock-exynos5 also use "exynos5-fb".
>

Ok, will change it.
Thanks for reviewing the patches.

> > +               .driver_data    = (unsigned
> > long)&exynos5_drm_fimd_driver_data,
> > +       },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
> > +
> >  static const struct dev_pm_ops fimd_pm_ops = {
> >         SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
> >         SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume,
> > NULL)
> > @@ -990,8 +1035,9 @@ static const struct dev_pm_ops fimd_pm_ops = {
> >  struct platform_driver fimd_driver = {
> >         .probe          = fimd_probe,
> >         .remove         = __devexit_p(fimd_remove),
> > +       .id_table       = exynos_drm_fimd_driver_ids,
> >         .driver         = {
> > -               .name   = "exynos4-fb",
> > +               .name   = "exynos-drm-fimd",
> >                 .owner  = THIS_MODULE,
> >                 .pm     = &fimd_pm_ops,
> >         },
> > --
> > 1.7.0.4
> >
> > _______________________________________________
> > devicetree-discuss mailing list
> > devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > https://lists.ozlabs.org/listinfo/devicetree-discuss
>
> Thanks.
>
> --
> - Joonyoung Shim
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH V3 2/2] video: drm: exynos: Add device tree support

[Leela Krishna Amudala]

Hello,

On Fri, Aug 17, 2012 at 6:55 AM, Joonyoung Shim <dofmind-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi,
>
> 2012/8/16 Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>:
>> Add device tree based discovery support for DRM-FIMD driver.
>>
>> Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/fb/drm-fimd.txt |   80 +++++++++++++++++
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c          |   95 ++++++++++++++++++++-
>>  2 files changed, 173 insertions(+), 2 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
>>
>> diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt b/Documentation/devicetree/bindings/fb/drm-fimd.txt
>> new file mode 100644
>> index 0000000..8ad8814
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
>> @@ -0,0 +1,80 @@
>> +* Samsung Display Controller using DRM frame work
>> +
>> +The display controller is used to transfer image data from memory to an
>> +external LCD driver interface. It supports various color formats such as
>> +rgb and yuv.
>> +
>> +Required properties:
>> + - compatible: Should be "samsung,exynos5-drm" for fimd using DRM frame work.
>
> Just use "samsung,exynos5-fb" and add to support exynos4-fb
>

In the first version of this patch set I used "samsung,exynos5-fb",
but as per Kyungmin Park's suggestion changed it to exynos5-drm.

>> + - reg: physical base address of the controller and length of memory
>> +   mapped region.
>> + - interrupts: Three interrupts should be specified. The interrupts should be
>> +   specified in the following order.
>> +   - VSYNC interrupt
>> +   - FIFO level interrupt
>> +   - FIMD System Interrupt
>> +
>> + - samsung,fimd-display: This property should specify the phandle of the
>> +   display device node which holds the video interface timing with the
>> +   below mentioned properties.
>> +
>> +   - lcd-htiming: Specifies the horizontal timing for the overlay. The
>> +     horizontal timing includes four parameters in the following order.
>> +
>> +     - horizontal back porch (in number of lcd clocks)
>> +     - horizontal front porch (in number of lcd clocks)
>> +     - hsync pulse width (in number of lcd clocks)
>> +     - Display panels X resolution.
>> +
>> +   - lcd-vtiming: Specifies the vertical timing for the overlay. The
>> +     vertical timing includes four parameters in the following order.
>> +
>> +     - vertical back porch (in number of lcd lines)
>> +     - vertical front porch (in number of lcd lines)
>> +     - vsync pulse width (in number of lcd clocks)
>> +     - Display panels Y resolution.
>> +
>> +
>> + - samsung,default-window: Specifies the default window number of the fimd controller.
>> +
>> + - samsung,fimd-win-bpp: Specifies the bits per pixel.
>> +
>> +Optional properties:
>> + - supports-mipi-panel: Specifies the lcd is mipi panel type
>
> How is this property used?
>
As this driver can be interfaced through MIPI or eDP, Arch side code
will check whether this property is available or not, if it is
available then it assumes mipi panel is connected and certain clock
rate will be set to fimd clock, otherwise assumes other panel is
connected and other clock rate will be set at arch side.

>> + - samsung,fimd-vidout-rgb: Video output format is RGB.
>> + - samsung,fimd-inv-vclk: invert video clock polarity.
>> + - samsung,fimd-frame-rate: Number of video frames per second.
>> +
>> +Example:
>> +
>> +       The following is an example for the fimd controller is split into
>> +       two portions. The SoC specific portion can be specified in the SoC
>> +       specific dts file. The board specific portion can be specified in the
>> +       board specific dts file.
>> +
>> +       - SoC Specific portion
>> +
>> +       fimd {
>> +               compatible = "samsung,exynos5-drm";
>> +               interrupt-parent = <&combiner>;
>> +               reg = <0x14400000 0x40000>;
>> +               interrupts = <18 5>, <18 4>, <18 6>;
>> +       };
>> +
>> +       - Board Specific portion
>> +
>> +       lcd_fimd0: lcd_panel0 {
>> +                       lcd-htiming = <4 4 4 480>;
>> +                       lcd-vtiming = <4 4 4 320>;
>> +                       supports-mipi-panel;
>> +       };
>> +
>> +       fimd {
>> +               samsung,fimd-display = <&lcd_fimd0>;
>> +               samsung,fimd-vidout-rgb;
>> +               samsung,fimd-inv-vclk;
>> +               samsung,fimd-frame-rate = <60>;
>> +               samsung,default-window = <0>;
>> +               samsung,fimd-win-bpp = <32>;
>> +       };
>> +
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 8379c59..1753846 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/clk.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/of.h>
>>
>>  #include <video/samsung_fimd.h>
>>  #include <drm/exynos_drm.h>
>> @@ -103,9 +104,18 @@ struct fimd_context {
>>         struct exynos_drm_panel_info *panel;
>>  };
>>
>> +static const struct of_device_id drm_fimd_dt_match[];
>> +
>
> Please remove drm_ prefix over all.
>

Ok, will do that

>>  static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
>>         struct platform_device *pdev)
>>  {
>> +#ifdef CONFIG_OF
>> +       if (pdev->dev.of_node) {
>> +               const struct of_device_id *match;
>> +               match = of_match_node(drm_fimd_dt_match, pdev->dev.of_node);
>
> Use of_match_ptr().
>

Ok, will change it

>> +               return (struct drm_fimd_driver_data *)match->data;
>> +       }
>> +#endif
>>         return (struct drm_fimd_driver_data *)
>>                 platform_get_device_id(pdev)->driver_data;
>>  }
>> @@ -821,12 +831,79 @@ static int fimd_power_on(struct fimd_context *ctx, bool enable)
>>         return 0;
>>  }
>>
>> +#ifdef CONFIG_OF
>> +static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
>> +{
>> +       struct device_node *np = dev->of_node;
>> +       struct device_node *disp_np;
>> +       struct exynos_drm_fimd_pdata *pd;
>> +       u32 data[4];
>> +
>> +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
>> +       if (!pd) {
>> +               dev_err(dev, "memory allocation for pdata failed\n");
>> +               return ERR_PTR(-ENOMEM);
>> +       }
>> +
>> +       if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
>> +               pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
>> +       if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
>> +               pd->vidcon0 |= VIDCON0_VIDOUT_TV;
>
> What is this?
>

I'll remove this property.

>> +       if (of_get_property(np, "samsung,fimd-inv-hsync", NULL))
>> +               pd->vidcon1 |= VIDCON1_INV_HSYNC;
>> +       if (of_get_property(np, "samsung,fimd-inv-vsync", NULL))
>> +               pd->vidcon1 |= VIDCON1_INV_VSYNC;
>> +       if (of_get_property(np, "samsung,fimd-inv-vclk", NULL))
>> +               pd->vidcon1 |= VIDCON1_INV_VCLK;
>> +       if (of_get_property(np, "samsung,fimd-inv-vden", NULL))
>> +               pd->vidcon1 |= VIDCON1_INV_VDEN;
>> +
>> +       disp_np = of_parse_phandle(np, "samsung,fimd-display", 0);
>> +       if (!disp_np) {
>> +               dev_err(dev, "unable to find display panel info\n");
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +
>> +       if (of_property_read_u32_array(disp_np, "lcd-htiming", data, 4)) {
>> +               dev_err(dev, "invalid horizontal timing\n");
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +       pd->panel.timing.left_margin = data[0];
>> +       pd->panel.timing.right_margin = data[1];
>> +       pd->panel.timing.hsync_len = data[2];
>> +       pd->panel.timing.xres = data[3];
>> +
>> +       if (of_property_read_u32_array(disp_np, "lcd-vtiming", data, 4)) {
>> +               dev_err(dev, "invalid vertical timing\n");
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +       pd->panel.timing.upper_margin = data[0];
>> +       pd->panel.timing.lower_margin = data[1];
>> +       pd->panel.timing.vsync_len = data[2];
>> +       pd->panel.timing.yres = data[3];
>> +
>> +       of_property_read_u32(np, "samsung,fimd-frame-rate",
>> +                               &pd->panel.timing.refresh);
>> +
>> +       of_property_read_u32(np, "samsung,default-window", &pd->default_win);
>> +
>> +       of_property_read_u32(np, "samsung,fimd-win-bpp", &pd->bpp);
>> +
>> +       return pd;
>> +}
>> +#else
>> +static int exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
>> +{
>> +       return NULL;
>> +}
>> +#endif /* CONFIG_OF */
>> +
>>  static int __devinit fimd_probe(struct platform_device *pdev)
>>  {
>>         struct device *dev = &pdev->dev;
>>         struct fimd_context *ctx;
>>         struct exynos_drm_subdrv *subdrv;
>> -       struct exynos_drm_fimd_pdata *pdata;
>> +       struct exynos_drm_fimd_pdata *pdata = pdev->dev.platform_data;
>>         struct exynos_drm_panel_info *panel;
>>         struct resource *res;
>>         int win;
>> @@ -834,7 +911,11 @@ static int __devinit fimd_probe(struct platform_device *pdev)
>>
>>         DRM_DEBUG_KMS("%s\n", __FILE__);
>>
>> -       pdata = pdev->dev.platform_data;
>> +       if (pdev->dev.of_node) {
>> +               pdata = drm_fimd_dt_parse_pdata(&pdev->dev);
>> +               if (IS_ERR(pdata))
>> +                       return PTR_ERR(pdata);
>> +       }
>>         if (!pdata) {
>>                 dev_err(dev, "no platform data specified\n");
>>                 return -EINVAL;
>> @@ -1027,6 +1108,15 @@ static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
>>  };
>>  MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
>>
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id drm_fimd_dt_match[] = {
>> +       { .compatible = "samsung,exynos5-drm",
>> +         .data = &exynos5_drm_fimd_driver_data },
>
> Add also samsung,exynos4-fb.
>

Ok, will add it.

>> +       {},
>> +};
>> +MODULE_DEVICE_TABLE(of, drm_fimd_dt_match);
>> +#endif
>> +
>>  static const struct dev_pm_ops fimd_pm_ops = {
>>         SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
>>         SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
>> @@ -1040,5 +1130,6 @@ struct platform_driver fimd_driver = {
>>                 .name   = "exynos-drm-fimd",
>>                 .owner  = THIS_MODULE,
>>                 .pm     = &fimd_pm_ops,
>> +               .of_match_table = of_match_ptr(drm_fimd_dt_match),
>>         },
>>  };
>> --
>> 1.7.0.4
>>
>> _______________________________________________
>> devicetree-discuss mailing list
>> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
> Thanks.
>
> --
> - Joonyoung Shim
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH V3 2/2] video: drm: exynos: Add device tree support

[Joonyoung Shim]

On 08/17/2012 06:37 PM, Leela Krishna Amudala wrote:
> Hello,
>
> On Fri, Aug 17, 2012 at 6:55 AM, Joonyoung Shim <dofmind@gmail.com> wrote:
>> Hi,
>>
>> 2012/8/16 Leela Krishna Amudala <l.krishna@samsung.com>:
>>> Add device tree based discovery support for DRM-FIMD driver.
>>>
>>> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
>>> ---
>>>   Documentation/devicetree/bindings/fb/drm-fimd.txt |   80 +++++++++++++++++
>>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c          |   95 ++++++++++++++++++++-
>>>   2 files changed, 173 insertions(+), 2 deletions(-)
>>>   create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt b/Documentation/devicetree/bindings/fb/drm-fimd.txt
>>> new file mode 100644
>>> index 0000000..8ad8814
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
>>> @@ -0,0 +1,80 @@
>>> +* Samsung Display Controller using DRM frame work
>>> +
>>> +The display controller is used to transfer image data from memory to an
>>> +external LCD driver interface. It supports various color formats such as
>>> +rgb and yuv.
>>> +
>>> +Required properties:
>>> + - compatible: Should be "samsung,exynos5-drm" for fimd using DRM frame work.
>> Just use "samsung,exynos5-fb" and add to support exynos4-fb
>>
> In the first version of this patch set I used "samsung,exynos5-fb",
> but as per Kyungmin Park's suggestion changed it to exynos5-drm.

OK, but this doesn't mean drm device so it is right to use exynos5-fimd.
Add "exynos5-fimd" compatible and also use exynos5-fb, it is used in 
s3c-fb driver.

>>> + - reg: physical base address of the controller and length of memory
>>> +   mapped region.
>>> + - interrupts: Three interrupts should be specified. The interrupts should be
>>> +   specified in the following order.
>>> +   - VSYNC interrupt
>>> +   - FIFO level interrupt
>>> +   - FIMD System Interrupt
>>> +
>>> + - samsung,fimd-display: This property should specify the phandle of the
>>> +   display device node which holds the video interface timing with the
>>> +   below mentioned properties.
>>> +
>>> +   - lcd-htiming: Specifies the horizontal timing for the overlay. The
>>> +     horizontal timing includes four parameters in the following order.
>>> +
>>> +     - horizontal back porch (in number of lcd clocks)
>>> +     - horizontal front porch (in number of lcd clocks)
>>> +     - hsync pulse width (in number of lcd clocks)
>>> +     - Display panels X resolution.
>>> +
>>> +   - lcd-vtiming: Specifies the vertical timing for the overlay. The
>>> +     vertical timing includes four parameters in the following order.
>>> +
>>> +     - vertical back porch (in number of lcd lines)
>>> +     - vertical front porch (in number of lcd lines)
>>> +     - vsync pulse width (in number of lcd clocks)
>>> +     - Display panels Y resolution.
>>> +
>>> +
>>> + - samsung,default-window: Specifies the default window number of the fimd controller.
>>> +
>>> + - samsung,fimd-win-bpp: Specifies the bits per pixel.
>>> +
>>> +Optional properties:
>>> + - supports-mipi-panel: Specifies the lcd is mipi panel type
>> How is this property used?
>>
> As this driver can be interfaced through MIPI or eDP, Arch side code
> will check whether this property is available or not, if it is
> available then it assumes mipi panel is connected and certain clock
> rate will be set to fimd clock, otherwise assumes other panel is
> connected and other clock rate will be set at arch side.

But it is not used currently in the driver.

>
>>> + - samsung,fimd-vidout-rgb: Video output format is RGB.
>>> + - samsung,fimd-inv-vclk: invert video clock polarity.
>>> + - samsung,fimd-frame-rate: Number of video frames per second.
>>> +
>>> +Example:
>>> +
>>> +       The following is an example for the fimd controller is split into
>>> +       two portions. The SoC specific portion can be specified in the SoC
>>> +       specific dts file. The board specific portion can be specified in the
>>> +       board specific dts file.
>>> +
>>> +       - SoC Specific portion
>>> +
>>> +       fimd {
>>> +               compatible = "samsung,exynos5-drm";
>>> +               interrupt-parent = <&combiner>;
>>> +               reg = <0x14400000 0x40000>;
>>> +               interrupts = <18 5>, <18 4>, <18 6>;
>>> +       };
>>> +
>>> +       - Board Specific portion
>>> +
>>> +       lcd_fimd0: lcd_panel0 {
>>> +                       lcd-htiming = <4 4 4 480>;
>>> +                       lcd-vtiming = <4 4 4 320>;
>>> +                       supports-mipi-panel;
>>> +       };
>>> +
>>> +       fimd {
>>> +               samsung,fimd-display = <&lcd_fimd0>;
>>> +               samsung,fimd-vidout-rgb;
>>> +               samsung,fimd-inv-vclk;
>>> +               samsung,fimd-frame-rate = <60>;
>>> +               samsung,default-window = <0>;
>>> +               samsung,fimd-win-bpp = <32>;
>>> +       };
>>> +
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index 8379c59..1753846 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -18,6 +18,7 @@
>>>   #include <linux/platform_device.h>
>>>   #include <linux/clk.h>
>>>   #include <linux/pm_runtime.h>
>>> +#include <linux/of.h>
>>>
>>>   #include <video/samsung_fimd.h>
>>>   #include <drm/exynos_drm.h>
>>> @@ -103,9 +104,18 @@ struct fimd_context {
>>>          struct exynos_drm_panel_info *panel;
>>>   };
>>>
>>> +static const struct of_device_id drm_fimd_dt_match[];
>>> +
>> Please remove drm_ prefix over all.
>>
> Ok, will do that
>
>>>   static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
>>>          struct platform_device *pdev)
>>>   {
>>> +#ifdef CONFIG_OF
>>> +       if (pdev->dev.of_node) {
>>> +               const struct of_device_id *match;
>>> +               match = of_match_node(drm_fimd_dt_match, pdev->dev.of_node);
>> Use of_match_ptr().
>>
> Ok, will change it
>
>>> +               return (struct drm_fimd_driver_data *)match->data;
>>> +       }
>>> +#endif
>>>          return (struct drm_fimd_driver_data *)
>>>                  platform_get_device_id(pdev)->driver_data;
>>>   }
>>> @@ -821,12 +831,79 @@ static int fimd_power_on(struct fimd_context *ctx, bool enable)
>>>          return 0;
>>>   }
>>>
>>> +#ifdef CONFIG_OF
>>> +static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
>>> +{
>>> +       struct device_node *np = dev->of_node;
>>> +       struct device_node *disp_np;
>>> +       struct exynos_drm_fimd_pdata *pd;
>>> +       u32 data[4];
>>> +
>>> +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
>>> +       if (!pd) {
>>> +               dev_err(dev, "memory allocation for pdata failed\n");
>>> +               return ERR_PTR(-ENOMEM);
>>> +       }
>>> +
>>> +       if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
>>> +               pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
>>> +       if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
>>> +               pd->vidcon0 |= VIDCON0_VIDOUT_TV;
>> What is this?
>>
> I'll remove this property.
>
>>> +       if (of_get_property(np, "samsung,fimd-inv-hsync", NULL))
>>> +               pd->vidcon1 |= VIDCON1_INV_HSYNC;
>>> +       if (of_get_property(np, "samsung,fimd-inv-vsync", NULL))
>>> +               pd->vidcon1 |= VIDCON1_INV_VSYNC;
>>> +       if (of_get_property(np, "samsung,fimd-inv-vclk", NULL))
>>> +               pd->vidcon1 |= VIDCON1_INV_VCLK;
>>> +       if (of_get_property(np, "samsung,fimd-inv-vden", NULL))
>>> +               pd->vidcon1 |= VIDCON1_INV_VDEN;
>>> +
>>> +       disp_np = of_parse_phandle(np, "samsung,fimd-display", 0);
>>> +       if (!disp_np) {
>>> +               dev_err(dev, "unable to find display panel info\n");
>>> +               return ERR_PTR(-EINVAL);
>>> +       }
>>> +
>>> +       if (of_property_read_u32_array(disp_np, "lcd-htiming", data, 4)) {
>>> +               dev_err(dev, "invalid horizontal timing\n");
>>> +               return ERR_PTR(-EINVAL);
>>> +       }
>>> +       pd->panel.timing.left_margin = data[0];
>>> +       pd->panel.timing.right_margin = data[1];
>>> +       pd->panel.timing.hsync_len = data[2];
>>> +       pd->panel.timing.xres = data[3];
>>> +
>>> +       if (of_property_read_u32_array(disp_np, "lcd-vtiming", data, 4)) {
>>> +               dev_err(dev, "invalid vertical timing\n");
>>> +               return ERR_PTR(-EINVAL);
>>> +       }
>>> +       pd->panel.timing.upper_margin = data[0];
>>> +       pd->panel.timing.lower_margin = data[1];
>>> +       pd->panel.timing.vsync_len = data[2];
>>> +       pd->panel.timing.yres = data[3];
>>> +
>>> +       of_property_read_u32(np, "samsung,fimd-frame-rate",
>>> +                               &pd->panel.timing.refresh);
>>> +
>>> +       of_property_read_u32(np, "samsung,default-window", &pd->default_win);
>>> +
>>> +       of_property_read_u32(np, "samsung,fimd-win-bpp", &pd->bpp);
>>> +
>>> +       return pd;
>>> +}
>>> +#else
>>> +static int exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
>>> +{
>>> +       return NULL;
>>> +}
>>> +#endif /* CONFIG_OF */
>>> +
>>>   static int __devinit fimd_probe(struct platform_device *pdev)
>>>   {
>>>          struct device *dev = &pdev->dev;
>>>          struct fimd_context *ctx;
>>>          struct exynos_drm_subdrv *subdrv;
>>> -       struct exynos_drm_fimd_pdata *pdata;
>>> +       struct exynos_drm_fimd_pdata *pdata = pdev->dev.platform_data;
>>>          struct exynos_drm_panel_info *panel;
>>>          struct resource *res;
>>>          int win;
>>> @@ -834,7 +911,11 @@ static int __devinit fimd_probe(struct platform_device *pdev)
>>>
>>>          DRM_DEBUG_KMS("%s\n", __FILE__);
>>>
>>> -       pdata = pdev->dev.platform_data;
>>> +       if (pdev->dev.of_node) {
>>> +               pdata = drm_fimd_dt_parse_pdata(&pdev->dev);
>>> +               if (IS_ERR(pdata))
>>> +                       return PTR_ERR(pdata);
>>> +       }
>>>          if (!pdata) {
>>>                  dev_err(dev, "no platform data specified\n");
>>>                  return -EINVAL;
>>> @@ -1027,6 +1108,15 @@ static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
>>>   };
>>>   MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
>>>
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id drm_fimd_dt_match[] = {
>>> +       { .compatible = "samsung,exynos5-drm",
>>> +         .data = &exynos5_drm_fimd_driver_data },
>> Add also samsung,exynos4-fb.
>>
> Ok, will add it.
>
>>> +       {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, drm_fimd_dt_match);
>>> +#endif
>>> +
>>>   static const struct dev_pm_ops fimd_pm_ops = {
>>>          SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
>>>          SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
>>> @@ -1040,5 +1130,6 @@ struct platform_driver fimd_driver = {
>>>                  .name   = "exynos-drm-fimd",
>>>                  .owner  = THIS_MODULE,
>>>                  .pm     = &fimd_pm_ops,
>>> +               .of_match_table = of_match_ptr(drm_fimd_dt_match),
>>>          },
>>>   };
>>> --
>>> 1.7.0.4
>>>
>>> _______________________________________________
>>> devicetree-discuss mailing list
>>> devicetree-discuss@lists.ozlabs.org
>>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>> Thanks.
>>
>> --
>> - Joonyoung Shim
>> _______________________________________________
>> devicetree-discuss mailing list
>> devicetree-discuss@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

Re: [PATCH V3 2/2] video: drm: exynos: Add device tree support

[Sascha Hauer]

On Thu, Aug 16, 2012 at 03:38:09PM +0530, Leela Krishna Amudala wrote:
> Add device tree based discovery support for DRM-FIMD driver.
> 
> Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/fb/drm-fimd.txt |   80 +++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c          |   95 ++++++++++++++++++++-
>  2 files changed, 173 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
> 
> diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> new file mode 100644
> index 0000000..8ad8814
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> @@ -0,0 +1,80 @@
> +* Samsung Display Controller using DRM frame work
> +
> +The display controller is used to transfer image data from memory to an
> +external LCD driver interface. It supports various color formats such as
> +rgb and yuv.
> +
> +Required properties:
> + - compatible: Should be "samsung,exynos5-drm" for fimd using DRM frame work.
> + - reg: physical base address of the controller and length of memory
> +   mapped region.
> + - interrupts: Three interrupts should be specified. The interrupts should be
> +   specified in the following order.
> +   - VSYNC interrupt
> +   - FIFO level interrupt
> +   - FIMD System Interrupt
> +
> + - samsung,fimd-display: This property should specify the phandle of the
> +   display device node which holds the video interface timing with the
> +   below mentioned properties.
> +
> +   - lcd-htiming: Specifies the horizontal timing for the overlay. The
> +     horizontal timing includes four parameters in the following order.
> +
> +     - horizontal back porch (in number of lcd clocks)
> +     - horizontal front porch (in number of lcd clocks)
> +     - hsync pulse width (in number of lcd clocks)
> +     - Display panels X resolution.
> +
> +   - lcd-vtiming: Specifies the vertical timing for the overlay. The
> +     vertical timing includes four parameters in the following order.
> +
> +     - vertical back porch (in number of lcd lines)
> +     - vertical front porch (in number of lcd lines)
> +     - vsync pulse width (in number of lcd clocks)
> +     - Display panels Y resolution.

I started an approach to add a common description for displays:

https://patchwork.kernel.org/patch/1154751/https://patchwork.kernel.org/patch/1154751/

There are still comments to this approach, but I think rather than
inventing SoC specific bindings we should use a common binding.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH V3 0/2] video: drm: Add Device tree support to DRM-FIMD

[Leela Krishna Amudala]

Hello Mr. Inki Dae,
Can you please review this patch set once.

Best Wishes,
Leela Krishna Amudala.

On Thu, Aug 16, 2012 at 3:38 PM, Leela Krishna Amudala
<l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>
> This patch set adds device tree support for DRM-FIMD for Samsung's
> Exynos5250.
> It includes parsing platform data from dts file. Also, adds the driver
> data
> for exynos5 device.
>
> This patchset is based and tested on top of v3.6-rc1
> Also depends on below patchset
> http://lists.freedesktop.org/archives/dri-devel/2012-August/026076.html
>
> Changes since V2:
>         - Added driver data to exynos5-drm-fimd as per Marek Szyprowski
> suggestion
>
> Changes since V1:
>         - Corrected typo errors and changed compatibility string
>
> Leela Krishna Amudala (2):
>   drm/exynos: add platform_device_id table and driver data for exynos5
>     drm fimd
>   video: drm: exynos: Add device tree support
>
>  Documentation/devicetree/bindings/fb/drm-fimd.txt |   80 +++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c          |  151
> ++++++++++++++++++++-
>  2 files changed, 224 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH V3 2/2] video: drm: exynos: Add device tree support

[InKi Dae]

2012/8/16 Leela Krishna Amudala <l.krishna@samsung.com>:
> Add device tree based discovery support for DRM-FIMD driver.
>
> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
> ---
>  Documentation/devicetree/bindings/fb/drm-fimd.txt |   80 +++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c          |   95 ++++++++++++++++++++-
>  2 files changed, 173 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
>
> diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> new file mode 100644
> index 0000000..8ad8814
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> @@ -0,0 +1,80 @@
> +* Samsung Display Controller using DRM frame work
> +
> +The display controller is used to transfer image data from memory to an
> +external LCD driver interface. It supports various color formats such as
> +rgb and yuv.
> +
> +Required properties:
> + - compatible: Should be "samsung,exynos5-drm" for fimd using DRM frame work.
> + - reg: physical base address of the controller and length of memory
> +   mapped region.
> + - interrupts: Three interrupts should be specified. The interrupts should be
> +   specified in the following order.
> +   - VSYNC interrupt
> +   - FIFO level interrupt
> +   - FIMD System Interrupt
> +
> + - samsung,fimd-display: This property should specify the phandle of the
> +   display device node which holds the video interface timing with the
> +   below mentioned properties.
> +
> +   - lcd-htiming: Specifies the horizontal timing for the overlay. The
> +     horizontal timing includes four parameters in the following order.
> +
> +     - horizontal back porch (in number of lcd clocks)
> +     - horizontal front porch (in number of lcd clocks)
> +     - hsync pulse width (in number of lcd clocks)
> +     - Display panels X resolution.
> +
> +   - lcd-vtiming: Specifies the vertical timing for the overlay. The
> +     vertical timing includes four parameters in the following order.
> +
> +     - vertical back porch (in number of lcd lines)
> +     - vertical front porch (in number of lcd lines)
> +     - vsync pulse width (in number of lcd clocks)
> +     - Display panels Y resolution.
> +
> +
> + - samsung,default-window: Specifies the default window number of the fimd controller.
> +
> + - samsung,fimd-win-bpp: Specifies the bits per pixel.
> +
> +Optional properties:
> + - supports-mipi-panel: Specifies the lcd is mipi panel type
> + - samsung,fimd-vidout-rgb: Video output format is RGB.
> + - samsung,fimd-inv-vclk: invert video clock polarity.
> + - samsung,fimd-frame-rate: Number of video frames per second.
> +
> +Example:
> +
> +       The following is an example for the fimd controller is split into
> +       two portions. The SoC specific portion can be specified in the SoC
> +       specific dts file. The board specific portion can be specified in the
> +       board specific dts file.
> +
> +       - SoC Specific portion
> +
> +       fimd {
> +               compatible = "samsung,exynos5-drm";
> +               interrupt-parent = <&combiner>;
> +               reg = <0x14400000 0x40000>;
> +               interrupts = <18 5>, <18 4>, <18 6>;
> +       };
> +
> +       - Board Specific portion
> +
> +       lcd_fimd0: lcd_panel0 {
> +                       lcd-htiming = <4 4 4 480>;
> +                       lcd-vtiming = <4 4 4 320>;
> +                       supports-mipi-panel;
> +       };
> +
> +       fimd {
> +               samsung,fimd-display = <&lcd_fimd0>;
> +               samsung,fimd-vidout-rgb;
> +               samsung,fimd-inv-vclk;
> +               samsung,fimd-frame-rate = <60>;
> +               samsung,default-window = <0>;
> +               samsung,fimd-win-bpp = <32>;
> +       };
> +
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 8379c59..1753846 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -18,6 +18,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of.h>
>
>  #include <video/samsung_fimd.h>
>  #include <drm/exynos_drm.h>
> @@ -103,9 +104,18 @@ struct fimd_context {
>         struct exynos_drm_panel_info *panel;
>  };
>
> +static const struct of_device_id drm_fimd_dt_match[];
> +
>  static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
>         struct platform_device *pdev)
>  {
> +#ifdef CONFIG_OF
> +       if (pdev->dev.of_node) {
> +               const struct of_device_id *match;
> +               match = of_match_node(drm_fimd_dt_match, pdev->dev.of_node);
> +               return (struct drm_fimd_driver_data *)match->data;
> +       }
> +#endif
>         return (struct drm_fimd_driver_data *)
>                 platform_get_device_id(pdev)->driver_data;
>  }
> @@ -821,12 +831,79 @@ static int fimd_power_on(struct fimd_context *ctx, bool enable)
>         return 0;
>  }
>
> +#ifdef CONFIG_OF
> +static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
> +{
> +       struct device_node *np = dev->of_node;
> +       struct device_node *disp_np;
> +       struct exynos_drm_fimd_pdata *pd;
> +       u32 data[4];
> +
> +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> +       if (!pd) {
> +               dev_err(dev, "memory allocation for pdata failed\n");
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
> +               pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
> +       if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
> +               pd->vidcon0 |= VIDCON0_VIDOUT_TV;
> +       if (of_get_property(np, "samsung,fimd-inv-hsync", NULL))
> +               pd->vidcon1 |= VIDCON1_INV_HSYNC;
> +       if (of_get_property(np, "samsung,fimd-inv-vsync", NULL))
> +               pd->vidcon1 |= VIDCON1_INV_VSYNC;
> +       if (of_get_property(np, "samsung,fimd-inv-vclk", NULL))
> +               pd->vidcon1 |= VIDCON1_INV_VCLK;
> +       if (of_get_property(np, "samsung,fimd-inv-vden", NULL))
> +               pd->vidcon1 |= VIDCON1_INV_VDEN;
> +
> +       disp_np = of_parse_phandle(np, "samsung,fimd-display", 0);
> +       if (!disp_np) {
> +               dev_err(dev, "unable to find display panel info\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (of_property_read_u32_array(disp_np, "lcd-htiming", data, 4)) {
> +               dev_err(dev, "invalid horizontal timing\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +       pd->panel.timing.left_margin = data[0];
> +       pd->panel.timing.right_margin = data[1];
> +       pd->panel.timing.hsync_len = data[2];
> +       pd->panel.timing.xres = data[3];
> +
> +       if (of_property_read_u32_array(disp_np, "lcd-vtiming", data, 4)) {
> +               dev_err(dev, "invalid vertical timing\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +       pd->panel.timing.upper_margin = data[0];
> +       pd->panel.timing.lower_margin = data[1];
> +       pd->panel.timing.vsync_len = data[2];
> +       pd->panel.timing.yres = data[3];
> +
> +       of_property_read_u32(np, "samsung,fimd-frame-rate",
> +                               &pd->panel.timing.refresh);
> +
> +       of_property_read_u32(np, "samsung,default-window", &pd->default_win);
> +
> +       of_property_read_u32(np, "samsung,fimd-win-bpp", &pd->bpp);
> +
> +       return pd;
> +}
> +#else
> +static int exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)

use 'struct' instead of 'int'. maybe it's your mistake.

>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V3 1/2] drm/exynos: add platform_device_id table and driver data for exynos5 drm fimd

[InKi Dae]

2012/8/16 Leela Krishna Amudala <l.krishna@samsung.com>:
> The name of the exynos drm fimd device is renamed to exynos-drm-fimd
> and two device ids are created for exynos4-fb and exynos5-drm-fimd.
> Also, added driver data for exynos5 to pick the fimd version at runtime and
> to choose the VIDTCON register offsets accordingly.
>
> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   56 +++++++++++++++++++++++++++---
>  1 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 24c0bd4..8379c59 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -57,6 +57,18 @@
>
>  #define get_fimd_context(dev)  platform_get_drvdata(to_platform_device(dev))
>
> +enum fimd_version_type {
> +       VERSION_8, /* FIMD_VERSION8 */
> +};
> +
> +struct drm_fimd_driver_data {
> +       enum fimd_version_type fimd_ver;
> +};
> +
> +struct drm_fimd_driver_data exynos5_drm_fimd_driver_data = {
> +       .fimd_ver = VERSION_8,
> +};
> +
>  struct fimd_win_data {
>         unsigned int            offset_x;
>         unsigned int            offset_y;
> @@ -91,6 +103,13 @@ struct fimd_context {
>         struct exynos_drm_panel_info *panel;
>  };
>
> +static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
> +       struct platform_device *pdev)
> +{
> +       return (struct drm_fimd_driver_data *)
> +               platform_get_device_id(pdev)->driver_data;
> +}
> +
>  static bool fimd_display_is_connected(struct device *dev)
>  {
>         DRM_DEBUG_KMS("%s\n", __FILE__);
> @@ -194,32 +213,47 @@ static void fimd_commit(struct device *dev)
>         struct fimd_context *ctx = get_fimd_context(dev);
>         struct exynos_drm_panel_info *panel = ctx->panel;
>         struct fb_videomode *timing = &panel->timing;
> +       struct drm_fimd_driver_data *driver_data;
> +       struct platform_device *pdev = to_platform_device(dev);
>         u32 val;
>
> +       driver_data = drm_fimd_get_driver_data(pdev);
>         if (ctx->suspended)
>                 return;
>
>         DRM_DEBUG_KMS("%s\n", __FILE__);
>
>         /* setup polarity values from machine code. */
> -       writel(ctx->vidcon1, ctx->regs + VIDCON1);
> +       if (driver_data->fimd_ver == VERSION_8)
> +               writel(ctx->vidcon1, ctx->regs + FIMD_V8_VIDCON1);

where are FIMD_V8_VIDCON0/1/2 defined? it seems like that you missed some codes.

> +       else
> +               writel(ctx->vidcon1, ctx->regs + VIDCON1);
>
>         /* setup vertical timing values. */
>         val = VIDTCON0_VBPD(timing->upper_margin - 1) |
>                VIDTCON0_VFPD(timing->lower_margin - 1) |
>                VIDTCON0_VSPW(timing->vsync_len - 1);
> -       writel(val, ctx->regs + VIDTCON0);
> +       if (driver_data->fimd_ver == VERSION_8)
> +               writel(val, ctx->regs + FIMD_V8_VIDTCON0);
> +       else
> +               writel(val, ctx->regs + VIDTCON0);
>
>         /* setup horizontal timing values.  */
>         val = VIDTCON1_HBPD(timing->left_margin - 1) |
>                VIDTCON1_HFPD(timing->right_margin - 1) |
>                VIDTCON1_HSPW(timing->hsync_len - 1);
> -       writel(val, ctx->regs + VIDTCON1);
> +       if (driver_data->fimd_ver == VERSION_8)
> +               writel(val, ctx->regs + FIMD_V8_VIDTCON1);
> +       else
> +               writel(val, ctx->regs + VIDTCON1);
>
>         /* setup horizontal and vertical display size. */
>         val = VIDTCON2_LINEVAL(timing->yres - 1) |
>                VIDTCON2_HOZVAL(timing->xres - 1);
> -       writel(val, ctx->regs + VIDTCON2);
> +       if (driver_data->fimd_ver == VERSION_8)
> +               writel(val, ctx->regs + FIMD_V8_VIDTCON2);
> +       else
> +               writel(val, ctx->regs + VIDTCON2);
>
>         /* setup clock source, clock divider, enable dma. */
>         val = ctx->vidcon0;
> @@ -982,6 +1016,17 @@ static int fimd_runtime_resume(struct device *dev)
>  }
>  #endif
>
> +static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
> +       {
> +               .name           = "exynos4-fb",
> +       }, {
> +               .name           = "exynos5-drm-fimd",
> +               .driver_data    = (unsigned long)&exynos5_drm_fimd_driver_data,
> +       },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
> +
>  static const struct dev_pm_ops fimd_pm_ops = {
>         SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
>         SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
> @@ -990,8 +1035,9 @@ static const struct dev_pm_ops fimd_pm_ops = {
>  struct platform_driver fimd_driver = {
>         .probe          = fimd_probe,
>         .remove         = __devexit_p(fimd_remove),
> +       .id_table       = exynos_drm_fimd_driver_ids,
>         .driver         = {
> -               .name   = "exynos4-fb",
> +               .name   = "exynos-drm-fimd",
>                 .owner  = THIS_MODULE,
>                 .pm     = &fimd_pm_ops,
>         },
> --
> 1.7.0.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V3 2/2] video: drm: exynos: Add device tree support

[Leela Krishna Amudala]

Hello Inki Dae,

On Tue, Sep 4, 2012 at 7:42 PM, InKi Dae <inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>
> 2012/8/16 Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>:
> > Add device tree based discovery support for DRM-FIMD driver.
> >
> > Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/fb/drm-fimd.txt |   80
> > +++++++++++++++++
> >  drivers/gpu/drm/exynos/exynos_drm_fimd.c          |   95
> > ++++++++++++++++++++-
> >  2 files changed, 173 insertions(+), 2 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
> >
> > diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt
> > b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> > new file mode 100644
> > index 0000000..8ad8814
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> > @@ -0,0 +1,80 @@
> > +* Samsung Display Controller using DRM frame work
> > +
> > +The display controller is used to transfer image data from memory to an
> > +external LCD driver interface. It supports various color formats such
> > as
> > +rgb and yuv.
> > +
> > +Required properties:
> > + - compatible: Should be "samsung,exynos5-drm" for fimd using DRM frame
> > work.
> > + - reg: physical base address of the controller and length of memory
> > +   mapped region.
> > + - interrupts: Three interrupts should be specified. The interrupts
> > should be
> > +   specified in the following order.
> > +   - VSYNC interrupt
> > +   - FIFO level interrupt
> > +   - FIMD System Interrupt
> > +
> > + - samsung,fimd-display: This property should specify the phandle of
> > the
> > +   display device node which holds the video interface timing with the
> > +   below mentioned properties.
> > +
> > +   - lcd-htiming: Specifies the horizontal timing for the overlay. The
> > +     horizontal timing includes four parameters in the following order.
> > +
> > +     - horizontal back porch (in number of lcd clocks)
> > +     - horizontal front porch (in number of lcd clocks)
> > +     - hsync pulse width (in number of lcd clocks)
> > +     - Display panels X resolution.
> > +
> > +   - lcd-vtiming: Specifies the vertical timing for the overlay. The
> > +     vertical timing includes four parameters in the following order.
> > +
> > +     - vertical back porch (in number of lcd lines)
> > +     - vertical front porch (in number of lcd lines)
> > +     - vsync pulse width (in number of lcd clocks)
> > +     - Display panels Y resolution.
> > +
> > +
> > + - samsung,default-window: Specifies the default window number of the
> > fimd controller.
> > +
> > + - samsung,fimd-win-bpp: Specifies the bits per pixel.
> > +
> > +Optional properties:
> > + - supports-mipi-panel: Specifies the lcd is mipi panel type
> > + - samsung,fimd-vidout-rgb: Video output format is RGB.
> > + - samsung,fimd-inv-vclk: invert video clock polarity.
> > + - samsung,fimd-frame-rate: Number of video frames per second.
> > +
> > +Example:
> > +
> > +       The following is an example for the fimd controller is split
> > into
> > +       two portions. The SoC specific portion can be specified in the
> > SoC
> > +       specific dts file. The board specific portion can be specified
> > in the
> > +       board specific dts file.
> > +
> > +       - SoC Specific portion
> > +
> > +       fimd {
> > +               compatible = "samsung,exynos5-drm";
> > +               interrupt-parent = <&combiner>;
> > +               reg = <0x14400000 0x40000>;
> > +               interrupts = <18 5>, <18 4>, <18 6>;
> > +       };
> > +
> > +       - Board Specific portion
> > +
> > +       lcd_fimd0: lcd_panel0 {
> > +                       lcd-htiming = <4 4 4 480>;
> > +                       lcd-vtiming = <4 4 4 320>;
> > +                       supports-mipi-panel;
> > +       };
> > +
> > +       fimd {
> > +               samsung,fimd-display = <&lcd_fimd0>;
> > +               samsung,fimd-vidout-rgb;
> > +               samsung,fimd-inv-vclk;
> > +               samsung,fimd-frame-rate = <60>;
> > +               samsung,default-window = <0>;
> > +               samsung,fimd-win-bpp = <32>;
> > +       };
> > +
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > index 8379c59..1753846 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/clk.h>
> >  #include <linux/pm_runtime.h>
> > +#include <linux/of.h>
> >
> >  #include <video/samsung_fimd.h>
> >  #include <drm/exynos_drm.h>
> > @@ -103,9 +104,18 @@ struct fimd_context {
> >         struct exynos_drm_panel_info *panel;
> >  };
> >
> > +static const struct of_device_id drm_fimd_dt_match[];
> > +
> >  static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
> >         struct platform_device *pdev)
> >  {
> > +#ifdef CONFIG_OF
> > +       if (pdev->dev.of_node) {
> > +               const struct of_device_id *match;
> > +               match = of_match_node(drm_fimd_dt_match,
> > pdev->dev.of_node);
> > +               return (struct drm_fimd_driver_data *)match->data;
> > +       }
> > +#endif
> >         return (struct drm_fimd_driver_data *)
> >                 platform_get_device_id(pdev)->driver_data;
> >  }
> > @@ -821,12 +831,79 @@ static int fimd_power_on(struct fimd_context *ctx,
> > bool enable)
> >         return 0;
> >  }
> >
> > +#ifdef CONFIG_OF
> > +static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct
> > device *dev)
> > +{
> > +       struct device_node *np = dev->of_node;
> > +       struct device_node *disp_np;
> > +       struct exynos_drm_fimd_pdata *pd;
> > +       u32 data[4];
> > +
> > +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> > +       if (!pd) {
> > +               dev_err(dev, "memory allocation for pdata failed\n");
> > +               return ERR_PTR(-ENOMEM);
> > +       }
> > +
> > +       if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
> > +               pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
> > +       if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
> > +               pd->vidcon0 |= VIDCON0_VIDOUT_TV;
> > +       if (of_get_property(np, "samsung,fimd-inv-hsync", NULL))
> > +               pd->vidcon1 |= VIDCON1_INV_HSYNC;
> > +       if (of_get_property(np, "samsung,fimd-inv-vsync", NULL))
> > +               pd->vidcon1 |= VIDCON1_INV_VSYNC;
> > +       if (of_get_property(np, "samsung,fimd-inv-vclk", NULL))
> > +               pd->vidcon1 |= VIDCON1_INV_VCLK;
> > +       if (of_get_property(np, "samsung,fimd-inv-vden", NULL))
> > +               pd->vidcon1 |= VIDCON1_INV_VDEN;
> > +
> > +       disp_np = of_parse_phandle(np, "samsung,fimd-display", 0);
> > +       if (!disp_np) {
> > +               dev_err(dev, "unable to find display panel info\n");
> > +               return ERR_PTR(-EINVAL);
> > +       }
> > +
> > +       if (of_property_read_u32_array(disp_np, "lcd-htiming", data, 4))
> > {
> > +               dev_err(dev, "invalid horizontal timing\n");
> > +               return ERR_PTR(-EINVAL);
> > +       }
> > +       pd->panel.timing.left_margin = data[0];
> > +       pd->panel.timing.right_margin = data[1];
> > +       pd->panel.timing.hsync_len = data[2];
> > +       pd->panel.timing.xres = data[3];
> > +
> > +       if (of_property_read_u32_array(disp_np, "lcd-vtiming", data, 4))
> > {
> > +               dev_err(dev, "invalid vertical timing\n");
> > +               return ERR_PTR(-EINVAL);
> > +       }
> > +       pd->panel.timing.upper_margin = data[0];
> > +       pd->panel.timing.lower_margin = data[1];
> > +       pd->panel.timing.vsync_len = data[2];
> > +       pd->panel.timing.yres = data[3];
> > +
> > +       of_property_read_u32(np, "samsung,fimd-frame-rate",
> > +                               &pd->panel.timing.refresh);
> > +
> > +       of_property_read_u32(np, "samsung,default-window",
> > &pd->default_win);
> > +
> > +       of_property_read_u32(np, "samsung,fimd-win-bpp", &pd->bpp);
> > +
> > +       return pd;
> > +}
> > +#else
> > +static int exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device
> > *dev)
>
> use 'struct' instead of 'int'. maybe it's your mistake.
>

Yes, This is my mistake, will change in next version.
Thanks for reviewing the patch.

> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V3 1/2] drm/exynos: add platform_device_id table and driver data for exynos5 drm fimd

[Leela Krishna Amudala]

Hello Inki Dae,

On Tue, Sep 4, 2012 at 7:45 PM, InKi Dae <inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> 2012/8/16 Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>:
>> The name of the exynos drm fimd device is renamed to exynos-drm-fimd
>> and two device ids are created for exynos4-fb and exynos5-drm-fimd.
>> Also, added driver data for exynos5 to pick the fimd version at runtime and
>> to choose the VIDTCON register offsets accordingly.
>>
>> Signed-off-by: Leela Krishna Amudala <l.krishna-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   56 +++++++++++++++++++++++++++---
>>  1 files changed, 51 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 24c0bd4..8379c59 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -57,6 +57,18 @@
>>
>>  #define get_fimd_context(dev)  platform_get_drvdata(to_platform_device(dev))
>>
>> +enum fimd_version_type {
>> +       VERSION_8, /* FIMD_VERSION8 */
>> +};
>> +
>> +struct drm_fimd_driver_data {
>> +       enum fimd_version_type fimd_ver;
>> +};
>> +
>> +struct drm_fimd_driver_data exynos5_drm_fimd_driver_data = {
>> +       .fimd_ver = VERSION_8,
>> +};
>> +
>>  struct fimd_win_data {
>>         unsigned int            offset_x;
>>         unsigned int            offset_y;
>> @@ -91,6 +103,13 @@ struct fimd_context {
>>         struct exynos_drm_panel_info *panel;
>>  };
>>
>> +static inline struct drm_fimd_driver_data *drm_fimd_get_driver_data(
>> +       struct platform_device *pdev)
>> +{
>> +       return (struct drm_fimd_driver_data *)
>> +               platform_get_device_id(pdev)->driver_data;
>> +}
>> +
>>  static bool fimd_display_is_connected(struct device *dev)
>>  {
>>         DRM_DEBUG_KMS("%s\n", __FILE__);
>> @@ -194,32 +213,47 @@ static void fimd_commit(struct device *dev)
>>         struct fimd_context *ctx = get_fimd_context(dev);
>>         struct exynos_drm_panel_info *panel = ctx->panel;
>>         struct fb_videomode *timing = &panel->timing;
>> +       struct drm_fimd_driver_data *driver_data;
>> +       struct platform_device *pdev = to_platform_device(dev);
>>         u32 val;
>>
>> +       driver_data = drm_fimd_get_driver_data(pdev);
>>         if (ctx->suspended)
>>                 return;
>>
>>         DRM_DEBUG_KMS("%s\n", __FILE__);
>>
>>         /* setup polarity values from machine code. */
>> -       writel(ctx->vidcon1, ctx->regs + VIDCON1);
>> +       if (driver_data->fimd_ver == VERSION_8)
>> +               writel(ctx->vidcon1, ctx->regs + FIMD_V8_VIDCON1);
>
> where are FIMD_V8_VIDCON0/1/2 defined? it seems like that you missed some codes.
>

Details are given in [PATCH V3 0/2] video: drm: Add Device tree
support to DRM-FIMD
This patch set depends on
http://lists.freedesktop.org/archives/dri-devel/2012-August/026076.html,
which is already merged into Kukjin's for-next branch.

>> +       else
>> +               writel(ctx->vidcon1, ctx->regs + VIDCON1);
>>
>>         /* setup vertical timing values. */
>>         val = VIDTCON0_VBPD(timing->upper_margin - 1) |
>>                VIDTCON0_VFPD(timing->lower_margin - 1) |
>>                VIDTCON0_VSPW(timing->vsync_len - 1);
>> -       writel(val, ctx->regs + VIDTCON0);
>> +       if (driver_data->fimd_ver == VERSION_8)
>> +               writel(val, ctx->regs + FIMD_V8_VIDTCON0);
>> +       else
>> +               writel(val, ctx->regs + VIDTCON0);
>>
>>         /* setup horizontal timing values.  */
>>         val = VIDTCON1_HBPD(timing->left_margin - 1) |
>>                VIDTCON1_HFPD(timing->right_margin - 1) |
>>                VIDTCON1_HSPW(timing->hsync_len - 1);
>> -       writel(val, ctx->regs + VIDTCON1);
>> +       if (driver_data->fimd_ver == VERSION_8)
>> +               writel(val, ctx->regs + FIMD_V8_VIDTCON1);
>> +       else
>> +               writel(val, ctx->regs + VIDTCON1);
>>
>>         /* setup horizontal and vertical display size. */
>>         val = VIDTCON2_LINEVAL(timing->yres - 1) |
>>                VIDTCON2_HOZVAL(timing->xres - 1);
>> -       writel(val, ctx->regs + VIDTCON2);
>> +       if (driver_data->fimd_ver == VERSION_8)
>> +               writel(val, ctx->regs + FIMD_V8_VIDTCON2);
>> +       else
>> +               writel(val, ctx->regs + VIDTCON2);
>>
>>         /* setup clock source, clock divider, enable dma. */
>>         val = ctx->vidcon0;
>> @@ -982,6 +1016,17 @@ static int fimd_runtime_resume(struct device *dev)
>>  }
>>  #endif
>>
>> +static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
>> +       {
>> +               .name           = "exynos4-fb",
>> +       }, {
>> +               .name           = "exynos5-drm-fimd",
>> +               .driver_data    = (unsigned long)&exynos5_drm_fimd_driver_data,
>> +       },
>> +       {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);
>> +
>>  static const struct dev_pm_ops fimd_pm_ops = {
>>         SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
>>         SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
>> @@ -990,8 +1035,9 @@ static const struct dev_pm_ops fimd_pm_ops = {
>>  struct platform_driver fimd_driver = {
>>         .probe          = fimd_probe,
>>         .remove         = __devexit_p(fimd_remove),
>> +       .id_table       = exynos_drm_fimd_driver_ids,
>>         .driver         = {
>> -               .name   = "exynos4-fb",
>> +               .name   = "exynos-drm-fimd",
>>                 .owner  = THIS_MODULE,
>>                 .pm     = &fimd_pm_ops,
>>         },
>> --
>> 1.7.0.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V3 1/2] drm/exynos: add platform_device_id table and driver data for exynos5 drm fimd

[Tomasz Figa]

Hi Leela,

See my comments inline.

On Thursday 16 of August 2012 12:08:08 Leela Krishna Amudala wrote:
> +enum fimd_version_type {
> +	VERSION_8, /* FIMD_VERSION8 */
> +};
> +
> +struct drm_fimd_driver_data {
> +	enum fimd_version_type fimd_ver;
> +};
> +
> +struct drm_fimd_driver_data exynos5_drm_fimd_driver_data = {
> +	.fimd_ver = VERSION_8,
> +};

I think that the approach with timing_base, as suggested by Joonyoung Shim, 
would be much cleaner.

> +static struct platform_device_id exynos_drm_fimd_driver_ids[] = {
> +	{
> +		.name		= "exynos4-fb",
> +	}, {
> +		.name		= "exynos5-drm-fimd",
> +		.driver_data	= (unsigned long)&exynos5_drm_fimd_driver_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(platform, exynos_drm_fimd_driver_ids);

If I see correctly, this will crash on a null pointer dereference on 
Exynos4 without DT, because of NULL driver_data.

P.S. I think you should CC linux-arm-kernel and linux-samsung-soc lists 
when submitting patches related to ARM and Samsung SoCs.

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center