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@xxxxxxxxxxxxxxxxxx> > > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@xxxxxxxxx> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx