[RFC 00/11] atomic pageflip v3

* [RFC 00/11] atomic pageflip v3
@ 2012-10-13  0:49 Rob Clark
  2012-10-13  0:49 ` [RFC 01/11] drm: add atomic fxns Rob Clark
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Rob Clark @ 2012-10-13  0:49 UTC (permalink / raw)
  To: dri-devel; +Cc: patches, daniel.vetter, Rob Clark

From: Rob Clark <rob@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.

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

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread