[PATCH 0/6] Fixing sink count related detection over

[PATCH 0/6] Fixing sink count related detection over

[Shubhangi Shrivastava]

This patch set cleans up DP detection logic to bring all DPCD
operations at one place and to create a clear demarcation
between handling of long and short pulses. This simplifies
fixing of sink count related detection for DP panels.

Patches:
1. First two patches clean up intel_dp_detect and form a new
function which will include all DPCD related operations.
2. Second patch splits up intel_dp_check_link_status to form
a new function which will handle short pulse requests.
3. Last three patches fixes the detection logic related to
sink count i.e detect changes in sink count and handle them
appropriately. 

Note: this is tested on BXT with non-mst panels,
will get back ASAP with results for MST panels too. 

Shubhangi Shrivastava (6):
  drm/i915: Splitting intel_dp_detect
  drm/i915: Cleaning up intel_dp_hpd_pulse
  drm/i915: Splitting intel_dp_check_link_status
  drm/i915: Save sink_count for tracking changes to it
  drm/i915: read sink_count dpcd always
  drm/i915: force full detect on sink count change

 drivers/gpu/drm/i915/intel_dp.c  | 170 +++++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h |   1 +
 2 files changed, 110 insertions(+), 61 deletions(-)

-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/6] drm/i915: Splitting intel_dp_detect

[Shubhangi Shrivastava]

This patch moves probing for panel, DPCD read etc to another
function intel_dp_long_pulse, while intel_dp_detect returns
the status as connected or disconnected depending on
whether the edid is available or not.
This change will be required by further patches in the series
to avoid performing multiple DPCD operations and removing
their duplication.

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 66 ++++++++++++++++++++++++++++++-----------
 1 file changed, 49 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1cb1f3f..a0fe827 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4785,9 +4785,10 @@ intel_dp_power_put(struct intel_dp *dp,
 	intel_display_power_put(to_i915(encoder->base.dev), power_domain);
 }
 
-static enum drm_connector_status
-intel_dp_detect(struct drm_connector *connector, bool force)
+static void
+intel_dp_long_pulse(struct intel_connector *intel_connector)
 {
+	struct drm_connector *connector = &intel_connector->base;
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
@@ -4797,17 +4798,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	bool ret;
 	u8 sink_irq_vector;
 
-	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
-		      connector->base.id, connector->name);
-	intel_dp_unset_edid(intel_dp);
-
-	if (intel_dp->is_mst) {
-		/* MST devices are disconnected from a monitor POV */
-		if (intel_encoder->type != INTEL_OUTPUT_EDP)
-			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
-		return connector_status_disconnected;
-	}
-
 	power_domain = intel_dp_power_get(intel_dp);
 
 	/* Can't disconnect eDP, but you can close the lid... */
@@ -4817,19 +4807,35 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		status = ironlake_dp_detect(intel_dp);
 	else
 		status = g4x_dp_detect(intel_dp);
-	if (status != connector_status_connected)
+	if (status != connector_status_connected) {
+		intel_dp_unset_edid(intel_dp);
 		goto out;
+	}
 
 	intel_dp_probe_oui(intel_dp);
 
 	ret = intel_dp_probe_mst(intel_dp);
 	if (ret) {
-		/* if we are in MST mode then this connector
-		   won't appear connected or have anything with EDID on it */
+		/*
+		 * if we are in MST mode then this connector
+		 * won't appear connected or have anything with
+		 * EDID on it
+		 */
 		if (intel_encoder->type != INTEL_OUTPUT_EDP)
 			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 		status = connector_status_disconnected;
 		goto out;
+	} else if (connector->status == connector_status_connected) {
+
+		/*
+		 * If display was connected already and is still connected
+		 * check links status, there has been known issues of
+		 * link loss triggerring long pulse!!!!
+		 */
+		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+		intel_dp_check_link_status(intel_dp);
+		drm_modeset_unlock(&dev->mode_config.connection_mutex);
+		goto out;
 	}
 
 	intel_dp_set_edid(intel_dp);
@@ -4854,7 +4860,33 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 
 out:
 	intel_dp_power_put(intel_dp, power_domain);
-	return status;
+	return;
+}
+
+static enum drm_connector_status
+intel_dp_detect(struct drm_connector *connector, bool force)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+		      connector->base.id, connector->name);
+
+	if (intel_dp->is_mst) {
+		/* MST devices are disconnected from a monitor POV */
+		if (intel_encoder->type != INTEL_OUTPUT_EDP)
+			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+		return connector_status_disconnected;
+	}
+
+	intel_dp_long_pulse(intel_dp->attached_connector);
+
+	if (intel_connector->detect_edid)
+		return connector_status_connected;
+	else
+		return connector_status_disconnected;
 }
 
 static void
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Splitting intel_dp_detect

[Ander Conselvan De Oliveira]

On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
> This patch moves probing for panel, DPCD read etc to another
> function intel_dp_long_pulse, while intel_dp_detect returns
> the status as connected or disconnected depending on
> whether the edid is available or not.

Why is the change in connector status necessary?

> This change will be required by further patches in the series
> to avoid performing multiple DPCD operations and removing
> their duplication.
> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 66 ++++++++++++++++++++++++++++++----------
> -
>  1 file changed, 49 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1cb1f3f..a0fe827 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4785,9 +4785,10 @@ intel_dp_power_put(struct intel_dp *dp,
>  	intel_display_power_put(to_i915(encoder->base.dev), power_domain);
>  }
>  
> -static enum drm_connector_status
> -intel_dp_detect(struct drm_connector *connector, bool force)
> +static void
> +intel_dp_long_pulse(struct intel_connector *intel_connector)
>  {
> +	struct drm_connector *connector = &intel_connector->base;
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> @@ -4797,17 +4798,6 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  	bool ret;
>  	u8 sink_irq_vector;
>  
> -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> -		      connector->base.id, connector->name);
> -	intel_dp_unset_edid(intel_dp);
> -
> -	if (intel_dp->is_mst) {
> -		/* MST devices are disconnected from a monitor POV */
> -		if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> -		return connector_status_disconnected;
> -	}
> -
>  	power_domain = intel_dp_power_get(intel_dp);
>  
>  	/* Can't disconnect eDP, but you can close the lid... */
> @@ -4817,19 +4807,35 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  		status = ironlake_dp_detect(intel_dp);
>  	else
>  		status = g4x_dp_detect(intel_dp);
> -	if (status != connector_status_connected)
> +	if (status != connector_status_connected) {
> +		intel_dp_unset_edid(intel_dp);
>  		goto out;
> +	}
>  
>  	intel_dp_probe_oui(intel_dp);
>  
>  	ret = intel_dp_probe_mst(intel_dp);
>  	if (ret) {
> -		/* if we are in MST mode then this connector
> -		   won't appear connected or have anything with EDID on it */
> +		/*
> +		 * if we are in MST mode then this connector
> +		 * won't appear connected or have anything with
> +		 * EDID on it
> +		 */
>  		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>  			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>  		status = connector_status_disconnected;
>  		goto out;

Here the function will exit without a call to intel_dp_unset_edid(), which is
different from the behavior prior to this patch.

> +	} else if (connector->status == connector_status_connected) {
> +
> +		/*
> +		 * If display was connected already and is still connected
> +		 * check links status, there has been known issues of
> +		 * link loss triggerring long pulse!!!!
> +		 */
> +		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> +		intel_dp_check_link_status(intel_dp);
> +		drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +		goto out;
>  	}

The hunk above should be in a separate patch. It also doesn't call
intel_dp_unset_edid(). Might be easier to just add

out:
	if (status == connector_status_disconnected)
		intel_dp_unset_edid(intel_dp);

to the end of the function.

Ander

>  	intel_dp_set_edid(intel_dp);
> @@ -4854,7 +4860,33 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  
>  out:
>  	intel_dp_power_put(intel_dp, power_domain);
> -	return status;
> +	return;
> +}
> +
> +static enum drm_connector_status
> +intel_dp_detect(struct drm_connector *connector, bool force)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> +	struct intel_connector *intel_connector =
> to_intel_connector(connector);
> +
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> +		      connector->base.id, connector->name);
> +
> +	if (intel_dp->is_mst) {
> +		/* MST devices are disconnected from a monitor POV */
> +		if (intel_encoder->type != INTEL_OUTPUT_EDP)
> +			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> +		return connector_status_disconnected;
> +	}
> +
> +	intel_dp_long_pulse(intel_dp->attached_connector);
> +
> +	if (intel_connector->detect_edid)
> +		return connector_status_connected;
> +	else
> +		return connector_status_disconnected;
>  }
>  
>  static void
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Splitting intel_dp_detect

[Shubhangi Shrivastava]

On Monday 16 November 2015 07:10 PM, Ander Conselvan De Oliveira wrote:
> On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
>> This patch moves probing for panel, DPCD read etc to another
>> function intel_dp_long_pulse, while intel_dp_detect returns
>> the status as connected or disconnected depending on
>> whether the edid is available or not.
> Why is the change in connector status necessary?
Since intel_dp_detect is also called in mode enumeration, so all detect 
related operations are moved to intel_dp_long_pulse to avoid any such 
operations to be performed during mode enumeration. intel_dp_long_pulse 
will perform all the panel probing, DPCD read operations etc. which will 
end with EDID being read. In intel_dp_detect, it is checked if EDID has 
been read or not. If EDID has been read then the status is returned as 
connected to allow further operations otherwise the status is returned 
as disconnected.

>
>> This change will be required by further patches in the series
>> to avoid performing multiple DPCD operations and removing
>> their duplication.
>>
>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c | 66 ++++++++++++++++++++++++++++++----------
>> -
>>   1 file changed, 49 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 1cb1f3f..a0fe827 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4785,9 +4785,10 @@ intel_dp_power_put(struct intel_dp *dp,
>>   	intel_display_power_put(to_i915(encoder->base.dev), power_domain);
>>   }
>>   
>> -static enum drm_connector_status
>> -intel_dp_detect(struct drm_connector *connector, bool force)
>> +static void
>> +intel_dp_long_pulse(struct intel_connector *intel_connector)
>>   {
>> +	struct drm_connector *connector = &intel_connector->base;
>>   	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>   	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> @@ -4797,17 +4798,6 @@ intel_dp_detect(struct drm_connector *connector, bool
>> force)
>>   	bool ret;
>>   	u8 sink_irq_vector;
>>   
>> -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>> -		      connector->base.id, connector->name);
>> -	intel_dp_unset_edid(intel_dp);
>> -
>> -	if (intel_dp->is_mst) {
>> -		/* MST devices are disconnected from a monitor POV */
>> -		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>> -			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>> -		return connector_status_disconnected;
>> -	}
>> -
>>   	power_domain = intel_dp_power_get(intel_dp);
>>   
>>   	/* Can't disconnect eDP, but you can close the lid... */
>> @@ -4817,19 +4807,35 @@ intel_dp_detect(struct drm_connector *connector, bool
>> force)
>>   		status = ironlake_dp_detect(intel_dp);
>>   	else
>>   		status = g4x_dp_detect(intel_dp);
>> -	if (status != connector_status_connected)
>> +	if (status != connector_status_connected) {
>> +		intel_dp_unset_edid(intel_dp);
>>   		goto out;
>> +	}
>>   
>>   	intel_dp_probe_oui(intel_dp);
>>   
>>   	ret = intel_dp_probe_mst(intel_dp);
>>   	if (ret) {
>> -		/* if we are in MST mode then this connector
>> -		   won't appear connected or have anything with EDID on it */
>> +		/*
>> +		 * if we are in MST mode then this connector
>> +		 * won't appear connected or have anything with
>> +		 * EDID on it
>> +		 */
>>   		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>   			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>>   		status = connector_status_disconnected;
>>   		goto out;
> Here the function will exit without a call to intel_dp_unset_edid(), which is
> different from the behavior prior to this patch.
Moved call to intel_dp_unset_edid() to out, as per next comment.
>
>> +	} else if (connector->status == connector_status_connected) {
>> +
>> +		/*
>> +		 * If display was connected already and is still connected
>> +		 * check links status, there has been known issues of
>> +		 * link loss triggerring long pulse!!!!
>> +		 */
>> +		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>> +		intel_dp_check_link_status(intel_dp);
>> +		drm_modeset_unlock(&dev->mode_config.connection_mutex);
>> +		goto out;
>>   	}
> The hunk above should be in a separate patch. It also doesn't call
> intel_dp_unset_edid(). Might be easier to just add
>
> out:
> 	if (status == connector_status_disconnected)
> 		intel_dp_unset_edid(intel_dp);
>
> to the end of the function.
>
> Ander
Done. Moved the hunk to next patch and call to intel_dp_unset_edid to out.
>
>>   	intel_dp_set_edid(intel_dp);
>> @@ -4854,7 +4860,33 @@ intel_dp_detect(struct drm_connector *connector, bool
>> force)
>>   
>>   out:
>>   	intel_dp_power_put(intel_dp, power_domain);
>> -	return status;
>> +	return;
>> +}
>> +
>> +static enum drm_connector_status
>> +intel_dp_detect(struct drm_connector *connector, bool force)
>> +{
>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> +	struct intel_connector *intel_connector =
>> to_intel_connector(connector);
>> +
>> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>> +		      connector->base.id, connector->name);
>> +
>> +	if (intel_dp->is_mst) {
>> +		/* MST devices are disconnected from a monitor POV */
>> +		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>> +			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>> +		return connector_status_disconnected;
>> +	}
>> +
>> +	intel_dp_long_pulse(intel_dp->attached_connector);
>> +
>> +	if (intel_connector->detect_edid)
>> +		return connector_status_connected;
>> +	else
>> +		return connector_status_disconnected;
>>   }
>>   
>>   static void

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Splitting intel_dp_detect

[Shubhangi Shrivastava]

This patch moves probing for panel, DPCD read etc to another
function intel_dp_long_pulse, while intel_dp_detect returns
the status as connected or disconnected depending on
whether the edid is available or not.
This change will be required by further patches in the series
to avoid performing multiple DPCD operations and removing
their duplication.

v2: Moved a hunk to next patch of the series.
    Moved intel_dp_unset_edid to out. (Ander)

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8071247..dd2b9da 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4631,9 +4631,10 @@ intel_dp_power_put(struct intel_dp *dp,
 	intel_display_power_put(to_i915(encoder->base.dev), power_domain);
 }
 
-static enum drm_connector_status
-intel_dp_detect(struct drm_connector *connector, bool force)
+static void
+intel_dp_long_pulse(struct intel_connector *intel_connector)
 {
+	struct drm_connector *connector = &intel_connector->base;
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
@@ -4643,17 +4644,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	bool ret;
 	u8 sink_irq_vector;
 
-	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
-		      connector->base.id, connector->name);
-	intel_dp_unset_edid(intel_dp);
-
-	if (intel_dp->is_mst) {
-		/* MST devices are disconnected from a monitor POV */
-		if (intel_encoder->type != INTEL_OUTPUT_EDP)
-			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
-		return connector_status_disconnected;
-	}
-
 	power_domain = intel_dp_power_get(intel_dp);
 
 	/* Can't disconnect eDP, but you can close the lid... */
@@ -4670,8 +4660,10 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 
 	ret = intel_dp_probe_mst(intel_dp);
 	if (ret) {
-		/* if we are in MST mode then this connector
-		   won't appear connected or have anything with EDID on it */
+		/*
+		 * if we are in MST mode then this connector
+		 * won't appear connected or have anything with EDID on it
+		 */
 		if (intel_encoder->type != INTEL_OUTPUT_EDP)
 			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 		status = connector_status_disconnected;
@@ -4699,8 +4691,36 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	}
 
 out:
+	if (status != connector_status_connected)
+		intel_dp_unset_edid(intel_dp);
 	intel_dp_power_put(intel_dp, power_domain);
-	return status;
+	return;
+}
+
+static enum drm_connector_status
+intel_dp_detect(struct drm_connector *connector, bool force)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+		      connector->base.id, connector->name);
+
+	if (intel_dp->is_mst) {
+		/* MST devices are disconnected from a monitor POV */
+		if (intel_encoder->type != INTEL_OUTPUT_EDP)
+			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+		return connector_status_disconnected;
+	}
+
+	intel_dp_long_pulse(intel_dp->attached_connector);
+
+	if (intel_connector->detect_edid)
+		return connector_status_connected;
+	else
+		return connector_status_disconnected;
 }
 
 static void
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Splitting intel_dp_detect

[Ander Conselvan De Oliveira]

On Tue, 2015-11-17 at 11:56 +0530, Shubhangi Shrivastava wrote:
> This patch moves probing for panel, DPCD read etc to another
> function intel_dp_long_pulse, while intel_dp_detect returns
> the status as connected or disconnected depending on
> whether the edid is available or not.

May i suggest a slight rephrase?

"This patch moves probing for panel, DPCD read, etc done in intel_dp_detect() to
a new function: intel_dp_long_pulse(). Note that the behavior of
intel_dp_detect() is changed to report connected or disconnected depending on
whether the edid is available or not."

I think we also need a note why the EDID change is necessary and at the same
doesn't break anything.


> This change will be required by further patches in the series
> to avoid performing multiple DPCD operations and removing
> their duplication.

I found this last part confusing. Maybe replace "avoid multiple DPCD operations
and removing their duplication" with "avoid duplicated DPCD operations on
hotplug", since we are not going into details here.

> 
> v2: Moved a hunk to next patch of the series.
>     Moved intel_dp_unset_edid to out. (Ander)
> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++------------
> -
>  1 file changed, 36 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8071247..dd2b9da 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4631,9 +4631,10 @@ intel_dp_power_put(struct intel_dp *dp,
>  	intel_display_power_put(to_i915(encoder->base.dev), power_domain);
>  }
>  
> -static enum drm_connector_status
> -intel_dp_detect(struct drm_connector *connector, bool force)
> +static void
> +intel_dp_long_pulse(struct intel_connector *intel_connector)
>  {
> +	struct drm_connector *connector = &intel_connector->base;
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> @@ -4643,17 +4644,6 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  	bool ret;
>  	u8 sink_irq_vector;
>  
> -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> -		      connector->base.id, connector->name);
> -	intel_dp_unset_edid(intel_dp);
> -
> -	if (intel_dp->is_mst) {
> -		/* MST devices are disconnected from a monitor POV */
> -		if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> -		return connector_status_disconnected;
> -	}
> -
>  	power_domain = intel_dp_power_get(intel_dp);
>  
>  	/* Can't disconnect eDP, but you can close the lid... */
> @@ -4670,8 +4660,10 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  
>  	ret = intel_dp_probe_mst(intel_dp);
>  	if (ret) {
> -		/* if we are in MST mode then this connector
> -		   won't appear connected or have anything with EDID on it */
> +		/*
> +		 * if we are in MST mode then this connector
> +		 * won't appear connected or have anything with EDID on it
> +		 */
>  		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>  			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>  		status = connector_status_disconnected;
> @@ -4699,8 +4691,36 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  	}
>  
>  out:
> +	if (status != connector_status_connected)
> +		intel_dp_unset_edid(intel_dp);

I didn't notice this at first, but intel_dp_set_edid() doesn't free the previous
edid, so I think we also need to call it just before intel_dp_set_edid(). Patch
looks good otherwise.

Ander

>  	intel_dp_power_put(intel_dp, power_domain);
> -	return status;
> +	return;
> +}
> +
> +static enum drm_connector_status
> +intel_dp_detect(struct drm_connector *connector, bool force)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> +	struct intel_connector *intel_connector =
> to_intel_connector(connector);
> +
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> +		      connector->base.id, connector->name);
> +
> +	if (intel_dp->is_mst) {
> +		/* MST devices are disconnected from a monitor POV */
> +		if (intel_encoder->type != INTEL_OUTPUT_EDP)
> +			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> +		return connector_status_disconnected;
> +	}
> +
> +	intel_dp_long_pulse(intel_dp->attached_connector);
> +
> +	if (intel_connector->detect_edid)
> +		return connector_status_connected;
> +	else
> +		return connector_status_disconnected;
>  }
>  
>  static void
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Splitting intel_dp_detect

[Ander Conselvan De Oliveira]

On Tue, 2015-11-17 at 11:47 +0530, Shubhangi Shrivastava wrote:
> 
> On Monday 16 November 2015 07:10 PM, Ander Conselvan De Oliveira wrote:
> > On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
> > > This patch moves probing for panel, DPCD read etc to another
> > > function intel_dp_long_pulse, while intel_dp_detect returns
> > > the status as connected or disconnected depending on
> > > whether the edid is available or not.
> > Why is the change in connector status necessary?
> Since intel_dp_detect is also called in mode enumeration, so all detect 
> related operations are moved to intel_dp_long_pulse to avoid any such 
> operations to be performed during mode enumeration. intel_dp_long_pulse 
> will perform all the panel probing, DPCD read operations etc. which will 
> end with EDID being read. In intel_dp_detect, it is checked if EDID has 
> been read or not. If EDID has been read then the status is returned as 
> connected to allow further operations otherwise the status is returned 
> as disconnected.

So if I understood correctly, the EDID change is only a about doing a better
split of the code. My concern is that drm_get_edid() might fail. Before this
patch that wouldn't affect the connector status, but now a failure to read EDID 
implies that the connector is disconnected. I'm not sure in which scenario this
would happen, so I wanted to know if this change in behavior is intentional.

Ander

> 
> > 
> > > This change will be required by further patches in the series
> > > to avoid performing multiple DPCD operations and removing
> > > their duplication.
> > > 
> > > Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> > > Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_dp.c | 66 ++++++++++++++++++++++++++++++-----
> > > -----
> > > -
> > >   1 file changed, 49 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 1cb1f3f..a0fe827 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4785,9 +4785,10 @@ intel_dp_power_put(struct intel_dp *dp,
> > >   	intel_display_power_put(to_i915(encoder->base.dev),
> > > power_domain);
> > >   }
> > >   
> > > -static enum drm_connector_status
> > > -intel_dp_detect(struct drm_connector *connector, bool force)
> > > +static void
> > > +intel_dp_long_pulse(struct intel_connector *intel_connector)
> > >   {
> > > +	struct drm_connector *connector = &intel_connector->base;
> > >   	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > >   	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > >   	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > > @@ -4797,17 +4798,6 @@ intel_dp_detect(struct drm_connector *connector,
> > > bool
> > > force)
> > >   	bool ret;
> > >   	u8 sink_irq_vector;
> > >   
> > > -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > > -		      connector->base.id, connector->name);
> > > -	intel_dp_unset_edid(intel_dp);
> > > -
> > > -	if (intel_dp->is_mst) {
> > > -		/* MST devices are disconnected from a monitor POV */
> > > -		if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > > -			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> > > -		return connector_status_disconnected;
> > > -	}
> > > -
> > >   	power_domain = intel_dp_power_get(intel_dp);
> > >   
> > >   	/* Can't disconnect eDP, but you can close the lid... */
> > > @@ -4817,19 +4807,35 @@ intel_dp_detect(struct drm_connector *connector,
> > > bool
> > > force)
> > >   		status = ironlake_dp_detect(intel_dp);
> > >   	else
> > >   		status = g4x_dp_detect(intel_dp);
> > > -	if (status != connector_status_connected)
> > > +	if (status != connector_status_connected) {
> > > +		intel_dp_unset_edid(intel_dp);
> > >   		goto out;
> > > +	}
> > >   
> > >   	intel_dp_probe_oui(intel_dp);
> > >   
> > >   	ret = intel_dp_probe_mst(intel_dp);
> > >   	if (ret) {
> > > -		/* if we are in MST mode then this connector
> > > -		   won't appear connected or have anything with EDID on
> > > it */
> > > +		/*
> > > +		 * if we are in MST mode then this connector
> > > +		 * won't appear connected or have anything with
> > > +		 * EDID on it
> > > +		 */
> > >   		if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > >   			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> > >   		status = connector_status_disconnected;
> > >   		goto out;
> > Here the function will exit without a call to intel_dp_unset_edid(), which
> > is
> > different from the behavior prior to this patch.
> Moved call to intel_dp_unset_edid() to out, as per next comment.
> > 
> > > +	} else if (connector->status == connector_status_connected) {
> > > +
> > > +		/*
> > > +		 * If display was connected already and is still
> > > connected
> > > +		 * check links status, there has been known issues of
> > > +		 * link loss triggerring long pulse!!!!
> > > +		 */
> > > +		drm_modeset_lock(&dev->mode_config.connection_mutex,
> > > NULL);
> > > +		intel_dp_check_link_status(intel_dp);
> > > +		drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > > +		goto out;
> > >   	}
> > The hunk above should be in a separate patch. It also doesn't call
> > intel_dp_unset_edid(). Might be easier to just add
> > 
> > out:
> > 	if (status == connector_status_disconnected)
> > 		intel_dp_unset_edid(intel_dp);
> > 
> > to the end of the function.
> > 
> > Ander
> Done. Moved the hunk to next patch and call to intel_dp_unset_edid to out.
> > 
> > >   	intel_dp_set_edid(intel_dp);
> > > @@ -4854,7 +4860,33 @@ intel_dp_detect(struct drm_connector *connector,
> > > bool
> > > force)
> > >   
> > >   out:
> > >   	intel_dp_power_put(intel_dp, power_domain);
> > > -	return status;
> > > +	return;
> > > +}
> > > +
> > > +static enum drm_connector_status
> > > +intel_dp_detect(struct drm_connector *connector, bool force)
> > > +{
> > > +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > > +	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > > +	struct intel_connector *intel_connector =
> > > to_intel_connector(connector);
> > > +
> > > +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > > +		      connector->base.id, connector->name);
> > > +
> > > +	if (intel_dp->is_mst) {
> > > +		/* MST devices are disconnected from a monitor POV */
> > > +		if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > > +			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> > > +		return connector_status_disconnected;
> > > +	}
> > > +
> > > +	intel_dp_long_pulse(intel_dp->attached_connector);
> > > +
> > > +	if (intel_connector->detect_edid)
> > > +		return connector_status_connected;
> > > +	else
> > > +		return connector_status_disconnected;
> > >   }
> > >   
> > >   static void
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Splitting intel_dp_detect

[Shubhangi Shrivastava]

On Tuesday 17 November 2015 02:52 PM, Ander Conselvan De Oliveira wrote:
> On Tue, 2015-11-17 at 11:47 +0530, Shubhangi Shrivastava wrote:
>> On Monday 16 November 2015 07:10 PM, Ander Conselvan De Oliveira wrote:
>>> On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
>>>> This patch moves probing for panel, DPCD read etc to another
>>>> function intel_dp_long_pulse, while intel_dp_detect returns
>>>> the status as connected or disconnected depending on
>>>> whether the edid is available or not.
>>> Why is the change in connector status necessary?
>> Since intel_dp_detect is also called in mode enumeration, so all detect
>> related operations are moved to intel_dp_long_pulse to avoid any such
>> operations to be performed during mode enumeration. intel_dp_long_pulse
>> will perform all the panel probing, DPCD read operations etc. which will
>> end with EDID being read. In intel_dp_detect, it is checked if EDID has
>> been read or not. If EDID has been read then the status is returned as
>> connected to allow further operations otherwise the status is returned
>> as disconnected.
> So if I understood correctly, the EDID change is only a about doing a better
> split of the code. My concern is that drm_get_edid() might fail. Before this
> patch that wouldn't affect the connector status, but now a failure to read EDID
> implies that the connector is disconnected. I'm not sure in which scenario this
> would happen, so I wanted to know if this change in behavior is intentional.
>
> Ander
Yes, this is intentional. Intel_dp_detect is supposed to be called after 
the long pulse has been handled. If EDID is not present, that means 
either long pulse was never received or there is no panel connected.

>
>>>> This change will be required by further patches in the series
>>>> to avoid performing multiple DPCD operations and removing
>>>> their duplication.
>>>>
>>>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>>> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_dp.c | 66 ++++++++++++++++++++++++++++++-----
>>>> -----
>>>> -
>>>>    1 file changed, 49 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>>> b/drivers/gpu/drm/i915/intel_dp.c
>>>> index 1cb1f3f..a0fe827 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -4785,9 +4785,10 @@ intel_dp_power_put(struct intel_dp *dp,
>>>>    	intel_display_power_put(to_i915(encoder->base.dev),
>>>> power_domain);
>>>>    }
>>>>    
>>>> -static enum drm_connector_status
>>>> -intel_dp_detect(struct drm_connector *connector, bool force)
>>>> +static void
>>>> +intel_dp_long_pulse(struct intel_connector *intel_connector)
>>>>    {
>>>> +	struct drm_connector *connector = &intel_connector->base;
>>>>    	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>>>    	struct intel_digital_port *intel_dig_port =
>>>> dp_to_dig_port(intel_dp);
>>>>    	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>>>> @@ -4797,17 +4798,6 @@ intel_dp_detect(struct drm_connector *connector,
>>>> bool
>>>> force)
>>>>    	bool ret;
>>>>    	u8 sink_irq_vector;
>>>>    
>>>> -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>>>> -		      connector->base.id, connector->name);
>>>> -	intel_dp_unset_edid(intel_dp);
>>>> -
>>>> -	if (intel_dp->is_mst) {
>>>> -		/* MST devices are disconnected from a monitor POV */
>>>> -		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>>> -			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>>>> -		return connector_status_disconnected;
>>>> -	}
>>>> -
>>>>    	power_domain = intel_dp_power_get(intel_dp);
>>>>    
>>>>    	/* Can't disconnect eDP, but you can close the lid... */
>>>> @@ -4817,19 +4807,35 @@ intel_dp_detect(struct drm_connector *connector,
>>>> bool
>>>> force)
>>>>    		status = ironlake_dp_detect(intel_dp);
>>>>    	else
>>>>    		status = g4x_dp_detect(intel_dp);
>>>> -	if (status != connector_status_connected)
>>>> +	if (status != connector_status_connected) {
>>>> +		intel_dp_unset_edid(intel_dp);
>>>>    		goto out;
>>>> +	}
>>>>    
>>>>    	intel_dp_probe_oui(intel_dp);
>>>>    
>>>>    	ret = intel_dp_probe_mst(intel_dp);
>>>>    	if (ret) {
>>>> -		/* if we are in MST mode then this connector
>>>> -		   won't appear connected or have anything with EDID on
>>>> it */
>>>> +		/*
>>>> +		 * if we are in MST mode then this connector
>>>> +		 * won't appear connected or have anything with
>>>> +		 * EDID on it
>>>> +		 */
>>>>    		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>>>    			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>>>>    		status = connector_status_disconnected;
>>>>    		goto out;
>>> Here the function will exit without a call to intel_dp_unset_edid(), which
>>> is
>>> different from the behavior prior to this patch.
>> Moved call to intel_dp_unset_edid() to out, as per next comment.
>>>> +	} else if (connector->status == connector_status_connected) {
>>>> +
>>>> +		/*
>>>> +		 * If display was connected already and is still
>>>> connected
>>>> +		 * check links status, there has been known issues of
>>>> +		 * link loss triggerring long pulse!!!!
>>>> +		 */
>>>> +		drm_modeset_lock(&dev->mode_config.connection_mutex,
>>>> NULL);
>>>> +		intel_dp_check_link_status(intel_dp);
>>>> +		drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>>> +		goto out;
>>>>    	}
>>> The hunk above should be in a separate patch. It also doesn't call
>>> intel_dp_unset_edid(). Might be easier to just add
>>>
>>> out:
>>> 	if (status == connector_status_disconnected)
>>> 		intel_dp_unset_edid(intel_dp);
>>>
>>> to the end of the function.
>>>
>>> Ander
>> Done. Moved the hunk to next patch and call to intel_dp_unset_edid to out.
>>>>    	intel_dp_set_edid(intel_dp);
>>>> @@ -4854,7 +4860,33 @@ intel_dp_detect(struct drm_connector *connector,
>>>> bool
>>>> force)
>>>>    
>>>>    out:
>>>>    	intel_dp_power_put(intel_dp, power_domain);
>>>> -	return status;
>>>> +	return;
>>>> +}
>>>> +
>>>> +static enum drm_connector_status
>>>> +intel_dp_detect(struct drm_connector *connector, bool force)
>>>> +{
>>>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>>> +	struct intel_digital_port *intel_dig_port =
>>>> dp_to_dig_port(intel_dp);
>>>> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>>>> +	struct intel_connector *intel_connector =
>>>> to_intel_connector(connector);
>>>> +
>>>> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>>>> +		      connector->base.id, connector->name);
>>>> +
>>>> +	if (intel_dp->is_mst) {
>>>> +		/* MST devices are disconnected from a monitor POV */
>>>> +		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>>> +			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>>>> +		return connector_status_disconnected;
>>>> +	}
>>>> +
>>>> +	intel_dp_long_pulse(intel_dp->attached_connector);
>>>> +
>>>> +	if (intel_connector->detect_edid)
>>>> +		return connector_status_connected;
>>>> +	else
>>>> +		return connector_status_disconnected;
>>>>    }
>>>>    
>>>>    static void

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse

[Shubhangi Shrivastava]

Current DP detection has DPCD operations split across
intel_dp_hpd_pulse and intel_dp_detect which contains
duplicates as well. Also intel_dp_detect is called
during modes enumeration as well which will result
in multiple dpcd operations. So this patch tries
to solve both these by bringing all DPCD operations
in one single function and make intel_dp_detect
use existing values instead of repeating same steps.

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a0fe827..4e74cd6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4881,7 +4881,8 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		return connector_status_disconnected;
 	}
 
-	intel_dp_long_pulse(intel_dp->attached_connector);
+	if (force)
+		intel_dp_long_pulse(intel_dp->attached_connector);
 
 	if (intel_connector->detect_edid)
 		return connector_status_connected;
@@ -5211,21 +5212,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		/* indicate that we need to restart link training */
 		intel_dp->train_set_valid = false;
 
-		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
-			goto mst_fail;
+		intel_dp_long_pulse(intel_dp->attached_connector);
+		goto put_power;
 
-		if (!intel_dp_get_dpcd(intel_dp)) {
-			goto mst_fail;
-		}
-
-		intel_dp_probe_oui(intel_dp);
-
-		if (!intel_dp_probe_mst(intel_dp)) {
-			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-			intel_dp_check_link_status(intel_dp);
-			drm_modeset_unlock(&dev->mode_config.connection_mutex);
-			goto mst_fail;
-		}
 	} else {
 		if (intel_dp->is_mst) {
 			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse

[Ander Conselvan De Oliveira]

On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
> Current DP detection has DPCD operations split across
> intel_dp_hpd_pulse and intel_dp_detect which contains
> duplicates as well. Also intel_dp_detect is called
> during modes enumeration as well which will result
> in multiple dpcd operations. So this patch tries
> to solve both these by bringing all DPCD operations
> in one single function and make intel_dp_detect
> use existing values instead of repeating same steps.
> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a0fe827..4e74cd6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4881,7 +4881,8 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  		return connector_status_disconnected;
>  	}
>  
> -	intel_dp_long_pulse(intel_dp->attached_connector);
> +	if (force)
> +		intel_dp_long_pulse(intel_dp->attached_connector);
>  
>  	if (intel_connector->detect_edid)
>  		return connector_status_connected;
> @@ -5211,21 +5212,9 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
>  		/* indicate that we need to restart link training */
>  		intel_dp->train_set_valid = false;
>  
> -		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
> -			goto mst_fail;
> +		intel_dp_long_pulse(intel_dp->attached_connector);
> +		goto put_power;
>  
> -		if (!intel_dp_get_dpcd(intel_dp)) {
> -			goto mst_fail;
> -		}

So we don't call this for eDP anymore on long pulse, which I assume is harmless
since the bits we are reading from DPCD shouldn't change?

> -
> -		intel_dp_probe_oui(intel_dp);
> -
> -		if (!intel_dp_probe_mst(intel_dp)) {
> -			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> -			intel_dp_check_link_status(intel_dp);
> -			drm_modeset_unlock(&dev->mode_config.connection_mutex);
> -			goto mst_fail;
> -		}

Hmm, so this is where that hunk from patch 1 I said should be a separate patch
comes from. Looks like in belongs to this patch.

>  	} else {
>  		if (intel_dp->is_mst) {
>  			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse

[Ander Conselvan De Oliveira]

On Mon, 2015-11-16 at 16:33 +0200, Ander Conselvan De Oliveira wrote:
> On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
> > Current DP detection has DPCD operations split across
> > intel_dp_hpd_pulse and intel_dp_detect which contains
> > duplicates as well. Also intel_dp_detect is called
> > during modes enumeration as well which will result
> > in multiple dpcd operations. So this patch tries
> > to solve both these by bringing all DPCD operations
> > in one single function and make intel_dp_detect
> > use existing values instead of repeating same steps.
> > 
> > Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> > Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 19 ++++---------------
> >  1 file changed, 4 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index a0fe827..4e74cd6 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4881,7 +4881,8 @@ intel_dp_detect(struct drm_connector *connector, bool
> > force)
> >  		return connector_status_disconnected;
> >  	}
> >  
> > -	intel_dp_long_pulse(intel_dp->attached_connector);
> > +	if (force)
> > +		intel_dp_long_pulse(intel_dp->attached_connector);
> >  
> >  	if (intel_connector->detect_edid)
> >  		return connector_status_connected;
> > @@ -5211,21 +5212,9 @@ intel_dp_hpd_pulse(struct intel_digital_port
> > *intel_dig_port, bool long_hpd)
> >  		/* indicate that we need to restart link training */
> >  		intel_dp->train_set_valid = false;
> >  
> > -		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
> > -			goto mst_fail;
> > +		intel_dp_long_pulse(intel_dp->attached_connector);
> > +		goto put_power;

This skips the line that sets ret to IRQ_HANDLED, which will cause the hotplug
code to "fall back to old school hpd".

Ander

> >  
> > -		if (!intel_dp_get_dpcd(intel_dp)) {
> > -			goto mst_fail;
> > -		}
> 
> So we don't call this for eDP anymore on long pulse, which I assume is
> harmless
> since the bits we are reading from DPCD shouldn't change?
> 
> > -
> > -		intel_dp_probe_oui(intel_dp);
> > -
> > -		if (!intel_dp_probe_mst(intel_dp)) {
> > -			drm_modeset_lock(&dev
> > ->mode_config.connection_mutex, NULL);
> > -			intel_dp_check_link_status(intel_dp);
> > -			drm_modeset_unlock(&dev
> > ->mode_config.connection_mutex);
> > -			goto mst_fail;
> > -		}
> 
> Hmm, so this is where that hunk from patch 1 I said should be a separate patch
> comes from. Looks like in belongs to this patch.
> 
> >  	} else {
> >  		if (intel_dp->is_mst) {
> >  			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse

[Shubhangi Shrivastava]

On Monday 16 November 2015 08:16 PM, Ander Conselvan De Oliveira wrote:
> On Mon, 2015-11-16 at 16:33 +0200, Ander Conselvan De Oliveira wrote:
>> On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
>>> Current DP detection has DPCD operations split across
>>> intel_dp_hpd_pulse and intel_dp_detect which contains
>>> duplicates as well. Also intel_dp_detect is called
>>> during modes enumeration as well which will result
>>> in multiple dpcd operations. So this patch tries
>>> to solve both these by bringing all DPCD operations
>>> in one single function and make intel_dp_detect
>>> use existing values instead of repeating same steps.
>>>
>>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_dp.c | 19 ++++---------------
>>>   1 file changed, 4 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>> b/drivers/gpu/drm/i915/intel_dp.c
>>> index a0fe827..4e74cd6 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -4881,7 +4881,8 @@ intel_dp_detect(struct drm_connector *connector, bool
>>> force)
>>>   		return connector_status_disconnected;
>>>   	}
>>>   
>>> -	intel_dp_long_pulse(intel_dp->attached_connector);
>>> +	if (force)
>>> +		intel_dp_long_pulse(intel_dp->attached_connector);
>>>   
>>>   	if (intel_connector->detect_edid)
>>>   		return connector_status_connected;
>>> @@ -5211,21 +5212,9 @@ intel_dp_hpd_pulse(struct intel_digital_port
>>> *intel_dig_port, bool long_hpd)
>>>   		/* indicate that we need to restart link training */
>>>   		intel_dp->train_set_valid = false;
>>>   
>>> -		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
>>> -			goto mst_fail;
>>> +		intel_dp_long_pulse(intel_dp->attached_connector);
>>> +		goto put_power;
> This skips the line that sets ret to IRQ_HANDLED, which will cause the hotplug
> code to "fall back to old school hpd".
>
> Ander
Notification of hotplug to user mode goes from i915_hotplug_work_func. 
Returning IRQ_NONE from here will notify user mode about hotplug.
>
>>>   
>>> -		if (!intel_dp_get_dpcd(intel_dp)) {
>>> -			goto mst_fail;
>>> -		}
>> So we don't call this for eDP anymore on long pulse, which I assume is
>> harmless
>> since the bits we are reading from DPCD shouldn't change?
>>
>>> -
>>> -		intel_dp_probe_oui(intel_dp);
>>> -
>>> -		if (!intel_dp_probe_mst(intel_dp)) {
>>> -			drm_modeset_lock(&dev
>>> ->mode_config.connection_mutex, NULL);
>>> -			intel_dp_check_link_status(intel_dp);
>>> -			drm_modeset_unlock(&dev
>>> ->mode_config.connection_mutex);
>>> -			goto mst_fail;
>>> -		}
>> Hmm, so this is where that hunk from patch 1 I said should be a separate patch
>> comes from. Looks like in belongs to this patch.
>>
>>>   	} else {
>>>   		if (intel_dp->is_mst) {
>>>   			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse

[Ander Conselvan De Oliveira]

On Tue, 2015-11-17 at 12:23 +0530, Shubhangi Shrivastava wrote:
> 
> On Monday 16 November 2015 08:16 PM, Ander Conselvan De Oliveira wrote:
> > On Mon, 2015-11-16 at 16:33 +0200, Ander Conselvan De Oliveira wrote:
> > > On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
> > > > Current DP detection has DPCD operations split across
> > > > intel_dp_hpd_pulse and intel_dp_detect which contains
> > > > duplicates as well. Also intel_dp_detect is called
> > > > during modes enumeration as well which will result
> > > > in multiple dpcd operations. So this patch tries
> > > > to solve both these by bringing all DPCD operations
> > > > in one single function and make intel_dp_detect
> > > > use existing values instead of repeating same steps.
> > > > 
> > > > Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> > > > Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/intel_dp.c | 19 ++++---------------
> > > >   1 file changed, 4 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index a0fe827..4e74cd6 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4881,7 +4881,8 @@ intel_dp_detect(struct drm_connector *connector,
> > > > bool
> > > > force)
> > > >   		return connector_status_disconnected;
> > > >   	}
> > > >   
> > > > -	intel_dp_long_pulse(intel_dp->attached_connector);
> > > > +	if (force)
> > > > +		intel_dp_long_pulse(intel_dp->attached_connector);
> > > >   
> > > >   	if (intel_connector->detect_edid)
> > > >   		return connector_status_connected;
> > > > @@ -5211,21 +5212,9 @@ intel_dp_hpd_pulse(struct intel_digital_port
> > > > *intel_dig_port, bool long_hpd)
> > > >   		/* indicate that we need to restart link training */
> > > >   		intel_dp->train_set_valid = false;
> > > >   
> > > > -		if (!intel_digital_port_connected(dev_priv,
> > > > intel_dig_port))
> > > > -			goto mst_fail;
> > > > +		intel_dp_long_pulse(intel_dp->attached_connector);
> > > > +		goto put_power;
> > This skips the line that sets ret to IRQ_HANDLED, which will cause the
> > hotplug
> > code to "fall back to old school hpd".
> > 
> > Ander
> Notification of hotplug to user mode goes from i915_hotplug_work_func. 
> Returning IRQ_NONE from here will notify user mode about hotplug.

So if I understand correctly, previously if the device was connected, we
succeeded in getting the dpcd and the device was mst, there was no hotplug event
being sent. With the change above the hotplug event is generated
unconditionally, and we never do the error handling in mst_fail for the long pul
se.

Ander


> > 
> > > >   
> > > > -		if (!intel_dp_get_dpcd(intel_dp)) {
> > > > -			goto mst_fail;
> > > > -		}
> > > So we don't call this for eDP anymore on long pulse, which I assume is
> > > harmless
> > > since the bits we are reading from DPCD shouldn't change?
> > > 
> > > > -
> > > > -		intel_dp_probe_oui(intel_dp);
> > > > -
> > > > -		if (!intel_dp_probe_mst(intel_dp)) {
> > > > -			drm_modeset_lock(&dev
> > > > ->mode_config.connection_mutex, NULL);
> > > > -			intel_dp_check_link_status(intel_dp);
> > > > -			drm_modeset_unlock(&dev
> > > > ->mode_config.connection_mutex);
> > > > -			goto mst_fail;
> > > > -		}
> > > Hmm, so this is where that hunk from patch 1 I said should be a separate
> > > patch
> > > comes from. Looks like in belongs to this patch.
> > > 
> > > >   	} else {
> > > >   		if (intel_dp->is_mst) {
> > > >   			if (intel_dp_check_mst_status(intel_dp) == 
> > > > -EINVAL)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse

[Shubhangi Shrivastava]

On Monday 16 November 2015 08:03 PM, Ander Conselvan De Oliveira wrote:
> On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
>> Current DP detection has DPCD operations split across
>> intel_dp_hpd_pulse and intel_dp_detect which contains
>> duplicates as well. Also intel_dp_detect is called
>> during modes enumeration as well which will result
>> in multiple dpcd operations. So this patch tries
>> to solve both these by bringing all DPCD operations
>> in one single function and make intel_dp_detect
>> use existing values instead of repeating same steps.
>>
>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c | 19 ++++---------------
>>   1 file changed, 4 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index a0fe827..4e74cd6 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4881,7 +4881,8 @@ intel_dp_detect(struct drm_connector *connector, bool
>> force)
>>   		return connector_status_disconnected;
>>   	}
>>   
>> -	intel_dp_long_pulse(intel_dp->attached_connector);
>> +	if (force)
>> +		intel_dp_long_pulse(intel_dp->attached_connector);
>>   
>>   	if (intel_connector->detect_edid)
>>   		return connector_status_connected;
>> @@ -5211,21 +5212,9 @@ intel_dp_hpd_pulse(struct intel_digital_port
>> *intel_dig_port, bool long_hpd)
>>   		/* indicate that we need to restart link training */
>>   		intel_dp->train_set_valid = false;
>>   
>> -		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
>> -			goto mst_fail;
>> +		intel_dp_long_pulse(intel_dp->attached_connector);
>> +		goto put_power;
>>   
>> -		if (!intel_dp_get_dpcd(intel_dp)) {
>> -			goto mst_fail;
>> -		}
> So we don't call this for eDP anymore on long pulse, which I assume is harmless
> since the bits we are reading from DPCD shouldn't change?
>
>> -
>> -		intel_dp_probe_oui(intel_dp);
>> -
>> -		if (!intel_dp_probe_mst(intel_dp)) {
>> -			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>> -			intel_dp_check_link_status(intel_dp);
>> -			drm_modeset_unlock(&dev->mode_config.connection_mutex);
>> -			goto mst_fail;
>> -		}
> Hmm, so this is where that hunk from patch 1 I said should be a separate patch
> comes from. Looks like in belongs to this patch.
Pulled in that hunk from patch 1 to this patch.
>
>>   	} else {
>>   		if (intel_dp->is_mst) {
>>   			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Cleaning up intel_dp_hpd_pulse

[Shubhangi Shrivastava]

Current DP detection has DPCD operations split across
intel_dp_hpd_pulse and intel_dp_detect which contains
duplicates as well. Also intel_dp_detect is called
during modes enumeration as well which will result
in multiple dpcd operations. So this patch tries
to solve both these by bringing all DPCD operations
in one single function and make intel_dp_detect
use existing values instead of repeating same steps.

v2: Pulled in a hunk from last patch of the series to
    this patch. (Ander)

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dd2b9da..e4d6d33 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4668,6 +4668,16 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 		status = connector_status_disconnected;
 		goto out;
+	} else if (connector->status == connector_status_connected) {
+		/*
+		 * If display was connected already and is still connected
+		 * check links status, there has been known issues of
+		 * link loss triggerring long pulse!!!!
+		 */
+		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+		intel_dp_check_link_status(intel_dp);
+		drm_modeset_unlock(&dev->mode_config.connection_mutex);
+		goto out;
 	}
 
 	intel_dp_set_edid(intel_dp);
@@ -4715,7 +4725,8 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		return connector_status_disconnected;
 	}
 
-	intel_dp_long_pulse(intel_dp->attached_connector);
+	if (force)
+		intel_dp_long_pulse(intel_dp->attached_connector);
 
 	if (intel_connector->detect_edid)
 		return connector_status_connected;
@@ -5045,21 +5056,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		/* indicate that we need to restart link training */
 		intel_dp->train_set_valid = false;
 
-		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
-			goto mst_fail;
-
-		if (!intel_dp_get_dpcd(intel_dp)) {
-			goto mst_fail;
-		}
-
-		intel_dp_probe_oui(intel_dp);
+		intel_dp_long_pulse(intel_dp->attached_connector);
+		goto put_power;
 
-		if (!intel_dp_probe_mst(intel_dp)) {
-			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-			intel_dp_check_link_status(intel_dp);
-			drm_modeset_unlock(&dev->mode_config.connection_mutex);
-			goto mst_fail;
-		}
 	} else {
 		if (intel_dp->is_mst) {
 			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Cleaning up intel_dp_hpd_pulse

[Ander Conselvan De Oliveira]

On Tue, 2015-11-17 at 12:17 +0530, Shubhangi Shrivastava wrote:
> Current DP detection has DPCD operations split across
> intel_dp_hpd_pulse and intel_dp_detect which contains
> duplicates as well. Also intel_dp_detect is called
> during modes enumeration as well which will result
> in multiple dpcd operations. So this patch tries
> to solve both these by bringing all DPCD operations
> in one single function and make intel_dp_detect
> use existing values instead of repeating same steps.
> 
> v2: Pulled in a hunk from last patch of the series to
>     this patch. (Ander)
> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index dd2b9da..e4d6d33 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4668,6 +4668,16 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>  		status = connector_status_disconnected;
>  		goto out;
> +	} else if (connector->status == connector_status_connected) {
> +		/*
> +		 * If display was connected already and is still connected
> +		 * check links status, there has been known issues of
> +		 * link loss triggerring long pulse!!!!
> +		 */
> +		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> +		intel_dp_check_link_status(intel_dp);
> +		drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +		goto out;
>  	}
>  
>  	intel_dp_set_edid(intel_dp);
> @@ -4715,7 +4725,8 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  		return connector_status_disconnected;
>  	}
>  
> -	intel_dp_long_pulse(intel_dp->attached_connector);
> +	if (force)
> +		intel_dp_long_pulse(intel_dp->attached_connector);
>  
>  	if (intel_connector->detect_edid)
>  		return connector_status_connected;
> @@ -5045,21 +5056,9 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
>  		/* indicate that we need to restart link training */
>  		intel_dp->train_set_valid = false;
>  
> -		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
> -			goto mst_fail;
> -
> -		if (!intel_dp_get_dpcd(intel_dp)) {
> -			goto mst_fail;
> -		}
> -
> -		intel_dp_probe_oui(intel_dp);
> +		intel_dp_long_pulse(intel_dp->attached_connector);
> +		goto put_power;

This has the problem with handling an mst device that is gone which Sivakumar
and I discussed on IRC.

I'm thinking whether it makes sense to move the code in the mst_fail label to 
intel_dp_detect(), since it gets called every time we reach that label. We would
just need a way to tell it about the failure, since we can't rely on connector
status being disconnected (because of MST). That could help us clean up
intel_dp_hpd_pulse even more.
Thoughts?

Ander

>  
> -		if (!intel_dp_probe_mst(intel_dp)) {
> -			drm_modeset_lock(&dev->mode_config.connection_mutex,
> NULL);
> -			intel_dp_check_link_status(intel_dp);
> -			drm_modeset_unlock(&dev
> ->mode_config.connection_mutex);
> -			goto mst_fail;
> -		}
>  	} else {
>  		if (intel_dp->is_mst) {
>  			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/6] drm/i915: Splitting intel_dp_check_link_status

[Shubhangi Shrivastava]

The link retraining part when EQ is not correct is
retained to intel_dp_check_link_status whereas other
operations are handled as part of intel_dp_short_pulse.
This change is required to avoid performing all DPCD
related operations on performing link retraining.

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 49 +++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4e74cd6..92378b4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4421,6 +4421,31 @@ go_again:
 	return -EINVAL;
 }
 
+static void
+intel_dp_check_link_status(struct intel_dp *intel_dp)
+{
+	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
+	u8 link_status[DP_LINK_STATUS_SIZE];
+
+	if (!intel_dp_get_link_status(intel_dp, link_status)) {
+		DRM_ERROR("Failed to get link status\n");
+		return;
+	}
+
+	if (!intel_encoder->base.crtc)
+		return;
+
+	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+		return;
+
+	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
+		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
+				intel_encoder->base.name);
+		intel_dp_start_link_train(intel_dp);
+		intel_dp_stop_link_train(intel_dp);
+	}
+}
+
 /*
  * According to DP spec
  * 5.1.2:
@@ -4430,21 +4455,12 @@ go_again:
  *  4. Check link status on receipt of hot-plug interrupt
  */
 static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
+intel_dp_short_pulse(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-
-	if (!intel_encoder->base.crtc)
-		return;
-
-	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
-		return;
-
 	/* Try to read receiver status if the link appears to be up */
 	if (!intel_dp_get_link_status(intel_dp, link_status)) {
 		return;
@@ -4469,12 +4485,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
-	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
-		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
-			      intel_encoder->base.name);
-		intel_dp_start_link_train(intel_dp);
-		intel_dp_stop_link_train(intel_dp);
-	}
+	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+	intel_dp_check_link_status(intel_dp);
+	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 }
 
 /* XXX this is probably wrong for multiple downstream ports */
@@ -5222,9 +5235,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		}
 
 		if (!intel_dp->is_mst) {
-			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-			intel_dp_check_link_status(intel_dp);
-			drm_modeset_unlock(&dev->mode_config.connection_mutex);
+			intel_dp_short_pulse(intel_dp);
 		}
 	}
 
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/6] drm/i915: Save sink_count for tracking changes to it

[Shubhangi Shrivastava]

Sink count can change between short pulse hpd hence this patch
adds a member variable to intel_dp so we can track any changes
between short pulse interrupts.

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 7 +++----
 drivers/gpu/drm/i915/intel_drv.h | 1 +
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 92378b4..8014322 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4507,14 +4507,13 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	/* If we're HPD-aware, SINK_COUNT changes dynamically */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
 	    intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
-		uint8_t reg;
 
 		if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
-					    &reg, 1) < 0)
+					    &intel_dp->sink_count, 1) < 0)
 			return connector_status_unknown;
 
-		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
-					      : connector_status_disconnected;
+		return DP_GET_SINK_COUNT(intel_dp->sink_count) ?
+		connector_status_connected : connector_status_disconnected;
 	}
 
 	/* If no HPD, poke DDC gently */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a3bbdc..abcedc5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -748,6 +748,7 @@ struct intel_dp {
 	uint32_t DP;
 	int link_rate;
 	uint8_t lane_count;
+	uint8_t sink_count;
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	bool limited_color_range;
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/6] drm/i915: read sink_count dpcd always

[Shubhangi Shrivastava]

This patch reads sink_count dpcd always and removes its
read operation based on values in downstream port dpcd.

SINK_COUNT dpcd is not dependent on DOWNSTREAM_PORT_PRESENT dpcd.
SINK_COUNT denotes if a display is attached, while
DOWNSTREAM_PORT_PRESET indicates how many ports are available
in the dongle where display can be attached. so it is possible
for sink count to change irrespective of value in downstream
port dpcd.

Here is a table of possible values and scenarios

sink_count      downstream_port
                present
0               0               no display is attached
0               1               dongle is connected without display
1               0               display connected directly
1               1               display connected through dongle

v2: moved out crtc enabled checks to prior patch(Jani)

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8014322..2acff47 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3988,6 +3988,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
 		return false; /* DPCD not present */
 
+	if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+				    &intel_dp->sink_count, 1) < 0)
+		return false;
+
+	if (!DP_GET_SINK_COUNT(intel_dp->sink_count))
+		return false;
+
 	/* Check if the panel supports PSR */
 	memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
 	if (is_edp(intel_dp)) {
@@ -4508,10 +4515,6 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
 	    intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
 
-		if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
-					    &intel_dp->sink_count, 1) < 0)
-			return connector_status_unknown;
-
 		return DP_GET_SINK_COUNT(intel_dp->sink_count) ?
 		connector_status_connected : connector_status_disconnected;
 	}
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/6] drm/i915: force full detect on sink count change

[Shubhangi Shrivastava]

This patch checks for changes in sink count between short pulse
hpds and forces full detect when there is a change.

This will allow both detection of hotplug and unplug of panels
through dongles that give only short pulse for such events.

v2: changed variable type from u8 to bool (Jani)
    return immediately if perform_full_detect is set(Siva)

v3: changed method of determining full detection from using
    pointer to return code (Siva)

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2acff47..4ce41e7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4461,21 +4461,30 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
  *  3. Use Link Training from 2.5.3.3 and 3.5.1.3
  *  4. Check link status on receipt of hot-plug interrupt
  */
-static void
+static bool
 intel_dp_short_pulse(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	u8 sink_irq_vector;
 	u8 link_status[DP_LINK_STATUS_SIZE];
+	u8 old_sink_count = intel_dp->sink_count;
+	bool ret;
 
 	/* Try to read receiver status if the link appears to be up */
 	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		return;
+		return false;
 	}
 
-	/* Now read the DPCD to see if it's actually running */
-	if (!intel_dp_get_dpcd(intel_dp)) {
-		return;
+	/* Now read the DPCD to see if it's actually running
+	 * Don't return immediately if dpcd read failed,
+	 * if sink count was 1 and dpcd read failed we need
+	 * to do full detection
+	 */
+	ret = intel_dp_get_dpcd(intel_dp);
+
+	if ((old_sink_count != intel_dp->sink_count) || !ret) {
+		/* No need to proceed if we are going to do full detect */
+		return false;
 	}
 
 	/* Try to read the source of the interrupt */
@@ -4495,6 +4504,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	intel_dp_check_link_status(intel_dp);
 	drm_modeset_unlock(&dev->mode_config.connection_mutex);
+
+	return true;
 }
 
 /* XXX this is probably wrong for multiple downstream ports */
@@ -5237,7 +5248,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		}
 
 		if (!intel_dp->is_mst) {
-			intel_dp_short_pulse(intel_dp);
+			if (!intel_dp_short_pulse(intel_dp)) {
+				intel_dp_long_pulse(intel_dp->attached_connector);
+				goto put_power;
+			}
 		}
 	}
 
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 0/6] Fixing sink count related detection over

[Shubhangi Shrivastava]

This patch set cleans up DP detection logic to bring all DPCD
operations at one place and to create a clear demarcation
between handling of long and short pulses. This simplifies
fixing of sink count related detection for DP panels.

Patches:
1. First two patches clean up intel_dp_detect and form a new
function which will include all DPCD related operations.
2. Third patch splits up intel_dp_check_link_status to form
a new function which will handle short pulse requests.
3. Last three patches fixes the detection logic related to
sink count i.e detect changes in sink count and handle them
appropriately. 

Shubhangi Shrivastava (6):
  drm/i915: Splitting intel_dp_detect
  drm/i915: Cleaning up intel_dp_hpd_pulse
  drm/i915: Splitting intel_dp_check_link_status
  drm/i915: Save sink_count for tracking changes to it
  drm/i915: read sink_count dpcd always
  drm/i915: force full detect on sink count change

 drivers/gpu/drm/i915/intel_dp.c  | 170 +++++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h |   1 +
 2 files changed, 110 insertions(+), 61 deletions(-)

-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 0/6] Fixing sink count related detection over

[Shubhangi Shrivastava]

This patch set cleans up DP detection logic to bring all DPCD
operations at one place and to create a clear demarcation
between handling of long and short pulses. This simplifies
fixing of sink count related detection for DP panels.

Patches:
1. First two patches clean up intel_dp_detect and form a new
function which will include all DPCD related operations.
2. Second patch splits up intel_dp_check_link_status to form
a new function which will handle short pulse requests.
3. Last three patches fixes the detection logic related to
sink count i.e detect changes in sink count and handle them
appropriately. 

Note: this is tested on BXT with non-mst panels,
will get back ASAP with results for MST panels too. 

Shubhangi Shrivastava (6):
  drm/i915: Splitting intel_dp_detect
  drm/i915: Cleaning up intel_dp_hpd_pulse
  drm/i915: Splitting intel_dp_check_link_status
  drm/i915: Save sink_count for tracking changes to it
  drm/i915: read sink_count dpcd always
  drm/i915: force full detect on sink count change

 drivers/gpu/drm/i915/intel_dp.c  | 170 +++++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h |   1 +
 2 files changed, 110 insertions(+), 61 deletions(-)

-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Fixing sink count related detection over

[Daniel Vetter]

On Mon, Nov 02, 2015 at 06:25:10PM +0530, Shubhangi Shrivastava wrote:
> This patch set cleans up DP detection logic to bring all DPCD
> operations at one place and to create a clear demarcation
> between handling of long and short pulses. This simplifies
> fixing of sink count related detection for DP panels.
> 
> Patches:
> 1. First two patches clean up intel_dp_detect and form a new
> function which will include all DPCD related operations.
> 2. Second patch splits up intel_dp_check_link_status to form
> a new function which will handle short pulse requests.
> 3. Last three patches fixes the detection logic related to
> sink count i.e detect changes in sink count and handle them
> appropriately. 
> 
> Note: this is tested on BXT with non-mst panels,
> will get back ASAP with results for MST panels too. 

Sivakumar and Ander are working on reorganizing all the DP hpd handling,
at least they have been before I went on vacation. I think it would make
sense to land that work first, before we start to apply functional fixes.

Please coordinate with them.
-Daniel

> 
> Shubhangi Shrivastava (6):
>   drm/i915: Splitting intel_dp_detect
>   drm/i915: Cleaning up intel_dp_hpd_pulse
>   drm/i915: Splitting intel_dp_check_link_status
>   drm/i915: Save sink_count for tracking changes to it
>   drm/i915: read sink_count dpcd always
>   drm/i915: force full detect on sink count change
> 
>  drivers/gpu/drm/i915/intel_dp.c  | 170 +++++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_drv.h |   1 +
>  2 files changed, 110 insertions(+), 61 deletions(-)
> 
> -- 
> 2.6.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Fixing sink count related detection over

[Shubhangi Shrivastava]

On Wednesday 18 November 2015 01:23 AM, Daniel Vetter wrote:
> On Mon, Nov 02, 2015 at 06:25:10PM +0530, Shubhangi Shrivastava wrote:
>> This patch set cleans up DP detection logic to bring all DPCD
>> operations at one place and to create a clear demarcation
>> between handling of long and short pulses. This simplifies
>> fixing of sink count related detection for DP panels.
>>
>> Patches:
>> 1. First two patches clean up intel_dp_detect and form a new
>> function which will include all DPCD related operations.
>> 2. Second patch splits up intel_dp_check_link_status to form
>> a new function which will handle short pulse requests.
>> 3. Last three patches fixes the detection logic related to
>> sink count i.e detect changes in sink count and handle them
>> appropriately.
>>
>> Note: this is tested on BXT with non-mst panels,
>> will get back ASAP with results for MST panels too.
> Sivakumar and Ander are working on reorganizing all the DP hpd handling,
> at least they have been before I went on vacation. I think it would make
> sense to land that work first, before we start to apply functional fixes.
>
> Please coordinate with them.
> -Daniel
Yes.. The reorganization of DP HPD handling is in progress.
I am working with Siva on the same. :)
We have found certain bugs in MST related code, pointed out
by Ander. Once it is fixed, we will re-upload the patches.
>
>> Shubhangi Shrivastava (6):
>>    drm/i915: Splitting intel_dp_detect
>>    drm/i915: Cleaning up intel_dp_hpd_pulse
>>    drm/i915: Splitting intel_dp_check_link_status
>>    drm/i915: Save sink_count for tracking changes to it
>>    drm/i915: read sink_count dpcd always
>>    drm/i915: force full detect on sink count change
>>
>>   drivers/gpu/drm/i915/intel_dp.c  | 170 +++++++++++++++++++++++++--------------
>>   drivers/gpu/drm/i915/intel_drv.h |   1 +
>>   2 files changed, 110 insertions(+), 61 deletions(-)
>>
>> -- 
>> 2.6.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Fixing sink count related detection over

[Ander Conselvan De Oliveira]

On Wed, 2015-11-18 at 11:12 +0530, Shubhangi Shrivastava wrote:
> 
> On Wednesday 18 November 2015 01:23 AM, Daniel Vetter wrote:
> > On Mon, Nov 02, 2015 at 06:25:10PM +0530, Shubhangi Shrivastava wrote:
> > > This patch set cleans up DP detection logic to bring all DPCD
> > > operations at one place and to create a clear demarcation
> > > between handling of long and short pulses. This simplifies
> > > fixing of sink count related detection for DP panels.
> > > 
> > > Patches:
> > > 1. First two patches clean up intel_dp_detect and form a new
> > > function which will include all DPCD related operations.
> > > 2. Second patch splits up intel_dp_check_link_status to form
> > > a new function which will handle short pulse requests.
> > > 3. Last three patches fixes the detection logic related to
> > > sink count i.e detect changes in sink count and handle them
> > > appropriately.
> > > 
> > > Note: this is tested on BXT with non-mst panels,
> > > will get back ASAP with results for MST panels too.
> > Sivakumar and Ander are working on reorganizing all the DP hpd handling,
> > at least they have been before I went on vacation. I think it would make
> > sense to land that work first, before we start to apply functional fixes.
> > 
> > Please coordinate with them.
> > -Daniel
> Yes.. The reorganization of DP HPD handling is in progress.
> I am working with Siva on the same. :)

I thought this was the series that reorganizes HPD. Should I be looking at some
other patches too?

Ander

> We have found certain bugs in MST related code, pointed out
> by Ander. Once it is fixed, we will re-upload the patches.
> > 
> > > Shubhangi Shrivastava (6):
> > >    drm/i915: Splitting intel_dp_detect
> > >    drm/i915: Cleaning up intel_dp_hpd_pulse
> > >    drm/i915: Splitting intel_dp_check_link_status
> > >    drm/i915: Save sink_count for tracking changes to it
> > >    drm/i915: read sink_count dpcd always
> > >    drm/i915: force full detect on sink count change
> > > 
> > >   drivers/gpu/drm/i915/intel_dp.c  | 170 +++++++++++++++++++++++++--------
> > > ------
> > >   drivers/gpu/drm/i915/intel_drv.h |   1 +
> > >   2 files changed, 110 insertions(+), 61 deletions(-)
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Fixing sink count related detection over

[Shubhangi Shrivastava]

On Wednesday 18 November 2015 02:53 PM, Ander Conselvan De Oliveira wrote:
> On Wed, 2015-11-18 at 11:12 +0530, Shubhangi Shrivastava wrote:
>> On Wednesday 18 November 2015 01:23 AM, Daniel Vetter wrote:
>>> On Mon, Nov 02, 2015 at 06:25:10PM +0530, Shubhangi Shrivastava wrote:
>>>> This patch set cleans up DP detection logic to bring all DPCD
>>>> operations at one place and to create a clear demarcation
>>>> between handling of long and short pulses. This simplifies
>>>> fixing of sink count related detection for DP panels.
>>>>
>>>> Patches:
>>>> 1. First two patches clean up intel_dp_detect and form a new
>>>> function which will include all DPCD related operations.
>>>> 2. Second patch splits up intel_dp_check_link_status to form
>>>> a new function which will handle short pulse requests.
>>>> 3. Last three patches fixes the detection logic related to
>>>> sink count i.e detect changes in sink count and handle them
>>>> appropriately.
>>>>
>>>> Note: this is tested on BXT with non-mst panels,
>>>> will get back ASAP with results for MST panels too.
>>> Sivakumar and Ander are working on reorganizing all the DP hpd handling,
>>> at least they have been before I went on vacation. I think it would make
>>> sense to land that work first, before we start to apply functional fixes.
>>>
>>> Please coordinate with them.
>>> -Daniel
>> Yes.. The reorganization of DP HPD handling is in progress.
>> I am working with Siva on the same. :)
> I thought this was the series that reorganizes HPD. Should I be looking at some
> other patches too?
>
> Ander
Its the same Ander.. :)
>
>> We have found certain bugs in MST related code, pointed out
>> by Ander. Once it is fixed, we will re-upload the patches.
>>>> Shubhangi Shrivastava (6):
>>>>     drm/i915: Splitting intel_dp_detect
>>>>     drm/i915: Cleaning up intel_dp_hpd_pulse
>>>>     drm/i915: Splitting intel_dp_check_link_status
>>>>     drm/i915: Save sink_count for tracking changes to it
>>>>     drm/i915: read sink_count dpcd always
>>>>     drm/i915: force full detect on sink count change
>>>>
>>>>    drivers/gpu/drm/i915/intel_dp.c  | 170 +++++++++++++++++++++++++--------
>>>> ------
>>>>    drivers/gpu/drm/i915/intel_drv.h |   1 +
>>>>    2 files changed, 110 insertions(+), 61 deletions(-)
>>>>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx