Re: [PATCH] drm/i915: Improve DP downstream HPD handling

[Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>]

On 7/7/2015 4:40 PM, Ville Syrjälä wrote:
> On Tue, Jul 07, 2015 at 03:26:36PM +0530, Sivakumar Thulasimani wrote:
>>
>> On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> DP dongles may signal downstream HPD via short HPD pulses. If we know
>>> the device has a HPD capable downstream port, make sure we kick off the
>>> full hotplug processing even for short HPDs.
>>>
>>> Additonally setting the sink to DPMS off kills the downstream HPD (at
>>> least on my DP->VGA dongle), so skip the DPMS off for such dongles
>>> when we turn off the port.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
>>>    1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index e88cec2..f424833 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>>>    	}
>>>    }
>>>    
>>> +static bool intel_dp_has_downstream_hpd(struct intel_dp *intel_dp)
>>> +{
>>> +	return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
>>> +		intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
>>> +		intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
>>> +}
>>> +
>>>    static void intel_disable_dp(struct intel_encoder *encoder)
>>>    {
>>>    	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>>> @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
>>>    	 * ensure that we have vdd while we switch off the panel. */
>>>    	intel_edp_panel_vdd_on(intel_dp);
>>>    	intel_edp_backlight_off(intel_dp);
>>> -	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>> +	/* Skip power down to keep downstream HPD working */
>>> +	if (!intel_dp_has_downstream_hpd(intel_dp))
>>> +		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>>    	intel_edp_panel_off(intel_dp);
>>>    
>>>    	/* disable the port before the pipe on g4x */
>>> @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>>>    			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>>>    			intel_dp_check_link_status(intel_dp);
>>>    			drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>> +
>>> +			/*
>>> +			 * Downstream HPD will generate a short HPD,
>>> +			 * so we want full hotplug processing here.
>>> +			 */
>>> +			if (intel_dp_has_downstream_hpd(intel_dp))
>>> +				goto put_power;
>>>    		}
>>>    	}
>>>    
>> I am looking into compliance changes for DP and this seems a relevant
>> change for compliance as well. but as per Link CTS 1.2 section 4.2.2.8,
>> we are supposed to read the sink_count and do full detection if
>> sink_count is >1.  So instead of checking for DP_DS_PORT_HPD can we just
>> check SINK_COUNT and do full detect ?
> ->detect() will be called from the hotplug work and that will
> check SINK_COUNT.
>
No, the Compliance Sink tool, will not set the DP_DS_PORT_HPD resulting 
in detect not getting executed for
the short pulse generated. The spec requires the sink to set only the 
sink count so it is not a must for
the sink to update the DP_DOWNSTREAM_PORT_0. so only a check for 
SINK_COUNT will pass the
compliance test.

-- 
regards,
Sivakumar

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