Revision v1 of 2 in this series.
13 messages, 5 authors, 2017-11-14 · open the first message on its own page
From: Lothar Waßmann <hidden>
Date: 2017-10-26 12:50:03
These patches implement some measures to prevent backlight flicker when the backlight is being switched on for a display with an active low brightness control pin.
From: Lothar Waßmann <hidden>
Date: 2017-10-26 12:49:59
These patches implement some measures to prevent backlight flicker when the backlight is being switched on for a display with an active low brightness control pin. GIT: [PATCH 1/2] backlight: pwm_bl: Enable PWM before switching regulator GIT: [PATCH 2/2] backlight: pwm_bl: add configurable delay between
From: Lothar Waßmann <hidden>
Date: 2017-10-26 12:50:20
When the PWM is re-enabled after being switched off, the backlight may flicker due to the backlight power being switched on while the PWM is still unconfigured. This is especially true if the PWM output is active low (assuming that the PWM output is low when the PWM is not enabled). Configure the PWM before switching backlight power on to prevent this. Signed-off-by: Lothar Waßmann <redacted> --- drivers/video/backlight/pwm_bl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 002f1ce..921f322 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c@@ -50,6 +50,8 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness) if (pb->enabled) return; + pwm_enable(pb->pwm); + err = regulator_enable(pb->power_supply); if (err < 0) dev_err(pb->dev, "failed to enable power supply\n");
@@ -57,7 +59,6 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness) if (pb->enable_gpio) gpiod_set_value_cansleep(pb->enable_gpio, 1); - pwm_enable(pb->pwm); pb->enabled = true; }
--
2.1.4
From: Lothar Waßmann <hidden>
Date: 2017-10-26 12:50:35
When switching the backlight on, the LCD may need some time to adjust to the configured PWM duty cycle. Add a configurable delay between configuring the PWM and enabling the backlight regulator to account for this. Signed-off-by: Lothar Waßmann <redacted> --- .../bindings/leds/backlight/pwm-backlight.txt | 4 ++++ drivers/video/backlight/pwm_bl.c | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
index 764db86..95594c3 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt@@ -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]) + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching PWM on. This may be required to eliminate + flicker when switching the PWM on after it has been + disabled. [0]: Documentation/devicetree/bindings/pwm/pwm.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 921f322..9578f65 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c@@ -23,6 +23,7 @@ #include <linux/pwm_backlight.h> #include <linux/regulator/consumer.h> #include <linux/slab.h> +#include <linux/delay.h> struct pwm_bl_data { struct pwm_device *pwm;
@@ -35,6 +36,7 @@ struct pwm_bl_data { struct gpio_desc *enable_gpio; unsigned int scale; bool legacy; + int turn_on_delay_ms; int (*notify)(struct device *, int brightness); void (*notify_after)(struct device *,
@@ -52,6 +54,17 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness) pwm_enable(pb->pwm); + if (pb->turn_on_delay_ms > 0) { + dev_dbg(pb->dev, "Sleeping %u..%uµs\n", + pb->turn_on_delay_ms * 1000, + pb->turn_on_delay_ms * 1100); + if (pb->turn_on_delay_ms > 20) + msleep(pb->turn_on_delay_ms); + else + usleep_range(pb->turn_on_delay_ms * 1000, + pb->turn_on_delay_ms * 1100); + } + err = regulator_enable(pb->power_supply); if (err < 0) dev_err(pb->dev, "failed to enable power supply\n");
@@ -108,8 +121,9 @@ static int pwm_backlight_update_status(struct backlight_device *bl) duty_cycle = compute_duty_cycle(pb, brightness); pwm_config(pb->pwm, duty_cycle, pb->period); pwm_backlight_power_on(pb, brightness); - } else + } else { pwm_backlight_power_off(pb); + } if (pb->notify_after) pb->notify_after(pb->dev, brightness);
@@ -264,9 +278,9 @@ static int pwm_backlight_probe(struct platform_device *pdev) pb->scale = data->levels[i]; pb->levels = data->levels; - } else + } else { pb->scale = data->max_brightness; - + } pb->notify = data->notify; pb->notify_after = data->notify_after; pb->check_fb = data->check_fb;
@@ -329,6 +343,8 @@ static int pwm_backlight_probe(struct platform_device *pdev) goto err_alloc; } + of_property_read_u32(node, "turn-on-delay-ms", &pb->turn_on_delay_ms); + dev_dbg(&pdev->dev, "got pwm for backlight\n"); /*
--
2.1.4
From: Rob Herring <robh@kernel.org>
Date: 2017-10-27 14:38:24
On Thu, Oct 26, 2017 at 02:49:46PM +0200, Lothar Waßmann wrote:
When switching the backlight on, the LCD may need some time to adjust to the configured PWM duty cycle. Add a configurable delay between configuring the PWM and enabling the backlight regulator to account for this. Signed-off-by: Lothar Waßmann <redacted> --- .../bindings/leds/backlight/pwm-backlight.txt | 4 ++++ drivers/video/backlight/pwm_bl.c | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt index 764db86..95594c3 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt@@ -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]) + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching PWM on. This may be required to eliminate + flicker when switching the PWM on after it has been + disabled.
The subject talks about turning on the backlight regulator, but this only talks about the PWM.
[0]: Documentation/devicetree/bindings/pwm/pwm.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt
From: Lee Jones <hidden>
Date: 2017-10-30 10:33:48
On Fri, 27 Oct 2017, Rob Herring wrote:
On Thu, Oct 26, 2017 at 02:49:46PM +0200, Lothar Waßmann wrote:quoted
When switching the backlight on, the LCD may need some time to adjust to the configured PWM duty cycle. Add a configurable delay between configuring the PWM and enabling the backlight regulator to account for this. Signed-off-by: Lothar Waßmann <redacted> --- .../bindings/leds/backlight/pwm-backlight.txt | 4 ++++ drivers/video/backlight/pwm_bl.c | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt index 764db86..95594c3 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt@@ -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]) + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching PWM on. This may be required to eliminate + flicker when switching the PWM on after it has been + disabled.The subject talks about turning on the backlight regulator, but this only talks about the PWM.
From my understanding "switching PWM on" = enabling the regulator. I don't see any disparity here. Or am I missing something?
quoted
[0]: Documentation/devicetree/bindings/pwm/pwm.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
From: Rob Herring <robh@kernel.org>
Date: 2017-11-08 00:46:43
On Mon, Oct 30, 2017 at 5:33 AM, Lee Jones [off-list ref] wrote:
On Fri, 27 Oct 2017, Rob Herring wrote:quoted
On Thu, Oct 26, 2017 at 02:49:46PM +0200, Lothar Waßmann wrote:quoted
When switching the backlight on, the LCD may need some time to adjust to the configured PWM duty cycle. Add a configurable delay between configuring the PWM and enabling the backlight regulator to account for this. Signed-off-by: Lothar Waßmann <redacted> --- .../bindings/leds/backlight/pwm-backlight.txt | 4 ++++ drivers/video/backlight/pwm_bl.c | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt index 764db86..95594c3 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt@@ -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]) + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching PWM on. This may be required to eliminate + flicker when switching the PWM on after it has been + disabled.The subject talks about turning on the backlight regulator, but this only talks about the PWM.From my understanding "switching PWM on" = enabling the regulator. I don't see any disparity here. Or am I missing something?
The delay is from (re-)enabling the PWM (turning it on) to switching backlight power on. That sounds like 2 different things to me. Rob
From: Lothar Waßmann <hidden>
Date: 2017-11-08 07:34:16
Hi, On Tue, 7 Nov 2017 18:46:18 -0600 Rob Herring wrote:
On Mon, Oct 30, 2017 at 5:33 AM, Lee Jones [off-list ref] wrote:quoted
On Fri, 27 Oct 2017, Rob Herring wrote:quoted
On Thu, Oct 26, 2017 at 02:49:46PM +0200, Lothar Waßmann wrote:quoted
When switching the backlight on, the LCD may need some time to adjust to the configured PWM duty cycle. Add a configurable delay between configuring the PWM and enabling the backlight regulator to account for this. Signed-off-by: Lothar Waßmann <redacted> --- .../bindings/leds/backlight/pwm-backlight.txt | 4 ++++ drivers/video/backlight/pwm_bl.c | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt index 764db86..95594c3 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt@@ -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]) + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching PWM on. This may be required to eliminate + flicker when switching the PWM on after it has been + disabled.The subject talks about turning on the backlight regulator, but this only talks about the PWM.From my understanding "switching PWM on" = enabling the regulator. I don't see any disparity here. Or am I missing something?The delay is from (re-)enabling the PWM (turning it on) to switching backlight power on. That sounds like 2 different things to me.
The text should better read: + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching the backlight regulator on. This may be + required to eliminate flicker when switching the backlight + on after it had been disabled. Lothar Waßmann
From: Lee Jones <hidden>
Date: 2017-11-08 09:12:22
On Wed, 08 Nov 2017, Lothar Waßmann wrote:
Hi, On Tue, 7 Nov 2017 18:46:18 -0600 Rob Herring wrote:quoted
On Mon, Oct 30, 2017 at 5:33 AM, Lee Jones [off-list ref] wrote:quoted
On Fri, 27 Oct 2017, Rob Herring wrote:quoted
On Thu, Oct 26, 2017 at 02:49:46PM +0200, Lothar Waßmann wrote:quoted
When switching the backlight on, the LCD may need some time to adjust to the configured PWM duty cycle. Add a configurable delay between configuring the PWM and enabling the backlight regulator to account for this. Signed-off-by: Lothar Waßmann <redacted> --- .../bindings/leds/backlight/pwm-backlight.txt | 4 ++++ drivers/video/backlight/pwm_bl.c | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt index 764db86..95594c3 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt@@ -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]) + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching PWM on. This may be required to eliminate + flicker when switching the PWM on after it has been + disabled.The subject talks about turning on the backlight regulator, but this only talks about the PWM.From my understanding "switching PWM on" = enabling the regulator. I don't see any disparity here. Or am I missing something?The delay is from (re-)enabling the PWM (turning it on) to switching backlight power on. That sounds like 2 different things to me.
I read it as the delay being between "configuring the PWM" (still not on) and "switching <the> PWM on" (enabling the regulator).
The text should better read: + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching the backlight regulator on. This may be + required to eliminate flicker when switching the backlight + on after it had been disabled.
How is the regulator described in the datasheet? Is it for the Backlight or the PWM? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
From: Lothar Waßmann <hidden>
Date: 2017-11-08 09:44:53
Hi, On Wed, 8 Nov 2017 09:12:12 +0000 Lee Jones wrote:
On Wed, 08 Nov 2017, Lothar Waßmann wrote:quoted
Hi, On Tue, 7 Nov 2017 18:46:18 -0600 Rob Herring wrote:quoted
On Mon, Oct 30, 2017 at 5:33 AM, Lee Jones [off-list ref] wrote:quoted
On Fri, 27 Oct 2017, Rob Herring wrote:quoted
On Thu, Oct 26, 2017 at 02:49:46PM +0200, Lothar Waßmann wrote:quoted
When switching the backlight on, the LCD may need some time to adjust to the configured PWM duty cycle. Add a configurable delay between configuring the PWM and enabling the backlight regulator to account for this. Signed-off-by: Lothar Waßmann <redacted> --- .../bindings/leds/backlight/pwm-backlight.txt | 4 ++++ drivers/video/backlight/pwm_bl.c | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt index 764db86..95594c3 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt@@ -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]) + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching PWM on. This may be required to eliminate + flicker when switching the PWM on after it has been + disabled.The subject talks about turning on the backlight regulator, but this only talks about the PWM.From my understanding "switching PWM on" = enabling the regulator. I don't see any disparity here. Or am I missing something?The delay is from (re-)enabling the PWM (turning it on) to switching backlight power on. That sounds like 2 different things to me.I read it as the delay being between "configuring the PWM" (still not on) and "switching <the> PWM on" (enabling the regulator).quoted
The text should better read: + - turn-on-delay-ms: delay in milliseconds between configuring the PWM + and switching the backlight regulator on. This may be + required to eliminate flicker when switching the backlight + on after it had been disabled.How is the regulator described in the datasheet? Is it for the Backlight or the PWM?
It actually is a Power Control input of the LCD panel, that is modeled as a fiexd-regulator in Linux. Lothar Waßmann
From: Daniel Thompson <hidden>
Date: 2017-11-08 10:48:16
On 26/10/17 13:49, Lothar Waßmann wrote:
These patches implement some measures to prevent backlight flicker when the backlight is being switched on for a display with an active low brightness control pin. GIT: [PATCH 1/2] backlight: pwm_bl: Enable PWM before switching regulator GIT: [PATCH 2/2] backlight: pwm_bl: add configurable delay between
Other than hoisting the pwm_enable() even earlier in the setup sequence this patchset seems to have a significant overlap with Enric's recent posting: https://lkml.org/lkml/2017/7/21/211 Any chance of a shared view on this, especially on the DT bindings? Daniel.
From: Enric Balletbo i Serra <hidden>
Date: 2017-11-10 11:22:24
Hi all, On 08/11/17 11:48, Daniel Thompson wrote:
On 26/10/17 13:49, Lothar Waßmann wrote:quoted
These patches implement some measures to prevent backlight flicker when the backlight is being switched on for a display with an active low brightness control pin. GIT: [PATCH 1/2] backlight: pwm_bl: Enable PWM before switching regulator GIT: [PATCH 2/2] backlight: pwm_bl: add configurable delay betweenOther than hoisting the pwm_enable() even earlier in the setup sequence this patchset seems to have a significant overlap with Enric's recent posting: https://lkml.org/lkml/2017/7/21/211 Any chance of a shared view on this, especially on the DT bindings?
The DT binding were discussed some time ago for the patch series I sent, though there isn't a final ack from DT maintainer. Lothar, the series I sent have been reviewed and acked, can you test if the series fixes the problem for you too? Thanks, Enric
Daniel.
From: Lothar Waßmann <hidden>
Date: 2017-11-14 07:56:26
Hi, On Fri, 10 Nov 2017 12:22:15 +0100 Enric Balletbo i Serra wrote:
Hi all, On 08/11/17 11:48, Daniel Thompson wrote:quoted
On 26/10/17 13:49, Lothar Waßmann wrote:quoted
These patches implement some measures to prevent backlight flicker when the backlight is being switched on for a display with an active low brightness control pin. GIT: [PATCH 1/2] backlight: pwm_bl: Enable PWM before switching regulator GIT: [PATCH 2/2] backlight: pwm_bl: add configurable delay betweenOther than hoisting the pwm_enable() even earlier in the setup sequence this patchset seems to have a significant overlap with Enric's recent posting: https://lkml.org/lkml/2017/7/21/211 Any chance of a shared view on this, especially on the DT bindings?The DT binding were discussed some time ago for the patch series I sent, though there isn't a final ack from DT maintainer. Lothar, the series I sent have been reviewed and acked, can you test if the series fixes the problem for you too?
I'll try to test it within the next couple of days and will report back. Lothar Waßmann