[PATCH 1/6] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions

[PATCH 1/6] drm/i915: Cleanup SHOTPLUG_CTL status bits definitions

[Damien Lespiau]

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

[PATCH 2/6] drm/i915/hdmi: Read the HPD status before trying to read the EDID

[Damien Lespiau]

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.

v2: Rename the function to ibx_ (Chris Wilson)

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    |  9 +++++++--
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 115bf62..21fedf7 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)
 	}
 }
 
+/*
+ * ibx_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 ibx_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..53d4c8f 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 ibx_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..9f834d3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -793,16 +793,21 @@ 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) &&
+		 !ibx_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

Re: [PATCH 2/6] drm/i915/hdmi: Read the HPD status before trying to read the EDID

[Jani Nikula]

On Thu, 13 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.
>
> v2: Rename the function to ibx_ (Chris Wilson)

The name's a bit funny in the intermediate step because the patch
doesn't really do anything for IBX before patch 5/6... :) I don't mind,
just an observation, and perhaps Daniel will fold the two together once
we get tested-by on ILK+IBX.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> 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    |  9 +++++++--
>  3 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 115bf62..21fedf7 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)
>  	}
>  }
>  
> +/*
> + * ibx_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 ibx_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..53d4c8f 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 ibx_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..9f834d3 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -793,16 +793,21 @@ 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) &&
> +		 !ibx_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

[PATCH 3/6] drm/i915/dp: Read the HPD status before trying to read the DPCD

[Damien Lespiau]

From: Damien Lespiau <damien.lespiau@intel.com>

Just like:

  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.

v2: Adapt to the ibx_ name change and don't add commit hash (Chris
Wilson, Jani Nikula)

Reviewed-by: Jani Nikula <jani.nikula@intel.com>
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..19a0d89 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 (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+		return connector_status_disconnected;
+
 	return intel_dp_detect_dpcd(intel_dp);
 }
 
-- 
1.7.11.7

Re: [PATCH 3/6] drm/i915/dp: Read the HPD status before trying to read the DPCD

[Daniel Vetter]

On Thu, Dec 13, 2012 at 04:09:01PM +0000, Damien Lespiau wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> Just like:
> 
>   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.
> 
> v2: Adapt to the ibx_ name change and don't add commit hash (Chris
> Wilson, Jani Nikula)
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 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..19a0d89 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 (!ibx_digital_port_connected(dev_priv, intel_dig_port))
> +		return connector_status_disconnected;

Can I volunteer you to do the same nice unification with
g4x_digital_port_connected? Despite the different names between hdmi and
dp, they're the same bits. I have a g4x here, so can test whether it
breaks.
-Daniel

> +
>  	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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 4/6] drm/i915/dp: Log the DPCD only if we have successfully retrieved one

[Damien Lespiau]

From: Damien Lespiau <damien.lespiau@intel.com>

Moving the DPCD just after a successful read will allow to:
  - log all DPCD reads (eDP ones, changes signalled by HPD IRQ)
  - don't log it if we haven't been able to read it

v2: Be sure to log the DPCD when a downstream port does not have HPD
    support and the branch device asserts HPD (Jani Nikula)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 19a0d89..5ed8fb3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2084,10 +2084,16 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 static bool
 intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
+	char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
+
 	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
 					   sizeof(intel_dp->dpcd)) == 0)
 		return false; /* aux transfer failed */
 
+	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 (intel_dp->dpcd[DP_DPCD_REV] == 0)
 		return false; /* DPCD not present */
 
@@ -2353,7 +2359,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	struct drm_device *dev = connector->dev;
 	enum drm_connector_status status;
 	struct edid *edid = NULL;
-	char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
 
 	intel_dp->has_audio = false;
 
@@ -2362,10 +2367,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	else
 		status = g4x_dp_detect(intel_dp);
 
-	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;
 
-- 
1.7.11.7

Re: [PATCH 4/6] drm/i915/dp: Log the DPCD only if we have successfully retrieved one

[Jani Nikula]

On Thu, 13 Dec 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> Moving the DPCD just after a successful read will allow to:
>   - log all DPCD reads (eDP ones, changes signalled by HPD IRQ)
>   - don't log it if we haven't been able to read it

The first part does actually increase DPCD logging a bit, but with the
second part this should result in a nice reduction of dmesg noise. /me
likes.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> v2: Be sure to log the DPCD when a downstream port does not have HPD
>     support and the branch device asserts HPD (Jani Nikula)
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 19a0d89..5ed8fb3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2084,10 +2084,16 @@ intel_dp_link_down(struct intel_dp *intel_dp)
>  static bool
>  intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  {
> +	char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
> +
>  	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
>  					   sizeof(intel_dp->dpcd)) == 0)
>  		return false; /* aux transfer failed */
>  
> +	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 (intel_dp->dpcd[DP_DPCD_REV] == 0)
>  		return false; /* DPCD not present */
>  
> @@ -2353,7 +2359,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>  	struct drm_device *dev = connector->dev;
>  	enum drm_connector_status status;
>  	struct edid *edid = NULL;
> -	char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
>  
>  	intel_dp->has_audio = false;
>  
> @@ -2362,10 +2367,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>  	else
>  		status = g4x_dp_detect(intel_dp);
>  
> -	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;
>  
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/6] drm/i915: Implement ibx_digital_port_connected() for IBX

[Damien Lespiau]

From: Damien Lespiau <damien.lespiau@intel.com>

CPT+ PCHs have different bit definition to read the HPD live status. I
don't have an ILK with digital ports handy, which is why this patch is
separate from the CPT+ implementation. If the docs don't lie, it should
all be fine though.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 44 +++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 21fedf7..10759ea 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1006,22 +1006,34 @@ bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
 {
 	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;
+	if (HAS_PCH_IBX(dev_priv->dev)) {
+		switch(port->port) {
+		case PORT_B:
+			bit = SDE_PORTB_HOTPLUG;
+			break;
+		case PORT_C:
+			bit = SDE_PORTC_HOTPLUG;
+			break;
+		case PORT_D:
+			bit = SDE_PORTD_HOTPLUG;
+			break;
+		default:
+			return true;
+		}
+	} else {
+		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;
-- 
1.7.11.7

Re: [PATCH 5/6] drm/i915: Implement ibx_digital_port_connected() for IBX

[Jani Nikula]

On Thu, 13 Dec 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> CPT+ PCHs have different bit definition to read the HPD live status. I
> don't have an ILK with digital ports handy, which is why this patch is
> separate from the CPT+ implementation. If the docs don't lie, it should
> all be fine though.

As said, could be squashed into 2/6, but I understand your reason to
keep it separate.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 44 +++++++++++++++++++++++-------------
>  1 file changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 21fedf7..10759ea 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1006,22 +1006,34 @@ bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>  {
>  	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;
> +	if (HAS_PCH_IBX(dev_priv->dev)) {
> +		switch(port->port) {
> +		case PORT_B:
> +			bit = SDE_PORTB_HOTPLUG;
> +			break;
> +		case PORT_C:
> +			bit = SDE_PORTC_HOTPLUG;
> +			break;
> +		case PORT_D:
> +			bit = SDE_PORTD_HOTPLUG;
> +			break;
> +		default:
> +			return true;
> +		}
> +	} else {
> +		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;
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/6] drm/i915: Remove stale comment about intel_dp_detect()

[Damien Lespiau]

From: Damien Lespiau <damien.lespiau@intel.com>

The function doesn't use any of the registers mentioned, nor does it
return true or false. Hard to do worse. Remove it, the function is
absolutely descriptive enough to not need any comment.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5ed8fb3..185bf4e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2343,13 +2343,6 @@ intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *ada
 	return intel_ddc_get_modes(connector, adapter);
 }
 
-
-/**
- * Uses CRT_HOTPLUG_EN and CRT_HOTPLUG_STAT to detect DP connection.
- *
- * \return true if DP port is connected.
- * \return false if DP port is disconnected.
- */
 static enum drm_connector_status
 intel_dp_detect(struct drm_connector *connector, bool force)
 {
-- 
1.7.11.7

Re: [PATCH 6/6] drm/i915: Remove stale comment about intel_dp_detect()

[Jani Nikula]

On Thu, 13 Dec 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> The function doesn't use any of the registers mentioned, nor does it
> return true or false. Hard to do worse. Remove it, the function is
> absolutely descriptive enough to not need any comment.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5ed8fb3..185bf4e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2343,13 +2343,6 @@ intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *ada
>  	return intel_ddc_get_modes(connector, adapter);
>  }
>  
> -
> -/**
> - * Uses CRT_HOTPLUG_EN and CRT_HOTPLUG_STAT to detect DP connection.
> - *
> - * \return true if DP port is connected.
> - * \return false if DP port is disconnected.
> - */
>  static enum drm_connector_status
>  intel_dp_detect(struct drm_connector *connector, bool force)
>  {
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Remove stale comment about intel_dp_detect()

[Daniel Vetter]

On Fri, Dec 14, 2012 at 11:46:20AM +0200, Jani Nikula wrote:
> On Thu, 13 Dec 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> > From: Damien Lespiau <damien.lespiau@intel.com>
> >
> > The function doesn't use any of the registers mentioned, nor does it
> > return true or false. Hard to do worse. Remove it, the function is
> > absolutely descriptive enough to not need any comment.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Entire series queued for -next, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch