mode_valid() and get_modes() called
from drm_helper_probe_single_connector_modes()
may need to look at connector->state because what a valid mode is may
depend on connector properties being set. For example some HDMI modes
might be rejected when a connector property forces the connector
into DVI mode.

Some implementations of detect() already lock all state,
so we have to pass an acquire_ctx to them to prevent a deadlock.

This means changing the function signature of detect() slightly,
and passing the acquire_ctx for locking multiple crtc's.
It might be NULL, in which case expensive operations should be avoided.

intel_dp.c however ignores the force flag, so still lock
connection_mutex there if needed.

Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Cc: Boris Brezillon <boris.brezil...@free-electrons.com>
Cc: Manasi Navare <manasi.d.nav...@intel.com>
---
 drivers/gpu/drm/drm_probe_helper.c   | 41 ++++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_crt.c     | 29 +++++++++++++------------
 drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++-----------
 drivers/gpu/drm/i915/intel_dp.c      | 22 ++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  8 +++----
 drivers/gpu/drm/i915/intel_hotplug.c |  6 +++++-
 drivers/gpu/drm/i915/intel_tv.c      | 24 ++++++++++-----------
 include/drm/drm_connector.h          |  5 +++++
 8 files changed, 101 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 85005d57bde6..a48cff963871 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -169,11 +169,15 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
 EXPORT_SYMBOL(drm_kms_helper_poll_enable);
 
 static enum drm_connector_status
-drm_connector_detect(struct drm_connector *connector, bool force)
+drm_connector_detect(struct drm_connector *connector,
+                    struct drm_modeset_acquire_ctx *ctx)
 {
-       return connector->funcs->detect ?
-               connector->funcs->detect(connector, force) :
-               connector_status_connected;
+       if (connector->funcs->detect_ctx)
+               return connector->funcs->detect_ctx(connector, ctx);
+       else if (connector->funcs->detect)
+               return connector->funcs->detect(connector, !!ctx);
+       else
+               return connector_status_connected;
 }
 
 /**
@@ -239,15 +243,27 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
        struct drm_display_mode *mode;
        const struct drm_connector_helper_funcs *connector_funcs =
                connector->helper_private;
-       int count = 0;
+       int count = 0, ret;
        int mode_flags = 0;
        bool verbose_prune = true;
        enum drm_connector_status old_status;
+       struct drm_modeset_acquire_ctx ctx;
 
        WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
 
+       drm_modeset_acquire_init(&ctx, 0);
+
        DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
                        connector->name);
+
+retry:
+       ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
+       if (ret == -EDEADLK) {
+               drm_modeset_backoff(&ctx);
+               goto retry;
+       } else
+               WARN_ON(ret < 0);
+
        /* set all old modes to the stale state */
        list_for_each_entry(mode, &connector->modes, head)
                mode->status = MODE_STALE;
@@ -263,7 +279,15 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
                if (connector->funcs->force)
                        connector->funcs->force(connector);
        } else {
-               connector->status = drm_connector_detect(connector, true);
+               ret = drm_connector_detect(connector, &ctx);
+
+               if (ret == -EDEADLK) {
+                       drm_modeset_backoff(&ctx);
+                       goto retry;
+               } else if (WARN(ret < 0, "Invalid return value %i for connector 
detection\n", ret))
+                       ret = connector_status_unknown;
+
+               connector->status = ret;
        }
 
        /*
@@ -355,6 +379,9 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
 prune:
        drm_mode_prune_invalid(dev, &connector->modes, verbose_prune);
 
+       drm_modeset_drop_locks(&ctx);
+       drm_modeset_acquire_fini(&ctx);
+
        if (list_empty(&connector->modes))
                return 0;
 
@@ -440,7 +467,7 @@ static void output_poll_execute(struct work_struct *work)
 
                repoll = true;
 
-               connector->status = drm_connector_detect(connector, false);
+               connector->status = drm_connector_detect(connector, NULL);
                if (old_status != connector->status) {
                        const char *old, *new;
 
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 8c82607294c6..1186c3f5fea0 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -669,19 +669,19 @@ static const struct dmi_system_id 
intel_spurious_crt_detect[] = {
        { }
 };
 
-static enum drm_connector_status
-intel_crt_detect(struct drm_connector *connector, bool force)
+static int
+intel_crt_detect(struct drm_connector *connector,
+                struct drm_modeset_acquire_ctx *ctx)
 {
        struct drm_i915_private *dev_priv = to_i915(connector->dev);
        struct intel_crt *crt = intel_attached_crt(connector);
        struct intel_encoder *intel_encoder = &crt->base;
-       enum drm_connector_status status;
+       int status, ret;
        struct intel_load_detect_pipe tmp;
-       struct drm_modeset_acquire_ctx ctx;
 
        DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
                      connector->base.id, connector->name,
-                     force);
+                     !!ctx);
 
        /* Skip machines without VGA that falsely report hotplug events */
        if (dmi_check_system(intel_spurious_crt_detect))
@@ -716,15 +716,19 @@ intel_crt_detect(struct drm_connector *connector, bool 
force)
                goto out;
        }
 
-       if (!force) {
+       if (!ctx) {
                status = connector->status;
                goto out;
        }
 
-       drm_modeset_acquire_init(&ctx, 0);
-
        /* for pre-945g platforms use load detect */
-       if (intel_get_load_detect_pipe(connector, NULL, &tmp, &ctx)) {
+       ret = intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
+       if (ret < 0) {
+               status = ret;
+               goto out;
+       }
+
+       if (ret > 0) {
                if (intel_crt_detect_ddc(connector))
                        status = connector_status_connected;
                else if (INTEL_GEN(dev_priv) < 4)
@@ -734,13 +738,10 @@ intel_crt_detect(struct drm_connector *connector, bool 
force)
                        status = connector_status_disconnected;
                else
                        status = connector_status_unknown;
-               intel_release_load_detect_pipe(connector, &tmp, &ctx);
+               intel_release_load_detect_pipe(connector, &tmp, ctx);
        } else
                status = connector_status_unknown;
 
-       drm_modeset_drop_locks(&ctx);
-       drm_modeset_acquire_fini(&ctx);
-
 out:
        intel_display_power_put(dev_priv, intel_encoder->power_domain);
        return status;
@@ -811,7 +812,7 @@ void intel_crt_reset(struct drm_encoder *encoder)
 
 static const struct drm_connector_funcs intel_crt_connector_funcs = {
        .dpms = drm_atomic_helper_connector_dpms,
-       .detect = intel_crt_detect,
+       .detect_ctx = intel_crt_detect,
        .fill_modes = drm_helper_probe_single_connector_modes,
        .late_register = intel_connector_register,
        .early_unregister = intel_connector_unregister,
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a79063fac951..baa8d836c8e7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9503,10 +9503,10 @@ static int intel_modeset_setup_plane_state(struct 
drm_atomic_state *state,
        return 0;
 }
 
-bool intel_get_load_detect_pipe(struct drm_connector *connector,
-                               struct drm_display_mode *mode,
-                               struct intel_load_detect_pipe *old,
-                               struct drm_modeset_acquire_ctx *ctx)
+int intel_get_load_detect_pipe(struct drm_connector *connector,
+                              struct drm_display_mode *mode,
+                              struct intel_load_detect_pipe *old,
+                              struct drm_modeset_acquire_ctx *ctx)
 {
        struct intel_crtc *intel_crtc;
        struct intel_encoder *intel_encoder =
@@ -9529,10 +9529,7 @@ bool intel_get_load_detect_pipe(struct drm_connector 
*connector,
 
        old->restore_state = NULL;
 
-retry:
-       ret = drm_modeset_lock(&config->connection_mutex, ctx);
-       if (ret)
-               goto fail;
+       WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
 
        /*
         * Algorithm gets a little messy:
@@ -9682,10 +9679,8 @@ bool intel_get_load_detect_pipe(struct drm_connector 
*connector,
                restore_state = NULL;
        }
 
-       if (ret == -EDEADLK) {
-               drm_modeset_backoff(ctx);
-               goto retry;
-       }
+       if (ret == -EDEADLK)
+               return ret;
 
        return false;
 }
@@ -15112,6 +15107,7 @@ static void intel_enable_pipe_a(struct drm_device *dev)
        struct drm_connector *crt = NULL;
        struct intel_load_detect_pipe load_detect_temp;
        struct drm_modeset_acquire_ctx *ctx = dev->mode_config.acquire_ctx;
+       int ret;
 
        /* We can't just switch on the pipe A, we need to set things up with a
         * proper mode and output configuration. As a gross hack, enable pipe A
@@ -15128,7 +15124,10 @@ static void intel_enable_pipe_a(struct drm_device *dev)
        if (!crt)
                return;
 
-       if (intel_get_load_detect_pipe(crt, NULL, &load_detect_temp, ctx))
+       ret = intel_get_load_detect_pipe(crt, NULL, &load_detect_temp, ctx);
+       WARN(ret < 0, "All modeset mutexes are locked, but 
intel_get_load_detect_pipe failed\n");
+
+       if (ret > 0)
                intel_release_load_detect_pipe(crt, &load_detect_temp, ctx);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fd96a6cf7326..10b3727b7381 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4567,7 +4567,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 }
 
 static enum drm_connector_status
-intel_dp_long_pulse(struct intel_connector *intel_connector)
+intel_dp_long_pulse(struct intel_connector *intel_connector,
+                   struct drm_modeset_acquire_ctx *ctx)
 {
        struct drm_connector *connector = &intel_connector->base;
        struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -4640,9 +4641,13 @@ intel_dp_long_pulse(struct intel_connector 
*intel_connector)
                 * check links status, there has been known issues of
                 * link loss triggerring long pulse!!!!
                 */
-               drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+               if (!ctx)
+                       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 (!ctx)
+                       drm_modeset_unlock(&dev->mode_config.connection_mutex);
                goto out;
        }
 
@@ -4682,18 +4687,19 @@ intel_dp_long_pulse(struct intel_connector 
*intel_connector)
        return status;
 }
 
-static enum drm_connector_status
-intel_dp_detect(struct drm_connector *connector, bool force)
+static int
+intel_dp_detect(struct drm_connector *connector,
+               struct drm_modeset_acquire_ctx *ctx)
 {
        struct intel_dp *intel_dp = intel_attached_dp(connector);
-       enum drm_connector_status status = connector->status;
+       int status = connector->status;
 
        DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
                      connector->base.id, connector->name);
 
        /* If full detect is not performed yet, do a full detect */
        if (!intel_dp->detect_done)
-               status = intel_dp_long_pulse(intel_dp->attached_connector);
+               status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
 
        intel_dp->detect_done = false;
 
@@ -5014,7 +5020,7 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder)
 
 static const struct drm_connector_funcs intel_dp_connector_funcs = {
        .dpms = drm_atomic_helper_connector_dpms,
-       .detect = intel_dp_detect,
+       .detect_ctx = intel_dp_detect,
        .force = intel_dp_force,
        .fill_modes = drm_helper_probe_single_connector_modes,
        .set_property = intel_dp_set_property,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e24641b559e2..6ea8b9cd68c5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1362,10 +1362,10 @@ int ironlake_get_lanes_required(int target_clock, int 
link_bw, int bpp);
 void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
                         struct intel_digital_port *dport,
                         unsigned int expected_mask);
-bool intel_get_load_detect_pipe(struct drm_connector *connector,
-                               struct drm_display_mode *mode,
-                               struct intel_load_detect_pipe *old,
-                               struct drm_modeset_acquire_ctx *ctx);
+int intel_get_load_detect_pipe(struct drm_connector *connector,
+                              struct drm_display_mode *mode,
+                              struct intel_load_detect_pipe *old,
+                              struct drm_modeset_acquire_ctx *ctx);
 void intel_release_load_detect_pipe(struct drm_connector *connector,
                                    struct intel_load_detect_pipe *old,
                                    struct drm_modeset_acquire_ctx *ctx);
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c 
b/drivers/gpu/drm/i915/intel_hotplug.c
index 7d210097eefa..d2d2af7ef305 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -243,7 +243,11 @@ static bool intel_hpd_irq_event(struct drm_device *dev,
        WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
        old_status = connector->status;
 
-       connector->status = connector->funcs->detect(connector, false);
+       if (connector->funcs->detect_ctx)
+               connector->status = connector->funcs->detect_ctx(connector, 
NULL);
+       else
+               connector->status = connector->funcs->detect(connector, false);
+
        if (old_status == connector->status)
                return false;
 
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 6ed1a3ce47b7..d2c0120048b2 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1315,8 +1315,9 @@ static void intel_tv_find_better_format(struct 
drm_connector *connector)
  * Currently this always returns CONNECTOR_STATUS_UNKNOWN, as we need to be 
sure
  * we have a pipe programmed in order to probe the TV.
  */
-static enum drm_connector_status
-intel_tv_detect(struct drm_connector *connector, bool force)
+static int
+intel_tv_detect(struct drm_connector *connector,
+               struct drm_modeset_acquire_ctx *ctx)
 {
        struct drm_display_mode mode;
        struct intel_tv *intel_tv = intel_attached_tv(connector);
@@ -1325,27 +1326,26 @@ intel_tv_detect(struct drm_connector *connector, bool 
force)
 
        DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
                      connector->base.id, connector->name,
-                     force);
+                     !!ctx);
 
        mode = reported_modes[0];
 
-       if (force) {
+       if (ctx) {
                struct intel_load_detect_pipe tmp;
-               struct drm_modeset_acquire_ctx ctx;
+               int ret;
 
-               drm_modeset_acquire_init(&ctx, 0);
+               ret = intel_get_load_detect_pipe(connector, &mode, &tmp, ctx);
+               if (ret < 0)
+                       return ret;
 
-               if (intel_get_load_detect_pipe(connector, &mode, &tmp, &ctx)) {
+               if (ret > 0) {
                        type = intel_tv_detect_type(intel_tv, connector);
-                       intel_release_load_detect_pipe(connector, &tmp, &ctx);
+                       intel_release_load_detect_pipe(connector, &tmp, ctx);
                        status = type < 0 ?
                                connector_status_disconnected :
                                connector_status_connected;
                } else
                        status = connector_status_unknown;
-
-               drm_modeset_drop_locks(&ctx);
-               drm_modeset_acquire_fini(&ctx);
        } else
                return connector->status;
 
@@ -1516,7 +1516,7 @@ intel_tv_set_property(struct drm_connector *connector, 
struct drm_property *prop
 
 static const struct drm_connector_funcs intel_tv_connector_funcs = {
        .dpms = drm_atomic_helper_connector_dpms,
-       .detect = intel_tv_detect,
+       .detect_ctx = intel_tv_detect,
        .late_register = intel_connector_register,
        .early_unregister = intel_connector_unregister,
        .destroy = intel_tv_destroy,
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 941f57f311aa..834d1ba709ea 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -32,6 +32,7 @@
 struct drm_device;
 
 struct drm_connector_helper_funcs;
+struct drm_modeset_acquire_ctx;
 struct drm_device;
 struct drm_crtc;
 struct drm_encoder;
@@ -385,6 +386,10 @@ struct drm_connector_funcs {
        enum drm_connector_status (*detect)(struct drm_connector *connector,
                                            bool force);
 
+       /* ctx = NULL <> detect(force = false), but may also return -EDEADLK. */
+       int (*detect_ctx)(struct drm_connector *connector,
+                         struct drm_modeset_acquire_ctx *ctx);
+
        /**
         * @force:
         *
-- 
2.7.4

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

Reply via email to