From patchwork Sun Oct 14 15:12:01 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 983775 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 42Y4mm63qWz9s9J for ; Mon, 15 Oct 2018 02:12:08 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726400AbeJNWxX (ORCPT ); Sun, 14 Oct 2018 18:53:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38522 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726309AbeJNWxX (ORCPT ); Sun, 14 Oct 2018 18:53:23 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ED16D30820C4; Sun, 14 Oct 2018 15:12:07 +0000 (UTC) Received: from shalem.localdomain.com (ovpn-116-22.ams2.redhat.com [10.36.116.22]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2A9A72E64D; Sun, 14 Oct 2018 15:12:06 +0000 (UTC) From: Hans de Goede To: Thierry Reding , Andy Shevchenko Cc: Hans de Goede , linux-pwm@vger.kernel.org, linux-acpi@vger.kernel.org, "Rafael J . Wysocki" , Len Brown Subject: [PATCH 1/2] pwm: lpss: Force runtime-resume on suspend on Cherry Trail Date: Sun, 14 Oct 2018 17:12:01 +0200 Message-Id: <20181014151202.29955-2-hdegoede@redhat.com> In-Reply-To: <20181014151202.29955-1-hdegoede@redhat.com> References: <20181014151202.29955-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Sun, 14 Oct 2018 15:12:08 +0000 (UTC) Sender: linux-pwm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org On Cherry Trail devices under Windows the PWM controller used for the backlight is considered part of the GPU even though it is part of the LPSS block and thus is an entirely different independent hardware unit. Because of this on Cherry Trail the GPU's (GFX0 ACPI node) _PS3 and _PS0 methods save and restore the PWM controller registers. If userspace blanks the screen before suspending, such as e.g. GNOME does, then the PWM controller will be runtime-suspended when the suspend starts. This causes the GFX0 _PS? methods to save a value of 0xffffffff for the PWM control register and to restore this value on resume. 0xffffffff is not a valid value for the register and writing this causes problems such as e.g. a flickering backlight. This commit adds a prepare method to the dev_pm_ops and makes it return 0 on Cherry Trail devices forcing a runtime-resume before other device's suspend methods run. This fixes the reading and writing back of 0xffffffff. Since we now always runtime-resume the device on suspend, it will be resumed on resume too and we no longer need to check for the GFX0 _PS0 method having resumed it underneath us, so this commit removes the now no longer necessary complete dev_pm_op. Signed-off-by: Hans de Goede --- drivers/pwm/pwm-lpss-platform.c | 24 +++++++++++------------- drivers/pwm/pwm-lpss.h | 7 +++++-- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c index b6edf8af26cc..757230e1f575 100644 --- a/drivers/pwm/pwm-lpss-platform.c +++ b/drivers/pwm/pwm-lpss-platform.c @@ -30,7 +30,7 @@ static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { .clk_rate = 19200000, .npwm = 1, .base_unit_bits = 16, - .check_power_on_resume = true, + .other_devices_aml_touches_pwm_regs = true, }; /* Broxton */ @@ -61,6 +61,7 @@ static int pwm_lpss_probe_platform(struct platform_device *pdev) platform_set_drvdata(pdev, lpwm); + dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); @@ -75,25 +76,22 @@ static int pwm_lpss_remove_platform(struct platform_device *pdev) return pwm_lpss_remove(lpwm); } -static void pwm_lpss_complete(struct device *dev) +static int pwm_lpss_prepare(struct device *dev) { struct pwm_lpss_chip *lpwm = dev_get_drvdata(dev); - int ret, state; - /* The PWM may be turned on by AML code, update our state to match */ - if (pm_runtime_suspended(dev) && lpwm->info->check_power_on_resume) { - pm_runtime_disable(dev); + /* + * If other device's AML code touches the PWM regs on suspend/resume + * force runtime-resume the PWM controller to allow this. + */ + if (lpwm->info->other_devices_aml_touches_pwm_regs) + return 0; /* Force runtime-resume */ - ret = acpi_device_get_power(ACPI_COMPANION(dev), &state); - if (ret == 0 && state == ACPI_STATE_D0) - pm_runtime_set_active(dev); - - pm_runtime_enable(dev); - } + return 1; /* If runtime-suspended leave as is */ } static const struct dev_pm_ops pwm_lpss_platform_pm_ops = { - .complete = pwm_lpss_complete, + .prepare = pwm_lpss_prepare, SET_SYSTEM_SLEEP_PM_OPS(pwm_lpss_suspend, pwm_lpss_resume) }; diff --git a/drivers/pwm/pwm-lpss.h b/drivers/pwm/pwm-lpss.h index 1a2575d25bea..3236be835bd9 100644 --- a/drivers/pwm/pwm-lpss.h +++ b/drivers/pwm/pwm-lpss.h @@ -30,8 +30,11 @@ struct pwm_lpss_boardinfo { unsigned int npwm; unsigned long base_unit_bits; bool bypass; - /* Some devices have AML code messing with the state underneath us */ - bool check_power_on_resume; + /* + * On some devices the _PS0/_PS3 AML code of the GPU (GFX0) device + * messes with the PWM0 controllers state, + */ + bool other_devices_aml_touches_pwm_regs; }; struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, struct resource *r, From patchwork Sun Oct 14 15:12:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 983776 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 42Y4mp2br2z9s3T for ; Mon, 15 Oct 2018 02:12:10 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726417AbeJNWxZ (ORCPT ); Sun, 14 Oct 2018 18:53:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34708 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726309AbeJNWxZ (ORCPT ); Sun, 14 Oct 2018 18:53:25 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AFF86308424F; Sun, 14 Oct 2018 15:12:09 +0000 (UTC) Received: from shalem.localdomain.com (ovpn-116-22.ams2.redhat.com [10.36.116.22]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3E44D2A71F; Sun, 14 Oct 2018 15:12:08 +0000 (UTC) From: Hans de Goede To: Thierry Reding , Andy Shevchenko Cc: Hans de Goede , linux-pwm@vger.kernel.org, linux-acpi@vger.kernel.org, "Rafael J . Wysocki" , Len Brown Subject: [PATCH 2/2] pwm: lpss: Only set update bit if we are actually changing the settings Date: Sun, 14 Oct 2018 17:12:02 +0200 Message-Id: <20181014151202.29955-3-hdegoede@redhat.com> In-Reply-To: <20181014151202.29955-1-hdegoede@redhat.com> References: <20181014151202.29955-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Sun, 14 Oct 2018 15:12:09 +0000 (UTC) Sender: linux-pwm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org 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 --- 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)) {