| Message ID | 20230608-backlight-pwm-avoid-flicker-v1-1-afd380d50174@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | backlight: pwm_bl: Avoid backlight flicker applying initial PWM state | expand |
On Thu, Jun 08, 2023 at 04:11:14PM +0200, Philipp Zabel wrote: > The initial PWM state returned by pwm_init_state() has a duty cycle > of 0 ns. To avoid backlight flicker when taking over an enabled > display from the bootloader, skip the initial pwm_apply_state() > and leave the PWM be until backlight_update_state() will apply the > state with the desired brightness. backlight_update_state() uses pwm_get_state() to update the PWM. Without applying something that came from pwm_init_state() then we will never adopt the reference values from pwm->args. Daniel.
Hello Philipp, On Thu, Jun 08, 2023 at 04:11:14PM +0200, Philipp Zabel wrote: > The initial PWM state returned by pwm_init_state() has a duty cycle > of 0 ns. This is only true for drivers without a .get_state() callback, isn't it? > To avoid backlight flicker when taking over an enabled > display from the bootloader, skip the initial pwm_apply_state() > and leave the PWM be until backlight_update_state() will apply the > state with the desired brightness. > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > With a PWM driver that allows to inherit PWM state from the bootloader, > postponing the initial pwm_apply_state() with 0 ns duty cycle allows to > set the desired duty cycle before the PWM is set, avoiding a short flicker > if the backlight was previously enabled and will be enabled again. > --- > drivers/video/backlight/pwm_bl.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index fce412234d10..47a917038f58 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -531,12 +531,10 @@ static int pwm_backlight_probe(struct platform_device *pdev) > if (!state.period && (data->pwm_period_ns > 0)) > state.period = data->pwm_period_ns; > > - ret = pwm_apply_state(pb->pwm, &state); > - if (ret) { > - dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n", > - ret); > - goto err_alloc; > - } > + /* > + * No need to apply initial state, except in the error path. Why do you want to modify the PWM in the error path? I would have expected not touching it at all in .probe() is fine?! > + * State will be applied by backlight_update_status() on success. > + */ > > memset(&props, 0, sizeof(struct backlight_properties)); > Best regards Uwe
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index fce412234d10..47a917038f58 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -531,12 +531,10 @@ static int pwm_backlight_probe(struct platform_device *pdev) if (!state.period && (data->pwm_period_ns > 0)) state.period = data->pwm_period_ns; - ret = pwm_apply_state(pb->pwm, &state); - if (ret) { - dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n", - ret); - goto err_alloc; - } + /* + * No need to apply initial state, except in the error path. + * State will be applied by backlight_update_status() on success. + */ memset(&props, 0, sizeof(struct backlight_properties)); @@ -573,7 +571,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) if (ret < 0) { dev_err(&pdev->dev, "failed to setup default brightness table\n"); - goto err_alloc; + goto err_apply; } for (i = 0; i <= data->max_brightness; i++) { @@ -602,7 +600,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) if (IS_ERR(bl)) { dev_err(&pdev->dev, "failed to register backlight\n"); ret = PTR_ERR(bl); - goto err_alloc; + goto err_apply; } if (data->dft_brightness > data->max_brightness) { @@ -619,6 +617,8 @@ static int pwm_backlight_probe(struct platform_device *pdev) platform_set_drvdata(pdev, bl); return 0; +err_apply: + pwm_apply_state(pb->pwm, &state); err_alloc: if (data->exit) data->exit(&pdev->dev);
The initial PWM state returned by pwm_init_state() has a duty cycle of 0 ns. To avoid backlight flicker when taking over an enabled display from the bootloader, skip the initial pwm_apply_state() and leave the PWM be until backlight_update_state() will apply the state with the desired brightness. Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- With a PWM driver that allows to inherit PWM state from the bootloader, postponing the initial pwm_apply_state() with 0 ns duty cycle allows to set the desired duty cycle before the PWM is set, avoiding a short flicker if the backlight was previously enabled and will be enabled again. --- drivers/video/backlight/pwm_bl.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) --- base-commit: ac9a78681b921877518763ba0e89202254349d1b change-id: 20230608-backlight-pwm-avoid-flicker-d2dd8d12f826 Best regards,