Hi Dave,
So here's my atomic series, finally all debugged&reviewed. Sean Paul has done a full detailed pass over it all, and a lot of other people have commented and provided feedback on some parts. Rob Clark also converted msm over the w/e and seems happy. The only small thing is that Rob wants to export the wait_for_vblank, which imo makes sense. Since there's other stuff still to do I think we should apply Rob's patch (once it has grown appropriate kerneldoc) later on top of this.
This is just the core<->driver interface plus a big pile of helpers. Short recap of the main ideas:
- There are essentially three helper libraries in this patch set:
* Transitional helpers to use the new plane callbacks for legacy plane updates and in the crtc helper's ->mode_set callback. These helpers are only temporarily used to convert drivers to atomic, but they allow a nice separation between changing the driver backend and switching to the atomic commit logic.
* Legacy helpers to implement all the legacy driver entry points (page_flip, set_config, plane vfuncs) on top of the new atomic driver interface. These are completely driver agnostic. The reason for having the legacy support as helpers is that drivers can switch step-by-step. And they could e.g. even keep the legacy page_flip code around for some old platforms where converting to full-blown atomic isn't worth it.
* Atomic helpers which implement the various new ->atomic_* driver interfaces in terms of the revised crtc helper and new plane helper hooks.
- The revised crtc helper implemenation essentially implements all the lessons learned in the i915 modeset rework (when using the atomic helpers only):
* Enable/disable sequence for a given config are always the same and callbacks are always called in the same order. This contrast starkly with the crtc helpers, where the sequence of operations is heavily dependent on the previous config.
One corollary of this is that if the configuration of a crtc only partially changes (e.g. a connector moves in a cloned config) the helper code will still disable/enable the full display pipeline. This is the only way to ensure that the enable/disable sequence is always the same.
* It won't call disable or enable hooks more than once any more because it lost track of state, thanks to the atomic state tracking. And if drivers implement the ->reset hook properly (by either resetting the hw or reading out the hw state into the atomic structures) this even extends to the hardware state. So no more disable-me-harder kind of nonsense.
* The only thing missing is the hw state readout/cross-check support, but if drivers have hw state readout support in their ->reset handlers it's simple to extend that to cross-check the hw state.
* The crtc->mode_set callback is gone and its replacement only sets crtc timings and no longer updates the primary plane state. This way we can finally implement primary planes properly.
- The new plane helpers should be suitable enough for pretty much everything, and a perfect fit for hardware with GO bits. Even if they don't fit the atomic helper library is rather flexible and exports all the functions for the individual steps to drivers. So drivers can pick what matches and implement their own magic for everything else.
- A big difference compared to all previous atomic series is that this one doesn't implement async commit in a generic way. Imo driver requirements for that are too diverse to create anything reasonable sane which would actually work on a reasonable amount of different drivers. Also, we've never had a helper library for page_flips even, so it's really hard to know what might work and what's stupid without a bit of experience in the form of a few driver implementations.
I think with the current flexibility for drivers to pick individual stages and existing helpers like drm_flip_queue it's rather easy though to implement proper async commit.
- There's a few other differences of minor importance to earlier atomic series:
* Common/generic properties are parsed in the callers/core and not in drivers, and passed to drivers by directly setting the right members in atomic state structures. That greatly simplifies all the transitional and legacy helpers an removes a lot of boilerplate code.
* There's no crazy trylock mode used for the async commit since these helpers don't do async commit. A simple ordered flip queue of atomic state updates should be sufficient for preventing concurrent hw access anyway, as long as synchronous updates stall correctly with e.g. flush_work_queue or similar function. Abusing locks to enforce ordering isn't a good idea imo anyway.
* These helpers reuse the existing ->mode_fixup hooks in the atomic_check callback. Which means that drivers need to adapat and move a lot less code into their atomic_check callbacks.
Now this isn't everything needed in the drm core and helpers for full atomic support. But it's enough to start with converting drivers, and except for actually testing multiplane and multicrtc updates also enough to implement full atomic updates. Still missing are:
- Per-plane locking. Since these helpers here encapsulate the locking completely this should be fairly easy to implement.
- fbdev support for atomic_check/commit, so that multi-pipe finally works sanely in fbcon.
- Adding and decoding shared/core properties. That just needs to be rebased from Rob's latest patch series, with minor adjustments so that the decoding happens in the core instead of in drivers.
- Actually adding the atomic ioctl. Again just rebasing Rob's latest patch should be all that's needed.
- Resolving how to deal with DPMS in atomic. Atomic is a good excuse to fix up the crazy semantics dpms currently has. I'm floating an RFC about this topic already.
- Finally I couldn't test connector/encoder stealing properly since my test vehicle here doesn't allow a connector on different crtcs. So drivers which support this might see some surprises in that area. There is no semantic change though in how encoder stealing and assignment works (or at least no intended one), so I think the risk is minimal.
As just mentioned I've done a fake conversion of an existing driver using crtc helpers to debug the helper code and validate the smooth transition approach. And that smooth transition was the really big motivation for this. It seems to actually work and consists of 3 phases:
Phase 1: Rework driver backend for crtc/plane helpers
The requirement here is that universal plane support is already implement. If universal plane support isn't implement yet it might be better though to just do it as part of this phase, directly using the new plane helpers. There are two big things to do:
- Split up the existing ->update/disable_plane hooks into check/commit hooks and extract the crtc-wide prep/flush parts (like setting/clearing GO bits).
- The other big change is to split the crtc->mode_set hook into the plane update (done using the plane helpers) and the crtc setup in a new ->mode_set_nofb hook.
When phase 1 is complete the driver implements all the new callbacks which push the software state into hardware, but still using all the legacy entry points and crtc helpers. The transitional helpers serve as impendance mismatch here.
Phase 2: Rework state handling
This consists of rolling out the state handling helpers for planes, crtcs and connectors and reviewing all ->mode_fixup and similar hooks to make sure they don't depend upon implicit global state which might change in the atomic world. Any such code must be moved into ->atomic_check functions which just rely on the free-standing atomic state update structures.
This phase also adds a few small pieces of fixup code to make sure the atomic state doesn't get out of sync in the legacy driver callbacks.
Phase 3: Roll out atomic support
Now it's just about replacing vfuncs with the ones provided by the helper and filling out the small missing pieces (like atomic_check logic or async commit support needed for page_flips). Due to the prep work in phase 1 no changes to the driver backend functions should be required, and because of the prep work in phase 2 atomic implementations can be rolled out step-by-step. So if async commit ins't implemented yet page_flip can be implemented with the legacy functions without wreaking havoc in the other operations.
Cheers, Daniel
The following changes since commit bbf0ef0334f2267687a92ec6d8114fd67b8157a3:
Merge tag 'drm-intel-next-2014-10-03-no-ppgtt' of git://anongit.freedesktop.org/drm-intel into drm-next (2014-10-28 12:37:58 +1000)
are available in the git repository at:
git://anongit.freedesktop.org/drm-intel tags/topic/atomic-helpers-2014-11-09
for you to fetch changes up to 321ebf04dc7ab5c54d658f93db0ffe35277664ab:
drm/atomic: Refcounting for plane_state->fb (2014-11-06 21:08:37 +0100)
---------------------------------------------------------------- Daniel Vetter (17): drm: Move drm_crtc_init from drm_crtc.h to drm_plane_helper.h drm: Pull drm_crtc.h into the kerneldoc template drm: fixup kerneldoc in drm_crtc.h drm/modeset_lock: document trylock_only in kerneldoc drm: Add atomic driver interface definitions for objects drm: Global atomic state handling drm: Add atomic/plane helpers drm/plane-helper: transitional atomic plane helpers drm/crtc-helper: Transitional functions using atomic plane helpers drm: Atomic crtc/connector updates using crtc/plane helper interfaces drm/atomic-helper: implementatations for legacy interfaces drm/atomic: Integrate fence support drm/atomic-helpers: document how to implement async commit drm/atomic-helper: implement ->page_flip drm/atomic-helpers: functions for state duplicate/destroy/reset drm: Docbook integration and over sections for all the new helpers drm/atomic: Refcounting for plane_state->fb
Documentation/DocBook/drm.tmpl | 28 +- drivers/gpu/drm/Makefile | 4 +- drivers/gpu/drm/armada/armada_crtc.c | 1 + drivers/gpu/drm/ast/ast_mode.c | 1 + drivers/gpu/drm/bochs/bochs_kms.c | 1 + drivers/gpu/drm/cirrus/cirrus_mode.c | 1 + drivers/gpu/drm/drm_atomic.c | 628 +++++++++ drivers/gpu/drm/drm_atomic_helper.c | 1906 ++++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc_helper.c | 132 ++ drivers/gpu/drm/drm_plane_helper.c | 198 ++- drivers/gpu/drm/gma500/psb_intel_display.c | 1 + drivers/gpu/drm/mgag200/mgag200_mode.c | 1 + drivers/gpu/drm/nouveau/dispnv04/crtc.c | 1 + drivers/gpu/drm/nouveau/nv50_display.c | 1 + drivers/gpu/drm/omapdrm/omap_crtc.c | 1 + drivers/gpu/drm/qxl/qxl_display.c | 1 + drivers/gpu/drm/radeon/radeon_display.c | 1 + drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 + drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 1 + drivers/gpu/drm/sti/sti_drm_crtc.c | 1 + drivers/gpu/drm/tegra/dc.c | 2 + drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 1 + drivers/gpu/drm/udl/udl_modeset.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 1 + drivers/staging/imx-drm/imx-drm-core.c | 1 + include/drm/drm_atomic.h | 67 + include/drm/drm_atomic_helper.h | 97 ++ include/drm/drm_crtc.h | 246 +++- include/drm/drm_crtc_helper.h | 13 + include/drm/drm_modeset_lock.h | 1 + include/drm/drm_plane_helper.h | 39 + 32 files changed, 3344 insertions(+), 36 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