[RFC 1/9] drm: add atomic fxns

[Rob Clark]

On Wed, Sep 12, 2012 at 2:05 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> On Wed, 12 Sep 2012 12:35:01 -0500
> Rob Clark <rob.clark at linaro.org> 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.
>>
>> 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.

well, that is is basically what is happening.. for example, the
driver's set_property() code would, if the driver doesn't have any
custom properties, basically just be:

int xyz_set_property(crtc, state, property, val)
{
  return drm_crtc_set_property(crtc,
        xyz_get_crtc_state(state, crtc->pipe), property, val);
}

so the driver basically just has to map the generic void *state to the
appropriate 'struct drm_crtc_state *', and then call helpers.

But the driver is also free to intercept property values, if needed.
For example, with the private-plane setup in omapdrm, in the crtc I
intercept the fb-id property and also set it on the crtc's private
plane.

I suppose you could move the for loop iterating over an array of
properties into the driver.  I'm not really sure what that buys you,
since none of this is really applying state to hw at this stage.  Plus
I think we'd end up needing both fxn ptrs that take a single property
plus one that takes an array.

The part that is more important to give the driver flexibility is that
point where you need to apply the state to the hw, and here the driver
has complete control.  Although perhaps there is some room for
crtc-helpers to plug in below that for the modeset.

BR,
-R

>> > 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.
>
> --
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel