[RFC 00/11] atomic pageflip v3

Rob Clark rob at ti.com
Fri Oct 12 17:58:21 PDT 2012

On Fri, Oct 12, 2012 at 7:49 PM, Rob Clark <rob.clark at linaro.org> wrote:
> From: Rob Clark <rob at ti.com>
> This is following a bit the approach that Ville is taking for atomic-
> modeset, in that it is switching over to using properties for everything.
> The advantage of this approach is that it makes it easier to add new
> attributes to set as part of a page-flip (and even opens the option to
> add new object types).
> The basic principles are:
>  a) split out object state (in this case, plane and crtc, although I
>     expect more to be added as atomic-modeset is added) into separate
>     structures that can be atomically committed or rolled back.  The
>     property values array is also split out into the state structs,
>     so that the property values visible to userspace automatically
>     reflect the current state (ie. property changes are either
>     committed or discarded depending on whether the state changes
>     are committed or discarded).
>  b) expand the object property API (set_property()) to take a state
>     object.  The obj->set_property() simply updates the state object
>     without actually applying the changes to the hw.
>  c) after all the property updates are done, the updated state can
>     be checked for correctness and against the hw capabilities, and
>     then either discarded or committed atomically.
> Since we need to include properties in the atomic-pageflip scheme,
> doing everything via properties avoids updating a bunch of additional
> driver provided callbacks.
> For drivers that have been atomic-ified, they no longer need to
> implement .page_flip(), .set_plane(), .disable_plane().  Instead they
> should implement the atomic fxns, and their various .set_property()
> functions would not actually touch the hw, but instead update state
> objects.  State objects should hold all the state which could
> potentially be applied to the hw.  The .set_property() functions
> should not actually update the hw, because it might be the users
> intention to only test a new configuration.
> In drm_crtc / drm_plane, the userspace visible attributes are split
> out into separate drm_crtc_state / drm_plane_state structs.  (Note
> that this is only partially done for crtc, only the page-flip related
> attributes are split out.  I wanted to do this first, and then add the
> modeset related attributes in a later patch to split things up into
> smaller pieces.)  The intention is that drivers can wrap the state
> structs, and add whatever other information they need.  For example:
>     struct omap_plane_state {
>            struct drm_plane_state base;
>            uint8_t rotation;
>            uint8_t zorder;
>            uint8_t enabled;
>     };
> It doesn't strictly need to be just property values.  If driver needs
> to calculate some clock settings, fifo thresholds, or other internal
> state, based the external state set from userspace, it could stuff
> that stuff into it's state structs as well if it wanted.  But at a
> minimum the state objects should encapsulate the userspace visible
> state.
> For atomic-ified drivers, all updates, whether it be userspace
> setproperty ioctl updating a single property, or legacy setplane or
> pageflip ioctls, or new atomic ioctl(s), all go through the same path
> from the driver's perspective:
>     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);
> The global driver state token/object returned from .atomic_begin() is
> opaque from drm core perspective.  It somehow encapsulates the state
> of all crtc/plane/etc.  Inside the driver's different .set_property()
> fxns, this global state object gets mapped to per crtc/plane state
> objects.  Core drm helpers for dealing with common attributes (fb_id,
> crtc_x/y/w/h, etc) are provided.  (ie. drm_plane_set_property(),
> drm_plane_check_state(), etc.)   This is to avoid forcing each driver
> to duplicate code for setting common properties and sanity checking.
> After all the property updates have been passed to the driver via
> .set_property() calls, .atomic_check() is called.  This should check
> all the modified crtc/plane's (using drm_{plane,crtc}_check_state()
> for common check), and do any needed global sanity checks for the hw
> (ie. can this combination of planes meet hw bandwidth limitations,
> etc).
> For ioctls with a 'test' flag, the .atomic_commit() step might be
> skipped if userspace is only testing the new configuration.  In either
> case, .atomic_commit() is only called if .atomic_check() succeeds, so
> at this point it is already known that the new configuration is
> "good", so .atomic_commit() should really only fail for catastrophic
> things (unable to allocate memory, hardware is on fire, etc).
> The .atomic_end() is called at the end if the driver needs to do some
> cleanup.
> ------
> The intention is for this to also simplify atomic-modeset, by
> providing some of the necessary infrastructure (object property types,
> signed property values, and property's whose usespace visible values
> automatically tracks the object state which is either committed or
> discarded depending on whether the state change was committed to hw
> or not) which will also be needed for atomic-modeset.
> So far, I've only updated omapdrm to the new APIs, as a proof of
> concept.  Only a few drivers support drm plane, so I expect the
> updates to convert drm-plane to properties should not be so hard.
> Possibly for crtc/pageflip we might need to have a transition period
> where we still support crtc->page_flip() code path until all drivers
> are updated.
> My complete branch is here:
>   https://github.com/robclark/kernel-omap4/commits/drm_nuclear-2
>   git://github.com/robclark/kernel-omap4.git drm_nuclear-2
> v1: original RFC
> v2: I don't remember
> v3: don't remove .page_flip(), .update_plane(), etc.  These are still
>     used for drivers that don't implement the atomic functions, to
>     allow for a transition period
> Note that I haven't changed the format of the atomic-pageflip ioctl
> itself since the last patchset.  At this point I'm more interested in
> feedback on the first nine patches.

Note, for the next version of this series, I was planning to update
all the other drivers to compile again, so update for new property fxn
prototypes, and crtc->fb to crtc->state->fb, etc.  But that will touch
a lot of code.  So if anyone has some major KMS changes in progress in
their driver that I should base on top of to minimize conflicts later,
let me know.


> Rob Clark (11):
>   drm: add atomic fxns
>   drm: add object property type
>   drm: add DRM_MODE_PROP_DYNAMIC property flag
>   drm: add DRM_MODE_PROP_SIGNED property flag
>   drm: split property values out
>   drm: convert plane to properties
>   drm: add drm_plane_state
>   drm: convert page_flip to properties
>   drm: add drm_crtc_state
>   drm: atomic pageflip
>   drm/omap: update for atomic age
>  drivers/gpu/drm/drm_crtc.c            |  916 ++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/drm_crtc_helper.c     |   51 +-
>  drivers/gpu/drm/drm_drv.c             |    1 +
>  drivers/gpu/drm/drm_fb_helper.c       |   12 +-
>  drivers/staging/omapdrm/Makefile      |    1 +
>  drivers/staging/omapdrm/omap_atomic.c |  347 +++++++++++++
>  drivers/staging/omapdrm/omap_atomic.h |   52 ++
>  drivers/staging/omapdrm/omap_crtc.c   |  237 +++++----
>  drivers/staging/omapdrm/omap_drv.c    |   26 +-
>  drivers/staging/omapdrm/omap_drv.h    |   45 +-
>  drivers/staging/omapdrm/omap_fb.c     |   44 +-
>  drivers/staging/omapdrm/omap_plane.c  |  295 ++++++-----
>  include/drm/drm.h                     |    2 +
>  include/drm/drmP.h                    |   52 ++
>  include/drm/drm_crtc.h                |  161 +++++-
>  include/drm/drm_mode.h                |   50 ++
>  16 files changed, 1818 insertions(+), 474 deletions(-)
>  create mode 100644 drivers/staging/omapdrm/omap_atomic.c
>  create mode 100644 drivers/staging/omapdrm/omap_atomic.h
> --

More information about the dri-devel mailing list