Re: [Intel-gfx] [PATCH 12/26] drm/atomic-helper: Massage swap_state signature somewhat

Maarten Lankhorst
2016-05-31T08:43:56Z

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org