| Message ID | 20230109204758.610400-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [1/2] backlight: pwm_bl: configure pwm only once per backlight toggle | expand |
On Mon, Jan 09, 2023 at 09:47:57PM +0100, 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. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> This will reverse the order in which the regulator is toggled versus the PWM starting/stopping. It would be nice to that in the description. However I can't see why it would be a problem (since both remain in the same place relative to the sleeps) so: Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> Daniel.
Hello Daniel, On Tue, Jan 10, 2023 at 04:17:16PM +0000, Daniel Thompson wrote: > On Mon, Jan 09, 2023 at 09:47:57PM +0100, 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. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > This will reverse the order in which the regulator is toggled versus the > PWM starting/stopping. It would be nice to that in the description. Oh, that wasn't a concious choice. I agree this should be noted. The current state is also a bit confused because the duty cycle is setup before the regulator but the PWM only gets enabled afterwards. Expect a v2 with an updated commit log. Best regards Uwe
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)
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. 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(-) base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2 prerequisite-patch-id: a0c7497a32092d284bc47eda60e4b3690338ba6e