The DSDTs on most Cherry Trail devices have an ugly clutch where the PWM controller gets turned off from the _PS3 method of the graphics-card dev:

Method (_PS3, 0, Serialized) // _PS3: Power State 3 { ... PWMB = PWMC /* _SB_.PCI0.GFX0.PWMC */ PSAT |= 0x03 Local0 = PSAT /* _SB_.PCI0.GFX0.PSAT */ ... }

Where PSAT is the power-status register of the PWM controller.

Since the i915 driver will do a pwm_get on the pwm device as it uses it to control the LCD panel backlight, there is a device-link marking the i915 device as a consumer of the pwm device. So that the PWM controller will always be suspended after the i915 driver suspends (which is the right thing to do). This causes the above GFX0 PS3 AML code to run before acpi_lpss.c calls acpi_lpss_save_ctx().

So on these devices the PWM controller will already be off when acpi_lpss_save_ctx() runs. This causes it to read/save all 1-s (0xffffffff) as ctx register values.

When these bogus values get restored on resume the PWM controller actually keeps working, since most bits are reserved, but this does set bit 3 of the LPSS General purpose register, which for the PWM controller has the following function: "This bit is re-used to support 32kHz slow mode. Default is 19.2MHz as PWM source clock".

This causes the clock of the PWM controller to switch from 19.2MHz to 32KHz, which is a slow-down of a factor 600. Suprisingly enough so far there have been few bug reports about this. This is likely because the i915 driver was hardcoding the PWM frequency to 46 KHz, which divided by 600 would result in a PWM frequency of aprox. 78 Hz, which mostly still works fine. There are some bug reports about the LCD backlight flickering after suspend/resume which are likely caused by this issue.

But with the upcoming patch-series to finally switch the i915 drivers code for external PWM controllers to use the atomic API and to honor the PWM frequency specified in the video BIOS (VBT), this becomes a much bigger problem. On most cases the VBT specifies either 200 Hz or 20 KHz as PWM frequency, which with the mentioned issue ends up being either 1/3 Hz, where the backlight actually visible blinks on and off every 3s, or in 33 Hz and horrible flickering of the backlight.

There are a number of possible solutions to this problem:

1. Make acpi_lpss_save_ctx() run before GFX0._PS3 Pro: Clean solution from pov of not medling with save/restore ctx code Con: As mentioned the current ordering is the right thing to do Con: Requires assymmetry in at what suspend/resume phase we do the save vs restore, requiring more suspend/resume ordering hacks in already convoluted acpi_lpss.c suspend/resume code. 2. Do some sort of save once mode for the LPSS ctx Pro: Reasonably clean Con: Needs a new LPSS flag + code changes to handle the flag 3. Detect we have failed to save the ctx registers and do not restore them Pro: Not PWM specific, might help with issues on other LPSS devices too Con: If we can get away with not restoring the ctx why bother with it at all? 4. Do not save the ctx for CHT PWM controllers Pro: Clean, as simple as dropping a flag? Con: Not so simple as dropping a flag, needs a new flag to ensure that we still do lpss_deassert_reset() on device activation. 5. Make the pwm-lpss code fixup the LPSS-context registers Pro: Keeps acpi_lpss.c code clean Con: Moves knowledge of LPSS-context into the pwm-lpss.c code

1 and 5 both do not seem to be a desirable way forward.

3 and 4 seem ok, but they both assume that restoring the LPSS-context registers is not necessary. I have done a couple of test and those do show that restoring the LPSS-context indeed does not seem to be necessary on devices using s2idle suspend (and successfully reaching S0i3). But I have no hardware to test deep / S3 suspend. So I'm not sure that not restoring the context is safe.

That leaves solution 2, which is about as simple / clean as 3 and 4, so this commit fixes the described problem by implementing a new LPSS_SAVE_CTX_ONCE flag and setting that for the CHT PWM controllers.

Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/acpi_lpss.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index 67892fc0b822..26933e6b7b8c 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -68,6 +68,14 @@ ACPI_MODULE_NAME("acpi_lpss"); #define LPSS_LTR BIT(3) #define LPSS_SAVE_CTX BIT(4) #define LPSS_NO_D3_DELAY BIT(5) +/* + * For some devices the DSDT AML code for another device turns off the device + * before our suspend handler runs, causing us to read/save all 1-s (0xffffffff) + * as ctx register values. + * Luckily these devices always use the same ctx register values, so we can + * work around this by saving the ctx registers once on activation. + */ +#define LPSS_SAVE_CTX_ONCE BIT(6)

struct lpss_private_data;

@@ -254,7 +262,7 @@ static const struct lpss_device_desc byt_pwm_dev_desc = { };

static const struct lpss_device_desc bsw_pwm_dev_desc = { - .flags = LPSS_SAVE_CTX | LPSS_NO_D3_DELAY, + .flags = LPSS_SAVE_CTX_ONCE | LPSS_NO_D3_DELAY, .prv_offset = 0x800, .setup = bsw_pwm_setup, .resume_from_noirq = true, @@ -885,9 +893,14 @@ static int acpi_lpss_activate(struct device *dev) * we have to deassert reset line to be sure that ->probe() will * recognize the device. */ - if (pdata->dev_desc->flags & LPSS_SAVE_CTX) + if (pdata->dev_desc->flags & (LPSS_SAVE_CTX | LPSS_SAVE_CTX_ONCE)) lpss_deassert_reset(pdata);

+#ifdef CONFIG_PM + if (pdata->dev_desc->flags & LPSS_SAVE_CTX_ONCE) + acpi_lpss_save_ctx(dev, pdata); +#endif + return 0; }

@@ -1031,7 +1044,7 @@ static int acpi_lpss_resume(struct device *dev)

acpi_lpss_d3_to_d0_delay(pdata);

- if (pdata->dev_desc->flags & LPSS_SAVE_CTX) + if (pdata->dev_desc->flags & (LPSS_SAVE_CTX | LPSS_SAVE_CTX_ONCE)) acpi_lpss_restore_ctx(dev, pdata);

return 0;

According to the data-sheet the way the PWM controller works is that each input clock-cycle the base_unit gets added to a N bit counter and that counter overflowing determines the PWM output frequency.

So assuming e.g. a 16 bit counter this means that if base_unit is set to 1, after 65535 input clock-cycles the counter has been increased from 0 to 65535 and it will overflow on the next cycle, so it will overflow after every 65536 clock cycles and thus the calculations done in pwm_lpss_prepare() should use 65536 and not 65535.

This commit fixes this. Note this also aligns the calculations in pwm_lpss_prepare() with those in pwm_lpss_get_state().

Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/pwm/pwm-lpss.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index cae74ce61654..a764e062103b 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -93,7 +93,7 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, * The equation is: * base_unit = round(base_unit_range * freq / c) */ - base_unit_range = BIT(lpwm->info->base_unit_bits) - 1; + base_unit_range = BIT(lpwm->info->base_unit_bits); freq *= base_unit_range;

base_unit = DIV_ROUND_CLOSEST_ULL(freq, c); @@ -112,7 +112,7 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm,

orig_ctrl = ctrl = pwm_lpss_read(pwm); ctrl &= ~PWM_ON_TIME_DIV_MASK; - ctrl &= ~(base_unit_range << PWM_BASE_UNIT_SHIFT); + ctrl &= ~((base_unit_range - 1) << PWM_BASE_UNIT_SHIFT); ctrl |= (u32) base_unit << PWM_BASE_UNIT_SHIFT; ctrl |= on_time_div;

Add debug prints, test patch for moving i915 to atomic PWM.

Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/pwm/pwm-lpss.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index 2cb0e2a9c08c..c1f8e6da0cd7 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -119,6 +119,8 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, ctrl |= PWM_ENABLE;

if (orig_ctrl != ctrl) { + dev_err(pwm->chip->dev, "prepare 0x%08x -> 0x%08lx\n", + orig_ctrl, ctrl | PWM_SW_UPDATE); pwm_lpss_write(pwm, ctrl); pwm_lpss_write(pwm, ctrl | PWM_SW_UPDATE); } @@ -126,8 +128,15 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm,

static inline void pwm_lpss_cond_enable(struct pwm_device *pwm, bool cond) { - if (cond) - pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_ENABLE); + if (cond) { + u32 orig_ctrl, ctrl; + + orig_ctrl = ctrl = pwm_lpss_read(pwm); + ctrl |= PWM_ENABLE; + dev_err(pwm->chip->dev, "enable 0x%08x -> 0x%08x\n", + orig_ctrl, ctrl); + pwm_lpss_write(pwm, ctrl); + } }

static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm, @@ -200,6 +209,9 @@ static void pwm_lpss_get_state(struct pwm_chip *chip, struct pwm_device *pwm, state->enabled = !!(ctrl & PWM_ENABLE);

pm_runtime_put(chip->dev); + + dev_err(pwm->chip->dev, "initial state 0x%08x period %d duty_cycle %d enabled %d\n", + ctrl, state->period, state->duty_cycle, state->enabled); }

static const struct pwm_ops pwm_lpss_ops = {

The CRC PWM controller has a clock-divider which divides the clock with a value between 1-128. But as can seen from the PWM_DIV_CLK_xxx defines, this range maps to a register value of 0-127.

So after calculating the clock-divider we must subtract 1 to get the register value, unless the requested frequency was so high that the calculation has already resulted in a (rounded) divider value of 0.

Note that before this fix, setting a period of PWM_MAX_PERIOD_NS which corresponds to the max. divider value of 128 could have resulted in a bug where the code would use 128 as divider-register value which would have resulted in an actual divider value of 0 (and the enable bit being set). A rounding error stopped this bug from actually happen. This same rounding error means that after the subtraction of 1 it is impossible to set the divider to 128. Also bump PWM_MAX_PERIOD_NS by 1 ns to allow setting a divider of 128 (register-value 127).

Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/pwm/pwm-crc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c index 43fc912c1fe9..5ba2a65c524c 100644 --- a/drivers/pwm/pwm-crc.c +++ b/drivers/pwm/pwm-crc.c @@ -22,7 +22,7 @@ #define PWM_MAX_LEVEL 0xFF

#define PWM_BASE_CLK_MHZ 6 /* 6 MHz */ -#define PWM_MAX_PERIOD_NS 5461333 /* 183 Hz */ +#define PWM_MAX_PERIOD_NS 5461334 /* 183 Hz */

#define NSEC_PER_MHZ 1000

@@ -75,6 +75,9 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm, /* changing the clk divisor, need to disable fisrt */ crc_pwm_disable(c, pwm); clk_div = PWM_BASE_CLK_MHZ * period_ns / (256 * NSEC_PER_MHZ); + /* clk_div 1 - 128, maps to register values 0-127 */ + if (clk_div > 0) + clk_div--;

regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, clk_div | PWM_OUTPUT_ENABLE);

The pwm-crc code is using 2 different enable bits: 1. bit 7 of the PWM0_CLK_DIV (PWM_OUTPUT_ENABLE) 2. bit 0 of the BACKLIGHT_EN register

So far we've kept the PWM_OUTPUT_ENABLE bit set when disabling the PWM, this commit makes crc_pwm_disable() clear it on disable and makes crc_pwm_enable() set it again on re-enable.

This should disable the internal (divided) PWM clock and tri-state the PWM output pin when disabled, saving some power.

Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/pwm/pwm-crc.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c index ef49a6e3c4d6..53734bcf67e1 100644 --- a/drivers/pwm/pwm-crc.c +++ b/drivers/pwm/pwm-crc.c @@ -41,10 +41,24 @@ static inline struct crystalcove_pwm *to_crc_pwm(struct pwm_chip *pc) return container_of(pc, struct crystalcove_pwm, chip); }

+static int crc_pwm_calc_clk_div(int period_ns) +{ + int clk_div; + + clk_div = PWM_BASE_CLK_MHZ * period_ns / (256 * NSEC_PER_MHZ); + /* clk_div 1 - 128, maps to register values 0-127 */ + if (clk_div > 0) + clk_div--; + + return clk_div; +} + static int crc_pwm_enable(struct pwm_chip *c, struct pwm_device *pwm) { struct crystalcove_pwm *crc_pwm = to_crc_pwm(c); + int div = crc_pwm_calc_clk_div(pwm_get_period(pwm));

+ regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, div | PWM_OUTPUT_ENABLE); regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 1);

return 0; @@ -53,8 +67,10 @@ static int crc_pwm_enable(struct pwm_chip *c, struct pwm_device *pwm) static void crc_pwm_disable(struct pwm_chip *c, struct pwm_device *pwm) { struct crystalcove_pwm *crc_pwm = to_crc_pwm(c); + int div = crc_pwm_calc_clk_div(pwm_get_period(pwm));

regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 0); + regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, div); }

static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm, @@ -70,16 +86,10 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm, }

if (pwm_get_period(pwm) != period_ns) { - int clk_div; + int clk_div = crc_pwm_calc_clk_div(period_ns);

/* changing the clk divisor, clear PWM_OUTPUT_ENABLE first */ regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, 0); - - clk_div = PWM_BASE_CLK_MHZ * period_ns / (256 * NSEC_PER_MHZ); - /* clk_div 1 - 128, maps to register values 0-127 */ - if (clk_div > 0) - clk_div--; - regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, clk_div | PWM_OUTPUT_ENABLE); }

Implement the pwm_ops.get_state() method to complete the support for the new atomic PWM API.

Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/pwm/pwm-crc.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)

diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c index 58c7e9ef7278..6c75a3470bc8 100644 --- a/drivers/pwm/pwm-crc.c +++ b/drivers/pwm/pwm-crc.c @@ -114,8 +114,37 @@ static int crc_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, return 0; }

+static void crc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct crystalcove_pwm *crc_pwm = to_crc_pwm(chip); + struct device *dev = crc_pwm->chip.dev; + unsigned int clk_div, clk_div_reg, duty_cycle_reg; + int error; + + error = regmap_read(crc_pwm->regmap, PWM0_CLK_DIV, &clk_div_reg); + if (error) { + dev_err(dev, "Error reading PWM0_CLK_DIV %d\n", error); + return; + } + + error = regmap_read(crc_pwm->regmap, PWM0_DUTY_CYCLE, &duty_cycle_reg); + if (error) { + dev_err(dev, "Error reading PWM0_DUTY_CYCLE %d\n", error); + return; + } + + clk_div = (clk_div_reg & ~PWM_OUTPUT_ENABLE) + 1; + + state->period = clk_div * NSEC_PER_MHZ * 256 / PWM_BASE_CLK_MHZ; + state->duty_cycle = duty_cycle_reg * state->period / PWM_MAX_LEVEL; + state->polarity = PWM_POLARITY_NORMAL; + state->enabled = !!(clk_div_reg & PWM_OUTPUT_ENABLE); +} + static const struct pwm_ops crc_pwm_ops = { .apply = crc_pwm_apply, + .get_state = crc_pwm_get_state, };

static int crystalcove_pwm_probe(struct platform_device *pdev)

So far for devices using an external PWM controller (devices using pwm_setup_backlight()), we have been hardcoding the period-time passed to pwm_config() to 21333 ns.

I suspect this was done because many VBTs set the PWM frequency to 200 which corresponds to a period-time of 5000000 ns, which greatly exceeds the PWM_MAX_PERIOD_NS define in the Crystal Cove PMIC PWM driver, which used to be 21333.

This PWM_MAX_PERIOD_NS define was actually based on a bug in the PWM driver where its period and duty-cycle times where off by a factor of 256.

Due to this bug the hardcoded CRC_PMIC_PWM_PERIOD_NS value of 21333 would result in the PWM driver using its divider of 128, which would result in a PWM output frequency of 6000000 Hz / 256 / 128 = 183 Hz. So actually pretty close to the default VBT value of 200 Hz.

Now that this bug in the pwm-crc driver is fixed, we can actually use the VBT defined frequency.

This is important because:

a) With the pwm-crc driver fixed it will now translate the hardcoded CRC_PMIC_PWM_PERIOD_NS value of 21333 ns / 46 Khz to a PWM output frequency of 23 KHz (the max it can do).

b) The pwm-lpss driver used on many models has always honored the 21333 ns / 46 Khz request

Some panels do not like such high output frequencies. E.g. on a Terra Pad 1061 tablet, using the LPSS PWM controller, the backlight would go from off to max, when changing the sysfs backlight brightness value from 90-100%, anything under aprox. 90% would turn the backlight fully off.

Honoring the VBT specified PWM frequency will also hopefully fix the various bug reports which we have received about users perceiving the backlight to flicker after a suspend/resume cycle.

Signed-off-by: Hans de Goede hdegoede@redhat.com --- .../drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_panel.c | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index b24266c624fa..24ea4a7b6dde 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -223,6 +223,7 @@ struct intel_panel { bool util_pin_active_low; /* bxt+ */ u8 controller; /* bxt+ only */ struct pwm_device *pwm; + int pwm_period_ns;

/* DPCD backlight */ u8 pwmgen_bit_count; diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 8efdd9f08a08..14e611c92194 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -40,8 +40,6 @@ #include "intel_dsi_dcs_backlight.h" #include "intel_panel.h"

-#define CRC_PMIC_PWM_PERIOD_NS 21333 - void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, struct drm_display_mode *adjusted_mode) @@ -597,7 +595,7 @@ static u32 pwm_get_backlight(struct intel_connector *connector) int duty_ns;

duty_ns = pwm_get_duty_cycle(panel->backlight.pwm); - return DIV_ROUND_UP(duty_ns * 100, CRC_PMIC_PWM_PERIOD_NS); + return DIV_ROUND_UP(duty_ns * 100, panel->backlight.pwm_period_ns); }

static void lpt_set_backlight(const struct drm_connector_state *conn_state, u32 level) @@ -671,9 +669,10 @@ static void bxt_set_backlight(const struct drm_connector_state *conn_state, u32 static void pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level) { struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel; - int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100); + int duty_ns = DIV_ROUND_UP(level * panel->backlight.pwm_period_ns, 100);

- pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS); + pwm_config(panel->backlight.pwm, duty_ns, + panel->backlight.pwm_period_ns); }

static void @@ -1917,6 +1916,9 @@ static int pwm_setup_backlight(struct intel_connector *connector, return -ENODEV; }

+ panel->backlight.pwm_period_ns = NSEC_PER_SEC / + get_vbt_pwm_freq(dev_priv); + /* * FIXME: pwm_apply_args() should be removed when switching to * the atomic PWM API. @@ -1926,9 +1928,10 @@ static int pwm_setup_backlight(struct intel_connector *connector, panel->backlight.min = 0; /* 0% */ panel->backlight.max = 100; /* 100% */ level = intel_panel_compute_brightness(connector, 100); - ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100); + ns = DIV_ROUND_UP(level * panel->backlight.pwm_period_ns, 100);

- retval = pwm_config(panel->backlight.pwm, ns, CRC_PMIC_PWM_PERIOD_NS); + retval = pwm_config(panel->backlight.pwm, ns, + panel->backlight.pwm_period_ns); if (retval < 0) { drm_err(&dev_priv->drm, "Failed to configure the pwm chip\n"); pwm_put(panel->backlight.pwm); @@ -1937,7 +1940,7 @@ static int pwm_setup_backlight(struct intel_connector *connector, }

level = DIV_ROUND_UP(pwm_get_duty_cycle(panel->backlight.pwm) * 100, - CRC_PMIC_PWM_PERIOD_NS); + panel->backlight.pwm_period_ns); panel->backlight.level = intel_panel_compute_brightness(connector, level); panel->backlight.enabled = panel->backlight.level != 0;

Now that the PWM drivers which we use have been converted to the atomic PWM API, we can move the i915 panel code over to using the atomic PWM API.

The removes a long standing FIXME and this removes a flicker where the backlight brightness would jump to 100% when i915 loads even if using the fastset path.

Signed-off-by: Hans de Goede hdegoede@redhat.com --- .../drm/i915/display/intel_display_types.h | 3 +- drivers/gpu/drm/i915/display/intel_panel.c | 73 +++++++++---------- 2 files changed, 37 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 24ea4a7b6dde..48afb2925271 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -28,6 +28,7 @@

#include <linux/async.h> #include <linux/i2c.h> +#include <linux/pwm.h> #include <linux/sched/clock.h>

#include <drm/drm_atomic.h> @@ -223,7 +224,7 @@ struct intel_panel { bool util_pin_active_low; /* bxt+ */ u8 controller; /* bxt+ only */ struct pwm_device *pwm; - int pwm_period_ns; + struct pwm_state pwm_state;

/* DPCD backlight */ u8 pwmgen_bit_count; diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index cb28b9908ca4..a0f76343f381 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -592,10 +592,11 @@ static u32 bxt_get_backlight(struct intel_connector *connector) static u32 pwm_get_backlight(struct intel_connector *connector) { struct intel_panel *panel = &connector->panel; - int duty_ns; + int duty_ns, period_ns;

duty_ns = pwm_get_duty_cycle(panel->backlight.pwm); - return DIV_ROUND_UP(duty_ns * 100, panel->backlight.pwm_period_ns); + period_ns = pwm_get_period(panel->backlight.pwm); + return DIV_ROUND_UP(duty_ns * 100, period_ns); }

static void lpt_set_backlight(const struct drm_connector_state *conn_state, u32 level) @@ -669,10 +670,10 @@ static void bxt_set_backlight(const struct drm_connector_state *conn_state, u32 static void pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level) { struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel; - int duty_ns = DIV_ROUND_UP(level * panel->backlight.pwm_period_ns, 100);

- pwm_config(panel->backlight.pwm, duty_ns, - panel->backlight.pwm_period_ns); + panel->backlight.pwm_state.duty_cycle = + DIV_ROUND_UP(level * panel->backlight.pwm_state.period, 100); + pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state); }

static void @@ -841,10 +842,8 @@ static void pwm_disable_backlight(const struct drm_connector_state *old_conn_sta struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct intel_panel *panel = &connector->panel;

- /* Disable the backlight */ - intel_panel_actually_set_backlight(old_conn_state, 0); - usleep_range(2000, 3000); - pwm_disable(panel->backlight.pwm); + panel->backlight.pwm_state.enabled = false; + pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state); }

void intel_panel_disable_backlight(const struct drm_connector_state *old_conn_state) @@ -1176,9 +1175,14 @@ static void pwm_enable_backlight(const struct intel_crtc_state *crtc_state, { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct intel_panel *panel = &connector->panel; + int level = panel->backlight.level;

- pwm_enable(panel->backlight.pwm); - intel_panel_actually_set_backlight(conn_state, panel->backlight.level); + level = intel_panel_compute_brightness(connector, level); + + panel->backlight.pwm_state.duty_cycle = + DIV_ROUND_UP(level * panel->backlight.pwm_state.period, 100); + panel->backlight.pwm_state.enabled = true; + pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state); }

static void __intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state, @@ -1897,8 +1901,7 @@ static int pwm_setup_backlight(struct intel_connector *connector, struct drm_i915_private *dev_priv = to_i915(dev); struct intel_panel *panel = &connector->panel; const char *desc; - u32 level, ns; - int retval; + u32 level;

/* Get the right PWM chip for DSI backlight according to VBT */ if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) { @@ -1916,36 +1919,30 @@ static int pwm_setup_backlight(struct intel_connector *connector, return -ENODEV; }

- panel->backlight.pwm_period_ns = NSEC_PER_SEC / - get_vbt_pwm_freq(dev_priv); - - /* - * FIXME: pwm_apply_args() should be removed when switching to - * the atomic PWM API. - */ - pwm_apply_args(panel->backlight.pwm); - panel->backlight.max = 100; /* 100% */ panel->backlight.min = get_backlight_min_vbt(connector); - level = intel_panel_compute_brightness(connector, 100); - ns = DIV_ROUND_UP(level * panel->backlight.pwm_period_ns, 100);

- retval = pwm_config(panel->backlight.pwm, ns, - panel->backlight.pwm_period_ns); - if (retval < 0) { - drm_err(&dev_priv->drm, "Failed to configure the pwm chip\n"); - pwm_put(panel->backlight.pwm); - panel->backlight.pwm = NULL; - return retval; + if (pwm_is_enabled(panel->backlight.pwm) && + pwm_get_period(panel->backlight.pwm)) { + /* PWM is already enabled, use existing settings */ + pwm_get_state(panel->backlight.pwm, &panel->backlight.pwm_state); + + level = DIV_ROUND_UP(panel->backlight.pwm_state.duty_cycle * + 100, panel->backlight.pwm_state.period); + level = intel_panel_compute_brightness(connector, level); + panel->backlight.level = clamp(level, panel->backlight.min, + panel->backlight.max); + panel->backlight.enabled = true; + + drm_dbg_kms(&dev_priv->drm, "PWM already enabled at freq %ld, VBT freq %d, level %d\n", + NSEC_PER_SEC / panel->backlight.pwm_state.period, + get_vbt_pwm_freq(dev_priv), level); + } else { + /* Set period from VBT frequency, leave other setting at 0. */ + panel->backlight.pwm_state.period = + NSEC_PER_SEC / get_vbt_pwm_freq(dev_priv); }

- level = DIV_ROUND_UP(pwm_get_duty_cycle(panel->backlight.pwm) * 100, - panel->backlight.pwm_period_ns); - level = intel_panel_compute_brightness(connector, level); - panel->backlight.level = clamp(level, panel->backlight.min, - panel->backlight.max); - panel->backlight.enabled = panel->backlight.level != 0; - drm_info(&dev_priv->drm, "Using %s PWM for LCD backlight control\n", desc); return 0;