On Thursday 14 January 2016 07:20 PM, Ander Conselvan De Oliveira wrote:
On Tue, 2016-01-05 at 18:20 +0530, Shubhangi Shrivastava wrote:
This patch checks for changes in sink count between short pulse
hpds and forces full detect when there is a change.
This will allow both detection of hotplug and unplug of panels
through dongles that give only short pulse for such events.
v2: changed variable type from u8 to bool (Jani)
return immediately if perform_full_detect is set(Siva)
v3: changed method of determining full detection from using
pointer to return code (Siva)
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 | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0d58bfd..8a659ee 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4331,12 +4331,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
* 3. Use Link Training from 2.5.3.3 and 3.5.1.3
* 4. Check link status on receipt of hot-plug interrupt
*/
-static void
+static bool
Please expand the comment above to indicate what the return value of this
function is supposed to mean.
Sure.. Will add..
intel_dp_short_pulse(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
u8 sink_irq_vector;
u8 link_status[DP_LINK_STATUS_SIZE];
+ u8 old_sink_count = intel_dp->sink_count;
+ bool ret;
/*
* Clearing compliance test variables to allow capturing
@@ -4348,12 +4350,20 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
/* Try to read receiver status if the link appears to be up */
if (!intel_dp_get_link_status(intel_dp, link_status)) {
- return;
+ return false;
}
- /* Now read the DPCD to see if it's actually running */
- if (!intel_dp_get_dpcd(intel_dp)) {
- return;
+ /*
+ * Now read the DPCD to see if it's actually running
+ * Don't return immediately if dpcd read failed,
+ * if sink count was 1 and dpcd read failed we need
+ * to do full detection
+ */
+ ret = intel_dp_get_dpcd(intel_dp);
+
+ if ((old_sink_count != intel_dp->sink_count) || !ret) {
I don't see the connection of the comment above with this. If the dpcd read
fails, the 'return false' will be reached regardless of the previous value of
intel_dp->sink_count. Did you intend to do something different or did I miss
something?
The code was changed but comment was not updated.. Will change the
comment to explain correctly.
+ /* No need to proceed if we are going to do full detect */
+ return false;
}
/* Try to read the source of the interrupt */
@@ -4373,6 +4383,8 @@ intel_dp_short_pulse(struct intel_dp *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);
+
+ return true;
}
/* XXX this is probably wrong for multiple downstream ports */
@@ -5095,8 +5107,12 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
}
}
- if (!intel_dp->is_mst)
- intel_dp_short_pulse(intel_dp);
+ if (!intel_dp->is_mst) {
+ if (!intel_dp_short_pulse(intel_dp)) {
+ intel_dp_long_pulse(intel_dp
->attached_connector);
+ goto put_power;
It could be in a follow up patch, but I think its a good moment to get rid of
the goto put_power. The only thing they do is skip the 'ret = IRQ_HANDLED'
assignment now.
Ander
Sure.. Will remove the goto put_power in follow up patch.
+ }
+ }
}
ret = IRQ_HANDLED;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
