From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 20/23] drm/i915/dp: Markup pps lock power well
Date: Tue, 4 Sep 2018 22:57:39 +0100 [thread overview]
Message-ID: <20180904215742.20276-20-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <20180904215742.20276-1-chris@chris-wilson.co.uk>
Track where and when we acquire and release the power well for pps
access along the dp aux link, with a view to detecting if we leak any
wakerefs.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_dp.c | 223 +++++++++++++++++---------------
1 file changed, 116 insertions(+), 107 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4c897d63f8d2..98c43c4b38db 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -652,28 +652,35 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
static void
intel_dp_pps_init(struct intel_dp *intel_dp);
-static void pps_lock(struct intel_dp *intel_dp)
+static intel_wakeref_t pps_lock(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+ intel_wakeref_t wakeref;
/*
* See intel_power_sequencer_reset() why we need
* a power domain reference here.
*/
- intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+ wakeref = intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
mutex_lock(&dev_priv->pps_mutex);
+
+ return wakeref;
}
-static void pps_unlock(struct intel_dp *intel_dp)
+static intel_wakeref_t pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
mutex_unlock(&dev_priv->pps_mutex);
- intel_display_power_put_unchecked(dev_priv, intel_dp->aux_power_domain);
+ intel_display_power_put(dev_priv, intel_dp->aux_power_domain, wakeref);
+ return 0;
}
+#define with_pps_lock(dp, wf) \
+ for (wf = pps_lock(dp); wf; wf = pps_unlock(dp, wf))
+
static void
vlv_power_sequencer_kick(struct intel_dp *intel_dp)
{
@@ -1022,30 +1029,30 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code,
struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
edp_notifier);
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+ intel_wakeref_t wakeref;
if (!intel_dp_is_edp(intel_dp) || code != SYS_RESTART)
return 0;
- pps_lock(intel_dp);
-
- if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
- enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
- i915_reg_t pp_ctrl_reg, pp_div_reg;
- u32 pp_div;
-
- pp_ctrl_reg = PP_CONTROL(pipe);
- pp_div_reg = PP_DIVISOR(pipe);
- pp_div = I915_READ(pp_div_reg);
- pp_div &= PP_REFERENCE_DIVIDER_MASK;
-
- /* 0x1F write to PP_DIV_REG sets max cycle delay */
- I915_WRITE(pp_div_reg, pp_div | 0x1F);
- I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
- msleep(intel_dp->panel_power_cycle_delay);
+ with_pps_lock(intel_dp, wakeref) {
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+ i915_reg_t pp_ctrl_reg, pp_div_reg;
+ u32 pp_div;
+
+ pp_ctrl_reg = PP_CONTROL(pipe);
+ pp_div_reg = PP_DIVISOR(pipe);
+ pp_div = I915_READ(pp_div_reg);
+ pp_div &= PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+ PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp->panel_power_cycle_delay);
+ }
}
- pps_unlock(intel_dp);
-
return 0;
}
@@ -1231,16 +1238,17 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
to_i915(intel_dig_port->base.base.dev);
i915_reg_t ch_ctl, ch_data[5];
uint32_t aux_clock_divider;
+ intel_wakeref_t wakeref;
int i, ret, recv_bytes;
- uint32_t status;
int try, clock = 0;
+ uint32_t status;
bool vdd;
ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
for (i = 0; i < ARRAY_SIZE(ch_data); i++)
ch_data[i] = intel_dp->aux_ch_data_reg(intel_dp, i);
- pps_lock(intel_dp);
+ wakeref = pps_lock(intel_dp);
/*
* We will be called with VDD already enabled for dpcd/edid/oui reads.
@@ -1384,7 +1392,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
if (vdd)
edp_panel_vdd_off(intel_dp, false);
- pps_unlock(intel_dp);
+ pps_unlock(intel_dp, wakeref);
return ret;
}
@@ -2347,15 +2355,15 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
*/
void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
{
+ intel_wakeref_t wakeref;
bool vdd;
if (!intel_dp_is_edp(intel_dp))
return;
- pps_lock(intel_dp);
- vdd = edp_panel_vdd_on(intel_dp);
- pps_unlock(intel_dp);
-
+ vdd = false;
+ with_pps_lock(intel_dp, wakeref)
+ vdd = edp_panel_vdd_on(intel_dp);
I915_STATE_WARN(!vdd, "eDP port %c VDD already requested on\n",
port_name(dp_to_dig_port(intel_dp)->base.port));
}
@@ -2399,13 +2407,15 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
static void edp_panel_vdd_work(struct work_struct *__work)
{
- struct intel_dp *intel_dp = container_of(to_delayed_work(__work),
- struct intel_dp, panel_vdd_work);
+ struct intel_dp *intel_dp =
+ container_of(to_delayed_work(__work),
+ struct intel_dp, panel_vdd_work);
+ intel_wakeref_t wakeref;
- pps_lock(intel_dp);
- if (!intel_dp->want_panel_vdd)
- edp_panel_vdd_off_sync(intel_dp);
- pps_unlock(intel_dp);
+ with_pps_lock(intel_dp, wakeref) {
+ if (!intel_dp->want_panel_vdd)
+ edp_panel_vdd_off_sync(intel_dp);
+ }
}
static void edp_panel_vdd_schedule_off(struct intel_dp *intel_dp)
@@ -2495,12 +2505,13 @@ static void edp_panel_on(struct intel_dp *intel_dp)
void intel_edp_panel_on(struct intel_dp *intel_dp)
{
+ intel_wakeref_t wakeref;
+
if (!intel_dp_is_edp(intel_dp))
return;
- pps_lock(intel_dp);
- edp_panel_on(intel_dp);
- pps_unlock(intel_dp);
+ with_pps_lock(intel_dp, wakeref)
+ edp_panel_on(intel_dp);
}
@@ -2543,20 +2554,20 @@ static void edp_panel_off(struct intel_dp *intel_dp)
void intel_edp_panel_off(struct intel_dp *intel_dp)
{
+ intel_wakeref_t wakeref;
+
if (!intel_dp_is_edp(intel_dp))
return;
- pps_lock(intel_dp);
- edp_panel_off(intel_dp);
- pps_unlock(intel_dp);
+ with_pps_lock(intel_dp, wakeref)
+ edp_panel_off(intel_dp);
}
/* Enable backlight in the panel power control. */
static void _intel_edp_backlight_on(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
- u32 pp;
- i915_reg_t pp_ctrl_reg;
+ intel_wakeref_t wakeref;
/*
* If we enable the backlight right away following a panel power
@@ -2566,17 +2577,16 @@ static void _intel_edp_backlight_on(struct intel_dp *intel_dp)
*/
wait_backlight_on(intel_dp);
- pps_lock(intel_dp);
+ with_pps_lock(intel_dp, wakeref) {
+ i915_reg_t pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
+ u32 pp;
- pp = ironlake_get_pp_control(intel_dp);
- pp |= EDP_BLC_ENABLE;
+ pp = ironlake_get_pp_control(intel_dp);
+ pp |= EDP_BLC_ENABLE;
- pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
-
- I915_WRITE(pp_ctrl_reg, pp);
- POSTING_READ(pp_ctrl_reg);
-
- pps_unlock(intel_dp);
+ I915_WRITE(pp_ctrl_reg, pp);
+ POSTING_READ(pp_ctrl_reg);
+ }
}
/* Enable backlight PWM and backlight PP control. */
@@ -2598,23 +2608,21 @@ void intel_edp_backlight_on(const struct intel_crtc_state *crtc_state,
static void _intel_edp_backlight_off(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
- u32 pp;
- i915_reg_t pp_ctrl_reg;
+ intel_wakeref_t wakeref;
if (!intel_dp_is_edp(intel_dp))
return;
- pps_lock(intel_dp);
+ with_pps_lock(intel_dp, wakeref) {
+ i915_reg_t pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
+ u32 pp;
- pp = ironlake_get_pp_control(intel_dp);
- pp &= ~EDP_BLC_ENABLE;
-
- pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
-
- I915_WRITE(pp_ctrl_reg, pp);
- POSTING_READ(pp_ctrl_reg);
+ pp = ironlake_get_pp_control(intel_dp);
+ pp &= ~EDP_BLC_ENABLE;
- pps_unlock(intel_dp);
+ I915_WRITE(pp_ctrl_reg, pp);
+ POSTING_READ(pp_ctrl_reg);
+ }
intel_dp->last_backlight_off = jiffies;
edp_wait_backlight_off(intel_dp);
@@ -2642,12 +2650,12 @@ static void intel_edp_backlight_power(struct intel_connector *connector,
bool enable)
{
struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
+ intel_wakeref_t wakeref;
bool is_enabled;
- pps_lock(intel_dp);
- is_enabled = ironlake_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
- pps_unlock(intel_dp);
-
+ is_enabled = false;
+ with_pps_lock(intel_dp, wakeref)
+ is_enabled = ironlake_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
if (is_enabled == enable)
return;
@@ -3141,22 +3149,21 @@ static void intel_enable_dp(struct intel_encoder *encoder,
struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
uint32_t dp_reg = I915_READ(intel_dp->output_reg);
enum pipe pipe = crtc->pipe;
+ intel_wakeref_t wakeref;
if (WARN_ON(dp_reg & DP_PORT_EN))
return;
- pps_lock(intel_dp);
-
- if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- vlv_init_panel_power_sequencer(encoder, pipe_config);
-
- intel_dp_enable_port(intel_dp, pipe_config);
+ with_pps_lock(intel_dp, wakeref) {
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ vlv_init_panel_power_sequencer(encoder, pipe_config);
- edp_panel_vdd_on(intel_dp);
- edp_panel_on(intel_dp);
- edp_panel_vdd_off(intel_dp, true);
+ intel_dp_enable_port(intel_dp, pipe_config);
- pps_unlock(intel_dp);
+ edp_panel_vdd_on(intel_dp);
+ edp_panel_on(intel_dp);
+ edp_panel_vdd_off(intel_dp, true);
+ }
if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
unsigned int lane_mask = 0x0;
@@ -3854,9 +3861,10 @@ intel_dp_link_down(struct intel_encoder *encoder,
intel_dp->DP = DP;
if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
- pps_lock(intel_dp);
- intel_dp->active_pipe = INVALID_PIPE;
- pps_unlock(intel_dp);
+ intel_wakeref_t wakeref;
+
+ with_pps_lock(intel_dp, wakeref)
+ intel_dp->active_pipe = INVALID_PIPE;
}
}
@@ -5272,14 +5280,15 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
intel_dp_mst_encoder_cleanup(intel_dig_port);
if (intel_dp_is_edp(intel_dp)) {
+ intel_wakeref_t wakeref;
+
cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
/*
* vdd might still be enabled do to the delayed vdd off.
* Make sure vdd is actually turned off here.
*/
- pps_lock(intel_dp);
- edp_panel_vdd_off_sync(intel_dp);
- pps_unlock(intel_dp);
+ with_pps_lock(intel_dp, wakeref)
+ edp_panel_vdd_off_sync(intel_dp);
if (intel_dp->edp_notifier.notifier_call) {
unregister_reboot_notifier(&intel_dp->edp_notifier);
@@ -5296,6 +5305,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
+ intel_wakeref_t wakeref;
if (!intel_dp_is_edp(intel_dp))
return;
@@ -5305,9 +5315,8 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
* Make sure vdd is actually turned off here.
*/
cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
- pps_lock(intel_dp);
- edp_panel_vdd_off_sync(intel_dp);
- pps_unlock(intel_dp);
+ with_pps_lock(intel_dp, wakeref)
+ edp_panel_vdd_off_sync(intel_dp);
}
static
@@ -5581,6 +5590,7 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder)
struct drm_i915_private *dev_priv = to_i915(encoder->dev);
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
+ intel_wakeref_t wakeref;
if (!HAS_DDI(dev_priv))
intel_dp->DP = I915_READ(intel_dp->output_reg);
@@ -5590,18 +5600,19 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder)
intel_dp->reset_link_params = true;
- pps_lock(intel_dp);
-
- if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- intel_dp->active_pipe = vlv_active_pipe(intel_dp);
+ with_pps_lock(intel_dp, wakeref) {
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ intel_dp->active_pipe = vlv_active_pipe(intel_dp);
- if (intel_dp_is_edp(intel_dp)) {
- /* Reinit the power sequencer, in case BIOS did something with it. */
- intel_dp_pps_init(intel_dp);
- intel_edp_panel_vdd_sanitize(intel_dp);
+ if (intel_dp_is_edp(intel_dp)) {
+ /*
+ * Reinit the power sequencer, in case BIOS did
+ * something nasty with it.
+ */
+ intel_dp_pps_init(intel_dp);
+ intel_edp_panel_vdd_sanitize(intel_dp);
+ }
}
-
- pps_unlock(intel_dp);
}
static const struct drm_connector_funcs intel_dp_connector_funcs = {
@@ -6397,8 +6408,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
struct drm_display_mode *downclock_mode = NULL;
bool has_dpcd;
struct drm_display_mode *scan;
- struct edid *edid;
enum pipe pipe = INVALID_PIPE;
+ intel_wakeref_t wakeref;
+ struct edid *edid;
if (!intel_dp_is_edp(intel_dp))
return true;
@@ -6416,13 +6428,11 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
return false;
}
- pps_lock(intel_dp);
-
- intel_dp_init_panel_power_timestamps(intel_dp);
- intel_dp_pps_init(intel_dp);
- intel_edp_panel_vdd_sanitize(intel_dp);
-
- pps_unlock(intel_dp);
+ with_pps_lock(intel_dp, wakeref) {
+ intel_dp_init_panel_power_timestamps(intel_dp);
+ intel_dp_pps_init(intel_dp);
+ intel_edp_panel_vdd_sanitize(intel_dp);
+ }
/* Cache DPCD and EDID for edp. */
has_dpcd = intel_edp_init_dpcd(intel_dp);
@@ -6503,9 +6513,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
* vdd might still be enabled do to the delayed vdd off.
* Make sure vdd is actually turned off here.
*/
- pps_lock(intel_dp);
- edp_panel_vdd_off_sync(intel_dp);
- pps_unlock(intel_dp);
+ with_pps_lock(intel_dp, wakeref)
+ edp_panel_vdd_off_sync(intel_dp);
return false;
}
--
2.19.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-09-04 21:59 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-04 21:57 [PATCH 01/23] drm/i915: Reduce context HW ID lifetime Chris Wilson
2018-09-04 21:57 ` [PATCH 02/23] drm/i915/execlists: Avoid kicking priority on the current context Chris Wilson
2018-09-04 21:57 ` [PATCH 03/23] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
2018-09-04 21:57 ` [PATCH 04/23] drm/i915/selftests: Basic stress test for rapid context switching Chris Wilson
2018-09-04 21:57 ` [PATCH 05/23] drm/i915/execlists: Delay updating ring register state after resume Chris Wilson
2018-09-04 21:57 ` [PATCH 06/23] drm/i915/execlists: Use coherent writes into the context image Chris Wilson
2018-09-04 21:57 ` [PATCH 07/23] drm/i915/execlists: Onion unwind for logical_ring_init() failure Chris Wilson
2018-09-04 21:57 ` [PATCH 08/23] drm/i915: Report the number of closed vma held by each context in debugfs Chris Wilson
2018-09-04 21:57 ` [PATCH 09/23] drm/i915: Remove debugfs/i915_ppgtt_info Chris Wilson
2018-09-04 21:57 ` [PATCH 10/23] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
2018-09-04 21:57 ` [PATCH 11/23] drm/i915: Handle incomplete Z_FINISH for compressed error states Chris Wilson
2018-09-04 21:57 ` [PATCH 12/23] drm/i915: Clear the error PTE just once on finish Chris Wilson
2018-09-04 21:57 ` [PATCH 13/23] drm/i915: Cache the error string Chris Wilson
2018-09-04 21:57 ` [PATCH 14/23] drm/i915: Attach the pci match data to the device upon creation Chris Wilson
2018-09-05 13:36 ` Michal Wajdeczko
2018-09-05 13:58 ` Chris Wilson
2018-09-04 21:57 ` [PATCH 15/23] drm/i915: Track all held rpm wakerefs Chris Wilson
2018-09-04 21:57 ` [PATCH 16/23] drm/i915: Markup paired operations on wakerefs Chris Wilson
2018-09-04 21:57 ` [PATCH 17/23] drm/i915: Syntatic sugar for using intel_runtime_pm Chris Wilson
2018-09-04 21:57 ` [PATCH 18/23] drm/i915: Markup paired operations on display power domains Chris Wilson
2018-09-04 21:57 ` [PATCH 19/23] drm/i915: Track the wakeref used to initialise " Chris Wilson
2018-09-04 21:57 ` Chris Wilson [this message]
2018-09-04 21:57 ` [PATCH 21/23] drm/i915: Complain if hsw_get_pipe_config acquires the same power well twice Chris Wilson
2018-09-04 21:57 ` [PATCH 22/23] drm/i915: Mark up Ironlake ips with rpm wakerefs Chris Wilson
2018-09-04 21:57 ` [PATCH 23/23] drm/i915: Serialise concurrent calls to i915_gem_set_wedged() Chris Wilson
2018-09-04 22:10 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/23] drm/i915: Reduce context HW ID lifetime Patchwork
2018-09-04 22:17 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-09-04 22:30 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-05 5:42 ` ✓ Fi.CI.IGT: " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2018-08-16 21:07 [PATCH 01/23] drm/i915: Cache the error string Chris Wilson
2018-08-16 21:07 ` [PATCH 20/23] drm/i915/dp: Markup pps lock power well Chris Wilson
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=20180904215742.20276-20-chris@chris-wilson.co.uk \
--to=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.