Re: [Intel-gfx] 3.0 (or SNA?) regression: failed to train DP, aborting

2011-07-25 Thread Andrew Lutomirski
On Mon, Jul 25, 2011 at 12:54 AM, Keith Packard  wrote:
> On Sat, 23 Jul 2011 14:40:36 -0400, Andrew Lutomirski  wrote:
>> I have a Q67 (DQ67SW board) attached to a Dell U2711 via DP.  In
>> previous kernels, the DP link has worked flawlessly.  I just booted
>> 3.0-final and simultaneously enabled SNA, and now when my screen goes
>> to sleep I don't get an image back until I power cycle the monitor.
>> dmesg says:
>>
>> [drm:intel_dp_complete_link_train] *ERROR* failed to train DP,
>> aborting
>
> Jesse put together a set of 7 display port patches around July 7 which
> were merged just after 3.0-rc6. If you try 3.0-rc6 and find that it
> works, you should be able to bisect that really quickly; there are only
> 13 patches post-rc6 in drivers/gpu/drm/i915:
>
> $ git bisect start v3.0 v3.0-rc6 -- drivers/gpu/drm/i915

The offending commit appears to be:

commit 885a50147f00a8a80108904bf58a18af357717f3
Author: Jesse Barnes 
Date:   Thu Jul 7 11:11:01 2011 -0700

drm/i915/dp: remove DPMS mode tracking from DP

We currently use this when a hot plug event is received, only checking
the link status and re-training if we had previously configured a link.
However if we want to preserve the DP configuration across both hot plug
and DPMS events (which we do for userspace apps that don't respond to
hot plug uevents), we need to unconditionally check the link and try to
bring it up on hot plug.

Signed-off-by: Jesse Barnes 
Reviewed-by: Keith Packard 
Signed-off-by: Keith Packard 

A debugging patch and its output are attached.

If I had to guess, though, it's a race: a hotplug event happens during
the intel_dp_dpms callback, confusing the code that's trying to train
the link.

--Andy
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] 3.0 (or SNA?) regression: failed to train DP, aborting

2011-07-25 Thread Keith Packard
On Mon, 25 Jul 2011 11:23:17 -0400, Andrew Lutomirski  wrote:

> A debugging patch and its output are attached.

I didn't get any attachment.

> If I had to guess, though, it's a race: a hotplug event happens during
> the intel_dp_dpms callback, confusing the code that's trying to train
> the link.

Interesting possibility. Please re-send the attachments and I'll take a
look.

-- 
keith.pack...@intel.com


pgpcGIfKsGI8Y.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2011-07-25 Thread 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 
---
 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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] 3.0 (or SNA?) regression: failed to train DP, aborting

2011-07-25 Thread Andrew Lutomirski
On Mon, Jul 25, 2011 at 12:08 PM, Keith Packard  wrote:
> On Mon, 25 Jul 2011 11:23:17 -0400, Andrew Lutomirski  wrote:
>
>> A debugging patch and its output are attached.
>
> I didn't get any attachment.
>
>> If I had to guess, though, it's a race: a hotplug event happens during
>> the intel_dp_dpms callback, confusing the code that's trying to train
>> the link.
>
> Interesting possibility. Please re-send the attachments and I'll take a
> look.

Done.

I'm pretty sure the debugging patch is barking up the wrong tree.  If
you like, I can do a different one to instrument intel_dp_dpms and
hotplug later on.

>
> --
> keith.pack...@intel.com
>
[  437.718439] [drm:intel_dp_link_down], 
[  439.250105] [drm:i915_hotplug_work_func], running encoder hotplug functions
[  439.250322] [drm:intel_dp_check_link_status], DPCD was 110A8401
[  439.250536] [drm:intel_dp_check_link_status], DPCD is now 110A8401
[  439.301732] [drm:intel_wait_for_vblank], vblank wait timed out
[  439.303716] [drm:intel_dp_complete_link_train], Training worked. 
DPCD=110A8401
[  439.303942] [drm:intel_ironlake_crt_detect_hotplug], ironlake hotplug 
adpa=0xf4, result 0
[  439.303946] [drm:intel_crt_detect], CRT not detected via hotplug
[  439.303950] [drm:output_poll_execute], [CONNECTOR:5:VGA-1] status updated 
from 2 to 2
[  439.316359] [drm:output_poll_execute], [CONNECTOR:8:HDMI-A-1] status updated 
from 2 to 2
[  439.316363] [drm:ironlake_dp_detect], DPCD was 
[  439.316878] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  439.316882] [drm:ironlake_dp_detect], Try 0: ret=-110 DPCD=
[  439.319216] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  439.319219] [drm:ironlake_dp_detect], Try 1: ret=-110 DPCD=
[  439.321217] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  439.321222] [drm:ironlake_dp_detect], Try 2: ret=-110 DPCD=
[  439.322704] [drm:ironlake_dp_detect], No link. DPCD: 
[  439.322711] [drm:output_poll_execute], [CONNECTOR:11:DP-1] status updated 
from 2 to 2
[  439.335104] [drm:output_poll_execute], [CONNECTOR:14:HDMI-A-2] status 
updated from 2 to 2
[  439.347505] [drm:output_poll_execute], [CONNECTOR:16:HDMI-A-3] status 
updated from 2 to 2
[  439.347509] [drm:ironlake_dp_detect], DPCD was 110A8401
[  439.347724] [drm:ironlake_dp_detect], Try 0: ret=4 DPCD=110A8401
[  439.347730] [drm:ironlake_dp_detect], Happy now!
[  439.347732] [drm:ironlake_dp_detect], No link. DPCD: 110a8401
[  439.348687] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  439.376262] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  439.403831] [drm:i2c_algo_dp_aux_xfer], dp_aux_xfer return 2
[  439.403835] [drm:drm_detect_monitor_audio], Monitor has basic audio support
[  439.403838] [drm:output_poll_execute], [CONNECTOR:17:DP-2] status updated 
from 1 to 1
[  439.403842] [drm:ironlake_dp_detect], DPCD was 
[  439.404357] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  439.404360] [drm:ironlake_dp_detect], Try 0: ret=-110 DPCD=
[  439.406164] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  439.406165] [drm:ironlake_dp_detect], Try 1: ret=-110 DPCD=
[  439.408167] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5143003e
[  439.408169] [drm:ironlake_dp_detect], Try 2: ret=-110 DPCD=
[  439.409663] [drm:ironlake_dp_detect], No link. DPCD: 
[  439.409671] [drm:output_poll_execute], [CONNECTOR:19:DP-3] status updated 
from 2 to 2
[  442.956501] [drm:ironlake_crtc_dpms], crtc 0/0 dpms on
[  443.120115] [drm:intel_dp_link_down], 
[  443.137460] [drm:ironlake_crtc_dpms], crtc 0/0 dpms off
[  443.189440] [drm:intel_wait_for_vblank], vblank wait timed out
[  443.211838] [drm:sandybridge_update_wm], FIFO watermarks For pipe A - plane 
13, cursor: 6
[  443.211845] [drm:ironlake_check_srwm], watermark 1: display plane 25, fbc 
lines 3, cursor 6
[  443.211849] [drm:ironlake_check_srwm], watermark 2: display plane 33, fbc 
lines 3, cursor 6
[  443.211854] [drm:ironlake_check_srwm], watermark 3: display plane 169, fbc 
lines 4, cursor 10
[  443.211858] [drm:intel_update_fbc], 
[  444.644607] [drm:i915_hotplug_work_func], running encoder hotplug functions
[  444.644823] [drm:intel_dp_check_link_status], DPCD was 110A8401
[  444.645037] [drm:intel_dp_check_link_status], DPCD is now 110A8401
[  444.696526] [drm:intel_wait_for_vblank], vblank wait timed out
[  444.751506] [drm:intel_wait_for_vblank], vblank wait timed out
[  444.806485] [drm:intel_wait_for_vblank], vblank wait timed out
[  444.861428] [drm:intel_wait_for_vblank], vblank wait timed out
[  444.916419] [drm:intel_wait_for_vblank], vblank wait timed out
[  444.971376] [drm:intel_wait_for_vblank], vblank wait timed out
[  445.026356] [drm:intel_wait_for_vblank], vblank wait timed out
[  445.028771] [drm:intel_dp_complete_link_train] *ERROR* failed to train DP, 
aborting
[  445.028775] [drm:intel_dp_complete_link_train], DPCD is 110A8401
[  445.028780] [drm

Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing

2011-07-25 Thread Jesse Barnes
On Mon, 25 Jul 2011 10:10:29 -0700
Keith Packard  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 
> ---
>  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 

Let's get this one upstream asap.  Should probably be cc'd to
sta...@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


Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing

2011-07-25 Thread Andrew Lutomirski
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  wrote:
> On Mon, 25 Jul 2011 10:10:29 -0700
> Keith Packard  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 
>> ---
>>  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 
>
> Let's get this one upstream asap.  Should probably be cc'd to
> sta...@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: (a0093026)
a0093027 intel_dp_dpms 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
a002d6e8 drm_mode_connector_property_set_ioctl 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
a002085f drm_ioctl 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
811049a5 do_vfs_ioctl ([kernel.kallsyms])
81104a3c sys_ioctl ([kernel.kallsyms])
814376bb system_call ([kernel.kallsyms])
  30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8895.005199: intel_dp_dpms_return: (a0093026 <- 
a006321b)
81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
a002d6e8 drm_mode_connector_property_set_ioctl 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
a002085f drm_ioctl 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
811049a5 do_vfs_ioctl ([kernel.kallsyms])
81104a3c sys_ioctl ([kernel.kallsyms])
814376bb system_call ([kernel.kallsyms])
  30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8905.765634: intel_dp_dpms: (a0093026)
a0093027 intel_dp_dpms 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
a002d6e8 drm_mode_connector_property_set_ioctl 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
a002085f drm_ioctl 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
811049a5 do_vfs_ioctl ([kernel.kallsyms])
81104a3c sys_ioctl ([kernel.kallsyms])
814376bb system_call ([kernel.kallsyms])
  30260d8957 __GI_ioctl (/lib64/libc-2.14.so)

Xorg  1463/1463  [000]  8905.765650: intel_dp_start_link_train: 
(a009225f)
a0092260 intel_dp_start_link_train 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/i915/i915.ko)
81433108 kretprobe_trampoline_holder ([kernel.kallsyms])
a002d6e8 drm_mode_connector_property_set_ioctl 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
a002085f drm_ioctl 
(/lib/modules/3.0.0-rc6-bpp+/kernel/drivers/gpu/drm/drm.ko)
811049a5 do_vfs_ioctl ([kernel.kallsyms])
81104a3c sys_ioctl ([kernel.kallsyms])
814376bb system_call ([kernel.kallsyms])
  30260d8957 __GI

Re: [Intel-gfx] 3.0 (or SNA?) regression: failed to train DP, aborting

2011-07-25 Thread Keith Packard
On Mon, 25 Jul 2011 13:15:54 -0400, Andrew Lutomirski  wrote:

> Done.

Jesse Barnes and I discovered, much to our horror, that the hotplugging
path isn't holding the struct mutex while attempting to retrain the DP
link. That means that an application could well be doing a bit of
overlapping modesetting fun. And, it seems like the trace you provided
shows precisely that happening.

Care to try this patch? It's fixed my persistent hot-unplug problems.

From bba6ccca57f0536fb5aae278527939d7247a1f53 Mon Sep 17 00:00:00 2001
From: Keith Packard 
Date: Mon, 25 Jul 2011 10:04:56 -0700
Subject: [PATCH] drm/i915: Hold struct_mutex during hotplug processing

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



-- 
keith.pack...@intel.com


pgp9btxUzGoD9.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing

2011-07-25 Thread Keith Packard
On Mon, 25 Jul 2011 13:40:58 -0400, Andrew Lutomirski  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.pack...@intel.com


pgpgagyk9x7eA.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing

2011-07-25 Thread Andrew Lutomirski
On Mon, Jul 25, 2011 at 1:37 PM, Jesse Barnes  wrote:
> On Mon, 25 Jul 2011 10:10:29 -0700
> Keith Packard  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 
>> ---
>>  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
[  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=0xf4, result 0
[  227.296543] [drm:intel_crt_detect], CRT not detected via hotplug
[  227.296547] [drm:output_pol

Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing

2011-07-25 Thread Keith Packard
On Mon, 25 Jul 2011 22:52:15 -0400, Andrew Lutomirski  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 
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 
---
 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.pack...@intel.com


pgptAWUmOjQnv.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 3/5] drm/i915: In intel_dp_init, replace read of DPCD with intel_dp_get_dpcd

2011-07-25 Thread Keith Packard
Eliminates an open-coded read and also gains the retry behaviour of
intel_dp_get_dpcd, which seems like a good idea.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_dp.c |8 +++-
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 41674e1..9f134d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2015,7 +2015,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 
/* Cache some DPCD data in the eDP case */
if (is_edp(intel_dp)) {
-   int ret;
+   bool ret;
u32 pp_on, pp_div;
 
pp_on = I915_READ(PCH_PP_ON_DELAYS);
@@ -2028,11 +2028,9 @@ intel_dp_init(struct drm_device *dev, int output_reg)
dev_priv->panel_t12 *= 100; /* t12 in 100ms units */
 
ironlake_edp_panel_vdd_on(intel_dp);
-   ret = intel_dp_aux_native_read(intel_dp, DP_DPCD_REV,
-  intel_dp->dpcd,
-  sizeof(intel_dp->dpcd));
+   ret = intel_dp_get_dpcd(intel_dp);
ironlake_edp_panel_vdd_off(intel_dp);
-   if (ret == sizeof(intel_dp->dpcd)) {
+   if (ret) {
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
dev_priv->no_aux_handshake =
intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
-- 
1.7.5.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/5] drm/i915: Use dp_detect_common in hotplug helper function

2011-07-25 Thread Keith Packard
This uses the common dpcd reading routine, i915_dp_detect_common,
instead of open-coding a call to intel_dp_aux_native_read. Besides
reducing duplicated code, this also gains the read retries which
may be necessary when a cable is first plugged back in and the link
needs to be retrained.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_dp.c |   39 +++
 1 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dcc7ae6..45db810 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1567,6 +1567,20 @@ intel_dp_link_down(struct intel_dp *intel_dp)
POSTING_READ(intel_dp->output_reg);
 }
 
+static enum drm_connector_status
+i915_dp_detect_common(struct intel_dp *intel_dp)
+{
+   enum drm_connector_status status = connector_status_disconnected;
+
+   if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
+  sizeof (intel_dp->dpcd)) &&
+   (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
+   status = connector_status_connected;
+   }
+
+   return status;
+}
+
 /*
  * According to DP spec
  * 5.1.2:
@@ -1579,45 +1593,30 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 static void
 intel_dp_check_link_status(struct intel_dp *intel_dp)
 {
-   int ret;
-
if (!intel_dp->base.base.crtc)
return;
 
+   /* Try to read receiver status if the link appears to be up */
if (!intel_dp_get_link_status(intel_dp)) {
intel_dp_link_down(intel_dp);
return;
}
 
-   /* Try to read receiver status if the link appears to be up */
-   ret = intel_dp_aux_native_read(intel_dp,
-  0x000, intel_dp->dpcd,
-  sizeof (intel_dp->dpcd));
-   if (ret != sizeof(intel_dp->dpcd)) {
+   /* Now read the DPCD to see if it's actually running */
+   if (i915_dp_detect_common(intel_dp) != connector_status_connected) {
intel_dp_link_down(intel_dp);
return;
}
 
if (!intel_channel_eq_ok(intel_dp)) {
+   DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
+ drm_get_encoder_name(&intel_dp->base.base));
intel_dp_start_link_train(intel_dp);
intel_dp_complete_link_train(intel_dp);
}
 }
 
 static enum drm_connector_status
-i915_dp_detect_common(struct intel_dp *intel_dp)
-{
-   enum drm_connector_status status = connector_status_disconnected;
-
-   if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-  sizeof (intel_dp->dpcd)) &&
-   (intel_dp->dpcd[DP_DPCD_REV] != 0))
-   status = connector_status_connected;
-
-   return status;
-}
-
-static enum drm_connector_status
 ironlake_dp_detect(struct intel_dp *intel_dp)
 {
enum drm_connector_status status;
-- 
1.7.5.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/5] drm/i915: Rename i915_dp_detect_common to intel_dp_get_dpcd

2011-07-25 Thread Keith Packard
This describes the function better, allowing it to be used where the
DPCD value is relevant.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_dp.c |   24 +++-
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 45db810..41674e1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1567,18 +1567,16 @@ intel_dp_link_down(struct intel_dp *intel_dp)
POSTING_READ(intel_dp->output_reg);
 }
 
-static enum drm_connector_status
-i915_dp_detect_common(struct intel_dp *intel_dp)
+static bool
+intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
-   enum drm_connector_status status = connector_status_disconnected;
-
if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
   sizeof (intel_dp->dpcd)) &&
(intel_dp->dpcd[DP_DPCD_REV] != 0)) {
-   status = connector_status_connected;
+   return true;
}
 
-   return status;
+   return false;
 }
 
 /*
@@ -1603,7 +1601,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
}
 
/* Now read the DPCD to see if it's actually running */
-   if (i915_dp_detect_common(intel_dp) != connector_status_connected) {
+   if (!intel_dp_get_dpcd(intel_dp)) {
intel_dp_link_down(intel_dp);
return;
}
@@ -1617,6 +1615,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 }
 
 static enum drm_connector_status
+intel_dp_detect_dpcd(struct intel_dp *intel_dp)
+{
+   if (intel_dp_get_dpcd(intel_dp))
+   return connector_status_connected;
+   return connector_status_disconnected;
+}
+
+static enum drm_connector_status
 ironlake_dp_detect(struct intel_dp *intel_dp)
 {
enum drm_connector_status status;
@@ -1629,7 +1635,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
return status;
}
 
-   return i915_dp_detect_common(intel_dp);
+   return intel_dp_detect_dpcd(intel_dp);
 }
 
 static enum drm_connector_status
@@ -1658,7 +1664,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
if ((temp & bit) == 0)
return connector_status_disconnected;
 
-   return i915_dp_detect_common(intel_dp);
+   return intel_dp_detect_dpcd(intel_dp);
 }
 
 /**
-- 
1.7.5.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 5/5] drm/i915: DP_PIPE_ENABLED must check transcoder on CPT

2011-07-25 Thread Keith Packard
Display port pipe selection on CPT is not done with a bit in the
output register, rather it is controlled by a couple of bits in the
separate transcoder register which indicate which display port output
is connected to the transcoder.

This patch replaces the simplistic macro DP_PIPE_ENABLED with the
rather more complicated function dp_pipe_enabled which checks the
output register to see if that is enabled, and then goes on to either
check the output register pipe selection bit (on non-CPT) or the
transcoder DP selection bits (on CPT).

Before this patch, any time the mode of pipe A was changed, any
display port outputs on pipe B would get disabled as
intel_disable_pch_ports would ensure that the mode setting operation
could occur on pipe A without interference from other outputs
connected to that pch port

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_reg.h  |3 --
 drivers/gpu/drm/i915/intel_display.c |   45 +
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5d5def7..f731565 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2083,9 +2083,6 @@
 #define   DP_PIPEB_SELECT  (1 << 30)
 #define   DP_PIPE_MASK (1 << 30)
 
-#define DP_PIPE_ENABLED(V, P) \
-   (((V) & (DP_PIPE_MASK | DP_PORT_EN)) == ((P) << 30 | DP_PORT_EN))
-
 /* Link training mode - select a suitable mode for each stage */
 #define   DP_LINK_TRAIN_PAT_1  (0 << 28)
 #define   DP_LINK_TRAIN_PAT_2  (1 << 28)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5609c06..fc26cb4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -979,11 +979,29 @@ static void assert_transcoder_disabled(struct 
drm_i915_private *dev_priv,
 pipe_name(pipe));
 }
 
+static bool dp_pipe_enabled(struct drm_i915_private *dev_priv, enum pipe pipe,
+   int reg, u32 port_sel, u32 val)
+{
+   if ((val & DP_PORT_EN) == 0)
+   return false;
+
+   if (HAS_PCH_CPT(dev_priv->dev)) {
+   u32 trans_dp_ctl_reg = TRANS_DP_CTL(pipe);
+   u32 trans_dp_ctl = I915_READ(trans_dp_ctl_reg);
+   if ((trans_dp_ctl & TRANS_DP_PORT_SEL_MASK) != port_sel)
+   return false;
+   } else {
+   if ((val & DP_PIPE_MASK) != (pipe << 30))
+   return false;
+   }
+   return true;
+}
+
 static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
-  enum pipe pipe, int reg)
+  enum pipe pipe, int reg, u32 port_sel)
 {
u32 val = I915_READ(reg);
-   WARN(DP_PIPE_ENABLED(val, pipe),
+   WARN(dp_pipe_enabled(dev_priv, pipe, reg, port_sel, val),
 "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n",
 reg, pipe_name(pipe));
 }
@@ -1003,9 +1021,9 @@ static void assert_pch_ports_disabled(struct 
drm_i915_private *dev_priv,
int reg;
u32 val;
 
-   assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B);
-   assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C);
-   assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D);
+   assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
+   assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
+   assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
 
reg = PCH_ADPA;
val = I915_READ(reg);
@@ -1334,19 +1352,24 @@ static void intel_disable_plane(struct drm_i915_private 
*dev_priv,
 }
 
 static void disable_pch_dp(struct drm_i915_private *dev_priv,
-  enum pipe pipe, int reg)
+  enum pipe pipe, int reg, u32 port_sel)
 {
u32 val = I915_READ(reg);
-   if (DP_PIPE_ENABLED(val, pipe))
+   if (dp_pipe_enabled(dev_priv, pipe, reg, port_sel, val)) {
+   DRM_DEBUG_KMS("Disabling pch dp %x on pipe %d\n", reg, pipe);
I915_WRITE(reg, val & ~DP_PORT_EN);
+   }
 }
 
 static void disable_pch_hdmi(struct drm_i915_private *dev_priv,
 enum pipe pipe, int reg)
 {
u32 val = I915_READ(reg);
-   if (HDMI_PIPE_ENABLED(val, pipe))
+   if (HDMI_PIPE_ENABLED(val, pipe)) {
+   DRM_DEBUG_KMS("Disabling pch HDMI %x on pipe %d\n",
+ reg, pipe);
I915_WRITE(reg, val & ~PORT_ENABLE);
+   }
 }
 
 /* Disable any ports connected to this transcoder */
@@ -1358,9 +1381,9 @@ static void intel_disable_pch_ports(struct 
drm_i915_private *dev_priv,
val = I915_READ(PCH_PP_CONTROL);
I915_WRITE(PCH_PP_CONTROL, val | PANEL_UNLOCK_REGS);
 
-   disable_pch_dp(dev_priv, pipe, PCH_DP_B);
-   disable_pch_dp(dev_priv, pipe, PCH_

[Intel-gfx] [PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code

2011-07-25 Thread Keith Packard
If the connector is inserted or removed slowly, the hotplug line may
well change state before the data lines do. So, assume the user isn't
trying to fool us and give them 250ms to get the connector plugged or
unplugged.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_irq.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9da2a2c..e3ce1c3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -306,6 +306,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
struct drm_mode_config *mode_config = &dev->mode_config;
struct intel_encoder *encoder;
 
+   /* Wait a bit so that the connector change can be completed */
+   msleep(250);
mutex_lock(&mode_config->mutex);
DRM_DEBUG_KMS("running encoder hotplug functions\n");
 
-- 
1.7.5.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] drm/i915: A selection of display port fixes

2011-07-25 Thread Keith Packard
 [PATCH 1/5] drm/i915: Use dp_detect_common in hotplug helper
 [PATCH 2/5] drm/i915: Rename i915_dp_detect_common to
 [PATCH 3/5] drm/i915: In intel_dp_init, replace read of DPCD with

These three are simple cleanups to centralize all places where the
DPCD block was read from the device. Now everyone shares the same
function, and that function retries the reads.

 [PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code

I was experimenting with DP hotplugging -- moving the plug in and out
of the jack very slowly and discovered that the hotplug interrupt
occurred well before or after the link for the aux data channel was
connected or disconnected. The result of this was that a sufficiently
rapid cycle back through user mode could easily beat the motion of the
plug and cause the hotplug detection to get the wrong status. Sticking
a 250ms delay before doing anything gives the user sufficient time to
actually get the plug connected or disconnected.

 [PATCH 5/5] drm/i915: DP_PIPE_ENABLED must check transcoder on CPT

This is a fairly nice bug fix to finally have. The symptom I saw was
that going from one two-head configuration to another would sometimes
turn off the monitor which was *not* being modified. For instance, I
would do:

 $ xrandr --output LVDS1 --below DP2

This would always turn off the DP2 monitor, and sometimes it wouldn't
turn back on.

Turns out the bug wasn't that the mode setting code was doing it wrong
and turning the DP2 output off intentionally as a part of the mode
change. Instead, the intel driver was trying to adjust the PCH link
for the LVDS1 output and thought it needed to turn the DP2 output off
because it mistakenly believed the DP2 output was sharing the same
pipe as the LVDS1 output. Just a matter of using the wrong mechanism
to detect which pipe the DP2 output was connected to.

In any case, review and testing appreciated.

-keith
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx