diff mbox

drm/i915: Hold struct_mutex during hotplug processing

Message ID 1311613829-4990-1-git-send-email-keithp@keithp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Keith Packard July 25, 2011, 5:10 p.m. UTC 
Hotplug detection is a mode setting operation and must hold the
struct_mutex or risk colliding with other mode setting operations.

In particular, the display port hotplug function attempts to re-train
the link if the monitor is supposed to be running when plugged back
in. If that happens while mode setting is underway, the link will get
scrambled, leaving it in an inconsistent state.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/i915_irq.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Jesse Barnes July 25, 2011, 5:37 p.m. UTC | #1 
On Mon, 25 Jul 2011 10:10:29 -0700
Keith Packard <keithp@keithp.com> wrote:

> Hotplug detection is a mode setting operation and must hold the
> struct_mutex or risk colliding with other mode setting operations.
> 
> In particular, the display port hotplug function attempts to re-train
> the link if the monitor is supposed to be running when plugged back
> in. If that happens while mode setting is underway, the link will get
> scrambled, leaving it in an inconsistent state.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 3b03f85..5fe8f28 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -306,12 +306,15 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  	struct drm_mode_config *mode_config = &dev->mode_config;
>  	struct intel_encoder *encoder;
>  
> +	mutex_lock(&dev_priv->dev->struct_mutex);
>  	DRM_DEBUG_KMS("running encoder hotplug functions\n");
>  
>  	list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
>  		if (encoder->hot_plug)
>  			encoder->hot_plug(encoder);
>  
> +	mutex_unlock(&dev_priv->dev->struct_mutex);
> +
>  	/* Just fire off a uevent and let userspace tell us what to do */
>  	drm_helper_hpd_irq_event(dev);
>  }

yay, sounds like this will fix Andrew's problem and probably lots of
other random DP related failures.

Looks like the ->detect function is similarly protected at the call
site (though one level up in ->fill_modes), so it should be safe.
Looks like all the call sites in the link_status function are safe too.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Let's get this one upstream asap.  Should probably be cc'd to
stable@kernel.org as well.
Andrew Lutomirski July 25, 2011, 5:40 p.m. UTC | #2 
Will test tonight.

It looks like there is a lot of hotplug activity when 'xset dpms force
off' gets run.  (That's not a typo.  I do mean "off," not "on."

See attached trace.  perf rocks, even over ssh :)

--Andy

On Mon, Jul 25, 2011 at 1:37 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 25 Jul 2011 10:10:29 -0700
> Keith Packard <keithp@keithp.com> wrote:
>
>> Hotplug detection is a mode setting operation and must hold the
>> struct_mutex or risk colliding with other mode setting operations.
>>
>> In particular, the display port hotplug function attempts to re-train
>> the link if the monitor is supposed to be running when plugged back
>> in. If that happens while mode setting is underway, the link will get
>> scrambled, leaving it in an inconsistent state.
>>
>> Signed-off-by: Keith Packard <keithp@keithp.com>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 3b03f85..5fe8f28 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -306,12 +306,15 @@ static void i915_hotplug_work_func(struct work_struct *work)
>>       struct drm_mode_config *mode_config = &dev->mode_config;
>>       struct intel_encoder *encoder;
>>
>> +     mutex_lock(&dev_priv->dev->struct_mutex);
>>       DRM_DEBUG_KMS("running encoder hotplug functions\n");
>>
>>       list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
>>               if (encoder->hot_plug)
>>                       encoder->hot_plug(encoder);
>>
>> +     mutex_unlock(&dev_priv->dev->struct_mutex);
>> +
>>       /* Just fire off a uevent and let userspace tell us what to do */
>>       drm_helper_hpd_irq_event(dev);
>>  }
>
> yay, sounds like this will fix Andrew's problem and probably lots of
> other random DP related failures.
>
> Looks like the ->detect function is similarly protected at the call
> site (though one level up in ->fill_modes), so it should be safe.
> Looks like all the call sites in the link_status function are safe too.
>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> Let's get this one upstream asap.  Should probably be cc'd to
> stable@kernel.org as well.
>
> --
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Xorg  1463/1463  [000]  8894.988115: intel_dp_dpms: (ffffffffa0093026)
	ffffffffa0093027 intel_dp_dpms (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8895.005199: intel_dp_dpms_return: (ffffffffa0093026 <- ffffffffa006321b)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8905.765634: intel_dp_dpms: (ffffffffa0093026)
	ffffffffa0093027 intel_dp_dpms (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8905.765650: intel_dp_start_link_train: (ffffffffa009225f)
	ffffffffa0092260 intel_dp_start_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8905.817808: intel_dp_complete_link_train: (ffffffffa0092919)
	ffffffffa009291a intel_dp_complete_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8905.819069: intel_dp_dpms_return: (ffffffffa0093026 <- ffffffffa00631f5)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8920.030535: intel_dp_dpms: (ffffffffa0093026)
	ffffffffa0093027 intel_dp_dpms (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8920.048507: intel_dp_dpms_return: (ffffffffa0093026 <- ffffffffa006321b)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

kworker/u:5    82/82    [000]  8921.555293: intel_dp_hot_plug: (ffffffffa0092b5d)
	ffffffffa0092b5e intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.555297: intel_dp_check_link_status: (ffffffffa0092b6b)
	ffffffffa0092b6c intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.555298: intel_dp_hot_plug_return: (ffffffffa0092b5d <- ffffffffa006eb6c)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.555300: intel_dp_hot_plug: (ffffffffa0092b5d)
	ffffffffa0092b5e intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.555301: intel_dp_check_link_status: (ffffffffa0092b6b)
	ffffffffa0092b6c intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.555734: intel_dp_start_link_train: (ffffffffa009225f)
	ffffffffa0092260 intel_dp_start_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.610002: intel_dp_complete_link_train: (ffffffffa0092919)
	ffffffffa009291a intel_dp_complete_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.611037: intel_dp_start_link_train: (ffffffffa009225f)
	ffffffffa0092260 intel_dp_start_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0092c6a intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.666002: intel_dp_start_link_train: (ffffffffa009225f)
	ffffffffa0092260 intel_dp_start_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0092c6a intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.720970: intel_dp_start_link_train: (ffffffffa009225f)
	ffffffffa0092260 intel_dp_start_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0092c6a intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.775934: intel_dp_start_link_train: (ffffffffa009225f)
	ffffffffa0092260 intel_dp_start_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0092c6a intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.830905: intel_dp_start_link_train: (ffffffffa009225f)
	ffffffffa0092260 intel_dp_start_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0092c6a intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.885873: intel_dp_start_link_train: (ffffffffa009225f)
	ffffffffa0092260 intel_dp_start_link_train (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0092c6a intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.957591: intel_dp_hot_plug_return: (ffffffffa0092b5d <- ffffffffa006eb6c)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.957594: intel_dp_hot_plug: (ffffffffa0092b5d)
	ffffffffa0092b5e intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.957596: intel_dp_check_link_status: (ffffffffa0092b6b)
	ffffffffa0092b6c intel_dp_hot_plug (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kworker/u:5    82/82    [000]  8921.957597: intel_dp_hot_plug_return: (ffffffffa0092b5d <- ffffffffa006eb6c)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kthreadd  8526/8526  [000]  8921.970066: ironlake_dp_detect: (ffffffffa0092443)
	ffffffffa0092444 intel_dp_detect (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0063313 output_poll_execute (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm_kms_helper.ko)
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kthreadd  8526/8526  [000]  8922.001120: ironlake_dp_detect: (ffffffffa0092443)
	ffffffffa0092444 intel_dp_detect (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0063313 output_poll_execute (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm_kms_helper.ko)
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

kthreadd  8526/8526  [000]  8922.057433: ironlake_dp_detect: (ffffffffa0092443)
	ffffffffa0092444 intel_dp_detect (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa0063313 output_poll_execute (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm_kms_helper.ko)
	ffffffff8105c783 process_one_work ([kernel.kallsyms])
	ffffffff8105d2f4 worker_thread ([kernel.kallsyms])
	ffffffff81060722 kthread ([kernel.kallsyms])
	ffffffff81438414 kernel_thread_helper ([kernel.kallsyms])

Xorg  1463/1463  [006]  8924.761468: intel_dp_dpms: (ffffffffa0093026)
	ffffffffa0093027 intel_dp_dpms (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [006]  8924.761480: intel_dp_dpms_return: (ffffffffa0093026 <- ffffffffa00631f5)
	ffffffff81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
	ffffffffa002d6e8 drm_mode_connector_property_set_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffffa002085f drm_ioctl (/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
	ffffffff811049a5 do_vfs_ioctl ([kernel.kallsyms])
	ffffffff81104a3c sys_ioctl ([kernel.kallsyms])
	ffffffff814376bb system_call ([kernel.kallsyms])
	      30260d8957 __GI_ioctl (/lib64/libc-2.14.so)
[ 8894.988133] [drm:intel_dp_link_down], 
[ 8895.005211] [drm:ironlake_crtc_dpms], crtc 0/0 dpms off
[ 8895.057169] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8895.091566] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - plane 13, cursor: 6
[ 8895.091572] [drm:ironlake_check_srwm], watermark 1: display plane 25, fbc lines 3, cursor 6
[ 8895.091577] [drm:ironlake_check_srwm], watermark 2: display plane 33, fbc lines 3, cursor 6
[ 8895.091581] [drm:ironlake_check_srwm], watermark 3: display plane 169, fbc lines 4, cursor 10
[ 8895.091586] [drm:intel_update_fbc], 
[ 8905.658955] [drm:ironlake_crtc_dpms], crtc 0/0 dpms on
[ 8905.658962] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - plane 13, cursor: 6
[ 8905.658967] [drm:ironlake_check_srwm], watermark 1: display plane 25, fbc lines 3, cursor 6
[ 8905.658971] [drm:ironlake_check_srwm], watermark 2: display plane 33, fbc lines 3, cursor 6
[ 8905.658975] [drm:ironlake_check_srwm], watermark 3: display plane 169, fbc lines 4, cursor 10
[ 8905.710915] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8905.762883] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8905.763697] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x100
[ 8905.763701] [drm:gen6_fdi_link_train], FDI train 1 done.
[ 8905.764360] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x600
[ 8905.764364] [drm:gen6_fdi_link_train], FDI train 2 done.
[ 8905.764366] [drm:gen6_fdi_link_train], FDI train done.
[ 8905.765627] [drm:intel_update_fbc], 
[ 8905.816858] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8905.818850] [drm:intel_dp_complete_link_train], Training worked. DPCD=110A8401
[ 8920.030552] [drm:intel_dp_link_down], 
[ 8920.048519] [drm:ironlake_crtc_dpms], crtc 0/0 dpms off
[ 8920.100481] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8920.122880] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - plane 13, cursor: 6
[ 8920.122887] [drm:ironlake_check_srwm], watermark 1: display plane 25, fbc lines 3, cursor 6
[ 8920.122892] [drm:ironlake_check_srwm], watermark 2: display plane 33, fbc lines 3, cursor 6
[ 8920.122896] [drm:ironlake_check_srwm], watermark 3: display plane 169, fbc lines 4, cursor 10
[ 8920.122900] [drm:intel_update_fbc], 
[ 8921.555286] [drm:i915_hotplug_work_func], running encoder hotplug functions
[ 8921.555514] [drm:intel_dp_check_link_status], DPCD was 110A8401
[ 8921.555730] [drm:intel_dp_check_link_status], DPCD is now 110A8401
[ 8921.607586] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8921.662555] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8921.717522] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8921.772487] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8921.827456] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8921.882426] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8921.937393] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8921.939808] [drm:intel_dp_complete_link_train] *ERROR* failed to train DP, aborting
[ 8921.939813] [drm:intel_dp_complete_link_train], DPCD is 110A8401
[ 8921.939818] [drm:intel_dp_link_down], 
[ 8921.957373] [drm:intel_dp_complete_link_train], Training worked. DPCD=110A8401
[ 8921.957605] [drm:intel_ironlake_crt_detect_hotplug], ironlake hotplug adpa=0xf40000, result 0
[ 8921.957609] [drm:intel_crt_detect], CRT not detected via hotplug
[ 8921.957613] [drm:output_poll_execute], [CONNECTOR:5:VGA-1] status updated from 2 to 2
[ 8921.970060] [drm:output_poll_execute], [CONNECTOR:8:HDMI-A-1] status updated from 2 to 2
[ 8921.970072] [drm:ironlake_dp_detect], DPCD was 00000000
[ 8921.970588] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[ 8921.970592] [drm:ironlake_dp_detect], Try 0: ret=-110 DPCD=00000000
[ 8921.972865] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[ 8921.972866] [drm:ironlake_dp_detect], Try 1: ret=-110 DPCD=00000000
[ 8921.974867] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[ 8921.974869] [drm:ironlake_dp_detect], Try 2: ret=-110 DPCD=00000000
[ 8921.976354] [drm:ironlake_dp_detect], No link. DPCD: 00000000
[ 8921.976360] [drm:output_poll_execute], [CONNECTOR:11:DP-1] status updated from 2 to 2
[ 8921.988736] [drm:output_poll_execute], [CONNECTOR:14:HDMI-A-2] status updated from 2 to 2
[ 8922.001117] [drm:output_poll_execute], [CONNECTOR:16:HDMI-A-3] status updated from 2 to 2
[ 8922.001123] [drm:ironlake_dp_detect], DPCD was 110A8401
[ 8922.001337] [drm:ironlake_dp_detect], Try 0: ret=4 DPCD=110A8401
[ 8922.001339] [drm:ironlake_dp_detect], Happy now!
[ 8922.001341] [drm:ironlake_dp_detect], No link. DPCD: 110a8401
[ 8922.002389] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[ 8922.029953] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[ 8922.057428] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[ 8922.057429] [drm:drm_detect_monitor_audio], Monitor has basic audio support
[ 8922.057431] [drm:output_poll_execute], [CONNECTOR:17:DP-2] status updated from 1 to 1
[ 8922.057435] [drm:ironlake_dp_detect], DPCD was 00000000
[ 8922.057948] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[ 8922.057949] [drm:ironlake_dp_detect], Try 0: ret=-110 DPCD=00000000
[ 8922.059815] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[ 8922.059818] [drm:ironlake_dp_detect], Try 1: ret=-110 DPCD=00000000
[ 8922.061817] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[ 8922.061823] [drm:ironlake_dp_detect], Try 2: ret=-110 DPCD=00000000
[ 8922.063303] [drm:ironlake_dp_detect], No link. DPCD: 00000000
[ 8922.063309] [drm:output_poll_execute], [CONNECTOR:19:DP-3] status updated from 2 to 2
[ 8924.655081] [drm:ironlake_crtc_dpms], crtc 0/0 dpms on
[ 8924.655087] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - plane 13, cursor: 6
[ 8924.655091] [drm:ironlake_check_srwm], watermark 1: display plane 25, fbc lines 3, cursor 6
[ 8924.655096] [drm:ironlake_check_srwm], watermark 2: display plane 33, fbc lines 3, cursor 6
[ 8924.655100] [drm:ironlake_check_srwm], watermark 3: display plane 169, fbc lines 4, cursor 10
[ 8924.706753] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8924.758721] [drm:intel_wait_for_vblank], vblank wait timed out
[ 8924.759533] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x100
[ 8924.759537] [drm:gen6_fdi_link_train], FDI train 1 done.
[ 8924.760196] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x600
[ 8924.760200] [drm:gen6_fdi_link_train], FDI train 2 done.
[ 8924.760202] [drm:gen6_fdi_link_train], FDI train done.
[ 8924.761462] [drm:intel_update_fbc],
Keith Packard July 25, 2011, 5:55 p.m. UTC | #3 
On Mon, 25 Jul 2011 13:40:58 -0400, Andrew Lutomirski <luto@mit.edu> wrote:

> Will test tonight.

Thanks. 

> It looks like there is a lot of hotplug activity when 'xset dpms force
> off' gets run.  (That's not a typo.  I do mean "off," not "on."

Yup, that's what I've seen as well -- do a mode set to turn stuff off
and you get spammed with hotplug events.

> See attached trace.  perf rocks, even over ssh :)

Wow. You're nested about three deep in the mode setting code due to
overlapping hotplug events. What could possibly go wrong?

Makes me optimistic that a bit of locking will help a lot here.
Daniel Vetter July 26, 2011, 7:24 a.m. UTC | #4 
Two things I've noticed:
- Why not dev->mode_config.mutex? I was under the impression that
mode_config.mutex protects most of the modesetting state and
dev->struct_mutex protects things related to the gpu execution cores
(i.e. all things gem), with struct_mutex nested within
mode_config.mutex. It's hazy at the edges and likely broken in tons of
corner cases, but still ... This has also the benefit that it won't
stall execbuf.
- And a nitpick: Why the dev_priv->dev indirection, when dev is
already lying around?

-Daniel
Keith Packard July 26, 2011, 3:23 p.m. UTC | #5 
On Tue, 26 Jul 2011 09:24:39 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> Two things I've noticed:

> - Why not dev->mode_config.mutex?

You're right, of course. I noticed that just after posting that version
and updated it; the updated version is on my drm-intel-fixes branch
already (having been reviewed by Jesse).

> - And a nitpick: Why the dev_priv->dev indirection, when dev is
> already lying around?

All nicely cleaned up by using &mode_config->mutex instead :-)

Thanks for looking it over!
Jesse Barnes July 26, 2011, 7:12 p.m. UTC | #6 
On Tue, 26 Jul 2011 08:23:13 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Tue, 26 Jul 2011 09:24:39 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> > Two things I've noticed:
> 
> > - Why not dev->mode_config.mutex?
> 
> You're right, of course. I noticed that just after posting that version
> and updated it; the updated version is on my drm-intel-fixes branch
> already (having been reviewed by Jesse).
> 
> > - And a nitpick: Why the dev_priv->dev indirection, when dev is
> > already lying around?
> 
> All nicely cleaned up by using &mode_config->mutex instead :-)
> 
> Thanks for looking it over!

I'd like to amend my reviewed by and say the lock shouldn't be held
around the call to the drm helper function.  It queues some work that
also takes the mode config lock, which will break.  So you can drop it
before that...  Previously I had only checked our internal driver
callbacks but missed that the lock was held across the helper too.
Keith Packard July 27, 2011, 9:21 a.m. UTC | #7 
On Tue, 26 Jul 2011 12:12:25 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> I'd like to amend my reviewed by and say the lock shouldn't be held
> around the call to the drm helper function.  It queues some work that
> also takes the mode config lock, which will break.  So you can drop it
> before that...  Previously I had only checked our internal driver
> callbacks but missed that the lock was held across the helper too.

So the work may get executed immediately rather than being run later at
some point?
Jesse Barnes July 27, 2011, 4:03 p.m. UTC | #8 
On Wed, 27 Jul 2011 02:21:24 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Tue, 26 Jul 2011 12:12:25 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > I'd like to amend my reviewed by and say the lock shouldn't be held
> > around the call to the drm helper function.  It queues some work that
> > also takes the mode config lock, which will break.  So you can drop it
> > before that...  Previously I had only checked our internal driver
> > callbacks but missed that the lock was held across the helper too.
> 
> So the work may get executed immediately rather than being run later at
> some point?

It sure looks that way... but I don't remember any rule about work
queue items having inter dependencies like this.
Keith Packard July 28, 2011, 10:50 p.m. UTC | #9 
On Wed, 27 Jul 2011 09:03:31 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Wed, 27 Jul 2011 02:21:24 -0700
> Keith Packard <keithp@keithp.com> wrote:
> 
> > On Tue, 26 Jul 2011 12:12:25 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > 
> > > I'd like to amend my reviewed by and say the lock shouldn't be held
> > > around the call to the drm helper function.  It queues some work that
> > > also takes the mode config lock, which will break.  So you can drop it
> > > before that...  Previously I had only checked our internal driver
> > > callbacks but missed that the lock was held across the helper too.
> > 
> > So the work may get executed immediately rather than being run later at
> > some point?
> 
> It sure looks that way... but I don't remember any rule about work
> queue items having inter dependencies like this.

Sounds fine; I've stuck a fixup that moves the mutex_unlock:

 		if (encoder->hot_plug)
 			encoder->hot_plug(encoder);
 
+	mutex_unlock(&mode_config->mutex);
+
 	/* Just fire off a uevent and let userspace tell us what to do */
 	drm_helper_hpd_irq_event(dev);
-
-	mutex_unlock(&mode_config->mutex);
 }
Daniel Vetter July 29, 2011, 10:25 a.m. UTC | #10 
On Thu, Jul 28, 2011 at 03:50:00PM -0700, Keith Packard wrote:
> On Wed, 27 Jul 2011 09:03:31 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Wed, 27 Jul 2011 02:21:24 -0700
> > Keith Packard <keithp@keithp.com> wrote:
> > > So the work may get executed immediately rather than being run later at
> > > some point?
> > 
> > It sure looks that way... but I don't remember any rule about work
> > queue items having inter dependencies like this.

I've checked the workqueue code and haven't found it to run a work
immediately, it's always queued. Further this problem is very easy to
diagnose: Even without lockdep the scheduler will notice the stuck task
after about 120s and the backtrace should make matters extremely clear. On
the other hand if somebody adds some nice state clobbering in the drm
helper, we have a very hard bug to track down.

Generally modesetting isn't perf critical, so I vote for more locking,
just in case.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 3b03f85..5fe8f28 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -306,12 +306,15 @@  static void i915_hotplug_work_func(struct work_struct *work)
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct intel_encoder *encoder;
 
+	mutex_lock(&dev_priv->dev->struct_mutex);
 	DRM_DEBUG_KMS("running encoder hotplug functions\n");
 
 	list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
 		if (encoder->hot_plug)
 			encoder->hot_plug(encoder);
 
+	mutex_unlock(&dev_priv->dev->struct_mutex);
+
 	/* Just fire off a uevent and let userspace tell us what to do */
 	drm_helper_hpd_irq_event(dev);
 }