All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3.2] drm/i915 patches for 3.2
@ 2012-01-31 15:51 Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 1/7] drm/i915: HDMI hot remove notification to audio driver Eugeni Dodonov
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Eugeni Dodonov @ 2012-01-31 15:51 UTC (permalink / raw)
  To: stable; +Cc: intel-gfx, Eugeni Dodonov

Hi Greg,

those are some patches related to i915 module for inclusion in the 3.2
series.

The force_wake-related ones are particularly interesting as the 'drm/i915:
paper over missed irq issues with force wake voodoo' patch was queued for this
kernel. They prevent WARNs and possible hangs on Sandy Bridge / Ivy Bridge.

Thanks,
Eugeni

Daniel Vetter (3):
  drm/i915: check ACTHD of all rings
  drm/i915: convert force_wake_get to func pointer in the gpu reset
    code
  drm/i915: protect force_wake_(get|put) with the gt_lock

Eugeni Dodonov (1):
  drm/i915: handle IVB 3rd pipe during suspend

Rodrigo Vivi (1):
  drm/i915: Fix TV Out refresh rate.

Wu Fengguang (2):
  drm/i915: HDMI hot remove notification to audio driver
  drm/i915: DisplayPort hot remove notification to audio driver

 drivers/gpu/drm/i915/i915_debugfs.c |    8 ++++++--
 drivers/gpu/drm/i915/i915_dma.c     |    1 +
 drivers/gpu/drm/i915/i915_drv.c     |   20 +++++++++++++-------
 drivers/gpu/drm/i915/i915_drv.h     |   12 +++++++++---
 drivers/gpu/drm/i915/i915_irq.c     |   13 ++++++++++---
 drivers/gpu/drm/i915/i915_suspend.c |    4 ++++
 drivers/gpu/drm/i915/intel_dp.c     |    1 +
 drivers/gpu/drm/i915/intel_hdmi.c   |    8 ++++++--
 drivers/gpu/drm/i915/intel_tv.c     |   16 ++++++++--------
 9 files changed, 58 insertions(+), 25 deletions(-)

-- 
1.7.8.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/7] drm/i915: HDMI hot remove notification to audio driver
  2012-01-31 15:51 [PATCH 3.2] drm/i915 patches for 3.2 Eugeni Dodonov
@ 2012-01-31 15:51 ` Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 2/7] drm/i915: DisplayPort " Eugeni Dodonov
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Eugeni Dodonov @ 2012-01-31 15:51 UTC (permalink / raw)
  To: stable; +Cc: intel-gfx, Wu Fengguang, Wang Zhenyu, Eugeni Dodonov

From: Wu Fengguang <fengguang.wu@intel.com>

On HDMI monitor hot remove, clear SDVO_AUDIO_ENABLE accordingly, so that
the audio driver will receive hot plug events and take action to refresh
its device state and ELD contents.

The cleared SDVO_AUDIO_ENABLE bit needs to be restored to prevent losing
HDMI audio after DPMS on.

Upstream patch:
commit 2deed761188d7480eb5f7efbfe7aa77f09322ed8
Author: Wu Fengguang <fengguang.wu@intel.com>
Date:   Fri Dec 9 20:42:20 2011 +0800

    drm/i915: HDMI hot remove notification to audio driver

CC: Wang Zhenyu <zhenyu.z.wang@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index d4f5a0b..64541f7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -269,6 +269,10 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	u32 temp;
+	u32 enable_bits = SDVO_ENABLE;
+
+	if (intel_hdmi->has_audio)
+		enable_bits |= SDVO_AUDIO_ENABLE;
 
 	temp = I915_READ(intel_hdmi->sdvox_reg);
 
@@ -281,9 +285,9 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode)
 	}
 
 	if (mode != DRM_MODE_DPMS_ON) {
-		temp &= ~SDVO_ENABLE;
+		temp &= ~enable_bits;
 	} else {
-		temp |= SDVO_ENABLE;
+		temp |= enable_bits;
 	}
 
 	I915_WRITE(intel_hdmi->sdvox_reg, temp);
-- 
1.7.8.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/7] drm/i915: DisplayPort hot remove notification to audio driver
  2012-01-31 15:51 [PATCH 3.2] drm/i915 patches for 3.2 Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 1/7] drm/i915: HDMI hot remove notification to audio driver Eugeni Dodonov
@ 2012-01-31 15:51 ` Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 3/7] drm/i915: check ACTHD of all rings Eugeni Dodonov
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Eugeni Dodonov @ 2012-01-31 15:51 UTC (permalink / raw)
  To: stable; +Cc: intel-gfx, Wu Fengguang, Eugeni Dodonov

From: Wu Fengguang <fengguang.wu@intel.com>

On DP monitor hot remove, clear DP_AUDIO_OUTPUT_ENABLE accordingly,
so that the audio driver will receive hot plug events and take action
to refresh its device state and ELD contents.

Note that the DP_AUDIO_OUTPUT_ENABLE bit may be enabled or disabled
only when the link training is complete and set to "Normal".

Tested OK for both hot plug/remove and DPMS on/off.

Upstream patch:
commit 832afda6a7d7235ef0e09f4ec46736861540da6d
Author: Wu Fengguang <fengguang.wu@intel.com>
Date:   Fri Dec 9 20:42:21 2011 +0800

    drm/i915: DisplayPort hot remove notification to audio driver

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 92b041b..db3b461 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1926,6 +1926,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 			intel_wait_for_vblank(dev, to_intel_crtc(crtc)->pipe);
 	}
 
+	DP &= ~DP_AUDIO_OUTPUT_ENABLE;
 	I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN);
 	POSTING_READ(intel_dp->output_reg);
 	msleep(intel_dp->panel_power_down_delay);
-- 
1.7.8.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/7] drm/i915: check ACTHD of all rings
  2012-01-31 15:51 [PATCH 3.2] drm/i915 patches for 3.2 Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 1/7] drm/i915: HDMI hot remove notification to audio driver Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 2/7] drm/i915: DisplayPort " Eugeni Dodonov
@ 2012-01-31 15:51 ` Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 4/7] drm/i915: Fix TV Out refresh rate Eugeni Dodonov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Eugeni Dodonov @ 2012-01-31 15:51 UTC (permalink / raw)
  To: stable; +Cc: Daniel Vetter, intel-gfx, Eugeni Dodonov

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Otherwise hangcheck spuriously fires when running blitter/bsd-only
workloads.

Contrary to a similar patch by Ben Widawsky this does not check
INSTDONE of the other rings. Chris Wilson implied that in a failure to
detect a hang, most likely because INSTDONE was fluctuating. Thus only
check ACTHD, which as far as I know is rather reliable. Also, blitter
and bsd rings can't launch complex tasks from a single instruction
(like 3D_PRIM on the render with complex or even infinite shaders).

This fixes spurious gpu hang detection when running
tests/gem_hangcheck_forcewake on snb/ivb.

Upstream patch:
commit 097354eb14fa94d31a09c64d640643f58e4a5a9a
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sun Nov 27 18:58:17 2011 +0100

    drm/i915: check ACTHD of all rings

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |    2 ++
 drivers/gpu/drm/i915/i915_irq.c |   13 ++++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 554bef7..505af3f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -337,6 +337,8 @@ typedef struct drm_i915_private {
 	struct timer_list hangcheck_timer;
 	int hangcheck_count;
 	uint32_t last_acthd;
+	uint32_t last_acthd_bsd;
+	uint32_t last_acthd_blt;
 	uint32_t last_instdone;
 	uint32_t last_instdone1;
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b40004b..d47a53b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1669,7 +1669,7 @@ void i915_hangcheck_elapsed(unsigned long data)
 {
 	struct drm_device *dev = (struct drm_device *)data;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	uint32_t acthd, instdone, instdone1;
+	uint32_t acthd, instdone, instdone1, acthd_bsd, acthd_blt;
 	bool err = false;
 
 	if (!i915_enable_hangcheck)
@@ -1686,16 +1686,21 @@ void i915_hangcheck_elapsed(unsigned long data)
 	}
 
 	if (INTEL_INFO(dev)->gen < 4) {
-		acthd = I915_READ(ACTHD);
 		instdone = I915_READ(INSTDONE);
 		instdone1 = 0;
 	} else {
-		acthd = I915_READ(ACTHD_I965);
 		instdone = I915_READ(INSTDONE_I965);
 		instdone1 = I915_READ(INSTDONE1);
 	}
+	acthd = intel_ring_get_active_head(&dev_priv->ring[RCS]);
+	acthd_bsd = HAS_BSD(dev) ?
+		intel_ring_get_active_head(&dev_priv->ring[VCS]) : 0;
+	acthd_blt = HAS_BLT(dev) ?
+		intel_ring_get_active_head(&dev_priv->ring[BCS]) : 0;
 
 	if (dev_priv->last_acthd == acthd &&
+	    dev_priv->last_acthd_bsd == acthd_bsd &&
+	    dev_priv->last_acthd_blt == acthd_blt &&
 	    dev_priv->last_instdone == instdone &&
 	    dev_priv->last_instdone1 == instdone1) {
 		if (dev_priv->hangcheck_count++ > 1) {
@@ -1727,6 +1732,8 @@ void i915_hangcheck_elapsed(unsigned long data)
 		dev_priv->hangcheck_count = 0;
 
 		dev_priv->last_acthd = acthd;
+		dev_priv->last_acthd_bsd = acthd_bsd;
+		dev_priv->last_acthd_blt = acthd_blt;
 		dev_priv->last_instdone = instdone;
 		dev_priv->last_instdone1 = instdone1;
 	}
-- 
1.7.8.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/7] drm/i915: Fix TV Out refresh rate.
  2012-01-31 15:51 [PATCH 3.2] drm/i915 patches for 3.2 Eugeni Dodonov
                   ` (2 preceding siblings ...)
  2012-01-31 15:51 ` [PATCH 3/7] drm/i915: check ACTHD of all rings Eugeni Dodonov
@ 2012-01-31 15:51 ` Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 5/7] drm/i915: handle IVB 3rd pipe during suspend Eugeni Dodonov
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Eugeni Dodonov @ 2012-01-31 15:51 UTC (permalink / raw)
  To: stable; +Cc: intel-gfx, Eugeni Dodonov

From: Rodrigo Vivi <rodrigo.vivi@gmail.com>

TV Out refresh rate was half of the specification for almost all modes.
Due to this reason pixel clock was so low for some modes causing flickering screen.

Upstream patch:
commit 23bd15ec662344dc10e9918fdd0dbc58bc71526d
Author: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Date:   Wed Dec 14 21:10:06 2011 -0200

    drm/i915: Fix TV Out refresh rate.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/intel_tv.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index f3c6a9a..2b1fcad 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -417,7 +417,7 @@ static const struct tv_mode tv_modes[] = {
 	{
 		.name		= "NTSC-M",
 		.clock		= 108000,
-		.refresh	= 29970,
+		.refresh	= 59940,
 		.oversample	= TV_OVERSAMPLE_8X,
 		.component_only = 0,
 		/* 525 Lines, 60 Fields, 15.734KHz line, Sub-Carrier 3.580MHz */
@@ -460,7 +460,7 @@ static const struct tv_mode tv_modes[] = {
 	{
 		.name		= "NTSC-443",
 		.clock		= 108000,
-		.refresh	= 29970,
+		.refresh	= 59940,
 		.oversample	= TV_OVERSAMPLE_8X,
 		.component_only = 0,
 		/* 525 Lines, 60 Fields, 15.734KHz line, Sub-Carrier 4.43MHz */
@@ -502,7 +502,7 @@ static const struct tv_mode tv_modes[] = {
 	{
 		.name		= "NTSC-J",
 		.clock		= 108000,
-		.refresh	= 29970,
+		.refresh	= 59940,
 		.oversample	= TV_OVERSAMPLE_8X,
 		.component_only = 0,
 
@@ -545,7 +545,7 @@ static const struct tv_mode tv_modes[] = {
 	{
 		.name		= "PAL-M",
 		.clock		= 108000,
-		.refresh	= 29970,
+		.refresh	= 59940,
 		.oversample	= TV_OVERSAMPLE_8X,
 		.component_only = 0,
 
@@ -589,7 +589,7 @@ static const struct tv_mode tv_modes[] = {
 		/* 625 Lines, 50 Fields, 15.625KHz line, Sub-Carrier 4.434MHz */
 		.name	    = "PAL-N",
 		.clock		= 108000,
-		.refresh	= 25000,
+		.refresh	= 50000,
 		.oversample	= TV_OVERSAMPLE_8X,
 		.component_only = 0,
 
@@ -634,7 +634,7 @@ static const struct tv_mode tv_modes[] = {
 		/* 625 Lines, 50 Fields, 15.625KHz line, Sub-Carrier 4.434MHz */
 		.name	    = "PAL",
 		.clock		= 108000,
-		.refresh	= 25000,
+		.refresh	= 50000,
 		.oversample	= TV_OVERSAMPLE_8X,
 		.component_only = 0,
 
@@ -821,7 +821,7 @@ static const struct tv_mode tv_modes[] = {
 	{
 		.name       = "1080i@50Hz",
 		.clock		= 148800,
-		.refresh	= 25000,
+		.refresh	= 50000,
 		.oversample     = TV_OVERSAMPLE_2X,
 		.component_only = 1,
 
@@ -847,7 +847,7 @@ static const struct tv_mode tv_modes[] = {
 	{
 		.name       = "1080i@60Hz",
 		.clock		= 148800,
-		.refresh	= 30000,
+		.refresh	= 60000,
 		.oversample     = TV_OVERSAMPLE_2X,
 		.component_only = 1,
 
-- 
1.7.8.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/7] drm/i915: handle IVB 3rd pipe during suspend
  2012-01-31 15:51 [PATCH 3.2] drm/i915 patches for 3.2 Eugeni Dodonov
                   ` (3 preceding siblings ...)
  2012-01-31 15:51 ` [PATCH 4/7] drm/i915: Fix TV Out refresh rate Eugeni Dodonov
@ 2012-01-31 15:51 ` Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 6/7] drm/i915: convert force_wake_get to func pointer in the gpu reset code Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 7/7] drm/i915: protect force_wake_(get|put) with the gt_lock Eugeni Dodonov
  6 siblings, 0 replies; 8+ messages in thread
From: Eugeni Dodonov @ 2012-01-31 15:51 UTC (permalink / raw)
  To: stable; +Cc: intel-gfx, Eugeni Dodonov

We don't need to check 3rd pipe specifically, as it shares PLL with some
other one.

(v2 - for stable kernel: clarify bug description to explain that this
fixes suspend when 3rd pipe is active - fd.o bug #41977).

Upstream commit:
commit 07c1e8c1462fa7324de4c36ae9e55da2abd79cee
Author: Eugeni Dodonov <eugeni.dodonov@intel.com>
Date:   Sat Jan 7 23:40:35 2012 -0200

    drm/i915: handle 3rd pipe

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41977
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_suspend.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 7886e4f..b815140 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -34,6 +34,10 @@ static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32	dpll_reg;
 
+	/* On IVB, 3rd pipe shares PLL with another one */
+	if (pipe > 1)
+		return false;
+
 	if (HAS_PCH_SPLIT(dev))
 		dpll_reg = (pipe == PIPE_A) ? _PCH_DPLL_A : _PCH_DPLL_B;
 	else
-- 
1.7.8.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/7] drm/i915: convert force_wake_get to func pointer in the gpu reset code
  2012-01-31 15:51 [PATCH 3.2] drm/i915 patches for 3.2 Eugeni Dodonov
                   ` (4 preceding siblings ...)
  2012-01-31 15:51 ` [PATCH 5/7] drm/i915: handle IVB 3rd pipe during suspend Eugeni Dodonov
@ 2012-01-31 15:51 ` Eugeni Dodonov
  2012-01-31 15:51 ` [PATCH 7/7] drm/i915: protect force_wake_(get|put) with the gt_lock Eugeni Dodonov
  6 siblings, 0 replies; 8+ messages in thread
From: Eugeni Dodonov @ 2012-01-31 15:51 UTC (permalink / raw)
  To: stable; +Cc: intel-gfx, Eugeni Dodonov

From: Daniel Vetter <daniel@ffwll.ch>

This was forgotten in the original multi-threaded forcewake
conversion:

commit 8d715f0024f64ad1b1be85d8c081cf577944c847
Author: Keith Packard <keithp at keithp.com>
Date:   Fri Nov 18 20:39:01 2011 -0800

    drm/i915: add multi-threaded forcewake support

Upstream patch:
commit 8109021313c7a3d8947677391ce6ab9cd0bb1d28
Author: Daniel Vetter <daniel@ffwll.ch>
Date:   Fri Jan 13 16:20:06 2012 -0800

    drm/i915: convert force_wake_get to func pointer in the gpu reset code

Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a1103fc..d7c9d99 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -645,7 +645,7 @@ int i915_reset(struct drm_device *dev, u8 flags)
 		ret = gen6_do_reset(dev, flags);
 		/* If reset with a user forcewake, try to restore */
 		if (atomic_read(&dev_priv->forcewake_count))
-			__gen6_gt_force_wake_get(dev_priv);
+			dev_priv->display.force_wake_get(dev_priv);
 		break;
 	case 5:
 		ret = ironlake_do_reset(dev, flags);
-- 
1.7.8.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 7/7] drm/i915: protect force_wake_(get|put) with the gt_lock
  2012-01-31 15:51 [PATCH 3.2] drm/i915 patches for 3.2 Eugeni Dodonov
                   ` (5 preceding siblings ...)
  2012-01-31 15:51 ` [PATCH 6/7] drm/i915: convert force_wake_get to func pointer in the gpu reset code Eugeni Dodonov
@ 2012-01-31 15:51 ` Eugeni Dodonov
  6 siblings, 0 replies; 8+ messages in thread
From: Eugeni Dodonov @ 2012-01-31 15:51 UTC (permalink / raw)
  To: stable; +Cc: Daniel Vetter, intel-gfx, Eugeni Dodonov

From: Daniel Vetter <daniel.vetter@ffwll.ch>

The problem this patch solves is that the forcewake accounting
necessary for register reads is protected by dev->struct_mutex. But the
hangcheck and error_capture code need to access registers without
grabbing this mutex because we hold it while waiting for the gpu.
So a new lock is required. Because currently the error_state capture
is called from the error irq handler and the hangcheck code runs from
a timer, it needs to be an irqsafe spinlock (note that the registers
used by the irq handler (neglecting the error handling part) only uses
registers that don't need the forcewake dance).

We could tune this down to a normal spinlock when we rework the
error_state capture and hangcheck code to run from a workqueue.  But
we don't have any read in a fastpath that needs forcewake, so I've
decided to not care much about overhead.

This prevents tests/gem_hangcheck_forcewake from i-g-t from killing my
snb on recent kernels - something must have slightly changed the
timings. On previous kernels it only trigger a WARN about the broken
locking.

v2: Drop the previous patch for the register writes.

v3: Improve the commit message per Chris Wilson's suggestions.

Upstream patch:
commit 9f1f46a45a681d357d1ceedecec3671a5ae957f4
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Dec 14 13:57:03 2011 +0100

    drm/i915: protect force_wake_(get|put) with the gt_lock

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |    8 ++++++--
 drivers/gpu/drm/i915/i915_dma.c     |    1 +
 drivers/gpu/drm/i915/i915_drv.c     |   18 ++++++++++++------
 drivers/gpu/drm/i915/i915_drv.h     |   10 +++++++---
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 004b048..b2e3c97 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1314,9 +1314,13 @@ static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned forcewake_count;
 
-	seq_printf(m, "forcewake count = %d\n",
-		   atomic_read(&dev_priv->forcewake_count));
+	spin_lock_irq(&dev_priv->gt_lock);
+	forcewake_count = dev_priv->forcewake_count;
+	spin_unlock_irq(&dev_priv->gt_lock);
+
+	seq_printf(m, "forcewake count = %u\n", forcewake_count);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a9ae374..c4da951 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2042,6 +2042,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (!IS_I945G(dev) && !IS_I945GM(dev))
 		pci_enable_msi(dev->pdev);
 
+	spin_lock_init(&dev_priv->gt_lock);
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->error_lock);
 	spin_lock_init(&dev_priv->rps_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d7c9d99..e2d85a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -368,11 +368,12 @@ void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
  */
 void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 {
-	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+	unsigned long irqflags;
 
-	/* Forcewake is atomic in case we get in here without the lock */
-	if (atomic_add_return(1, &dev_priv->forcewake_count) == 1)
+	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
+	if (dev_priv->forcewake_count++ == 0)
 		dev_priv->display.force_wake_get(dev_priv);
+	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
 }
 
 void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
@@ -392,10 +393,12 @@ void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
  */
 void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 {
-	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+	unsigned long irqflags;
 
-	if (atomic_dec_and_test(&dev_priv->forcewake_count))
+	spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
+	if (--dev_priv->forcewake_count == 0)
 		dev_priv->display.force_wake_put(dev_priv);
+	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
 }
 
 void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
@@ -626,6 +629,7 @@ int i915_reset(struct drm_device *dev, u8 flags)
 	 * need to
 	 */
 	bool need_display = true;
+	unsigned long irqflags;
 	int ret;
 
 	if (!i915_try_reset)
@@ -644,8 +648,10 @@ int i915_reset(struct drm_device *dev, u8 flags)
 	case 6:
 		ret = gen6_do_reset(dev, flags);
 		/* If reset with a user forcewake, try to restore */
-		if (atomic_read(&dev_priv->forcewake_count))
+		spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
+		if (dev_priv->forcewake_count)
 			dev_priv->display.force_wake_get(dev_priv);
+		spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
 		break;
 	case 5:
 		ret = ironlake_do_reset(dev, flags);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 505af3f..ae294a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -286,7 +286,13 @@ typedef struct drm_i915_private {
 	int relative_constants_mode;
 
 	void __iomem *regs;
-	u32 gt_fifo_count;
+	/** gt_fifo_count and the subsequent register write are synchronized
+	 * with dev->struct_mutex. */
+	unsigned gt_fifo_count;
+	/** forcewake_count is protected by gt_lock */
+	unsigned forcewake_count;
+	/** gt_lock is also taken in irq contexts. */
+	struct spinlock gt_lock;
 
 	struct intel_gmbus {
 		struct i2c_adapter adapter;
@@ -738,8 +744,6 @@ typedef struct drm_i915_private {
 
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
-
-	atomic_t forcewake_count;
 } drm_i915_private_t;
 
 enum i915_cache_level {
-- 
1.7.8.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-01-31 15:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-31 15:51 [PATCH 3.2] drm/i915 patches for 3.2 Eugeni Dodonov
2012-01-31 15:51 ` [PATCH 1/7] drm/i915: HDMI hot remove notification to audio driver Eugeni Dodonov
2012-01-31 15:51 ` [PATCH 2/7] drm/i915: DisplayPort " Eugeni Dodonov
2012-01-31 15:51 ` [PATCH 3/7] drm/i915: check ACTHD of all rings Eugeni Dodonov
2012-01-31 15:51 ` [PATCH 4/7] drm/i915: Fix TV Out refresh rate Eugeni Dodonov
2012-01-31 15:51 ` [PATCH 5/7] drm/i915: handle IVB 3rd pipe during suspend Eugeni Dodonov
2012-01-31 15:51 ` [PATCH 6/7] drm/i915: convert force_wake_get to func pointer in the gpu reset code Eugeni Dodonov
2012-01-31 15:51 ` [PATCH 7/7] drm/i915: protect force_wake_(get|put) with the gt_lock Eugeni Dodonov

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.