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

[Shubhangi Shrivastava]

On Thursday 14 January 2016 06:34 PM, Ander Conselvan De Oliveira wrote:
> On Tue, 2016-01-05 at 18:20 +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
>>
>> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu at intel.com>
>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani at intel.com>
>> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava at 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 c2e8516..0d58bfd 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3865,6 +3865,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;
>> +
> My understanding is that this function should only read the DPCD data while
> detection based on that data is done in intel_dp_detect_dpcd(). With the return
> on sink_count == 0 here, we skip the end of the function, which updates the
> cached downstream port information. Is there a reason why we need this early
> return here?
>
> Also, I think this could be squashed with the previous patch.
>
> Ander
As described in the commit message, if sink_count is 0, then there is no 
display present. So, irrespective of value of downstream port, we should 
terminate the function and thus, an early return is present here.

>>   	/* Check if the panel supports PSR */
>>   	memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
>>   	if (is_edp(intel_dp)) {
>> @@ -4386,10 +4393,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;
>>   	}