<div dir="ltr">Hi Jingoo,<br><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Apr 18, 2014 at 2:17 PM, Jingoo Han <span dir="ltr"><<a href="mailto:jg1.han@samsung.com" target="_blank">jg1.han@samsung.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5">On Wednesday, April 16, 2014 11:33 PM, Ajay Kumar wrote:<br>
><br>
> This patch adds a simple driver to handle all the LCD and LED<br>
> powerup/down routines needed to support eDP/eDP-LVDS panels<br>
> supported on exynos boards.<br>
><br>
> Most of the eDP/LVDS panels need this sequence for powerup:<br>
>       -- LCD unit powerup/LCD_EN<br>
>       -- video data on<br>
>       -- LED unit powerup/BL_EN<br>
><br>
> The LCD and LED units are usually powered up via regulators,<br>
> and almost on all boards, we will have a BL_EN pin to enable/<br>
> disable the backlight. Sometimes, we can have LCD_EN switches<br>
> as well. The routines in this driver can be used to control<br>
> panel power sequence on such boards.<br>
><br>
> Signed-off-by: Ajay Kumar <<a href="mailto:ajaykumar.rs@samsung.com">ajaykumar.rs@samsung.com</a>><br>
> ---<br>
>  .../devicetree/bindings/panel/exynos-dp-panel.txt  |  32 ++++<br>
>  drivers/gpu/drm/panel/Kconfig                      |   9 +<br>
>  drivers/gpu/drm/panel/Makefile                     |   1 +<br>
>  drivers/gpu/drm/panel/panel-exynos-dp.c            | 213 +++++++++++++++++++++<br>
>  4 files changed, 255 insertions(+)<br>
>  create mode 100644 Documentation/devicetree/bindings/panel/exynos-dp-panel.txt<br>
>  create mode 100644 drivers/gpu/drm/panel/panel-exynos-dp.c<br>
><br>
> diff --git a/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt<br>
> b/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt<br>
> new file mode 100644<br>
> index 0000000..a1428d2<br>
> --- /dev/null<br>
> +++ b/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt<br>
> @@ -0,0 +1,32 @@<br>
> +exynos_DP_panel/DP_to_LVDS_panel<br>
<br>
</div></div>Please remove unnecessary under lines as below.<br>
<br></blockquote><div>Ok. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Exynos DP panel/DP to LVDS panel<br>
<div><div class="h5"><br>
> +==================================<br>
> +<br>
> +Required properties:<br>
> +  - compatible: "samsung,exynos-dp-panel"<br>
> +<br>
> +Optional properties:<br>
> +     -samsung,lcd-en-gpio:<br>
> +             eDP panel LCD poweron GPIO.<br>
> +                     Indicates which GPIO needs to be powered up as output<br>
> +                     to powerup/enable the switch to the LCD panel.<br>
> +     -samsung,led-en-gpio:<br>
> +             eDP panel LED enable GPIO.<br>
> +                     Indicates which GPIO needs to be powered up as output<br>
> +                     to enable the backlight.<br>
> +     -samsung,power-up-delay:<br>
> +             eDP panel powerup delay value in ms.<br>
> +                     Delay in ms needed for the eDP panel to properly<br>
> +                     powerup after giving powerup signals to the panel.<br>
> +     -samsung,power-down-delay:<br>
> +             eDP panel powerdown delay value in ms.<br>
> +                     Delay in ms needed for the eDP panel to properly<br>
> +                     powerdown after giving powerdown signals to the panel.<br>
> +<br>
> +Example:<br>
> +<br>
> +     dp-panel {<br>
> +             compatible = "samsung,exynos-dp-panel";<br>
> +             samsung,led-en-gpio = <&gpx3 0 1>;<br>
> +             samsung,power-up-delay = <40>;<br>
> +             samsung,power-down-delay = <50>;<br>
> +     };<br>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig<br>
> index 4ec874d..ea9d5ac 100644<br>
> --- a/drivers/gpu/drm/panel/Kconfig<br>
> +++ b/drivers/gpu/drm/panel/Kconfig<br>
> @@ -30,4 +30,13 @@ config DRM_PANEL_S6E8AA0<br>
>       select DRM_MIPI_DSI<br>
>       select VIDEOMODE_HELPERS<br>
><br>
> +config DRM_PANEL_EXYNOS_DP<br>
> +     tristate "support for DP panels"<br>
> +     depends on OF && DRM_PANEL && DRM_EXYNOS_DP<br>
> +     help<br>
> +       DRM panel driver for DP panels and LVDS connected via DP bridges<br>
> +       that need at most a regulator for LCD unit, a regulator for LED unit<br>
> +       and/or enable GPIOs for LCD or LED units. Delay values can also be<br>
> +       specified to support powerup and powerdown process.<br>
> +<br>
>  endmenu<br>
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile<br>
> index 8b92921..30311a4 100644<br>
> --- a/drivers/gpu/drm/panel/Makefile<br>
> +++ b/drivers/gpu/drm/panel/Makefile<br>
> @@ -1,3 +1,4 @@<br>
>  obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o<br>
>  obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o<br>
>  obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o<br>
> +obj-$(CONFIG_DRM_PANEL_EXYNOS_DP) += panel-exynos-dp.o<br>
> diff --git a/drivers/gpu/drm/panel/panel-exynos-dp.c b/drivers/gpu/drm/panel/panel-exynos-dp.c<br>
> new file mode 100644<br>
> index 0000000..e85a7b2<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/panel/panel-exynos-dp.c<br>
> @@ -0,0 +1,213 @@<br>
> +/*<br>
> + * Exynos DP panel driver<br>
> + *<br>
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd<br>
> + *<br>
> + * Ajay Kumar <<a href="mailto:ajaykumar.rs@samsung.com">ajaykumar.rs@samsung.com</a>><br>
> + *<br>
> + * This program is free software; you can redistribute it and/or modify<br>
> + * it under the terms of the GNU General Public License version 2 as<br>
> + * published by the Free Software Foundation.<br>
> + */<br>
> +<br>
> +#include <linux/component.h><br>
> +#include <linux/gpio.h><br>
> +#include <linux/module.h><br>
> +#include <linux/of_gpio.h><br>
> +#include <linux/of_platform.h><br>
> +#include <linux/platform_device.h><br>
> +#include <linux/regulator/consumer.h><br>
> +<br>
> +#include <drm/drmP.h><br>
> +#include <drm/drm_crtc.h><br>
> +#include <drm/drm_panel.h><br>
> +<br>
> +struct panel_exynos_dp {<br>
> +     struct drm_panel        base;<br>
> +     struct regulator        *bck_fet;<br>
> +     struct regulator        *lcd_fet;<br>
<br>
</div></div>'bck' means 'backlight'? Then, just use 'backlight_fet'.<br></blockquote><div>backlight_fet is more meaningful. Will change it.<br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

Also, I cannot understand the meaning of 'fet'.<br>
What's the meaning of the 'fet'?<br>
<div><div class="h5"><br></div></div></blockquote><div>FET is an output from the regulator. On exynos5250-snow board,<br></div><div>FET1 controls power to the backlight unit.<br></div><div>See more here: <a href="https://lkml.org/lkml/2014/4/15/618">https://lkml.org/lkml/2014/4/15/618</a> <br>
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5">
> +     int                     led_en_gpio;<br>
> +     int                     lcd_en_gpio;<br>
> +     int                     power_up_delay;<br>
> +     int                     power_down_delay;<br>
> +     bool                    enabled;<br>
> +};<br>
> +<br>
> +static inline struct panel_exynos_dp *to_panel(struct drm_panel *panel)<br>
> +{<br>
> +     return container_of(panel, struct panel_exynos_dp, base);<br>
> +}<br>
> +<br>
> +static int panel_exynos_dp_disable(struct drm_panel *panel)<br>
> +{<br>
> +     struct panel_exynos_dp *dp_panel = to_panel(panel);<br>
> +     bool enable_delay = false;<br>
> +<br>
> +     if (!dp_panel->enabled)<br>
> +             return 0;<br>
> +<br>
> +     if (gpio_is_valid(dp_panel->led_en_gpio))<br>
> +             gpio_set_value(dp_panel->led_en_gpio, 0);<br>
> +<br>
> +     if (!IS_ERR_OR_NULL(dp_panel->bck_fet))<br>
> +             regulator_disable(dp_panel->bck_fet);<br>
> +<br>
> +     if (gpio_is_valid(dp_panel->lcd_en_gpio)) {<br>
> +             gpio_set_value(dp_panel->lcd_en_gpio, 0);<br>
> +             enable_delay = true;<br>
> +     }<br>
> +<br>
> +     if (!IS_ERR_OR_NULL(dp_panel->lcd_fet)) {<br>
> +             regulator_disable(dp_panel->lcd_fet);<br>
> +             enable_delay = true;<br>
> +     }<br>
> +<br>
> +     if (enable_delay)<br>
> +             msleep(dp_panel->power_down_delay);<br>
> +<br>
> +     dp_panel->enabled = false;<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int panel_exynos_dp_pre_enable(struct drm_panel *panel)<br>
<br>
</div></div>panel_exynos_dp_pre_enable() always returns '0'.<br>
These are two ways. Either one will be better.<br>
1. Make return values meaningful. In other words, add the case<br>
   returning error values.<br>
<br>
2. Change the return type to 'void'<br>
<br></blockquote><div>Ok. Will add a check for all failure cases.<br><br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5"><br>
> +{<br>
> +     struct panel_exynos_dp *dp_panel = to_panel(panel);<br>
> +     bool enable_delay = false;<br>
> +<br>
> +     if (dp_panel->enabled)<br>
> +             return 0;<br>
> +<br>
> +     if (!IS_ERR_OR_NULL(dp_panel->lcd_fet)) {<br>
> +             if (regulator_enable(dp_panel->lcd_fet))<br>
> +                     DRM_ERROR("Failed to enable LCD fet\n");<br>
> +             enable_delay = true;<br>
> +     }<br>
> +<br>
> +     if (gpio_is_valid(dp_panel->lcd_en_gpio)) {<br>
> +             gpio_set_value(dp_panel->lcd_en_gpio, 1);<br>
> +             enable_delay = true;<br>
> +     }<br>
> +<br>
> +     if (enable_delay)<br>
> +             msleep(dp_panel->power_up_delay);<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int panel_exynos_dp_enable(struct drm_panel *panel)<br>
> +{<br>
> +     struct panel_exynos_dp *dp_panel = to_panel(panel);<br>
> +<br>
> +     if (dp_panel->enabled)<br>
> +             return 0;<br>
> +<br>
> +     if (!IS_ERR_OR_NULL(dp_panel->bck_fet))<br>
> +             if (regulator_enable(dp_panel->bck_fet))<br>
> +                             DRM_ERROR("Failed to enable LED fet\n");<br>
> +<br>
> +     if (gpio_is_valid(dp_panel->led_en_gpio))<br>
> +             gpio_set_value(dp_panel->led_en_gpio, 1);<br>
> +<br>
> +     dp_panel->enabled = true;<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static const struct drm_panel_funcs panel_exynos_dp_funcs = {<br>
> +     .disable = panel_exynos_dp_disable,<br>
> +     .pre_enable = panel_exynos_dp_pre_enable,<br>
> +     .enable = panel_exynos_dp_enable,<br>
> +};<br>
> +<br>
> +static int panel_exynos_dp_probe(struct platform_device *pdev)<br>
> +{<br>
> +     struct panel_exynos_dp *dp_panel;<br>
> +     struct device *dev = &pdev->dev;<br>
> +     int ret;<br>
> +<br>
> +     dp_panel = devm_kzalloc(dev, sizeof(*dp_panel), GFP_KERNEL);<br>
> +     if (!dp_panel)<br>
> +             return -ENOMEM;<br>
> +<br>
> +     dp_panel->enabled = false;<br>
> +<br>
> +     dp_panel->lcd_en_gpio = of_get_named_gpio(dev->of_node,<br>
> +                                             "samsung,lcd-en-gpio", 0);<br>
> +     dp_panel->led_en_gpio = of_get_named_gpio(dev->of_node,<br>
> +                                             "samsung,led-en-gpio", 0);<br>
> +<br>
> +     of_property_read_u32(dev->of_node, "samsung,power-up-delay",<br>
> +                                             &dp_panel->power_up_delay);<br>
> +     of_property_read_u32(dev->of_node, "samsung,power-down-delay",<br>
> +                                             &dp_panel->power_down_delay);<br>
> +<br>
> +     dp_panel->lcd_fet = devm_regulator_get(dev, "lcd_vdd");<br>
> +     if (IS_ERR(dp_panel->lcd_fet))<br>
> +             return PTR_ERR(dp_panel->lcd_fet);<br>
> +<br>
> +     dp_panel->bck_fet = devm_regulator_get(dev, "vcd_led");<br>
> +     if (IS_ERR(dp_panel->bck_fet))<br>
> +             return PTR_ERR(dp_panel->bck_fet);<br>
> +<br>
> +     if (gpio_is_valid(dp_panel->lcd_en_gpio)) {<br>
> +             ret = devm_gpio_request_one(dev, dp_panel->lcd_en_gpio,<br>
> +                                     GPIOF_OUT_INIT_LOW, "lcd_en_gpio");<br>
> +             if (ret) {<br>
> +                     DRM_ERROR("failed to get lcd-en gpio [%d]\n", ret);<br>
> +                     return ret;<br>
> +             }<br>
> +     } else {<br>
> +             dp_panel->lcd_en_gpio = -ENODEV;<br>
> +     }<br>
> +<br>
> +     if (gpio_is_valid(dp_panel->led_en_gpio)) {<br>
> +             ret = devm_gpio_request_one(dev, dp_panel->led_en_gpio,<br>
> +                                     GPIOF_OUT_INIT_LOW, "led_en_gpio");<br>
> +             if (ret) {<br>
> +                     DRM_ERROR("failed to get led-en gpio [%d]\n", ret);<br>
> +                     return ret;<br>
> +             }<br>
> +     } else {<br>
> +             dp_panel->led_en_gpio = -ENODEV;<br>
> +     }<br>
> +<br>
> +     drm_panel_init(&dp_panel->base);<br>
> +     dp_panel->base.dev = dev;<br>
> +     dp_panel->base.funcs = &panel_exynos_dp_funcs;<br>
> +<br>
> +     ret = drm_panel_add(&dp_panel->base);<br>
> +     if (ret < 0)<br>
> +             return ret;<br>
> +<br>
> +     dev_set_drvdata(dev, dp_panel);<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int panel_exynos_dp_remove(struct platform_device *pdev)<br>
> +{<br>
> +     struct panel_exynos_dp *dp_panel = dev_get_drvdata(&pdev->dev);<br>
> +<br>
> +     drm_panel_detach(&dp_panel->base);<br>
> +     drm_panel_remove(&dp_panel->base);<br>
> +<br>
> +     panel_exynos_dp_disable(&dp_panel->base);<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static const struct of_device_id exynos_dp_panel_dt_match[] = {<br>
> +     { .compatible = "samsung,exynos-dp-panel" },<br>
> +     {},<br>
> +};<br>
> +<br>
> +struct platform_driver exynos_dp_panel_driver = {<br>
> +     .driver = {<br>
> +             .name = "exynos-dp-panel",<br>
> +             .owner = THIS_MODULE,<br>
> +             .of_match_table = exynos_dp_panel_dt_match,<br>
> +     },<br>
> +     .probe = panel_exynos_dp_probe,<br>
> +     .remove = panel_exynos_dp_remove,<br>
> +};<br>
> --<br>
> 1.8.1.2<br>
<br>
</div></div></blockquote></div><br></div><div class="gmail_extra">Thanks and regards,<br>Ajay Kumar<br></div></div></div>