On Wed, Jun 08, 2016 at 04:44:24PM +0200, Maarten Lankhorst wrote: > Op 08-06-16 om 14:19 schreef Daniel Vetter: > > Design ideas: > > > > - split up the actual commit into different phases, and have > > completions for each of them. This will be useful for the future > > when we want to interleave phases much more aggressively, for e.g. > > queue depth > 1. For not it's just a minimal optimization compared > > to current common nonblocking implementation patterns from drivers, > > which all stall for the entire commit to complete, including vblank > > waits and cleanups. > > > > - Extract a separate atomic_commit_hw hook since that's the part most > > drivers will need to overwrite, hopefully allowing even more shared > > code. > > > > - Enforce EBUSY seamntics by attaching one of the completions to the > > flip_done vblank event. Side benefit of forcing atomic drivers using > > these helpers to implement event handlign at least semi-correct. I'm > > evil that way ;-) > > > > - Ridiculously modular, as usual. > > > > - The main tracking unit for a commit stays struct drm_atomic_state, > > and the ownership rules for that are unchanged. Ownership still > > gets transferred to the driver (and subsequently to the worker) on > > successful commits. What is added is a small, per-crtc, refcounted > > structure to track pending commits called struct drm_crtc_commit. > > No actual state is attached to that though, it's purely for ordering > > and waiting. > > > > - Dependencies are implicitly handled by assuming that any CRTC part > > of &drm_atomic_state is a dependency, and that the current commit > > must wait for any commits to complete on those CRTC. This way > > drivers can easily add more depencies using > > drm_atomic_get_crtc_state(), which is very natural since in most > > case a dependency exists iff there's some bit of state that needs to > > be cross checked. > > > > Removing depencies is not possible, drivers simply need to be > > careful to not include every CRTC in a commit if that's not > > necessary. Which is a good idea anyway, since that also avoids > > ww_mutex lock contention. > > > > - Queue depth > 1 sees some prep work in this patch by adding a stall > > paramater to drm_atomic_helper_swap_states(). To be able to push > > commits entirely free-standing and in a deeper queue through the > > back-end the driver must not access any obj->state pointers. This > > means we need to track the old state in drm_atomic_state (much > > easier with the consolidated arrays), and pass them all explicitly > > to driver backends (this will be serious amounts of churn). > ^Typo, and was done 9 commits before? Hm, what typo? And the patches are just prep, what we still need is explicitly passing crtc/plane/connector state into driver callbacks, so that they don't look at crtc/plane/connector->state any more. > > Once that's done stall can be set to false in swap_states. > > > > Features: Contains bugs because totally untested. > ^I Hope not.. Indeed, tested on iirc 5 drivers now. Will remove when merging. -Daniel > > v2: Dont ask for flip_done signalling when the CRTC is off and stays > > off: Drivers don't handle events in that case. Instead complete right > > away. This way future commits don't need to have special-case logic, > > but can keep blocking for the flip_done completion. > > > > v3: Tons of fixes: > > - Stall for preceeding commit for real, not the current one by > > accident. > > - Add WARN_ON in case drivers don't fire the drm event. > > - Don't double-free drm events. > > > > v4: Make legacy cursor not stall. > > > > v5: Extend the helper hook to cover the entire commit tail. Some > > drivers need special code for cleanup and vblank waiting, this makes > > it a bit more useful. Inspired by the rockchip driver. > > > > v6: Add WARN_ON to catch drivers who forget to send out the > > drm event. > > > > v7: Fixup the stalls in swap_state for real!! > > > > v8: > > - Fixup trailing whitespace, spotted by Maarten. > > - Actually wait for flip_done in cleanup_done, like the comment says > > we should do. Thanks a lot for Tomeu for helping with debugging this > > on. > > > > v9: Now with awesome kerneldoc! > > > > v10: Split out drm_crtc_commit tracking infrastructure. > > > > v: > > - Add missing static (Gustavo). > > - Split out the sync functions, only do the actual nonblocking > > logic in this patch (Maarten). > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> -- 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
