According to the datasheet the update bit must be set if the on-time-div or the base-unit changes. Now that we properly order device resume on Cherry Trail so that the GFX0 _PS0 method no longer exits with an error, we end up with a sequence of events where we are writing the same values twice in a row. First the _PS0 method restores the duty cycle of 0% the GPU driver set on suspend and then the GPU driver first updates just the enabled bit in the pwm_state from 0 to 1, causing us to write the same values again, before restoring the pre-suspend duty-cycle in a separate pwm_apply call. When writing the update bit the second time, without changing any of the values the update bit clears immediately / instantly, instead of staying 1 for a while as usual. After this the next setting of the update bit seems to be ignored, causing the restoring of the pre-suspend duty-cycle to not get applied. This makes the backlight come up with a 0% dutycycle after suspend/resume. Any further brightness changes after this do work. This commit moves the setting of the update bit into pwm_lpss_prepare() and only sets the bit if we have actually changed any of the values. This avoids the setting of the update bit the second time we configure the PWM to 0% dutycycle, this fixes the backlight coming up with 0% duty-cycle after a suspend/resume. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- drivers/pwm/pwm-lpss.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index 293e5139b091..f170873e6fb4 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -88,7 +88,7 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, unsigned long long on_time_div; unsigned long c = lpwm->info->clk_rate, base_unit_range; unsigned long long base_unit, freq = NSEC_PER_SEC; - u32 ctrl; + u32 orig_ctrl, ctrl; do_div(freq, period_ns); @@ -105,13 +105,17 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, do_div(on_time_div, period_ns); on_time_div = 255ULL - on_time_div; - ctrl = pwm_lpss_read(pwm); + orig_ctrl = ctrl = pwm_lpss_read(pwm); ctrl &= ~PWM_ON_TIME_DIV_MASK; ctrl &= ~(base_unit_range << PWM_BASE_UNIT_SHIFT); base_unit &= base_unit_range; ctrl |= (u32) base_unit << PWM_BASE_UNIT_SHIFT; ctrl |= on_time_div; - pwm_lpss_write(pwm, ctrl); + + if (orig_ctrl != ctrl) { + pwm_lpss_write(pwm, ctrl); + pwm_lpss_write(pwm, ctrl | PWM_SW_UPDATE); + } } static inline void pwm_lpss_cond_enable(struct pwm_device *pwm, bool cond) @@ -135,7 +139,6 @@ static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm, return ret; } pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period); - pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_SW_UPDATE); pwm_lpss_cond_enable(pwm, lpwm->info->bypass == false); ret = pwm_lpss_wait_for_update(pwm); if (ret) { @@ -148,7 +151,6 @@ static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (ret) return ret; pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period); - pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_SW_UPDATE); return pwm_lpss_wait_for_update(pwm); } } else if (pwm_is_enabled(pwm)) { -- 2.19.0