diff mbox series

[v2,1/2] backlight: pwm_bl: Configure pwm only once per backlight toggle

Message ID 20230120120018.161103-2-u.kleine-koenig@pengutronix.de (mailing list archive)
State Handled Elsewhere
Headers show
Series backlight: pwm_bl: Two PWM releated changes | expand

Commit Message

Uwe Kleine-König Jan. 20, 2023, noon UTC 
When the function pwm_backlight_update_status() was called with
brightness > 0, pwm_get_state() was called twice (once directly and once
in compute_duty_cycle). Also pwm_apply_state() was called twice (once in
pwm_backlight_power_on() and once directly).

Optimize this to do both calls only once.

Note that with this affects the order of regulator and PWM setup. It's
not expected to have a relevant effect on hardware. The rationale for
this is that the regulator (and the GPIO) are reasonable to switch in
pwm_backlight_power_on()/pwm_backlight_power_off() but the PWM has
nothing to do with power. (The post_pwm_on_delay and pwm_off_delay are
still there though.)

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/backlight/pwm_bl.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

Comments

Lee Jones Jan. 20, 2023, 5:07 p.m. UTC | #1 
On Fri, 20 Jan 2023, Uwe Kleine-König wrote:

> When the function pwm_backlight_update_status() was called with
> brightness > 0, pwm_get_state() was called twice (once directly and once
> in compute_duty_cycle). Also pwm_apply_state() was called twice (once in
> pwm_backlight_power_on() and once directly).
> 
> Optimize this to do both calls only once.
> 
> Note that with this affects the order of regulator and PWM setup. It's
> not expected to have a relevant effect on hardware. The rationale for
> this is that the regulator (and the GPIO) are reasonable to switch in
> pwm_backlight_power_on()/pwm_backlight_power_off() but the PWM has
> nothing to do with power. (The post_pwm_on_delay and pwm_off_delay are
> still there though.)
> 
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/video/backlight/pwm_bl.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)

Applied, thanks
diff mbox series

Patch

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index d0b22158cd70..0509fecd5715 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -40,10 +40,8 @@  struct pwm_bl_data {
 
 static void pwm_backlight_power_on(struct pwm_bl_data *pb)
 {
-	struct pwm_state state;
 	int err;
 
-	pwm_get_state(pb->pwm, &state);
 	if (pb->enabled)
 		return;
 
@@ -51,9 +49,6 @@  static void pwm_backlight_power_on(struct pwm_bl_data *pb)
 	if (err < 0)
 		dev_err(pb->dev, "failed to enable power supply\n");
 
-	state.enabled = true;
-	pwm_apply_state(pb->pwm, &state);
-
 	if (pb->post_pwm_on_delay)
 		msleep(pb->post_pwm_on_delay);
 
@@ -65,9 +60,6 @@  static void pwm_backlight_power_on(struct pwm_bl_data *pb)
 
 static void pwm_backlight_power_off(struct pwm_bl_data *pb)
 {
-	struct pwm_state state;
-
-	pwm_get_state(pb->pwm, &state);
 	if (!pb->enabled)
 		return;
 
@@ -77,28 +69,21 @@  static void pwm_backlight_power_off(struct pwm_bl_data *pb)
 	if (pb->pwm_off_delay)
 		msleep(pb->pwm_off_delay);
 
-	state.enabled = false;
-	state.duty_cycle = 0;
-	pwm_apply_state(pb->pwm, &state);
-
 	regulator_disable(pb->power_supply);
 	pb->enabled = false;
 }
 
-static int compute_duty_cycle(struct pwm_bl_data *pb, int brightness)
+static int compute_duty_cycle(struct pwm_bl_data *pb, int brightness, struct pwm_state *state)
 {
 	unsigned int lth = pb->lth_brightness;
-	struct pwm_state state;
 	u64 duty_cycle;
 
-	pwm_get_state(pb->pwm, &state);
-
 	if (pb->levels)
 		duty_cycle = pb->levels[brightness];
 	else
 		duty_cycle = brightness;
 
-	duty_cycle *= state.period - lth;
+	duty_cycle *= state->period - lth;
 	do_div(duty_cycle, pb->scale);
 
 	return duty_cycle + lth;
@@ -115,11 +100,18 @@  static int pwm_backlight_update_status(struct backlight_device *bl)
 
 	if (brightness > 0) {
 		pwm_get_state(pb->pwm, &state);
-		state.duty_cycle = compute_duty_cycle(pb, brightness);
+		state.duty_cycle = compute_duty_cycle(pb, brightness, &state);
+		state.enabled = true;
 		pwm_apply_state(pb->pwm, &state);
+
 		pwm_backlight_power_on(pb);
 	} else {
 		pwm_backlight_power_off(pb);
+
+		pwm_get_state(pb->pwm, &state);
+		state.enabled = false;
+		state.duty_cycle = 0;
+		pwm_apply_state(pb->pwm, &state);
 	}
 
 	if (pb->notify_after)