On 8/26/2015 3:32 PM, Jani Nikula wrote:
On Tue, 25 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasim...@intel.com> 
wrote:
From: "Thulasimani,Sivakumar" <sivakumar.thulasim...@intel.com>

This patch checks for changes in sink count between short pulse
hpds and forces full detect when there is a change. This will
result in compliance test case 4.2.2.8 passing since it tests
for this behavior.

This will allow both detection of hotplug and unplug of panels
through dongles that give only short pulse for such events.

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasim...@intel.com>
---
  drivers/gpu/drm/i915/intel_dp.c |   20 ++++++++++++++++----
  1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 834f513..eb3ab41 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4370,26 +4370,34 @@ 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_check_link_status(struct intel_dp *intel_dp, bool 
*perform_full_detect)
You could turn perform_full_detect into a boolean return value which
this function doesn't have.
ok thinking a few mins brings back the reason for this change :).

we return from this function for the following reasons,
> dpcd read failed
> sink count change
> crtc is not active
> ATR was requested and it was handled/ignored
> link status was good or if bad we retrained it.
    and it might have passed or failed.

lets say i return true or false. what does that mean ?

i'll need full detection only if display was already present and is not anymore
which can be due to
dpcd read failure while the display was enabled ? (basically sink count change from 1 to 0)
or sink count changed from 0 to 1.

so it is better to have an explicit parameter rather than


  {
        struct drm_device *dev = intel_dp_to_dev(intel_dp);
        struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
        struct intel_crtc *crtc =
                to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
        u8 sink_irq_vector;
+       u8 old_sink_count = intel_dp->sink_count;
+       u8 ret;
Isn't this a bool, really?

        u8 link_status[DP_LINK_STATUS_SIZE];
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); + *perform_full_detect = false;
+
        /* Try to read receiver status if the link appears to be up */
        if (!intel_dp_get_link_status(intel_dp, link_status)) {
                return;
        }
/* Now read the DPCD to see if it's actually running */
-       if (!intel_dp_get_dpcd(intel_dp)) {
+       ret = intel_dp_get_dpcd(intel_dp);
+
+       if (old_sink_count != intel_dp->sink_count)
+               *perform_full_detect = true;
Can't trust intel_dp->sink_count without checking the return value. So I
guess you can call intel_dp_get_dpcd the way it's done now, and move the
sink count check below it.

BR,
Jani.

+
+       if (!ret)
                return;
-       }
if (!intel_encoder->base.crtc)
                return;
@@ -5031,13 +5039,17 @@ intel_dp_hpd_pulse(struct intel_digital_port 
*intel_dig_port, bool long_hpd)
                }
if (!intel_dp->is_mst) {
+                       bool full_detect;
                        /*
                         * 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);
+                       intel_dp_check_link_status(intel_dp, &full_detect);
                        drm_modeset_unlock(&dev->mode_config.connection_mutex);
+
+                       if (full_detect)
+                               goto put_power;
                }
        }
--
1.7.9.5


--
regards,
Sivakumar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to