On Wed, Jan 16, 2019 at 3:06 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > Hi, I am using drm_simple_kms_helper and writing a new driver. > > The code is here: > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git/log/?h=ux500-mcde > It's starting to look acceptable, but I just don't wanna post the driver > until I get a clean probe. > > This is using a command mode-only DSI panel, which naturally > will not start generating any vblank interrups all by itself: you > tell it explicitly to scan out a frame. > > However I have set it up to scan out frames continously and > generate interrupts continously at each completed frame if > struct drm_simple_display_pipe_funcs > .enable_vblank() is called, and stop it if .disable_vblank() > is called. > > This works mostly: graphics come out on the display and > are then continously updated. Some interrupts come out > too: > > 77: 569 0 GIC-0 80 Level mcde > > As expected with framebuffer emulation, there is some vblank > use when setting it up, then it turns vblanks off again and > just count on the display being continously streamed out. > > But I get a snag when starting up: > > mcde a0350000.mcde: mcde_display_enable_vblank > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 1 at ../drivers/gpu/drm/drm_atomic_helper.c:1424 > drm_atomic_helper_wait_for_vblanks.part.1+0x29c/0x2a0 > [CRTC:34:crtc-0] vblank wait timed out > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 5.0.0-rc1-00005-ga1691ef3e833-dirty #487 > Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support) > (unwind_backtrace) from [<c010ba88>] (show_stack+0x10/0x14) > (show_stack) from [<c0709020>] (dump_stack+0x88/0x9c) > (dump_stack) from [<c011cd48>] (__warn+0xdc/0xf4) > (__warn) from [<c011cda8>] (warn_slowpath_fmt+0x48/0x6c) > (warn_slowpath_fmt) from [<c03eab04>] > (drm_atomic_helper_wait_for_vblanks.part.1+0x29c/0x2a0) > (drm_atomic_helper_wait_for_vblanks.part.1) from [<c03ec3fc>] > (drm_atomic_helper_commit_tail+0x5c/0x6c) > (drm_atomic_helper_commit_tail) from [<c03ec474>] (commit_tail+0x68/0x6c) > (commit_tail) from [<c03ec53c>] (drm_atomic_helper_commit+0xbc/0x128) > (drm_atomic_helper_commit) from [<c03ef23c>] > (restore_fbdev_mode_atomic+0x1c8/0x1d8) > (restore_fbdev_mode_atomic) from [<c03f2c6c>] > (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0) > (drm_fb_helper_restore_fbdev_mode_unlocked) from [<c03f2ce8>] > (drm_fb_helper_set_par+0x30/0x54) > (drm_fb_helper_set_par) from [<c0393ec8>] (fbcon_init+0x464/0x574) > (fbcon_init) from [<c03cd4e8>] (visual_init+0xbc/0x104) > (visual_init) from [<c03cf344>] (do_bind_con_driver+0x1e0/0x3f4) > (do_bind_con_driver) from [<c03cf854>] (do_take_over_console+0x84/0x1dc) > (do_take_over_console) from [<c039404c>] (do_fbcon_takeover+0x74/0xcc) > (do_fbcon_takeover) from [<c013b200>] (notifier_call_chain+0x48/0x84) > (notifier_call_chain) from [<c013b9e8>] (blocking_notifier_call_chain+0x44/0x5c) > (blocking_notifier_call_chain) from [<c038d54c>] > (register_framebuffer+0x228/0x310) > (register_framebuffer) from [<c03f2730>] > (__drm_fb_helper_initial_config_and_unlock+0x234/0x460) > (__drm_fb_helper_initial_config_and_unlock) from [<c03f2a10>] > (drm_fb_helper_fbdev_setup+0xb4/0x1e8) > (drm_fb_helper_fbdev_setup) from [<c03f30a0>] (drm_fbdev_cma_init+0x8c/0xbc) > (drm_fbdev_cma_init) from [<c03f30d8>] (drm_fb_cma_fbdev_init+0x8/0x14) > (drm_fb_cma_fbdev_init) from [<c041e478>] (mcde_drm_bind+0xec/0x114) > (mcde_drm_bind) from [<c04215e0>] (try_to_bring_up_master+0x140/0x17c) > (try_to_bring_up_master) from [<c0421820>] > (component_master_add_with_match+0xc8/0xfc) > (component_master_add_with_match) from [<c041ea80>] (mcde_probe+0x544/0x58c) > (mcde_probe) from [<c0428c44>] (platform_drv_probe+0x48/0x98) > (platform_drv_probe) from [<c04270f0>] (really_probe+0x228/0x2d0) > (really_probe) from [<c04272fc>] (driver_probe_device+0x60/0x164) > (driver_probe_device) from [<c04274d0>] (__driver_attach+0xd0/0xd4) > (__driver_attach) from [<c04253e8>] (bus_for_each_dev+0x74/0xb4) > (bus_for_each_dev) from [<c0426544>] (bus_add_driver+0x188/0x20c) > (bus_add_driver) from [<c0427e08>] (driver_register+0x7c/0x114) > (driver_register) from [<c010271c>] (do_one_initcall+0x54/0x194) > (do_one_initcall) from [<c0b00e38>] (kernel_init_freeable+0x148/0x1e4) > (kernel_init_freeable) from [<c071ee14>] (kernel_init+0x8/0x10c) > (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c) > > I *think* this is because of a catch 22: the FB emulation want to > send the first frame out, so it starts the vblank and wait for the > first blank before sending the first new fresh framebuffer update > by calling .update(). This shouldn't have anything to do with the fb emulation or fb helpers - all it doesn is light up the display like any other kms clients. Doing a dpms off/on with X or similar should confirm that. > But since this DSI command mode panel is not going to give any > IRQs and thus no vblanks before the first frame is sent, it waits > in vain. Video mode DSI panels does not have this characteristic > and it seems all other panels we handle are video mode. > > After the timeout and sending the first frame anyways, of course > the vblank starts working. > > So it seems to me that the FB helper does not really deal very well > with this kind of displays that do not support vblank interrupts until > you send the first frame. > > The crtcs state no_vblank should according to documentation > be used to overcome these situations. At least sometimes. > I could not get that to help at all, but it seems unclear whether > any of the helper-based drivers actually use that. > > My driver will deal with arming the vblank event on the first > update (drm_crtc_vblank_get(crtc) == 0)) and sending vblank > events in response to updates when the CRTC is not active > as is custom. This is just exactly what all other drivers do. Not sure what exactly is going wrong here. Two thoughts: - Might be good to switch over the simple kms helpers to use wait_for_flip_done instead of wait_for_vblanks. Latter is the older and more fragile version of the same function really. Aside: we should make the wait_for_flip_done the default, but that means changing tons of drivers ... - Maybe you're missing some drm_crtc_vblank_on/off calls. Those make sure that the vblank code knows when your pipe is on/off and able to actually generate vblanks. If you don't call these, then the wait_for_vblank() code will succeed in the drm_crtc_vblank_get (when it should have failed because the pipe is still off), and then hit the WARN_ON because it timed out. > Has somebody else seen the same problem or is it just me? Everybody has fun with vblanks and flip completion events. It's the most tricky part of kms, which is why the code is full of WARN_ON checks to catch driver implementation issues. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
