[RFC 1/9] drm: add atomic fxns

[Rob Clark]

On Wed, Sep 12, 2012 at 1:03 PM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> On Wed, Sep 12, 2012 at 12:35:01PM -0500, Rob Clark wrote:
>> On Wed, Sep 12, 2012 at 11:57 AM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
>> > On Sun,  9 Sep 2012 22:03:14 -0500
>> > Rob Clark <rob.clark at linaro.org> wrote:
>> >
>> >> From: Rob Clark <rob at 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.
>
> Usually you would need to build up the full state anyway before you can
> tell if it's good or bad. I don't see what some big array would buy here.

yup.. this was why I added drm_{crtc,plane,etc}_check_state(), to
bring back the common state checking that used to be done in the ioctl
fxns

>> > 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.
>
> This was my original idea as well. Separate the current and future
> states cleanly. At the moment it's all mixed up inside the objects
> somewhere. I sort of abandoned the idea so that I could avoid touching
> too much driver code while prototyping the atomic modesetting, but
> that's certainly the way I would design the system.

Yeah, I don't see any other way to do it sanely other than separating
out the current/future state.  Fortunately for planes, there are only
a few drivers that support planes so it isn't too bad.  For full
modesetting, it gets to be a lot of search and replace.  But it makes
it so much cleaner so I think it is worth doing.

We could probably also simplify a bunch of the crtc helper code that
handles reverting to previous state.

BR,
-R

> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel