| Message ID | 1421851260-23297-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 21, 2015 at 9:41 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > 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> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Rob Clark <robdclark@gmail.com> and with the monitor that started all this, plus the addition of: http://lists.freedesktop.org/archives/dri-devel/2015-January/075901.html Tested-by: Rob Clark <robdclark@gmail.com> > --- > drivers/gpu/drm/drm_probe_helper.c | 36 ++++++++++++++++++++++++++++++++++-- > include/drm/drm_crtc.h | 1 + > 2 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 2fbdcca7ca9a..33bf550a1d3f 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -103,6 +103,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect > int count = 0; > int mode_flags = 0; > bool verbose_prune = true; > + enum drm_connector_status old_status; > > 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 > if (connector->funcs->force) > connector->funcs->force(connector); > } else { > + old_status = connector->status; > + > 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. */ > @@ -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_connector *connector; > 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) > - return; > + goto out; > > mutex_lock(&dev->mode_config.mutex); > list_for_each_entry(connector, &dev->mode_config.connector_list, head) { > @@ -319,6 +350,7 @@ static void output_poll_execute(struct work_struct *work) > > mutex_unlock(&dev->mode_config.mutex); > > +out: > if (changed) > drm_kms_helper_hotplug_event(dev); > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index f444263055c5..65da9fb939a7 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1089,6 +1089,7 @@ struct drm_mode_config { > /* output poll support */ > bool poll_enabled; > bool poll_running; > + bool delayed_event; > struct delayed_work output_poll_work; > > /* pointers to standard properties */ > -- > 2.1.4 >
On Wed, Jan 21, 2015 at 4:40 PM, Rob Clark <robdclark@gmail.com> wrote: > On Wed, Jan 21, 2015 at 9:41 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> 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> >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Reviewed-by: Rob Clark <robdclark@gmail.com> > > and with the monitor that started all this, plus the addition of: > http://lists.freedesktop.org/archives/dri-devel/2015-January/075901.html > > Tested-by: Rob Clark <robdclark@gmail.com> bleh, but on latest upstream (plus drm-next, etc).. possibly just not caught earlier due to insufficient debug stuff enabled.. I could probably solve this by moving drm_kms_helper_poll_init() earlier.. otoh that might cause other surprises and other drivers also seem to do this late in their load.. [ 4.053197] WARNING: CPU: 3 PID: 52 at ../kernel/workqueue.c:1427 __queue_delayed_work+0x14c/0x15c() [ 4.058040] Modules linked in: [ 4.070017] CPU: 3 PID: 52 Comm: kworker/u8:1 Tainted: G U 3.19.0-rc5-00821-g415f9e0-dirty #1113 [ 4.070107] Hardware name: Qualcomm (Flattened Device Tree) [ 4.080016] Workqueue: deferwq deferred_probe_work_func [ 4.090549] [<c021654c>] (unwind_backtrace) from [<c0211f68>] (show_stack+0x10/0x14) [ 4.090714] [<c0211f68>] (show_stack) from [<c091d3bc>] (dump_stack+0x8c/0x9c) [ 4.098519] [<c091d3bc>] (dump_stack) from [<c0246a2c>] (warn_slowpath_common+0x84/0xb4) [ 4.105547] [<c0246a2c>] (warn_slowpath_common) from [<c0246af8>] (warn_slowpath_null+0x1c/0x24) [ 4.113790] [<c0246af8>] (warn_slowpath_null) from [<c0259b5c>] (__queue_delayed_work+0x14c/0x15c) [ 4.122556] [<c0259b5c>] (__queue_delayed_work) from [<c0259bbc>] (queue_delayed_work_on+0x50/0x5c) [ 4.131337] [<c0259bbc>] (queue_delayed_work_on) from [<c0561114>] (drm_helper_probe_single_connector_modes_merge_bits+0x2fc/0x49c) [ 4.140290] [<c0561114>] (drm_helper_probe_single_connector_modes_merge_bits) from [<c0569824>] (drm_fb_helper_probe_connector_modes.isra.5+0x4c/0x6c) [ 4.152089] [<c0569824>] (drm_fb_helper_probe_connector_modes.isra.5) from [<c056a8a8>] (drm_fb_helper_initial_config+0x34/0x3d8) [ 4.165624] [<c056a8a8>] (drm_fb_helper_initial_config) from [<c05a62dc>] (msm_fbdev_init+0x70/0xa0) [ 4.177342] [<c05a62dc>] (msm_fbdev_init) from [<c05a1614>] (msm_load+0x268/0x36c) [ 4.186537] [<c05a1614>] (msm_load) from [<c0573af4>] (drm_dev_register+0xa8/0x104) [ 4.193912] [<c0573af4>] (drm_dev_register) from [<c05759dc>] (drm_platform_init+0x44/0xd8) [ 4.201475] [<c05759dc>] (drm_platform_init) from [<c05c1654>] (try_to_bring_up_master.part.3+0xc8/0x108) [ 4.209804] [<c05c1654>] (try_to_bring_up_master.part.3) from [<c05c1740>] (component_master_add_with_match+0xac/0x124) [ 4.219525] [<c05c1740>] (component_master_add_with_match) from [<c05a116c>] (msm_pdev_probe+0x64/0x70) [ 4.230116] [<c05a116c>] (msm_pdev_probe) from [<c05c73c8>] (platform_drv_probe+0x44/0xa4) [ 4.239485] [<c05c73c8>] (platform_drv_probe) from [<c05c5c48>] (driver_probe_device+0x108/0x23c) [ 4.247813] [<c05c5c48>] (driver_probe_device) from [<c05c42ac>] (bus_for_each_drv+0x64/0x98) [ 4.256752] [<c05c42ac>] (bus_for_each_drv) from [<c05c5b10>] (device_attach+0x74/0x88) [ 4.265259] [<c05c5b10>] (device_attach) from [<c05c51fc>] (bus_probe_device+0x84/0xa8) [ 4.273071] [<c05c51fc>] (bus_probe_device) from [<c05c561c>] (deferred_probe_work_func+0x64/0x94) [ 4.281064] [<c05c561c>] (deferred_probe_work_func) from [<c025a354>] (process_one_work+0x134/0x334) [ 4.290091] [<c025a354>] (process_one_work) from [<c025ac30>] (worker_thread+0x4c/0x4b0) [ 4.299383] [<c025ac30>] (worker_thread) from [<c025ecfc>] (kthread+0xdc/0xf4) [ 4.307459] [<c025ecfc>] (kthread) from [<c020e978>] (ret_from_fork+0x14/0x3c) BR, -R > >> --- >> drivers/gpu/drm/drm_probe_helper.c | 36 ++++++++++++++++++++++++++++++++++-- >> include/drm/drm_crtc.h | 1 + >> 2 files changed, 35 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c >> index 2fbdcca7ca9a..33bf550a1d3f 100644 >> --- a/drivers/gpu/drm/drm_probe_helper.c >> +++ b/drivers/gpu/drm/drm_probe_helper.c >> @@ -103,6 +103,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect >> int count = 0; >> int mode_flags = 0; >> bool verbose_prune = true; >> + enum drm_connector_status old_status; >> >> 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 >> if (connector->funcs->force) >> connector->funcs->force(connector); >> } else { >> + old_status = connector->status; >> + >> 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. */ >> @@ -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_connector *connector; >> 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) >> - return; >> + goto out; >> >> mutex_lock(&dev->mode_config.mutex); >> list_for_each_entry(connector, &dev->mode_config.connector_list, head) { >> @@ -319,6 +350,7 @@ static void output_poll_execute(struct work_struct *work) >> >> mutex_unlock(&dev->mode_config.mutex); >> >> +out: >> if (changed) >> drm_kms_helper_hotplug_event(dev); >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >> index f444263055c5..65da9fb939a7 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -1089,6 +1089,7 @@ struct drm_mode_config { >> /* output poll support */ >> bool poll_enabled; >> bool poll_running; >> + bool delayed_event; >> struct delayed_work output_poll_work; >> >> /* pointers to standard properties */ >> -- >> 2.1.4 >>
On Sun, Jan 25, 2015 at 12:37:31PM -0500, Rob Clark wrote: > On Wed, Jan 21, 2015 at 4:40 PM, Rob Clark <robdclark@gmail.com> wrote: > > On Wed, Jan 21, 2015 at 9:41 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > >> 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> > >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Reviewed-by: Rob Clark <robdclark@gmail.com> > > > > and with the monitor that started all this, plus the addition of: > > http://lists.freedesktop.org/archives/dri-devel/2015-January/075901.html > > > > Tested-by: Rob Clark <robdclark@gmail.com> > > bleh, but on latest upstream (plus drm-next, etc).. possibly just not > caught earlier due to insufficient debug stuff enabled.. > > I could probably solve this by moving drm_kms_helper_poll_init() > earlier.. otoh that might cause other surprises and other drivers also > seem to do this late in their load.. Well I'm confused - I check for poll_enabled and that is only set _after_ INIT_DELAYED_WORK. Which should prevent schedule_delayed_work on an uninitialized work and was the entire point of v2. Nothing else writes to poll_enabled. What do I miss? Totally confused ... -Daniel > > [ 4.053197] WARNING: CPU: 3 PID: 52 at ../kernel/workqueue.c:1427 > __queue_delayed_work+0x14c/0x15c() > [ 4.058040] Modules linked in: > [ 4.070017] CPU: 3 PID: 52 Comm: kworker/u8:1 Tainted: G U > 3.19.0-rc5-00821-g415f9e0-dirty #1113 > [ 4.070107] Hardware name: Qualcomm (Flattened Device Tree) > [ 4.080016] Workqueue: deferwq deferred_probe_work_func > [ 4.090549] [<c021654c>] (unwind_backtrace) from [<c0211f68>] > (show_stack+0x10/0x14) > [ 4.090714] [<c0211f68>] (show_stack) from [<c091d3bc>] > (dump_stack+0x8c/0x9c) > [ 4.098519] [<c091d3bc>] (dump_stack) from [<c0246a2c>] > (warn_slowpath_common+0x84/0xb4) > [ 4.105547] [<c0246a2c>] (warn_slowpath_common) from [<c0246af8>] > (warn_slowpath_null+0x1c/0x24) > [ 4.113790] [<c0246af8>] (warn_slowpath_null) from [<c0259b5c>] > (__queue_delayed_work+0x14c/0x15c) > [ 4.122556] [<c0259b5c>] (__queue_delayed_work) from [<c0259bbc>] > (queue_delayed_work_on+0x50/0x5c) > [ 4.131337] [<c0259bbc>] (queue_delayed_work_on) from [<c0561114>] > (drm_helper_probe_single_connector_modes_merge_bits+0x2fc/0x49c) > [ 4.140290] [<c0561114>] > (drm_helper_probe_single_connector_modes_merge_bits) from [<c0569824>] > (drm_fb_helper_probe_connector_modes.isra.5+0x4c/0x6c) > [ 4.152089] [<c0569824>] > (drm_fb_helper_probe_connector_modes.isra.5) from [<c056a8a8>] > (drm_fb_helper_initial_config+0x34/0x3d8) > [ 4.165624] [<c056a8a8>] (drm_fb_helper_initial_config) from > [<c05a62dc>] (msm_fbdev_init+0x70/0xa0) > [ 4.177342] [<c05a62dc>] (msm_fbdev_init) from [<c05a1614>] > (msm_load+0x268/0x36c) > [ 4.186537] [<c05a1614>] (msm_load) from [<c0573af4>] > (drm_dev_register+0xa8/0x104) > [ 4.193912] [<c0573af4>] (drm_dev_register) from [<c05759dc>] > (drm_platform_init+0x44/0xd8) > [ 4.201475] [<c05759dc>] (drm_platform_init) from [<c05c1654>] > (try_to_bring_up_master.part.3+0xc8/0x108) > [ 4.209804] [<c05c1654>] (try_to_bring_up_master.part.3) from > [<c05c1740>] (component_master_add_with_match+0xac/0x124) > [ 4.219525] [<c05c1740>] (component_master_add_with_match) from > [<c05a116c>] (msm_pdev_probe+0x64/0x70) > [ 4.230116] [<c05a116c>] (msm_pdev_probe) from [<c05c73c8>] > (platform_drv_probe+0x44/0xa4) > [ 4.239485] [<c05c73c8>] (platform_drv_probe) from [<c05c5c48>] > (driver_probe_device+0x108/0x23c) > [ 4.247813] [<c05c5c48>] (driver_probe_device) from [<c05c42ac>] > (bus_for_each_drv+0x64/0x98) > [ 4.256752] [<c05c42ac>] (bus_for_each_drv) from [<c05c5b10>] > (device_attach+0x74/0x88) > [ 4.265259] [<c05c5b10>] (device_attach) from [<c05c51fc>] > (bus_probe_device+0x84/0xa8) > [ 4.273071] [<c05c51fc>] (bus_probe_device) from [<c05c561c>] > (deferred_probe_work_func+0x64/0x94) > [ 4.281064] [<c05c561c>] (deferred_probe_work_func) from > [<c025a354>] (process_one_work+0x134/0x334) > [ 4.290091] [<c025a354>] (process_one_work) from [<c025ac30>] > (worker_thread+0x4c/0x4b0) > [ 4.299383] [<c025ac30>] (worker_thread) from [<c025ecfc>] > (kthread+0xdc/0xf4) > [ 4.307459] [<c025ecfc>] (kthread) from [<c020e978>] > (ret_from_fork+0x14/0x3c) > > BR, > -R > > > > >> --- > >> drivers/gpu/drm/drm_probe_helper.c | 36 ++++++++++++++++++++++++++++++++++-- > >> include/drm/drm_crtc.h | 1 + > >> 2 files changed, 35 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > >> index 2fbdcca7ca9a..33bf550a1d3f 100644 > >> --- a/drivers/gpu/drm/drm_probe_helper.c > >> +++ b/drivers/gpu/drm/drm_probe_helper.c > >> @@ -103,6 +103,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect > >> int count = 0; > >> int mode_flags = 0; > >> bool verbose_prune = true; > >> + enum drm_connector_status old_status; > >> > >> 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 > >> if (connector->funcs->force) > >> connector->funcs->force(connector); > >> } else { > >> + old_status = connector->status; > >> + > >> 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. */ > >> @@ -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_connector *connector; > >> 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) > >> - return; > >> + goto out; > >> > >> mutex_lock(&dev->mode_config.mutex); > >> list_for_each_entry(connector, &dev->mode_config.connector_list, head) { > >> @@ -319,6 +350,7 @@ static void output_poll_execute(struct work_struct *work) > >> > >> mutex_unlock(&dev->mode_config.mutex); > >> > >> +out: > >> if (changed) > >> drm_kms_helper_hotplug_event(dev); > >> > >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > >> index f444263055c5..65da9fb939a7 100644 > >> --- a/include/drm/drm_crtc.h > >> +++ b/include/drm/drm_crtc.h > >> @@ -1089,6 +1089,7 @@ struct drm_mode_config { > >> /* output poll support */ > >> bool poll_enabled; > >> bool poll_running; > >> + bool delayed_event; > >> struct delayed_work output_poll_work; > >> > >> /* pointers to standard properties */ > >> -- > >> 2.1.4 > >>
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 2fbdcca7ca9a..33bf550a1d3f 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -103,6 +103,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect int count = 0; int mode_flags = 0; bool verbose_prune = true; + enum drm_connector_status old_status; 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 if (connector->funcs->force) connector->funcs->force(connector); } else { + old_status = connector->status; + 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. */ @@ -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_connector *connector; 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) - return; + goto out; mutex_lock(&dev->mode_config.mutex); list_for_each_entry(connector, &dev->mode_config.connector_list, head) { @@ -319,6 +350,7 @@ static void output_poll_execute(struct work_struct *work) mutex_unlock(&dev->mode_config.mutex); +out: if (changed) drm_kms_helper_hotplug_event(dev); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index f444263055c5..65da9fb939a7 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1089,6 +1089,7 @@ struct drm_mode_config { /* output poll support */ bool poll_enabled; bool poll_running; + bool delayed_event; struct delayed_work output_poll_work; /* pointers to standard properties */