[PATCH 0/8] tilcdc-panel: Backlight and GPIO devicetree support

[PATCH 0/8] tilcdc-panel: Backlight and GPIO devicetree support

[Ezequiel Garcia]

Hello all,

This patchset adds the required changes to support an optional backlight
and GPIO for the tilcdc panel driver.

There was some code to support a backlight, but it was somewhat broken
and undocumented. I've followed the nice implementation in panel-simple
and added a similar one here.

The enable GPIO is required to turn on and off devices with such capability.
Also here, I've followed panel-simple which looks correct.

In addition to this there are very minor cosmetic cleanups and a larger
error path fix in tilcdc's DRM driver .load error path.

This patchset applies on top of drm-next branch which contains the latest
tilcdc pushed by Guido.

If at all possible, I'd like to get this merged for v3.17. If a pull request
is needed, don't hesitate to ask and I'll prepare one.

Comments and tests welcome!

Ezequiel Garcia (8):
  drm/tilcdc: Fix the error path in tilcdc_load()
  drm/tilcdc: panel: Add missing of_node_put
  drm/tilcdc: panel: Remove unused variable
  drm/tilcdc: panel: Spurious whitespace removal
  drm/tilcdc: panel: Use devm_kzalloc to simplify the error path
  drm/tilcdc: panel: Fix backlight devicetree support
  drm/tilcdc: panel: Set return value explicitly
  drm/tilcdc: panel: Add support for enable GPIO

 .../devicetree/bindings/drm/tilcdc/panel.txt       |  7 ++
 drivers/gpu/drm/tilcdc/tilcdc_drv.c                | 60 +++++++++++++++---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c              | 74 +++++++++++++++++-----
 3 files changed, 114 insertions(+), 27 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/8] drm/tilcdc: Fix the error path in tilcdc_load()

[Ezequiel Garcia]

The current error path calls tilcdc_unload() in case of an error to release
the resources. However, this is wrong because not all resources have been
allocated by the time an error occurs in tilcdc_load().

To fix it, this commit adds proper labels to bail out at the different
stages in the load function, and release only the resources actually allocated.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 60 ++++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 6be623b..000428e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -84,6 +84,7 @@ static int modeset_init(struct drm_device *dev)
 	if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) {
 		/* oh nos! */
 		dev_err(dev->dev, "no encoders/connectors found\n");
+		drm_mode_config_cleanup(dev);
 		return -ENXIO;
 	}
 
@@ -172,33 +173,37 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	dev->dev_private = priv;
 
 	priv->wq = alloc_ordered_workqueue("tilcdc", 0);
+	if (!priv->wq) {
+		ret = -ENOMEM;
+		goto fail_free_priv;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(dev->dev, "failed to get memory resource\n");
 		ret = -EINVAL;
-		goto fail;
+		goto fail_free_wq;
 	}
 
 	priv->mmio = ioremap_nocache(res->start, resource_size(res));
 	if (!priv->mmio) {
 		dev_err(dev->dev, "failed to ioremap\n");
 		ret = -ENOMEM;
-		goto fail;
+		goto fail_free_wq;
 	}
 
 	priv->clk = clk_get(dev->dev, "fck");
 	if (IS_ERR(priv->clk)) {
 		dev_err(dev->dev, "failed to get functional clock\n");
 		ret = -ENODEV;
-		goto fail;
+		goto fail_iounmap;
 	}
 
 	priv->disp_clk = clk_get(dev->dev, "dpll_disp_ck");
 	if (IS_ERR(priv->clk)) {
 		dev_err(dev->dev, "failed to get display clock\n");
 		ret = -ENODEV;
-		goto fail;
+		goto fail_put_clk;
 	}
 
 #ifdef CONFIG_CPU_FREQ
@@ -208,7 +213,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 			CPUFREQ_TRANSITION_NOTIFIER);
 	if (ret) {
 		dev_err(dev->dev, "failed to register cpufreq notifier\n");
-		goto fail;
+		goto fail_put_disp_clk;
 	}
 #endif
 
@@ -253,13 +258,13 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	ret = modeset_init(dev);
 	if (ret < 0) {
 		dev_err(dev->dev, "failed to initialize mode setting\n");
-		goto fail;
+		goto fail_cpufreq_unregister;
 	}
 
 	ret = drm_vblank_init(dev, 1);
 	if (ret < 0) {
 		dev_err(dev->dev, "failed to initialize vblank\n");
-		goto fail;
+		goto fail_mode_config_cleanup;
 	}
 
 	pm_runtime_get_sync(dev->dev);
@@ -267,7 +272,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	pm_runtime_put_sync(dev->dev);
 	if (ret < 0) {
 		dev_err(dev->dev, "failed to install IRQ handler\n");
-		goto fail;
+		goto fail_vblank_cleanup;
 	}
 
 	platform_set_drvdata(pdev, dev);
@@ -283,13 +288,48 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	priv->fbdev = drm_fbdev_cma_init(dev, bpp,
 			dev->mode_config.num_crtc,
 			dev->mode_config.num_connector);
+	if (IS_ERR(priv->fbdev)) {
+		ret = PTR_ERR(priv->fbdev);
+		goto fail_irq_uninstall;
+	}
 
 	drm_kms_helper_poll_init(dev);
 
 	return 0;
 
-fail:
-	tilcdc_unload(dev);
+fail_irq_uninstall:
+	pm_runtime_get_sync(dev->dev);
+	drm_irq_uninstall(dev);
+	pm_runtime_put_sync(dev->dev);
+
+fail_vblank_cleanup:
+	drm_vblank_cleanup(dev);
+
+fail_mode_config_cleanup:
+	drm_mode_config_cleanup(dev);
+
+fail_cpufreq_unregister:
+	pm_runtime_disable(dev->dev);
+#ifdef CONFIG_CPU_FREQ
+	cpufreq_unregister_notifier(&priv->freq_transition,
+			CPUFREQ_TRANSITION_NOTIFIER);
+fail_put_disp_clk:
+	clk_put(priv->disp_clk);
+#endif
+
+fail_put_clk:
+	clk_put(priv->clk);
+
+fail_iounmap:
+	iounmap(priv->mmio);
+
+fail_free_wq:
+	flush_workqueue(priv->wq);
+	destroy_workqueue(priv->wq);
+
+fail_free_priv:
+	dev->dev_private = NULL;
+	kfree(priv);
 	return ret;
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/8] drm/tilcdc: panel: Add missing of_node_put

[Ezequiel Garcia]

This commit adds the missing calls to of_node_put to release the node
that's currently held by the of_get_child_by_name() call in the panel
info parsing code.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 4c7aa1d..d581c53 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -311,6 +311,7 @@ static struct tilcdc_panel_info *of_get_panel_info(struct device_node *np)
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (!info) {
 		pr_err("%s: allocation failed\n", __func__);
+		of_node_put(info_np);
 		return NULL;
 	}
 
@@ -331,8 +332,10 @@ static struct tilcdc_panel_info *of_get_panel_info(struct device_node *np)
 	if (ret) {
 		pr_err("%s: error reading panel-info properties\n", __func__);
 		kfree(info);
+		of_node_put(info_np);
 		return NULL;
 	}
+	of_node_put(info_np);
 
 	return info;
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/8] drm/tilcdc: panel: Remove unused variable

[Ezequiel Garcia]

Just a trivial cleanup to remove the variable.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index d581c53..8f88bfd 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -340,8 +340,6 @@ static struct tilcdc_panel_info *of_get_panel_info(struct device_node *np)
 	return info;
 }
 
-static struct of_device_id panel_of_match[];
-
 static int panel_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] drm/tilcdc: panel: Spurious whitespace removal

[Ezequiel Garcia]

Just a cosmetic cleanup.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 8f88bfd..4b36e68 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -348,7 +348,6 @@ static int panel_probe(struct platform_device *pdev)
 	struct pinctrl *pinctrl;
 	int ret = -EINVAL;
 
-
 	/* bail out early if no DT data: */
 	if (!node) {
 		dev_err(&pdev->dev, "device-tree data is missing\n");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/8] drm/tilcdc: panel: Use devm_kzalloc to simplify the error path

[Ezequiel Garcia]

Using the managed variant to allocate the resource makes the code simpler
and less error-prone.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 4b36e68..c716c12 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -354,7 +354,7 @@ static int panel_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	panel_mod = kzalloc(sizeof(*panel_mod), GFP_KERNEL);
+	panel_mod = devm_kzalloc(&pdev->dev, sizeof(*panel_mod), GFP_KERNEL);
 	if (!panel_mod)
 		return -ENOMEM;
 
@@ -391,7 +391,6 @@ fail_timings:
 	display_timings_release(panel_mod->timings);
 
 fail_free:
-	kfree(panel_mod);
 	tilcdc_module_cleanup(mod);
 	return ret;
 }
@@ -405,7 +404,6 @@ static int panel_remove(struct platform_device *pdev)
 
 	tilcdc_module_cleanup(mod);
 	kfree(panel_mod->info);
-	kfree(panel_mod);
 
 	return 0;
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/8] drm/tilcdc: panel: Fix backlight devicetree support

[Ezequiel Garcia]

The current backlight support is broken; the driver expects a backlight-class
in the panel devicetree node. Fix this by implementing it properly, getting
an optional backlight from a phandle.

This shouldn't cause any backward-compatibility DT issue because the current
implementation doesn't work and is not even documented.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 .../devicetree/bindings/drm/tilcdc/panel.txt       |  5 +++++
 drivers/gpu/drm/tilcdc/tilcdc_panel.c              | 23 +++++++++++++++++-----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/drm/tilcdc/panel.txt b/Documentation/devicetree/bindings/drm/tilcdc/panel.txt
index 9301c33..10a06e8 100644
--- a/Documentation/devicetree/bindings/drm/tilcdc/panel.txt
+++ b/Documentation/devicetree/bindings/drm/tilcdc/panel.txt
@@ -18,6 +18,9 @@ Required properties:
    Documentation/devicetree/bindings/video/display-timing.txt for display
    timing binding details.
 
+Optional properties:
+- backlight: phandle of the backlight device attached to the panel
+
 Recommended properties:
  - pinctrl-names, pinctrl-0: the pincontrol settings to configure
    muxing properly for pins that connect to TFP410 device
@@ -29,6 +32,8 @@ Example:
 		compatible = "ti,tilcdc,panel";
 		pinctrl-names = "default";
 		pinctrl-0 = <&bone_lcd3_cape_lcd_pins>;
+		backlight = <&backlight>;
+
 		panel-info {
 			ac-bias           = <255>;
 			ac-bias-intrpt    = <0>;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index c716c12..3dcf08e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -342,7 +342,7 @@ static struct tilcdc_panel_info *of_get_panel_info(struct device_node *np)
 
 static int panel_probe(struct platform_device *pdev)
 {
-	struct device_node *node = pdev->dev.of_node;
+	struct device_node *bl_node, *node = pdev->dev.of_node;
 	struct panel_module *panel_mod;
 	struct tilcdc_module *mod;
 	struct pinctrl *pinctrl;
@@ -358,6 +358,17 @@ static int panel_probe(struct platform_device *pdev)
 	if (!panel_mod)
 		return -ENOMEM;
 
+	bl_node = of_parse_phandle(node, "backlight", 0);
+	if (bl_node) {
+		panel_mod->backlight = of_find_backlight_by_node(bl_node);
+		of_node_put(bl_node);
+
+		if (!panel_mod->backlight)
+			return -EPROBE_DEFER;
+
+		dev_info(&pdev->dev, "found backlight\n");
+	}
+
 	mod = &panel_mod->base;
 	pdev->dev.platform_data = mod;
 
@@ -381,10 +392,6 @@ static int panel_probe(struct platform_device *pdev)
 
 	mod->preferred_bpp = panel_mod->info->bpp;
 
-	panel_mod->backlight = of_find_backlight_by_node(node);
-	if (panel_mod->backlight)
-		dev_info(&pdev->dev, "found backlight\n");
-
 	return 0;
 
 fail_timings:
@@ -392,6 +399,8 @@ fail_timings:
 
 fail_free:
 	tilcdc_module_cleanup(mod);
+	if (panel_mod->backlight)
+		put_device(&panel_mod->backlight->dev);
 	return ret;
 }
 
@@ -399,6 +408,10 @@ static int panel_remove(struct platform_device *pdev)
 {
 	struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
 	struct panel_module *panel_mod = to_panel_module(mod);
+	struct backlight_device *backlight = panel_mod->backlight;
+
+	if (backlight)
+		put_device(&backlight->dev);
 
 	display_timings_release(panel_mod->timings);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] drm/tilcdc: panel: Set return value explicitly

[Ezequiel Garcia]

Instead of setting an initial value for the return code, set it explicitly
on each error path. This is just a cosmetic cleanup, as preparation for the
enable GPIO support.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 3dcf08e..f2a5b23 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -346,7 +346,7 @@ static int panel_probe(struct platform_device *pdev)
 	struct panel_module *panel_mod;
 	struct tilcdc_module *mod;
 	struct pinctrl *pinctrl;
-	int ret = -EINVAL;
+	int ret;
 
 	/* bail out early if no DT data: */
 	if (!node) {
@@ -381,12 +381,14 @@ static int panel_probe(struct platform_device *pdev)
 	panel_mod->timings = of_get_display_timings(node);
 	if (!panel_mod->timings) {
 		dev_err(&pdev->dev, "could not get panel timings\n");
+		ret = -EINVAL;
 		goto fail_free;
 	}
 
 	panel_mod->info = of_get_panel_info(node);
 	if (!panel_mod->info) {
 		dev_err(&pdev->dev, "could not get panel info\n");
+		ret = -EINVAL;
 		goto fail_timings;
 	}
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] drm/tilcdc: panel: Add support for enable GPIO

[Ezequiel Garcia]

In order to support the "enable GPIO" available in many panel devices,
this commit adds a proper devicetree binding.

By providing an enable GPIO in the devicetree, the driver can now turn
off and on the panel device, and/or the backlight device. Both the
backlight and the GPIO are optional properties.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 .../devicetree/bindings/drm/tilcdc/panel.txt       |  2 ++
 drivers/gpu/drm/tilcdc/tilcdc_panel.c              | 37 +++++++++++++++++++---
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/drm/tilcdc/panel.txt b/Documentation/devicetree/bindings/drm/tilcdc/panel.txt
index 10a06e8..4ab9e23 100644
--- a/Documentation/devicetree/bindings/drm/tilcdc/panel.txt
+++ b/Documentation/devicetree/bindings/drm/tilcdc/panel.txt
@@ -20,6 +20,7 @@ Required properties:
 
 Optional properties:
 - backlight: phandle of the backlight device attached to the panel
+- enable-gpios: GPIO pin to enable or disable the panel
 
 Recommended properties:
  - pinctrl-names, pinctrl-0: the pincontrol settings to configure
@@ -33,6 +34,7 @@ Example:
 		pinctrl-names = "default";
 		pinctrl-0 = <&bone_lcd3_cape_lcd_pins>;
 		backlight = <&backlight>;
+		enable-gpios = <&gpio3 19 0>;
 
 		panel-info {
 			ac-bias           = <255>;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index f2a5b23..7a03158 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -18,6 +18,7 @@
 #include <linux/pinctrl/pinmux.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/backlight.h>
+#include <linux/gpio/consumer.h>
 #include <video/display_timing.h>
 #include <video/of_display_timing.h>
 #include <video/videomode.h>
@@ -29,6 +30,7 @@ struct panel_module {
 	struct tilcdc_panel_info *info;
 	struct display_timings *timings;
 	struct backlight_device *backlight;
+	struct gpio_desc *enable_gpio;
 };
 #define to_panel_module(x) container_of(x, struct panel_module, base)
 
@@ -55,13 +57,17 @@ static void panel_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct panel_encoder *panel_encoder = to_panel_encoder(encoder);
 	struct backlight_device *backlight = panel_encoder->mod->backlight;
+	struct gpio_desc *gpio = panel_encoder->mod->enable_gpio;
 
-	if (!backlight)
-		return;
+	if (backlight) {
+		backlight->props.power = mode == DRM_MODE_DPMS_ON ?
+					 FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
+		backlight_update_status(backlight);
+	}
 
-	backlight->props.power = mode == DRM_MODE_DPMS_ON
-				     ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
-	backlight_update_status(backlight);
+	if (gpio)
+		gpiod_set_value_cansleep(gpio,
+					 mode == DRM_MODE_DPMS_ON ? 1 : 0);
 }
 
 static bool panel_encoder_mode_fixup(struct drm_encoder *encoder,
@@ -369,6 +375,25 @@ static int panel_probe(struct platform_device *pdev)
 		dev_info(&pdev->dev, "found backlight\n");
 	}
 
+	panel_mod->enable_gpio = devm_gpiod_get(&pdev->dev, "enable");
+	if (IS_ERR(panel_mod->enable_gpio)) {
+		ret = PTR_ERR(panel_mod->enable_gpio);
+		if (ret != -ENOENT) {
+			dev_err(&pdev->dev, "failed to request enable GPIO\n");
+			goto fail_backlight;
+		}
+
+		/* Optional GPIO is not here, continue silently. */
+		panel_mod->enable_gpio = NULL;
+	} else {
+		ret = gpiod_direction_output(panel_mod->enable_gpio, 0);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to setup GPIO\n");
+			goto fail_backlight;
+		}
+		dev_info(&pdev->dev, "found enable GPIO\n");
+	}
+
 	mod = &panel_mod->base;
 	pdev->dev.platform_data = mod;
 
@@ -401,6 +426,8 @@ fail_timings:
 
 fail_free:
 	tilcdc_module_cleanup(mod);
+
+fail_backlight:
 	if (panel_mod->backlight)
 		put_device(&panel_mod->backlight->dev);
 	return ret;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/8] drm/tilcdc: panel: Add support for enable GPIO

[Fabio Estevam]

On Fri, Jul 11, 2014 at 11:18 AM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> In order to support the "enable GPIO" available in many panel devices,
> this commit adds a proper devicetree binding.
>
> By providing an enable GPIO in the devicetree, the driver can now turn
> off and on the panel device, and/or the backlight device. Both the
> backlight and the GPIO are optional properties.
> +       panel_mod->enable_gpio = devm_gpiod_get(&pdev->dev, "enable");
> +       if (IS_ERR(panel_mod->enable_gpio)) {
> +               ret = PTR_ERR(panel_mod->enable_gpio);
> +               if (ret != -ENOENT) {

Shouldn't this be controlled by a regulator instead? What if the panel
is powered from a PMIC output?

Re: [PATCH 8/8] drm/tilcdc: panel: Add support for enable GPIO

[Ezequiel Garcia]

Hello Fabio,

On 11 Jul 12:08 PM, Fabio Estevam wrote:
> On Fri, Jul 11, 2014 at 11:18 AM, Ezequiel Garcia
> <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
> > In order to support the "enable GPIO" available in many panel devices,
> > this commit adds a proper devicetree binding.
> >
> > By providing an enable GPIO in the devicetree, the driver can now turn
> > off and on the panel device, and/or the backlight device. Both the
> > backlight and the GPIO are optional properties.
> > +       panel_mod->enable_gpio = devm_gpiod_get(&pdev->dev, "enable");
> > +       if (IS_ERR(panel_mod->enable_gpio)) {
> > +               ret = PTR_ERR(panel_mod->enable_gpio);
> > +               if (ret != -ENOENT) {
> 
> Shouldn't this be controlled by a regulator instead? What if the panel
> is powered from a PMIC output?

I'm not sure I understand how is that related. I have a New Heaven LCD panel
(NHD-4.3-480272EF-ATXL#-T) and it has a signal called "Display On/Off" that
I'm using to enable and disable the panel from a GPIO.

This is useful when switching the output from the panel to the HDMI for
instance, and turn off the display panel when the output goes to the HDMI.

Probably I'm missing something, I can't really see how regulators fit here.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] tilcdc-panel: Backlight and GPIO devicetree support

[Ezequiel Garcia]

On 11 Jul 11:18 AM, Ezequiel Garcia wrote:
> Hello all,
> 
> This patchset adds the required changes to support an optional backlight
> and GPIO for the tilcdc panel driver.
> 
> There was some code to support a backlight, but it was somewhat broken
> and undocumented. I've followed the nice implementation in panel-simple
> and added a similar one here.
> 
> The enable GPIO is required to turn on and off devices with such capability.
> Also here, I've followed panel-simple which looks correct.
> 
> In addition to this there are very minor cosmetic cleanups and a larger
> error path fix in tilcdc's DRM driver .load error path.
> 
> This patchset applies on top of drm-next branch which contains the latest
> tilcdc pushed by Guido.
> 
> If at all possible, I'd like to get this merged for v3.17. If a pull request
> is needed, don't hesitate to ask and I'll prepare one.
> 
> Comments and tests welcome!
> 

Dave,

If there are no comments and we are still in time, I'd like to merge this for
v3.17. Let me know if you need a pull.
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] tilcdc-panel: Backlight and GPIO devicetree support

[Darren Etheridge]

On 07/11/2014 09:18 AM, Ezequiel Garcia wrote:
> Hello all,
>
> This patchset adds the required changes to support an optional backlight
> and GPIO for the tilcdc panel driver.
>
> There was some code to support a backlight, but it was somewhat broken
> and undocumented. I've followed the nice implementation in panel-simple
> and added a similar one here.
>
> The enable GPIO is required to turn on and off devices with such capability.
> Also here, I've followed panel-simple which looks correct.
>
> In addition to this there are very minor cosmetic cleanups and a larger
> error path fix in tilcdc's DRM driver .load error path.
>
> This patchset applies on top of drm-next branch which contains the latest
> tilcdc pushed by Guido.
>
> If at all possible, I'd like to get this merged for v3.17. If a pull request
> is needed, don't hesitate to ask and I'll prepare one.
>
> Comments and tests welcome!
>

All of the changes seem to make sense and I tested on AM335x-EVM both 
with and without the addition "backlight = " in the dts for the panel node.

I see no issues in either case, continued to work as before.

Tested against 3.16-rc6 with this patchset and the earlier patchset from 
Guido applied.

Also tested on BeagleBone Black even though it doesn't have a panel, 
just to make sure nothing changed there.

For the series:
Tested-by: Darren Etheridge <detheridge@ti.com>

> Ezequiel Garcia (8):
>    drm/tilcdc: Fix the error path in tilcdc_load()
>    drm/tilcdc: panel: Add missing of_node_put
>    drm/tilcdc: panel: Remove unused variable
>    drm/tilcdc: panel: Spurious whitespace removal
>    drm/tilcdc: panel: Use devm_kzalloc to simplify the error path
>    drm/tilcdc: panel: Fix backlight devicetree support
>    drm/tilcdc: panel: Set return value explicitly
>    drm/tilcdc: panel: Add support for enable GPIO
>
>   .../devicetree/bindings/drm/tilcdc/panel.txt       |  7 ++
>   drivers/gpu/drm/tilcdc/tilcdc_drv.c                | 60 +++++++++++++++---
>   drivers/gpu/drm/tilcdc/tilcdc_panel.c              | 74 +++++++++++++++++-----
>   3 files changed, 114 insertions(+), 27 deletions(-)
>

Re: [PATCH 0/8] tilcdc-panel: Backlight and GPIO devicetree support

[Ezequiel Garcia]

On 24 Jul 04:25 PM, Darren Etheridge wrote:
> On 07/11/2014 09:18 AM, Ezequiel Garcia wrote:
> >Hello all,
> >
> >This patchset adds the required changes to support an optional backlight
> >and GPIO for the tilcdc panel driver.
> >
> >There was some code to support a backlight, but it was somewhat broken
> >and undocumented. I've followed the nice implementation in panel-simple
> >and added a similar one here.
> >
> >The enable GPIO is required to turn on and off devices with such capability.
> >Also here, I've followed panel-simple which looks correct.
> >
> >In addition to this there are very minor cosmetic cleanups and a larger
> >error path fix in tilcdc's DRM driver .load error path.
> >
> >This patchset applies on top of drm-next branch which contains the latest
> >tilcdc pushed by Guido.
> >
> >If at all possible, I'd like to get this merged for v3.17. If a pull request
> >is needed, don't hesitate to ask and I'll prepare one.
> >
> >Comments and tests welcome!
> >
> 
> All of the changes seem to make sense and I tested on AM335x-EVM both with
> and without the addition "backlight = " in the dts for the panel node.
> 
> I see no issues in either case, continued to work as before.
> 
> Tested against 3.16-rc6 with this patchset and the earlier patchset from
> Guido applied.
> 
> Also tested on BeagleBone Black even though it doesn't have a panel, just to
> make sure nothing changed there.
> 
> For the series:
> Tested-by: Darren Etheridge <detheridge@ti.com>
> 

Thanks a lot for the test!
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar