From: Enric Balletbo i Serra <hidden> Date: 2018-02-16 17:40:45
Before this patch the enable signal was set before the PWM signal and
vice-versa on power off. This sequence is wrong, at least, it is on
the different panels datasheets that I checked, so I inverted the sequence
to follow the specs.
For reference the following panels have the mentioned sequence:
- N133HSE-EA1 (Innolux)
- N116BGE (Innolux)
- N156BGE-L21 (Innolux)
- B101EAN0 (Auo)
- B101AW03 (Auo)
- LTN101NT05 (Samsung)
- CLAA101WA01A (Chunghwa)
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Daniel Thompson <redacted>
Acked-by: Jingoo Han <jingoohan1@gmail.com>
---
Changes since v4:
- Rebase on top of mainline.
- Add the acks from Daniel Thompson and Jingoo Han.
Changes since v3:
- List the part numbers for the panel checked (Daniel Thompson)
Changes since v2:
- Add this as a separate patch (Thierry Reding)
Changes since v1:
- None
drivers/video/backlight/pwm_bl.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
@@ -54,10 +54,11 @@ 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->enable_gpio)gpiod_set_value_cansleep(pb->enable_gpio,1);-pwm_enable(pb->pwm);pb->enabled=true;}
From: Enric Balletbo i Serra <hidden> Date: 2018-02-16 17:40:48
Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
---
Based on the original Huang Lin [off-list ref] work.
Changes since v4:
- Rebase on top of mainline.
Changes since v3:
- Replace us for ms.
- Add Acked-by: Pavel Machek [off-list ref]
Changes since v2:
- Use separate properties (Rob Herring)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 6 ++++++
1 file changed, 6 insertions(+)
@@ -17,6 +17,10 @@ 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])+ - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM+ and enabling the backlight using GPIO.+ - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO+ and setting PWM value to 0. [0]: Documentation/devicetree/bindings/pwm/pwm.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt
From: Enric Balletbo i Serra <hidden> Date: 2018-02-16 17:40:50
The minnie devices comes with an AUO B101EAN01 panel which is different
from default veyron devices, thus the power on/off timing sequence is
slightly different. The datasheet specifies a pwm delay of 200 ms, so
update the PMW delay proprieties accordingly.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
---
Changes since v4:
- Rebase on top of mainline.
Changes since v3:
- Use new -ms names for proprieties.
- Fix the delay, should be 200ms instead of 20ms (Pavel)
Changes since v2:
- Use new names for proprieties.
Changes since v1:
- Add this new patch as minnie has differents timings
arch/arm/boot/dts/rk3288-veyron-minnie.dts | 2 ++
1 file changed, 2 insertions(+)
From: Enric Balletbo i Serra <hidden> Date: 2018-02-16 17:41:07
For veyron the binding should provide both PWM timings, the delay between
you enable the PWM and set the enable signal, and the delay between you
disable the PWM signal and clear the enable signal. Update the binding
accordingly, in this case the panels connected to the veyron boards have
a symmetric power sequence, hence the same value is used.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
---
Changes since v4:
- Rebased on top of mainline.
Changes since v3:
- Use new -ms names for proprieties.
Changes since v2:
- Use new names for proprieties.
Changes since v1:
- Add this new patch to fix current binding on veyron.
arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Enric Balletbo i Serra <hidden> Date: 2018-02-16 17:41:24
Some panels (i.e. N116BGE-L41), in their power sequence specifications,
request a delay between set the PWM signal and enable the backlight and
between clear the PWM signal and disable the backlight. Add support for
the new post-pwm-on-delay-ms and pwm-off-delay-ms proprieties to meet
the timings.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
Acked-by: Daniel Thompson <redacted>
Acked-by: Jingoo Han <jingoohan1@gmail.com>
---
Changes since v4:
- Rebased on top of mainline.
- Added the acks from Pavel Machek, Daniel thompson and Jingoo Han
Changes since v3:
- Use two named members instead of pwm_delay[] (Daniel and Pavel)
- Use msleep instead of usleep_range. (Pavel)
Changes since v2:
- Move the pwm/enable sequence to another patch (Thierry Reding)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
- Move the check of dt property to the parse dt function.
drivers/video/backlight/pwm_bl.c | 19 +++++++++++++++++++
include/linux/pwm_backlight.h | 2 ++
2 files changed, 21 insertions(+)
@@ -14,6 +14,8 @@ struct platform_pwm_backlight_data {unsignedintlth_brightness;unsignedintpwm_period_ns;unsignedint*levels;+unsignedintpost_pwm_on_delay;+unsignedintpwm_off_delay;/* TODO remove once all users are switched to gpiod_* API */intenable_gpio;int(*init)(structdevice*dev);
From: Lee Jones <hidden> Date: 2018-02-19 09:22:52
On Fri, 16 Feb 2018, Enric Balletbo i Serra wrote:
Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
---
Based on the original Huang Lin [off-list ref] work.
Changes since v4:
- Rebase on top of mainline.
Changes since v3:
- Replace us for ms.
- Add Acked-by: Pavel Machek [off-list ref]
Changes since v2:
- Use separate properties (Rob Herring)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 6 ++++++
1 file changed, 6 insertions(+)
Looks like this still needs an Ack.
Also, you didn't supply a 0th (cover-letter) patch, so I have no idea
of patch history or your future intentions.
Are there build-time dependencies between them or can each patch be
merged via their respective trees without the fear of merge/build
failure?
--
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Hi Lee,
Am Montag, 19. Februar 2018, 10:22:47 CET schrieb Lee Jones:
On Fri, 16 Feb 2018, Enric Balletbo i Serra wrote:
quoted
Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
---
Based on the original Huang Lin [off-list ref] work.
Changes since v4:
- Rebase on top of mainline.
Changes since v3:
- Replace us for ms.
- Add Acked-by: Pavel Machek [off-list ref]
Changes since v2:
- Use separate properties (Rob Herring)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 6 ++++++
1 file changed, 6 insertions(+)
Looks like this still needs an Ack.
Also, you didn't supply a 0th (cover-letter) patch, so I have no idea
of patch history or your future intentions.
Are there build-time dependencies between them or can each patch be
merged via their respective trees without the fear of merge/build
failure?
When looking at the patches, there do not seem to be any build-time
dependencies between the patch 1-3 (backlight) and 4+5 devicetree,
so after Rob hopefully has time to look at the binding change, I'd suspect
you take the code changes and I'll take the dts changes.
Heiko
From: Lee Jones <hidden> Date: 2018-02-19 10:39:42
On Mon, 19 Feb 2018, Heiko Stuebner wrote:
Am Montag, 19. Februar 2018, 10:22:47 CET schrieb Lee Jones:
quoted
On Fri, 16 Feb 2018, Enric Balletbo i Serra wrote:
quoted
Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
---
Based on the original Huang Lin [off-list ref] work.
Changes since v4:
- Rebase on top of mainline.
Changes since v3:
- Replace us for ms.
- Add Acked-by: Pavel Machek [off-list ref]
Changes since v2:
- Use separate properties (Rob Herring)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 6 ++++++
1 file changed, 6 insertions(+)
Looks like this still needs an Ack.
Also, you didn't supply a 0th (cover-letter) patch, so I have no idea
of patch history or your future intentions.
Are there build-time dependencies between them or can each patch be
merged via their respective trees without the fear of merge/build
failure?
When looking at the patches, there do not seem to be any build-time
dependencies between the patch 1-3 (backlight) and 4+5 devicetree,
so after Rob hopefully has time to look at the binding change, I'd suspect
you take the code changes and I'll take the dts changes.
Works for me.
--
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Daniel Thompson <hidden> Date: 2018-02-19 11:11:43
On Fri, Feb 16, 2018 at 06:40:31PM +0100, Enric Balletbo i Serra wrote:
Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
Acked-by: Daniel Thompson <redacted>
quoted hunk
---
Based on the original Huang Lin [off-list ref] work.
Changes since v4:
- Rebase on top of mainline.
Changes since v3:
- Replace us for ms.
- Add Acked-by: Pavel Machek [off-list ref]
Changes since v2:
- Use separate properties (Rob Herring)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 6 ++++++
1 file changed, 6 insertions(+)
@@ -17,6 +17,10 @@ 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])+ - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM+ and enabling the backlight using GPIO.+ - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO+ and setting PWM value to 0. [0]: Documentation/devicetree/bindings/pwm/pwm.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt
Hi Rob,
oh, devicetree mailing list is not added, sorry, so cc'ing ...
2018-02-19 12:11 GMT+01:00 Daniel Thompson [off-list ref]:
On Fri, Feb 16, 2018 at 06:40:31PM +0100, Enric Balletbo i Serra wrote:
quoted
Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
Acked-by: Daniel Thompson <redacted>
Lee, I guess that after Daniel ack the only ack missing here is one
from Rob Herring?
Rob, if you have a chance to look at this I'd appreciate.
Thanks,
Enric
quoted
---
Based on the original Huang Lin [off-list ref] work.
Changes since v4:
- Rebase on top of mainline.
Changes since v3:
- Replace us for ms.
- Add Acked-by: Pavel Machek [off-list ref]
Changes since v2:
- Use separate properties (Rob Herring)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 6 ++++++
1 file changed, 6 insertions(+)
@@ -17,6 +17,10 @@ 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])+ - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM+ and enabling the backlight using GPIO.+ - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO+ and setting PWM value to 0. [0]: Documentation/devicetree/bindings/pwm/pwm.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt
From: Lee Jones <hidden> Date: 2018-03-12 09:10:47
FAO Thierry,
oh, devicetree mailing list is not added, sorry, so cc'ing ...
2018-02-19 12:11 GMT+01:00 Daniel Thompson [off-list ref]:
quoted
On Fri, Feb 16, 2018 at 06:40:31PM +0100, Enric Balletbo i Serra wrote:
quoted
Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
Acked-by: Daniel Thompson <redacted>
Lee, I guess that after Daniel ack the only ack missing here is one
from Rob Herring?
I'd also like an ACK from the PWM Maintainer if possible.
Rob, if you have a chance to look at this I'd appreciate.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Fri, Feb 16, 2018 at 06:40:31PM +0100, Enric Balletbo i Serra wrote:
Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
---
Based on the original Huang Lin [off-list ref] work.
Changes since v4:
- Rebase on top of mainline.
Changes since v3:
- Replace us for ms.
- Add Acked-by: Pavel Machek [off-list ref]
Changes since v2:
- Use separate properties (Rob Herring)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 6 ++++++
1 file changed, 6 insertions(+)
On Fri, Feb 16, 2018 at 06:40:32PM +0100, Enric Balletbo i Serra wrote:
Some panels (i.e. N116BGE-L41), in their power sequence specifications,
request a delay between set the PWM signal and enable the backlight and
between clear the PWM signal and disable the backlight. Add support for
the new post-pwm-on-delay-ms and pwm-off-delay-ms proprieties to meet
the timings.
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Pavel Machek <redacted>
Acked-by: Daniel Thompson <redacted>
Acked-by: Jingoo Han <jingoohan1@gmail.com>
---
Changes since v4:
- Rebased on top of mainline.
- Added the acks from Pavel Machek, Daniel thompson and Jingoo Han
Changes since v3:
- Use two named members instead of pwm_delay[] (Daniel and Pavel)
- Use msleep instead of usleep_range. (Pavel)
Changes since v2:
- Move the pwm/enable sequence to another patch (Thierry Reding)
Changes since v1:
- As suggested by Daniel Thompson
- Do not assume power-on delay and power-off delay will be the same
- Move the check of dt property to the parse dt function.
drivers/video/backlight/pwm_bl.c | 19 +++++++++++++++++++
include/linux/pwm_backlight.h | 2 ++
2 files changed, 21 insertions(+)
On Fri, Feb 16, 2018 at 06:40:30PM +0100, Enric Balletbo i Serra wrote:
Before this patch the enable signal was set before the PWM signal and
vice-versa on power off. This sequence is wrong, at least, it is on
the different panels datasheets that I checked, so I inverted the sequence
to follow the specs.
For reference the following panels have the mentioned sequence:
- N133HSE-EA1 (Innolux)
- N116BGE (Innolux)
- N156BGE-L21 (Innolux)
- B101EAN0 (Auo)
- B101AW03 (Auo)
- LTN101NT05 (Samsung)
- CLAA101WA01A (Chunghwa)
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Daniel Thompson <redacted>
Acked-by: Jingoo Han <jingoohan1@gmail.com>
---
Changes since v4:
- Rebase on top of mainline.
- Add the acks from Daniel Thompson and Jingoo Han.
Changes since v3:
- List the part numbers for the panel checked (Daniel Thompson)
Changes since v2:
- Add this as a separate patch (Thierry Reding)
Changes since v1:
- None
drivers/video/backlight/pwm_bl.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
I'm surprised that panels even care about this. Do you see actual
breakage if these are the other way around?
Anyway, this seems to me as legit as the other way around, so:
Acked-by: Thierry Reding <redacted>
From: Enric Balletbo i Serra <hidden> Date: 2018-03-19 19:00:21
Hi Thierry,
Thanks for your ack.
On 19/03/18 17:58, Thierry Reding wrote:
On Fri, Feb 16, 2018 at 06:40:30PM +0100, Enric Balletbo i Serra wrote:
quoted
Before this patch the enable signal was set before the PWM signal and
vice-versa on power off. This sequence is wrong, at least, it is on
the different panels datasheets that I checked, so I inverted the sequence
to follow the specs.
For reference the following panels have the mentioned sequence:
- N133HSE-EA1 (Innolux)
- N116BGE (Innolux)
- N156BGE-L21 (Innolux)
- B101EAN0 (Auo)
- B101AW03 (Auo)
- LTN101NT05 (Samsung)
- CLAA101WA01A (Chunghwa)
Signed-off-by: Enric Balletbo i Serra <redacted>
Acked-by: Daniel Thompson <redacted>
Acked-by: Jingoo Han <jingoohan1@gmail.com>
---
Changes since v4:
- Rebase on top of mainline.
- Add the acks from Daniel Thompson and Jingoo Han.
Changes since v3:
- List the part numbers for the panel checked (Daniel Thompson)
Changes since v2:
- Add this as a separate patch (Thierry Reding)
Changes since v1:
- None
drivers/video/backlight/pwm_bl.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
I'm surprised that panels even care about this. Do you see actual
breakage if these are the other way around?
At least the B116XTN02 requires enable first the PWM, wait 10ms and then BL_EN
to avoid garbage. I did not observe the issue on other panels but the datasheets
I checked specifies this sequence as correct.
Anyway, this seems to me as legit as the other way around, so:
Acked-by: Thierry Reding <redacted>