drm/i915: Avoid endless HPD poll detect loop via runtime suspend/resume

The issue fixed in

commit a8ddac7c9f06 ("drm/i915: Avoid HPD poll detect triggering a new detect cycle")

on VLV, CHV is still present on platforms where the display hotplug
detection functionality is available whenever the device is in D0 state
(hence these platforms switch to HPD polling only when the device is
runtime suspended).

The above commit avoids an endless i915_hpd_poll_init_work() ->
connector detect loop by making sure that by the end of
i915_hpd_poll_init_work() all display power references acquired by the
connector detect functions which can trigger a new cycle (display core
power domain) are dropped. However on platforms where HPD polling is
enabled/disabled only from the runtime suspend/resume handlers, this is
not ensured: for instance eDP VDD, TypeC port PHYs and the runtime
autosuspend delay may still keep the device runtime resumed (via a power
reference acquired during connector detection and hence result in an
endless loop like the above).

Solve the problem described in the above commit on all platforms, by
making sure that a i915_hpd_poll_init_work() -> connector detect
sequence can't take any power reference in the first place which would
trigger a new cycle, instead of relying on these power references to be
dropped by the end of the sequence.

With the default runtime autosuspend delay (10 sec) this issue didn't
happen in practice, since the device remained runtime resumed for the
whole duration of the above sequence. CI/IGT tests however set the
autosuspend delay to 0, which makes the problem visible, see References:
below.

Tested on GLK, CHV.

v2: Don't warn about a requeued work, to account for disabling
    polling directly during driver loading, reset and system resume.

References: https://gitlab.freedesktop.org/drm/intel/-/issues/7940#note_1997403
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230809104307.1218058-1-imre.deak@intel.com

diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
index 8090747586877e0b653302e292bba57c0470e38f..f66340b4caf0f85e5bb3c3e6963b5d8017736ca1 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -907,12 +907,6 @@ load_detect:
 out:
        intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
 
-       /*
-        * Make sure the refs for power wells enabled during detect are
-        * dropped to avoid a new detect cycle triggered by HPD polling.
-        */
-       intel_display_power_flush_work(dev_priv);
-
        return status;
 }
 

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 12bd2f322e6279a7b85f2ef29a8474903f18ec9d..964bf0551bdc9293dfdd7cb348247dd234bd775d 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4957,12 +4957,6 @@ out:
        if (status != connector_status_connected && !intel_dp->is_mst)
                intel_dp_unset_edid(intel_dp);
 
-       /*
-        * Make sure the refs for power wells enabled during detect are
-        * dropped to avoid a new detect cycle triggered by HPD polling.
-        */
-       intel_display_power_flush_work(dev_priv);
-
        if (!intel_dp_is_edp(intel_dp))
                drm_dp_set_subconnector_property(connector,
                                                 status,

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 94a7e1537f42780282960c434903cf24d9dba742..9442bf43550ee52b17405cb6fd9df4baf824c5e1 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2522,12 +2522,6 @@ out:
        if (status != connector_status_connected)
                cec_notifier_phys_addr_invalidate(intel_hdmi->cec_notifier);
 
-       /*
-        * Make sure the refs for power wells enabled during detect are
-        * dropped to avoid a new detect cycle triggered by HPD polling.
-        */
-       intel_display_power_flush_work(dev_priv);
-
        return status;
 }
 

diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 0ff5ed46ae1e748b5a8cfd91688ca9c18a15770d..dd7eb9fc78610d312dd0801e3f0d966941ecd9f9 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -25,6 +25,7 @@
 
 #include "i915_drv.h"
 #include "i915_irq.h"
+#include "intel_display_power.h"
 #include "intel_display_types.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"
@@ -638,11 +639,25 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
                             display.hotplug.poll_init_work);
        struct drm_connector_list_iter conn_iter;
        struct intel_connector *connector;
+       intel_wakeref_t wakeref;
        bool enabled;
 
        mutex_lock(&dev_priv->drm.mode_config.mutex);
 
        enabled = READ_ONCE(dev_priv->display.hotplug.poll_enabled);
+       /*
+        * Prevent taking a power reference from this sequence of
+        * i915_hpd_poll_init_work() -> drm_helper_hpd_irq_event() ->
+        * connector detect which would requeue i915_hpd_poll_init_work()
+        * and so risk an endless loop of this same sequence.
+        */
+       if (!enabled) {
+               wakeref = intel_display_power_get(dev_priv,
+                                                 POWER_DOMAIN_DISPLAY_CORE);
+               drm_WARN_ON(&dev_priv->drm,
+                           READ_ONCE(dev_priv->display.hotplug.poll_enabled));
+               cancel_work(&dev_priv->display.hotplug.poll_init_work);
+       }
 
        drm_connector_list_iter_begin(&dev_priv->drm, &conn_iter);
        for_each_intel_connector_iter(connector, &conn_iter) {
@@ -669,8 +684,13 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
         * We might have missed any hotplugs that happened while we were
         * in the middle of disabling polling
         */
-       if (!enabled)
+       if (!enabled) {
                drm_helper_hpd_irq_event(&dev_priv->drm);
+
+               intel_display_power_put(dev_priv,
+                                       POWER_DOMAIN_DISPLAY_CORE,
+                                       wakeref);
+       }
 }
 
 /**