On Tuesday 26 January 2016 06:52 PM, Ander Conselvan De Oliveira wrote:
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
Call to intel_dp_detect() from get_modes is with force set to true while
from resume the call is with force set to false.. It should be in the
opposite manner as get_modes should not require full detection whereas
resume should. So, this needs to be cleaned up there. After merge of
these patches, will look into cleaning up that part of the code.
Moreover, intel_dp_detect() will be called from
drm_helper_hpd_irq_event() in polling scenarios only (when
DRM_CONNECTOR_POLL_HPD flag is set in connector->polled). So, seems like
this code here, doesn't really create a regression for realtime scenarios.
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
