| Message ID | 1355341841-4595-2-git-send-email-damien.lespiau@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 12 Dec 2012 19:50:39 +0000, Damien Lespiau <damien.lespiau@gmail.com> wrote: > From: Damien Lespiau <damien.lespiau@intel.com> > > If you unplug the hdmi connector slowly enough, the hotplug interrupt > fires but then the kernel code tries to read the EDID and succeeds > (because the connector is still half connected, the HPD pin is shorter > than the others, and DDC works). Since EDID succeeds it thinks the > monitor is still connected. > > To prevent that, read the live HPD status in the hotplug handler before > trying to read the EDID. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55372 > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 33 +++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_hdmi.c | 10 ++++++++-- > 3 files changed, 44 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 115bf62..117d9e6 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -994,6 +994,39 @@ void intel_wait_for_pipe_off(struct drm_device *dev, int pipe) > } > } > > +/* > + * intel_ironlake_digital_port_connected - is the specified port connected? > + * @dev_priv: i915 private structure > + * @port: the port to test > + * > + * Returns true if @port is connected, false otherwise. > + */ > +bool intel_ironlake_digital_port_connected(struct drm_i915_private *dev_priv, > + struct intel_digital_port *port) intel_ironlake_digital_port_connected() is a horrible bastard of a naming scheme. ibx_digital_port_connected() please. ^intel_ -> applies to (nearly-)all display generations ^ilk_/ironlake_ -> applies to ironlake and maybe subsequent generations In this case since we are PCH specific, using the PCH chipset identifiers makes more sense. -Chris
On Wed, 12 Dec 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote: > From: Damien Lespiau <damien.lespiau@intel.com> > > If you unplug the hdmi connector slowly enough, the hotplug interrupt > fires but then the kernel code tries to read the EDID and succeeds > (because the connector is still half connected, the HPD pin is shorter > than the others, and DDC works). Since EDID succeeds it thinks the > monitor is still connected. > > To prevent that, read the live HPD status in the hotplug handler before > trying to read the EDID. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55372 > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 33 +++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_hdmi.c | 10 ++++++++-- > 3 files changed, 44 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 115bf62..117d9e6 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -994,6 +994,39 @@ void intel_wait_for_pipe_off(struct drm_device *dev, int pipe) > } > } > > +/* > + * intel_ironlake_digital_port_connected - is the specified port connected? > + * @dev_priv: i915 private structure > + * @port: the port to test > + * > + * Returns true if @port is connected, false otherwise. > + */ > +bool intel_ironlake_digital_port_connected(struct drm_i915_private *dev_priv, > + struct intel_digital_port *port) > +{ > + u32 bit; > + > + /* XXX: IBX has different SDEISR bits */ > + if (HAS_PCH_IBX(dev_priv->dev)) > + return true; They are: #define SDE_PORTD_HOTPLUG (1 << 10) #define SDE_PORTC_HOTPLUG (1 << 9) #define SDE_PORTB_HOTPLUG (1 << 8) Any reason not to add those now? > + > + switch(port->port) { > + case PORT_B: > + bit = SDE_PORTB_HOTPLUG_CPT; > + break; > + case PORT_C: > + bit = SDE_PORTC_HOTPLUG_CPT; > + break; > + case PORT_D: > + bit = SDE_PORTD_HOTPLUG_CPT; > + break; > + default: > + return true; > + } > + > + return I915_READ(SDEISR) & bit; The wording in the SDEISR register spec isn't exactly convincing, but after reading it a few times along with the bit definitions, I think I agree with you. I hope the hardware agrees with us too. :) BR, Jani. > +} > + > static const char *state_string(bool enabled) > { > return enabled ? "on" : "off"; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 22728f2..0e069d8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -545,6 +545,9 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi) > return container_of(intel_hdmi, struct intel_digital_port, hdmi); > } > > +bool intel_ironlake_digital_port_connected(struct drm_i915_private *dev_priv, > + struct intel_digital_port *port); > + > extern void intel_connector_attach_encoder(struct intel_connector *connector, > struct intel_encoder *encoder); > extern struct drm_encoder *intel_best_encoder(struct drm_connector *connector); > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 53df0a8..966efef5 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -793,16 +793,22 @@ static bool g4x_hdmi_connected(struct intel_hdmi *intel_hdmi) > static enum drm_connector_status > intel_hdmi_detect(struct drm_connector *connector, bool force) > { > + struct drm_device *dev = connector->dev; > struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); > struct intel_digital_port *intel_dig_port = > hdmi_to_dig_port(intel_hdmi); > struct intel_encoder *intel_encoder = &intel_dig_port->base; > - struct drm_i915_private *dev_priv = connector->dev->dev_private; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct edid *edid; > enum drm_connector_status status = connector_status_disconnected; > > - if (IS_G4X(connector->dev) && !g4x_hdmi_connected(intel_hdmi)) > + > + if (IS_G4X(dev) && !g4x_hdmi_connected(intel_hdmi)) > return status; > + else if (HAS_PCH_SPLIT(dev) && > + !intel_ironlake_digital_port_connected(dev_priv, > + intel_dig_port)) > + return status; > > intel_hdmi->has_hdmi_sink = false; > intel_hdmi->has_audio = false; > -- > 1.7.11.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 115bf62..117d9e6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -994,6 +994,39 @@ void intel_wait_for_pipe_off(struct drm_device *dev, int pipe) } } +/* + * intel_ironlake_digital_port_connected - is the specified port connected? + * @dev_priv: i915 private structure + * @port: the port to test + * + * Returns true if @port is connected, false otherwise. + */ +bool intel_ironlake_digital_port_connected(struct drm_i915_private *dev_priv, + struct intel_digital_port *port) +{ + u32 bit; + + /* XXX: IBX has different SDEISR bits */ + if (HAS_PCH_IBX(dev_priv->dev)) + return true; + + switch(port->port) { + case PORT_B: + bit = SDE_PORTB_HOTPLUG_CPT; + break; + case PORT_C: + bit = SDE_PORTC_HOTPLUG_CPT; + break; + case PORT_D: + bit = SDE_PORTD_HOTPLUG_CPT; + break; + default: + return true; + } + + return I915_READ(SDEISR) & bit; +} + static const char *state_string(bool enabled) { return enabled ? "on" : "off"; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 22728f2..0e069d8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -545,6 +545,9 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi) return container_of(intel_hdmi, struct intel_digital_port, hdmi); } +bool intel_ironlake_digital_port_connected(struct drm_i915_private *dev_priv, + struct intel_digital_port *port); + extern void intel_connector_attach_encoder(struct intel_connector *connector, struct intel_encoder *encoder); extern struct drm_encoder *intel_best_encoder(struct drm_connector *connector); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 53df0a8..966efef5 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -793,16 +793,22 @@ static bool g4x_hdmi_connected(struct intel_hdmi *intel_hdmi) static enum drm_connector_status intel_hdmi_detect(struct drm_connector *connector, bool force) { + struct drm_device *dev = connector->dev; struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); struct intel_digital_port *intel_dig_port = hdmi_to_dig_port(intel_hdmi); struct intel_encoder *intel_encoder = &intel_dig_port->base; - struct drm_i915_private *dev_priv = connector->dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct edid *edid; enum drm_connector_status status = connector_status_disconnected; - if (IS_G4X(connector->dev) && !g4x_hdmi_connected(intel_hdmi)) + + if (IS_G4X(dev) && !g4x_hdmi_connected(intel_hdmi)) return status; + else if (HAS_PCH_SPLIT(dev) && + !intel_ironlake_digital_port_connected(dev_priv, + intel_dig_port)) + return status; intel_hdmi->has_hdmi_sink = false; intel_hdmi->has_audio = false;