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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
