* [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions
@ 2012-12-12 19:50 Damien Lespiau
2012-12-12 19:50 ` [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID Damien Lespiau
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Damien Lespiau @ 2012-12-12 19:50 UTC (permalink / raw)
To: intel-gfx
From: Damien Lespiau <damien.lespiau@intel.com>
Those status bits don't follow the usual pattern: _MASK (those bits are
write 1 to clear, useful to select the value we want to read) and the
values shifted by the same amount.
Cleaned that that up when poking at the register for testing purposes,
might as well upstream that cleanup.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f2a5ea6..f834804 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3558,27 +3558,30 @@
#define PORTD_PULSE_DURATION_6ms (2 << 18)
#define PORTD_PULSE_DURATION_100ms (3 << 18)
#define PORTD_PULSE_DURATION_MASK (3 << 18)
-#define PORTD_HOTPLUG_NO_DETECT (0)
-#define PORTD_HOTPLUG_SHORT_DETECT (1 << 16)
-#define PORTD_HOTPLUG_LONG_DETECT (1 << 17)
+#define PORTD_HOTPLUG_STATUS_MASK (0x3 << 16)
+#define PORTD_HOTPLUG_NO_DETECT (0 << 16)
+#define PORTD_HOTPLUG_SHORT_DETECT (1 << 16)
+#define PORTD_HOTPLUG_LONG_DETECT (2 << 16)
#define PORTC_HOTPLUG_ENABLE (1 << 12)
#define PORTC_PULSE_DURATION_2ms (0)
#define PORTC_PULSE_DURATION_4_5ms (1 << 10)
#define PORTC_PULSE_DURATION_6ms (2 << 10)
#define PORTC_PULSE_DURATION_100ms (3 << 10)
#define PORTC_PULSE_DURATION_MASK (3 << 10)
-#define PORTC_HOTPLUG_NO_DETECT (0)
-#define PORTC_HOTPLUG_SHORT_DETECT (1 << 8)
-#define PORTC_HOTPLUG_LONG_DETECT (1 << 9)
+#define PORTC_HOTPLUG_STATUS_MASK (0x3 << 8)
+#define PORTC_HOTPLUG_NO_DETECT (0 << 8)
+#define PORTC_HOTPLUG_SHORT_DETECT (1 << 8)
+#define PORTC_HOTPLUG_LONG_DETECT (2 << 8)
#define PORTB_HOTPLUG_ENABLE (1 << 4)
#define PORTB_PULSE_DURATION_2ms (0)
#define PORTB_PULSE_DURATION_4_5ms (1 << 2)
#define PORTB_PULSE_DURATION_6ms (2 << 2)
#define PORTB_PULSE_DURATION_100ms (3 << 2)
#define PORTB_PULSE_DURATION_MASK (3 << 2)
-#define PORTB_HOTPLUG_NO_DETECT (0)
-#define PORTB_HOTPLUG_SHORT_DETECT (1 << 0)
-#define PORTB_HOTPLUG_LONG_DETECT (1 << 1)
+#define PORTB_HOTPLUG_STATUS_MASK (0x3 << 0)
+#define PORTB_HOTPLUG_NO_DETECT (0 << 0)
+#define PORTB_HOTPLUG_SHORT_DETECT (1 << 0)
+#define PORTB_HOTPLUG_LONG_DETECT (2 << 0)
#define PCH_GPIOA 0xc5010
#define PCH_GPIOB 0xc5014
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID
2012-12-12 19:50 [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions Damien Lespiau
@ 2012-12-12 19:50 ` Damien Lespiau
2012-12-13 8:40 ` Chris Wilson
2012-12-13 11:11 ` Jani Nikula
2012-12-12 19:50 ` [PATCH 3/4] drm/i915/dp: Read the HPD status before trying to read the DPCD Damien Lespiau
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Damien Lespiau @ 2012-12-12 19:50 UTC (permalink / raw)
To: intel-gfx
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;
+
+ 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;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] drm/i915/dp: Read the HPD status before trying to read the DPCD
2012-12-12 19:50 [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions Damien Lespiau
2012-12-12 19:50 ` [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID Damien Lespiau
@ 2012-12-12 19:50 ` Damien Lespiau
2012-12-13 11:16 ` Jani Nikula
2012-12-12 19:50 ` [PATCH 4/4] drm/i915/dp: Don't log the DPCD if we are disconnected Damien Lespiau
2012-12-13 8:39 ` [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions Jani Nikula
3 siblings, 1 reply; 9+ messages in thread
From: Damien Lespiau @ 2012-12-12 19:50 UTC (permalink / raw)
To: intel-gfx
From: Damien Lespiau <damien.lespiau@intel.com>
Just like:
commit a93cd34234db2269fa2481464ffd39263d617aed
Author: Damien Lespiau <damien.lespiau@intel.com>
Date: Wed Dec 12 19:37:22 2012 +0000
drm/i915/hdmi: Read the HPD status before trying to read the EDID
But this time for DiplayPort.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b2130bc..fe3c22f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2248,6 +2248,8 @@ static enum drm_connector_status
ironlake_dp_detect(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
enum drm_connector_status status;
/* Can't disconnect eDP, but you can close the lid... */
@@ -2258,6 +2260,9 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
return status;
}
+ if (!intel_ironlake_digital_port_connected(dev_priv, intel_dig_port))
+ return connector_status_disconnected;
+
return intel_dp_detect_dpcd(intel_dp);
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] drm/i915/dp: Don't log the DPCD if we are disconnected
2012-12-12 19:50 [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions Damien Lespiau
2012-12-12 19:50 ` [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID Damien Lespiau
2012-12-12 19:50 ` [PATCH 3/4] drm/i915/dp: Read the HPD status before trying to read the DPCD Damien Lespiau
@ 2012-12-12 19:50 ` Damien Lespiau
2012-12-13 8:33 ` Jani Nikula
2012-12-13 8:39 ` [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions Jani Nikula
3 siblings, 1 reply; 9+ messages in thread
From: Damien Lespiau @ 2012-12-12 19:50 UTC (permalink / raw)
To: intel-gfx
From: Damien Lespiau <damien.lespiau@intel.com>
It's a bit useless to print out an all null DPCD when we are
disconnected and just clutter the debug logs.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fe3c22f..42058fa 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2362,13 +2362,13 @@ intel_dp_detect(struct drm_connector *connector, bool force)
else
status = g4x_dp_detect(intel_dp);
+ if (status != connector_status_connected)
+ return status;
+
hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),
32, 1, dpcd_hex_dump, sizeof(dpcd_hex_dump), false);
DRM_DEBUG_KMS("DPCD: %s\n", dpcd_hex_dump);
- if (status != connector_status_connected)
- return status;
-
intel_dp_probe_oui(intel_dp);
if (intel_dp->force_audio != HDMI_AUDIO_AUTO) {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] drm/i915/dp: Don't log the DPCD if we are disconnected
2012-12-12 19:50 ` [PATCH 4/4] drm/i915/dp: Don't log the DPCD if we are disconnected Damien Lespiau
@ 2012-12-13 8:33 ` Jani Nikula
0 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2012-12-13 8:33 UTC (permalink / raw)
To: Damien Lespiau, intel-gfx
On Wed, 12 Dec 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> It's a bit useless to print out an all null DPCD when we are
> disconnected and just clutter the debug logs.
NAK.
Please have a look at intel_dp_detect_dpcd(). There are a number of ways
to get a non-zero DPCD that lead to something other than
connector_status_connected. The comment says, "XXX this is probably
wrong for multiple downstream ports", so there's also some work to do in
the area. IMO we need the debugs.
Even intel_dp_get_dpcd() has one branch that returns false when we have
non-zero DPCD. But if we ignored that part (it's an aux channel error
anyway), we could move the debug dump to intel_dp_detect_dpcd() right
after the intel_dp_get_dpcd() call. I think that would get maximum debug
coverage with minimum zero DPCD noise.
What do you think?
BR,
Jani.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fe3c22f..42058fa 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2362,13 +2362,13 @@ intel_dp_detect(struct drm_connector *connector, bool force)
> else
> status = g4x_dp_detect(intel_dp);
>
> + if (status != connector_status_connected)
> + return status;
> +
> hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),
> 32, 1, dpcd_hex_dump, sizeof(dpcd_hex_dump), false);
> DRM_DEBUG_KMS("DPCD: %s\n", dpcd_hex_dump);
>
> - if (status != connector_status_connected)
> - return status;
> -
> intel_dp_probe_oui(intel_dp);
>
> if (intel_dp->force_audio != HDMI_AUDIO_AUTO) {
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions
2012-12-12 19:50 [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions Damien Lespiau
` (2 preceding siblings ...)
2012-12-12 19:50 ` [PATCH 4/4] drm/i915/dp: Don't log the DPCD if we are disconnected Damien Lespiau
@ 2012-12-13 8:39 ` Jani Nikula
3 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2012-12-13 8:39 UTC (permalink / raw)
To: Damien Lespiau, intel-gfx
On Wed, 12 Dec 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> Those status bits don't follow the usual pattern: _MASK (those bits are
> write 1 to clear, useful to select the value we want to read) and the
> values shifted by the same amount.
>
> Cleaned that that up when poking at the register for testing purposes,
> might as well upstream that cleanup.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f2a5ea6..f834804 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3558,27 +3558,30 @@
> #define PORTD_PULSE_DURATION_6ms (2 << 18)
> #define PORTD_PULSE_DURATION_100ms (3 << 18)
> #define PORTD_PULSE_DURATION_MASK (3 << 18)
> -#define PORTD_HOTPLUG_NO_DETECT (0)
> -#define PORTD_HOTPLUG_SHORT_DETECT (1 << 16)
> -#define PORTD_HOTPLUG_LONG_DETECT (1 << 17)
> +#define PORTD_HOTPLUG_STATUS_MASK (0x3 << 16)
> +#define PORTD_HOTPLUG_NO_DETECT (0 << 16)
> +#define PORTD_HOTPLUG_SHORT_DETECT (1 << 16)
> +#define PORTD_HOTPLUG_LONG_DETECT (2 << 16)
> #define PORTC_HOTPLUG_ENABLE (1 << 12)
> #define PORTC_PULSE_DURATION_2ms (0)
> #define PORTC_PULSE_DURATION_4_5ms (1 << 10)
> #define PORTC_PULSE_DURATION_6ms (2 << 10)
> #define PORTC_PULSE_DURATION_100ms (3 << 10)
> #define PORTC_PULSE_DURATION_MASK (3 << 10)
> -#define PORTC_HOTPLUG_NO_DETECT (0)
> -#define PORTC_HOTPLUG_SHORT_DETECT (1 << 8)
> -#define PORTC_HOTPLUG_LONG_DETECT (1 << 9)
> +#define PORTC_HOTPLUG_STATUS_MASK (0x3 << 8)
> +#define PORTC_HOTPLUG_NO_DETECT (0 << 8)
> +#define PORTC_HOTPLUG_SHORT_DETECT (1 << 8)
> +#define PORTC_HOTPLUG_LONG_DETECT (2 << 8)
> #define PORTB_HOTPLUG_ENABLE (1 << 4)
> #define PORTB_PULSE_DURATION_2ms (0)
> #define PORTB_PULSE_DURATION_4_5ms (1 << 2)
> #define PORTB_PULSE_DURATION_6ms (2 << 2)
> #define PORTB_PULSE_DURATION_100ms (3 << 2)
> #define PORTB_PULSE_DURATION_MASK (3 << 2)
> -#define PORTB_HOTPLUG_NO_DETECT (0)
> -#define PORTB_HOTPLUG_SHORT_DETECT (1 << 0)
> -#define PORTB_HOTPLUG_LONG_DETECT (1 << 1)
> +#define PORTB_HOTPLUG_STATUS_MASK (0x3 << 0)
> +#define PORTB_HOTPLUG_NO_DETECT (0 << 0)
> +#define PORTB_HOTPLUG_SHORT_DETECT (1 << 0)
> +#define PORTB_HOTPLUG_LONG_DETECT (2 << 0)
>
> #define PCH_GPIOA 0xc5010
> #define PCH_GPIOB 0xc5014
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID
2012-12-12 19:50 ` [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID Damien Lespiau
@ 2012-12-13 8:40 ` Chris Wilson
2012-12-13 11:11 ` Jani Nikula
1 sibling, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2012-12-13 8:40 UTC (permalink / raw)
To: Damien Lespiau, intel-gfx
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
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID
2012-12-12 19:50 ` [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID Damien Lespiau
2012-12-13 8:40 ` Chris Wilson
@ 2012-12-13 11:11 ` Jani Nikula
1 sibling, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2012-12-13 11:11 UTC (permalink / raw)
To: Damien Lespiau, intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] drm/i915/dp: Read the HPD status before trying to read the DPCD
2012-12-12 19:50 ` [PATCH 3/4] drm/i915/dp: Read the HPD status before trying to read the DPCD Damien Lespiau
@ 2012-12-13 11:16 ` Jani Nikula
0 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2012-12-13 11:16 UTC (permalink / raw)
To: Damien Lespiau, intel-gfx
On Wed, 12 Dec 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> Just like:
>
> commit a93cd34234db2269fa2481464ffd39263d617aed
This is your local commit id, and will be meaningless upstream.
Otherwise,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Author: Damien Lespiau <damien.lespiau@intel.com>
> Date: Wed Dec 12 19:37:22 2012 +0000
>
> drm/i915/hdmi: Read the HPD status before trying to read the EDID
>
> But this time for DiplayPort.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b2130bc..fe3c22f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2248,6 +2248,8 @@ static enum drm_connector_status
> ironlake_dp_detect(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> enum drm_connector_status status;
>
> /* Can't disconnect eDP, but you can close the lid... */
> @@ -2258,6 +2260,9 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
> return status;
> }
>
> + if (!intel_ironlake_digital_port_connected(dev_priv, intel_dig_port))
> + return connector_status_disconnected;
> +
> return intel_dp_detect_dpcd(intel_dp);
> }
>
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-12-13 11:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-12 19:50 [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions Damien Lespiau
2012-12-12 19:50 ` [PATCH 2/4] drm/i915/hdmi: Read the HPD status before trying to read the EDID Damien Lespiau
2012-12-13 8:40 ` Chris Wilson
2012-12-13 11:11 ` Jani Nikula
2012-12-12 19:50 ` [PATCH 3/4] drm/i915/dp: Read the HPD status before trying to read the DPCD Damien Lespiau
2012-12-13 11:16 ` Jani Nikula
2012-12-12 19:50 ` [PATCH 4/4] drm/i915/dp: Don't log the DPCD if we are disconnected Damien Lespiau
2012-12-13 8:33 ` Jani Nikula
2012-12-13 8:39 ` [PATCH 1/4] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions Jani Nikula
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.