Re: [PATCH 3/7] drm/panel: Add driver for exynos_dp based panels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jingoo,


On Fri, Apr 18, 2014 at 2:17 PM, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote:
On Wednesday, April 16, 2014 11:33 PM, Ajay Kumar wrote:
>
> This patch adds a simple driver to handle all the LCD and LED
> powerup/down routines needed to support eDP/eDP-LVDS panels
> supported on exynos boards.
>
> Most of the eDP/LVDS panels need this sequence for powerup:
>       -- LCD unit powerup/LCD_EN
>       -- video data on
>       -- LED unit powerup/BL_EN
>
> The LCD and LED units are usually powered up via regulators,
> and almost on all boards, we will have a BL_EN pin to enable/
> disable the backlight. Sometimes, we can have LCD_EN switches
> as well. The routines in this driver can be used to control
> panel power sequence on such boards.
>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx>
> ---
>  .../devicetree/bindings/panel/exynos-dp-panel.txt  |  32 ++++
>  drivers/gpu/drm/panel/Kconfig                      |   9 +
>  drivers/gpu/drm/panel/Makefile                     |   1 +
>  drivers/gpu/drm/panel/panel-exynos-dp.c            | 213 +++++++++++++++++++++
>  4 files changed, 255 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/panel/exynos-dp-panel.txt
>  create mode 100644 drivers/gpu/drm/panel/panel-exynos-dp.c
>
> diff --git a/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt
> b/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt
> new file mode 100644
> index 0000000..a1428d2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt
> @@ -0,0 +1,32 @@
> +exynos_DP_panel/DP_to_LVDS_panel

Please remove unnecessary under lines as below.

Ok.
Exynos DP panel/DP to LVDS panel

> +==================================
> +
> +Required properties:
> +  - compatible: "samsung,exynos-dp-panel"
> +
> +Optional properties:
> +     -samsung,lcd-en-gpio:
> +             eDP panel LCD poweron GPIO.
> +                     Indicates which GPIO needs to be powered up as output
> +                     to powerup/enable the switch to the LCD panel.
> +     -samsung,led-en-gpio:
> +             eDP panel LED enable GPIO.
> +                     Indicates which GPIO needs to be powered up as output
> +                     to enable the backlight.
> +     -samsung,power-up-delay:
> +             eDP panel powerup delay value in ms.
> +                     Delay in ms needed for the eDP panel to properly
> +                     powerup after giving powerup signals to the panel.
> +     -samsung,power-down-delay:
> +             eDP panel powerdown delay value in ms.
> +                     Delay in ms needed for the eDP panel to properly
> +                     powerdown after giving powerdown signals to the panel.
> +
> +Example:
> +
> +     dp-panel {
> +             compatible = "samsung,exynos-dp-panel";
> +             samsung,led-en-gpio = <&gpx3 0 1>;
> +             samsung,power-up-delay = <40>;
> +             samsung,power-down-delay = <50>;
> +     };
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 4ec874d..ea9d5ac 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -30,4 +30,13 @@ config DRM_PANEL_S6E8AA0
>       select DRM_MIPI_DSI
>       select VIDEOMODE_HELPERS
>
> +config DRM_PANEL_EXYNOS_DP
> +     tristate "support for DP panels"
> +     depends on OF && DRM_PANEL && DRM_EXYNOS_DP
> +     help
> +       DRM panel driver for DP panels and LVDS connected via DP bridges
> +       that need at most a regulator for LCD unit, a regulator for LED unit
> +       and/or enable GPIOs for LCD or LED units. Delay values can also be
> +       specified to support powerup and powerdown process.
> +
>  endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 8b92921..30311a4 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>  obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
>  obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
> +obj-$(CONFIG_DRM_PANEL_EXYNOS_DP) += panel-exynos-dp.o
> diff --git a/drivers/gpu/drm/panel/panel-exynos-dp.c b/drivers/gpu/drm/panel/panel-exynos-dp.c
> new file mode 100644
> index 0000000..e85a7b2
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-exynos-dp.c
> @@ -0,0 +1,213 @@
> +/*
> + * Exynos DP panel driver
> + *
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd
> + *
> + * Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/component.h>
> +#include <linux/gpio.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_panel.h>
> +
> +struct panel_exynos_dp {
> +     struct drm_panel        base;
> +     struct regulator        *bck_fet;
> +     struct regulator        *lcd_fet;

'bck' means 'backlight'? Then, just use 'backlight_fet'.
backlight_fet is more meaningful. Will change it.

Also, I cannot understand the meaning of 'fet'.
What's the meaning of the 'fet'?

FET is an output from the regulator. On exynos5250-snow board,
FET1 controls power to the backlight unit.
> +     int                     led_en_gpio;
> +     int                     lcd_en_gpio;
> +     int                     power_up_delay;
> +     int                     power_down_delay;
> +     bool                    enabled;
> +};
> +
> +static inline struct panel_exynos_dp *to_panel(struct drm_panel *panel)
> +{
> +     return container_of(panel, struct panel_exynos_dp, base);
> +}
> +
> +static int panel_exynos_dp_disable(struct drm_panel *panel)
> +{
> +     struct panel_exynos_dp *dp_panel = to_panel(panel);
> +     bool enable_delay = false;
> +
> +     if (!dp_panel->enabled)
> +             return 0;
> +
> +     if (gpio_is_valid(dp_panel->led_en_gpio))
> +             gpio_set_value(dp_panel->led_en_gpio, 0);
> +
> +     if (!IS_ERR_OR_NULL(dp_panel->bck_fet))
> +             regulator_disable(dp_panel->bck_fet);
> +
> +     if (gpio_is_valid(dp_panel->lcd_en_gpio)) {
> +             gpio_set_value(dp_panel->lcd_en_gpio, 0);
> +             enable_delay = true;
> +     }
> +
> +     if (!IS_ERR_OR_NULL(dp_panel->lcd_fet)) {
> +             regulator_disable(dp_panel->lcd_fet);
> +             enable_delay = true;
> +     }
> +
> +     if (enable_delay)
> +             msleep(dp_panel->power_down_delay);
> +
> +     dp_panel->enabled = false;
> +
> +     return 0;
> +}
> +
> +static int panel_exynos_dp_pre_enable(struct drm_panel *panel)

panel_exynos_dp_pre_enable() always returns '0'.
These are two ways. Either one will be better.
1. Make return values meaningful. In other words, add the case
   returning error values.

2. Change the return type to 'void'

Ok. Will add a check for all failure cases.


> +{
> +     struct panel_exynos_dp *dp_panel = to_panel(panel);
> +     bool enable_delay = false;
> +
> +     if (dp_panel->enabled)
> +             return 0;
> +
> +     if (!IS_ERR_OR_NULL(dp_panel->lcd_fet)) {
> +             if (regulator_enable(dp_panel->lcd_fet))
> +                     DRM_ERROR("Failed to enable LCD fet\n");
> +             enable_delay = true;
> +     }
> +
> +     if (gpio_is_valid(dp_panel->lcd_en_gpio)) {
> +             gpio_set_value(dp_panel->lcd_en_gpio, 1);
> +             enable_delay = true;
> +     }
> +
> +     if (enable_delay)
> +             msleep(dp_panel->power_up_delay);
> +
> +     return 0;
> +}
> +
> +static int panel_exynos_dp_enable(struct drm_panel *panel)
> +{
> +     struct panel_exynos_dp *dp_panel = to_panel(panel);
> +
> +     if (dp_panel->enabled)
> +             return 0;
> +
> +     if (!IS_ERR_OR_NULL(dp_panel->bck_fet))
> +             if (regulator_enable(dp_panel->bck_fet))
> +                             DRM_ERROR("Failed to enable LED fet\n");
> +
> +     if (gpio_is_valid(dp_panel->led_en_gpio))
> +             gpio_set_value(dp_panel->led_en_gpio, 1);
> +
> +     dp_panel->enabled = true;
> +
> +     return 0;
> +}
> +
> +static const struct drm_panel_funcs panel_exynos_dp_funcs = {
> +     .disable = panel_exynos_dp_disable,
> +     .pre_enable = panel_exynos_dp_pre_enable,
> +     .enable = panel_exynos_dp_enable,
> +};
> +
> +static int panel_exynos_dp_probe(struct platform_device *pdev)
> +{
> +     struct panel_exynos_dp *dp_panel;
> +     struct device *dev = &pdev->dev;
> +     int ret;
> +
> +     dp_panel = devm_kzalloc(dev, sizeof(*dp_panel), GFP_KERNEL);
> +     if (!dp_panel)
> +             return -ENOMEM;
> +
> +     dp_panel->enabled = false;
> +
> +     dp_panel->lcd_en_gpio = of_get_named_gpio(dev->of_node,
> +                                             "samsung,lcd-en-gpio", 0);
> +     dp_panel->led_en_gpio = of_get_named_gpio(dev->of_node,
> +                                             "samsung,led-en-gpio", 0);
> +
> +     of_property_read_u32(dev->of_node, "samsung,power-up-delay",
> +                                             &dp_panel->power_up_delay);
> +     of_property_read_u32(dev->of_node, "samsung,power-down-delay",
> +                                             &dp_panel->power_down_delay);
> +
> +     dp_panel->lcd_fet = devm_regulator_get(dev, "lcd_vdd");
> +     if (IS_ERR(dp_panel->lcd_fet))
> +             return PTR_ERR(dp_panel->lcd_fet);
> +
> +     dp_panel->bck_fet = devm_regulator_get(dev, "vcd_led");
> +     if (IS_ERR(dp_panel->bck_fet))
> +             return PTR_ERR(dp_panel->bck_fet);
> +
> +     if (gpio_is_valid(dp_panel->lcd_en_gpio)) {
> +             ret = devm_gpio_request_one(dev, dp_panel->lcd_en_gpio,
> +                                     GPIOF_OUT_INIT_LOW, "lcd_en_gpio");
> +             if (ret) {
> +                     DRM_ERROR("failed to get lcd-en gpio [%d]\n", ret);
> +                     return ret;
> +             }
> +     } else {
> +             dp_panel->lcd_en_gpio = -ENODEV;
> +     }
> +
> +     if (gpio_is_valid(dp_panel->led_en_gpio)) {
> +             ret = devm_gpio_request_one(dev, dp_panel->led_en_gpio,
> +                                     GPIOF_OUT_INIT_LOW, "led_en_gpio");
> +             if (ret) {
> +                     DRM_ERROR("failed to get led-en gpio [%d]\n", ret);
> +                     return ret;
> +             }
> +     } else {
> +             dp_panel->led_en_gpio = -ENODEV;
> +     }
> +
> +     drm_panel_init(&dp_panel->base);
> +     dp_panel->base.dev = dev;
> +     dp_panel->base.funcs = &panel_exynos_dp_funcs;
> +
> +     ret = drm_panel_add(&dp_panel->base);
> +     if (ret < 0)
> +             return ret;
> +
> +     dev_set_drvdata(dev, dp_panel);
> +
> +     return 0;
> +}
> +
> +static int panel_exynos_dp_remove(struct platform_device *pdev)
> +{
> +     struct panel_exynos_dp *dp_panel = dev_get_drvdata(&pdev->dev);
> +
> +     drm_panel_detach(&dp_panel->base);
> +     drm_panel_remove(&dp_panel->base);
> +
> +     panel_exynos_dp_disable(&dp_panel->base);
> +
> +     return 0;
> +}
> +
> +static const struct of_device_id exynos_dp_panel_dt_match[] = {
> +     { .compatible = "samsung,exynos-dp-panel" },
> +     {},
> +};
> +
> +struct platform_driver exynos_dp_panel_driver = {
> +     .driver = {
> +             .name = "exynos-dp-panel",
> +             .owner = THIS_MODULE,
> +             .of_match_table = exynos_dp_panel_dt_match,
> +     },
> +     .probe = panel_exynos_dp_probe,
> +     .remove = panel_exynos_dp_remove,
> +};
> --
> 1.8.1.2


Thanks and regards,
Ajay Kumar
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux