* [PATCH 2/2] drm/i915/fbc: Deactivate fbc when switching pipes
2017-10-27 19:42 [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous Chris Wilson
@ 2017-10-27 19:42 ` Chris Wilson
2017-10-27 20:08 ` Ville Syrjälä
2017-10-27 19:56 ` [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous Ville Syrjälä
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2017-10-27 19:42 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Daniel Vetter, Rodrigo Vivi
If we are transfering an fb from one crtc to another, we will keep FBC
activated (due to only having a single pipe) but then we will call
intel_fbc_disable() from intel_atomic_commit_tail() on the old pipe
before enabling the new pipe. However, we insist that before disabling
FBC, it is deactivated.
Otherwise we generate warnings such as:
[ 346.741263] WARN_ON(fbc->active)
[ 346.741308] ------------[ cut here ]------------
[ 346.741387] WARNING: CPU: 3 PID: 4014 at drivers/gpu/drm/i915/intel_fbc.c:1176 __intel_fbc_disable+0xdf/0x110 [i915]
[ 346.741394] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core ghash_clmulni_intel snd_pcm e1000e mei_me mei lpc_ich ptp pps_core prime_numbers i2c_hid
[ 346.741531] CPU: 3 PID: 4014 Comm: kms_frontbuffer Tainted: G U 4.14.0-rc6-CI-Patchwork_6242+ #1
[ 346.741537] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0362.2017.0118.0940 01/18/2017
[ 346.741544] task: ffff880236502800 task.stack: ffffc90002874000
[ 346.741617] RIP: 0010:__intel_fbc_disable+0xdf/0x110 [i915]
[ 346.741624] RSP: 0018:ffffc900028779c0 EFLAGS: 00010282
[ 346.741635] RAX: 0000000000000014 RBX: ffff880240df0000 RCX: 0000000000000006
[ 346.741641] RDX: 000000000000136a RSI: ffffffff81d0e984 RDI: ffffffff81cc2576
[ 346.741647] RBP: ffffc900028779d0 R08: ffff880236503138 R09: 0000000000000000
[ 346.741653] R10: ffffc900028779d0 R11: 0000000000000000 R12: ffff880240dec138
[ 346.741659] R13: ffff880240df4960 R14: ffff880240df0000 R15: ffff880240dec138
[ 346.741666] FS: 00007fb237b2ca40(0000) GS:ffff880256d80000(0000) knlGS:0000000000000000
[ 346.741673] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 346.741679] CR2: 00007f9c1805b008 CR3: 0000000236b5f004 CR4: 00000000003606e0
[ 346.741685] Call Trace:
[ 346.741762] intel_fbc_disable+0x61/0x70 [i915]
[ 346.741837] intel_atomic_commit_tail+0x11c/0xbf0 [i915]
[ 346.741920] intel_atomic_commit+0x223/0x2d0 [i915]
[ 346.742046] drm_atomic_commit+0x4b/0x50
[ 346.742128] hsw_pipe_A_crc_wa+0x87/0x180 [i915]
[ 346.742226] get_new_crc_ctl_reg+0x13d/0x320 [i915]
[ 346.742304] intel_crtc_set_crc_source+0x7c/0x1d0 [i915]
[ 346.742329] crtc_crc_open+0xa2/0x2b0
[ 346.742346] ? rcu_read_lock_sched_held+0x7a/0x90
[ 346.742359] ? kmem_cache_alloc_trace+0x270/0x2d0
[ 346.742382] full_proxy_open+0xfd/0x1b0
[ 346.742401] ? u32_array_release+0x20/0x20
[ 346.742419] do_dentry_open.isra.1+0x1d3/0x2e0
[ 346.742440] vfs_open+0x47/0x70
[ 346.742458] path_openat+0x274/0x990
[ 346.742489] do_filp_open+0x8a/0xf0
[ 346.742530] ? _raw_spin_unlock+0x31/0x50
[ 346.742547] ? __alloc_fd+0xf8/0x210
[ 346.742573] do_sys_open+0x12f/0x200
[ 346.742588] ? do_sys_open+0x12f/0x200
[ 346.742615] SyS_openat+0x14/0x20
[ 346.742631] entry_SYSCALL_64_fastpath+0x1c/0xb1
[ 346.742643] RIP: 0033:0x7fb235d1d0fa
[ 346.742654] RSP: 002b:00007ffd72cd60a0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
[ 346.742675] RAX: ffffffffffffffda RBX: ffffffff81491ef3 RCX: 00007fb235d1d0fa
[ 346.742689] RDX: 0000000000000000 RSI: 00007ffd72cd6160 RDI: 0000000000000006
[ 346.742702] RBP: ffffc90002877f88 R08: 0000000000000000 R09: 000000000000000f
[ 346.742713] R10: 0000000000000000 R11: 0000000000000246 R12: 000055e1cbb31298
[ 346.742723] R13: 000055e1cb91aaca R14: 0000000000000001 R15: 000055e1cbce1450
[ 346.742744] ? __this_cpu_preempt_check+0x13/0x20
[ 346.742772] Code: 74 4a 2a a0 e8 b4 35 ed e0 0f ff 80 bb a2 4a 00 00 00 0f 84 6f ff ff ff 48 c7 c6 8e 4a 2a a0 48 c7 c7 74 4a 2a a0 e8 92 35 ed e0 <0f> ff 41 80 bc 24 e8 05 00 00 00 0f 84 5a ff ff ff 48 c7 c6 a3
[ 346.743496] ---[ end trace 5331a8d111243000 ]---
[ 346.746293] WARN_ON(fbc->active)
[ 346.746313] ------------[ cut here ]------------
[ 346.746351] WARNING: CPU: 3 PID: 4014 at drivers/gpu/drm/i915/intel_fbc.c:1144 intel_fbc_enable+0x4b6/0x560 [i915]
[ 346.746356] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core ghash_clmulni_intel snd_pcm e1000e mei_me mei lpc_ich ptp pps_core prime_numbers i2c_hid
[ 346.746432] CPU: 3 PID: 4014 Comm: kms_frontbuffer Tainted: G U W 4.14.0-rc6-CI-Patchwork_6242+ #1
[ 346.746435] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0362.2017.0118.0940 01/18/2017
[ 346.746438] task: ffff880236502800 task.stack: ffffc90002874000
[ 346.746474] RIP: 0010:intel_fbc_enable+0x4b6/0x560 [i915]
[ 346.746478] RSP: 0018:ffffc90002877950 EFLAGS: 00010282
[ 346.746485] RAX: 0000000000000014 RBX: ffff880240df0000 RCX: 0000000000000006
[ 346.746488] RDX: 000000000000136a RSI: ffffffff81d0e984 RDI: ffffffff81cc2576
[ 346.746491] RBP: ffffc900028779a0 R08: ffff880236503138 R09: 0000000000000000
[ 346.746494] R10: ffffc90002877940 R11: 0000000000000000 R12: ffff88024470c138
[ 346.746497] R13: ffff8802497fc6f8 R14: ffff88024470a548 R15: ffff880240df0000
[ 346.746501] FS: 00007fb237b2ca40(0000) GS:ffff880256d80000(0000) knlGS:0000000000000000
[ 346.746504] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 346.746507] CR2: 00007f9c1805b008 CR3: 0000000236b5f004 CR4: 00000000003606e0
[ 346.746511] Call Trace:
[ 346.746551] intel_update_crtc+0x67/0x90 [i915]
[ 346.746590] intel_update_crtcs+0x5d/0x70 [i915]
[ 346.746630] intel_atomic_commit_tail+0x286/0xbf0 [i915]
[ 346.746674] intel_atomic_commit+0x223/0x2d0 [i915]
[ 346.746684] drm_atomic_commit+0x4b/0x50
[ 346.746714] hsw_pipe_A_crc_wa+0x87/0x180 [i915]
[ 346.746750] get_new_crc_ctl_reg+0x13d/0x320 [i915]
[ 346.746782] intel_crtc_set_crc_source+0x7c/0x1d0 [i915]
[ 346.746789] crtc_crc_open+0xa2/0x2b0
[ 346.746795] ? rcu_read_lock_sched_held+0x7a/0x90
[ 346.746800] ? kmem_cache_alloc_trace+0x270/0x2d0
[ 346.746808] full_proxy_open+0xfd/0x1b0
[ 346.746814] ? u32_array_release+0x20/0x20
[ 346.746820] do_dentry_open.isra.1+0x1d3/0x2e0
[ 346.746827] vfs_open+0x47/0x70
[ 346.746833] path_openat+0x274/0x990
[ 346.746842] do_filp_open+0x8a/0xf0
[ 346.746854] ? _raw_spin_unlock+0x31/0x50
[ 346.746860] ? __alloc_fd+0xf8/0x210
[ 346.746869] do_sys_open+0x12f/0x200
[ 346.746874] ? do_sys_open+0x12f/0x200
[ 346.746917] SyS_openat+0x14/0x20
[ 346.746928] entry_SYSCALL_64_fastpath+0x1c/0xb1
[ 346.746935] RIP: 0033:0x7fb235d1d0fa
[ 346.746941] RSP: 002b:00007ffd72cd60a0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
[ 346.746955] RAX: ffffffffffffffda RBX: ffffffff81491ef3 RCX: 00007fb235d1d0fa
[ 346.746963] RDX: 0000000000000000 RSI: 00007ffd72cd6160 RDI: 0000000000000006
[ 346.746969] RBP: ffffc90002877f88 R08: 0000000000000000 R09: 000000000000000f
[ 346.746975] R10: 0000000000000000 R11: 0000000000000246 R12: 000055e1cbb31298
[ 346.746981] R13: 000055e1cb91aaca R14: 0000000000000001 R15: 000055e1cbce1450
[ 346.746993] ? __this_cpu_preempt_check+0x13/0x20
[ 346.747008] Code: c6 10 1c 2c a0 48 c7 c7 74 4a 2a a0 e8 75 1e ed e0 0f ff e9 e3 fb ff ff 48 c7 c6 8e 4a 2a a0 48 c7 c7 74 4a 2a a0 e8 5b 1e ed e0 <0f> ff e9 bb fb ff ff 49 8b 94 24 00 4a 00 00 41 03 94 24 10 62
[ 346.747357] ---[ end trace 5331a8d111243001 ]---
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102473
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_fbc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index f4c3a3b9a8e6..8a597165190d 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -927,14 +927,11 @@ void intel_fbc_pre_update(struct intel_crtc *crtc,
goto deactivate;
}
- if (fbc->crtc != crtc)
- goto unlock;
-
- intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
+ if (fbc->crtc == crtc)
+ intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
deactivate:
intel_fbc_deactivate(dev_priv);
-unlock:
mutex_unlock(&fbc->lock);
}
--
2.15.0.rc2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] drm/i915/fbc: Deactivate fbc when switching pipes
2017-10-27 19:42 ` [PATCH 2/2] drm/i915/fbc: Deactivate fbc when switching pipes Chris Wilson
@ 2017-10-27 20:08 ` Ville Syrjälä
2017-10-27 20:18 ` Chris Wilson
0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2017-10-27 20:08 UTC (permalink / raw)
To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi, Paulo Zanoni
On Fri, Oct 27, 2017 at 08:42:40PM +0100, Chris Wilson wrote:
> If we are transfering an fb from one crtc to another, we will keep FBC
> activated (due to only having a single pipe) but then we will call
> intel_fbc_disable() from intel_atomic_commit_tail() on the old pipe
> before enabling the new pipe. However, we insist that before disabling
> FBC, it is deactivated.
>
> Otherwise we generate warnings such as:
>
> [ 346.741263] WARN_ON(fbc->active)
> [ 346.741308] ------------[ cut here ]------------
> [ 346.741387] WARNING: CPU: 3 PID: 4014 at drivers/gpu/drm/i915/intel_fbc.c:1176 __intel_fbc_disable+0xdf/0x110 [i915]
> [ 346.741394] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core ghash_clmulni_intel snd_pcm e1000e mei_me mei lpc_ich ptp pps_core prime_numbers i2c_hid
> [ 346.741531] CPU: 3 PID: 4014 Comm: kms_frontbuffer Tainted: G U 4.14.0-rc6-CI-Patchwork_6242+ #1
> [ 346.741537] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0362.2017.0118.0940 01/18/2017
> [ 346.741544] task: ffff880236502800 task.stack: ffffc90002874000
> [ 346.741617] RIP: 0010:__intel_fbc_disable+0xdf/0x110 [i915]
> [ 346.741624] RSP: 0018:ffffc900028779c0 EFLAGS: 00010282
> [ 346.741635] RAX: 0000000000000014 RBX: ffff880240df0000 RCX: 0000000000000006
> [ 346.741641] RDX: 000000000000136a RSI: ffffffff81d0e984 RDI: ffffffff81cc2576
> [ 346.741647] RBP: ffffc900028779d0 R08: ffff880236503138 R09: 0000000000000000
> [ 346.741653] R10: ffffc900028779d0 R11: 0000000000000000 R12: ffff880240dec138
> [ 346.741659] R13: ffff880240df4960 R14: ffff880240df0000 R15: ffff880240dec138
> [ 346.741666] FS: 00007fb237b2ca40(0000) GS:ffff880256d80000(0000) knlGS:0000000000000000
> [ 346.741673] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 346.741679] CR2: 00007f9c1805b008 CR3: 0000000236b5f004 CR4: 00000000003606e0
> [ 346.741685] Call Trace:
> [ 346.741762] intel_fbc_disable+0x61/0x70 [i915]
> [ 346.741837] intel_atomic_commit_tail+0x11c/0xbf0 [i915]
> [ 346.741920] intel_atomic_commit+0x223/0x2d0 [i915]
> [ 346.742046] drm_atomic_commit+0x4b/0x50
> [ 346.742128] hsw_pipe_A_crc_wa+0x87/0x180 [i915]
> [ 346.742226] get_new_crc_ctl_reg+0x13d/0x320 [i915]
> [ 346.742304] intel_crtc_set_crc_source+0x7c/0x1d0 [i915]
> [ 346.742329] crtc_crc_open+0xa2/0x2b0
> [ 346.742346] ? rcu_read_lock_sched_held+0x7a/0x90
> [ 346.742359] ? kmem_cache_alloc_trace+0x270/0x2d0
> [ 346.742382] full_proxy_open+0xfd/0x1b0
> [ 346.742401] ? u32_array_release+0x20/0x20
> [ 346.742419] do_dentry_open.isra.1+0x1d3/0x2e0
> [ 346.742440] vfs_open+0x47/0x70
> [ 346.742458] path_openat+0x274/0x990
> [ 346.742489] do_filp_open+0x8a/0xf0
> [ 346.742530] ? _raw_spin_unlock+0x31/0x50
> [ 346.742547] ? __alloc_fd+0xf8/0x210
> [ 346.742573] do_sys_open+0x12f/0x200
> [ 346.742588] ? do_sys_open+0x12f/0x200
> [ 346.742615] SyS_openat+0x14/0x20
> [ 346.742631] entry_SYSCALL_64_fastpath+0x1c/0xb1
> [ 346.742643] RIP: 0033:0x7fb235d1d0fa
> [ 346.742654] RSP: 002b:00007ffd72cd60a0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
> [ 346.742675] RAX: ffffffffffffffda RBX: ffffffff81491ef3 RCX: 00007fb235d1d0fa
> [ 346.742689] RDX: 0000000000000000 RSI: 00007ffd72cd6160 RDI: 0000000000000006
> [ 346.742702] RBP: ffffc90002877f88 R08: 0000000000000000 R09: 000000000000000f
> [ 346.742713] R10: 0000000000000000 R11: 0000000000000246 R12: 000055e1cbb31298
> [ 346.742723] R13: 000055e1cb91aaca R14: 0000000000000001 R15: 000055e1cbce1450
> [ 346.742744] ? __this_cpu_preempt_check+0x13/0x20
> [ 346.742772] Code: 74 4a 2a a0 e8 b4 35 ed e0 0f ff 80 bb a2 4a 00 00 00 0f 84 6f ff ff ff 48 c7 c6 8e 4a 2a a0 48 c7 c7 74 4a 2a a0 e8 92 35 ed e0 <0f> ff 41 80 bc 24 e8 05 00 00 00 0f 84 5a ff ff ff 48 c7 c6 a3
> [ 346.743496] ---[ end trace 5331a8d111243000 ]---
> [ 346.746293] WARN_ON(fbc->active)
> [ 346.746313] ------------[ cut here ]------------
> [ 346.746351] WARNING: CPU: 3 PID: 4014 at drivers/gpu/drm/i915/intel_fbc.c:1144 intel_fbc_enable+0x4b6/0x560 [i915]
> [ 346.746356] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core ghash_clmulni_intel snd_pcm e1000e mei_me mei lpc_ich ptp pps_core prime_numbers i2c_hid
> [ 346.746432] CPU: 3 PID: 4014 Comm: kms_frontbuffer Tainted: G U W 4.14.0-rc6-CI-Patchwork_6242+ #1
> [ 346.746435] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0362.2017.0118.0940 01/18/2017
> [ 346.746438] task: ffff880236502800 task.stack: ffffc90002874000
> [ 346.746474] RIP: 0010:intel_fbc_enable+0x4b6/0x560 [i915]
> [ 346.746478] RSP: 0018:ffffc90002877950 EFLAGS: 00010282
> [ 346.746485] RAX: 0000000000000014 RBX: ffff880240df0000 RCX: 0000000000000006
> [ 346.746488] RDX: 000000000000136a RSI: ffffffff81d0e984 RDI: ffffffff81cc2576
> [ 346.746491] RBP: ffffc900028779a0 R08: ffff880236503138 R09: 0000000000000000
> [ 346.746494] R10: ffffc90002877940 R11: 0000000000000000 R12: ffff88024470c138
> [ 346.746497] R13: ffff8802497fc6f8 R14: ffff88024470a548 R15: ffff880240df0000
> [ 346.746501] FS: 00007fb237b2ca40(0000) GS:ffff880256d80000(0000) knlGS:0000000000000000
> [ 346.746504] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 346.746507] CR2: 00007f9c1805b008 CR3: 0000000236b5f004 CR4: 00000000003606e0
> [ 346.746511] Call Trace:
> [ 346.746551] intel_update_crtc+0x67/0x90 [i915]
> [ 346.746590] intel_update_crtcs+0x5d/0x70 [i915]
> [ 346.746630] intel_atomic_commit_tail+0x286/0xbf0 [i915]
> [ 346.746674] intel_atomic_commit+0x223/0x2d0 [i915]
> [ 346.746684] drm_atomic_commit+0x4b/0x50
> [ 346.746714] hsw_pipe_A_crc_wa+0x87/0x180 [i915]
> [ 346.746750] get_new_crc_ctl_reg+0x13d/0x320 [i915]
> [ 346.746782] intel_crtc_set_crc_source+0x7c/0x1d0 [i915]
> [ 346.746789] crtc_crc_open+0xa2/0x2b0
> [ 346.746795] ? rcu_read_lock_sched_held+0x7a/0x90
> [ 346.746800] ? kmem_cache_alloc_trace+0x270/0x2d0
> [ 346.746808] full_proxy_open+0xfd/0x1b0
> [ 346.746814] ? u32_array_release+0x20/0x20
> [ 346.746820] do_dentry_open.isra.1+0x1d3/0x2e0
> [ 346.746827] vfs_open+0x47/0x70
> [ 346.746833] path_openat+0x274/0x990
> [ 346.746842] do_filp_open+0x8a/0xf0
> [ 346.746854] ? _raw_spin_unlock+0x31/0x50
> [ 346.746860] ? __alloc_fd+0xf8/0x210
> [ 346.746869] do_sys_open+0x12f/0x200
> [ 346.746874] ? do_sys_open+0x12f/0x200
> [ 346.746917] SyS_openat+0x14/0x20
> [ 346.746928] entry_SYSCALL_64_fastpath+0x1c/0xb1
> [ 346.746935] RIP: 0033:0x7fb235d1d0fa
> [ 346.746941] RSP: 002b:00007ffd72cd60a0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
> [ 346.746955] RAX: ffffffffffffffda RBX: ffffffff81491ef3 RCX: 00007fb235d1d0fa
> [ 346.746963] RDX: 0000000000000000 RSI: 00007ffd72cd6160 RDI: 0000000000000006
> [ 346.746969] RBP: ffffc90002877f88 R08: 0000000000000000 R09: 000000000000000f
> [ 346.746975] R10: 0000000000000000 R11: 0000000000000246 R12: 000055e1cbb31298
> [ 346.746981] R13: 000055e1cb91aaca R14: 0000000000000001 R15: 000055e1cbce1450
> [ 346.746993] ? __this_cpu_preempt_check+0x13/0x20
> [ 346.747008] Code: c6 10 1c 2c a0 48 c7 c7 74 4a 2a a0 e8 75 1e ed e0 0f ff e9 e3 fb ff ff 48 c7 c6 8e 4a 2a a0 48 c7 c7 74 4a 2a a0 e8 5b 1e ed e0 <0f> ff e9 bb fb ff ff 49 8b 94 24 00 4a 00 00 41 03 94 24 10 62
> [ 346.747357] ---[ end trace 5331a8d111243001 ]---
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102473
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index f4c3a3b9a8e6..8a597165190d 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -927,14 +927,11 @@ void intel_fbc_pre_update(struct intel_crtc *crtc,
> goto deactivate;
> }
>
> - if (fbc->crtc != crtc)
> - goto unlock;
> -
> - intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
> + if (fbc->crtc == crtc)
> + intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
Hmm. We shouldn't disable fbc when there's a plane update on another
pipe. Looks like this patch would cause that to happen.
The order in which the fbc functions get called by the modeset code
doesn't make much sense. Seems to me that intel_fbc_disable() should be
called before we shut down the plane/pipe so that FBC won't try to
re-enable at the wrong time.
And the intel_fbc_enable() call happens before we've turned
on/reconfigured the plane, which also seems like it could end up
allowing FBC to be enabled before the plane is ready.
>
> deactivate:
> intel_fbc_deactivate(dev_priv);
> -unlock:
> mutex_unlock(&fbc->lock);
> }
>
> --
> 2.15.0.rc2
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/i915/fbc: Deactivate fbc when switching pipes
2017-10-27 20:08 ` Ville Syrjälä
@ 2017-10-27 20:18 ` Chris Wilson
0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-10-27 20:18 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi, Paulo Zanoni
Quoting Ville Syrjälä (2017-10-27 21:08:55)
> On Fri, Oct 27, 2017 at 08:42:40PM +0100, Chris Wilson wrote:
> > If we are transfering an fb from one crtc to another, we will keep FBC
> > activated (due to only having a single pipe) but then we will call
> > intel_fbc_disable() from intel_atomic_commit_tail() on the old pipe
> > before enabling the new pipe. However, we insist that before disabling
> > FBC, it is deactivated.
> >
> > Otherwise we generate warnings such as:
> >
> > [ 346.741263] WARN_ON(fbc->active)
> > [ 346.741308] ------------[ cut here ]------------
> > [ 346.741387] WARNING: CPU: 3 PID: 4014 at drivers/gpu/drm/i915/intel_fbc.c:1176 __intel_fbc_disable+0xdf/0x110 [i915]
> > [ 346.741394] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core ghash_clmulni_intel snd_pcm e1000e mei_me mei lpc_ich ptp pps_core prime_numbers i2c_hid
> > [ 346.741531] CPU: 3 PID: 4014 Comm: kms_frontbuffer Tainted: G U 4.14.0-rc6-CI-Patchwork_6242+ #1
> > [ 346.741537] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0362.2017.0118.0940 01/18/2017
> > [ 346.741544] task: ffff880236502800 task.stack: ffffc90002874000
> > [ 346.741617] RIP: 0010:__intel_fbc_disable+0xdf/0x110 [i915]
> > [ 346.741624] RSP: 0018:ffffc900028779c0 EFLAGS: 00010282
> > [ 346.741635] RAX: 0000000000000014 RBX: ffff880240df0000 RCX: 0000000000000006
> > [ 346.741641] RDX: 000000000000136a RSI: ffffffff81d0e984 RDI: ffffffff81cc2576
> > [ 346.741647] RBP: ffffc900028779d0 R08: ffff880236503138 R09: 0000000000000000
> > [ 346.741653] R10: ffffc900028779d0 R11: 0000000000000000 R12: ffff880240dec138
> > [ 346.741659] R13: ffff880240df4960 R14: ffff880240df0000 R15: ffff880240dec138
> > [ 346.741666] FS: 00007fb237b2ca40(0000) GS:ffff880256d80000(0000) knlGS:0000000000000000
> > [ 346.741673] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 346.741679] CR2: 00007f9c1805b008 CR3: 0000000236b5f004 CR4: 00000000003606e0
> > [ 346.741685] Call Trace:
> > [ 346.741762] intel_fbc_disable+0x61/0x70 [i915]
> > [ 346.741837] intel_atomic_commit_tail+0x11c/0xbf0 [i915]
> > [ 346.741920] intel_atomic_commit+0x223/0x2d0 [i915]
> > [ 346.742046] drm_atomic_commit+0x4b/0x50
> > [ 346.742128] hsw_pipe_A_crc_wa+0x87/0x180 [i915]
> > [ 346.742226] get_new_crc_ctl_reg+0x13d/0x320 [i915]
> > [ 346.742304] intel_crtc_set_crc_source+0x7c/0x1d0 [i915]
> > [ 346.742329] crtc_crc_open+0xa2/0x2b0
> > [ 346.742346] ? rcu_read_lock_sched_held+0x7a/0x90
> > [ 346.742359] ? kmem_cache_alloc_trace+0x270/0x2d0
> > [ 346.742382] full_proxy_open+0xfd/0x1b0
> > [ 346.742401] ? u32_array_release+0x20/0x20
> > [ 346.742419] do_dentry_open.isra.1+0x1d3/0x2e0
> > [ 346.742440] vfs_open+0x47/0x70
> > [ 346.742458] path_openat+0x274/0x990
> > [ 346.742489] do_filp_open+0x8a/0xf0
> > [ 346.742530] ? _raw_spin_unlock+0x31/0x50
> > [ 346.742547] ? __alloc_fd+0xf8/0x210
> > [ 346.742573] do_sys_open+0x12f/0x200
> > [ 346.742588] ? do_sys_open+0x12f/0x200
> > [ 346.742615] SyS_openat+0x14/0x20
> > [ 346.742631] entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [ 346.742643] RIP: 0033:0x7fb235d1d0fa
> > [ 346.742654] RSP: 002b:00007ffd72cd60a0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
> > [ 346.742675] RAX: ffffffffffffffda RBX: ffffffff81491ef3 RCX: 00007fb235d1d0fa
> > [ 346.742689] RDX: 0000000000000000 RSI: 00007ffd72cd6160 RDI: 0000000000000006
> > [ 346.742702] RBP: ffffc90002877f88 R08: 0000000000000000 R09: 000000000000000f
> > [ 346.742713] R10: 0000000000000000 R11: 0000000000000246 R12: 000055e1cbb31298
> > [ 346.742723] R13: 000055e1cb91aaca R14: 0000000000000001 R15: 000055e1cbce1450
> > [ 346.742744] ? __this_cpu_preempt_check+0x13/0x20
> > [ 346.742772] Code: 74 4a 2a a0 e8 b4 35 ed e0 0f ff 80 bb a2 4a 00 00 00 0f 84 6f ff ff ff 48 c7 c6 8e 4a 2a a0 48 c7 c7 74 4a 2a a0 e8 92 35 ed e0 <0f> ff 41 80 bc 24 e8 05 00 00 00 0f 84 5a ff ff ff 48 c7 c6 a3
> > [ 346.743496] ---[ end trace 5331a8d111243000 ]---
> > [ 346.746293] WARN_ON(fbc->active)
> > [ 346.746313] ------------[ cut here ]------------
> > [ 346.746351] WARNING: CPU: 3 PID: 4014 at drivers/gpu/drm/i915/intel_fbc.c:1144 intel_fbc_enable+0x4b6/0x560 [i915]
> > [ 346.746356] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core ghash_clmulni_intel snd_pcm e1000e mei_me mei lpc_ich ptp pps_core prime_numbers i2c_hid
> > [ 346.746432] CPU: 3 PID: 4014 Comm: kms_frontbuffer Tainted: G U W 4.14.0-rc6-CI-Patchwork_6242+ #1
> > [ 346.746435] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0362.2017.0118.0940 01/18/2017
> > [ 346.746438] task: ffff880236502800 task.stack: ffffc90002874000
> > [ 346.746474] RIP: 0010:intel_fbc_enable+0x4b6/0x560 [i915]
> > [ 346.746478] RSP: 0018:ffffc90002877950 EFLAGS: 00010282
> > [ 346.746485] RAX: 0000000000000014 RBX: ffff880240df0000 RCX: 0000000000000006
> > [ 346.746488] RDX: 000000000000136a RSI: ffffffff81d0e984 RDI: ffffffff81cc2576
> > [ 346.746491] RBP: ffffc900028779a0 R08: ffff880236503138 R09: 0000000000000000
> > [ 346.746494] R10: ffffc90002877940 R11: 0000000000000000 R12: ffff88024470c138
> > [ 346.746497] R13: ffff8802497fc6f8 R14: ffff88024470a548 R15: ffff880240df0000
> > [ 346.746501] FS: 00007fb237b2ca40(0000) GS:ffff880256d80000(0000) knlGS:0000000000000000
> > [ 346.746504] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 346.746507] CR2: 00007f9c1805b008 CR3: 0000000236b5f004 CR4: 00000000003606e0
> > [ 346.746511] Call Trace:
> > [ 346.746551] intel_update_crtc+0x67/0x90 [i915]
> > [ 346.746590] intel_update_crtcs+0x5d/0x70 [i915]
> > [ 346.746630] intel_atomic_commit_tail+0x286/0xbf0 [i915]
> > [ 346.746674] intel_atomic_commit+0x223/0x2d0 [i915]
> > [ 346.746684] drm_atomic_commit+0x4b/0x50
> > [ 346.746714] hsw_pipe_A_crc_wa+0x87/0x180 [i915]
> > [ 346.746750] get_new_crc_ctl_reg+0x13d/0x320 [i915]
> > [ 346.746782] intel_crtc_set_crc_source+0x7c/0x1d0 [i915]
> > [ 346.746789] crtc_crc_open+0xa2/0x2b0
> > [ 346.746795] ? rcu_read_lock_sched_held+0x7a/0x90
> > [ 346.746800] ? kmem_cache_alloc_trace+0x270/0x2d0
> > [ 346.746808] full_proxy_open+0xfd/0x1b0
> > [ 346.746814] ? u32_array_release+0x20/0x20
> > [ 346.746820] do_dentry_open.isra.1+0x1d3/0x2e0
> > [ 346.746827] vfs_open+0x47/0x70
> > [ 346.746833] path_openat+0x274/0x990
> > [ 346.746842] do_filp_open+0x8a/0xf0
> > [ 346.746854] ? _raw_spin_unlock+0x31/0x50
> > [ 346.746860] ? __alloc_fd+0xf8/0x210
> > [ 346.746869] do_sys_open+0x12f/0x200
> > [ 346.746874] ? do_sys_open+0x12f/0x200
> > [ 346.746917] SyS_openat+0x14/0x20
> > [ 346.746928] entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [ 346.746935] RIP: 0033:0x7fb235d1d0fa
> > [ 346.746941] RSP: 002b:00007ffd72cd60a0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
> > [ 346.746955] RAX: ffffffffffffffda RBX: ffffffff81491ef3 RCX: 00007fb235d1d0fa
> > [ 346.746963] RDX: 0000000000000000 RSI: 00007ffd72cd6160 RDI: 0000000000000006
> > [ 346.746969] RBP: ffffc90002877f88 R08: 0000000000000000 R09: 000000000000000f
> > [ 346.746975] R10: 0000000000000000 R11: 0000000000000246 R12: 000055e1cbb31298
> > [ 346.746981] R13: 000055e1cb91aaca R14: 0000000000000001 R15: 000055e1cbce1450
> > [ 346.746993] ? __this_cpu_preempt_check+0x13/0x20
> > [ 346.747008] Code: c6 10 1c 2c a0 48 c7 c7 74 4a 2a a0 e8 75 1e ed e0 0f ff e9 e3 fb ff ff 48 c7 c6 8e 4a 2a a0 48 c7 c7 74 4a 2a a0 e8 5b 1e ed e0 <0f> ff e9 bb fb ff ff 49 8b 94 24 00 4a 00 00 41 03 94 24 10 62
> > [ 346.747357] ---[ end trace 5331a8d111243001 ]---
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102473
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_fbc.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index f4c3a3b9a8e6..8a597165190d 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -927,14 +927,11 @@ void intel_fbc_pre_update(struct intel_crtc *crtc,
> > goto deactivate;
> > }
> >
> > - if (fbc->crtc != crtc)
> > - goto unlock;
> > -
> > - intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
> > + if (fbc->crtc == crtc)
> > + intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
>
> Hmm. We shouldn't disable fbc when there's a plane update on another
> pipe. Looks like this patch would cause that to happen.
>
> The order in which the fbc functions get called by the modeset code
> doesn't make much sense. Seems to me that intel_fbc_disable() should be
> called before we shut down the plane/pipe so that FBC won't try to
> re-enable at the wrong time.
>
> And the intel_fbc_enable() call happens before we've turned
> on/reconfigured the plane, which also seems like it could end up
> allowing FBC to be enabled before the plane is ready.
I also couldn't work out what the cache is doing here either, and why
the deactivate in prepare long before the commit. atomic would be that
you have the current fbc_state attached to the old_state, and in prepare
you capture the new state, then in commit you update the hw for the
changes in state.
Who was last making noises about integrating atomic + fbc?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
2017-10-27 19:42 [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous Chris Wilson
2017-10-27 19:42 ` [PATCH 2/2] drm/i915/fbc: Deactivate fbc when switching pipes Chris Wilson
@ 2017-10-27 19:56 ` Ville Syrjälä
2017-10-27 20:04 ` Chris Wilson
2017-10-27 21:06 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2017-10-27 19:56 UTC (permalink / raw)
To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi
On Fri, Oct 27, 2017 at 08:42:39PM +0100, Chris Wilson wrote:
> Slightly reduce the chance for confusion by only having one variable
> instead of two to tell us when intel_fbc is enabled on a crtc.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 -
> drivers/gpu/drm/i915/intel_fbc.c | 33 +++++++++++++--------------------
> 2 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 61c155cbf9d7..c0f8a13ec957 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1083,7 +1083,6 @@ struct intel_fbc {
>
> bool false_color;
>
> - bool enabled;
> bool active;
>
> bool underrun_detected;
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 1a0f5e0c8d10..f4c3a3b9a8e6 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -472,7 +472,7 @@ static void intel_fbc_schedule_activation(struct intel_crtc *crtc)
> struct intel_fbc_work *work = &fbc->work;
>
> WARN_ON(!mutex_is_locked(&fbc->lock));
> - if (WARN_ON(!fbc->enabled))
> + if (WARN_ON(fbc->crtc != crtc))
> return;
I was a bit worried that this might trip needlessly, but looks like
__intel_fbc_post_update() should keep that from happening.
>
> if (drm_crtc_vblank_get(&crtc->base)) {
> @@ -927,7 +927,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc,
> goto deactivate;
> }
>
> - if (!fbc->enabled || fbc->crtc != crtc)
> + if (fbc->crtc != crtc)
> goto unlock;
>
> intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
> @@ -946,7 +946,7 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc)
>
> WARN_ON(!mutex_is_locked(&fbc->lock));
>
> - if (!fbc->enabled || fbc->crtc != crtc)
> + if (fbc->crtc != crtc)
> return;
>
> if (!intel_fbc_can_activate(crtc)) {
> @@ -986,7 +986,7 @@ void intel_fbc_post_update(struct intel_crtc *crtc)
>
> static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc)
> {
> - if (fbc->enabled)
> + if (fbc->crtc)
> return to_intel_plane(fbc->crtc->base.primary)->frontbuffer_bit;
> else
> return fbc->possible_framebuffer_bits;
> @@ -1008,7 +1008,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
>
> fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits;
>
> - if (fbc->enabled && fbc->busy_bits)
> + if (fbc->crtc && fbc->busy_bits)
> intel_fbc_deactivate(dev_priv);
>
> mutex_unlock(&fbc->lock);
> @@ -1029,7 +1029,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
> if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
> goto out;
>
> - if (!fbc->busy_bits && fbc->enabled &&
> + if (!fbc->busy_bits && fbc->crtc &&
> (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) {
> if (fbc->active)
> intel_fbc_recompress(dev_priv);
> @@ -1129,20 +1129,17 @@ void intel_fbc_enable(struct intel_crtc *crtc,
>
> mutex_lock(&fbc->lock);
>
> - if (fbc->enabled) {
> - WARN_ON(fbc->crtc == NULL);
> - if (fbc->crtc == crtc) {
> - WARN_ON(!crtc_state->enable_fbc);
> - WARN_ON(fbc->active);
> - }
> - goto out;
> + if (fbc->crtc == crtc) {
> + WARN_ON(!crtc_state->enable_fbc);
> + WARN_ON(fbc->active);
> }
> + if (fbc->crtc)
> + goto out;
>
> if (!crtc_state->enable_fbc)
> goto out;
>
> WARN_ON(fbc->active);
> - WARN_ON(fbc->crtc != NULL);
>
> intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
> if (intel_fbc_alloc_cfb(crtc)) {
> @@ -1153,7 +1150,6 @@ void intel_fbc_enable(struct intel_crtc *crtc,
> DRM_DEBUG_KMS("Enabling FBC on pipe %c\n", pipe_name(crtc->pipe));
> fbc->no_fbc_reason = "FBC enabled but not active yet\n";
>
> - fbc->enabled = true;
> fbc->crtc = crtc;
> out:
> mutex_unlock(&fbc->lock);
> @@ -1172,7 +1168,6 @@ static void __intel_fbc_disable(struct drm_i915_private *dev_priv)
> struct intel_crtc *crtc = fbc->crtc;
>
> WARN_ON(!mutex_is_locked(&fbc->lock));
> - WARN_ON(!fbc->enabled);
WARN(!crtc) maybe? Though the callers are pretty clear, so not sure it's
even worth it.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> WARN_ON(fbc->active);
> WARN_ON(crtc->active);
>
> @@ -1180,7 +1175,6 @@ static void __intel_fbc_disable(struct drm_i915_private *dev_priv)
>
> __intel_fbc_cleanup_cfb(dev_priv);
>
> - fbc->enabled = false;
> fbc->crtc = NULL;
> }
>
> @@ -1220,7 +1214,7 @@ void intel_fbc_global_disable(struct drm_i915_private *dev_priv)
> return;
>
> mutex_lock(&fbc->lock);
> - if (fbc->enabled)
> + if (fbc->crtc)
> __intel_fbc_disable(dev_priv);
> mutex_unlock(&fbc->lock);
>
> @@ -1236,7 +1230,7 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work)
> mutex_lock(&fbc->lock);
>
> /* Maybe we were scheduled twice. */
> - if (fbc->underrun_detected || !fbc->enabled)
> + if (fbc->underrun_detected || !fbc->crtc)
> goto out;
>
> DRM_DEBUG_KMS("Disabling FBC due to FIFO underrun.\n");
> @@ -1351,7 +1345,6 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
> INIT_WORK(&fbc->work.work, intel_fbc_work_fn);
> INIT_WORK(&fbc->underrun_work, intel_fbc_underrun_work_fn);
> mutex_init(&fbc->lock);
> - fbc->enabled = false;
> fbc->active = false;
> fbc->work.scheduled = false;
>
> --
> 2.15.0.rc2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
2017-10-27 19:56 ` [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous Ville Syrjälä
@ 2017-10-27 20:04 ` Chris Wilson
0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-10-27 20:04 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi
Quoting Ville Syrjälä (2017-10-27 20:56:44)
> On Fri, Oct 27, 2017 at 08:42:39PM +0100, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index 1a0f5e0c8d10..f4c3a3b9a8e6 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -472,7 +472,7 @@ static void intel_fbc_schedule_activation(struct intel_crtc *crtc)
> > struct intel_fbc_work *work = &fbc->work;
> >
> > WARN_ON(!mutex_is_locked(&fbc->lock));
> > - if (WARN_ON(!fbc->enabled))
> > + if (WARN_ON(fbc->crtc != crtc))
> > return;
>
> I was a bit worried that this might trip needlessly, but looks like
> __intel_fbc_post_update() should keep that from happening.
Yeah, I was actually wondering if we might trip this and explain a few
of the oddities. :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
2017-10-27 19:42 [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous Chris Wilson
2017-10-27 19:42 ` [PATCH 2/2] drm/i915/fbc: Deactivate fbc when switching pipes Chris Wilson
2017-10-27 19:56 ` [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous Ville Syrjälä
@ 2017-10-27 21:06 ` Patchwork
2017-10-27 22:11 ` ✓ Fi.CI.IGT: " Patchwork
2017-10-30 15:50 ` [PATCH 1/2] " Jani Nikula
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-10-27 21:06 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
URL : https://patchwork.freedesktop.org/series/32784/
State : success
== Summary ==
Series 32784v1 series starting with [1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
https://patchwork.freedesktop.org/api/1.0/series/32784/revisions/1/mbox/
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:441s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:457s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:375s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:535s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:266s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:501s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:495s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:493s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:469s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:606s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:420s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:249s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:577s
fi-glk-dsi total:289 pass:258 dwarn:0 dfail:0 fail:1 skip:30 time:485s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:425s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:427s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:424s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:485s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:460s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:491s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:572s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:476s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:582s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:543s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:455s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:587s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:645s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:522s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:498s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:458s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:564s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:418s
d0582552491e17f5e386747f82147c1f2d9158c9 drm-tip: 2017y-10m-27d-19h-16m-21s UTC integration manifest
8431cccb3701 drm/i915/fbc: Deactivate fbc when switching pipes
f0ec5e5ad52a drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6249/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
2017-10-27 19:42 [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous Chris Wilson
` (2 preceding siblings ...)
2017-10-27 21:06 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
@ 2017-10-27 22:11 ` Patchwork
2017-10-30 15:50 ` [PATCH 1/2] " Jani Nikula
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-10-27 22:11 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
URL : https://patchwork.freedesktop.org/series/32784/
State : success
== Summary ==
Test perf:
Subgroup oa-exponents:
fail -> PASS (shard-hsw) fdo#102254
Subgroup blocking:
pass -> FAIL (shard-hsw) fdo#102252
Test kms_flip:
Subgroup modeset-vs-vblank-race-interruptible:
fail -> PASS (shard-hsw) fdo#103060 +1
Test kms_busy:
Subgroup extended-modeset-hang-newfb-with-reset-render-A:
pass -> DMESG-WARN (shard-hsw) fdo#102249
Test drv_suspend:
Subgroup fence-restore-untiled:
skip -> PASS (shard-hsw)
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
shard-hsw total:2539 pass:1431 dwarn:1 dfail:0 fail:10 skip:1097 time:9219s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6249/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous
2017-10-27 19:42 [PATCH 1/2] drm/i915/fbc: intel_fbc.crtc and intel_fbc.enabled are synonymous Chris Wilson
` (3 preceding siblings ...)
2017-10-27 22:11 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-10-30 15:50 ` Jani Nikula
4 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2017-10-30 15:50 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi
On Fri, 27 Oct 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Slightly reduce the chance for confusion by only having one variable
> instead of two to tell us when intel_fbc is enabled on a crtc.
Would be great to have a simple comment within struct intel_fbc stating
this.
BR,
Jani.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 -
> drivers/gpu/drm/i915/intel_fbc.c | 33 +++++++++++++--------------------
> 2 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 61c155cbf9d7..c0f8a13ec957 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1083,7 +1083,6 @@ struct intel_fbc {
>
> bool false_color;
>
> - bool enabled;
> bool active;
>
> bool underrun_detected;
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 1a0f5e0c8d10..f4c3a3b9a8e6 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -472,7 +472,7 @@ static void intel_fbc_schedule_activation(struct intel_crtc *crtc)
> struct intel_fbc_work *work = &fbc->work;
>
> WARN_ON(!mutex_is_locked(&fbc->lock));
> - if (WARN_ON(!fbc->enabled))
> + if (WARN_ON(fbc->crtc != crtc))
> return;
>
> if (drm_crtc_vblank_get(&crtc->base)) {
> @@ -927,7 +927,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc,
> goto deactivate;
> }
>
> - if (!fbc->enabled || fbc->crtc != crtc)
> + if (fbc->crtc != crtc)
> goto unlock;
>
> intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
> @@ -946,7 +946,7 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc)
>
> WARN_ON(!mutex_is_locked(&fbc->lock));
>
> - if (!fbc->enabled || fbc->crtc != crtc)
> + if (fbc->crtc != crtc)
> return;
>
> if (!intel_fbc_can_activate(crtc)) {
> @@ -986,7 +986,7 @@ void intel_fbc_post_update(struct intel_crtc *crtc)
>
> static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc)
> {
> - if (fbc->enabled)
> + if (fbc->crtc)
> return to_intel_plane(fbc->crtc->base.primary)->frontbuffer_bit;
> else
> return fbc->possible_framebuffer_bits;
> @@ -1008,7 +1008,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
>
> fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits;
>
> - if (fbc->enabled && fbc->busy_bits)
> + if (fbc->crtc && fbc->busy_bits)
> intel_fbc_deactivate(dev_priv);
>
> mutex_unlock(&fbc->lock);
> @@ -1029,7 +1029,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
> if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
> goto out;
>
> - if (!fbc->busy_bits && fbc->enabled &&
> + if (!fbc->busy_bits && fbc->crtc &&
> (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) {
> if (fbc->active)
> intel_fbc_recompress(dev_priv);
> @@ -1129,20 +1129,17 @@ void intel_fbc_enable(struct intel_crtc *crtc,
>
> mutex_lock(&fbc->lock);
>
> - if (fbc->enabled) {
> - WARN_ON(fbc->crtc == NULL);
> - if (fbc->crtc == crtc) {
> - WARN_ON(!crtc_state->enable_fbc);
> - WARN_ON(fbc->active);
> - }
> - goto out;
> + if (fbc->crtc == crtc) {
> + WARN_ON(!crtc_state->enable_fbc);
> + WARN_ON(fbc->active);
> }
> + if (fbc->crtc)
> + goto out;
>
> if (!crtc_state->enable_fbc)
> goto out;
>
> WARN_ON(fbc->active);
> - WARN_ON(fbc->crtc != NULL);
>
> intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
> if (intel_fbc_alloc_cfb(crtc)) {
> @@ -1153,7 +1150,6 @@ void intel_fbc_enable(struct intel_crtc *crtc,
> DRM_DEBUG_KMS("Enabling FBC on pipe %c\n", pipe_name(crtc->pipe));
> fbc->no_fbc_reason = "FBC enabled but not active yet\n";
>
> - fbc->enabled = true;
> fbc->crtc = crtc;
> out:
> mutex_unlock(&fbc->lock);
> @@ -1172,7 +1168,6 @@ static void __intel_fbc_disable(struct drm_i915_private *dev_priv)
> struct intel_crtc *crtc = fbc->crtc;
>
> WARN_ON(!mutex_is_locked(&fbc->lock));
> - WARN_ON(!fbc->enabled);
> WARN_ON(fbc->active);
> WARN_ON(crtc->active);
>
> @@ -1180,7 +1175,6 @@ static void __intel_fbc_disable(struct drm_i915_private *dev_priv)
>
> __intel_fbc_cleanup_cfb(dev_priv);
>
> - fbc->enabled = false;
> fbc->crtc = NULL;
> }
>
> @@ -1220,7 +1214,7 @@ void intel_fbc_global_disable(struct drm_i915_private *dev_priv)
> return;
>
> mutex_lock(&fbc->lock);
> - if (fbc->enabled)
> + if (fbc->crtc)
> __intel_fbc_disable(dev_priv);
> mutex_unlock(&fbc->lock);
>
> @@ -1236,7 +1230,7 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work)
> mutex_lock(&fbc->lock);
>
> /* Maybe we were scheduled twice. */
> - if (fbc->underrun_detected || !fbc->enabled)
> + if (fbc->underrun_detected || !fbc->crtc)
> goto out;
>
> DRM_DEBUG_KMS("Disabling FBC due to FIFO underrun.\n");
> @@ -1351,7 +1345,6 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
> INIT_WORK(&fbc->work.work, intel_fbc_work_fn);
> INIT_WORK(&fbc->underrun_work, intel_fbc_underrun_work_fn);
> mutex_init(&fbc->lock);
> - fbc->enabled = false;
> fbc->active = false;
> fbc->work.scheduled = false;
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread