[Intel-gfx] [PATCH 1/2] drm/i915: Read sink_count dpcd always for short hpd

Sivakumar Thulasimani sivakumar.thulasimani at intel.com
Mon Aug 17 21:36:39 PDT 2015 


On 8/17/2015 5:39 PM, Jani Nikula wrote:
> On Mon, 17 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani at intel.com> wrote:
>> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani at intel.com>
>>
>> Compliance test 4.2.2.8 requires driver to read the sink_count for
>> short pulse interrupt even when the panel is not enabled.
>> This patch performs the following
>> a) reading sink_count by reusing intel_dp_detect_dpcd
>> instead of using intel_dp_get_dpcd
>> b) moving crtc enabled checks post sink_count read call
>>
>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c |  117 ++++++++++++++++++++-------------------
>>   1 file changed, 59 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index b905c19..e4de8e5 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4342,6 +4342,56 @@ go_again:
>>   	return -EINVAL;
>>   }
>>   
>> +/* XXX this is probably wrong for multiple downstream ports */
>> +static enum drm_connector_status
>> +intel_dp_detect_dpcd(struct intel_dp *intel_dp)
>> +{
>> +	uint8_t *dpcd = intel_dp->dpcd;
>> +	uint8_t type;
>> +
>> +	if (!intel_dp_get_dpcd(intel_dp))
>> +		return connector_status_disconnected;
>> +
>> +	/* if there's no downstream port, we're done */
>> +	if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
>> +		return connector_status_connected;
>> +
>> +	/* 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;
>> +	}
>> +
>> +	/* If no HPD, poke DDC gently */
>> +	if (drm_probe_ddc(&intel_dp->aux.ddc))
>> +		return connector_status_connected;
>> +
>> +	/* Well we tried, say unknown for unreliable port types */
>> +	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
>> +		type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
>> +		if (type == DP_DS_PORT_TYPE_VGA ||
>> +		    type == DP_DS_PORT_TYPE_NON_EDID)
>> +			return connector_status_unknown;
>> +	} else {
>> +		type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>> +			DP_DWN_STRM_PORT_TYPE_MASK;
>> +		if (type == DP_DWN_STRM_PORT_TYPE_ANALOG ||
>> +		    type == DP_DWN_STRM_PORT_TYPE_OTHER)
>> +			return connector_status_unknown;
>> +	}
>> +
>> +	/* Anything else is out of spec, warn and ignore */
>> +	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
>> +	return connector_status_disconnected;
>> +}
> Please either a) just add a forward declaration for
> intel_dp_detect_dpcd, or b) add a separate non-functional prep patch
> that moves the function around. Please don't combine code movement with
> functional changes.
>
> BR,
> Jani.
uploaded V2 of the patch with forward declaration.
>
>> +
>>   /*
>>    * According to DP spec
>>    * 5.1.2:
>> @@ -4362,21 +4412,22 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>>   
>>   	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>>   
>> -	if (!intel_encoder->base.crtc)
>> +	/* 4.2.2.8 requires source to read link_status, 0 - 12 DPCD &
>> +	 * sink_count even for short pulse irrespective of the sink is
>> +	 * in use or not
>> +	 */
>> +	if (!intel_dp_get_link_status(intel_dp, link_status))
>>   		return;
>>   
>> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>> +	/* reuse to read both 0 - 12 DPCD & sink_count */
>> +	if (intel_dp_detect_dpcd(intel_dp) != connector_status_connected)
>>   		return;
>>   
>> -	/* Try to read receiver status if the link appears to be up */
>> -	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>> +	if (!intel_encoder->base.crtc)
>>   		return;
>> -	}
>>   
>> -	/* Now read the DPCD to see if it's actually running */
>> -	if (!intel_dp_get_dpcd(intel_dp)) {
>> +	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>>   		return;
>> -	}
>>   
>>   	/* Try to read the source of the interrupt */
>>   	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
>> @@ -4401,56 +4452,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>>   	}
>>   }
>>   
>> -/* XXX this is probably wrong for multiple downstream ports */
>> -static enum drm_connector_status
>> -intel_dp_detect_dpcd(struct intel_dp *intel_dp)
>> -{
>> -	uint8_t *dpcd = intel_dp->dpcd;
>> -	uint8_t type;
>> -
>> -	if (!intel_dp_get_dpcd(intel_dp))
>> -		return connector_status_disconnected;
>> -
>> -	/* if there's no downstream port, we're done */
>> -	if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
>> -		return connector_status_connected;
>> -
>> -	/* 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;
>> -	}
>> -
>> -	/* If no HPD, poke DDC gently */
>> -	if (drm_probe_ddc(&intel_dp->aux.ddc))
>> -		return connector_status_connected;
>> -
>> -	/* Well we tried, say unknown for unreliable port types */
>> -	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
>> -		type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
>> -		if (type == DP_DS_PORT_TYPE_VGA ||
>> -		    type == DP_DS_PORT_TYPE_NON_EDID)
>> -			return connector_status_unknown;
>> -	} else {
>> -		type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>> -			DP_DWN_STRM_PORT_TYPE_MASK;
>> -		if (type == DP_DWN_STRM_PORT_TYPE_ANALOG ||
>> -		    type == DP_DWN_STRM_PORT_TYPE_OTHER)
>> -			return connector_status_unknown;
>> -	}
>> -
>> -	/* Anything else is out of spec, warn and ignore */
>> -	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
>> -	return connector_status_disconnected;
>> -}
>> -
>>   static enum drm_connector_status
>>   edp_detect(struct intel_dp *intel_dp)
>>   {
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
regards,
Sivakumar

More information about the Intel-gfx mailing list