[PATCH 1/2] dt-bindings: pwm-backlight: Add pwm-delay-us property

From: huang lin <redacted>

Add a pwm-delay-us property to specify the delay between setting an
initial (non-zero) PWM value and enabling the backlight, and also the
delay between disabling the backlight and setting PWM value to 0.

Signed-off-by: huang lin <redacted>
Signed-off-by: Enric Balletbo i Serra <redacted>
---
 Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
index 764db86..f75b08f 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt

@@ -17,6 +17,9 @@ Optional properties:
                "pwms" property (see PWM binding[0])
   - enable-gpios: contains a single GPIO specifier for the GPIO which enables
                   and disables the backlight (see GPIO binding[1])
+  - pwm-delay-us: delay between setting an initial (non-zero) PWM value and
+                  enabling the backlight, and also the delay between disabling
+                  the backlight and setting PWM value to 0.
 
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt
 [1]: Documentation/devicetree/bindings/gpio/gpio.txt

@@ -32,4 +35,5 @@ Example:
 
 		power-supply = <&vdd_bl_reg>;
 		enable-gpios = <&gpio 58 0>;
+		pwm-delay-us = <10000>;
 	};

-- 
2.9.3

From: huang lin <redacted>

Some panel backlight (i.e. N116BGE-L41), in their power sequence
specifications, request a delay between PWM signal and the backlight
enable signal, so use the pwm-delay-us property to meet the timing.

Signed-off-by: huang lin <redacted>
Signed-off-by: Enric Balletbo i Serra <redacted>
---
 drivers/video/backlight/pwm_bl.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 002f1ce..c5c4ef0 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c

@@ -10,6 +10,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio.h>
 #include <linux/module.h>

@@ -30,6 +31,7 @@ struct pwm_bl_data {
 	unsigned int		period;
 	unsigned int		lth_brightness;
 	unsigned int		*levels;
+	unsigned int		pwm_delay;
 	bool			enabled;
 	struct regulator	*power_supply;
 	struct gpio_desc	*enable_gpio;

@@ -54,10 +56,14 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness)
 	if (err < 0)
 		dev_err(pb->dev, "failed to enable power supply\n");
 
+	pwm_enable(pb->pwm);
+
+	if (pb->pwm_delay)
+		usleep_range(pb->pwm_delay, pb->pwm_delay + 2000);
+
 	if (pb->enable_gpio)
 		gpiod_set_value_cansleep(pb->enable_gpio, 1);
 
-	pwm_enable(pb->pwm);
 	pb->enabled = true;
 }
 

@@ -66,12 +72,15 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb)
 	if (!pb->enabled)
 		return;
 
-	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
-
 	if (pb->enable_gpio)
 		gpiod_set_value_cansleep(pb->enable_gpio, 0);
 
+	if (pb->pwm_delay)
+		usleep_range(pb->pwm_delay, pb->pwm_delay + 2000);
+
+	pwm_config(pb->pwm, 0, pb->period);
+	pwm_disable(pb->pwm);
+
 	regulator_disable(pb->power_supply);
 	pb->enabled = false;
 }

@@ -273,6 +282,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 	pb->dev = &pdev->dev;
 	pb->enabled = false;
 
+	of_property_read_u32(pdev->dev.of_node, "pwm-delay-us", &pb->pwm_delay);
 	pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
 						  GPIOD_ASIS);
 	if (IS_ERR(pb->enable_gpio)) {

-- 
2.9.3

On 28/06/17 12:16, Enric Balletbo i Serra wrote:

If is safe to assume power-on delay and power-off delay will be the same?

I've only took a quick look but several backlight controllers support 
asymetric power-on/off sequences...


Daniel.

On Wed 2017-06-28 13:16:31, Enric Balletbo i Serra wrote:

From: huang lin <redacted>

Add a pwm-delay-us property to specify the delay between setting an
initial (non-zero) PWM value and enabling the backlight, and also the
delay between disabling the backlight and setting PWM value to 0.

I understood it as "you set PWM and it takes a while for display to
light up"
but that's not correct. Changelog from second patch makes it
clear. Please
clarify it here, too.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Hi Daniel, Pavel,

2017-06-28 15:30 GMT+02:00 Pavel Machek [off-list ref]:

On Wed 2017-06-28 13:16:31, Enric Balletbo i Serra wrote:

quoted

From: huang lin <redacted>

Add a pwm-delay-us property to specify the delay between setting an
initial (non-zero) PWM value and enabling the backlight, and also the
delay between disabling the backlight and setting PWM value to 0.

quoted

Signed-off-by: huang lin <redacted>
Signed-off-by: Enric Balletbo i Serra <redacted>
---
 Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
index 764db86..f75b08f 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt

@@ -17,6 +17,9 @@ Optional properties:
                "pwms" property (see PWM binding[0])
   - enable-gpios: contains a single GPIO specifier for the GPIO which enables
                   and disables the backlight (see GPIO binding[1])
+  - pwm-delay-us: delay between setting an initial (non-zero) PWM value and
+                  enabling the backlight, and also the delay between disabling
+                  the backlight and setting PWM value to 0.

quoted

If is safe to assume power-on delay and power-off delay will be the same?

I've only took a quick look but several backlight controllers support asymetric power-on/off sequences..

Daniel, right the ones I checked are symmetric but asymmetric timings
are possible I guess, so I think now that specify the two delays is
more accurate, maybe the binding should be something like this?

  pwm-delay-us = <delay_before_on  delay_after_off>;

I understood it as "you set PWM and it takes a while for display to
light up"
but that's not correct. Changelog from second patch makes it
clear. Please
clarify it here, too.

Pavel, oh, your dude is interesting ...

That's the idea, the sequence is:
  Power on, you set the PWM signal, wait a bit and set the LED_EN signal.
  Power off, you clear the LED_EN signal, wait a bit and stop the PWM signal.

Note that the patch inverts the sequence, before this patch first you
set LED_EN signal and then the PWM signal

I assumed that the sequence was wrong but maybe I'm mistaken and there
are some panels that follow the original sequence. On the few panels I
checked the power on/off sequence is how I described above, i.e. see
[1] p. 17, the sequence is first PWM and then LED_EN. I'll take a look
at other panel datasheets, or if you know one, could you provide me
the datasheet?

Thanks,

[1] http://www.jxlcd.com/Upload/PicFiles/N116BGE-L41.pdf

                                                                Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

On 28/06/17 15:15, Enric Balletbo Serra wrote:

Hi Daniel, Pavel,

2017-06-28 15:30 GMT+02:00 Pavel Machek [off-list ref]:

quoted

On Wed 2017-06-28 13:16:31, Enric Balletbo i Serra wrote:

quoted

From: huang lin <redacted>

Add a pwm-delay-us property to specify the delay between setting an
initial (non-zero) PWM value and enabling the backlight, and also the
delay between disabling the backlight and setting PWM value to 0.

quoted

Signed-off-by: huang lin <redacted>
Signed-off-by: Enric Balletbo i Serra <redacted>
---
  Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
index 764db86..f75b08f 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt

@@ -17,6 +17,9 @@ Optional properties:
                 "pwms" property (see PWM binding[0])
    - enable-gpios: contains a single GPIO specifier for the GPIO which enables
                    and disables the backlight (see GPIO binding[1])
+  - pwm-delay-us: delay between setting an initial (non-zero) PWM value and
+                  enabling the backlight, and also the delay between disabling
+                  the backlight and setting PWM value to 0.

quoted

If is safe to assume power-on delay and power-off delay will be the same?

I've only took a quick look but several backlight controllers support asymetric power-on/off sequences..

Daniel, right the ones I checked are symmetric but asymmetric timings
are possible I guess, so I think now that specify the two delays is
more accurate, maybe the binding should be something like this?

   pwm-delay-us = <delay_before_on  delay_after_off>;

I think so. Like you I can't actually point at any asymmetric power 
sequence diagrams but there are a controller devices with power 
sequencing registers that support asymmetry. I'm inclined to take that 
as a hint...

quoted

I understood it as "you set PWM and it takes a while for display to
light up"
but that's not correct. Changelog from second patch makes it
clear. Please
clarify it here, too.

Pavel, oh, your dude is interesting ...

That's the idea, the sequence is:
   Power on, you set the PWM signal, wait a bit and set the LED_EN signal.
   Power off, you clear the LED_EN signal, wait a bit and stop the PWM signal.

Note that the patch inverts the sequence, before this patch first you
set LED_EN signal and then the PWM signal

I assumed that the sequence was wrong but maybe I'm mistaken and there
are some panels that follow the original sequence. On the few panels I
checked the power on/off sequence is how I described above, i.e. see
[1] p. 17, the sequence is first PWM and then LED_EN. I'll take a look
at other panel datasheets, or if you know one, could you provide me
the datasheet?

Thanks,

[1] http://www.jxlcd.com/Upload/PicFiles/N116BGE-L41.pdf

quoted

                                                                 Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html