date:20170719

Re: [Nouveau] [PATCH] drm: disable vblank only if it got previously enabled

2017-07-19 Thread Ilia Mirkin

I believe the solution is to not call drm_crtc_vblank_off for atomic
modesetting in nouveau_display_fini. I think Ben's working on it.

On Wed, Jul 19, 2017 at 1:25 PM, Tobias Klausmann
 wrote:
> mimic the behavior of vblank_disable_fn(), another caller of
> drm_vblank_disable_and_save().
>
> This avoids oopsing, while trying to disable vblank on a not connected 
> display:
>
> [   12.768079] WARNING: CPU: 0 PID: 274 at drivers/gpu/drm/drm_vblank.c:609 
> drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
> [   12.768080] Modules linked in: bnep snd_hda_codec_hdmi rtsx_usb_sdmmc 
> uvcvideo rtsx_usb_ms mmc_core videobuf2_vmalloc memstick videobuf2_memops 
> videobuf2_v4l2 videobuf2_core rtsx_usb videodev btusb btrtl arc4 
> snd_hda_codec_realtek snd_hda_codec_generic joydev nls_iso8859_1 
> hid_multitouch nls_cp437 intel_rapl x86_pkg_temp_thermal intel_powerclamp 
> vfat coretemp fat kvm_intel iTCO_wdt iTCO_vendor_support kvm irqbypass 
> crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc 
> aesni_intel ath10k_pci snd_hda_intel ath10k_core aes_x86_64 snd_hda_codec 
> crypto_simd ath glue_helper cryptd snd_hda_core mac80211 snd_hwdep snd_pcm 
> pcspkr r8169 cfg80211 mii snd_timer acer_wmi snd sparse_keymap wmi_bmof 
> idma64 hci_uart virt_dma mei_me soundcore i2c_i801 mei btbcm shpchp 
> intel_lpss_pci intel_pch_thermal
> [   12.768130]  serdev btqca ucsi_acpi btintel typec_ucsi thermal typec 
> bluetooth ecdh_generic battery ac pinctrl_sunrisepoint rfkill intel_lpss_acpi 
> pinctrl_intel intel_lpss acpi_pad nouveau serio_raw i915 mxm_wmi ttm 
> i2c_algo_bit drm_kms_helper xhci_pci syscopyarea sysfillrect sysimgblt 
> xhci_hcd fb_sys_fops usbcore drm i2c_hid wmi video button sg efivarfs
> [   12.768158] CPU: 0 PID: 274 Comm: kworker/0:2 Not tainted 
> 4.12.0-desktop-debug-drm+ #2
> [   12.768160] Hardware name: Acer Aspire VN7-593G/Pluto_KLS, BIOS V1.04 
> 03/30/2017
> [   12.768164] Workqueue: pm pm_runtime_work
> [   12.768166] task: 889bf1627040 task.stack: 9541013e4000
> [   12.768180] RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 
> [drm]
> [   12.768181] RSP: 0018:9541013e7b30 EFLAGS: 00010086
> [   12.768183] RAX: 001c RBX: 889b4cebd000 RCX: 
> 0004
> [   12.768184] RDX: 8004 RSI: 87a2d952 RDI: 
> 
> [   12.768186] RBP: 9541013e7b90 R08: 0001 R09: 
> 039f
> [   12.768187] R10: c05fe530 R11:  R12: 
> 
> [   12.768188] R13: 9541013e7ba4 R14: 889bf0426088 R15: 
> 889bf0426000
> [   12.768190] FS:  () GS:889bfec0() 
> knlGS:
> [   12.768191] CS:  0010 DS:  ES:  CR0: 80050033
> [   12.768192] CR2: 00edb16580b8 CR3: 00020cc09000 CR4: 
> 003406f0
> [   12.768193] Call Trace:
> [   12.768198]  ? enqueue_task_fair+0x64/0x600
> [   12.768211]  ? drm_get_last_vbltimestamp+0x47/0x70 [drm]
> [   12.768223]  ? drm_update_vblank_count+0x65/0x240 [drm]
> [   12.768227]  ? pci_pm_runtime_resume+0xa0/0xa0
> [   12.768238]  ? drm_vblank_disable_and_save+0x55/0xc0 [drm]
> [   12.768250]  ? drm_crtc_vblank_off+0xa9/0x1e0 [drm]
> [   12.768253]  ? pci_pm_runtime_resume+0xa0/0xa0
> [   12.768299]  ? nouveau_display_fini+0x56/0xd0 [nouveau]
> [   12.768339]  ? nouveau_display_suspend+0x51/0x110 [nouveau]
> [   12.768378]  ? nouveau_do_suspend+0x76/0x1c0 [nouveau]
> [   12.768413]  ? nouveau_pmops_runtime_suspend+0x54/0xb0 [nouveau]
> [   12.768416]  ? pci_pm_runtime_suspend+0x5c/0x160
> [   12.768419]  ? __rpm_callback+0xb6/0x1e0
> [   12.768423]  ? kobject_uevent_env+0x111/0x5e0
> [   12.768425]  ? pci_pm_runtime_resume+0xa0/0xa0
> [   12.768427]  ? rpm_callback+0x1f/0x70
> [   12.768429]  ? pci_pm_runtime_resume+0xa0/0xa0
> [   12.768431]  ? rpm_suspend+0x11f/0x640
> [   12.768441]  ? drm_fb_helper_hotplug_event+0x9a/0xe0 [drm_kms_helper]
> [   12.768447]  ? output_poll_execute+0x17b/0x1a0 [drm_kms_helper]
> [   12.768449]  ? pm_runtime_work+0x64/0xa0
> [   12.768453]  ? process_one_work+0x1db/0x410
> [   12.768456]  ? worker_thread+0x47/0x3d0
> [   12.768459]  ? process_one_work+0x410/0x410
> [   12.768461]  ? kthread+0x117/0x130
> [   12.768463]  ? kthread_create_on_node+0x40/0x40
> [   12.768466]  ? ret_from_fork+0x25/0x30
> [   12.768468] Code: 80 3d 26 f3 01 00 00 0f 85 ad fd ff ff 48 8b 43 20 48 c7 
> c7 31 a2 20 c0 c6 05 0e f3 01 00 01 48 8b b0 60 01 00 00 e8 75 2e ec c6 <0f> 
> ff e9 88 fd ff ff 31 f6 44 88 55 b0 e8 38 fa ed c6 44 0f b6
> [   12.768508] ---[ end trace d9bb853af3659bd5 ]---
>
> Signed-off-by: Tobias Klausmann 
> ---
>  drivers/gpu/drm/drm_vblank.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index a233a6be934a..4a21756bf2bd 100644
> ---

[Nouveau] [PATCH] drm: disable vblank only if it got previously enabled

2017-07-19 Thread Tobias Klausmann

mimic the behavior of vblank_disable_fn(), another caller of
drm_vblank_disable_and_save().

This avoids oopsing, while trying to disable vblank on a not connected display:

[   12.768079] WARNING: CPU: 0 PID: 274 at drivers/gpu/drm/drm_vblank.c:609 
drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
[   12.768080] Modules linked in: bnep snd_hda_codec_hdmi rtsx_usb_sdmmc 
uvcvideo rtsx_usb_ms mmc_core videobuf2_vmalloc memstick videobuf2_memops 
videobuf2_v4l2 videobuf2_core rtsx_usb videodev btusb btrtl arc4 
snd_hda_codec_realtek snd_hda_codec_generic joydev nls_iso8859_1 hid_multitouch 
nls_cp437 intel_rapl x86_pkg_temp_thermal intel_powerclamp vfat coretemp fat 
kvm_intel iTCO_wdt iTCO_vendor_support kvm irqbypass crct10dif_pclmul 
crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc aesni_intel ath10k_pci 
snd_hda_intel ath10k_core aes_x86_64 snd_hda_codec crypto_simd ath glue_helper 
cryptd snd_hda_core mac80211 snd_hwdep snd_pcm pcspkr r8169 cfg80211 mii 
snd_timer acer_wmi snd sparse_keymap wmi_bmof idma64 hci_uart virt_dma mei_me 
soundcore i2c_i801 mei btbcm shpchp intel_lpss_pci intel_pch_thermal
[   12.768130]  serdev btqca ucsi_acpi btintel typec_ucsi thermal typec 
bluetooth ecdh_generic battery ac pinctrl_sunrisepoint rfkill intel_lpss_acpi 
pinctrl_intel intel_lpss acpi_pad nouveau serio_raw i915 mxm_wmi ttm 
i2c_algo_bit drm_kms_helper xhci_pci syscopyarea sysfillrect sysimgblt xhci_hcd 
fb_sys_fops usbcore drm i2c_hid wmi video button sg efivarfs
[   12.768158] CPU: 0 PID: 274 Comm: kworker/0:2 Not tainted 
4.12.0-desktop-debug-drm+ #2
[   12.768160] Hardware name: Acer Aspire VN7-593G/Pluto_KLS, BIOS V1.04 
03/30/2017
[   12.768164] Workqueue: pm pm_runtime_work
[   12.768166] task: 889bf1627040 task.stack: 9541013e4000
[   12.768180] RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x296/0x320 [drm]
[   12.768181] RSP: 0018:9541013e7b30 EFLAGS: 00010086
[   12.768183] RAX: 001c RBX: 889b4cebd000 RCX: 0004
[   12.768184] RDX: 8004 RSI: 87a2d952 RDI: 
[   12.768186] RBP: 9541013e7b90 R08: 0001 R09: 039f
[   12.768187] R10: c05fe530 R11:  R12: 
[   12.768188] R13: 9541013e7ba4 R14: 889bf0426088 R15: 889bf0426000
[   12.768190] FS:  () GS:889bfec0() 
knlGS:
[   12.768191] CS:  0010 DS:  ES:  CR0: 80050033
[   12.768192] CR2: 00edb16580b8 CR3: 00020cc09000 CR4: 003406f0
[   12.768193] Call Trace:
[   12.768198]  ? enqueue_task_fair+0x64/0x600
[   12.768211]  ? drm_get_last_vbltimestamp+0x47/0x70 [drm]
[   12.768223]  ? drm_update_vblank_count+0x65/0x240 [drm]
[   12.768227]  ? pci_pm_runtime_resume+0xa0/0xa0
[   12.768238]  ? drm_vblank_disable_and_save+0x55/0xc0 [drm]
[   12.768250]  ? drm_crtc_vblank_off+0xa9/0x1e0 [drm]
[   12.768253]  ? pci_pm_runtime_resume+0xa0/0xa0
[   12.768299]  ? nouveau_display_fini+0x56/0xd0 [nouveau]
[   12.768339]  ? nouveau_display_suspend+0x51/0x110 [nouveau]
[   12.768378]  ? nouveau_do_suspend+0x76/0x1c0 [nouveau]
[   12.768413]  ? nouveau_pmops_runtime_suspend+0x54/0xb0 [nouveau]
[   12.768416]  ? pci_pm_runtime_suspend+0x5c/0x160
[   12.768419]  ? __rpm_callback+0xb6/0x1e0
[   12.768423]  ? kobject_uevent_env+0x111/0x5e0
[   12.768425]  ? pci_pm_runtime_resume+0xa0/0xa0
[   12.768427]  ? rpm_callback+0x1f/0x70
[   12.768429]  ? pci_pm_runtime_resume+0xa0/0xa0
[   12.768431]  ? rpm_suspend+0x11f/0x640
[   12.768441]  ? drm_fb_helper_hotplug_event+0x9a/0xe0 [drm_kms_helper]
[   12.768447]  ? output_poll_execute+0x17b/0x1a0 [drm_kms_helper]
[   12.768449]  ? pm_runtime_work+0x64/0xa0
[   12.768453]  ? process_one_work+0x1db/0x410
[   12.768456]  ? worker_thread+0x47/0x3d0
[   12.768459]  ? process_one_work+0x410/0x410
[   12.768461]  ? kthread+0x117/0x130
[   12.768463]  ? kthread_create_on_node+0x40/0x40
[   12.768466]  ? ret_from_fork+0x25/0x30
[   12.768468] Code: 80 3d 26 f3 01 00 00 0f 85 ad fd ff ff 48 8b 43 20 48 c7 
c7 31 a2 20 c0 c6 05 0e f3 01 00 01 48 8b b0 60 01 00 00 e8 75 2e ec c6 <0f> ff 
e9 88 fd ff ff 31 f6 44 88 55 b0 e8 38 fa ed c6 44 0f b6
[   12.768508] ---[ end trace d9bb853af3659bd5 ]---

Signed-off-by: Tobias Klausmann 
---
 drivers/gpu/drm/drm_vblank.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index a233a6be934a..4a21756bf2bd 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1140,8 +1140,11 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
 
/* Avoid redundant vblank disables without previous
 * drm_crtc_vblank_on(). */
-   if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
+   if (drm_core_check_feature(dev, DRIVER_ATOMIC) || (!vblank->inmodeset &&
+   vblank->enabled)) {
+

[Nouveau] [PATCH v2 6/7] drm/nouveau: Convert nouveau to use new iterator macros, v2.

2017-07-19 Thread Maarten Lankhorst

Use the new atomic iterator macros, the old ones are about to be
removed. With the new macros, it's more easy to get old and new state so
get them from the macros instead of from obj->state.

Changes since v1:
- Don't mix up old and new state. (danvet)
- Rebase on top of interruptible swap_state changes.

Signed-off-by: Maarten Lankhorst 
Cc: Ben Skeggs 
Cc: nouveau@lists.freedesktop.org
---
 drivers/gpu/drm/nouveau/nv50_display.c | 72 +-
 1 file changed, 37 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
b/drivers/gpu/drm/nouveau/nv50_display.c
index 747c99c1e474..7abfb561b00c 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2103,7 +2103,7 @@ nv50_head_atomic_check(struct drm_crtc *crtc, struct 
drm_crtc_state *state)
 
NV_ATOMIC(drm, "%s atomic_check %d\n", crtc->name, asyh->state.active);
if (asyh->state.active) {
-   for_each_connector_in_state(asyh->state.state, conn, conns, i) {
+   for_each_new_connector_in_state(asyh->state.state, conn, conns, 
i) {
if (conns->crtc == crtc) {
asyc = nouveau_conn_atom(conns);
break;
@@ -3905,9 +3905,9 @@ static void
 nv50_disp_atomic_commit_tail(struct drm_atomic_state *state)
 {
struct drm_device *dev = state->dev;
-   struct drm_crtc_state *crtc_state;
+   struct drm_crtc_state *new_crtc_state;
struct drm_crtc *crtc;
-   struct drm_plane_state *plane_state;
+   struct drm_plane_state *new_plane_state;
struct drm_plane *plane;
struct nouveau_drm *drm = nouveau_drm(dev);
struct nv50_disp *disp = nv50_disp(dev);
@@ -3926,8 +3926,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
mutex_lock(>mutex);
 
/* Disable head(s). */
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   struct nv50_head_atom *asyh = nv50_head_atom(crtc->state);
+   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+   struct nv50_head_atom *asyh = nv50_head_atom(new_crtc_state);
struct nv50_head *head = nv50_head(crtc);
 
NV_ATOMIC(drm, "%s: clr %04x (set %04x)\n", crtc->name,
@@ -3940,8 +3940,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
}
 
/* Disable plane(s). */
-   for_each_plane_in_state(state, plane, plane_state, i) {
-   struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+   for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+   struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
struct nv50_wndw *wndw = nv50_wndw(plane);
 
NV_ATOMIC(drm, "%s: clr %02x (set %02x)\n", plane->name,
@@ -4006,8 +4006,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
}
 
/* Update head(s). */
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   struct nv50_head_atom *asyh = nv50_head_atom(crtc->state);
+   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+   struct nv50_head_atom *asyh = nv50_head_atom(new_crtc_state);
struct nv50_head *head = nv50_head(crtc);
 
NV_ATOMIC(drm, "%s: set %04x (clr %04x)\n", crtc->name,
@@ -4019,14 +4019,14 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
}
}
 
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   if (crtc->state->event)
+   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+   if (new_crtc_state->event)
drm_crtc_vblank_get(crtc);
}
 
/* Update plane(s). */
-   for_each_plane_in_state(state, plane, plane_state, i) {
-   struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+   for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+   struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
struct nv50_wndw *wndw = nv50_wndw(plane);
 
NV_ATOMIC(drm, "%s: set %02x (clr %02x)\n", plane->name,
@@ -4056,23 +4056,23 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
mutex_unlock(>mutex);
 
/* Wait for HW to signal completion. */
-   for_each_plane_in_state(state, plane, plane_state, i) {
-   struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+   for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+   struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
struct nv50_wndw *wndw = nv50_wndw(plane);
int ret = nv50_wndw_wait_armed(wndw, asyw);
if (ret)
NV_ERROR(drm, "%s:

[Nouveau] [PATCH 01/12] drm/nouveau: Fix error handling in nv50_disp_atomic_commit

2017-07-19 Thread Maarten Lankhorst

Make it more clear that post commit return ret is really return 0,

and add a missing drm_atomic_helper_cleanup_planes when
drm_atomic_helper_wait_for_fences fails.

Fixes: 839ca903f12e ("drm/nouveau/kms/nv50: transition to atomic interfaces 
internally")
Cc: Ben Skeggs 
Cc: dri-de...@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc:  # v4.10+
Signed-off-by: Maarten Lankhorst 
Link: 
http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-2-maarten.lankho...@linux.intel.com
Reviewed-by: Sean Paul 
[mlankhorst: Use if (ret) to remove the goto in success case.]
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/nouveau/nv50_display.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
b/drivers/gpu/drm/nouveau/nv50_display.c
index 5f71e304022e..c1e7307a2538 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -4120,7 +4120,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
if (!nonblock) {
ret = drm_atomic_helper_wait_for_fences(dev, state, true);
if (ret)
-   goto done;
+   goto err_cleanup;
}
 
for_each_plane_in_state(state, plane, plane_state, i) {
@@ -4148,7 +4148,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
if (crtc->state->enable) {
if (!drm->have_disp_power_ref) {
drm->have_disp_power_ref = true;
-   return ret;
+   return 0;
}
active = true;
break;
@@ -4160,6 +4160,9 @@ nv50_disp_atomic_commit(struct drm_device *dev,
drm->have_disp_power_ref = false;
}
 
+err_cleanup:
+   if (ret)
+   drm_atomic_helper_cleanup_planes(dev, state);
 done:
pm_runtime_put_autosuspend(dev->dev);
return ret;
-- 
2.11.0

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH 03/12] drm/nouveau: Handle drm_atomic_helper_swap_state failure

2017-07-19 Thread Maarten Lankhorst

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

Signed-off-by: Maarten Lankhorst 
Cc: Ben Skeggs 
Cc: nouveau@lists.freedesktop.org
Link: 
http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-4-maarten.lankho...@linux.intel.com
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/nouveau/nv50_display.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
b/drivers/gpu/drm/nouveau/nv50_display.c
index c1e7307a2538..747c99c1e474 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -4123,6 +4123,10 @@ nv50_disp_atomic_commit(struct drm_device *dev,
goto err_cleanup;
}
 
+   ret = drm_atomic_helper_swap_state(state, true);
+   if (ret)
+   goto err_cleanup;
+
for_each_plane_in_state(state, plane, plane_state, i) {
struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane_state);
struct nv50_wndw *wndw = nv50_wndw(plane);
@@ -4136,7 +4140,6 @@ nv50_disp_atomic_commit(struct drm_device *dev,
}
}
 
-   drm_atomic_helper_swap_state(state, true);
drm_atomic_state_get(state);
 
if (nonblock)
-- 
2.11.0

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm: disable vblank only if it got previously enabled

[Nouveau] [PATCH] drm: disable vblank only if it got previously enabled

[Nouveau] [PATCH v2 6/7] drm/nouveau: Convert nouveau to use new iterator macros, v2.

[Nouveau] [PATCH 01/12] drm/nouveau: Fix error handling in nv50_disp_atomic_commit

[Nouveau] [PATCH 03/12] drm/nouveau: Handle drm_atomic_helper_swap_state failure

5 matches

Site Navigation

Mail list logo

Footer information