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..