On Wed, 12 Sep 2012 12:35:01 -0500 Rob Clark rob.clark@linaro.org wrote:
On Wed, Sep 12, 2012 at 11:57 AM, Jesse Barnes jbarnes@virtuousgeek.org wrote:
On Sun, 9 Sep 2012 22:03:14 -0500 Rob Clark rob.clark@linaro.org wrote:
From: Rob Clark rob@ti.com
The 'atomic' mechanism allows for multiple properties to be updated, checked, and commited atomically. This will be the basis of atomic- modeset and nuclear-pageflip.
The basic flow is:
state = dev->atomic_begin(); for (... one or more ...) obj->set_property(obj, state, prop, value); if (dev->atomic_check(state)) dev->atomic_commit(state, event); dev->atomic_end(state);
I think the above is more suited to drm_crtc_helper code. I think the top level callback should contain the arrays and be a single "multi flip" hook (or maybe a check then set double callback). For some drivers that'll end up being a lot simpler, rather than sprinkling atomic handling code in all the set_property callbacks.
well, there are a few other places in drm_crtc.c where I want to use the new API, to avoid drivers having to support both atomic API and old set_plane/page_flip stuff.. the transactional API makes that a bit easier, I think.. or at least I don't have to construct an array on the stack.
But I'm not entirely convinced that it is a problem.. with drm_{crtc,plane,etc}_state, it is just building up a set of values in a state struct, and that state struct gets atomically committed.
Yeah I know it can work this way, it just seemed like the begin, end, and set_property callbacks might be unnecessary if the props were all part of the state. The driver call roll things back (or just not touch hw) if the check or commit fails...
I guess ultimately, given the choice, I'd rather have the drivers calling into helper functions where possible, rather than having the core impose a specific set of semi-fine grained hooks.
Having a transactional API just seems a little messier with both the atomic state and per-property state to track and rollback in the case of failure.
For the rollback, I think I'm just going to move the array of property values into drm_{crtc,plane,etc}_state. That way, userspace doesn't see updated property values if they are not committed. It makes the property value rollback automatic.
Ok that seems reasonable.