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

[Maarten Lankhorst]

Op 31-05-16 om 12:46 schreef Daniel Vetter:
> 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.
Yeah but this is pseudocode. I don't think waiting belongs to swap_state, should be done before so we can still back out if wait fails..

~Maarten