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.lankhorst@xxxxxxxxxxxxxxx> Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx> --- 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx