On Tue, May 31, 2016 at 10:43:56AM +0200, Maarten Lankhorst wrote: > Op 30-05-16 om 17:09 schreef Daniel Vetter: > > On Mon, May 30, 2016 at 03:08:39PM +0200, Maarten Lankhorst wrote: > >> Op 29-05-16 om 20:35 schreef Daniel Vetter: > >>> - dev is redundant, we have state->atomic > >>> - add stall parameter, which must be set when swapping needs to stall > >>> for preceeding commits to stop looking at ->state pointers. Currently > >>> all drivers need this to be, just prep work for a glorious future. > >> I have to disagree, if you want to stall it should be done in a separate helper before swapping state. > >> That function should also have the ability to fail, since we haven't called swap_state yet. :) > >> > >> Maybe with nonblock as parameter, so it could fail with -EBUSY if it would stall, and -EINTR if interrupted? > > This is not the stalling you're thinking of. The EBUSY check is in > > drm_atomic_helper_setup_commit (and as such entirely optional). > > > > And if you don't ever use the nonblocking helpers, no struct > > drm_crtc_commit will ever show up in crtc->commit_list, which means this > > here also doesn't do anything. > > > > This stall here is just to make sure that the previous commit doesn't get > > confused because we've ripped out crtc/plane/connector->state from > > underneath it. And as soon as we add previous_state pointers to > > drm_atomic_state and wire it up through all the hooks we can set > > stall=false here, since then pipelining with a queue depth > 1 is > > possible. > If a driver supports depth > 1 it could skip the extra help waiter and just call swap_state. > The helper should return -EINTR when needed. > > if (nonblock) { > ret = drm_atomic_helper_wait_for_completion(state); > if (ret) > return ret; > } > > drm_atomic_helper_swap_state.. That's why there's this stall parameter ... Also, wait_for_completion(state) is not enough, there's 3 different completions. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx