Skip to content

Commit 6c5ed5a

Browse files
mlankhorstmlankhorst
committed
drm/atomic: Acquire connection_mutex lock in drm_helper_probe_single_connector_modes, v4.
mode_valid() 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. For the callbacks, it will always be non-zero. To allow callers not to worry about this, drm_helper_probe_detect_ctx is added which might handle -EDEADLK for you. Changes since v1: - Always set ctx parameter. Changes since v2: - Always take connection_mutex when probing. Changes since v3: - Remove the ctx from intel_dp_long_pulse, and add WARN_ON(!connection_mutex) (danvet) - Update docs to clarify the locking situation. (danvet) Signed-off-by: Maarten Lankhorst <[email protected]> Cc: Boris Brezillon <[email protected]> Reviewed-by: Daniel Vetter <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/1491504920-4017-1-git-send-email-maarten.lankhorst@linux.intel.com
1 parent 99748ab commit 6c5ed5a

File tree

10 files changed

+187
-61
lines changed

10 files changed

+187
-61
lines changed

‎drivers/gpu/drm/drm_probe_helper.c‎

Lines changed: 92 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,73 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
169169
EXPORT_SYMBOL(drm_kms_helper_poll_enable);
170170

171171
static enum drm_connector_status
172-
drm_connector_detect(struct drm_connector *connector, bool force)
172+
drm_helper_probe_detect_ctx(struct drm_connector *connector, bool force)
173173
{
174-
return connector->funcs->detect ?
175-
connector->funcs->detect(connector, force) :
176-
connector_status_connected;
174+
const struct drm_connector_helper_funcs *funcs = connector->helper_private;
175+
struct drm_modeset_acquire_ctx ctx;
176+
int ret;
177+
178+
drm_modeset_acquire_init(&ctx, 0);
179+
180+
retry:
181+
ret = drm_modeset_lock(&connector->dev->mode_config.connection_mutex, &ctx);
182+
if (!ret) {
183+
if (funcs->detect_ctx)
184+
ret = funcs->detect_ctx(connector, &ctx, force);
185+
else if (connector->funcs->detect)
186+
ret = connector->funcs->detect(connector, force);
187+
else
188+
ret = connector_status_connected;
189+
}
190+
191+
if (ret == -EDEADLK) {
192+
drm_modeset_backoff(&ctx);
193+
goto retry;
194+
}
195+
196+
if (WARN_ON(ret < 0))
197+
ret = connector_status_unknown;
198+
199+
drm_modeset_drop_locks(&ctx);
200+
drm_modeset_acquire_fini(&ctx);
201+
202+
return ret;
203+
}
204+
205+
/**
206+
* drm_helper_probe_detect - probe connector status
207+
* @connector: connector to probe
208+
* @ctx: acquire_ctx, or NULL to let this function handle locking.
209+
* @force: Whether destructive probe operations should be performed.
210+
*
211+
* This function calls the detect callbacks of the connector.
212+
* This function returns &drm_connector_status, or
213+
* if @ctx is set, it might also return -EDEADLK.
214+
*/
215+
int
216+
drm_helper_probe_detect(struct drm_connector *connector,
217+
struct drm_modeset_acquire_ctx *ctx,
218+
bool force)
219+
{
220+
const struct drm_connector_helper_funcs *funcs = connector->helper_private;
221+
struct drm_device *dev = connector->dev;
222+
int ret;
223+
224+
if (!ctx)
225+
return drm_helper_probe_detect_ctx(connector, force);
226+
227+
ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
228+
if (ret)
229+
return ret;
230+
231+
if (funcs->detect_ctx)
232+
return funcs->detect_ctx(connector, ctx, force);
233+
else if (connector->funcs->detect)
234+
return connector->funcs->detect(connector, force);
235+
else
236+
return connector_status_connected;
177237
}
238+
EXPORT_SYMBOL(drm_helper_probe_detect);
178239

179240
/**
180241
* drm_helper_probe_single_connector_modes - get complete set of display modes
@@ -239,15 +300,27 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
239300
struct drm_display_mode *mode;
240301
const struct drm_connector_helper_funcs *connector_funcs =
241302
connector->helper_private;
242-
int count = 0;
303+
int count = 0, ret;
243304
int mode_flags = 0;
244305
bool verbose_prune = true;
245306
enum drm_connector_status old_status;
307+
struct drm_modeset_acquire_ctx ctx;
246308

247309
WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
248310

311+
drm_modeset_acquire_init(&ctx, 0);
312+
249313
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
250314
connector->name);
315+
316+
retry:
317+
ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
318+
if (ret == -EDEADLK) {
319+
drm_modeset_backoff(&ctx);
320+
goto retry;
321+
} else
322+
WARN_ON(ret < 0);
323+
251324
/* set all old modes to the stale state */
252325
list_for_each_entry(mode, &connector->modes, head)
253326
mode->status = MODE_STALE;
@@ -263,7 +336,15 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
263336
if (connector->funcs->force)
264337
connector->funcs->force(connector);
265338
} else {
266-
connector->status = drm_connector_detect(connector, true);
339+
ret = drm_helper_probe_detect(connector, &ctx, true);
340+
341+
if (ret == -EDEADLK) {
342+
drm_modeset_backoff(&ctx);
343+
goto retry;
344+
} else if (WARN(ret < 0, "Invalid return value %i for connector detection\n", ret))
345+
ret = connector_status_unknown;
346+
347+
connector->status = ret;
267348
}
268349

269350
/*
@@ -355,6 +436,9 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
355436
prune:
356437
drm_mode_prune_invalid(dev, &connector->modes, verbose_prune);
357438

439+
drm_modeset_drop_locks(&ctx);
440+
drm_modeset_acquire_fini(&ctx);
441+
358442
if (list_empty(&connector->modes))
359443
return 0;
360444

@@ -440,7 +524,7 @@ static void output_poll_execute(struct work_struct *work)
440524

441525
repoll = true;
442526

443-
connector->status = drm_connector_detect(connector, false);
527+
connector->status = drm_helper_probe_detect(connector, NULL, false);
444528
if (old_status != connector->status) {
445529
const char *old, *new;
446530

@@ -588,7 +672,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
588672

589673
old_status = connector->status;
590674

591-
connector->status = drm_connector_detect(connector, false);
675+
connector->status = drm_helper_probe_detect(connector, NULL, false);
592676
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
593677
connector->base.id,
594678
connector->name,

‎drivers/gpu/drm/i915/intel_crt.c‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -669,15 +669,16 @@ static const struct dmi_system_id intel_spurious_crt_detect[] = {
669669
{ }
670670
};
671671

672-
static enum drm_connector_status
673-
intel_crt_detect(struct drm_connector *connector, bool force)
672+
static int
673+
intel_crt_detect(struct drm_connector *connector,
674+
struct drm_modeset_acquire_ctx *ctx,
675+
bool force)
674676
{
675677
struct drm_i915_private *dev_priv = to_i915(connector->dev);
676678
struct intel_crt *crt = intel_attached_crt(connector);
677679
struct intel_encoder *intel_encoder = &crt->base;
678-
enum drm_connector_status status;
680+
int status, ret;
679681
struct intel_load_detect_pipe tmp;
680-
struct drm_modeset_acquire_ctx ctx;
681682

682683
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
683684
connector->base.id, connector->name,
@@ -721,10 +722,9 @@ intel_crt_detect(struct drm_connector *connector, bool force)
721722
goto out;
722723
}
723724

724-
drm_modeset_acquire_init(&ctx, 0);
725-
726725
/* for pre-945g platforms use load detect */
727-
if (intel_get_load_detect_pipe(connector, NULL, &tmp, &ctx)) {
726+
ret = intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
727+
if (ret > 0) {
728728
if (intel_crt_detect_ddc(connector))
729729
status = connector_status_connected;
730730
else if (INTEL_GEN(dev_priv) < 4)
@@ -734,12 +734,11 @@ intel_crt_detect(struct drm_connector *connector, bool force)
734734
status = connector_status_disconnected;
735735
else
736736
status = connector_status_unknown;
737-
intel_release_load_detect_pipe(connector, &tmp, &ctx);
738-
} else
737+
intel_release_load_detect_pipe(connector, &tmp, ctx);
738+
} else if (ret == 0)
739739
status = connector_status_unknown;
740-
741-
drm_modeset_drop_locks(&ctx);
742-
drm_modeset_acquire_fini(&ctx);
740+
else if (ret < 0)
741+
status = ret;
743742

744743
out:
745744
intel_display_power_put(dev_priv, intel_encoder->power_domain);
@@ -811,7 +810,6 @@ void intel_crt_reset(struct drm_encoder *encoder)
811810

812811
static const struct drm_connector_funcs intel_crt_connector_funcs = {
813812
.dpms = drm_atomic_helper_connector_dpms,
814-
.detect = intel_crt_detect,
815813
.fill_modes = drm_helper_probe_single_connector_modes,
816814
.late_register = intel_connector_register,
817815
.early_unregister = intel_connector_unregister,
@@ -823,6 +821,7 @@ static const struct drm_connector_funcs intel_crt_connector_funcs = {
823821
};
824822

825823
static const struct drm_connector_helper_funcs intel_crt_connector_helper_funcs = {
824+
.detect_ctx = intel_crt_detect,
826825
.mode_valid = intel_crt_mode_valid,
827826
.get_modes = intel_crt_get_modes,
828827
};

‎drivers/gpu/drm/i915/intel_display.c‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9480,10 +9480,10 @@ static int intel_modeset_setup_plane_state(struct drm_atomic_state *state,
94809480
return 0;
94819481
}
94829482

9483-
bool intel_get_load_detect_pipe(struct drm_connector *connector,
9484-
struct drm_display_mode *mode,
9485-
struct intel_load_detect_pipe *old,
9486-
struct drm_modeset_acquire_ctx *ctx)
9483+
int intel_get_load_detect_pipe(struct drm_connector *connector,
9484+
struct drm_display_mode *mode,
9485+
struct intel_load_detect_pipe *old,
9486+
struct drm_modeset_acquire_ctx *ctx)
94879487
{
94889488
struct intel_crtc *intel_crtc;
94899489
struct intel_encoder *intel_encoder =
@@ -9506,10 +9506,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
95069506

95079507
old->restore_state = NULL;
95089508

9509-
retry:
9510-
ret = drm_modeset_lock(&config->connection_mutex, ctx);
9511-
if (ret)
9512-
goto fail;
9509+
WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
95139510

95149511
/*
95159512
* Algorithm gets a little messy:
@@ -9659,10 +9656,8 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
96599656
restore_state = NULL;
96609657
}
96619658

9662-
if (ret == -EDEADLK) {
9663-
drm_modeset_backoff(ctx);
9664-
goto retry;
9665-
}
9659+
if (ret == -EDEADLK)
9660+
return ret;
96669661

96679662
return false;
96689663
}
@@ -15030,6 +15025,7 @@ static void intel_enable_pipe_a(struct drm_device *dev)
1503015025
struct drm_connector *crt = NULL;
1503115026
struct intel_load_detect_pipe load_detect_temp;
1503215027
struct drm_modeset_acquire_ctx *ctx = dev->mode_config.acquire_ctx;
15028+
int ret;
1503315029

1503415030
/* We can't just switch on the pipe A, we need to set things up with a
1503515031
* proper mode and output configuration. As a gross hack, enable pipe A
@@ -15046,7 +15042,10 @@ static void intel_enable_pipe_a(struct drm_device *dev)
1504615042
if (!crt)
1504715043
return;
1504815044

15049-
if (intel_get_load_detect_pipe(crt, NULL, &load_detect_temp, ctx))
15045+
ret = intel_get_load_detect_pipe(crt, NULL, &load_detect_temp, ctx);
15046+
WARN(ret < 0, "All modeset mutexes are locked, but intel_get_load_detect_pipe failed\n");
15047+
15048+
if (ret > 0)
1505015049
intel_release_load_detect_pipe(crt, &load_detect_temp, ctx);
1505115050
}
1505215051

‎drivers/gpu/drm/i915/intel_dp.c‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4566,7 +4566,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
45664566
intel_dp->has_audio = false;
45674567
}
45684568

4569-
static enum drm_connector_status
4569+
static int
45704570
intel_dp_long_pulse(struct intel_connector *intel_connector)
45714571
{
45724572
struct drm_connector *connector = &intel_connector->base;
@@ -4577,6 +4577,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
45774577
enum drm_connector_status status;
45784578
u8 sink_irq_vector = 0;
45794579

4580+
WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
4581+
45804582
intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
45814583

45824584
/* Can't disconnect eDP, but you can close the lid... */
@@ -4635,14 +4637,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
46354637
status = connector_status_disconnected;
46364638
goto out;
46374639
} else if (connector->status == connector_status_connected) {
4638-
/*
4639-
* If display was connected already and is still connected
4640-
* check links status, there has been known issues of
4641-
* link loss triggerring long pulse!!!!
4642-
*/
4643-
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
46444640
intel_dp_check_link_status(intel_dp);
4645-
drm_modeset_unlock(&dev->mode_config.connection_mutex);
46464641
goto out;
46474642
}
46484643

@@ -4682,11 +4677,13 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
46824677
return status;
46834678
}
46844679

4685-
static enum drm_connector_status
4686-
intel_dp_detect(struct drm_connector *connector, bool force)
4680+
static int
4681+
intel_dp_detect(struct drm_connector *connector,
4682+
struct drm_modeset_acquire_ctx *ctx,
4683+
bool force)
46874684
{
46884685
struct intel_dp *intel_dp = intel_attached_dp(connector);
4689-
enum drm_connector_status status = connector->status;
4686+
int status = connector->status;
46904687

46914688
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
46924689
connector->base.id, connector->name);
@@ -5014,7 +5011,6 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder)
50145011

50155012
static const struct drm_connector_funcs intel_dp_connector_funcs = {
50165013
.dpms = drm_atomic_helper_connector_dpms,
5017-
.detect = intel_dp_detect,
50185014
.force = intel_dp_force,
50195015
.fill_modes = drm_helper_probe_single_connector_modes,
50205016
.set_property = intel_dp_set_property,
@@ -5027,6 +5023,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
50275023
};
50285024

50295025
static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
5026+
.detect_ctx = intel_dp_detect,
50305027
.get_modes = intel_dp_get_modes,
50315028
.mode_valid = intel_dp_mode_valid,
50325029
};

‎drivers/gpu/drm/i915/intel_drv.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,10 +1353,10 @@ int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
13531353
void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
13541354
struct intel_digital_port *dport,
13551355
unsigned int expected_mask);
1356-
bool intel_get_load_detect_pipe(struct drm_connector *connector,
1357-
struct drm_display_mode *mode,
1358-
struct intel_load_detect_pipe *old,
1359-
struct drm_modeset_acquire_ctx *ctx);
1356+
int intel_get_load_detect_pipe(struct drm_connector *connector,
1357+
struct drm_display_mode *mode,
1358+
struct intel_load_detect_pipe *old,
1359+
struct drm_modeset_acquire_ctx *ctx);
13601360
void intel_release_load_detect_pipe(struct drm_connector *connector,
13611361
struct intel_load_detect_pipe *old,
13621362
struct drm_modeset_acquire_ctx *ctx);

‎drivers/gpu/drm/i915/intel_hotplug.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,8 @@ static bool intel_hpd_irq_event(struct drm_device *dev,
243243
WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
244244
old_status = connector->status;
245245

246-
connector->status = connector->funcs->detect(connector, false);
246+
connector->status = drm_helper_probe_detect(connector, NULL, false);
247+
247248
if (old_status == connector->status)
248249
return false;
249250

0 commit comments

Comments
 (0)