Commit 162b6a57 authored by Daniel Vetter's avatar Daniel Vetter

drm/probe-helper: don't lose hotplug event

There's a race window (small for hpd, 10s large for polled outputs)
where userspace could sneak in with an unrelated connnector probe
ioctl call and eat the hotplug event (since neither the hpd nor the
poll code see a state change).

To avoid this, check whether the connector state changes in all other
->detect calls (in the current helper code that's only probe_single)
and if that's the case, fire off a hotplug event. Note that we can't
directly call the hotplug event handler, since that expects that no
locks are held (due to reentrancy with the fb code to update the kms
console).

Also, this requires that drivers using the probe_single helper
function set up the poll work. All current drivers do that already,
and with the reworked hpd handling there'll be no downside to
unconditionally setting up the poll work any more.

v2: Review from Rob Clark
- Don't bail out of the output poll work immediately if it's disabled
  to make sure we deliver the delayed hoptplug events. Instead just
  jump to the tail.
- Don't scheduel the work when it's not set up. Would be a driver bug
  since using probe helpers for anything dynamic without them
  initialized makes them all noops.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Cc: Rob Clark <robdclark@gmail.com>
Reviewed-by: default avatarRob Clark <robdclark@gmail.com>
Tested-by: default avatarRob Clark <robdclark@gmail.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 42c5814c
...@@ -103,6 +103,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect ...@@ -103,6 +103,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
int count = 0; int count = 0;
int mode_flags = 0; int mode_flags = 0;
bool verbose_prune = true; bool verbose_prune = true;
enum drm_connector_status old_status;
WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
...@@ -121,7 +122,33 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect ...@@ -121,7 +122,33 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
if (connector->funcs->force) if (connector->funcs->force)
connector->funcs->force(connector); connector->funcs->force(connector);
} else { } else {
old_status = connector->status;
connector->status = connector->funcs->detect(connector, true); connector->status = connector->funcs->detect(connector, true);
/*
* Normally either the driver's hpd code or the poll loop should
* pick up any changes and fire the hotplug event. But if
* userspace sneaks in a probe, we might miss a change. Hence
* check here, and if anything changed start the hotplug code.
*/
if (old_status != connector->status) {
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
connector->base.id,
connector->name,
old_status, connector->status);
/*
* The hotplug event code might call into the fb
* helpers, and so expects that we do not hold any
* locks. Fire up the poll struct instead, it will
* disable itself again.
*/
dev->mode_config.delayed_event = true;
if (dev->mode_config.poll_enabled)
schedule_delayed_work(&dev->mode_config.output_poll_work,
0);
}
} }
/* Re-enable polling in case the global poll config changed. */ /* Re-enable polling in case the global poll config changed. */
...@@ -274,10 +301,14 @@ static void output_poll_execute(struct work_struct *work) ...@@ -274,10 +301,14 @@ static void output_poll_execute(struct work_struct *work)
struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.output_poll_work); struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.output_poll_work);
struct drm_connector *connector; struct drm_connector *connector;
enum drm_connector_status old_status; enum drm_connector_status old_status;
bool repoll = false, changed = false; bool repoll = false, changed;
/* Pick up any changes detected by the probe functions. */
changed = dev->mode_config.delayed_event;
dev->mode_config.delayed_event = false;
if (!drm_kms_helper_poll) if (!drm_kms_helper_poll)
return; goto out;
mutex_lock(&dev->mode_config.mutex); mutex_lock(&dev->mode_config.mutex);
list_for_each_entry(connector, &dev->mode_config.connector_list, head) { list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
...@@ -319,6 +350,7 @@ static void output_poll_execute(struct work_struct *work) ...@@ -319,6 +350,7 @@ static void output_poll_execute(struct work_struct *work)
mutex_unlock(&dev->mode_config.mutex); mutex_unlock(&dev->mode_config.mutex);
out:
if (changed) if (changed)
drm_kms_helper_hotplug_event(dev); drm_kms_helper_hotplug_event(dev);
......
...@@ -1089,6 +1089,7 @@ struct drm_mode_config { ...@@ -1089,6 +1089,7 @@ struct drm_mode_config {
/* output poll support */ /* output poll support */
bool poll_enabled; bool poll_enabled;
bool poll_running; bool poll_running;
bool delayed_event;
struct delayed_work output_poll_work; struct delayed_work output_poll_work;
/* pointers to standard properties */ /* pointers to standard properties */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment