[Intel-gfx] [PATCH 2/2] drm/i915/fbc: Deactivate fbc when switching pipes
Chris Wilson
chris at chris-wilson.co.uk
Fri Oct 27 20:18:20 UTC 2017
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 at chris-wilson.co.uk>
> > Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Cc: Paulo Zanoni <paulo.r.zanoni at 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
More information about the Intel-gfx
mailing list