Well, that escalated quickly.
I've been looking at adding modesetting support to the atomic ioctl, and this
is what I've ended up with so far. It's definitely not perfect, but given how
out of hand it's got at the moment, I wanted to send this out as an RFC before
I spent too long polishing it up.
This series ends up touching pretty much all the drivers, by virtue of turning
crtc->mode (in particular) into both a const and a pointer.
The reasoning behind this is that currently, we just treat modes as unboxed
data to shovel around. With atomic modesetting, and user-supplied modes, we
want to do better here. Whilst sketching out userspace/compositor
requirements, I came up with the following invariants, which necessitated
turning modes into a refcounted, immutable, type:
- modes can come from one of three sources: connector list, current
mode, userspace-created
- as far as possible, modes should be relateable to their source, e.g. if
Plymouth pulls a particular mode from the encoder, and you pick up on that
mode as part of current configuration during handover, you should be able
to work backwards to where Plymouth sourced it, i.e. the encoder list
- userspace should be able to tell the current status by looking at the IDs
returned by property queries, rather than having to pull the entire mode
out: if we make them do that, they won't bother minimising the deltas and
will just dump the full state in every time, and that makes debugging the
entire thing that much harder
- setting a mode current should hold a reference for as long as it's current,
e.g. if you create a magic user-supplied mode, set that on the CRTC and
then terminate the connection, the mode should still live on for handover
purposes
- persistence of system-supplied (from-connector or from-CRTC) modes is not
important beyond their natural lifetime: if you pick a mode up from a
connector's probed list and then that connector disappears, setting that
mode is unlikely to be what you want, so failure because the mode no
longer exists is entirely acceptable
The patchset so far breaks down into a few natural sets:
Hopefully-uncontroversial fixes (2).
drm: mode: Fix typo in kerneldoc
drm: fb_helper: Simplify exit condition
drm: mode: Allow NULL modes for equality check
Stop drivers changing crtc->mode (7) - some drivers were directly bashing
crtc->mode at various points they shouldn't, which destroys our immutability
guarantees, and is also generally bad form. Some of them didn't have an
obvious way to pass adjusted_mode through, so I've resurrected the earlier
usage of hwmode, to allow them to store and retrieve adjusted_mode from there.
drm: crtc_helper: Update hwmode before mode_set call
drm: Exynos: Remove mode validation inside mode_fixup
drm: Exynos: Use hwmode for adjusted_mode
drm: sti: Use crtc->hwmode for adjusted mode
drm: ast: Split register set from get_vbios_mode_info
drm: ast: Split mode adjustment from get_vbios_mode_info
drm: armada: Use crtc->hwmode for adjusted mode
Make crtc->mode const (9) - this is what I used to discover the previous
7, and is perhaps a good idea anyway. Currently this causes a bunch of
warnings, which I'll get to below; the vmwgfx conversion is also totally
botched and needs re-doing. i915 is probably already broken by this
point.
drm: bridge: Constify mode parameters
drm: encoder-slave: Constify mode parameters
drm: connector-helper: Constify mode_valid parameter
drm: connector-helper: Constify mode_set parameters
drm: crtc-helper: Constify mode_set parameters
drm: fb_helper: Constify modeset mode member
DRM: Atomic: Use pointer for mode in CRTC state
DRM: CRTC: Use pointer for display mode
DRM: Constify crtc->mode pointer
Modes as first-class objects (9) - make modes refcountable; by this stage
they're already immutable.
DRM: mode: Rename and combine drm_crtc_convert_umode
DRM: mode: Un-staticise drm_mode_new_from_umode
DRM: mode: Un-staticise drm_crtc_convert_to_umode
drm: mode: Cache userspace mode representation
drm: mode: Use cached usermode representation
drm: atomic: Expose CRTC active property
drm: atomic: Allow setting CRTC active property
drm: mode: Add kref to modes
drm: mode: Add drm_mode_reference
Use mode references throughout (4) - duplicating modes invalidates a bunch of
the requirements listed above. Go through and push the core to reference every
mode it gets, rather than duplicating/copying it. Applying this series causes a
boatload of constness warnings due to reference counting, which I intend to fix.
drm: fb_helper: Reference, not duplicate, modes
drm: crtc_helper: Reference, not duplicate, modes
drm: atomic_helper: Reference, not duplicate, modes
drm: i915/tegra: atomic: Reference, not duplicate, modes
Modes as properties (4) - expose the mode as a property, and also allow users
to create their own mode-properties. The getblob one I'm probably most iffy
about: I'd previously created a drm_property_blob underneath the mode, however
juggling the translation between our two object IDs (mode and blob) was much
more error-prone. It also meant we lost information just looking at the raw
property stream, and had to add type/priv pointers to drm_property_blob. In
the end I settled on just keeping a single drm_display_mode object, and
overloading getblob to also be able to pull the userspace representation of
modes.
drm: mode: Allow userspace to fetch mode as blob
drm: atomic: Add MODE_ID property
drm: property: Allow non-global blob properties
drm: mode: Add user object-creation ioctl
Misc (1) - hi Thierry. This could probably just get squashed into the commit
where we make crtc->mode a pointer though ...
Tegra: SOR: Don't always assume a valid mode
If this doesn't seem like a totally insane approach, I'll continue on with it.
I think it's going to be really hard to develop a credible userspace interface
without the invariance guarantees above - without that, userspace is just going
to dump (hopefully) its entire state on us every frame, which is extra overhead
to process, and also makes it more difficult to reason about the deltas it's
providing us; debugging that is a pain.
This is in a pretty rough state right now. It's enough to get it working on
Tegra (well, once hacked to claim DRIVER_ATOMIC), and is compile-tested on
everything else I could enable on ARM. I haven't been able to run it on i915 or
Exynos as yet, thanks to unrelated breakage in linux-next. I'm planning to sort
that out this week, and get those two actually working.
Don't look too closely at the exact usage of reference counting: we're
definitely leaking quite a few references, though those are easily enough fixed.
Also, the constness warnings need to be fixed up, though it hits a moderate
impasse: having userspace-exposed mode objects be more obviously immutable is
a good thing, however drm_display_mode is also used for internal driver code,
and for generating modes in the first place; these shouldn't be immutable. My
rough thought around that would be to split modes like so:
struct drm_mode_timings {
hdisplay, vdisplay, ...;
};
struct drm_mode_internal {
struct drm_mode_timings base_timings;
struct drm_mode_timings crtc_timings;
};
struct drm_mode_user {
struct drm_mode_object base;
struct kref refcount;
struct drm_mode_timings base_timings;
};
struct drm_mode *drm_mode_from_internal(const struct drm_mode_internal *src);
struct drm_mode *drm_mode_from_user(const struct drm_mode_modeinfo *src);
That would be a pretty huge Coccinelle run though, and certainly more invasive
than what I've done right now, so whilst it's a good idea, I didn't want to go
off and do it if it was just going to be immediately shot down.
So ... anyone feel like casting an eye over it?
Cheers,
Daniel
drivers/gpu/drm/armada/armada_crtc.c | 21 +-
drivers/gpu/drm/armada/armada_output.c | 2 +-
drivers/gpu/drm/armada/armada_output.h | 2 +-
drivers/gpu/drm/armada/armada_overlay.c | 4 +-
drivers/gpu/drm/ast/ast_drv.h | 1 +
drivers/gpu/drm/ast/ast_mode.c | 174 ++++++-----
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +-
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 6 +-
drivers/gpu/drm/bochs/bochs.h | 2 +-
drivers/gpu/drm/bochs/bochs_hw.c | 2 +-
drivers/gpu/drm/bochs/bochs_kms.c | 10 +-
drivers/gpu/drm/bridge/dw_hdmi.c | 9 +-
drivers/gpu/drm/cirrus/cirrus_mode.c | 8 +-
drivers/gpu/drm/drm_atomic.c | 56 +++-
drivers/gpu/drm/drm_atomic_helper.c | 67 +++--
drivers/gpu/drm/drm_crtc.c | 332 ++++++++++++++-------
drivers/gpu/drm/drm_crtc_helper.c | 65 ++--
drivers/gpu/drm/drm_encoder_slave.c | 4 +-
drivers/gpu/drm/drm_fb_helper.c | 35 ++-
drivers/gpu/drm/drm_fops.c | 7 +-
drivers/gpu/drm/drm_ioctl.c | 2 +
drivers/gpu/drm/drm_modes.c | 147 ++++++++-
drivers/gpu/drm/drm_plane_helper.c | 6 +-
drivers/gpu/drm/exynos/exynos7_drm_decon.c | 4 +-
drivers/gpu/drm/exynos/exynos_drm_connector.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 14 +-
drivers/gpu/drm/exynos/exynos_drm_drv.h | 4 +-
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 4 +-
drivers/gpu/drm/exynos/exynos_drm_encoder.c | 4 +-
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_plane.c | 14 +-
drivers/gpu/drm/exynos/exynos_hdmi.c | 49 +--
drivers/gpu/drm/exynos/exynos_mixer.c | 2 +-
drivers/gpu/drm/exynos/exynos_mixer.h | 2 +-
drivers/gpu/drm/gma500/cdv_intel_crt.c | 6 +-
drivers/gpu/drm/gma500/cdv_intel_display.c | 4 +-
drivers/gpu/drm/gma500/cdv_intel_dp.c | 7 +-
drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 6 +-
drivers/gpu/drm/gma500/cdv_intel_lvds.c | 6 +-
drivers/gpu/drm/gma500/mdfld_dsi_dpi.c | 4 +-
drivers/gpu/drm/gma500/mdfld_dsi_dpi.h | 4 +-
drivers/gpu/drm/gma500/mdfld_dsi_output.c | 2 +-
drivers/gpu/drm/gma500/mdfld_intel_display.c | 8 +-
drivers/gpu/drm/gma500/oaktrail.h | 6 +-
drivers/gpu/drm/gma500/oaktrail_crtc.c | 4 +-
drivers/gpu/drm/gma500/oaktrail_hdmi.c | 10 +-
drivers/gpu/drm/gma500/oaktrail_lvds.c | 4 +-
drivers/gpu/drm/gma500/psb_intel_display.c | 4 +-
drivers/gpu/drm/gma500/psb_intel_lvds.c | 6 +-
drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 +-
drivers/gpu/drm/i2c/adv7511.c | 6 +-
drivers/gpu/drm/i2c/ch7006_drv.c | 6 +-
drivers/gpu/drm/i2c/sil164_drv.c | 8 +-
drivers/gpu/drm/i2c/tda998x_drv.c | 24 +-
drivers/gpu/drm/i915/intel_atomic.c | 12 +-
drivers/gpu/drm/i915/intel_crt.c | 2 +-
drivers/gpu/drm/i915/intel_dp.c | 2 +-
drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
drivers/gpu/drm/i915/intel_dsi.c | 2 +-
drivers/gpu/drm/i915/intel_dvo.c | 2 +-
drivers/gpu/drm/i915/intel_fbdev.c | 4 +
drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/intel_lvds.c | 2 +-
drivers/gpu/drm/i915/intel_sdvo.c | 2 +-
drivers/gpu/drm/i915/intel_tv.c | 2 +-
drivers/gpu/drm/imx/dw_hdmi-imx.c | 8 +-
drivers/gpu/drm/imx/imx-ldb.c | 4 +-
drivers/gpu/drm/imx/imx-tve.c | 6 +-
drivers/gpu/drm/imx/ipuv3-crtc.c | 4 +-
drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
drivers/gpu/drm/imx/ipuv3-plane.h | 2 +-
drivers/gpu/drm/imx/parallel-display.c | 4 +-
drivers/gpu/drm/mgag200/mgag200_mode.c | 12 +-
drivers/gpu/drm/msm/edp/edp_bridge.c | 4 +-
drivers/gpu/drm/msm/edp/edp_connector.c | 2 +-
drivers/gpu/drm/msm/edp/edp_ctrl.c | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 7 +-
drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +-
drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c | 6 +-
drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c | 6 +-
drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c | 2 +-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 4 +-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 6 +-
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 17 +-
drivers/gpu/drm/nouveau/dispnv04/cursor.c | 2 +-
drivers/gpu/drm/nouveau/dispnv04/dac.c | 4 +-
drivers/gpu/drm/nouveau/dispnv04/dfp.c | 4 +-
drivers/gpu/drm/nouveau/dispnv04/tvmodesnv17.c | 4 +-
drivers/gpu/drm/nouveau/dispnv04/tvnv04.c | 4 +-
drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 8 +-
drivers/gpu/drm/nouveau/nouveau_connector.c | 4 +-
drivers/gpu/drm/nouveau/nv50_display.c | 29 +-
drivers/gpu/drm/omapdrm/omap_connector.c | 9 +-
drivers/gpu/drm/omapdrm/omap_crtc.c | 10 +-
drivers/gpu/drm/omapdrm/omap_drv.h | 2 +-
drivers/gpu/drm/omapdrm/omap_encoder.c | 4 +-
drivers/gpu/drm/qxl/qxl_display.c | 16 +-
drivers/gpu/drm/radeon/atombios_crtc.c | 27 +-
drivers/gpu/drm/radeon/atombios_dp.c | 2 +-
drivers/gpu/drm/radeon/atombios_encoders.c | 10 +-
drivers/gpu/drm/radeon/cik.c | 8 +-
drivers/gpu/drm/radeon/evergreen.c | 14 +-
drivers/gpu/drm/radeon/r100.c | 8 +-
drivers/gpu/drm/radeon/radeon_asic.h | 4 +-
drivers/gpu/drm/radeon/radeon_audio.c | 20 +-
drivers/gpu/drm/radeon/radeon_audio.h | 7 +-
drivers/gpu/drm/radeon/radeon_connectors.c | 12 +-
drivers/gpu/drm/radeon/radeon_cursor.c | 4 +-
drivers/gpu/drm/radeon/radeon_display.c | 12 +-
drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 14 +-
drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 20 +-
drivers/gpu/drm/radeon/radeon_legacy_tv.c | 4 +-
drivers/gpu/drm/radeon/radeon_mode.h | 10 +-
drivers/gpu/drm/radeon/rs600.c | 8 +-
drivers/gpu/drm/radeon/rs690.c | 28 +-
drivers/gpu/drm/radeon/rs780_dpm.c | 4 +-
drivers/gpu/drm/radeon/rv515.c | 32 +-
drivers/gpu/drm/radeon/si.c | 14 +-
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 6 +-
drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c | 6 +-
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 2 +-
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 12 +-
drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 10 +-
drivers/gpu/drm/sti/sti_cursor.c | 4 +-
drivers/gpu/drm/sti/sti_drm_crtc.c | 19 +-
drivers/gpu/drm/sti/sti_drm_plane.c | 2 +-
drivers/gpu/drm/sti/sti_dvo.c | 6 +-
drivers/gpu/drm/sti/sti_gdp.c | 2 +-
drivers/gpu/drm/sti/sti_hda.c | 8 +-
drivers/gpu/drm/sti/sti_hdmi.c | 6 +-
drivers/gpu/drm/sti/sti_layer.c | 2 +-
drivers/gpu/drm/sti/sti_layer.h | 4 +-
drivers/gpu/drm/sti/sti_mixer.c | 4 +-
drivers/gpu/drm/sti/sti_mixer.h | 2 +-
drivers/gpu/drm/sti/sti_tvout.c | 4 +-
drivers/gpu/drm/sti/sti_vid.c | 2 +-
drivers/gpu/drm/tegra/dc.c | 7 +
drivers/gpu/drm/tegra/dsi.c | 10 +-
drivers/gpu/drm/tegra/hdmi.c | 10 +-
drivers/gpu/drm/tegra/rgb.c | 8 +-
drivers/gpu/drm/tegra/sor.c | 11 +-
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 17 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.h | 3 +-
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 6 +-
drivers/gpu/drm/tilcdc/tilcdc_slave.c | 3 +-
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 6 +-
drivers/gpu/drm/udl/udl_connector.c | 2 +-
drivers/gpu/drm/udl/udl_encoder.c | 4 +-
drivers/gpu/drm/udl/udl_modeset.c | 7 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 32 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 12 +-
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 10 +-
include/drm/bridge/dw_hdmi.h | 2 +-
include/drm/drmP.h | 6 +
include/drm/drm_crtc.h | 25 +-
include/drm/drm_crtc_helper.h | 20 +-
include/drm/drm_encoder_slave.h | 10 +-
include/drm/drm_fb_helper.h | 3 +
include/drm/drm_modes.h | 10 +
include/uapi/drm/drm.h | 2 +
include/uapi/drm/drm_mode.h | 22 ++
164 files changed, 1253 insertions(+), 798 deletions(-)
--
2.3.2
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
