diff mbox

drm/i915: Check live status before reading edid

Message ID 1436785500-26353-1-git-send-email-sonika.jindal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sonika.jindal@intel.com July 13, 2015, 11:05 a.m. UTC
Adding this for SKL onwards.

v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
to check digital port status. Adding a separate function to get bxt live
status (Daniel)

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
 drivers/gpu/drm/i915/intel_drv.h  |    2 ++
 drivers/gpu/drm/i915/intel_hdmi.c |   42 +++++++++++++++++++++++++++++++++----
 3 files changed, 42 insertions(+), 6 deletions(-)

Comments

Daniel Vetter July 13, 2015, 2:55 p.m. UTC | #1
On Mon, Jul 13, 2015 at 04:35:00PM +0530, Sonika Jindal wrote:
> Adding this for SKL onwards.
> 
> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
> to check digital port status. Adding a separate function to get bxt live
> status (Daniel)
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
>  drivers/gpu/drm/i915/intel_drv.h  |    2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c |   42 +++++++++++++++++++++++++++++++++----
>  3 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4ebfc3a..7b54b9d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>  	return intel_dp_detect_dpcd(intel_dp);
>  }
>  
> -static int g4x_digital_port_connected(struct drm_device *dev,
> -				       struct intel_digital_port *intel_dig_port)
> +int g4x_digital_port_connected(struct drm_device *dev,
> +			       struct intel_digital_port *intel_dig_port)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t bit;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a47fbc3..30c8dd6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
>  void intel_edp_drrs_invalidate(struct drm_device *dev,
>  		unsigned frontbuffer_bits);
>  void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
> +int g4x_digital_port_connected(struct drm_device *dev,
> +			       struct intel_digital_port *intel_dig_port);
>  
>  /* intel_dp_mst.c */
>  int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 1fb6919..7eb0d0e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1300,6 +1300,39 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>  	to_intel_connector(connector)->detect_edid = NULL;
>  }
>  
> +static bool bxt_port_connected(struct drm_i915_private *dev_priv,
> +			       struct intel_digital_port *port)
> +{
> +	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> +
> +	switch (port->port) {
> +	case PORT_B:
> +		return temp & BXT_DE_PORT_HP_DDIB;
> +
> +	case PORT_C:
> +		return temp & BXT_DE_PORT_HP_DDIC;
> +
> +	default:
> +		return false;
> +
> +	}
> +}
> +
> +static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
> +{
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (IS_VALLEYVIEW(dev))
> +		return g4x_digital_port_connected(dev, intel_dig_port);
> +	else if (IS_SKYLAKE(dev))
> +		return ibx_digital_port_connected(dev_priv, intel_dig_port);
> +	else if (IS_BROXTON(dev))
> +		return bxt_port_connected(dev_priv, intel_dig_port);

Really I want this to be rolled out for all platforms, together with the
fix for handling hpd interrupts. Together with a reference to the old
commits we had to revert because it didn't work.

I want to test this on my ivb (since that's the machine where I can
readily reproduce this bug), and if it still doesn't work there I won't
take this.
-Daniel
sonika.jindal@intel.com July 14, 2015, 4:46 a.m. UTC | #2
On 7/13/2015 8:25 PM, Daniel Vetter wrote:
> On Mon, Jul 13, 2015 at 04:35:00PM +0530, Sonika Jindal wrote:
>> Adding this for SKL onwards.
>>
>> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
>> to check digital port status. Adding a separate function to get bxt live
>> status (Daniel)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
>>   drivers/gpu/drm/i915/intel_drv.h  |    2 ++
>>   drivers/gpu/drm/i915/intel_hdmi.c |   42 +++++++++++++++++++++++++++++++++----
>>   3 files changed, 42 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 4ebfc3a..7b54b9d 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>>   	return intel_dp_detect_dpcd(intel_dp);
>>   }
>>
>> -static int g4x_digital_port_connected(struct drm_device *dev,
>> -				       struct intel_digital_port *intel_dig_port)
>> +int g4x_digital_port_connected(struct drm_device *dev,
>> +			       struct intel_digital_port *intel_dig_port)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	uint32_t bit;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index a47fbc3..30c8dd6 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
>>   void intel_edp_drrs_invalidate(struct drm_device *dev,
>>   		unsigned frontbuffer_bits);
>>   void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
>> +int g4x_digital_port_connected(struct drm_device *dev,
>> +			       struct intel_digital_port *intel_dig_port);
>>
>>   /* intel_dp_mst.c */
>>   int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 1fb6919..7eb0d0e 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1300,6 +1300,39 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>>   	to_intel_connector(connector)->detect_edid = NULL;
>>   }
>>
>> +static bool bxt_port_connected(struct drm_i915_private *dev_priv,
>> +			       struct intel_digital_port *port)
>> +{
>> +	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
>> +
>> +	switch (port->port) {
>> +	case PORT_B:
>> +		return temp & BXT_DE_PORT_HP_DDIB;
>> +
>> +	case PORT_C:
>> +		return temp & BXT_DE_PORT_HP_DDIC;
>> +
>> +	default:
>> +		return false;
>> +
>> +	}
>> +}
>> +
>> +static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
>> +{
>> +	struct drm_device *dev = intel_dig_port->base.base.dev;
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +
>> +	if (IS_VALLEYVIEW(dev))
>> +		return g4x_digital_port_connected(dev, intel_dig_port);
>> +	else if (IS_SKYLAKE(dev))
>> +		return ibx_digital_port_connected(dev_priv, intel_dig_port);
>> +	else if (IS_BROXTON(dev))
>> +		return bxt_port_connected(dev_priv, intel_dig_port);
>
> Really I want this to be rolled out for all platforms, together with the
> fix for handling hpd interrupts. Together with a reference to the old
> commits we had to revert because it didn't work.
>
> I want to test this on my ivb (since that's the machine where I can
> readily reproduce this bug), and if it still doesn't work there I won't
> take this.
> -Daniel
Is there a formal process to raise a test for hpd on all platforms which 
might be affected by this?

Regards,
Sonika
>
Daniel Vetter July 14, 2015, 7:55 a.m. UTC | #3
On Tue, Jul 14, 2015 at 10:16:17AM +0530, Jindal, Sonika wrote:
> 
> 
> On 7/13/2015 8:25 PM, Daniel Vetter wrote:
> >On Mon, Jul 13, 2015 at 04:35:00PM +0530, Sonika Jindal wrote:
> >>Adding this for SKL onwards.
> >>
> >>v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
> >>to check digital port status. Adding a separate function to get bxt live
> >>status (Daniel)
> >>
> >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
> >>  drivers/gpu/drm/i915/intel_drv.h  |    2 ++
> >>  drivers/gpu/drm/i915/intel_hdmi.c |   42 +++++++++++++++++++++++++++++++++----
> >>  3 files changed, 42 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>index 4ebfc3a..7b54b9d 100644
> >>--- a/drivers/gpu/drm/i915/intel_dp.c
> >>+++ b/drivers/gpu/drm/i915/intel_dp.c
> >>@@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
> >>  	return intel_dp_detect_dpcd(intel_dp);
> >>  }
> >>
> >>-static int g4x_digital_port_connected(struct drm_device *dev,
> >>-				       struct intel_digital_port *intel_dig_port)
> >>+int g4x_digital_port_connected(struct drm_device *dev,
> >>+			       struct intel_digital_port *intel_dig_port)
> >>  {
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>  	uint32_t bit;
> >>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>index a47fbc3..30c8dd6 100644
> >>--- a/drivers/gpu/drm/i915/intel_drv.h
> >>+++ b/drivers/gpu/drm/i915/intel_drv.h
> >>@@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
> >>  void intel_edp_drrs_invalidate(struct drm_device *dev,
> >>  		unsigned frontbuffer_bits);
> >>  void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
> >>+int g4x_digital_port_connected(struct drm_device *dev,
> >>+			       struct intel_digital_port *intel_dig_port);
> >>
> >>  /* intel_dp_mst.c */
> >>  int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> >>diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >>index 1fb6919..7eb0d0e 100644
> >>--- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>+++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>@@ -1300,6 +1300,39 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
> >>  	to_intel_connector(connector)->detect_edid = NULL;
> >>  }
> >>
> >>+static bool bxt_port_connected(struct drm_i915_private *dev_priv,
> >>+			       struct intel_digital_port *port)
> >>+{
> >>+	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> >>+
> >>+	switch (port->port) {
> >>+	case PORT_B:
> >>+		return temp & BXT_DE_PORT_HP_DDIB;
> >>+
> >>+	case PORT_C:
> >>+		return temp & BXT_DE_PORT_HP_DDIC;
> >>+
> >>+	default:
> >>+		return false;
> >>+
> >>+	}
> >>+}
> >>+
> >>+static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
> >>+{
> >>+	struct drm_device *dev = intel_dig_port->base.base.dev;
> >>+	struct drm_i915_private *dev_priv = to_i915(dev);
> >>+
> >>+	if (IS_VALLEYVIEW(dev))
> >>+		return g4x_digital_port_connected(dev, intel_dig_port);
> >>+	else if (IS_SKYLAKE(dev))
> >>+		return ibx_digital_port_connected(dev_priv, intel_dig_port);
> >>+	else if (IS_BROXTON(dev))
> >>+		return bxt_port_connected(dev_priv, intel_dig_port);
> >
> >Really I want this to be rolled out for all platforms, together with the
> >fix for handling hpd interrupts. Together with a reference to the old
> >commits we had to revert because it didn't work.
> >
> >I want to test this on my ivb (since that's the machine where I can
> >readily reproduce this bug), and if it still doesn't work there I won't
> >take this.
> >-Daniel
> Is there a formal process to raise a test for hpd on all platforms which
> might be affected by this?

Get it merged and wait for the regression reports to come in or not. The
entire problem I'm trying to explain here is that these hpd problems where
_not_ detected internally here at Intel, but only reported by external
people. Only later on did I come across a machine (by accident) which
seems to exhibit the same problem.

That's also why I want to enable it everywhere, increases the chances
we'll get a negative report if it doesn't work.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4ebfc3a..7b54b9d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4443,8 +4443,8 @@  ironlake_dp_detect(struct intel_dp *intel_dp)
 	return intel_dp_detect_dpcd(intel_dp);
 }
 
-static int g4x_digital_port_connected(struct drm_device *dev,
-				       struct intel_digital_port *intel_dig_port)
+int g4x_digital_port_connected(struct drm_device *dev,
+			       struct intel_digital_port *intel_dig_port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t bit;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a47fbc3..30c8dd6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1187,6 +1187,8 @@  void intel_edp_drrs_disable(struct intel_dp *intel_dp);
 void intel_edp_drrs_invalidate(struct drm_device *dev,
 		unsigned frontbuffer_bits);
 void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
+int g4x_digital_port_connected(struct drm_device *dev,
+			       struct intel_digital_port *intel_dig_port);
 
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 1fb6919..7eb0d0e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1300,6 +1300,39 @@  intel_hdmi_unset_edid(struct drm_connector *connector)
 	to_intel_connector(connector)->detect_edid = NULL;
 }
 
+static bool bxt_port_connected(struct drm_i915_private *dev_priv,
+			       struct intel_digital_port *port)
+{
+	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
+
+	switch (port->port) {
+	case PORT_B:
+		return temp & BXT_DE_PORT_HP_DDIB;
+
+	case PORT_C:
+		return temp & BXT_DE_PORT_HP_DDIC;
+
+	default:
+		return false;
+
+	}
+}
+
+static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
+{
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (IS_VALLEYVIEW(dev))
+		return g4x_digital_port_connected(dev, intel_dig_port);
+	else if (IS_SKYLAKE(dev))
+		return ibx_digital_port_connected(dev_priv, intel_dig_port);
+	else if (IS_BROXTON(dev))
+		return bxt_port_connected(dev_priv, intel_dig_port);
+
+	return true;
+}
+
 static bool
 intel_hdmi_set_edid(struct drm_connector *connector)
 {
@@ -1308,15 +1341,16 @@  intel_hdmi_set_edid(struct drm_connector *connector)
 	struct intel_encoder *intel_encoder =
 		&hdmi_to_dig_port(intel_hdmi)->base;
 	enum intel_display_power_domain power_domain;
-	struct edid *edid;
+	struct edid *edid = NULL;
 	bool connected = false;
 
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
-	edid = drm_get_edid(connector,
-			    intel_gmbus_get_adapter(dev_priv,
-						    intel_hdmi->ddc_bus));
+	if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
+		edid = drm_get_edid(connector,
+				intel_gmbus_get_adapter(dev_priv,
+					intel_hdmi->ddc_bus));
 
 	intel_display_power_put(dev_priv, power_domain);