On Tue, Jul 07, 2015 at 04:45:11PM +0530, Sivakumar Thulasimani wrote: > > > 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@xxxxxxxxxxxxxxx wrote: > >>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >>> > >>> 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@xxxxxxxxxxxxxxx> > >>> --- > >>> 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. That seems stupid. If the downstream port isn't HPD capable then we have no reason to check SINK_COUNT after a short HPD as the short HPD coudln't have been caused by a downstram HPD. Obviuously we still check SINK_COUNT after a long HPD to figure out if anything is connected when the branch device itself gets connected to the source. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx