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

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

[Enric Balletbo i Serra]

From: huang lin <hl@rock-chips.com>

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 <hl@rock-chips.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 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

[PATCH 2/2] pwm-backlight: Add support for pwm-delay-us property

[Enric Balletbo i Serra]

From: huang lin <hl@rock-chips.com>

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 <hl@rock-chips.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 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

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

[Daniel Thompson]

On 28/06/17 12:16, Enric Balletbo i Serra wrote:
> From: huang lin <hl@rock-chips.com>
> 
> 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 <hl@rock-chips.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>   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.

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.


>   
>   [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>;
>   	};
> 

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

[Pavel Machek]

On Wed 2017-06-28 13:16:31, Enric Balletbo i Serra wrote:
> From: huang lin <hl@rock-chips.com>
> 
> 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 <hl@rock-chips.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>  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.
>

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

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

[Enric Balletbo Serra]

Hi Daniel, Pavel,

2017-06-28 15:30 GMT+02:00 Pavel Machek <pavel@ucw.cz>:
> On Wed 2017-06-28 13:16:31, Enric Balletbo i Serra wrote:
>> From: huang lin <hl@rock-chips.com>
>>
>> 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 <hl@rock-chips.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>  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.
>>
>
>> 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

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

[Daniel Thompson]

On 28/06/17 15:15, Enric Balletbo Serra wrote:
> Hi Daniel, Pavel,
> 
> 2017-06-28 15:30 GMT+02:00 Pavel Machek <pavel@ucw.cz>:
>> On Wed 2017-06-28 13:16:31, Enric Balletbo i Serra wrote:
>>> From: huang lin <hl@rock-chips.com>
>>>
>>> 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 <hl@rock-chips.com>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> ---
>>>   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.
>>>
>>
>>> 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...


>> 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