[PATCH] drm/i915: Hold struct_mutex during hotplug processing

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH] drm/i915: Hold struct_mutex during hotplug processing
@ 2011-07-25 17:10 Keith Packard
  2011-07-25 17:37 ` [Intel-gfx] " Jesse Barnes
  2011-07-26  7:24 ` Daniel Vetter
  0 siblings, 2 replies; 14+ messages in thread
From: Keith Packard @ 2011-07-25 17:10 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-kernel, dri-devel, intel-gfx, Keith Packard

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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-25 17:10 [PATCH] drm/i915: Hold struct_mutex during hotplug processing Keith Packard
@ 2011-07-25 17:37 ` Jesse Barnes
  2011-07-25 17:40   ` Andrew Lutomirski
  2011-07-26  2:52   ` Andrew Lutomirski
  2011-07-26  7:24 ` Daniel Vetter
  1 sibling, 2 replies; 14+ messages in thread
From: Jesse Barnes @ 2011-07-25 17:37 UTC (permalink / raw)
  To: Keith Packard; +Cc: Dave Airlie, intel-gfx, linux-kernel, dri-devel

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-25 17:37 ` [Intel-gfx] " Jesse Barnes
@ 2011-07-25 17:40   ` Andrew Lutomirski
  2011-07-25 17:55     ` Keith Packard
  2011-07-26  2:52   ` Andrew Lutomirski
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Lutomirski @ 2011-07-25 17:40 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Keith Packard, Dave Airlie, intel-gfx, linux-kernel, dri-devel

[-- Attachment #1: Type: text/plain, Size: 2580 bytes --]

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
>

[-- Attachment #2: trace.txt --]
[-- Type: text/plain, Size: 15633 bytes --]

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)


[-- Attachment #3: dmesg_2.txt --]
[-- Type: text/plain, Size: 6523 bytes --]

[ 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], 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-25 17:40   ` Andrew Lutomirski
@ 2011-07-25 17:55     ` Keith Packard
  0 siblings, 0 replies; 14+ messages in thread
From: Keith Packard @ 2011-07-25 17:55 UTC (permalink / raw)
  To: Andrew Lutomirski, Jesse Barnes
  Cc: Dave Airlie, intel-gfx, linux-kernel, dri-devel

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

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.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-25 17:37 ` [Intel-gfx] " Jesse Barnes
  2011-07-25 17:40   ` Andrew Lutomirski
@ 2011-07-26  2:52   ` Andrew Lutomirski
  2011-07-26  5:54     ` Keith Packard
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Lutomirski @ 2011-07-26  2:52 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Keith Packard, Dave Airlie, intel-gfx, linux-kernel, dri-devel

[-- Attachment #1: Type: text/plain, Size: 2481 bytes --]

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.

Doesn't help :(

When I do 'xset dpms force off', one of two things happens.  Either
the display comes back all by itself or it never comes back until I
power cycle it.

If the display is generating a hotplug event after the dpms code drops
the link, then with "drm/i915/dp: remove DPMS mode tracking from DP"
applied the driver will try to bring the display back up.  Maybe my
display can't handle coming back up that quickly after being told to
go to sleep, or maybe there's another bug.

Is the original patch supposed to bring the display up if the user
unplugs it and re-plugs it?  If so, why?  And shouldn't a dpms off
command at least stick until a hotplug event reports that the display
isn't there?

--Andy

[-- Attachment #2: dmesg_4.txt --]
[-- Type: text/plain, Size: 17876 bytes --]

[  209.146016] xset dpms force off
[  214.014555] [drm:drm_mode_addfb], [FB:48]
[  214.162664] [drm:drm_mode_addfb], [FB:40]
[  214.485742] [drm:drm_mode_addfb], [FB:48]
[  214.652646] [drm:drm_mode_addfb], [FB:40]
[  214.934948] [drm:drm_mode_addfb], [FB:48]
[  215.119459] [drm:drm_mode_addfb], [FB:40]
[  215.378005] [drm:drm_mode_addfb], [FB:48]
[  215.581211] [drm:drm_mode_addfb], [FB:40]
[  217.541498] [drm:drm_mode_addfb], [FB:48]
[  217.579117] [drm:drm_mode_addfb], [FB:40]
[  217.588287] [drm:drm_mode_addfb], [FB:48]
[  217.604938] [drm:drm_mode_addfb], [FB:40]
[  217.638301] [drm:drm_mode_addfb], [FB:48]
[  217.654970] [drm:drm_mode_addfb], [FB:40]
[  217.671641] [drm:drm_mode_addfb], [FB:48]
[  217.728388] [drm:drm_mode_addfb], [FB:40]
[  218.007141] [drm:drm_mode_addfb], [FB:48]
[  218.030397] btrfs: free space inode generation (0) did not match free space cache generation (132273)
[  218.030402] btrfs: failed to load free space cache for block group 18282971136
[  218.275505] [drm:drm_mode_addfb], [FB:40]
[  218.353250] [drm:drm_mode_addfb], [FB:48]
[  218.378388] [drm:drm_mode_addfb], [FB:40]
[  218.405154] [drm:drm_mode_addfb], [FB:48]
[  218.422545] [drm:drm_mode_addfb], [FB:40]
[  218.455165] [drm:drm_mode_addfb], [FB:48]
[  218.471794] [drm:drm_mode_addfb], [FB:40]
[  218.490260] [drm:drm_mode_addfb], [FB:48]
[  218.521845] [drm:drm_mode_addfb], [FB:40]
[  218.538515] [drm:drm_mode_addfb], [FB:48]
[  218.556996] [drm:drm_mode_addfb], [FB:40]
[  218.600422] [drm:drm_mode_addfb], [FB:48]
[  218.806993] [drm:drm_mode_addfb], [FB:40]
[  225.711943] [drm:intel_dp_dpms], start dpms -> 3
[  225.712160] [drm:intel_dp_link_down], 
[  225.729245] [drm:intel_dp_dpms], finish dpms -> 3
[  227.242038] [drm:i915_hotplug_work_func], running encoder hotplug functions
[  227.242043] [drm:intel_dp_hot_plug], about to check status
[  227.242046] [drm:intel_dp_hot_plug], done checking status
[  227.242049] [drm:intel_dp_hot_plug], about to check status
[  227.242474] [drm:intel_dp_check_link_status], eq okay
[  227.294320] [drm:intel_wait_for_vblank], vblank wait timed out
[  227.295270] [drm:intel_dp_check_link_status], start_link_train done
[  227.296520] [drm:intel_dp_check_link_status], complete_link_train done
[  227.296523] [drm:intel_dp_hot_plug], done checking status
[  227.296525] [drm:intel_dp_hot_plug], about to check status
[  227.296527] [drm:intel_dp_hot_plug], done checking status
[  227.296539] [drm:intel_ironlake_crt_detect_hotplug], ironlake hotplug adpa=0xf40000, result 0
[  227.296543] [drm:intel_crt_detect], CRT not detected via hotplug
[  227.296547] [drm:output_poll_execute], [CONNECTOR:5:VGA-1] status updated from 2 to 2
[  227.308959] [drm:output_poll_execute], [CONNECTOR:8:HDMI-A-1] status updated from 2 to 2
[  227.309476] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  227.311820] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  227.313820] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  227.315302] [drm:ironlake_dp_detect], DPCD: 0000
[  227.315308] [drm:output_poll_execute], [CONNECTOR:11:DP-1] status updated from 2 to 2
[  227.327723] [drm:output_poll_execute], [CONNECTOR:14:HDMI-A-2] status updated from 2 to 2
[  227.340129] [drm:output_poll_execute], [CONNECTOR:16:HDMI-A-3] status updated from 2 to 2
[  227.340343] [drm:ironlake_dp_detect], DPCD: 11a841
[  227.341393] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  227.368970] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  227.396544] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  227.396548] [drm:drm_detect_monitor_audio], Monitor has basic audio support
[  227.396551] [drm:output_poll_execute], [CONNECTOR:17:DP-2] status updated from 1 to 1
[  227.397067] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  227.398764] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  227.400763] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  227.402249] [drm:ironlake_dp_detect], DPCD: 0000
[  227.402255] [drm:output_poll_execute], [CONNECTOR:19:DP-3] status updated from 2 to 2
[  233.549279] [drm:ironlake_crtc_dpms], crtc 0/0 dpms on
[  233.549286] [drm:intel_dp_dpms], start dpms -> 0
[  233.549500] [drm:intel_dp_dpms], finish dpms -> 0
[  234.690684] [drm:intel_crtc_cursor_set], 
[  234.690688] [drm:intel_crtc_cursor_set], cursor off
[  245.452566] display came back on its own and made noise.  trying again.
[  246.884278] [drm:intel_crtc_cursor_set], 
[  246.906823] [drm:drm_mode_addfb], [FB:48]
[  246.953632] [drm:intel_crtc_cursor_set], 
[  246.953638] [drm:intel_crtc_cursor_set], cursor off
[  247.044279] [drm:drm_mode_addfb], [FB:40]
[  248.976247] [drm:intel_crtc_cursor_set], 
[  250.440201] [drm:intel_dp_dpms], start dpms -> 3
[  250.440417] [drm:intel_dp_link_down], 
[  250.457923] [drm:intel_dp_dpms], finish dpms -> 3
[  250.457929] [drm:ironlake_crtc_dpms], crtc 0/0 dpms off
[  250.509877] [drm:intel_wait_for_vblank], vblank wait timed out
[  250.530282] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - plane 13, cursor: 6
[  250.530289] [drm:ironlake_check_srwm], watermark 1: display plane 25, fbc lines 3, cursor 6
[  250.530293] [drm:ironlake_check_srwm], watermark 2: display plane 33, fbc lines 3, cursor 6
[  250.530298] [drm:ironlake_check_srwm], watermark 3: display plane 169, fbc lines 4, cursor 10
[  250.530302] [drm:intel_update_fbc], 
[  251.965079] [drm:i915_hotplug_work_func], running encoder hotplug functions
[  251.965084] [drm:intel_dp_hot_plug], about to check status
[  251.965087] [drm:intel_dp_hot_plug], done checking status
[  251.965089] [drm:intel_dp_hot_plug], about to check status
[  251.965513] [drm:intel_dp_check_link_status], eq okay
[  252.017001] [drm:intel_wait_for_vblank], vblank wait timed out
[  252.019417] [drm:intel_dp_check_link_status], start_link_train done
[  252.071968] [drm:intel_wait_for_vblank], vblank wait timed out
[  252.126947] [drm:intel_wait_for_vblank], vblank wait timed out
[  252.181904] [drm:intel_wait_for_vblank], vblank wait timed out
[  252.236873] [drm:intel_wait_for_vblank], vblank wait timed out
[  252.291844] [drm:intel_wait_for_vblank], vblank wait timed out
[  252.346809] [drm:intel_wait_for_vblank], vblank wait timed out
[  252.349226] [drm:intel_dp_complete_link_train] *ERROR* failed to train DP, aborting
[  252.349231] [drm:intel_dp_link_down], 
[  252.367024] [drm:intel_dp_check_link_status], complete_link_train done
[  252.367029] [drm:intel_dp_hot_plug], done checking status
[  252.367032] [drm:intel_dp_hot_plug], about to check status
[  252.367034] [drm:intel_dp_hot_plug], done checking status
[  252.367048] [drm:intel_ironlake_crt_detect_hotplug], ironlake hotplug adpa=0xf40000, result 0
[  252.367053] [drm:intel_crt_detect], CRT not detected via hotplug
[  252.367058] [drm:output_poll_execute], [CONNECTOR:5:VGA-1] status updated from 2 to 2
[  252.379563] [drm:output_poll_execute], [CONNECTOR:8:HDMI-A-1] status updated from 2 to 2
[  252.380081] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  252.382294] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  252.384296] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  252.385781] [drm:ironlake_dp_detect], DPCD: 0000
[  252.385787] [drm:output_poll_execute], [CONNECTOR:11:DP-1] status updated from 2 to 2
[  252.398254] [drm:output_poll_execute], [CONNECTOR:14:HDMI-A-2] status updated from 2 to 2
[  252.410702] [drm:output_poll_execute], [CONNECTOR:16:HDMI-A-3] status updated from 2 to 2
[  252.410917] [drm:ironlake_dp_detect], DPCD: 11a841
[  252.411969] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  252.439573] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  252.467183] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  252.467187] [drm:drm_detect_monitor_audio], Monitor has basic audio support
[  252.467191] [drm:output_poll_execute], [CONNECTOR:17:DP-2] status updated from 1 to 1
[  252.467708] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  252.469241] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  252.471242] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  252.472731] [drm:ironlake_dp_detect], DPCD: 0000
[  252.472737] [drm:output_poll_execute], [CONNECTOR:19:DP-3] status updated from 2 to 2
[  260.423504] [drm:ironlake_crtc_dpms], crtc 0/0 dpms on
[  260.423510] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - plane 13, cursor: 6
[  260.423514] [drm:ironlake_check_srwm], watermark 1: display plane 25, fbc lines 3, cursor 6
[  260.423519] [drm:ironlake_check_srwm], watermark 2: display plane 33, fbc lines 3, cursor 6
[  260.423523] [drm:ironlake_check_srwm], watermark 3: display plane 169, fbc lines 4, cursor 10
[  260.475106] [drm:intel_wait_for_vblank], vblank wait timed out
[  260.527076] [drm:intel_wait_for_vblank], vblank wait timed out
[  260.527890] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x100
[  260.527894] [drm:gen6_fdi_link_train], FDI train 1 done.
[  260.528564] [drm:gen6_fdi_link_train], FDI_RX_IIR 0x600
[  260.528568] [drm:gen6_fdi_link_train], FDI train 2 done.
[  260.528570] [drm:gen6_fdi_link_train], FDI train done.
[  260.529839] [drm:intel_update_fbc], 
[  260.529846] [drm:intel_dp_dpms], start dpms -> 0
[  260.530060] [drm:intel_dp_dpms], finish dpms -> 0
[  280.100978] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.101391] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.101415] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.101872] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.101894] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.102375] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.102391] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.102891] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.102917] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.103373] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.103391] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.103394] hub 3-1.1:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
[  280.103873] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.103893] hub 3-1.1:1.0: cannot disable port 1 (err = -71)
[  280.104356] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.104372] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.104872] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.104891] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.105358] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.105373] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.105502] usb 3-1: USB disconnect, device number 2
[  280.105504] usb 3-1.1: USB disconnect, device number 3
[  280.105507] usb 3-1.1.1: USB disconnect, device number 4
[  280.105870] xhci_hcd 0000:02:00.0: WARN: transfer error on endpoint
[  280.105890] hub 3-1.1:1.0: cannot reset port 1 (err = -71)
[  280.105894] hub 3-1.1:1.0: cannot reset port 1 (err = -19)
[  280.105897] hub 3-1.1:1.0: cannot disable port 1 (err = -19)
[  280.105901] hub 3-1.1:1.0: cannot disable port 1 (err = -19)
[  280.996451] [drm:i915_hotplug_work_func], running encoder hotplug functions
[  280.996457] [drm:intel_dp_hot_plug], about to check status
[  280.996459] [drm:intel_dp_hot_plug], done checking status
[  280.996462] [drm:intel_dp_hot_plug], about to check status
[  280.996978] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5013003e
[  280.998628] [drm:intel_dp_check_link_status], eq okay
[  281.050190] [drm:intel_wait_for_vblank], vblank wait timed out
[  281.051140] [drm:intel_dp_check_link_status], start_link_train done
[  281.052389] [drm:intel_dp_check_link_status], complete_link_train done
[  281.052392] [drm:intel_dp_hot_plug], done checking status
[  281.052395] [drm:intel_dp_hot_plug], about to check status
[  281.052397] [drm:intel_dp_hot_plug], done checking status
[  281.052409] [drm:intel_ironlake_crt_detect_hotplug], ironlake hotplug adpa=0xf40000, result 0
[  281.052414] [drm:intel_crt_detect], CRT not detected via hotplug
[  281.052417] [drm:output_poll_execute], [CONNECTOR:5:VGA-1] status updated from 2 to 2
[  281.064820] [drm:output_poll_execute], [CONNECTOR:8:HDMI-A-1] status updated from 2 to 2
[  281.065337] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  281.067688] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  281.069688] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  281.071169] [drm:ironlake_dp_detect], DPCD: 0000
[  281.071175] [drm:output_poll_execute], [CONNECTOR:11:DP-1] status updated from 2 to 2
[  281.083592] [drm:output_poll_execute], [CONNECTOR:14:HDMI-A-2] status updated from 2 to 2
[  281.095999] [drm:output_poll_execute], [CONNECTOR:16:HDMI-A-3] status updated from 2 to 2
[  281.096214] [drm:ironlake_dp_detect], DPCD: 11a841
[  281.097265] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  281.124844] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  281.152425] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  281.152429] [drm:drm_detect_monitor_audio], Monitor has basic audio support
[  281.152432] [drm:output_poll_execute], [CONNECTOR:17:DP-2] status updated from 1 to 1
[  281.152949] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  281.154632] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  281.156635] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  281.158118] [drm:ironlake_dp_detect], DPCD: 0000
[  281.158124] [drm:output_poll_execute], [CONNECTOR:19:DP-3] status updated from 2 to 2
[  281.447987] usb 3-1: new high speed USB device number 5 using xhci_hcd
[  281.460447] usb 3-1: New USB device found, idVendor=0424, idProduct=2514
[  281.460452] usb 3-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
[  281.464721] hub 3-1:1.0: USB hub found
[  281.464892] xhci_hcd 0000:02:00.0: WARN: short transfer on control ep
[  281.465031] hub 3-1:1.0: 3 ports detected
[  281.741020] usb 3-1.1: new high speed USB device number 6 using xhci_hcd
[  281.753266] usb 3-1.1: New USB device found, idVendor=0424, idProduct=2640
[  281.753271] usb 3-1.1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
[  281.753933] hub 3-1.1:1.0: USB hub found
[  281.754089] xhci_hcd 0000:02:00.0: WARN: short transfer on control ep
[  281.754226] hub 3-1.1:1.0: 3 ports detected
[  282.029841] usb 3-1.1.1: new high speed USB device number 7 using xhci_hcd
[  282.052811] xhci_hcd 0000:02:00.0: WARN: short transfer on control ep
[  282.058310] xhci_hcd 0000:02:00.0: WARN: short transfer on control ep
[  282.062803] xhci_hcd 0000:02:00.0: WARN: short transfer on control ep
[  282.066427] xhci_hcd 0000:02:00.0: WARN: short transfer on control ep
[  282.066786] usb 3-1.1.1: New USB device found, idVendor=0424, idProduct=4063
[  282.066790] usb 3-1.1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[  282.066793] usb 3-1.1.1: Product: Ultra Fast Media Reader
[  282.066795] usb 3-1.1.1: Manufacturer: Generic
[  282.066797] usb 3-1.1.1: SerialNumber: 000000264001
[  282.067756] scsi9 : usb-storage 3-1.1.1:1.0
[  282.143739] mtp-probe[2660]: bus: 3, device: 7 was not an MTP device
[  283.072796] scsi 9:0:0:0: Direct-Access     Generic  Ultra HS-SD/MMC  1.91 PQ: 0 ANSI: 0
[  283.076322] sd 9:0:0:0: Attached scsi generic sg3 type 0
[  283.085085] sd 9:0:0:0: [sdc] Attached SCSI removable disk
[  283.841290] [drm:i915_hotplug_work_func], running encoder hotplug functions
[  283.841296] [drm:intel_dp_hot_plug], about to check status
[  283.841298] [drm:intel_dp_hot_plug], done checking status
[  283.841301] [drm:intel_dp_hot_plug], about to check status
[  283.841725] [drm:intel_dp_check_link_status], eq not okay
[  283.841728] [drm:intel_dp_hot_plug], done checking status
[  283.841730] [drm:intel_dp_hot_plug], about to check status
[  283.841733] [drm:intel_dp_hot_plug], done checking status
[  283.841760] [drm:intel_ironlake_crt_detect_hotplug], ironlake hotplug adpa=0xf40000, result 0
[  283.841764] [drm:intel_crt_detect], CRT not detected via hotplug
[  283.841768] [drm:output_poll_execute], [CONNECTOR:5:VGA-1] status updated from 2 to 2
[  283.854171] [drm:output_poll_execute], [CONNECTOR:8:HDMI-A-1] status updated from 2 to 2
[  283.854687] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  283.857067] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  283.859067] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  283.860554] [drm:ironlake_dp_detect], DPCD: 0000
[  283.860560] [drm:output_poll_execute], [CONNECTOR:11:DP-1] status updated from 2 to 2
[  283.872922] [drm:output_poll_execute], [CONNECTOR:14:HDMI-A-2] status updated from 2 to 2
[  283.885271] [drm:output_poll_execute], [CONNECTOR:16:HDMI-A-3] status updated from 2 to 2
[  283.885483] [drm:ironlake_dp_detect], DPCD: 11a841
[  283.886530] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  283.914083] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  283.941635] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  283.941636] [drm:drm_detect_monitor_audio], Monitor has basic audio support
[  283.941638] [drm:output_poll_execute], [CONNECTOR:17:DP-2] status updated from 1 to 1
[  283.942152] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  283.944017] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  283.946018] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  283.947505] [drm:ironlake_dp_detect], DPCD: 0000
[  283.947511] [drm:output_poll_execute], [CONNECTOR:19:DP-3] status updated from 2 to 2
[  290.913994] [drm:intel_crtc_cursor_set], 
[  290.913998] [drm:intel_crtc_cursor_set], cursor off
[  309.750888] this time it made noise and went to sleep.  it failed to come back on its own and needed a power cycle

[-- Attachment #3: i915_dpms_trace.patch --]
[-- Type: text/x-patch, Size: 1826 bytes --]

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e2aced6..550b329 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1020,6 +1020,8 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t dp_reg = I915_READ(intel_dp->output_reg);
 
+	DRM_DEBUG_KMS("start dpms -> %d\n", mode);
+
 	if (mode != DRM_MODE_DPMS_ON) {
 		if (is_edp(intel_dp))
 			ironlake_edp_backlight_off(dev);
@@ -1044,6 +1046,8 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 		if (is_edp(intel_dp))
 			ironlake_edp_backlight_on(dev);
 	}
+
+	DRM_DEBUG_KMS("finish dpms -> %d\n", mode);
 }
 
 /*
@@ -1577,6 +1581,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 
 	if (!intel_dp_get_link_status(intel_dp)) {
 		intel_dp_link_down(intel_dp);
+		DRM_DEBUG_KMS("link is down");
 		return;
 	}
 
@@ -1586,12 +1591,18 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 				       sizeof (intel_dp->dpcd));
 	if (ret != sizeof(intel_dp->dpcd)) {
 		intel_dp_link_down(intel_dp);
+		DRM_DEBUG_KMS("failed to read DPCD");
 		return;
 	}
 
 	if (!intel_channel_eq_ok(intel_dp)) {
+		DRM_DEBUG_KMS("eq okay");
 		intel_dp_start_link_train(intel_dp);
+		DRM_DEBUG_KMS("start_link_train done");
 		intel_dp_complete_link_train(intel_dp);
+		DRM_DEBUG_KMS("complete_link_train done");
+	} else {
+		DRM_DEBUG_KMS("eq not okay");
 	}
 }
 
@@ -1855,7 +1866,10 @@ intel_dp_hot_plug(struct intel_encoder *intel_encoder)
 {
 	struct intel_dp *intel_dp = container_of(intel_encoder, struct intel_dp, base);
 
+	DRM_DEBUG_KMS("about to check status");
 	intel_dp_check_link_status(intel_dp);
+	DRM_DEBUG_KMS("done checking status");
+
 }
 
 /* Return which DP Port should be selected for Transcoder DP control */

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-26  2:52   ` Andrew Lutomirski
@ 2011-07-26  5:54     ` Keith Packard
  2011-07-26 12:35       ` Andrew Lutomirski
  0 siblings, 1 reply; 14+ messages in thread
From: Keith Packard @ 2011-07-26  5:54 UTC (permalink / raw)
  To: Andrew Lutomirski, Jesse Barnes
  Cc: Dave Airlie, intel-gfx, linux-kernel, dri-devel

[-- Attachment #1: Type: text/plain, Size: 4512 bytes --]

On Mon, 25 Jul 2011 22:52:15 -0400, Andrew Lutomirski <luto@mit.edu> wrote:

> Doesn't help :(

Oh, it helps, just not this issue :-)

> When I do 'xset dpms force off', one of two things happens.  Either
> the display comes back all by itself or it never comes back until I
> power cycle it.

Oh. Right. Actual DPMS, as in power savings. So, I think Jesse and I
were both wrong -- we *do* need to track DPMS in the DP driver so that
we can skip link retraining when DPMS is off. The rest of the connection
is disabled, so doing the link retraining may well confuse the DP system
badly. Turning the monitor off and back on will generate suitable hot
plug messages and presumably clean things up once the rest of the stack
is running again.

> If the display is generating a hotplug event after the dpms code drops
> the link, then with "drm/i915/dp: remove DPMS mode tracking from DP"
> applied the driver will try to bring the display back up.  Maybe my
> display can't handle coming back up that quickly after being told to
> go to sleep, or maybe there's another bug.

Indeed you are correct -- we don't gate the retraining on whether the
monitor is supposed to be running or not.

> Is the original patch supposed to bring the display up if the user
> unplugs it and re-plugs it?  If so, why?

Yes. In the absence of an external agent listening to the hotplug events
(which is hard to arrange these days, unless you're running an X
environment that doesn't watch for monitor hot-plug), the kernel will
see the hot-plug event, decide that because the monitor is supposed to
be running (has a CRTC assigned), it will go ahead and try to retrain
the DP link.

> And shouldn't a dpms off
> command at least stick until a hotplug event reports that the display
> isn't there?

The DPMS off should stick until DPMS is set back on -- hotplug shouldn't
have any effect on DPMS.

Here's a patch which reverts the DPMS tracking, and then fixes the bug
that it had -- you wouldn't get retraining on hotplug after the driver
had been initialized because nothing in the mode setting path would set
the dpms_mode to DRM_MODE_DPMS_ON, so the hotplug code would bail every
time. With that fixed, this patch should work for you *and* for others.

Care to give it a try?

From 59b920597999381fab70c485c161dd50590e561a Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Mon, 25 Jul 2011 22:37:51 -0700
Subject: [PATCH] Revert and fix "drm/i915/dp: remove DPMS mode tracking from
 DP"

This reverts commit 885a50147f00a8a80108904bf58a18af357717f3.

We actually *do* need to track DPMS state so that on hotplug, we don't
retrain the link until DPMS is disabled.

However, that code had a small bug -- it wouldn't set the dpms_mode at
mode set time, and so link retraining would not actually occur on
monitor hotplug until the monitor had gone through a DPMS off/DPMS on
cycle. Setting dpms_mode to DRM_MODE_DPMS_ON in intel_dp_commit fixes that.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9f134d2..4493641 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -50,6 +50,7 @@ struct intel_dp {
 	bool has_audio;
 	int force_audio;
 	uint32_t color_range;
+	int dpms_mode;
 	uint8_t link_bw;
 	uint8_t lane_count;
 	uint8_t dpcd[8];
@@ -1011,6 +1012,8 @@ static void intel_dp_commit(struct drm_encoder *encoder)

 	if (is_edp(intel_dp))
 		ironlake_edp_backlight_on(dev);
+
+	intel_dp->dpms_mode = DRM_MODE_DPMS_ON;
 }

 static void
@@ -1045,6 +1048,7 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 		if (is_edp(intel_dp))
 			ironlake_edp_backlight_on(dev);
 	}
+	intel_dp->dpms_mode = mode;
 }

 /*
@@ -1591,6 +1595,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 static void
 intel_dp_check_link_status(struct intel_dp *intel_dp)
 {
+	if (intel_dp->dpms_mode != DRM_MODE_DPMS_ON)
+		return;
+
 	if (!intel_dp->base.base.crtc)
 		return;

@@ -1939,6 +1946,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 		return;

 	intel_dp->output_reg = output_reg;
+	intel_dp->dpms_mode = -1;

 	intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
 	if (!intel_connector) {
-- 
1.7.5.4

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-26  5:54     ` Keith Packard
@ 2011-07-26 12:35       ` Andrew Lutomirski
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Lutomirski @ 2011-07-26 12:35 UTC (permalink / raw)
  To: Keith Packard
  Cc: Jesse Barnes, Dave Airlie, intel-gfx, linux-kernel, dri-devel

On Tue, Jul 26, 2011 at 1:54 AM, Keith Packard <keithp@keithp.com> wrote:
> From 59b920597999381fab70c485c161dd50590e561a Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp@keithp.com>
> Date: Mon, 25 Jul 2011 22:37:51 -0700
> Subject: [PATCH] Revert and fix "drm/i915/dp: remove DPMS mode tracking from
>  DP"
>
> This reverts commit 885a50147f00a8a80108904bf58a18af357717f3.
>
> We actually *do* need to track DPMS state so that on hotplug, we don't
> retrain the link until DPMS is disabled.
>

>        intel_dp->output_reg = output_reg;
> +       intel_dp->dpms_mode = -1;

Should that be some actual mode constant instead of -1?

In any case, this patch, applied manually on top of the struct_mutex
fix, seems to work.  xset dpms force off turns the display off briefly
(presumably X or GNOME helpfully turns it back on), but if I let the
display turn off on its own, it comes back.

There's still an obnoxious bug, though: I use audio over DP, and when
the display turns off, the attached speaker crackles loudly for a
second or two.  I don't think it's a hardware problem with the
monitor, because if I just pull the plug it doesn't make any noise.
Should the driver do something to drop the audio link before turning
off the DP link as a whole?

--Andy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-25 17:10 [PATCH] drm/i915: Hold struct_mutex during hotplug processing Keith Packard
  2011-07-25 17:37 ` [Intel-gfx] " Jesse Barnes
@ 2011-07-26  7:24 ` Daniel Vetter
  2011-07-26 15:23   ` Keith Packard
  1 sibling, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2011-07-26  7:24 UTC (permalink / raw)
  To: Keith Packard; +Cc: Dave Airlie, intel-gfx, linux-kernel, dri-devel

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
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-26  7:24 ` Daniel Vetter
@ 2011-07-26 15:23   ` Keith Packard
  2011-07-26 19:12     ` Jesse Barnes
  0 siblings, 1 reply; 14+ messages in thread
From: Keith Packard @ 2011-07-26 15:23 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Dave Airlie, intel-gfx, linux-kernel, dri-devel

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

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!

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-26 15:23   ` Keith Packard
@ 2011-07-26 19:12     ` Jesse Barnes
  2011-07-27  9:21       ` Keith Packard
  0 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2011-07-26 19:12 UTC (permalink / raw)
  To: Keith Packard
  Cc: Daniel Vetter, Dave Airlie, intel-gfx, linux-kernel, dri-devel

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.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-26 19:12     ` Jesse Barnes
@ 2011-07-27  9:21       ` Keith Packard
  2011-07-27 16:03         ` Jesse Barnes
  0 siblings, 1 reply; 14+ messages in thread
From: Keith Packard @ 2011-07-27  9:21 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Daniel Vetter, Dave Airlie, intel-gfx, linux-kernel, dri-devel

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

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?

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-27  9:21       ` Keith Packard
@ 2011-07-27 16:03         ` Jesse Barnes
  2011-07-28 22:50           ` Keith Packard
  0 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2011-07-27 16:03 UTC (permalink / raw)
  To: Keith Packard
  Cc: Daniel Vetter, Dave Airlie, intel-gfx, linux-kernel, dri-devel

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.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-27 16:03         ` Jesse Barnes
@ 2011-07-28 22:50           ` Keith Packard
  2011-07-29 10:25             ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Keith Packard @ 2011-07-28 22:50 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Daniel Vetter, Dave Airlie, intel-gfx, linux-kernel, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]

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

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
  2011-07-28 22:50           ` Keith Packard
@ 2011-07-29 10:25             ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2011-07-29 10:25 UTC (permalink / raw)
  To: Keith Packard
  Cc: Jesse Barnes, Daniel Vetter, Dave Airlie, intel-gfx, linux-kernel,
	dri-devel

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
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-07-29 10:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-25 17:10 [PATCH] drm/i915: Hold struct_mutex during hotplug processing Keith Packard
2011-07-25 17:37 ` [Intel-gfx] " Jesse Barnes
2011-07-25 17:40   ` Andrew Lutomirski
2011-07-25 17:55     ` Keith Packard
2011-07-26  2:52   ` Andrew Lutomirski
2011-07-26  5:54     ` Keith Packard
2011-07-26 12:35       ` Andrew Lutomirski
2011-07-26  7:24 ` Daniel Vetter
2011-07-26 15:23   ` Keith Packard
2011-07-26 19:12     ` Jesse Barnes
2011-07-27  9:21       ` Keith Packard
2011-07-27 16:03         ` Jesse Barnes
2011-07-28 22:50           ` Keith Packard
2011-07-29 10:25             ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox