Re: [Intel-gfx] 3.0 (or SNA?) regression: failed to train DP, aborting
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
[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