All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] drm/i915: Splitting intel_dp_detect
@ 2016-02-01 11:42 Shubhangi Shrivastava
  2016-02-01 11:42 ` [PATCH 2/5] drm/i915: Cleaning up intel_dp_hpd_pulse Shubhangi Shrivastava
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-01 11:42 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

intel_dp_detect() is called for not just detection but
during modes enumeration as well. Repeating the whole
sequence during each of these calls is wasteful and
time consuming.
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.
This change will be required by further patches in the series
to avoid performing duplicated DPCD operations on hotplug.

v2: Moved a hunk to next patch of the series.
    Moved intel_dp_unset_edid to out. (Ander)
v3: Rephrased commit message and intel_dp_unset_dp() is called
    within intel_dp_set_dp() to free the previous EDID. (Ander)
v4: Added overriding of status to disconnected for MST. (Ander)

Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
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 | 63 ++++++++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1761254..65028a4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -129,6 +129,7 @@ static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
 static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp);
 static void vlv_steal_power_sequencer(struct drm_device *dev,
 				      enum pipe pipe);
+static void intel_dp_unset_edid(struct intel_dp *intel_dp);
 
 static unsigned int intel_dp_unused_lane_mask(int lane_count)
 {
@@ -4577,6 +4578,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	struct edid *edid;
 
+	intel_dp_unset_edid(intel_dp);
 	edid = intel_dp_get_edid(intel_dp);
 	intel_connector->detect_edid = edid;
 
@@ -4597,9 +4599,10 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 	intel_dp->has_audio = false;
 }
 
-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;
@@ -4609,17 +4612,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_display_port_aux_power_domain(intel_encoder);
 	intel_display_power_get(to_i915(dev), power_domain);
 
@@ -4640,14 +4632,18 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		goto out;
 	}
 
+	if (intel_encoder->type != INTEL_OUTPUT_EDP)
+		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+
 	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 (intel_encoder->type != INTEL_OUTPUT_EDP)
-			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+		/*
+		 * If we are in MST mode then this connector
+		 * won't appear connected or have anything
+		 * with EDID on it
+		 */
 		status = connector_status_disconnected;
 		goto out;
 	}
@@ -4662,8 +4658,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 
 	intel_dp_set_edid(intel_dp);
 
-	if (intel_encoder->type != INTEL_OUTPUT_EDP)
-		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 	status = connector_status_connected;
 
 	/* Try to read the source of the interrupt */
@@ -4681,8 +4675,37 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	}
 
 out:
+	if (status != connector_status_connected)
+		intel_dp_unset_edid(intel_dp);
 	intel_display_power_put(to_i915(dev), 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 */
+		intel_dp_unset_edid(intel_dp);
+		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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/5] drm/i915: Cleaning up intel_dp_hpd_pulse
  2016-02-01 11:42 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
@ 2016-02-01 11:42 ` Shubhangi Shrivastava
  2016-02-01 11:42 ` [PATCH 3/5] drm/i915: Reorganizing intel_dp_check_link_status Shubhangi Shrivastava
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-01 11:42 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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)
v3: Added MST hotplug handling. (Ander)

Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
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 | 65 +++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 65028a4..ce99d59 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4646,6 +4646,16 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		 */
 		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;
 	}
 
 	/*
@@ -4675,8 +4685,21 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	}
 
 out:
-	if (status != connector_status_connected)
+	if (status != connector_status_connected) {
 		intel_dp_unset_edid(intel_dp);
+		/*
+		 * If we were in MST mode, and device is not there,
+		 * get out of MST mode
+		 */
+		if (intel_dp->is_mst) {
+			DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n",
+				intel_dp->is_mst, intel_dp->mst_mgr.mst_state);
+			intel_dp->is_mst = false;
+			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
+							intel_dp->is_mst);
+		}
+	}
+
 	intel_display_power_put(to_i915(dev), power_domain);
 	return;
 }
@@ -5033,25 +5056,25 @@ 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);
+		if (intel_dp->is_mst)
+			ret = IRQ_HANDLED;
+		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)
-				goto mst_fail;
+			if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
+				/*
+				 * If we were in MST mode, and device is not
+				 * there, get out of MST mode
+				 */
+				DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n",
+					intel_dp->is_mst, intel_dp->mst_mgr.mst_state);
+				intel_dp->is_mst = false;
+				drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
+								intel_dp->is_mst);
+				goto put_power;
+			}
 		}
 
 		if (!intel_dp->is_mst) {
@@ -5063,14 +5086,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 
 	ret = IRQ_HANDLED;
 
-	goto put_power;
-mst_fail:
-	/* if we were in MST mode, and device is not there get out of MST mode */
-	if (intel_dp->is_mst) {
-		DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n", intel_dp->is_mst, intel_dp->mst_mgr.mst_state);
-		intel_dp->is_mst = false;
-		drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
-	}
 put_power:
 	intel_display_power_put(dev_priv, power_domain);
 
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/5] drm/i915: Reorganizing intel_dp_check_link_status
  2016-02-01 11:42 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
  2016-02-01 11:42 ` [PATCH 2/5] drm/i915: Cleaning up intel_dp_hpd_pulse Shubhangi Shrivastava
@ 2016-02-01 11:42 ` Shubhangi Shrivastava
  2016-02-01 11:42 ` [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always Shubhangi Shrivastava
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-01 11:42 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

When created originally intel_dp_check_link_status()
was supposed to handle only link training for short
pulse but has grown into handler for short pulse itself.
This patch cleans up this function by splitting it into
two halves. First intel_dp_short_pulse() is called,
which will be entry point and handle all logic for
short pulse handling while intel_dp_check_link_status()
will retain its original purpose of only doing link
status related work.

intel_dp_short_pulse: All existing code other than
link status read and link training upon error status.

intel_dp_check_link_status:
The link status should be read on short pulse
irrespective of panel being enabled or not so
intel_dp_get_link_status() performs dpcd read first
then based on crtc active / enabled it will
perform the link training.

This is because short pulse is a generic interrupt
which should always be handled, because it may mean:
	1. Hotplug/unplug of MST panel
	2. Hotplug/unplug of dongle
	3. Link status change for other DP panels

v2: Added WARN_ON to intel_dp_check_link_status()
    Removed a call to intel_dp_get_link_status() (Ander)

v3: Changed commit message to explain need of link status
    being read before performing encoder checks (Daniel)

v4: Changed commit message to explain need of reading
    link status on short pulse (Ander)

Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
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 | 65 +++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ce99d59..9cb6ea9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4279,6 +4279,36 @@ 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;
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	u8 link_status[DP_LINK_STATUS_SIZE];
+
+	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+
+	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 link training is requested we should perform it always */
+	if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
+		(!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:
@@ -4288,14 +4318,10 @@ 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));
 
 	/*
 	 * Clearing compliance test variables to allow capturing
@@ -4305,17 +4331,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 	intel_dp->compliance_test_type = 0;
 	intel_dp->compliance_test_data = 0;
 
-	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;
-	}
-
 	/* Now read the DPCD to see if it's actually running */
 	if (!intel_dp_get_dpcd(intel_dp)) {
 		return;
@@ -4335,14 +4350,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
-	/* if link training is requested we should perform it always */
-	if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
-		(!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 */
@@ -5077,11 +5087,8 @@ 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);
-		}
+		if (!intel_dp->is_mst)
+			intel_dp_short_pulse(intel_dp);
 	}
 
 	ret = IRQ_HANDLED;
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always
  2016-02-01 11:42 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
  2016-02-01 11:42 ` [PATCH 2/5] drm/i915: Cleaning up intel_dp_hpd_pulse Shubhangi Shrivastava
  2016-02-01 11:42 ` [PATCH 3/5] drm/i915: Reorganizing intel_dp_check_link_status Shubhangi Shrivastava
@ 2016-02-01 11:42 ` Shubhangi Shrivastava
  2016-02-01 11:42 ` [PATCH 5/5] drm/i915: force full detect on sink count change Shubhangi Shrivastava
  2016-02-01 12:15 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Splitting intel_dp_detect Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-01 11:42 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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: Storing value of intel_dp->sink_count that is ready
    for consumption. (Ander)
    Squashing two commits into one. (Ander)

v3: Added comment to explain the need of early return when
    sink count is 0. (Ander)

Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
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  | 30 +++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9cb6ea9..539f5a3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3855,6 +3855,27 @@ 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;
+
+	/*
+	 * Sink count can change between short pulse hpd hence
+	 * a member variable in intel_dp will track any changes
+	 * between short pulse interrupts.
+	 */
+	intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
+
+	/*
+	 * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that
+	 * a dongle is present but no display. Unless we require to know
+	 * if a dongle is present or not, we don't need to update
+	 * downstream port information. So, an early return here saves
+	 * time from performing other operations which are not required.
+	 */
+	if (!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)) {
@@ -4372,14 +4393,9 @@ 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)
-			return connector_status_unknown;
 
-		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
-					      : connector_status_disconnected;
+		return 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 059b46e..0879466 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -774,6 +774,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/5] drm/i915: force full detect on sink count change
  2016-02-01 11:42 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
                   ` (2 preceding siblings ...)
  2016-02-01 11:42 ` [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always Shubhangi Shrivastava
@ 2016-02-01 11:42 ` Shubhangi Shrivastava
  2016-02-01 12:15 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Splitting intel_dp_detect Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-01 11:42 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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)

v4: changed comments to indicate meaning of return value of
    intel_dp_short_pulse and explain the use of return value
    from intel_dp_get_dpcd in intel_dp_short_pulse (Ander)

Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 539f5a3..dd433e7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4337,12 +4337,19 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
  *  2. Configure link according to Receiver Capabilities
  *  3. Use Link Training from 2.5.3.3 and 3.5.1.3
  *  4. Check link status on receipt of hot-plug interrupt
+ *
+ * intel_dp_short_pulse -  handles short pulse interrupts
+ * when full detection is not required.
+ * Returns %true if short pulse is handled and full detection
+ * is NOT required and %false otherwise.
  */
-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 old_sink_count = intel_dp->sink_count;
+	bool ret;
 
 	/*
 	 * Clearing compliance test variables to allow capturing
@@ -4352,9 +4359,17 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 	intel_dp->compliance_test_type = 0;
 	intel_dp->compliance_test_data = 0;
 
-	/* 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
+	 * If the current value of sink count doesn't match with
+	 * the value that was stored earlier or 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 */
@@ -4374,6 +4389,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 */
@@ -5103,8 +5120,12 @@ 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->is_mst) {
+			if (!intel_dp_short_pulse(intel_dp)) {
+				intel_dp_long_pulse(intel_dp->attached_connector);
+				goto put_power;
+			}
+		}
 	}
 
 	ret = IRQ_HANDLED;
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Splitting intel_dp_detect
  2016-02-01 11:42 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
                   ` (3 preceding siblings ...)
  2016-02-01 11:42 ` [PATCH 5/5] drm/i915: force full detect on sink count change Shubhangi Shrivastava
@ 2016-02-01 12:15 ` Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2016-02-01 12:15 UTC (permalink / raw)
  To: Shubhangi Shrivastava; +Cc: intel-gfx

== Summary ==

Series 2978v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/2978/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (hsw-gt2)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (bsw-nuc-2)

bdw-nuci7        total:156  pass:147  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:159  pass:147  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:159  pass:129  dwarn:0   dfail:0   fail:0   skip:30 
byt-nuc          total:159  pass:136  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox      total:159  pass:146  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2          total:159  pass:149  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p      total:159  pass:111  dwarn:0   dfail:0   fail:0   skip:48 
ivb-t430s        total:159  pass:145  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2        total:159  pass:144  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps      total:159  pass:137  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220t        total:159  pass:137  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1335/

6b1049b84dcd979f631d15b2ada325d8e5b2c4e1 drm-intel-nightly: 2016y-01m-29d-22h-50m-57s UTC integration manifest
115af193cac6d62145809df8292fadbd3c370fb2 drm/i915: force full detect on sink count change
c8dced60ddfc26d53e37a7607767046880cc5178 drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always
9e65fb79da24330663dcbc888afeb61300507e71 drm/i915: Reorganizing intel_dp_check_link_status
c2e3381bf553cade9bbe15bc9f21d1677e8940e8 drm/i915: Cleaning up intel_dp_hpd_pulse
88fa9d50f75d5d5d939f49ce6441fd39c181acc2 drm/i915: Splitting intel_dp_detect

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always
  2016-02-10  9:04 ` [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always Shubhangi Shrivastava
@ 2016-02-12 17:18   ` Ander Conselvan De Oliveira
  0 siblings, 0 replies; 11+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-02-12 17:18 UTC (permalink / raw)
  To: Shubhangi Shrivastava, intel-gfx

I think I mentioned something about the patch summary (the title) being too big
before. The kernel documentation on submitting patches [1] says: "the "summary"
must be no more than 70-75 characters".

[1] https://www.kernel.org/doc/Documentation/SubmittingPatches

Cheers,
Ander

On Wed, 2016-02-10 at 14:34 +0530, Shubhangi Shrivastava wrote:
> 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: Storing value of intel_dp->sink_count that is ready
>     for consumption. (Ander)
>     Squashing two commits into one. (Ander)
> 
> v3: Added comment to explain the need of early return when
>     sink count is 0. (Ander)
> 
> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 30 +++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9fe78e1..a834c5f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3862,6 +3862,27 @@ 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;
> +
> +	/*
> +	 * Sink count can change between short pulse hpd hence
> +	 * a member variable in intel_dp will track any changes
> +	 * between short pulse interrupts.
> +	 */
> +	intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
> +
> +	/*
> +	 * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that
> +	 * a dongle is present but no display. Unless we require to know
> +	 * if a dongle is present or not, we don't need to update
> +	 * downstream port information. So, an early return here saves
> +	 * time from performing other operations which are not required.
> +	 */
> +	if (!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)) {
> @@ -4379,14 +4400,9 @@ 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)
> -			return connector_status_unknown;
>  
> -		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
> -					      :
> connector_status_disconnected;
> +		return 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 3d003d6..c04db07 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -752,6 +752,7 @@ struct intel_dp {
>  	uint32_t DP;
>  	int link_rate;
>  	uint8_t lane_count;
> +	uint8_t sink_count;
>  	bool has_audio;
>  	bool detect_done;
>  	enum hdmi_force_audio force_audio;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always
  2016-02-10  9:04 [PATCH 1/5] " Shubhangi Shrivastava
@ 2016-02-10  9:04 ` Shubhangi Shrivastava
  2016-02-12 17:18   ` Ander Conselvan De Oliveira
  0 siblings, 1 reply; 11+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-10  9:04 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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: Storing value of intel_dp->sink_count that is ready
    for consumption. (Ander)
    Squashing two commits into one. (Ander)

v3: Added comment to explain the need of early return when
    sink count is 0. (Ander)

Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 30 +++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9fe78e1..a834c5f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3862,6 +3862,27 @@ 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;
+
+	/*
+	 * Sink count can change between short pulse hpd hence
+	 * a member variable in intel_dp will track any changes
+	 * between short pulse interrupts.
+	 */
+	intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
+
+	/*
+	 * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that
+	 * a dongle is present but no display. Unless we require to know
+	 * if a dongle is present or not, we don't need to update
+	 * downstream port information. So, an early return here saves
+	 * time from performing other operations which are not required.
+	 */
+	if (!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)) {
@@ -4379,14 +4400,9 @@ 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)
-			return connector_status_unknown;
 
-		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
-					      : connector_status_disconnected;
+		return 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 3d003d6..c04db07 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -752,6 +752,7 @@ struct intel_dp {
 	uint32_t DP;
 	int link_rate;
 	uint8_t lane_count;
+	uint8_t sink_count;
 	bool has_audio;
 	bool detect_done;
 	enum hdmi_force_audio force_audio;
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always
  2016-01-19 10:37 ` [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always Shubhangi Shrivastava
@ 2016-01-20 14:31   ` Ander Conselvan De Oliveira
  0 siblings, 0 replies; 11+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-01-20 14:31 UTC (permalink / raw)
  To: Shubhangi Shrivastava, intel-gfx

The patch summary is now too long. Just use the one from patch 5 ("read
sink_count dpcd always"). You can mention in the commit message that the value
of sink count is now stored in struct intel_dp, but that doesn't need to be in
the summary.

One more comment below.
On Tue, 2016-01-19 at 16:07 +0530, Shubhangi Shrivastava wrote:
> 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.
> 
> 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: Storing value of intel_dp->sink_count that is ready
>     for consumption. (Ander)
>     Squashing two commits into one. (Ander)
> 
> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
> 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  | 18 +++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f8d9611..cdf4919 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3855,6 +3855,15 @@ 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;
> +
> +	intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
> +
> +	if (!intel_dp->sink_count)
> +		return false;
> +

You convinced me on the other email about this early return. Just add a comment
here with that justification.

With those two things, this is

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

>  	/* Check if the panel supports PSR */
>  	memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
>  	if (is_edp(intel_dp)) {
> @@ -4372,14 +4381,9 @@ 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)
> -			return connector_status_unknown;
>  
> -		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
> -					      :
> connector_status_disconnected;
> +		return 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 059b46e..0879466 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -774,6 +774,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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always
  2016-01-19 10:37 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
@ 2016-01-19 10:37 ` Shubhangi Shrivastava
  2016-01-20 14:31   ` Ander Conselvan De Oliveira
  0 siblings, 1 reply; 11+ messages in thread
From: Shubhangi Shrivastava @ 2016-01-19 10:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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.

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: Storing value of intel_dp->sink_count that is ready
    for consumption. (Ander)
    Squashing two commits into one. (Ander)

Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
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  | 18 +++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f8d9611..cdf4919 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3855,6 +3855,15 @@ 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;
+
+	intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
+
+	if (!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)) {
@@ -4372,14 +4381,9 @@ 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)
-			return connector_status_unknown;
 
-		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
-					      : connector_status_disconnected;
+		return 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 059b46e..0879466 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -774,6 +774,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always
  2016-01-19 10:20 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
@ 2016-01-19 10:21 ` Shubhangi Shrivastava
  0 siblings, 0 replies; 11+ messages in thread
From: Shubhangi Shrivastava @ 2016-01-19 10:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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.

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: Storing value of intel_dp->sink_count that is ready
    for consumption. (Ander)
    Squashing two commits into one. (Ander)

Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
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  | 18 +++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f8d9611..cdf4919 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3855,6 +3855,15 @@ 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;
+
+	intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
+
+	if (!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)) {
@@ -4372,14 +4381,9 @@ 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)
-			return connector_status_unknown;
 
-		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
-					      : connector_status_disconnected;
+		return 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 059b46e..0879466 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -774,6 +774,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-02-12 17:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-01 11:42 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
2016-02-01 11:42 ` [PATCH 2/5] drm/i915: Cleaning up intel_dp_hpd_pulse Shubhangi Shrivastava
2016-02-01 11:42 ` [PATCH 3/5] drm/i915: Reorganizing intel_dp_check_link_status Shubhangi Shrivastava
2016-02-01 11:42 ` [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always Shubhangi Shrivastava
2016-02-01 11:42 ` [PATCH 5/5] drm/i915: force full detect on sink count change Shubhangi Shrivastava
2016-02-01 12:15 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Splitting intel_dp_detect Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2016-02-10  9:04 [PATCH 1/5] " Shubhangi Shrivastava
2016-02-10  9:04 ` [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always Shubhangi Shrivastava
2016-02-12 17:18   ` Ander Conselvan De Oliveira
2016-01-19 10:37 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
2016-01-19 10:37 ` [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always Shubhangi Shrivastava
2016-01-20 14:31   ` Ander Conselvan De Oliveira
2016-01-19 10:20 [PATCH 1/5] drm/i915: Splitting intel_dp_detect Shubhangi Shrivastava
2016-01-19 10:21 ` [PATCH 4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always Shubhangi Shrivastava

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.