| Message ID | 1311713586-23557-1-git-send-email-ajax@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
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.
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);
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(-)