On Mon, Jan 16, 2017 at 12:00:46PM +0100, Maarten Lankhorst wrote: > Op 11-01-17 om 17:13 schreef Daniel Vetter: > > On Thu, Dec 15, 2016 at 03:29:43PM +0100, Maarten Lankhorst wrote: > >> If the crtc was brought up with audio before the driver loads, > >> then crtc_disable will remove a refcount to audio that doesn't exist > >> before. > >> > >> Fortunately we already set power domains on readout, so we can just add > >> the power domain handling to get_crtc_power_domains, which will update > >> the power domains correctly in all cases. > >> > >> This was found when testing module reload on CI with the crtc enabled, > >> which resulted in the following warn after module reload + modeset: > >> > >> [ 24.197041] ------------[ cut here ]------------ > >> [ 24.197075] WARNING: CPU: 0 PID: 99 at drivers/gpu/drm/i915/intel_runtime_pm.c:1790 intel_display_power_put+0x134/0x140 [i915] > >> [ 24.197076] Use count on domain AUDIO is already zero > >> [ 24.197098] CPU: 0 PID: 99 Comm: kworker/u8:2 Not tainted 4.9.0-CI-Trybot_393+ #1 > >> [ 24.197099] Hardware name: /NUC6i5SYB, BIOS SYSKLi35.86A.0042.2016.0409.1246 04/09/2016 > >> [ 24.197102] Workqueue: events_unbound async_run_entry_fn > >> [ 24.197105] ffffc900003c7688 ffffffff81435b35 ffffc900003c76d8 0000000000000000 > >> [ 24.197107] ffffc900003c76c8 ffffffff8107e4d6 000006fe5dc36f28 ffff88025dc30054 > >> [ 24.197109] ffff88025dc36f28 ffff88025dc30000 ffff88025dc30000 0000000000000015 > >> [ 24.197110] Call Trace: > >> [ 24.197113] [<ffffffff81435b35>] dump_stack+0x67/0x92 > >> [ 24.197116] [<ffffffff8107e4d6>] __warn+0xc6/0xe0 > >> [ 24.197118] [<ffffffff8107e53a>] warn_slowpath_fmt+0x4a/0x50 > >> [ 24.197149] [<ffffffffa039b4b4>] intel_display_power_put+0x134/0x140 [i915] > >> [ 24.197187] [<ffffffffa04217dd>] intel_disable_ddi+0x4d/0x80 [i915] > >> [ 24.197223] [<ffffffffa03f388f>] intel_encoders_disable.isra.74+0x7f/0x90 [i915] > >> [ 24.197257] [<ffffffffa03f6c05>] haswell_crtc_disable+0x55/0x170 [i915] > >> [ 24.197292] [<ffffffffa03fec88>] intel_atomic_commit_tail+0x108/0xfd0 [i915] > >> [ 24.197295] [<ffffffff810d47c6>] ? __lock_is_held+0x66/0x90 > >> [ 24.197330] [<ffffffffa03fff79>] intel_atomic_commit+0x429/0x560 [i915] > >> [ 24.197332] [<ffffffff81570186>] ?drm_atomic_add_affected_connectors+0x56/0xf0 > >> [ 24.197334] [<ffffffff8156f726>] drm_atomic_commit+0x46/0x50 > >> [ 24.197336] [<ffffffff81553f87>] restore_fbdev_mode+0x147/0x270 > >> [ 24.197337] [<ffffffff81555bee>] drm_fb_helper_restore_fbdev_mode_unlocked+0x2e/0x70 > >> [ 24.197339] [<ffffffff81555aa8>] drm_fb_helper_set_par+0x28/0x50 > >> [ 24.197374] [<ffffffffa041c7d3>] intel_fbdev_set_par+0x13/0x70 [i915] > >> [ 24.197376] [<ffffffff8149e07a>] fbcon_init+0x57a/0x600 > >> [ 24.197379] [<ffffffff81514b71>] visual_init+0xd1/0x130 > >> [ 24.197381] [<ffffffff8151603c>] do_bind_con_driver+0x1bc/0x3a0 > >> [ 24.197384] [<ffffffff81516521>] do_take_over_console+0x111/0x180 > >> [ 24.197386] [<ffffffff8149e152>] do_fbcon_takeover+0x52/0xb0 > >> [ 24.197387] [<ffffffff814a12c3>] fbcon_event_notify+0x723/0x850 > >> [ 24.197390] [<ffffffff810a4830>] ?__blocking_notifier_call_chain+0x30/0x70 > >> [ 24.197392] [<ffffffff810a44a4>] notifier_call_chain+0x34/0xa0 > >> [ 24.197394] [<ffffffff810a4848>] __blocking_notifier_call_chain+0x48/0x70 > >> [ 24.197397] [<ffffffff810a4881>] blocking_notifier_call_chain+0x11/0x20 > >> [ 24.197398] [<ffffffff814a4556>] fb_notifier_call_chain+0x16/0x20 > >> [ 24.197400] [<ffffffff814a678c>] register_framebuffer+0x24c/0x330 > >> [ 24.197402] [<ffffffff815558d9>] drm_fb_helper_initial_config+0x219/0x3c0 > >> [ 24.197436] [<ffffffffa041d373>] intel_fbdev_initial_config+0x13/0x30 [i915] > >> [ 24.197438] [<ffffffff810a5d44>] async_run_entry_fn+0x34/0x140 > >> [ 24.197440] [<ffffffff8109c26c>] process_one_work+0x1ec/0x6b0 > >> [ 24.197442] [<ffffffff8109c1e6>] ? process_one_work+0x166/0x6b0 > >> [ 24.197445] [<ffffffff8109c779>] worker_thread+0x49/0x490 > >> [ 24.197447] [<ffffffff8109c730>] ? process_one_work+0x6b0/0x6b0 > >> [ 24.197448] [<ffffffff810a2a9b>] kthread+0xeb/0x110 > >> [ 24.197451] [<ffffffff810a29b0>] ? kthread_park+0x60/0x60 > >> [ 24.197453] [<ffffffff818241a7>] ret_from_fork+0x27/0x40 > >> [ 24.197476] ---[ end trace bda64b683b8e8162 ]--- > >> > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > Do we still need this with patch 3? I know it'd be nice if we could > > faithfully restore any state we can also program, but then that's also a > > lot of complexity ... > > > > Otoh patch 3 means we'll stop testing a lot of the fastboot code while > > reloading the driver. But then that's been the thing in the past, and as > > long as we still boot up we have at least some test coverage fo the > > fastboot code (I'm mostly concerned about the plane/buffer readout code, > > since that's not covered by the state checker). > > > > But for now I'd say let's just go with patch 3 only. > We don't need this patch, but it makes the audio power domain tracked > like all other power domains. It's a nice cleanup in general and I would > like it even without the rest of the series. Could otherwise be merged > through the intel tree through. Hm, count me convinced. Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
