[PATCH] drm/i915/pch: Save/restore PCH_PORT_HOTPLUG across suspend

[PATCH] drm/i915/pch: Save/restore PCH_PORT_HOTPLUG across suspend

[Adam Jackson]

At least on a Lenovo X220 the HPD bits of this are enabled at boot but
cleared after resume, which means plug interrupts stop working.

This also happens to fix DP displays re-lighting on resume.  I'm quite
certain that's an accident: the first DP link train inevitably fails on
that machine, and it's only serendipity that we're getting multiple plug
interrupts and the second train works.  But I shall take my victories
where I get them.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_suspend.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf30f99..6b93fa5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -541,6 +541,7 @@ typedef struct drm_i915_private {
 	u32 savePIPEB_LINK_M1;
 	u32 savePIPEB_LINK_N1;
 	u32 saveMCHBAR_RENDER_STANDBY;
+	u32 savePCH_PORT_HOTPLUG;
 
 	struct {
 		/** Bridge to intel-gtt-ko */
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 5257cfc..27693c0 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -814,6 +814,7 @@ int i915_save_state(struct drm_device *dev)
 		dev_priv->saveFDI_RXB_IMR = I915_READ(_FDI_RXB_IMR);
 		dev_priv->saveMCHBAR_RENDER_STANDBY =
 			I915_READ(RSTDBYCTL);
+		dev_priv->savePCH_PORT_HOTPLUG = I915_READ(PCH_PORT_HOTPLUG);
 	} else {
 		dev_priv->saveIER = I915_READ(IER);
 		dev_priv->saveIMR = I915_READ(IMR);
@@ -865,6 +866,7 @@ int i915_restore_state(struct drm_device *dev)
 		I915_WRITE(GTIMR, dev_priv->saveGTIMR);
 		I915_WRITE(_FDI_RXA_IMR, dev_priv->saveFDI_RXA_IMR);
 		I915_WRITE(_FDI_RXB_IMR, dev_priv->saveFDI_RXB_IMR);
+		I915_WRITE(PCH_PORT_HOTPLUG, dev_priv->savePCH_PORT_HOTPLUG);
 	} else {
 		I915_WRITE(IER, dev_priv->saveIER);
 		I915_WRITE(IMR, dev_priv->saveIMR);
-- 
1.7.6

Re: [PATCH] drm/i915/pch: Save/restore PCH_PORT_HOTPLUG across suspend

[Jesse Barnes]

On Tue, 26 Jul 2011 16:53:06 -0400
Adam Jackson <ajax@redhat.com> wrote:

> At least on a Lenovo X220 the HPD bits of this are enabled at boot but
> cleared after resume, which means plug interrupts stop working.
> 
> This also happens to fix DP displays re-lighting on resume.  I'm quite
> certain that's an accident: the first DP link train inevitably fails on
> that machine, and it's only serendipity that we're getting multiple plug
> interrupts and the second train works.  But I shall take my victories
> where I get them.
> 
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |    1 +
>  drivers/gpu/drm/i915/i915_suspend.c |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cf30f99..6b93fa5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -541,6 +541,7 @@ typedef struct drm_i915_private {
>  	u32 savePIPEB_LINK_M1;
>  	u32 savePIPEB_LINK_N1;
>  	u32 saveMCHBAR_RENDER_STANDBY;
> +	u32 savePCH_PORT_HOTPLUG;
>  
>  	struct {
>  		/** Bridge to intel-gtt-ko */
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index 5257cfc..27693c0 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -814,6 +814,7 @@ int i915_save_state(struct drm_device *dev)
>  		dev_priv->saveFDI_RXB_IMR = I915_READ(_FDI_RXB_IMR);
>  		dev_priv->saveMCHBAR_RENDER_STANDBY =
>  			I915_READ(RSTDBYCTL);
> +		dev_priv->savePCH_PORT_HOTPLUG = I915_READ(PCH_PORT_HOTPLUG);
>  	} else {
>  		dev_priv->saveIER = I915_READ(IER);
>  		dev_priv->saveIMR = I915_READ(IMR);
> @@ -865,6 +866,7 @@ int i915_restore_state(struct drm_device *dev)
>  		I915_WRITE(GTIMR, dev_priv->saveGTIMR);
>  		I915_WRITE(_FDI_RXA_IMR, dev_priv->saveFDI_RXA_IMR);
>  		I915_WRITE(_FDI_RXB_IMR, dev_priv->saveFDI_RXB_IMR);
> +		I915_WRITE(PCH_PORT_HOTPLUG, dev_priv->savePCH_PORT_HOTPLUG);
>  	} else {
>  		I915_WRITE(IER, dev_priv->saveIER);
>  		I915_WRITE(IMR, dev_priv->saveIMR);

We should be writing this reg.  The only question is whether we should
be trusting the BIOS values (which may have custom pulse duration
settings) or just unconditionally enabling hot plug for ports we care
about with the default 2ms pulse width (per the DP spec).

If we assume the BIOS programs this reg to a good value (a very big
assumption) saving and restoring it is safest.  I just wonder if we'll
find machines where it's broken by default leading to weird DP behavior.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915/pch: Save/restore PCH_PORT_HOTPLUG across suspend

[Adam Jackson]

[-- Attachment #1.1: Type: text/plain, Size: 1354 bytes --]

On Tue, 2011-07-26 at 14:00 -0700, Jesse Barnes wrote:

> We should be writing this reg.  The only question is whether we should
> be trusting the BIOS values (which may have custom pulse duration
> settings) or just unconditionally enabling hot plug for ports we care
> about with the default 2ms pulse width (per the DP spec).
> 
> If we assume the BIOS programs this reg to a good value (a very big
> assumption) saving and restoring it is safest.  I just wonder if we'll
> find machines where it's broken by default leading to weird DP behavior.

Another option is reading the HPD input enable bits from the platform
value, but setting the pulse width ourselves.  I have a vague fear of
enabling HPD for all ports blindly, I suspect there's some platform out
there that miswires things such that if we did we'd get an infinite irq
storm.

IWBNI the documentation for the pulse width bits in that reg was worded
a bit more clearly.  It's not completely clear whether the "short pulse
duration" part sets a minimum or maximum.  My reading interprets it as
minimum, but then one wonders what the long pulse duration is.

I'll play around with it a bit more.  If - as evidence seems to suggest
- we're really getting two HPD IRQs for each plug, then there's probably
a good reason for it which the code should reflect.

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/pch: Save/restore PCH_PORT_HOTPLUG across suspend

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 508 bytes --]

On Tue, 26 Jul 2011 14:00:32 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> We should be writing this reg.  The only question is whether we should
> be trusting the BIOS values (which may have custom pulse duration
> settings) or just unconditionally enabling hot plug for ports we care
> about with the default 2ms pulse width (per the DP spec).

I'll merge the suspend/resume save/restore stuff, if we decide to
customize the value, we can do that later.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx