On 4/8/2015 11:53 AM, Paulo Zanoni wrote:
2015-04-01 15:00 GMT-03:00 Todd Previte <tprevite@xxxxxxxxx>:
Update the hot plug function to handle the SST case. Instead of placing
the SST case within the long/short pulse block, it is now handled after
determining that MST mode is not in use. This way, the topology management
layer can handle any MST-related operations while SST operations are still
correctly handled afterwards.
This patch also corrects the problem of SST mode only being handled in the
case of a short (0.5ms - 1.0ms) HPD pulse.
It's not clear to me what exactly is the problem with the current
code. Can you please clarify?
The main problem is that the comment in the code there is wrong. We
*don't* check the link status later as it says it will. The legacy HPD
handler (intel_dp_hot_plug) has been gutted but the function is still
there in the code. It probably should have been removed when the new
handler was implemented, but that's another matter entirely.
So as things stand, the only time we actually check the link status is
when we get a short pulse on the HPD line. Long pulses (i.e.
connect/disconnect events) don't get processed. This code corrects that
problem and properly handles both long and short HPD pulses for the SST
mode.
For compliance testing purposes
both short and long pulses are used by the different tests, thus both cases
need to be addressed for SST.
This patch replaces [PATCH 10/10] drm/i915: Fix intel_dp_hot_plug() in the
previous compliance testing patch sequence. Review feedback on that patch
indicated that updating intel_dp_hot_plug() was not the correct place for
the test handler.
For the SST case, the main stream is disabled for long HPD pulses as this
generally indicates either a connect/disconnect event or link failure. For
a number of case in compliance testing, the source is required to disable
the main link upon detection of a long HPD.
V2:
- N/A
V3:
- Place the SST mode link status check into the mst_fail case
- Remove obsolete comment regarding SST mode operation
- Removed an erroneous line of code that snuck in during rebasing
V4:
- Added a disable of the main stream (DP transport) for the long pulse case
for SST to support compliance testing
V5:
- Reworked SST handling to support tests 4.2.2.7 and 4.2.2.8
Signed-off-by: Todd Previte <tprevite@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_dp.c | 28 +++++++++++-----------------
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 16d35903..0a77f5a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4622,16 +4622,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
goto mst_fail;
}
-
- if (!intel_dp->is_mst) {
- /*
- * we'll check the link status via the normal hot plug path later -
- * but for short hpds we should check it now
- */
- drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
- intel_dp_check_link_status(intel_dp);
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
- }
}
ret = IRQ_HANDLED;
@@ -4639,18 +4629,22 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
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);
- } else {
- /* SST mode - handle short/long pulses here */
+ DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n", intel_dp->is_mst, intel_dp->mst_mgr.mst_state);
So now aren't we going to get MST log messages on non-MST cases, such
as a disconnect with long_hpd? I mean, even non-MST jumps to the
mst_fail label.
Yeah that got removed by mistake. I corrected this for the next version
of the patch.
+ intel_dp->is_mst = false;
+ drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
+
+put_power:
+ /* SST mode - handle short/long pulses here */
+ if (!intel_dp->is_mst) {
+ /* TO DO: Handle short/long pulses individually
+ Can save on training times and overhead by not doing
+ full link status updating/processing for short pulses
+ */
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
intel_dp_check_link_status(intel_dp);
drm_modeset_unlock(&dev->mode_config.connection_mutex);
ret = IRQ_HANDLED;
}
-put_power:
intel_display_power_put(dev_priv, power_domain);
return ret;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
