On Wednesday 13 January 2016 08:34 PM, Ander Conselvan De Oliveira wrote:
On Tue, 2016-01-05 at 18:20 +0530, Shubhangi Shrivastava wrote:
When created originally intel_dp_check_link_status()
was supposed to handle only link training for short
pulse but has grown into handler for short pulse itself.
This patch cleans up this function by splitting it into
two halves. First intel_dp_short_pulse() is called,
which will be entry point and handle all logic for
short pulse handling while intel_dp_check_link_status()
will retain its original purpose of only doing link
status related work.
The link retraining part when EQ is not correct is
retained to intel_dp_check_link_status whereas other
operations are handled as part of intel_dp_short_pulse.
This change is required to avoid performing all DPCD
related operations on performing link retraining.
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 | 56 ++++++++++++++++++++++++----------------
-
1 file changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 137757b..842790e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4289,6 +4289,33 @@ go_again:
return -EINVAL;
}
+static void
+intel_dp_check_link_status(struct intel_dp *intel_dp)
+{
+ struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)
->base;
+ u8 link_status[DP_LINK_STATUS_SIZE];
+
+ if (!intel_dp_get_link_status(intel_dp, link_status)) {
+ DRM_ERROR("Failed to get link status\n");
+ return;
+ }
+
+ if (!intel_encoder->base.crtc)
+ return;
+
+ if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+ return;
+
+ /* if link training is requested we should perform it always */
+ if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
+ (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
+ DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
+ intel_encoder->base.name);
+ intel_dp_start_link_train(intel_dp);
+ intel_dp_stop_link_train(intel_dp);
+ }
+}
+
/*
* According to DP spec
* 5.1.2:
@@ -4298,15 +4325,12 @@ go_again:
* 4. Check link status on receipt of hot-plug interrupt
*/
static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
+intel_dp_short_pulse(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
- struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)
->base;
u8 sink_irq_vector;
u8 link_status[DP_LINK_STATUS_SIZE];
- WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-
I think it's better to move this WARN to the new intel_dp_check_link_status().
Sure.. Done..
/*
* Clearing compliance test variables to allow capturing
* of values for next automated test request.
@@ -4315,12 +4339,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
intel_dp->compliance_test_type = 0;
intel_dp->compliance_test_data = 0;
- if (!intel_encoder->base.crtc)
- return;
-
- if (!to_intel_crtc(intel_encoder->base.crtc)->active)
- return;
-
/* Try to read receiver status if the link appears to be up */
if (!intel_dp_get_link_status(intel_dp, link_status)) {
return;
There is now two calls to intel_dp_get_link_status()and the value of link_status
is not used in this function, so maybe just remove it from here. Looks good
otherwise.
Ander
Sure.. Done..
@@ -4345,14 +4363,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
DRM_DEBUG_DRIVER("CP or sink specific irq
unhandled\n");
}
- /* if link training is requested we should perform it always */
- if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
- (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
- DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
- intel_encoder->base.name);
- intel_dp_start_link_train(intel_dp);
- intel_dp_stop_link_train(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);
}
/* XXX this is probably wrong for multiple downstream ports */
@@ -5080,11 +5093,8 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
}
}
- if (!intel_dp->is_mst) {
- drm_modeset_lock(&dev->mode_config.connection_mutex,
NULL);
- intel_dp_check_link_status(intel_dp);
- drm_modeset_unlock(&dev
->mode_config.connection_mutex);
- }
+ if (!intel_dp->is_mst)
+ intel_dp_short_pulse(intel_dp);
}
ret = IRQ_HANDLED;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
