[Intel-gfx] [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID
Jani Nikula
jani.nikula at linux.intel.com
Thu Dec 13 12:11:24 CET 2012
On Wed, 12 Dec 2012, Damien Lespiau <damien.lespiau at gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list