[PATCH 1/3] drm/i915: rip out gen2 reset code

[PATCH 1/3] drm/i915: rip out gen2 reset code

[Daniel Vetter]

At least on my i830M here it reliably results in hard system hangs
nowadays. This is much worse than falling back to software rendering,
so I think we should simply rip this out.

After all we don't have any gpu reset for gen3 either, and there are a
lot more of those still around.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_uncore.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 288a3a6..bfc1a65 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -490,36 +490,6 @@ int i915_reg_read_ioctl(struct drm_device *dev,
 	return 0;
 }
 
-static int i8xx_do_reset(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (IS_I85X(dev))
-		return -ENODEV;
-
-	I915_WRITE(D_STATE, I915_READ(D_STATE) | DSTATE_GFX_RESET_I830);
-	POSTING_READ(D_STATE);
-
-	if (IS_I830(dev) || IS_845G(dev)) {
-		I915_WRITE(DEBUG_RESET_I830,
-			   DEBUG_RESET_DISPLAY |
-			   DEBUG_RESET_RENDER |
-			   DEBUG_RESET_FULL);
-		POSTING_READ(DEBUG_RESET_I830);
-		msleep(1);
-
-		I915_WRITE(DEBUG_RESET_I830, 0);
-		POSTING_READ(DEBUG_RESET_I830);
-	}
-
-	msleep(1);
-
-	I915_WRITE(D_STATE, I915_READ(D_STATE) & ~DSTATE_GFX_RESET_I830);
-	POSTING_READ(D_STATE);
-
-	return 0;
-}
-
 static int i965_reset_complete(struct drm_device *dev)
 {
 	u8 gdrst;
@@ -621,7 +591,6 @@ int intel_gpu_reset(struct drm_device *dev)
 	case 6: return gen6_do_reset(dev);
 	case 5: return ironlake_do_reset(dev);
 	case 4: return i965_do_reset(dev);
-	case 2: return i8xx_do_reset(dev);
 	default: return -ENODEV;
 	}
 }
-- 
1.8.1.4

[PATCH 2/3] drm/i915/dvo: call ->mode_set callback only when the port is running

[Daniel Vetter]

The ns2501 controller seems to need the dpll and dvo port to accept
the timing update commands. Quick testing on my x30 here seems to
indicate that other dvo controllers don't mind. So let's move the
->mode_set callback to a place where we have the port up and running
already.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dvo.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 1b64145..3c77365 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -173,11 +173,16 @@ static void intel_enable_dvo(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
+	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	u32 dvo_reg = intel_dvo->dev.dvo_reg;
 	u32 temp = I915_READ(dvo_reg);
 
 	I915_WRITE(dvo_reg, temp | DVO_ENABLE);
 	I915_READ(dvo_reg);
+	intel_dvo->dev.dev_ops->mode_set(&intel_dvo->dev,
+					 &crtc->config.requested_mode,
+					 &crtc->config.adjusted_mode);
+
 	intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, true);
 }
 
@@ -186,6 +191,7 @@ static void intel_dvo_dpms(struct drm_connector *connector, int mode)
 {
 	struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
 	struct drm_crtc *crtc;
+	struct intel_crtc_config *config;
 
 	/* dvo supports only 2 dpms states. */
 	if (mode != DRM_MODE_DPMS_ON)
@@ -206,10 +212,16 @@ static void intel_dvo_dpms(struct drm_connector *connector, int mode)
 	/* We call connector dpms manually below in case pipe dpms doesn't
 	 * change due to cloning. */
 	if (mode == DRM_MODE_DPMS_ON) {
+		config = &to_intel_crtc(crtc)->config;
+
 		intel_dvo->base.connectors_active = true;
 
 		intel_crtc_update_dpms(crtc);
 
+		intel_dvo->dev.dev_ops->mode_set(&intel_dvo->dev,
+						 &config->requested_mode,
+						 &config->adjusted_mode);
+
 		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, true);
 	} else {
 		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, false);
@@ -296,10 +308,6 @@ static void intel_dvo_mode_set(struct intel_encoder *encoder)
 		break;
 	}
 
-	intel_dvo->dev.dev_ops->mode_set(&intel_dvo->dev,
-					 &crtc->config.requested_mode,
-					 adjusted_mode);
-
 	/* Save the data order, since I don't know what it should be set to. */
 	dvo_val = I915_READ(dvo_reg) &
 		  (DVO_PRESERVE_MASK | DVO_DATA_ORDER_GBRG);
-- 
1.8.1.4

Re: [PATCH 2/3] drm/i915/dvo: call ->mode_set callback only when the port is running

[Chris Wilson]

On Tue, Oct 08, 2013 at 12:25:42PM +0200, Daniel Vetter wrote:
> The ns2501 controller seems to need the dpll and dvo port to accept
> the timing update commands. Quick testing on my x30 here seems to
> indicate that other dvo controllers don't mind. So let's move the
> ->mode_set callback to a place where we have the port up and running
> already.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

It doesn't seem to make the ch7xxx dvo LVDS any worse here.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 3/3] drm/i915/ns2501: Rip out the reenable hack

[Daniel Vetter]

With the change in the modeset sequence this shouldn't be required
any more since the ->mode_set callback now gets called when the dvo
port is fully up and running.

Also limit the retry loop to 10 tries to avoid hanging the machine
while holding important modeset locks.

Cc: Thomas Richter <thor@math.tu-berlin.de>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/dvo_ns2501.c | 73 +++------------------------------------
 1 file changed, 4 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
index c4a255b..954acb2 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -87,49 +87,6 @@ struct ns2501_priv {
  * when switching the resolution.
  */
 
-static void enable_dvo(struct intel_dvo_device *dvo)
-{
-	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
-	struct i2c_adapter *adapter = dvo->i2c_bus;
-	struct intel_gmbus *bus = container_of(adapter,
-					       struct intel_gmbus,
-					       adapter);
-	struct drm_i915_private *dev_priv = bus->dev_priv;
-
-	DRM_DEBUG_KMS("%s: Trying to re-enable the DVO\n", __FUNCTION__);
-
-	ns->dvoc = I915_READ(DVO_C);
-	ns->pll_a = I915_READ(_DPLL_A);
-	ns->srcdim = I915_READ(DVOC_SRCDIM);
-	ns->fw_blc = I915_READ(FW_BLC);
-
-	I915_WRITE(DVOC, 0x10004084);
-	I915_WRITE(_DPLL_A, 0xd0820000);
-	I915_WRITE(DVOC_SRCDIM, 0x400300);	// 1024x768
-	I915_WRITE(FW_BLC, 0x1080304);
-
-	I915_WRITE(DVOC, 0x90004084);
-}
-
-/*
- * Restore the I915 registers modified by the above
- * trigger function.
- */
-static void restore_dvo(struct intel_dvo_device *dvo)
-{
-	struct i2c_adapter *adapter = dvo->i2c_bus;
-	struct intel_gmbus *bus = container_of(adapter,
-					       struct intel_gmbus,
-					       adapter);
-	struct drm_i915_private *dev_priv = bus->dev_priv;
-	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
-
-	I915_WRITE(DVOC, ns->dvoc);
-	I915_WRITE(_DPLL_A, ns->pll_a);
-	I915_WRITE(DVOC_SRCDIM, ns->srcdim);
-	I915_WRITE(FW_BLC, ns->fw_blc);
-}
-
 /*
 ** Read a register from the ns2501.
 ** Returns true if successful, false otherwise.
@@ -300,7 +257,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
 			    struct drm_display_mode *adjusted_mode)
 {
 	bool ok;
-	bool restore = false;
+	int retries = 10;
 	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
 
 	DRM_DEBUG_KMS
@@ -476,20 +433,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
 			ns->reg_8_shadow |= NS2501_8_BPAS;
 		}
 		ok &= ns2501_writeb(dvo, NS2501_REG8, ns->reg_8_shadow);
-
-		if (!ok) {
-			if (restore)
-				restore_dvo(dvo);
-			enable_dvo(dvo);
-			restore = true;
-		}
-	} while (!ok);
-	/*
-	 * Restore the old i915 registers before
-	 * forcing the ns2501 on.
-	 */
-	if (restore)
-		restore_dvo(dvo);
+	} while (!ok && retries--);
 }
 
 /* set the NS2501 power state */
@@ -510,7 +454,7 @@ static bool ns2501_get_hw_state(struct intel_dvo_device *dvo)
 static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
 {
 	bool ok;
-	bool restore = false;
+	int retries = 10;
 	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
 	unsigned char ch;
 
@@ -537,16 +481,7 @@ static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
 			ok &=
 			    ns2501_writeb(dvo, 0x35,
 					  enable ? 0xff : 0x00);
-			if (!ok) {
-				if (restore)
-					restore_dvo(dvo);
-				enable_dvo(dvo);
-				restore = true;
-			}
-		} while (!ok);
-
-		if (restore)
-			restore_dvo(dvo);
+		} while (!ok && retries--);
 	}
 }
 
-- 
1.8.1.4

Re: [PATCH 1/3] drm/i915: rip out gen2 reset code

[Chris Wilson]

On Tue, Oct 08, 2013 at 12:25:41PM +0200, Daniel Vetter wrote:
> At least on my i830M here it reliably results in hard system hangs
> nowadays. This is much worse than falling back to software rendering,
> so I think we should simply rip this out.
> 
> After all we don't have any gpu reset for gen3 either, and there are a
> lot more of those still around.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/3] drm/i915: rip out gen2 reset code

[Daniel Vetter]

On Tue, Oct 08, 2013 at 11:29:41AM +0100, Chris Wilson wrote:
> On Tue, Oct 08, 2013 at 12:25:41PM +0200, Daniel Vetter wrote:
> > At least on my i830M here it reliably results in hard system hangs
> > nowadays. This is much worse than falling back to software rendering,
> > so I think we should simply rip this out.
> > 
> > After all we don't have any gpu reset for gen3 either, and there are a
> > lot more of those still around.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>

Merged, thanks for sanity checking and the quick testrun on your i845g.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

drm/i915/ns2501: Update from git working

[Thomas Richter]

Hi Daniel, dear kernel developers,

just tried the 3.12.rc3+ from the intel-drm git. This version *worked* 
again, pretty much the same way the 3.10.10 "worked".

Side effects are, however, quite the same when you connect an external 
monitor:

If connected while the gdm login screen is on, the external monitor 
works fine with the internal TFT (mirroring is turned on). Resolutions
1024x768 and 800x600 are fine, switching to 640x480 locks up the 
internal DVO, the external monitor keeps working. Note that the DVO
does *not* lock up if the external monitor is not connect (probably a 
frequency issue, need to check).

Booting with the external monitor connected creates a "locked up" 
display on the internal TFT, and a flickering display on the external
screen as if the watermark levels are not set correctly. The external 
screen shows a horizontal offset of approximately half the screen
width (scrolled to the right), but jumps back to its correct position 
irregularly. Note that this type of flickering is different from the flicker
created by panning on the 835GM (already reported this problem), which 
shows a regular 30Hz flicker.

Once the gdm comes up, the internal TFT keeps dead, and the external 
shows only a black screen but with a working mouse pointer.
Logging in blindly restores a working screen on both monitors.

So thanks for the update, seems to have worked, but is still not working 
perfectly. External monitors are not supported correctly,
and panning is still a problem, even with a "linear" framebuffer.

Greetings,
     Thomas

Re: drm/i915/ns2501: Update from git working

[Daniel Vetter]

On Tue, Oct 08, 2013 at 10:56:41PM +0200, Thomas Richter wrote:
> Hi Daniel, dear kernel developers,
> 
> just tried the 3.12.rc3+ from the intel-drm git. This version
> *worked* again, pretty much the same way the 3.10.10 "worked".
> 
> Side effects are, however, quite the same when you connect an
> external monitor:
> 
> If connected while the gdm login screen is on, the external monitor
> works fine with the internal TFT (mirroring is turned on).
> Resolutions
> 1024x768 and 800x600 are fine, switching to 640x480 locks up the
> internal DVO, the external monitor keeps working. Note that the DVO
> does *not* lock up if the external monitor is not connect (probably
> a frequency issue, need to check).
> 
> Booting with the external monitor connected creates a "locked up"
> display on the internal TFT, and a flickering display on the
> external
> screen as if the watermark levels are not set correctly. The
> external screen shows a horizontal offset of approximately half the
> screen
> width (scrolled to the right), but jumps back to its correct
> position irregularly. Note that this type of flickering is different
> from the flicker
> created by panning on the 835GM (already reported this problem),
> which shows a regular 30Hz flicker.
> 
> Once the gdm comes up, the internal TFT keeps dead, and the external
> shows only a black screen but with a working mouse pointer.
> Logging in blindly restores a working screen on both monitors.
> 
> So thanks for the update, seems to have worked, but is still not
> working perfectly. External monitors are not supported correctly,
> and panning is still a problem, even with a "linear" framebuffer.

My drm-intel git branch doesn't yet have the two patches that are meant to
replace your ns2501 hack ... So I'm not too sure what exactly you've
tested that made things magically better?

For me the key to get rid of the black screen was the two sna patches, not
the kernel stuff here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: drm/i915/ns2501: Update from git working

[Thomas Richter]

On 09.10.2013 00:00, Daniel Vetter wrote:
>
> My drm-intel git branch doesn't yet have the two patches that are meant to
> replace your ns2501 hack ... So I'm not too sure what exactly you've
> tested that made things magically better?

Is there a difference between the 3.12rc4 from kernel.org and the 
3.12.rc3+ from your git repository that might explain the difference? 
These are the two kernels I tested, with the results as presented. 
Kernel.org is apparently one release cycle (and one bug) ahead.

Greetings,
	Thomas

Re: drm/i915/ns2501: Update from git working

[Daniel Vetter]

On Wed, Oct 9, 2013 at 7:02 AM, Thomas Richter <thor@math.tu-berlin.de> wrote:
> On 09.10.2013 00:00, Daniel Vetter wrote:
>>
>>
>> My drm-intel git branch doesn't yet have the two patches that are meant to
>> replace your ns2501 hack ... So I'm not too sure what exactly you've
>> tested that made things magically better?
>
>
> Is there a difference between the 3.12rc4 from kernel.org and the 3.12.rc3+
> from your git repository that might explain the difference? These are the
> two kernels I tested, with the results as presented. Kernel.org is
> apparently one release cycle (and one bug) ahead.

drm-intel-nightly already contains tons of changes for 3.13. But
nothing really comes to mind that might help out on your i830M. At
least here on mine I didn't see any difference ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch