* Re: Re: drm/i915: new warning (regression) in 3.7.10 and 3.8.3
@ 2013-04-09 11:54 Tomas Melin
2013-04-09 12:50 ` Daniel Vetter
0 siblings, 1 reply; 19+ messages in thread
From: Tomas Melin @ 2013-04-09 11:54 UTC (permalink / raw)
To: daniel.vetter; +Cc: richardcochran, linux-kernel, dri-devel, David Airlie
On Mon, 18 Mar 2013 09:32:51 +0100, Daniel Vetter wrote:
>
> On Sat, Mar 16, 2013 at 01:28:50PM +0100, Richard Cochran wrote:
>>
>> I have an Acer Aspire One netbook, and on it I get the following
>> warning when closing and opening the lid. I think this warning first
>> appeared in 3.7.
>>
>> Does this need fixing? If so, who can do it?
>
> Another pesky BIOS which changes the display state behind our back on lid
> closing! Should be duct-tapped over with
>
> commit 45e2b5f640b3766da3eda48f6c35f088155c06f3
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Fri Nov 23 18:16:34 2012 +0100
>
> drm/i915: force restore on lid open
>
> which is in 3.8.
I'm seeing the same problems on my eeepc using 3.8. Happens (most) of
the time when opening and closing the lid. Any ideas on how to fix
this?
-Tomas
[20314.679749] [drm:i9xx_crtc_mode_set] *ERROR* Couldn't find PLL
settings for mode!
[20314.679787] ------------[ cut here ]------------
[20314.679819] WARNING: at drivers/gpu/drm/i915/intel_display.c:7864
intel_modeset_check_state+0x4ad/0x620()
[20314.679831] Hardware name: 901
[20314.679843] encoder's hw state doesn't match sw tracking (expected
1, found 0)
[20314.679853] Modules linked in: bridge ipv6 rfcomm fuse btusb
rt2800pci rt2800lib rt2x00pci rt2x00lib atl1e
[20314.679917] Pid: 10069, comm: kworker/0:0 Tainted: G W 3.8.0 #8
[20314.679930] Call Trace:
[20314.679958] [<c1033d6d>] warn_slowpath_common+0x6d/0xa0
[20314.679983] [<c136645d>] ? intel_modeset_check_state+0x4ad/0x620
[20314.680006] [<c136645d>] ? intel_modeset_check_state+0x4ad/0x620
[20314.680017] [<c1033e1e>] warn_slowpath_fmt+0x2e/0x30
[20314.680017] [<c136645d>] intel_modeset_check_state+0x4ad/0x620
[20314.680103] [<c1366d31>] intel_set_mode+0x701/0x990
[20314.680170] [<c13695c5>] intel_modeset_setup_hw_state+0x5d5/0x8e0
[20314.680215] [<c12e3798>] ? acpi_lid_open+0x28/0x40
[20314.680236] [<c136b6db>] intel_lid_notify+0x9b/0xc0
[20314.680258] [<c1731ad5>] notifier_call_chain+0x45/0x60
[20314.680283] [<c1057f4e>] __blocking_notifier_call_chain+0x3e/0x60
[20314.680306] [<c1057f8a>] blocking_notifier_call_chain+0x1a/0x20
[20314.680329] [<c12e3839>] acpi_lid_send_state+0x78/0x9d
[20314.680351] [<c12e38aa>] acpi_button_notify+0x30/0xa4
[20314.680376] [<c12c435e>] acpi_device_notify+0x12/0x15
[20314.680402] [<c12cf786>] acpi_ev_notify_dispatch+0x2e/0x45
[20314.680424] [<c12c14cb>] acpi_os_execute_deferred+0x1b/0x26
[20314.680449] [<c104c800>] process_one_work+0x110/0x380
[20314.680475] [<c172ede9>] ? apic_timer_interrupt+0x2d/0x34
[20314.680498] [<c12c14b0>] ? acpi_os_wait_events_complete+0x19/0x19
[20314.680522] [<c104df89>] worker_thread+0x119/0x340
[20314.680545] [<c104de70>] ? manage_workers+0x260/0x260
[20314.680572] [<c10522af>] kthread+0x8f/0xa0
[20314.680598] [<c1734fb7>] ret_from_kernel_thread+0x1b/0x28
[20314.680622] [<c1052220>] ? flush_kthread_work+0xc0/0xc0
[20314.680640] ---[ end trace 037b36714b28cbdf ]---
[20314.680661] ------------[ cut here ]------------
[20314.680684] WARNING: at drivers/gpu/drm/i915/intel_display.c:7898
intel_modeset_check_state+0x47b/0x620()
[20314.680697] Hardware name: 901
[20314.680709] crtc's computed active state doesn't match tracked
active state (expected 1, found 0)
[20314.680719] Modules linked in: bridge ipv6 rfcomm fuse btusb
rt2800pci rt2800lib rt2x00pci rt2x00lib atl1e
[20314.680898] Pid: 10069, comm: kworker/0:0 Tainted: G W 3.8.0 #8
[20314.680910] Call Trace:
[20314.680934] [<c1033d6d>] warn_slowpath_common+0x6d/0xa0
[20314.680960] [<c136642b>] ? intel_modeset_check_state+0x47b/0x620
[20314.680983] [<c136642b>] ? intel_modeset_check_state+0x47b/0x620
[20314.681006] [<c1033e1e>] warn_slowpath_fmt+0x2e/0x30
[20314.681041] [<c136642b>] intel_modeset_check_state+0x47b/0x620
[20314.681052] [<c1366d31>] intel_set_mode+0x701/0x990
[20314.681095] [<c13695c5>] intel_modeset_setup_hw_state+0x5d5/0x8e0
[20314.681122] [<c12e3798>] ? acpi_lid_open+0x28/0x40
[20314.681147] [<c136b6db>] intel_lid_notify+0x9b/0xc0
[20314.681170] [<c1731ad5>] notifier_call_chain+0x45/0x60
[20314.681196] [<c1057f4e>] __blocking_notifier_call_chain+0x3e/0x60
[20314.681221] [<c1057f8a>] blocking_notifier_call_chain+0x1a/0x20
[20314.681246] [<c12e3839>] acpi_lid_send_state+0x78/0x9d
[20314.681270] [<c12e38aa>] acpi_button_notify+0x30/0xa4
[20314.681295] [<c12c435e>] acpi_device_notify+0x12/0x15
[20314.681320] [<c12cf786>] acpi_ev_notify_dispatch+0x2e/0x45
[20314.681344] [<c12c14cb>] acpi_os_execute_deferred+0x1b/0x26
[20314.681366] [<c104c800>] process_one_work+0x110/0x380
[20314.681392] [<c172ede9>] ? apic_timer_interrupt+0x2d/0x34
[20314.681416] [<c12c14b0>] ? acpi_os_wait_events_complete+0x19/0x19
[20314.681465] [<c104df89>] worker_thread+0x119/0x340
[20314.681516] [<c104de70>] ? manage_workers+0x260/0x260
[20314.681568] [<c10522af>] kthread+0x8f/0xa0
[20314.681614] [<c1734fb7>] ret_from_kernel_thread+0x1b/0x28
[20314.681660] [<c1052220>] ? flush_kthread_work+0xc0/0xc0
[20314.681681] ---[ end trace 037b36714b28cbe0 ]---
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Re: drm/i915: new warning (regression) in 3.7.10 and 3.8.3 2013-04-09 11:54 Re: drm/i915: new warning (regression) in 3.7.10 and 3.8.3 Tomas Melin @ 2013-04-09 12:50 ` Daniel Vetter 2013-04-09 13:21 ` Richard Cochran 0 siblings, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2013-04-09 12:50 UTC (permalink / raw) To: Tomas Melin Cc: daniel.vetter, richardcochran, linux-kernel, dri-devel, David Airlie On Tue, Apr 09, 2013 at 02:54:34PM +0300, Tomas Melin wrote: > On Mon, 18 Mar 2013 09:32:51 +0100, Daniel Vetter wrote: > > > > On Sat, Mar 16, 2013 at 01:28:50PM +0100, Richard Cochran wrote: > >> > >> I have an Acer Aspire One netbook, and on it I get the following > >> warning when closing and opening the lid. I think this warning first > >> appeared in 3.7. > >> > >> Does this need fixing? If so, who can do it? > > > > Another pesky BIOS which changes the display state behind our back on lid > > closing! Should be duct-tapped over with > > > > commit 45e2b5f640b3766da3eda48f6c35f088155c06f3 > > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > > Date: Fri Nov 23 18:16:34 2012 +0100 > > > > drm/i915: force restore on lid open > > > > which is in 3.8. > > I'm seeing the same problems on my eeepc using 3.8. Happens (most) of > the time when opening and closing the lid. Any ideas on how to fix > this? This should be fixed with the above mentioned patch. The issue is that the bios fumbles around with the output configuration behind our backs, so the new paranoid modeset code in 3.7+ freaks out about the state mismatch between sw and hw. The patch above should detect this situation and undo any bios-induced damage. -Daniel > > -Tomas > > [20314.679749] [drm:i9xx_crtc_mode_set] *ERROR* Couldn't find PLL > settings for mode! > > [20314.679787] ------------[ cut here ]------------ > [20314.679819] WARNING: at drivers/gpu/drm/i915/intel_display.c:7864 > intel_modeset_check_state+0x4ad/0x620() > [20314.679831] Hardware name: 901 > [20314.679843] encoder's hw state doesn't match sw tracking (expected > 1, found 0) > [20314.679853] Modules linked in: bridge ipv6 rfcomm fuse btusb > rt2800pci rt2800lib rt2x00pci rt2x00lib atl1e > [20314.679917] Pid: 10069, comm: kworker/0:0 Tainted: G W 3.8.0 #8 > [20314.679930] Call Trace: > [20314.679958] [<c1033d6d>] warn_slowpath_common+0x6d/0xa0 > [20314.679983] [<c136645d>] ? intel_modeset_check_state+0x4ad/0x620 > [20314.680006] [<c136645d>] ? intel_modeset_check_state+0x4ad/0x620 > [20314.680017] [<c1033e1e>] warn_slowpath_fmt+0x2e/0x30 > [20314.680017] [<c136645d>] intel_modeset_check_state+0x4ad/0x620 > [20314.680103] [<c1366d31>] intel_set_mode+0x701/0x990 > [20314.680170] [<c13695c5>] intel_modeset_setup_hw_state+0x5d5/0x8e0 > [20314.680215] [<c12e3798>] ? acpi_lid_open+0x28/0x40 > [20314.680236] [<c136b6db>] intel_lid_notify+0x9b/0xc0 > [20314.680258] [<c1731ad5>] notifier_call_chain+0x45/0x60 > [20314.680283] [<c1057f4e>] __blocking_notifier_call_chain+0x3e/0x60 > [20314.680306] [<c1057f8a>] blocking_notifier_call_chain+0x1a/0x20 > [20314.680329] [<c12e3839>] acpi_lid_send_state+0x78/0x9d > [20314.680351] [<c12e38aa>] acpi_button_notify+0x30/0xa4 > [20314.680376] [<c12c435e>] acpi_device_notify+0x12/0x15 > [20314.680402] [<c12cf786>] acpi_ev_notify_dispatch+0x2e/0x45 > [20314.680424] [<c12c14cb>] acpi_os_execute_deferred+0x1b/0x26 > [20314.680449] [<c104c800>] process_one_work+0x110/0x380 > [20314.680475] [<c172ede9>] ? apic_timer_interrupt+0x2d/0x34 > [20314.680498] [<c12c14b0>] ? acpi_os_wait_events_complete+0x19/0x19 > [20314.680522] [<c104df89>] worker_thread+0x119/0x340 > [20314.680545] [<c104de70>] ? manage_workers+0x260/0x260 > [20314.680572] [<c10522af>] kthread+0x8f/0xa0 > [20314.680598] [<c1734fb7>] ret_from_kernel_thread+0x1b/0x28 > [20314.680622] [<c1052220>] ? flush_kthread_work+0xc0/0xc0 > [20314.680640] ---[ end trace 037b36714b28cbdf ]--- > [20314.680661] ------------[ cut here ]------------ > [20314.680684] WARNING: at drivers/gpu/drm/i915/intel_display.c:7898 > intel_modeset_check_state+0x47b/0x620() > [20314.680697] Hardware name: 901 > [20314.680709] crtc's computed active state doesn't match tracked > active state (expected 1, found 0) > [20314.680719] Modules linked in: bridge ipv6 rfcomm fuse btusb > rt2800pci rt2800lib rt2x00pci rt2x00lib atl1e > [20314.680898] Pid: 10069, comm: kworker/0:0 Tainted: G W 3.8.0 #8 > [20314.680910] Call Trace: > [20314.680934] [<c1033d6d>] warn_slowpath_common+0x6d/0xa0 > [20314.680960] [<c136642b>] ? intel_modeset_check_state+0x47b/0x620 > [20314.680983] [<c136642b>] ? intel_modeset_check_state+0x47b/0x620 > [20314.681006] [<c1033e1e>] warn_slowpath_fmt+0x2e/0x30 > [20314.681041] [<c136642b>] intel_modeset_check_state+0x47b/0x620 > [20314.681052] [<c1366d31>] intel_set_mode+0x701/0x990 > [20314.681095] [<c13695c5>] intel_modeset_setup_hw_state+0x5d5/0x8e0 > [20314.681122] [<c12e3798>] ? acpi_lid_open+0x28/0x40 > [20314.681147] [<c136b6db>] intel_lid_notify+0x9b/0xc0 > [20314.681170] [<c1731ad5>] notifier_call_chain+0x45/0x60 > [20314.681196] [<c1057f4e>] __blocking_notifier_call_chain+0x3e/0x60 > [20314.681221] [<c1057f8a>] blocking_notifier_call_chain+0x1a/0x20 > [20314.681246] [<c12e3839>] acpi_lid_send_state+0x78/0x9d > [20314.681270] [<c12e38aa>] acpi_button_notify+0x30/0xa4 > [20314.681295] [<c12c435e>] acpi_device_notify+0x12/0x15 > [20314.681320] [<c12cf786>] acpi_ev_notify_dispatch+0x2e/0x45 > [20314.681344] [<c12c14cb>] acpi_os_execute_deferred+0x1b/0x26 > [20314.681366] [<c104c800>] process_one_work+0x110/0x380 > [20314.681392] [<c172ede9>] ? apic_timer_interrupt+0x2d/0x34 > [20314.681416] [<c12c14b0>] ? acpi_os_wait_events_complete+0x19/0x19 > [20314.681465] [<c104df89>] worker_thread+0x119/0x340 > [20314.681516] [<c104de70>] ? manage_workers+0x260/0x260 > [20314.681568] [<c10522af>] kthread+0x8f/0xa0 > [20314.681614] [<c1734fb7>] ret_from_kernel_thread+0x1b/0x28 > [20314.681660] [<c1052220>] ? flush_kthread_work+0xc0/0xc0 > [20314.681681] ---[ end trace 037b36714b28cbe0 ]--- -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: drm/i915: new warning (regression) in 3.7.10 and 3.8.3 2013-04-09 12:50 ` Daniel Vetter @ 2013-04-09 13:21 ` Richard Cochran 2013-04-09 17:28 ` Daniel Vetter 0 siblings, 1 reply; 19+ messages in thread From: Richard Cochran @ 2013-04-09 13:21 UTC (permalink / raw) To: Tomas Melin, linux-kernel, dri-devel, David Airlie On Tue, Apr 09, 2013 at 02:50:03PM +0200, Daniel Vetter wrote: > > This should be fixed with the above mentioned patch. The issue is that the > bios fumbles around with the output configuration behind our backs, so the > new paranoid modeset code in 3.7+ freaks out about the state mismatch > between sw and hw. > > The patch above should detect this situation and undo any bios-induced > damage. Even with the patch, I still am seeing the issue (in 3.8). Thanks, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: drm/i915: new warning (regression) in 3.7.10 and 3.8.3 2013-04-09 13:21 ` Richard Cochran @ 2013-04-09 17:28 ` Daniel Vetter 2013-04-09 19:51 ` [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring Daniel Vetter 0 siblings, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2013-04-09 17:28 UTC (permalink / raw) To: Richard Cochran; +Cc: Tomas Melin, linux-kernel, dri-devel, David Airlie On Tue, Apr 09, 2013 at 03:21:35PM +0200, Richard Cochran wrote: > On Tue, Apr 09, 2013 at 02:50:03PM +0200, Daniel Vetter wrote: > > > > This should be fixed with the above mentioned patch. The issue is that the > > bios fumbles around with the output configuration behind our backs, so the > > new paranoid modeset code in 3.7+ freaks out about the state mismatch > > between sw and hw. > > > > The patch above should detect this situation and undo any bios-induced > > damage. > > Even with the patch, I still am seeing the issue (in 3.8). Indeed, there's still a bug there with the state checking. Can you please test the below patch? Thanks, Daniel commit d206df2685801a832a728c7dca13428a6f5bf3ef Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Tue Apr 9 19:25:12 2013 +0200 drm/i915: don't check inconstent modeset state when force-restoring It will be only consistent once we've restored all the crtcs. Since a bunch of other callers also want to just restore a single crtc, add a boolean to disable checking only where it doesn't make sense. Note that intel_modeset_setup_hw_state already has a call to intel_modeset_check_state at the end, so we don't reduce the amount of checking. References: https://lkml.org/lkml/2013/3/16/60 Cc: Tomas Melin <tomas.melin@iki.fi> Cc: Richard Cochran <richardcochran@gmail.com> Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8809813..1e29201 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7916,9 +7916,9 @@ intel_modeset_check_state(struct drm_device *dev) } } -int intel_set_mode(struct drm_crtc *crtc, - struct drm_display_mode *mode, - int x, int y, struct drm_framebuffer *fb) +static int __intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; drm_i915_private_t *dev_priv = dev->dev_private; @@ -8012,8 +8012,6 @@ done: if (ret && crtc->enabled) { crtc->hwmode = *saved_hwmode; crtc->mode = *saved_mode; - } else { - intel_modeset_check_state(dev); } out: @@ -8022,9 +8020,27 @@ out: return ret; } -void intel_crtc_restore_mode(struct drm_crtc *crtc) +int intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) +{ + int ret; + + ret = __intel_set_mode(crtc, mode, x, y, fb); + + if (ret == 0) + intel_modeset_check_state(crtc->dev); + + return ret; +} + +void intel_crtc_restore_mode(struct drm_crtc *crtc, + bool check) { - intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); + __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); + + if (check) + intel_modeset_check_state(crtc->dev); } #undef for_each_intel_crtc_masked @@ -9336,7 +9352,7 @@ setup_pipes: for_each_pipe(pipe) { struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; - intel_crtc_restore_mode(crtc); + intel_crtc_restore_mode(crtc, false); } list_for_each_entry(plane, &dev->mode_config.plane_list, head) intel_plane_restore(plane); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 173add1..99b9f09 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2466,7 +2466,7 @@ intel_dp_set_property(struct drm_connector *connector, done: if (intel_encoder->base.crtc) - intel_crtc_restore_mode(intel_encoder->base.crtc); + intel_crtc_restore_mode(intel_encoder->base.crtc, true); return 0; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d7bd031..d7b1094 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -540,7 +540,7 @@ struct intel_set_config { extern int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, int x, int y, struct drm_framebuffer *old_fb); extern void intel_modeset_disable(struct drm_device *dev); -extern void intel_crtc_restore_mode(struct drm_crtc *crtc); +extern void intel_crtc_restore_mode(struct drm_crtc *crtc, bool check); extern void intel_crtc_load_lut(struct drm_crtc *crtc); extern void intel_crtc_update_dpms(struct drm_crtc *crtc); extern void intel_encoder_destroy(struct drm_encoder *encoder); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index ee4a8da..de07bd4 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -942,7 +942,7 @@ intel_hdmi_set_property(struct drm_connector *connector, done: if (intel_dig_port->base.base.crtc) - intel_crtc_restore_mode(intel_dig_port->base.base.crtc); + intel_crtc_restore_mode(intel_dig_port->base.base.crtc, true); return 0; } diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index ca2d903..4e80e76 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -666,7 +666,7 @@ static int intel_lvds_set_property(struct drm_connector *connector, * If the CRTC is enabled, the display will be changed * according to the new panel fitting mode. */ - intel_crtc_restore_mode(crtc); + intel_crtc_restore_mode(crtc, true); } } diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index f6a9f4a..777d0d4 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2044,7 +2044,7 @@ set_value: done: if (intel_sdvo->base.base.crtc) - intel_crtc_restore_mode(intel_sdvo->base.base.crtc); + intel_crtc_restore_mode(intel_sdvo->base.base.crtc, true); return 0; #undef CHECK_PROPERTY diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 6673726..bbe2925 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -1481,7 +1481,7 @@ intel_tv_set_property(struct drm_connector *connector, struct drm_property *prop } if (changed && crtc) - intel_crtc_restore_mode(crtc); + intel_crtc_restore_mode(crtc, true); out: return ret; } -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-09 17:28 ` Daniel Vetter @ 2013-04-09 19:51 ` Daniel Vetter 2013-04-10 11:59 ` Richard Cochran 2013-04-11 18:22 ` Daniel Vetter 0 siblings, 2 replies; 19+ messages in thread From: Daniel Vetter @ 2013-04-09 19:51 UTC (permalink / raw) To: Intel Graphics Development Cc: DRI Development, LKML, Dave Airlie, Daniel Vetter, Tomas Melin, Richard Cochran, Chris Wilson, stable It will be only consistent once we've restored all the crtcs. Since a bunch of other callers also want to just restore a single crtc, add a boolean to disable checking only where it doesn't make sense. Note that intel_modeset_setup_hw_state already has a call to intel_modeset_check_state at the end, so we don't reduce the amount of checking. v2: Try harder not to create a big patch (Chris). References: https://lkml.org/lkml/2013/3/16/60 Cc: Tomas Melin <tomas.melin@iki.fi> Cc: Richard Cochran <richardcochran@gmail.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8809813..2959fb8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7916,9 +7916,9 @@ intel_modeset_check_state(struct drm_device *dev) } } -int intel_set_mode(struct drm_crtc *crtc, - struct drm_display_mode *mode, - int x, int y, struct drm_framebuffer *fb) +static int __intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; drm_i915_private_t *dev_priv = dev->dev_private; @@ -8012,8 +8012,6 @@ done: if (ret && crtc->enabled) { crtc->hwmode = *saved_hwmode; crtc->mode = *saved_mode; - } else { - intel_modeset_check_state(dev); } out: @@ -8022,9 +8020,25 @@ out: return ret; } +int intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) +{ + int ret; + + ret = __intel_set_mode(crtc, mode, x, y, fb); + + if (ret == 0) + intel_modeset_check_state(crtc->dev); + + return ret; +} + void intel_crtc_restore_mode(struct drm_crtc *crtc) { - intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); + __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); + + intel_modeset_check_state(crtc->dev); } #undef for_each_intel_crtc_masked @@ -9333,10 +9347,16 @@ setup_pipes: } if (force_restore) { + /* + * We need to use raw interfaces for restoring state to avoid + * checking (bogus) intermediate states. + */ for_each_pipe(pipe) { struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; - intel_crtc_restore_mode(crtc); + + __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, + crtc->fb); } list_for_each_entry(plane, &dev->mode_config.plane_list, head) intel_plane_restore(plane); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-09 19:51 ` [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring Daniel Vetter @ 2013-04-10 11:59 ` Richard Cochran 2013-04-10 12:07 ` Daniel Vetter 2013-04-11 18:22 ` Daniel Vetter 1 sibling, 1 reply; 19+ messages in thread From: Richard Cochran @ 2013-04-10 11:59 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Chris Wilson, stable On Tue, Apr 09, 2013 at 09:51:30PM +0200, Daniel Vetter wrote: > It will be only consistent once we've restored all the crtcs. Since a > bunch of other callers also want to just restore a single crtc, add a > boolean to disable checking only where it doesn't make sense. > > Note that intel_modeset_setup_hw_state already has a call to > intel_modeset_check_state at the end, so we don't reduce the amount of > checking. > > v2: Try harder not to create a big patch (Chris). To what does tree does this patch apply? Tried v3.8.6 and master d02a9a89. Thanks, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-10 11:59 ` Richard Cochran @ 2013-04-10 12:07 ` Daniel Vetter 2013-04-10 17:27 ` Richard Cochran 0 siblings, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2013-04-10 12:07 UTC (permalink / raw) To: Richard Cochran Cc: Daniel Vetter, Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Chris Wilson, stable On Wed, Apr 10, 2013 at 01:59:02PM +0200, Richard Cochran wrote: > On Tue, Apr 09, 2013 at 09:51:30PM +0200, Daniel Vetter wrote: > > It will be only consistent once we've restored all the crtcs. Since a > > bunch of other callers also want to just restore a single crtc, add a > > boolean to disable checking only where it doesn't make sense. > > > > Note that intel_modeset_setup_hw_state already has a call to > > intel_modeset_check_state at the end, so we don't reduce the amount of > > checking. > > > > v2: Try harder not to create a big patch (Chris). > > To what does tree does this patch apply? > > Tried v3.8.6 and master d02a9a89. It's written against drm-intel-next-queued at http://cgit.freedesktop.org/~danvet/drm-intel I've thought that it should apply pretty cleanly against older kernels, too. Apparently it conflicts a bit in intel_modeset_setup_hw_state, you can just do the s/intel_crtc_restore_mode/__intel_set_mode/ change manually. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-10 12:07 ` Daniel Vetter @ 2013-04-10 17:27 ` Richard Cochran 2013-04-10 18:32 ` Tomas Melin 2013-04-10 20:03 ` [Intel-gfx] " Daniel Vetter 0 siblings, 2 replies; 19+ messages in thread From: Richard Cochran @ 2013-04-10 17:27 UTC (permalink / raw) To: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Chris Wilson, stable On Wed, Apr 10, 2013 at 02:07:24PM +0200, Daniel Vetter wrote: > > It's written against drm-intel-next-queued at > > http://cgit.freedesktop.org/~danvet/drm-intel > > I've thought that it should apply pretty cleanly against older kernels, > too. Apparently it conflicts a bit in intel_modeset_setup_hw_state, you > can just do the s/intel_crtc_restore_mode/__intel_set_mode/ change > manually. I couldn't see right away how to fix it up, so I just compiled your drm-intel-next-queued plus this patch. If I close the netbook's lid and open it again, the screen is blank, no backlight, and the machine seems to be frozen. I think I can live with the warning. Thanks anyhow, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-10 17:27 ` Richard Cochran @ 2013-04-10 18:32 ` Tomas Melin 2013-04-11 5:16 ` Tomas Melin 2013-04-10 20:03 ` [Intel-gfx] " Daniel Vetter 1 sibling, 1 reply; 19+ messages in thread From: Tomas Melin @ 2013-04-10 18:32 UTC (permalink / raw) To: Richard Cochran Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Chris Wilson, stable On Tue, Apr 9, 2013 at 10:51 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > v2: Try harder not to create a big patch (Chris). > Tested the patch applied to 3.9-rc6. Atleast on my machine that helped, although once I managed to get the error (but not warning and call trace as before): [drm:i9xx_crtc_mode_set] *ERROR* Couldn't find PLL settings for mode! On Wed, Apr 10, 2013 at 8:27 PM, Richard Cochran <richardcochran@gmail.com> wrote: > I couldn't see right away how to fix it up, so I just compiled your > drm-intel-next-queued plus this patch. If I close the netbook's lid > and open it again, the screen is blank, no backlight, and the machine > seems to be frozen. The patch doesn't apply at all to 3.8 since function crtc_restore_mode is missing and also 3.9-rc6 was quite different. This version of the patch applies atleast to 3.9-rc6 if you want to test it: >From 9f498da114cea3d82c291b7090d4441664d7870c Mon Sep 17 00:00:00 2001 From: Tomas Melin <tomas.melin@iki.fi> Date: Wed, 10 Apr 2013 18:53:42 +0300 Subject: [PATCH] applied patch --- drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b20d501..83b11c5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7771,9 +7771,9 @@ intel_modeset_check_state(struct drm_device *dev) } } -int intel_set_mode(struct drm_crtc *crtc, - struct drm_display_mode *mode, - int x, int y, struct drm_framebuffer *fb) +static int __intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; drm_i915_private_t *dev_priv = dev->dev_private; @@ -7863,18 +7863,33 @@ done: if (ret && crtc->enabled) { crtc->hwmode = *saved_hwmode; crtc->mode = *saved_mode; - } else { - intel_modeset_check_state(dev); } out: kfree(saved_mode); return ret; } +int intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) +{ + int ret; + + ret = __intel_set_mode(crtc, mode, x, y, fb); + + if (ret == 0) + intel_modeset_check_state(crtc->dev); + + return ret; +} + + void intel_crtc_restore_mode(struct drm_crtc *crtc) { - intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); + __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); + + intel_modeset_check_state(crtc->dev); } #undef for_each_intel_crtc_masked @@ -9172,8 +9187,15 @@ void intel_modeset_setup_hw_state(struct drm_device *dev, } if (force_restore) { + /* + * We need to use raw interfaces for restoring state to avoid + * checking (bogus) intermediate states. + */ for_each_pipe(pipe) { - intel_crtc_restore_mode(dev_priv->pipe_to_crtc_mapping[pipe]); + struct drm_crtc *crtc = + dev_priv->pipe_to_crtc_mapping[pipe]; + __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, + crtc->fb); } i915_redisable_vga(dev); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-10 18:32 ` Tomas Melin @ 2013-04-11 5:16 ` Tomas Melin 0 siblings, 0 replies; 19+ messages in thread From: Tomas Melin @ 2013-04-11 5:16 UTC (permalink / raw) To: Richard Cochran, daniel.vetter Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Chris Wilson, stable On Wed, Apr 10, 2013 at 9:32 PM, Tomas Melin <tomas.melin@iki.fi> wrote: > On Tue, Apr 9, 2013 at 10:51 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> v2: Try harder not to create a big patch (Chris). >> > Tested the patch applied to 3.9-rc6. Atleast on my machine that > helped, although once I managed to get the error (but not warning and > call trace as before): > [drm:i9xx_crtc_mode_set] *ERROR* Couldn't find PLL settings for mode! > Update: the situation where I still get an error is if the machine is in standby with the lid open. Then close and open will generate that kind of error. The patch however fixes the original problem. thanks, Tomas ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-10 17:27 ` Richard Cochran 2013-04-10 18:32 ` Tomas Melin @ 2013-04-10 20:03 ` Daniel Vetter 2013-04-10 20:10 ` Daniel Vetter 2013-04-11 17:52 ` [Intel-gfx] " Richard Cochran 1 sibling, 2 replies; 19+ messages in thread From: Daniel Vetter @ 2013-04-10 20:03 UTC (permalink / raw) To: Richard Cochran Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Chris Wilson, stable On Wed, Apr 10, 2013 at 7:27 PM, Richard Cochran <richardcochran@gmail.com> wrote: > On Wed, Apr 10, 2013 at 02:07:24PM +0200, Daniel Vetter wrote: >> >> It's written against drm-intel-next-queued at >> >> http://cgit.freedesktop.org/~danvet/drm-intel >> >> I've thought that it should apply pretty cleanly against older kernels, >> too. Apparently it conflicts a bit in intel_modeset_setup_hw_state, you >> can just do the s/intel_crtc_restore_mode/__intel_set_mode/ change >> manually. > > I couldn't see right away how to fix it up, so I just compiled your > drm-intel-next-queued plus this patch. If I close the netbook's lid > and open it again, the screen is blank, no backlight, and the machine > seems to be frozen. > > I think I can live with the warning. That patch should crash at all, so this is not expected. Can you pls check whether plain drm-intel-nightly is broken, too? I'll quickly port the patch (in it's latest v3 version) to 3.9-rc kernels for you to test. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-10 20:03 ` [Intel-gfx] " Daniel Vetter @ 2013-04-10 20:10 ` Daniel Vetter 2013-04-11 17:52 ` [Intel-gfx] " Richard Cochran 1 sibling, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2013-04-10 20:10 UTC (permalink / raw) To: Intel Graphics Development Cc: DRI Development, LKML, Dave Airlie, Daniel Vetter, Tomas Melin, Richard Cochran, Chris Wilson, stable It will be only consistent once we've restored all the crtcs. Since a bunch of other callers also want to just restore a single crtc, add a boolean to disable checking only where it doesn't make sense. Note that intel_modeset_setup_hw_state already has a call to intel_modeset_check_state at the end, so we don't reduce the amount of checking. v2: Try harder not to create a big patch (Chris). v3: Even smaller (still Chris). Also fix a trailing space. v4: Rebased on top of 3.9-rc6. References: https://lkml.org/lkml/2013/3/16/60 Cc: Tomas Melin <tomas.melin@iki.fi> Cc: Richard Cochran <richardcochran@gmail.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b20d501..5f1eb50 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7771,9 +7771,9 @@ intel_modeset_check_state(struct drm_device *dev) } } -int intel_set_mode(struct drm_crtc *crtc, - struct drm_display_mode *mode, - int x, int y, struct drm_framebuffer *fb) +static int __intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; drm_i915_private_t *dev_priv = dev->dev_private; @@ -7863,8 +7863,6 @@ done: if (ret && crtc->enabled) { crtc->hwmode = *saved_hwmode; crtc->mode = *saved_mode; - } else { - intel_modeset_check_state(dev); } out: @@ -7872,6 +7870,20 @@ out: return ret; } +int intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) +{ + int ret; + + ret = __intel_set_mode(crtc, mode, x, y, fb); + + if (ret == 0) + intel_modeset_check_state(crtc->dev); + + return ret; +} + void intel_crtc_restore_mode(struct drm_crtc *crtc) { intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); @@ -9172,8 +9184,16 @@ void intel_modeset_setup_hw_state(struct drm_device *dev, } if (force_restore) { + /* + * We need to use raw interfaces for restoring state to avoid + * checking (bogus) intermediate states. + */ for_each_pipe(pipe) { - intel_crtc_restore_mode(dev_priv->pipe_to_crtc_mapping[pipe]); + struct drm_crtc *crtc = + dev_priv->pipe_to_crtc_mapping[pipe]; + + __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, + crtc->fb); } i915_redisable_vga(dev); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-10 20:03 ` [Intel-gfx] " Daniel Vetter 2013-04-10 20:10 ` Daniel Vetter @ 2013-04-11 17:52 ` Richard Cochran 2013-04-11 18:14 ` Daniel Vetter 1 sibling, 1 reply; 19+ messages in thread From: Richard Cochran @ 2013-04-11 17:52 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Chris Wilson, stable On Wed, Apr 10, 2013 at 10:03:25PM +0200, Daniel Vetter wrote: > That patch should crash at all, so this is not expected. Can you pls > check whether plain drm-intel-nightly is broken, too? I did try drm-intel-nightly just now (1dd83e3), and it also freezes the machine. I first verified that the power button shutdown is working (before starting X). Then, with X running, closing and reopening the lid results in a blank screen (no backlight) and a seemingly frozen box. > I'll quickly port the patch (in it's latest v3 version) to 3.9-rc > kernels for you to test. Okay, I'll try this next. Thanks, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-11 17:52 ` [Intel-gfx] " Richard Cochran @ 2013-04-11 18:14 ` Daniel Vetter 2013-04-11 18:37 ` Richard Cochran 2013-04-12 6:59 ` Richard Cochran 0 siblings, 2 replies; 19+ messages in thread From: Daniel Vetter @ 2013-04-11 18:14 UTC (permalink / raw) To: Richard Cochran Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Chris Wilson, stable On Thu, Apr 11, 2013 at 7:52 PM, Richard Cochran <richardcochran@gmail.com> wrote: > On Wed, Apr 10, 2013 at 10:03:25PM +0200, Daniel Vetter wrote: > >> That patch should crash at all, so this is not expected. Can you pls >> check whether plain drm-intel-nightly is broken, too? > > I did try drm-intel-nightly just now (1dd83e3), and it also freezes > the machine. I first verified that the power button shutdown is > working (before starting X). Then, with X running, closing and > reopening the lid results in a blank screen (no backlight) and a > seemingly frozen box. I've just tracked down and fixed an bug which can lead to a hard-hang in the crtc restore code (which is used both in the lid handler when opening and on resume). If you could please test this patch (on top of drm-intel-nightly): https://patchwork.kernel.org/patch/2428971/ >> I'll quickly port the patch (in it's latest v3 version) to 3.9-rc >> kernels for you to test. > > Okay, I'll try this next. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-11 18:14 ` Daniel Vetter @ 2013-04-11 18:37 ` Richard Cochran 2013-04-12 6:59 ` Richard Cochran 1 sibling, 0 replies; 19+ messages in thread From: Richard Cochran @ 2013-04-11 18:37 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Chris Wilson, stable On Thu, Apr 11, 2013 at 08:14:10PM +0200, Daniel Vetter wrote: > > I've just tracked down and fixed an bug which can lead to a hard-hang > in the crtc restore code (which is used both in the lid handler when > opening and on resume). If you could please test this patch (on top of > drm-intel-nightly): > > https://patchwork.kernel.org/patch/2428971/ Okay, will do. > >> I'll quickly port the patch (in it's latest v3 version) to 3.9-rc > >> kernels for you to test. FWIW, this does work, no freeze, no warning, but instead: Apr 11 20:30:49 netboy laptop-mode: Laptop mode Apr 11 20:30:49 netboy laptop-mode: enabled, Apr 11 20:30:49 netboy laptop-mode: not active [unchanged] Apr 11 20:30:53 netboy kernel: [ 75.450783] [drm:i9xx_crtc_mode_set] *ERROR* Couldn't find PLL settings for mode! Apr 11 20:30:53 netboy laptop-mode: Laptop mode Apr 11 20:30:53 netboy laptop-mode: enabled, Apr 11 20:30:53 netboy laptop-mode: not active [unchanged] Thanks, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-11 18:14 ` Daniel Vetter 2013-04-11 18:37 ` Richard Cochran @ 2013-04-12 6:59 ` Richard Cochran 1 sibling, 0 replies; 19+ messages in thread From: Richard Cochran @ 2013-04-12 6:59 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Chris Wilson, stable On Thu, Apr 11, 2013 at 08:14:10PM +0200, Daniel Vetter wrote: > > I've just tracked down and fixed an bug which can lead to a hard-hang > in the crtc restore code (which is used both in the lid handler when > opening and on resume). If you could please test this patch (on top of > drm-intel-nightly): > > https://patchwork.kernel.org/patch/2428971/ Now I can confirm this works fine, with no warnings, errors, or freezes. Thanks, Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-09 19:51 ` [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring Daniel Vetter 2013-04-10 11:59 ` Richard Cochran @ 2013-04-11 18:22 ` Daniel Vetter 2013-04-11 19:17 ` Chris Wilson 1 sibling, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2013-04-11 18:22 UTC (permalink / raw) To: Intel Graphics Development Cc: DRI Development, LKML, Dave Airlie, Daniel Vetter, Tomas Melin, Richard Cochran, Chris Wilson, stable It will be only consistent once we've restored all the crtcs. Since a bunch of other callers also want to just restore a single crtc, add a boolean to disable checking only where it doesn't make sense. Note that intel_modeset_setup_hw_state already has a call to intel_modeset_check_state at the end, so we don't reduce the amount of checking. v2: Try harder not to create a big patch (Chris). v3: Even smaller (still Chris). Also fix a trailing space. References: https://lkml.org/lkml/2013/3/16/60 Cc: Tomas Melin <tomas.melin@iki.fi> Cc: Richard Cochran <richardcochran@gmail.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8809813..457a0a0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7916,9 +7916,9 @@ intel_modeset_check_state(struct drm_device *dev) } } -int intel_set_mode(struct drm_crtc *crtc, - struct drm_display_mode *mode, - int x, int y, struct drm_framebuffer *fb) +static int __intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; drm_i915_private_t *dev_priv = dev->dev_private; @@ -8012,8 +8012,6 @@ done: if (ret && crtc->enabled) { crtc->hwmode = *saved_hwmode; crtc->mode = *saved_mode; - } else { - intel_modeset_check_state(dev); } out: @@ -8022,6 +8020,20 @@ out: return ret; } +int intel_set_mode(struct drm_crtc *crtc, + struct drm_display_mode *mode, + int x, int y, struct drm_framebuffer *fb) +{ + int ret; + + ret = __intel_set_mode(crtc, mode, x, y, fb); + + if (ret == 0) + intel_modeset_check_state(crtc->dev); + + return ret; +} + void intel_crtc_restore_mode(struct drm_crtc *crtc) { intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); @@ -9333,10 +9345,16 @@ setup_pipes: } if (force_restore) { + /* + * We need to use raw interfaces for restoring state to avoid + * checking (bogus) intermediate states. + */ for_each_pipe(pipe) { struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; - intel_crtc_restore_mode(crtc); + + __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, + crtc->fb); } list_for_each_entry(plane, &dev->mode_config.plane_list, head) intel_plane_restore(plane); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-11 18:22 ` Daniel Vetter @ 2013-04-11 19:17 ` Chris Wilson 2013-04-11 19:28 ` Daniel Vetter 0 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2013-04-11 19:17 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Richard Cochran, stable On Thu, Apr 11, 2013 at 08:22:50PM +0200, Daniel Vetter wrote: > It will be only consistent once we've restored all the crtcs. Since a > bunch of other callers also want to just restore a single crtc, add a > boolean to disable checking only where it doesn't make sense. > > Note that intel_modeset_setup_hw_state already has a call to > intel_modeset_check_state at the end, so we don't reduce the amount of > checking. > > v2: Try harder not to create a big patch (Chris). > > v3: Even smaller (still Chris). Also fix a trailing space. > > References: https://lkml.org/lkml/2013/3/16/60 > Cc: Tomas Melin <tomas.melin@iki.fi> > Cc: Richard Cochran <richardcochran@gmail.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: stable@vger.kernel.org > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> I'm happy that you are not touching any paths other than mentioned in the changelog, and that I think being quiet on the modeset is a good use of the 'force' semantics, so Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring 2013-04-11 19:17 ` Chris Wilson @ 2013-04-11 19:28 ` Daniel Vetter 0 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2013-04-11 19:28 UTC (permalink / raw) To: Chris Wilson Cc: Daniel Vetter, Intel Graphics Development, DRI Development, LKML, Dave Airlie, Tomas Melin, Richard Cochran, stable On Thu, Apr 11, 2013 at 08:17:42PM +0100, Chris Wilson wrote: > On Thu, Apr 11, 2013 at 08:22:50PM +0200, Daniel Vetter wrote: > > It will be only consistent once we've restored all the crtcs. Since a > > bunch of other callers also want to just restore a single crtc, add a > > boolean to disable checking only where it doesn't make sense. > > > > Note that intel_modeset_setup_hw_state already has a call to > > intel_modeset_check_state at the end, so we don't reduce the amount of > > checking. > > > > v2: Try harder not to create a big patch (Chris). > > > > v3: Even smaller (still Chris). Also fix a trailing space. > > > > References: https://lkml.org/lkml/2013/3/16/60 > > Cc: Tomas Melin <tomas.melin@iki.fi> > > Cc: Richard Cochran <richardcochran@gmail.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: stable@vger.kernel.org > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > I'm happy that you are not touching any paths other than mentioned in > the changelog, and that I think being quiet on the modeset is a good use > of the 'force' semantics, so > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2013-04-12 6:59 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-04-09 11:54 Re: drm/i915: new warning (regression) in 3.7.10 and 3.8.3 Tomas Melin 2013-04-09 12:50 ` Daniel Vetter 2013-04-09 13:21 ` Richard Cochran 2013-04-09 17:28 ` Daniel Vetter 2013-04-09 19:51 ` [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring Daniel Vetter 2013-04-10 11:59 ` Richard Cochran 2013-04-10 12:07 ` Daniel Vetter 2013-04-10 17:27 ` Richard Cochran 2013-04-10 18:32 ` Tomas Melin 2013-04-11 5:16 ` Tomas Melin 2013-04-10 20:03 ` [Intel-gfx] " Daniel Vetter 2013-04-10 20:10 ` Daniel Vetter 2013-04-11 17:52 ` [Intel-gfx] " Richard Cochran 2013-04-11 18:14 ` Daniel Vetter 2013-04-11 18:37 ` Richard Cochran 2013-04-12 6:59 ` Richard Cochran 2013-04-11 18:22 ` Daniel Vetter 2013-04-11 19:17 ` Chris Wilson 2013-04-11 19:28 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).