Commit 6c5ed5ae authored by Maarten Lankhorst's avatar Maarten Lankhorst
Browse files

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: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1491504920-4017-1-git-send-email-maarten.lankhorst@linux.intel.com
parent 99748ab6

drivers/gpu/drm/drm_probe_helper.c

+92 −8
Original line number Diff line number Diff line
@@ -169,13 +169,74 @@ 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_helper_probe_detect_ctx(struct drm_connector *connector, bool force)

{

	return connector->funcs->detect ?

		connector->funcs->detect(connector, force) :

		connector_status_connected;

	const struct drm_connector_helper_funcs *funcs = connector->helper_private;

	struct drm_modeset_acquire_ctx ctx;

	int ret;



	drm_modeset_acquire_init(&ctx, 0);



retry:

	ret = drm_modeset_lock(&connector->dev->mode_config.connection_mutex, &ctx);

	if (!ret) {

		if (funcs->detect_ctx)

			ret = funcs->detect_ctx(connector, &ctx, force);

		else if (connector->funcs->detect)

			ret = connector->funcs->detect(connector, force);

		else

			ret = connector_status_connected;

	}



	if (ret == -EDEADLK) {

		drm_modeset_backoff(&ctx);

		goto retry;

	}



	if (WARN_ON(ret < 0))

		ret = connector_status_unknown;



	drm_modeset_drop_locks(&ctx);

	drm_modeset_acquire_fini(&ctx);



	return ret;

}



/**

 * drm_helper_probe_detect - probe connector status

 * @connector: connector to probe

 * @ctx: acquire_ctx, or NULL to let this function handle locking.

 * @force: Whether destructive probe operations should be performed.

 *

 * This function calls the detect callbacks of the connector.

 * This function returns &drm_connector_status, or

 * if @ctx is set, it might also return -EDEADLK.

 */

int

drm_helper_probe_detect(struct drm_connector *connector,

			struct drm_modeset_acquire_ctx *ctx,

			bool force)

{

	const struct drm_connector_helper_funcs *funcs = connector->helper_private;

	struct drm_device *dev = connector->dev;

	int ret;



	if (!ctx)

		return drm_helper_probe_detect_ctx(connector, force);



	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);

	if (ret)

		return ret;



	if (funcs->detect_ctx)

		return funcs->detect_ctx(connector, ctx, force);

	else if (connector->funcs->detect)

		return connector->funcs->detect(connector, force);

	else

		return connector_status_connected;

}

EXPORT_SYMBOL(drm_helper_probe_detect);



/**

 * drm_helper_probe_single_connector_modes - get complete set of display modes

 * @connector: connector to probe
@@ -239,15 +300,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 +336,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_helper_probe_detect(connector, &ctx, true);



		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 +436,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 +524,7 @@ static void output_poll_execute(struct work_struct *work) 


		repoll = true;



		connector->status = drm_connector_detect(connector, false);

		connector->status = drm_helper_probe_detect(connector, NULL, false);

		if (old_status != connector->status) {

			const char *old, *new;
@@ -588,7 +672,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) 


		old_status = connector->status;



		connector->status = drm_connector_detect(connector, false);

		connector->status = drm_helper_probe_detect(connector, NULL, false);

		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",

			      connector->base.id,

			      connector->name,

drivers/gpu/drm/i915/intel_crt.c

+12 −13
Original line number Diff line number Diff line
@@ -669,15 +669,16 @@ 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,

		 bool force)

{

	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,
@@ -721,10 +722,9 @@ intel_crt_detect(struct drm_connector *connector, bool force) 
		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) {

		if (intel_crt_detect_ddc(connector))

			status = connector_status_connected;

		else if (INTEL_GEN(dev_priv) < 4)
@@ -734,12 +734,11 @@ 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);

	} else

		intel_release_load_detect_pipe(connector, &tmp, ctx);

	} else if (ret == 0)

		status = connector_status_unknown;



	drm_modeset_drop_locks(&ctx);

	drm_modeset_acquire_fini(&ctx);

	else if (ret < 0)

		status = ret;



out:

	intel_display_power_put(dev_priv, intel_encoder->power_domain);
@@ -811,7 +810,6 @@ 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,

	.fill_modes = drm_helper_probe_single_connector_modes,

	.late_register = intel_connector_register,

	.early_unregister = intel_connector_unregister,
@@ -823,6 +821,7 @@ static const struct drm_connector_funcs intel_crt_connector_funcs = { 
};



static const struct drm_connector_helper_funcs intel_crt_connector_helper_funcs = {

	.detect_ctx = intel_crt_detect,

	.mode_valid = intel_crt_mode_valid,

	.get_modes = intel_crt_get_modes,

};

drivers/gpu/drm/i915/intel_display.c

+12 −13
Original line number Diff line number Diff line
@@ -9480,7 +9480,7 @@ static int intel_modeset_setup_plane_state(struct drm_atomic_state *state, 
	return 0;

}



bool intel_get_load_detect_pipe(struct drm_connector *connector,

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)
@@ -9506,10 +9506,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:
@@ -9659,10 +9656,8 @@ fail: 
		restore_state = NULL;

	}



	if (ret == -EDEADLK) {

		drm_modeset_backoff(ctx);

		goto retry;

	}

	if (ret == -EDEADLK)

		return ret;



	return false;

}
@@ -15030,6 +15025,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
@@ -15046,7 +15042,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);

}

drivers/gpu/drm/i915/intel_dp.c

+9 −12
Original line number Diff line number Diff line
@@ -4566,7 +4566,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) 
	intel_dp->has_audio = false;

}



static enum drm_connector_status

static int

intel_dp_long_pulse(struct intel_connector *intel_connector)

{

	struct drm_connector *connector = &intel_connector->base;
@@ -4577,6 +4577,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) 
	enum drm_connector_status status;

	u8 sink_irq_vector = 0;



	WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));



	intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);



	/* Can't disconnect eDP, but you can close the lid... */
@@ -4635,14 +4637,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) 
		status = connector_status_disconnected;

		goto out;

	} else if (connector->status == connector_status_connected) {

		/*

		 * If display was connected already and is still connected

		 * check links status, there has been known issues of

		 * link loss triggerring long pulse!!!!

		 */

		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);

		intel_dp_check_link_status(intel_dp);

		drm_modeset_unlock(&dev->mode_config.connection_mutex);

		goto out;

	}
@@ -4682,11 +4677,13 @@ out: 
	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,

		bool force)

{

	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);
@@ -5014,7 +5011,6 @@ 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,

	.force = intel_dp_force,

	.fill_modes = drm_helper_probe_single_connector_modes,

	.set_property = intel_dp_set_property,
@@ -5027,6 +5023,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = { 
};



static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {

	.detect_ctx = intel_dp_detect,

	.get_modes = intel_dp_get_modes,

	.mode_valid = intel_dp_mode_valid,

};

drivers/gpu/drm/i915/intel_drv.h

+4 −4
Original line number Diff line number Diff line
@@ -1353,7 +1353,7 @@ 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,

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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE