[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.
BR,
-R
> 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
>
More information about the dri-devel
mailing list