From: Hans de Goede <hdegoede@redhat.com>
To: "Thierry Reding" <thierry.reding@gmail.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Jani Nikula" <jani.nikula@linux.intel.com>,
"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Rafael J . Wysocki" <rjw@rjwysocki.net>,
"Len Brown" <lenb@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>,
linux-pwm@vger.kernel.org,
intel-gfx <intel-gfx@lists.freedesktop.org>,
dri-devel@lists.freedesktop.org,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
linux-acpi@vger.kernel.org, Jani Nikula <jani.nikula@intel.com>
Subject: [PATCH v9 17/17] drm/i915: panel: Use atomic PWM API for devs with an external PWM controller
Date: Thu, 3 Sep 2020 12:51:14 +0200 [thread overview]
Message-ID: <20200903105114.9969-18-hdegoede@redhat.com> (raw)
In-Reply-To: <20200903105114.9969-1-hdegoede@redhat.com>
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.
Note that this commit also simplifies pwm_disable_backlight(), by dropping
the intel_panel_actually_set_backlight(..., 0) call. This call sets the
PWM to 0% duty-cycle. I believe that this call was only present as a
workaround for a bug in the pwm-crc.c driver where it failed to clear the
PWM_OUTPUT_ENABLE bit. This is fixed by an earlier patch in this series.
After the dropping of this workaround, the usleep call, which seems
unnecessary to begin with, has no useful effect anymore, so drop that too.
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v7:
- Fix a u64 divide leading to undefined reference to `__udivdi3' errors on 32 bit
platforms by casting the divisor to an unsigned long
Changes in v6:
- Drop the pwm_get_period() check in pwm_setup(), it is now longer needed
now that we use pwm_get_relative_duty_cycle()
Changes in v4:
- Add a note to the commit message about the dropping of the
intel_panel_actually_set_backlight() and usleep() calls from
pwm_disable_backlight()
- Use the pwm_set/get_relative_duty_cycle() helpers instead of using DIY code
for this
---
.../drm/i915/display/intel_display_types.h | 3 +-
drivers/gpu/drm/i915/display/intel_panel.c | 70 ++++++++-----------
2 files changed, 33 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 7171e7c8d928..e3ebe7c313ba 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 2b27f9b07403..2eb1a3a93df1 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -592,10 +592,10 @@ 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;
+ struct pwm_state state;
- duty_ns = pwm_get_duty_cycle(panel->backlight.pwm);
- return DIV_ROUND_UP(duty_ns * 100, panel->backlight.pwm_period_ns);
+ pwm_get_state(panel->backlight.pwm, &state);
+ return pwm_get_relative_duty_cycle(&state, 100);
}
static void lpt_set_backlight(const struct drm_connector_state *conn_state, u32 level)
@@ -669,10 +669,9 @@ 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);
+ pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
+ pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
}
static void
@@ -841,10 +840,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 +1173,12 @@ 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);
+ pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 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 +1897,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,35 +1915,28 @@ 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 is already enabled, use existing settings */
+ pwm_get_state(panel->backlight.pwm, &panel->backlight.pwm_state);
- level = DIV_ROUND_UP_ULL(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;
+ level = pwm_get_relative_duty_cycle(&panel->backlight.pwm_state,
+ 100);
+ 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 / (unsigned long)panel->backlight.pwm_state.period,
+ get_vbt_pwm_freq(dev_priv), level);
+ } else {
+ /* Set period from VBT frequency, leave other settings at 0. */
+ panel->backlight.pwm_state.period =
+ NSEC_PER_SEC / get_vbt_pwm_freq(dev_priv);
+ }
drm_info(&dev_priv->drm, "Using %s PWM for LCD backlight control\n",
desc);
--
2.28.0
prev parent reply other threads:[~2020-09-03 10:52 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-03 10:50 [PATCH v9 00/17] acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API Hans de Goede
2020-09-03 10:50 ` [PATCH v9 01/17] ACPI / LPSS: Resume Cherry Trail PWM controller in no-irq phase Hans de Goede
2020-09-03 10:50 ` [PATCH v9 02/17] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (at activation) Hans de Goede
2020-09-03 10:51 ` [PATCH v9 03/17] pwm: lpss: Fix off by one error in base_unit math in pwm_lpss_prepare() Hans de Goede
2020-09-03 10:51 ` [PATCH v9 04/17] pwm: lpss: Add range limit check for the base_unit register value Hans de Goede
2020-09-03 10:51 ` [PATCH v9 05/17] pwm: lpss: Add pwm_lpss_prepare_enable() helper Hans de Goede
2020-09-03 10:51 ` [PATCH v9 06/17] pwm: lpss: Make pwm_lpss_apply() not rely on existing hardware state Hans de Goede
2020-09-03 10:59 ` Thierry Reding
2020-09-03 11:12 ` Hans de Goede
2020-09-03 10:51 ` [PATCH v9 07/17] pwm: lpss: Remove suspend/resume handlers Hans de Goede
2020-09-03 11:00 ` Thierry Reding
2020-09-03 10:51 ` [PATCH v9 08/17] pwm: crc: Fix period / duty_cycle times being off by a factor of 256 Hans de Goede
2020-09-03 10:51 ` [PATCH v9 09/17] pwm: crc: Fix off-by-one error in the clock-divider calculations Hans de Goede
2020-09-03 10:51 ` [PATCH v9 10/17] pwm: crc: Fix period changes not having any effect Hans de Goede
2020-09-03 10:51 ` [PATCH v9 11/17] pwm: crc: Enable/disable PWM output on enable/disable Hans de Goede
2020-09-03 10:51 ` [PATCH v9 12/17] pwm: crc: Implement apply() method to support the new atomic PWM API Hans de Goede
2020-09-03 10:51 ` [PATCH v9 13/17] pwm: crc: Implement get_state() method Hans de Goede
2020-09-03 10:51 ` [PATCH v9 14/17] drm/i915: panel: Add get_vbt_pwm_freq() helper Hans de Goede
2020-09-03 10:51 ` [PATCH v9 15/17] drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller Hans de Goede
2020-09-03 10:51 ` [PATCH v9 16/17] drm/i915: panel: Honor the VBT PWM min setting " Hans de Goede
2020-09-03 10:51 ` Hans de Goede [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200903105114.9969-18-hdegoede@redhat.com \
--to=hdegoede@redhat.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
--cc=jani.nikula@linux.intel.com \
--cc=joonas.lahtinen@linux.intel.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=rjw@rjwysocki.net \
--cc=rodrigo.vivi@intel.com \
--cc=thierry.reding@gmail.com \
--cc=u.kleine-koenig@pengutronix.de \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).