diff mbox

[4/5] drm/i915: Save sink_count for tracking changes to it and read sink_count dpcd always

Message ID 1455095087-9285-4-git-send-email-shubhangi.shrivastava@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shubhangi Shrivastava Feb. 10, 2016, 9:04 a.m. UTC
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(-)

Comments

Ander Conselvan de Oliveira Feb. 12, 2016, 5:18 p.m. UTC | #1
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;
diff mbox

Patch

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;