Hi all,
So I've figured instead of just talking I should draw up my ideas for atomic helpers and related stuff in some draft patches. Major differences compared to Rob's series:
- The software side state update is done synchronously, at least when using the helper code. Drivers are free to do whatever they want, as usual. This has a bunch of nice upsides:
+ All the tricky locking dances can be abolished, and if the ordering and synchronization is done correctly async worker threads don't need any modeset state locking at all. I'm fairly sure that Rob's scheme won't blow up, but I much prefer we don't need it at all.
+ In Rob's patches any atomic operation must wait for outstanding updates to complete. Otherwise the software state isn't committed yet. With the synchronous updates we can immediately proceed with the state construction and verification and will only block right before applying the state update. And even for that drivers can implement cancellation of the previous update.
This has the nice benefit that check-only operations (which compositors need to do right after having completed the current frame to know how to render the next one) can be done without blocking for completion of the previous update.
- Internal interfaces for atomic updates don't need to go through properties, but can directly set the state. Drivers can always compare updates in their ->check hook later on, so we don't lose expressiveness in the interface. But the resulting code in the callers looks much better.
In general I've ditched a lot of interfaces where drivers could overwrite default behaviour and boiled down the interface two state handling functions (duplicate+destroy), setting driver-private properties and check/commit.
- There is a clear helper separation between core code which should be used to use the atomic interface to drivers, and helper functions. Core never uses helper functions so that we can keep the clear separation we have in all other places in the kms api.
There's a set of helpers sitting in-between to implement legacy interfaces on top of atomic. Those _only_ use the core atomic interfaces.
- I've added a new set of plane helpers. Mostly this was motivated to make the atomic helpers work on less stellar hardware where you can't just stream the state and then atomically commit with some GO bits. But those helpers should also be useful for more gradual transitions of drivers to the atomic interface. Since a requirement for atomic is to have universal plane support they can be used bare-bones just for that, without all the other atomic baggage (i.e. all the state tracking for crtcs/connectors).
- State tracking for connectors. i915 has piles of relevant connector properties, which we want to save/restore in atomic ops (e.g. hdmi metadata and stuff).
- fbdev panic locking is implemented with trylock instead of nolock.
- I've thought a bit about resume. See the proposed state handling for the default reset functions and the addition of a plane reset callback.
There's also a metric pile of stuff missing:
- It's completely untested (except for the parts touching currently used code) since I lack suitable hw. Converting i915 is just a bit too much of a pain for a quick w/e code draft ;-)
- I didn't add all the properties for things currently exposed through ioctl structures. Since the interface here doesn't force internal code to use properties for everything (which really makes things look much better) we don't have a need for that until we merge the actual atomic ioctl. So I left that out.
- I didn't write code for the fbdev helper to use atomic. The w/e ran out of time for that ...
- Kerneldoc is occasionally not updated or still missing.
Anyway I think this draft code is good enough to explain how I'd like to address some of the concerns I have with the current atomic code. Comments, flames and ideas highly welcome. And if anyone is insane enought to try this on real hardware, that would certainly be interesting and I'm very much willing to help out as much as possible ... i915 just really isn't suitable since it won't be using the helpers, and all the other hw I have here doesn't support planes.
Cheers, Daniel
Daniel Vetter (18): drm: Add atomic driver interface definitions for objects drm: Add drm_plane/connector_index drm: Move modeset_lock_all helpers to drm_modeset_lock.[hc] drm: Handle legacy per-crtc locking with full acquire ctx drm: Global atomic state handling drm: Add atomic/plane helpers drm/irq: Implement a generic vblank_wait function drm/i915: Use generic vblank wait drm/plane-helper: transitional atomic plane helpers drm/crtc-helper: Transitional functions using atomic plane helpers drm: Atomic crtc/connector updates using crtc helper interfaces drm: Move ->old_fb from crtc to plane drm/atomic-helper: implementatations for legacy interfaces drm/plane-helper: Add async mode to prepare_fb drm/atomic-helpers: document how to implement async commit drm/atomic-helper: implement ->page_flip drm: trylock modest locking for fbdev panics drm/atomic-helpers: functions for state duplicate/destroy/reset
Ville Syrjälä (1): drm: Add drm_crtc_vblank_waitqueue()
Documentation/DocBook/drm.tmpl | 1 + drivers/gpu/drm/Makefile | 4 +- drivers/gpu/drm/drm_atomic.c | 529 ++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 1570 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc.c | 194 ++--- drivers/gpu/drm/drm_crtc_helper.c | 139 +++ drivers/gpu/drm/drm_fb_helper.c | 10 +- drivers/gpu/drm/drm_irq.c | 45 + drivers/gpu/drm/drm_modeset_lock.c | 204 ++++- drivers/gpu/drm/drm_plane_helper.c | 181 +++- drivers/gpu/drm/i915/intel_display.c | 41 +- include/drm/drmP.h | 13 + include/drm/drm_atomic.h | 63 ++ include/drm/drm_atomic_helper.h | 98 +++ include/drm/drm_crtc.h | 208 ++++- include/drm/drm_crtc_helper.h | 13 + include/drm/drm_modeset_lock.h | 16 + include/drm/drm_plane_helper.h | 31 + 18 files changed, 3188 insertions(+), 172 deletions(-) create mode 100644 drivers/gpu/drm/drm_atomic.c create mode 100644 drivers/gpu/drm/drm_atomic_helper.c create mode 100644 include/drm/drm_atomic.h create mode 100644 include/drm/drm_atomic_helper.h