On Tue, 2016-01-19 at 16:07 +0530, Shubhangi Shrivastava wrote:
> Current DP detection has DPCD operations split across
> intel_dp_hpd_pulse and intel_dp_detect which contains
> duplicates as well. Also intel_dp_detect is called
> during modes enumeration as well which will result
> in multiple dpcd operations. So this patch tries
> to solve both these by bringing all DPCD operations
> in one single function and make intel_dp_detect
> use existing values instead of repeating same steps.
>
> v2: Pulled in a hunk from last patch of the series to
> this patch. (Ander)
> v3: Added MST hotplug handling. (Ander)
>
> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@xxxxxxxxx>
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@xxxxxxxxx>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 71 +++++++++++++++++++++++++---------------
> -
> 1 file changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8969ff9..82ee18d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
[...]
> @@ -4693,7 +4717,8 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
> return connector_status_disconnected;
> }
>
> - intel_dp_long_pulse(intel_dp->attached_connector);
> + if (force)
> + intel_dp_long_pulse(intel_dp->attached_connector);
I didn't notice this at first, but force is not the right thing to check for
here. It is basically intended to avoid doing load detection (see
intel_get_load_detect_pipe()) on automated polling. But we still have to try
detection here when force = false, otherwise this will cause a regression.
If you plug in a DP device while suspended, that device won't get detected,
since we don't get an HPD for it. Previously, the call do intel_dp_detect() with
force = false from intel_drm_resume() (via drm_helper_hpd_irq_event()) would
cause a full detection.
To avoid the repeated DPCD operations, I think we need a more explicit mechanism
to signal that we already handled the long pulse via the HPD handler. In
intel_dp_hpd_pulse() we could set a flag that tells we just handled a long pulse
for the given port. The call to intel_dp_long_pulse() in intel_dp_detect() would
then be dependent on that flag.
For that reason I have to retract my R-b from this patch.
Ander
> if (intel_connector->detect_edid)
> return connector_status_connected;
> @@ -5026,25 +5051,25 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
> /* indicate that we need to restart link training */
> intel_dp->train_set_valid = false;
>
> - if (!intel_digital_port_connected(dev_priv, intel_dig_port))
> - goto mst_fail;
> -
> - if (!intel_dp_get_dpcd(intel_dp)) {
> - goto mst_fail;
> - }
> -
> - intel_dp_probe_oui(intel_dp);
> + intel_dp_long_pulse(intel_dp->attached_connector);
> + if (intel_dp->is_mst)
> + ret = IRQ_HANDLED;
> + goto put_power;
>
> - if (!intel_dp_probe_mst(intel_dp)) {
> - drm_modeset_lock(&dev->mode_config.connection_mutex,
> NULL);
> - intel_dp_check_link_status(intel_dp);
> - drm_modeset_unlock(&dev
> ->mode_config.connection_mutex);
> - goto mst_fail;
> - }
> } else {
> if (intel_dp->is_mst) {
> - if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> - goto mst_fail;
> + if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
> + /*
> + * If we were in MST mode, and device is not
> + * there, get out of MST mode
> + */
> + DRM_DEBUG_KMS("MST device may have
> disappeared %d vs %d\n",
> + intel_dp->is_mst, intel_dp
> ->mst_mgr.mst_state);
> + intel_dp->is_mst = false;
> + drm_dp_mst_topology_mgr_set_mst(&intel_dp
> ->mst_mgr,
> + intel_dp
> ->is_mst);
> + goto put_power;
> + }
> }
>
> if (!intel_dp->is_mst) {
> @@ -5056,14 +5081,6 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
>
> ret = IRQ_HANDLED;
>
> - goto put_power;
> -mst_fail:
> - /* if we were in MST mode, and device is not there get out of MST
> mode */
> - if (intel_dp->is_mst) {
> - DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n",
> intel_dp->is_mst, intel_dp->mst_mgr.mst_state);
> - intel_dp->is_mst = false;
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp
> ->is_mst);
> - }
> put_power:
> intel_display_power_put(dev_priv, power_domain);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
