From 14667a4bde4361b7ac420d68a2e9e9b9b2df5231 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 3 Apr 2012 17:58:35 +0100 Subject: drm/i915: Finish any pending operations on the framebuffer before disabling Similar to the case where we are changing from one framebuffer to another, we need to be sure that there are no pending WAIT_FOR_EVENTs on the pipe for the current framebuffer before switching. If we disable the pipe, and then try to execute a WAIT_FOR_EVENT it will block indefinitely and cause a GPU hang. We attempted to fix this in commit 85345517fe6d4de27b0d6ca19fef9d28ac947c4a (drm/i915: Retire any pending operations on the old scanout when switching) for the case of mode switching, but this leaves the condition where we are switching off the pipe vulnerable. There still remains the race condition were a display may be unplugged, switched off by the core, a uevent sent to notify the DDX and the DDX may issue a WAIT_FOR_EVENT before it processes the uevent. This window does not exist if the pipe is only switched off in response to the uevent. Time to make sure that is so... Reported-by: Francis Leblanc Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36515 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45413 Signed-off-by: Chris Wilson Reviewed-by: Eugeni Dodonov [danvet: fixup spelling in comment, noticed by Eugeni.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 65 +++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 19 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_display.c') diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 91b35fd1db8..f446e66cbda 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2244,6 +2244,33 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb, return 0; } +static int +intel_finish_fb(struct drm_framebuffer *old_fb) +{ + struct drm_i915_gem_object *obj = to_intel_framebuffer(old_fb)->obj; + struct drm_i915_private *dev_priv = obj->base.dev->dev_private; + bool was_interruptible = dev_priv->mm.interruptible; + int ret; + + wait_event(dev_priv->pending_flip_queue, + atomic_read(&dev_priv->mm.wedged) || + atomic_read(&obj->pending_flip) == 0); + + /* Big Hammer, we also need to ensure that any pending + * MI_WAIT_FOR_EVENT inside a user batch buffer on the + * current scanout is retired before unpinning the old + * framebuffer. + * + * This should only fail upon a hung GPU, in which case we + * can safely continue. + */ + dev_priv->mm.interruptible = false; + ret = i915_gem_object_finish_gpu(obj); + dev_priv->mm.interruptible = was_interruptible; + + return ret; +} + static int intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb) @@ -2282,25 +2309,8 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, return ret; } - if (old_fb) { - struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_i915_gem_object *obj = to_intel_framebuffer(old_fb)->obj; - - wait_event(dev_priv->pending_flip_queue, - atomic_read(&dev_priv->mm.wedged) || - atomic_read(&obj->pending_flip) == 0); - - /* Big Hammer, we also need to ensure that any pending - * MI_WAIT_FOR_EVENT inside a user batch buffer on the - * current scanout is retired before unpinning the old - * framebuffer. - * - * This should only fail upon a hung GPU, in which case we - * can safely continue. - */ - ret = i915_gem_object_finish_gpu(obj); - (void) ret; - } + if (old_fb) + intel_finish_fb(old_fb); ret = intel_pipe_set_base_atomic(crtc, crtc->fb, x, y, LEAVE_ATOMIC_MODE_SET); @@ -3371,6 +3381,23 @@ static void intel_crtc_disable(struct drm_crtc *crtc) struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; struct drm_device *dev = crtc->dev; + /* Flush any pending WAITs before we disable the pipe. Note that + * we need to drop the struct_mutex in order to acquire it again + * during the lowlevel dpms routines around a couple of the + * operations. It does not look trivial nor desirable to move + * that locking higher. So instead we leave a window for the + * submission of further commands on the fb before we can actually + * disable it. This race with userspace exists anyway, and we can + * only rely on the pipe being disabled by userspace after it + * receives the hotplug notification and has flushed any pending + * batches. + */ + if (crtc->fb) { + mutex_lock(&dev->struct_mutex); + intel_finish_fb(crtc->fb); + mutex_unlock(&dev->struct_mutex); + } + crtc_funcs->dpms(crtc, DRM_MODE_DPMS_OFF); assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane); assert_pipe_disabled(dev->dev_private, to_intel_crtc(crtc)->pipe); -- cgit v1.2.3-70-g09d2 From 80e829fade4eea5f07c410df6a551c42e2d0ca9c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 31 Mar 2012 11:21:57 +0200 Subject: drm/i915: implement ColorBlt w/a According to an internal workaround master list, we need to set bit 5 of register 9400 to avoid issues with color blits. Testing shows that this seems to fix the blitter hangs when fbc is enabled on snb, thanks to Chris Wilson for figuring this out. Tested-by: Chris Wilson Tested-by: Michael "brot" Groh Acked-by: Ben Widawsky Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 4 ++++ 2 files changed, 7 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_display.c') diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2abf4eb9403..b4bb1ef77dd 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3728,6 +3728,9 @@ #define GT_FIFO_FREE_ENTRIES 0x120008 #define GT_FIFO_NUM_RESERVED_ENTRIES 20 +#define GEN6_UCGCTL1 0x9400 +# define GEN6_BLBUNIT_CLOCK_GATE_DISABLE (1 << 5) + #define GEN6_UCGCTL2 0x9404 # define GEN6_RCZUNIT_CLOCK_GATE_DISABLE (1 << 13) # define GEN6_RCPBUNIT_CLOCK_GATE_DISABLE (1 << 12) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f446e66cbda..bae38acf44d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8556,6 +8556,10 @@ static void gen6_init_clock_gating(struct drm_device *dev) I915_WRITE(WM2_LP_ILK, 0); I915_WRITE(WM1_LP_ILK, 0); + I915_WRITE(GEN6_UCGCTL1, + I915_READ(GEN6_UCGCTL1) | + GEN6_BLBUNIT_CLOCK_GATE_DISABLE); + /* According to the BSpec vol1g, bit 12 (RCPBUNIT) clock * gating disable must be set. Failure to set it results in * flickering pixels due to Z write ordering failures after -- cgit v1.2.3-70-g09d2 From f9bef081c3c3f77bec54454872e98d3ec635756f Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 15 Apr 2012 19:53:19 +0200 Subject: drm/i915: don't clobber the special upscaling lvds timings This regression has been introduced in commit ca9bfa7eed20ea34e862804e62aae10eb159edbb Author: Daniel Vetter Date: Sat Jan 28 14:49:20 2012 +0100 drm/i915: fixup interlaced vertical timings confusion, part 1 Unfortunately that commit failed to take into account that the lvds code does some special adjustements to the crtc timings for upscaling an centering. Fix this by explicitly computing crtc timings in the lvds mode fixup function and setting a special flag in mode->private_flags if the crtc timings have been adjusted. v2: Add a comment to explain the new mode driver private flag, suggested by Eugeni Dodonov. v3: Kill the confusing and now redundant set_crtcinfo call in intel_fixed_panel_mode, noticed by Chris Wilson. Reported-and-Tested-by: Hans de Bruin Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43071 Reviewed-by: Eugeni Dodonov Reviewed-by: Chris Wilson Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 7 +++++-- drivers/gpu/drm/i915/intel_drv.h | 4 ++++ drivers/gpu/drm/i915/intel_lvds.c | 6 ++++++ drivers/gpu/drm/i915/intel_panel.c | 2 -- 4 files changed, 15 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_display.c') diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bae38acf44d..8be30917bce 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3478,8 +3478,11 @@ static bool intel_crtc_mode_fixup(struct drm_crtc *crtc, return false; } - /* All interlaced capable intel hw wants timings in frames. */ - drm_mode_set_crtcinfo(adjusted_mode, 0); + /* All interlaced capable intel hw wants timings in frames. Note though + * that intel_lvds_mode_fixup does some funny tricks with the crtc + * timings, so we need to be careful not to clobber these.*/ + if (!(adjusted_mode->private_flags & INTEL_MODE_CRTC_TIMINGS_SET)) + drm_mode_set_crtcinfo(adjusted_mode, 0); return true; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5a14149b379..715afa15302 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -105,6 +105,10 @@ #define INTEL_MODE_PIXEL_MULTIPLIER_SHIFT (0x0) #define INTEL_MODE_PIXEL_MULTIPLIER_MASK (0xf << INTEL_MODE_PIXEL_MULTIPLIER_SHIFT) #define INTEL_MODE_DP_FORCE_6BPC (0x10) +/* This flag must be set by the encoder's mode_fixup if it changes the crtc + * timings in the mode to prevent the crtc fixup from overwriting them. + * Currently only lvds needs that. */ +#define INTEL_MODE_CRTC_TIMINGS_SET (0x20) static inline void intel_mode_set_pixel_multiplier(struct drm_display_mode *mode, diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 95db2e98822..30e2c82101d 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -187,6 +187,8 @@ centre_horizontally(struct drm_display_mode *mode, mode->crtc_hsync_start = mode->crtc_hblank_start + sync_pos; mode->crtc_hsync_end = mode->crtc_hsync_start + sync_width; + + mode->private_flags |= INTEL_MODE_CRTC_TIMINGS_SET; } static void @@ -208,6 +210,8 @@ centre_vertically(struct drm_display_mode *mode, mode->crtc_vsync_start = mode->crtc_vblank_start + sync_pos; mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width; + + mode->private_flags |= INTEL_MODE_CRTC_TIMINGS_SET; } static inline u32 panel_fitter_scaling(u32 source, u32 target) @@ -283,6 +287,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder, for_each_pipe(pipe) I915_WRITE(BCLRPAT(pipe), 0); + drm_mode_set_crtcinfo(adjusted_mode, 0); + switch (intel_lvds->fitting_mode) { case DRM_MODE_SCALE_CENTER: /* diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 230a141dbea..48177ec4720 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -47,8 +47,6 @@ intel_fixed_panel_mode(struct drm_display_mode *fixed_mode, adjusted_mode->vtotal = fixed_mode->vtotal; adjusted_mode->clock = fixed_mode->clock; - - drm_mode_set_crtcinfo(adjusted_mode, 0); } /* adjusted_mode has been preset to be the panel's fixed mode */ -- cgit v1.2.3-70-g09d2 From 99d9acdd0c5ce970eaf8e8671c0bc4cb7c6fb0c1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 17 Apr 2012 20:37:00 +0100 Subject: drm/i915: Do not set "Enable Panel Fitter" on SNB pageflips Not only do the pageflip work without it at non-native modes (i.e. with the panel fitter enabled), it also causes normal (non-pageflipped) modesets to fail. Reported-by: Adam Jackson Tested-by: Adam Jackson Signed-off-by: Chris Wilson Wanted-by-for-fixes: Dave Airlie Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/intel_display.c') diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8be30917bce..5908cd56340 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7468,7 +7468,13 @@ static int intel_gen6_queue_flip(struct drm_device *dev, OUT_RING(fb->pitches[0] | obj->tiling_mode); OUT_RING(obj->gtt_offset); - pf = I915_READ(PF_CTL(intel_crtc->pipe)) & PF_ENABLE; + /* Contrary to the suggestions in the documentation, + * "Enable Panel Fitter" does not seem to be required when page + * flipping with a non-native mode, and worse causes a normal + * modeset to fail. + * pf = I915_READ(PF_CTL(intel_crtc->pipe)) & PF_ENABLE; + */ + pf = 0; pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff; OUT_RING(pf | pipesrc); ADVANCE_LP_RING(); -- cgit v1.2.3-70-g09d2 From 074b5e1a99fb5017122591d70098601e0484ca6a Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 2 May 2012 12:07:06 +0100 Subject: drm/i915: Do not read non-existent DPLL registers on PCH hardware We only execute intel_decrease_pllclock for pre-PCH hardware, typically gen4 mobiles. However, in the variable declaration we did read from the non-PCH DPLL register, quite naughty and detected by SandyBridge. Reported-and-tested-by: Andrey Rahmatullin Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49025 Signed-off-by: Chris Wilson Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_display.c') diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5908cd56340..1b1cf3b3ff5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7072,9 +7072,6 @@ static void intel_decrease_pllclock(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; drm_i915_private_t *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc->pipe; - int dpll_reg = DPLL(pipe); - int dpll = I915_READ(dpll_reg); if (HAS_PCH_SPLIT(dev)) return; @@ -7087,10 +7084,15 @@ static void intel_decrease_pllclock(struct drm_crtc *crtc) * the manual case. */ if (!HAS_PIPE_CXSR(dev) && intel_crtc->lowfreq_avail) { + int pipe = intel_crtc->pipe; + int dpll_reg = DPLL(pipe); + u32 dpll; + DRM_DEBUG_DRIVER("downclocking LVDS\n"); assert_panel_unlocked(dev_priv, pipe); + dpll = I915_READ(dpll_reg); dpll |= DISPLAY_RATE_SELECT_FPA1; I915_WRITE(dpll_reg, dpll); intel_wait_for_vblank(dev, pipe); @@ -7098,7 +7100,6 @@ static void intel_decrease_pllclock(struct drm_crtc *crtc) if (!(dpll & DISPLAY_RATE_SELECT_FPA1)) DRM_DEBUG_DRIVER("failed to downclock LVDS!\n"); } - } /** -- cgit v1.2.3-70-g09d2