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

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

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