From 0f3b6849dd55943d915f4b461d7db5e8308d4083 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 15 Jan 2013 12:05:55 +0000 Subject: [PATCH 1/4] drm/i915: Record DERRMR, FORCEWAKE and RING_CTL in error-state These are useful for investigating hangs involving WAIT_FOR_EVENT. Signed-off-by: Chris Wilson [danvet: Apply a droplet of Future-Proof in the if-ladder.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 3 +++ drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_irq.c | 11 +++++++++++ drivers/gpu/drm/i915/i915_reg.h | 2 ++ 4 files changed, 19 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e6a11ca85ea..7944d301518 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -641,6 +641,7 @@ static void i915_ring_error_state(struct seq_file *m, seq_printf(m, "%s command stream:\n", ring_str(ring)); seq_printf(m, " HEAD: 0x%08x\n", error->head[ring]); seq_printf(m, " TAIL: 0x%08x\n", error->tail[ring]); + seq_printf(m, " CTL: 0x%08x\n", error->ctl[ring]); seq_printf(m, " ACTHD: 0x%08x\n", error->acthd[ring]); seq_printf(m, " IPEIR: 0x%08x\n", error->ipeir[ring]); seq_printf(m, " IPEHR: 0x%08x\n", error->ipehr[ring]); @@ -693,6 +694,8 @@ static int i915_error_state(struct seq_file *m, void *unused) seq_printf(m, "EIR: 0x%08x\n", error->eir); seq_printf(m, "IER: 0x%08x\n", error->ier); seq_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er); + seq_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake); + seq_printf(m, "DERRMR: 0x%08x\n", error->derrmr); seq_printf(m, "CCID: 0x%08x\n", error->ccid); for (i = 0; i < dev_priv->num_fence_regs; i++) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ed305957557..12ab3bdea54 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -188,10 +188,13 @@ struct drm_i915_error_state { u32 pgtbl_er; u32 ier; u32 ccid; + u32 derrmr; + u32 forcewake; bool waiting[I915_NUM_RINGS]; u32 pipestat[I915_MAX_PIPES]; u32 tail[I915_NUM_RINGS]; u32 head[I915_NUM_RINGS]; + u32 ctl[I915_NUM_RINGS]; u32 ipeir[I915_NUM_RINGS]; u32 ipehr[I915_NUM_RINGS]; u32 instdone[I915_NUM_RINGS]; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2220dec3e5d..fe843389c7b 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1157,6 +1157,7 @@ static void i915_record_ring_state(struct drm_device *dev, error->acthd[ring->id] = intel_ring_get_active_head(ring); error->head[ring->id] = I915_READ_HEAD(ring); error->tail[ring->id] = I915_READ_TAIL(ring); + error->ctl[ring->id] = I915_READ_CTL(ring); error->cpu_ring_head[ring->id] = ring->head; error->cpu_ring_tail[ring->id] = ring->tail; @@ -1251,6 +1252,16 @@ static void i915_capture_error_state(struct drm_device *dev) else error->ier = I915_READ(IER); + if (INTEL_INFO(dev)->gen >= 6) + error->derrmr = I915_READ(DERRMR); + + if (IS_VALLEYVIEW(dev)) + error->forcewake = I915_READ(FORCEWAKE_VLV); + else if (INTEL_INFO(dev)->gen >= 7) + error->forcewake = I915_READ(FORCEWAKE_MT); + else if (INTEL_INFO(dev)->gen == 6) + error->forcewake = I915_READ(FORCEWAKE); + for_each_pipe(pipe) error->pipestat[pipe] = I915_READ(PIPESTAT(pipe)); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 186ee5c85b5..b401788e179 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -512,6 +512,8 @@ #define GEN7_ERR_INT 0x44040 #define ERR_INT_MMIO_UNCLAIMED (1<<13) +#define DERRMR 0x44050 + /* GM45+ chicken bits -- debug workaround bits that may be required * for various sorts of correct behavior. The top 16 bits of each are * the enables for writing to the corresponding low bit. From f30d26e468322b50d5e376bec40658683aff8ece Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Wed, 16 Jan 2013 10:53:40 +0200 Subject: [PATCH 2/4] drm/i915/eDP: do not write power sequence registers for ghost eDP Some machines detect an eDP port even if it's not really there, and eDP initialization has a fail path for this. Typically such machines have an LVDS display instead. A regression introduced in commit 82ed61fa1a4e08d5f9e86fb1b715b50ed678b6ac Author: Daniel Vetter Date: Sat Oct 20 20:57:41 2012 +0200 drm/i915: make edp panel power sequence setup more robust updated the power sequence registers PCH_PP_ON_DELAYS, PCH_PP_OFF_DELAYS, and PCH_PP_DIVISOR also in the ghost eDP case, messing up the LVDS display. Split the power sequencer initialization into two, delaying the register updates until after we know the eDP is real. Note: Keep the PP_CONTROL unlocking in the first part, even if it does not update registers, per the commit message of the above mentioned commit. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52601 Reported-and-tested-by: Ryan Coe Signed-off-by: Jani Nikula Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_dp.c | 47 ++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1b63d55318a..fb3715b4b09 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2579,7 +2579,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect static void intel_dp_init_panel_power_sequencer(struct drm_device *dev, - struct intel_dp *intel_dp) + struct intel_dp *intel_dp, + struct edp_power_seq *out) { struct drm_i915_private *dev_priv = dev->dev_private; struct edp_power_seq cur, vbt, spec, final; @@ -2650,16 +2651,35 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, intel_dp->panel_power_cycle_delay = get_delay(t11_t12); #undef get_delay + DRM_DEBUG_KMS("panel power up delay %d, power down delay %d, power cycle delay %d\n", + intel_dp->panel_power_up_delay, intel_dp->panel_power_down_delay, + intel_dp->panel_power_cycle_delay); + + DRM_DEBUG_KMS("backlight on delay %d, off delay %d\n", + intel_dp->backlight_on_delay, intel_dp->backlight_off_delay); + + if (out) + *out = final; +} + +static void +intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, + struct intel_dp *intel_dp, + struct edp_power_seq *seq) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + u32 pp_on, pp_off, pp_div; + /* And finally store the new values in the power sequencer. */ - pp_on = (final.t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | - (final.t8 << PANEL_LIGHT_ON_DELAY_SHIFT); - pp_off = (final.t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | - (final.t10 << PANEL_POWER_DOWN_DELAY_SHIFT); + pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | + (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); + pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | + (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT); /* Compute the divisor for the pp clock, simply match the Bspec * formula. */ pp_div = ((100 * intel_pch_rawclk(dev))/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT; - pp_div |= (DIV_ROUND_UP(final.t11_t12, 1000) + pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000) << PANEL_POWER_CYCLE_DELAY_SHIFT); /* Haswell doesn't have any port selection bits for the panel @@ -2675,14 +2695,6 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, I915_WRITE(PCH_PP_OFF_DELAYS, pp_off); I915_WRITE(PCH_PP_DIVISOR, pp_div); - - DRM_DEBUG_KMS("panel power up delay %d, power down delay %d, power cycle delay %d\n", - intel_dp->panel_power_up_delay, intel_dp->panel_power_down_delay, - intel_dp->panel_power_cycle_delay); - - DRM_DEBUG_KMS("backlight on delay %d, off delay %d\n", - intel_dp->backlight_on_delay, intel_dp->backlight_off_delay); - DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n", I915_READ(PCH_PP_ON_DELAYS), I915_READ(PCH_PP_OFF_DELAYS), @@ -2699,6 +2711,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, struct drm_device *dev = intel_encoder->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct drm_display_mode *fixed_mode = NULL; + struct edp_power_seq power_seq = { 0 }; enum port port = intel_dig_port->port; const char *name = NULL; int type; @@ -2771,7 +2784,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, } if (is_edp(intel_dp)) - intel_dp_init_panel_power_sequencer(dev, intel_dp); + intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); intel_dp_i2c_init(intel_dp, intel_connector, name); @@ -2798,6 +2811,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, return; } + /* We now know it's not a ghost, init power sequence regs. */ + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, + &power_seq); + ironlake_edp_panel_vdd_on(intel_dp); edid = drm_get_edid(connector, &intel_dp->adapter); if (edid) { From 262b6d363fcff16359c93bd58c297f961f6e6273 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 15 Jan 2013 16:17:54 +0000 Subject: [PATCH 3/4] drm/i915: Invalidate the relocation presumed_offsets along the slow path In the slow path, we are forced to copy the relocations prior to acquiring the struct mutex in order to handle pagefaults. We forgo copying the new offsets back into the relocation entries in order to prevent a recursive locking bug should we trigger a pagefault whilst holding the mutex for the reservations of the execbuffer. Therefore, we need to reset the presumed_offsets just in case the objects are rebound back into their old locations after relocating for this exexbuffer - if that were to happen we would assume the relocations were valid and leave the actual pointers to the kernels dangling, instant hang. Fixes regression from commit bcf50e2775bbc3101932d8e4ab8c7902aa4163b4 Author: Chris Wilson Date: Sun Nov 21 22:07:12 2010 +0000 drm/i915: Handle pagefaults in execbuffer user relocations Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55984 Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index d6a994a0739..26d08bb5821 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -539,6 +539,8 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, total = 0; for (i = 0; i < count; i++) { struct drm_i915_gem_relocation_entry __user *user_relocs; + u64 invalid_offset = (u64)-1; + int j; user_relocs = (void __user *)(uintptr_t)exec[i].relocs_ptr; @@ -549,6 +551,25 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, goto err; } + /* As we do not update the known relocation offsets after + * relocating (due to the complexities in lock handling), + * we need to mark them as invalid now so that we force the + * relocation processing next time. Just in case the target + * object is evicted and then rebound into its old + * presumed_offset before the next execbuffer - if that + * happened we would make the mistake of assuming that the + * relocations were valid. + */ + for (j = 0; j < exec[i].relocation_count; j++) { + if (copy_to_user(&user_relocs[j].presumed_offset, + &invalid_offset, + sizeof(invalid_offset))) { + ret = -EFAULT; + mutex_lock(&dev->struct_mutex); + goto err; + } + } + reloc_offset[i] = total; total += exec[i].relocation_count; } From b514407547890686572606c9dfa4b7f832db9958 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Thu, 17 Jan 2013 10:24:09 +0200 Subject: [PATCH 4/4] drm/i915: fix FORCEWAKE posting reads We stopped reading FORCEWAKE for posting reads in commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd Author: Ben Widawsky Date: Sat Sep 1 22:59:50 2012 -0700 drm/i915: Never read FORCEWAKE and started using something from the same cacheline instead. On the bug reporter's machine this broke entering rc6 states after a suspend/resume cycle. It turns out reading ECOBUS as posting read worked fine, while GTFIFODBG did not, preventing RC6 states after suspend/resume per the bug report referenced below. It's not entirely clear why, but clearly GTFIFODBG was nowhere near the same cacheline or address range as FORCEWAKE. Trying out various registers for posting reads showed that all tested registers for which NEEDS_FORCE_WAKE() (in i915_drv.c) returns true work. Conversely, most (but not quite all) registers for which NEEDS_FORCE_WAKE() returns false do not work. Details in the referenced bug. Based on the above, add posting reads on ECOBUS where GTFIFODBG was previously relied on. In true cargo cult spirit, add posting reads for FORCEWAKE_VLV writes as well, but instead of ECOBUS, use FORCEWAKE_ACK_VLV which is in the same address range as FORCEWAKE_VLV. v2: Add more details to the commit message. No functional changes. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52411 Reported-and-tested-by: Alexander Bersenev CC: Ben Widawsky Signed-off-by: Jani Nikula Reviewed-by: Chris Wilson Cc: stable@vger.kernel.org [danvet: add cc: stable and make the commit message a bit clearer that this is a regression fix and what exactly broke.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e83a1179417..3280cffe50f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4250,7 +4250,8 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv) static void __gen6_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(0xffff)); - POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */ + /* something from same cacheline, but !FORCEWAKE_MT */ + POSTING_READ(ECOBUS); } static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv) @@ -4267,7 +4268,8 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv) DRM_ERROR("Timed out waiting for forcewake old ack to clear.\n"); I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL)); - POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */ + /* something from same cacheline, but !FORCEWAKE_MT */ + POSTING_READ(ECOBUS); if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1), FORCEWAKE_ACK_TIMEOUT_MS)) @@ -4304,14 +4306,16 @@ void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv) static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE, 0); - /* gen6_gt_check_fifodbg doubles as the POSTING_READ */ + /* something from same cacheline, but !FORCEWAKE */ + POSTING_READ(ECOBUS); gen6_gt_check_fifodbg(dev_priv); } static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL)); - /* gen6_gt_check_fifodbg doubles as the POSTING_READ */ + /* something from same cacheline, but !FORCEWAKE_MT */ + POSTING_READ(ECOBUS); gen6_gt_check_fifodbg(dev_priv); } @@ -4351,6 +4355,8 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) static void vlv_force_wake_reset(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(0xffff)); + /* something from same cacheline, but !FORCEWAKE_VLV */ + POSTING_READ(FORCEWAKE_ACK_VLV); } static void vlv_force_wake_get(struct drm_i915_private *dev_priv) @@ -4371,7 +4377,8 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv) static void vlv_force_wake_put(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL)); - /* The below doubles as a POSTING_READ */ + /* something from same cacheline, but !FORCEWAKE_VLV */ + POSTING_READ(FORCEWAKE_ACK_VLV); gen6_gt_check_fifodbg(dev_priv); }