[PATCH 0/7] prepare for atomic.. the great propertyification
Daniel Vetter
daniel at ffwll.ch
Thu Jul 24 05:25:34 PDT 2014
On Wed, Jul 23, 2014 at 03:38:13PM -0400, Rob Clark wrote:
> This is mostly just a rebase+resend. Was going to send it a bit earlier
> but had a few things to fix up as a result of the rebase.
>
> At this point, I think next steps are roughly:
> 1) introduce plane->mutex
> 2) decide what we want to do about events
> 3) add actual ioctl
>
> I think we could shoot for merging this series next, and then adding
> plane->mutex in 3.18?
>
> Before we add the ioctl, I think we want to sort out events for updates
> to non-primary layers, and what the interface to drivers should look like.
> Ie. just add event to ->update_plane() or should we completely ditch
> ->page_flip() and ->update_plane()?
>
> Technically, I think we could get away without a new API and just let
> drivers grab all the events in their ->atomic_commit(), but I suspect
> core could provide something more useful to drivers. I guess it would
> be useful to have a few more drivers converted over to see what makes
> sense.
Ok so we've had a lot of discussions about this on irc and overall I'm not
terribly happy with what this looks like. I have a few concerns:
- The entire atomic conversion for drivers is monolithic, at least at the
interface level. So if you want to do this step-by-step you're forced to
add hacks and e.g. bypass pageflips until that's implemented. E.g. on
i915 I see no chance that we'll get atomic ready for all cases (so
nonblocking flips and multi-crtc and everything on all platforms) in one
go.
- Related to that is that we force legacy ioctls through atomic. E.g. on
older i915 platforms I very much expect that we won't ever convert the
pageflip code to atomic for them and just not support atomic flips of
cursor + primary plane. At least not at the beginning. I know that's
different from my stance a year ago, but I've become a bit more
realistic.
- The entire conversion is monolithic and we can't do it on a
driver-by-driver basis. Everyone has to go through the new atomic
interfaces on a flag day. drm is much bigger now and forcing everyone to
convert at the same time is really risky. Again I know I've changed my
mind again, but drm is a lot bigger and there's a lot more drivers that
implement pageflip and cursors, i.e. stuff that's special.
- The separation between interface marshalling code in the drm core and
helper functions for drivers to implement stuff is fuzzy at best. Thus
far we've had an excellent seperation in this are for kms, I don't think
it's vise to throw that out.
So here's my proposal for how to get this in without breaking the world
1. We add a complete new set of ->atomic_foo driver entry points to all
relevant objects. So instead of changing all the set_prop functions in a
flag-day like operation we just add a new interface. I haven't double
checked, but I guess that means per-obj ->atomic_set_prop functions plus a
global ->atomic_check and ->atomic_commit.
2. We add a new drm_atomic_helper.c library which provides functions to
implement all the legacy interfaces (page_flip, update_plane, set_prop)
using atomic interfaces. We should be able to 1:1 reuse Rob's code for
this, but instead of changing the actual current interface code we put
that into helpers.
We don't need anything for cursor ioctls since cursor plane helpers
already map the legacy cursor ioctls.
3. Rob's current code reuses the existing ->update_plane, ->pageflip and
other entry points to implement the atomic commit helper functions. Imo
that's a bad layering violation, and if we plug helpers in there we can't
use them.
Instead I think we should add a new per-plane ->atomic_commit functions
clearly marked as optional. Maybe even as part of an opaque
plane_helper_funcs pointer like we have with crtc/encoder/connector and
crtc helpers. msm would then implement it's atomic commit function in
there (since it's the only driver currently supporting atomic for real).
3b. We could adjust crtc helpers to use the plane commit hook instead of
the set_base function when avialable.
4. We do a pimped atomic helper based upon crtc helpers and the above
plane commit function added in 3. It would essentially implement what
Rob's current helper does, i.e.
Step 1: Shut down all crtcs which change using the crtc helpers. This step
is obviously optional and ommitted when there's nothing to do.
Step 2: Loop over all planes and call ->plane_commit or whatever we will
call the interface added in 3. above.
Step 3: Enable all crtcs that need enabling.
5. We start converting drivers. Every driver can convert at it's own pace,
opting in for atomic behaviour step-by-step.
6. We optionally use the atomic interface in the fb helper. It's imo
important to keep the code here parallel so that drivers can convert at
their own leisure.
7. We add the atomic ioctl.
8. Various cleanups once 5. is completed for all drivers - which will
likely take at least a year:
- Remove ->set_base from crtc helpers.
- Remove legacy functions from fbdev helpers.
- ...
I know this is a stack of work, but most of it just shuffles code around
that already exists 1:1 in Rob's series. And I really think that this is
where we should ultimately end up at, and going indirectly to that point
will be much more painful.
Also note that nothing in here really touches shared code, so we can
squeeze this into 3.17 even really late without risk to existing drivers.
Imo it's too late already in the 3.17 cycle for the current series, which
touches everything. If we get everything up to step 4. in now we can
convert 1-2 drivers in 3.18 and enable the atomic ioctl after that.
And I realize that everyon will totally hate this because apparently
google is making a hard move with adf and plans to enforce usage of that,
so this entire thing is a bit ugly.
Cheers, Daniel
>
> Rob Clark (5):
> drm: add atomic fxns
> drm: split propvals out and blob property support
> drm: convert plane to properties/state
> drm: convert crtc to properties/state
> drm/msm: add atomic support
>
> Sean Paul (1):
> drm: Fix up the atomic legacy paths so they work
>
> Ville Syrjälä (1):
> drm: Refactor object property check code
>
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/armada/armada_crtc.c | 14 +-
> drivers/gpu/drm/armada/armada_output.c | 3 +-
> drivers/gpu/drm/armada/armada_overlay.c | 14 +-
> drivers/gpu/drm/ast/ast_drv.c | 6 +
> drivers/gpu/drm/ast/ast_drv.h | 1 +
> drivers/gpu/drm/ast/ast_mode.c | 1 +
> drivers/gpu/drm/cirrus/cirrus_drv.c | 6 +
> drivers/gpu/drm/cirrus/cirrus_drv.h | 1 +
> drivers/gpu/drm/cirrus/cirrus_mode.c | 1 +
> drivers/gpu/drm/drm_atomic.c | 733 +++++++++++++++
> drivers/gpu/drm/drm_crtc.c | 1351 ++++++++++++++++++---------
> drivers/gpu/drm/drm_fb_helper.c | 55 +-
> drivers/gpu/drm/drm_irq.c | 8 +-
> drivers/gpu/drm/drm_modeset_lock.c | 28 +
> drivers/gpu/drm/drm_plane_helper.c | 2 +
> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 11 +-
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 7 +
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 11 +-
> drivers/gpu/drm/gma500/cdv_intel_crt.c | 4 +-
> drivers/gpu/drm/gma500/cdv_intel_display.c | 1 +
> drivers/gpu/drm/gma500/cdv_intel_dp.c | 7 +-
> drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 7 +-
> drivers/gpu/drm/gma500/cdv_intel_lvds.c | 10 +-
> drivers/gpu/drm/gma500/mdfld_dsi_output.c | 12 +-
> drivers/gpu/drm/gma500/psb_drv.c | 7 +
> drivers/gpu/drm/gma500/psb_drv.h | 1 +
> drivers/gpu/drm/gma500/psb_intel_display.c | 1 +
> drivers/gpu/drm/gma500/psb_intel_drv.h | 4 +-
> drivers/gpu/drm/gma500/psb_intel_lvds.c | 10 +-
> drivers/gpu/drm/gma500/psb_intel_sdvo.c | 23 +-
> drivers/gpu/drm/i2c/ch7006_drv.c | 4 +-
> drivers/gpu/drm/i915/i915_drv.c | 8 +
> drivers/gpu/drm/i915/intel_crt.c | 4 +-
> drivers/gpu/drm/i915/intel_display.c | 6 +-
> drivers/gpu/drm/i915/intel_dp.c | 7 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_hdmi.c | 7 +-
> drivers/gpu/drm/i915/intel_lvds.c | 4 +-
> drivers/gpu/drm/i915/intel_sdvo.c | 23 +-
> drivers/gpu/drm/i915/intel_sprite.c | 1 +
> drivers/gpu/drm/i915/intel_tv.c | 12 +-
> drivers/gpu/drm/mgag200/mgag200_drv.c | 7 +
> drivers/gpu/drm/mgag200/mgag200_drv.h | 1 +
> drivers/gpu/drm/mgag200/mgag200_mode.c | 1 +
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c | 66 +-
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 6 +
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.h | 1 +
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 14 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 65 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 6 +
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 2 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 14 +-
> drivers/gpu/drm/msm/msm_atomic.c | 141 +++
> drivers/gpu/drm/msm/msm_drv.c | 26 +
> drivers/gpu/drm/msm/msm_drv.h | 8 +
> drivers/gpu/drm/msm/msm_gem.c | 24 +-
> drivers/gpu/drm/msm/msm_gem.h | 13 +
> drivers/gpu/drm/msm/msm_kms.h | 1 +
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 1 +
> drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 +-
> drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 3 +-
> drivers/gpu/drm/nouveau/nouveau_connector.c | 7 +-
> drivers/gpu/drm/nouveau/nouveau_drm.c | 7 +
> drivers/gpu/drm/nouveau/nouveau_drm.h | 1 +
> drivers/gpu/drm/nouveau/nv50_display.c | 1 +
> drivers/gpu/drm/omapdrm/omap_crtc.c | 16 +-
> drivers/gpu/drm/omapdrm/omap_drv.c | 12 +-
> drivers/gpu/drm/omapdrm/omap_drv.h | 4 +-
> drivers/gpu/drm/omapdrm/omap_plane.c | 10 +-
> drivers/gpu/drm/qxl/qxl_display.c | 6 +-
> drivers/gpu/drm/qxl/qxl_drv.c | 9 +
> drivers/gpu/drm/radeon/radeon_connectors.c | 9 +-
> drivers/gpu/drm/radeon/radeon_display.c | 2 +
> drivers/gpu/drm/radeon/radeon_drv.c | 9 +
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 7 +
> drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 3 +-
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 12 +-
> drivers/gpu/drm/rcar-du/rcar_du_vgacon.c | 3 +-
> drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 6 +-
> drivers/gpu/drm/shmobile/shmob_drm_drv.c | 7 +
> drivers/gpu/drm/shmobile/shmob_drm_plane.c | 2 +
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 1 +
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 +
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 +
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 3 +-
> drivers/gpu/drm/udl/udl_connector.c | 6 +-
> drivers/gpu/drm/udl/udl_drv.c | 8 +
> drivers/gpu/drm/udl/udl_modeset.c | 2 +
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 7 +
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 1 +
> include/drm/drmP.h | 80 ++
> include/drm/drm_atomic.h | 170 ++++
> include/drm/drm_crtc.h | 238 ++++-
> include/drm/drm_modeset_lock.h | 47 +
> include/uapi/drm/drm_mode.h | 3 +
> 102 files changed, 2901 insertions(+), 650 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_atomic.c
> create mode 100644 drivers/gpu/drm/msm/msm_atomic.c
> create mode 100644 include/drm/drm_atomic.h
>
> --
> 1.9.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the dri-devel
mailing list