On Fri, Oct 12, 2012 at 7:49 PM, Rob Clark rob.clark@linaro.org wrote:
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.
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