In the atomic state we'll have an array of states for crtcs, planes and connectors and need to be able to at them by their index. We already have a drm_crtc_index function so add the missing ones for planes and connectors.

If it later on turns out that the list walking is too expensive we can add the index to the relevant modeset objects.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_crtc.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 3 +++ 2 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0f934a58702d..fdea713767b4 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -937,6 +937,29 @@ void drm_connector_cleanup(struct drm_connector *connector) EXPORT_SYMBOL(drm_connector_cleanup);

/** + * drm_plane_index - find the index of a registered CRTC + * @plane: CRTC to find index for + * + * Given a registered CRTC, return the index of that CRTC within a DRM + * device's list of CRTCs. + */ +unsigned int drm_connector_index(struct drm_connector *connector) +{ + unsigned int index = 0; + struct drm_connector *tmp; + + list_for_each_entry(tmp, &connector->dev->mode_config.connector_list, head) { + if (tmp == connector) + return index; + + index++; + } + + BUG(); +} +EXPORT_SYMBOL(drm_connector_index); + +/** * drm_connector_register - register a connector * @connector: the connector to register * @@ -1239,6 +1262,29 @@ void drm_plane_cleanup(struct drm_plane *plane) EXPORT_SYMBOL(drm_plane_cleanup);

/** + * drm_plane_index - find the index of a registered CRTC + * @plane: CRTC to find index for + * + * Given a registered CRTC, return the index of that CRTC within a DRM + * device's list of CRTCs. + */ +unsigned int drm_plane_index(struct drm_plane *plane) +{ + unsigned int index = 0; + struct drm_plane *tmp; + + list_for_each_entry(tmp, &plane->dev->mode_config.plane_list, head) { + if (tmp == plane) + return index; + + index++; + } + + BUG(); +} +EXPORT_SYMBOL(drm_plane_index); + +/** * drm_plane_force_disable - Forcibly disable a plane * @plane: plane to disable * diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index a1a8c694e765..d6546fd1e00c 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -680,6 +680,7 @@ struct drm_plane_state { /* Source values are 16.16 fixed point */ uint32_t src_x, src_y; uint32_t src_h, src_w; + struct drm_atomic_state *state; };

@@ -1052,6 +1053,7 @@ int drm_connector_register(struct drm_connector *connector); void drm_connector_unregister(struct drm_connector *connector);

extern void drm_connector_cleanup(struct drm_connector *connector); +extern unsigned int drm_connector_index(struct drm_connector *crtc); /* helper to unplug all connectors from sysfs for device */ extern void drm_connector_unplug_all(struct drm_device *dev);

@@ -1091,6 +1093,7 @@ extern int drm_plane_init(struct drm_device *dev, const uint32_t *formats, uint32_t format_count, bool is_primary); extern void drm_plane_cleanup(struct drm_plane *plane); +extern unsigned int drm_plane_index(struct drm_plane *crtc); extern void drm_plane_force_disable(struct drm_plane *plane); extern int drm_crtc_check_viewport(const struct drm_crtc *crtc, int x, int y,

So drivers using the atomic interfaces expect that they can acquire additional locks internal to the driver as-needed. Examples would be locks to protect shared state like shared display PLLs.

Unfortunately the legacy ioctls assume that all locking is fully done by the drm core. Now for those paths which grab all locks we already have to keep around an acquire context in dev->mode_config. Helper functions that implement legacy interfaces in terms of atomic support can therefore grab this acquire contexts and reuse it.

The only interfaces left are the cursor and pageflip ioctls. So add functions to grab the crtc lock these need using an acquire context and preserve it for atomic drivers to reuse.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_crtc.c | 8 ++-- drivers/gpu/drm/drm_modeset_lock.c | 86 ++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 6 +++ include/drm/drm_modeset_lock.h | 5 +++ 4 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index acb3b3121f67..e33aa3a3a8a5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2714,7 +2714,7 @@ static int drm_mode_cursor_common(struct drm_device *dev, if (crtc->cursor) return drm_mode_cursor_universal(crtc, req, file_priv);

- drm_modeset_lock(&crtc->mutex, NULL); + drm_modeset_lock_crtc(crtc); if (req->flags & DRM_MODE_CURSOR_BO) { if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) { ret = -ENXIO; @@ -2738,7 +2738,7 @@ static int drm_mode_cursor_common(struct drm_device *dev, } } out: - drm_modeset_unlock(&crtc->mutex); + drm_modeset_unlock_crtc(crtc);

return ret;

@@ -4474,7 +4474,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, if (!crtc) return -ENOENT;

- drm_modeset_lock(&crtc->mutex, NULL); + drm_modeset_lock_crtc(crtc); if (crtc->primary->fb == NULL) { /* The framebuffer is currently unbound, presumably * due to a hotplug event, that userspace has not @@ -4558,7 +4558,7 @@ out: drm_framebuffer_unreference(fb); if (old_fb) drm_framebuffer_unreference(old_fb); - drm_modeset_unlock(&crtc->mutex); + drm_modeset_unlock_crtc(crtc);

return ret; } diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 73e6534fd0aa..412b396af740 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -130,6 +130,90 @@ void drm_modeset_unlock_all(struct drm_device *dev) EXPORT_SYMBOL(drm_modeset_unlock_all);

/** + * drm_modeset_lock_crtc - lock crtc with hidden acquire ctx + * @crtc: drm crtc + * + * This function locks the given crtc using a hidden acquire context. This is + * necessary so that drivers internally using the atomic interfaces can grab + * furether locks with the lock acquire context. + */ +void drm_modeset_lock_crtc(struct drm_crtc *crtc) +{ + struct drm_modeset_acquire_ctx *ctx; + int ret; + + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + if (WARN_ON(!ctx)) + return; + + drm_modeset_acquire_init(ctx, 0); + +retry: + ret = drm_modeset_lock(&crtc->mutex, ctx); + if (ret) + goto fail; + + WARN_ON(crtc->acquire_ctx); + + /* now we hold the locks, so now that it is safe, stash the + * ctx for drm_modeset_unlock_all(): + */ + crtc->acquire_ctx = ctx; + + return; + +fail: + if (ret == -EDEADLK) { + drm_modeset_backoff(ctx); + goto retry; + } +} +EXPORT_SYMBOL(drm_modeset_lock_crtc); + +/** + * drm_modeset_legacy_acquire_ctx - find acquire ctx for legacy ioctls + * crtc: drm crtc + * + * Legacy ioctl operations like cursor updates or page flips only have per-crtc + * locking, and store the acquire ctx in the corresponding crtc. All other + * legacy operations take all locks and use a global acquire context. This + * function grabs the right one. + */ +struct drm_modeset_acquire_ctx * +drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc) +{ + if (crtc->acquire_ctx) + return crtc->acquire_ctx; + + WARN_ON(!crtc->dev->mode_config.acquire_ctx); + + return crtc->dev->mode_config.acquire_ctx; +} +EXPORT_SYMBOL(drm_modeset_legacy_acquire_ctx); + +/** + * drm_modeset_unlock_crtc - drop all modeset locks + * @crtc: drm crtc + * + * This drops the crtc lock acquire with drm_modeset_lock_crtc() and all other + * locks acquired through the hidden context. + */ +void drm_modeset_unlock_crtc(struct drm_crtc *crtc) +{ + struct drm_modeset_acquire_ctx *ctx = crtc->acquire_ctx; + + if (WARN_ON(!ctx)) + return; + + crtc->acquire_ctx = NULL; + drm_modeset_drop_locks(ctx); + drm_modeset_acquire_fini(ctx); + + kfree(ctx); +} +EXPORT_SYMBOL(drm_modeset_unlock_crtc); + +/** * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked * @dev: device * @@ -336,6 +420,8 @@ int drm_modeset_lock_all_crtcs(struct drm_device *dev, ret = drm_modeset_lock(&crtc->mutex, ctx); if (ret) return ret; + + WARN_ON(crtc->acquire_ctx); }

return 0; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index bd305ce7b705..39465d716db6 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -401,6 +401,12 @@ struct drm_crtc { struct drm_object_properties properties;

struct drm_crtc_state *state; + + /* + * For legacy crtc ioctls so that atomic drivers can get at the locking + * acquire context. + */ + struct drm_modeset_acquire_ctx *acquire_ctx; };

static inline struct drm_crtc_state * diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index cf61e857bc06..d38e1508f11a 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -120,10 +120,15 @@ int drm_modeset_lock_interruptible(struct drm_modeset_lock *lock, void drm_modeset_unlock(struct drm_modeset_lock *lock);

struct drm_device; +struct drm_crtc;

void drm_modeset_lock_all(struct drm_device *dev); void drm_modeset_unlock_all(struct drm_device *dev); +void drm_modeset_lock_crtc(struct drm_crtc *crtc); +void drm_modeset_unlock_crtc(struct drm_crtc *crtc); void drm_warn_on_modeset_not_all_locked(struct drm_device *dev); +struct drm_modeset_acquire_ctx * +drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc);

int drm_modeset_lock_all_crtcs(struct drm_device *dev, struct drm_modeset_acquire_ctx *ctx);

This is the first cut of atomic helper code. As-is it's only useful to implement a pure atomic interface for plane updates.

Later patches will integrate this with the crtc helpers so that full atomic updates are possible. We also need a pile of helpers to aid drivers in transitioning from the legacy world to the shiny new atomic age.

Finally we need helpers to implement legacy ioctls on top of the atomic interface.

The design of the helpers is fairly simple, but has a unfortunate large interface: - We have ->atomic_check callbacks for crtcs and planes. The idea is that connectors don't need any checking, and if they do they can adjust the relevant crtc driver-private state. So no connector hooks should be needed. Also the crtc helpers integration will do the ->best_encoder checks, so no need for that.

- Framebuffer pinning needs to be done before we can commit to the hw state. This is especially important for async updates where we must pin all buffers before returning to userspace, so that really only hw failures can happen in the asynchronous worker.

Hence we add ->prepare_fb and ->cleanup_fb hooks for this resources management.

- The actual atomic plane commit can't fail (except hw woes), so has void return type. It has three stages: 1. Prepare all affected crtcs with crtc->atomic_begin. Drivers can use this to unset the GO bit or similar latches to prevent plane updates. 2. Update plane state by looping over all changed planes and calling plane->atomic_update. Presuming the hardware is sane and has GO bits drivers can simply bash the state into the hardware in this function. Other drivers might use this to precompute hw state for the final step. 3. Finally latch the update for the next vblank with crtc->atomic_flush. Note that this function doesn't need to wait for the vblank to happen even for the synchronous case.

v2: Clear drm_<obj>_state->state to NULL when swapping in state.

v3: Add TODO that we don't short-circuit plane updates for now. Likely no one will care.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_atomic_helper.c | 349 ++++++++++++++++++++++++++++++++++++ include/drm/drm_atomic_helper.h | 44 +++++ include/drm/drm_crtc.h | 5 + include/drm/drm_crtc_helper.h | 6 + include/drm/drm_plane_helper.h | 30 ++++ 6 files changed, 435 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_atomic_helper.c create mode 100644 include/drm/drm_atomic_helper.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 82e922c02c6d..aae92c684f95 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -25,7 +25,7 @@ drm-$(CONFIG_OF) += drm_of.o drm-usb-y := drm_usb.o

drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ - drm_plane_helper.o drm_dp_mst_topology.o + drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c new file mode 100644 index 000000000000..71e7025068f7 --- /dev/null +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -0,0 +1,349 @@ +/* + * Copyright (C) 2014 Red Hat + * Copyright (C) 2014 Intel Corp. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * Authors: + * Rob Clark robdclark@gmail.com + * Daniel Vetter daniel.vetter@ffwll.ch + */ + +#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_plane_helper.h> +#include <drm/drm_crtc_helper.h> + +static void +drm_atomic_helper_plane_changed(struct drm_atomic_state *state, + struct drm_plane_state *plane_state, + struct drm_plane *plane) +{ + struct drm_crtc_state *crtc_state; + + /* + * TODO: For now we don't bother optimizing out no-op changes - plane + * updates should be fast. + */ + + if (plane->crtc) { + crtc_state = state->crtc_states[drm_crtc_index(plane->crtc)]; + + if (WARN_ON(!crtc_state)) + return; + + crtc_state->planes_changed = true; + } + + if (plane_state->crtc) { + crtc_state = + state->crtc_states[drm_crtc_index(plane_state->crtc)]; + + if (WARN_ON(!crtc_state)) + return; + + crtc_state->planes_changed = true; + } +} + +/** + * drm_atomic_helper_check - validate state object + * @dev: DRM device + * @state: the driver state object + * + * Check the state object to see if the requested state is physically possible. + * Only crtcs and planes have check callbacks, so for any additional (global) + * checking that a driver needs it can simply wrap that around this function. + * Drivers without such needs can directly use this as their ->atomic_check() + * callback. + * + * RETURNS + * Zero for success or -errno + */ +int drm_atomic_helper_check(struct drm_device *dev, + struct drm_atomic_state *state) +{ + int nplanes = dev->mode_config.num_total_plane; + int ncrtcs = dev->mode_config.num_crtc; + int i, ret = 0; + + for (i = 0; i < nplanes; i++) { + struct drm_plane_helper_funcs *funcs; + struct drm_plane *plane = state->planes[i]; + struct drm_plane_state *plane_state = state->plane_states[i]; + + if (!plane) + continue; + + funcs = plane->helper_private; + + drm_atomic_helper_plane_changed(state, plane_state, plane); + + if (!funcs || !funcs->atomic_check) + continue; + + ret = funcs->atomic_check(plane, plane_state); + if (ret) + return ret; + } + + for (i = 0; i < ncrtcs; i++) { + struct drm_crtc_helper_funcs *funcs; + struct drm_crtc *crtc = state->crtcs[i]; + + if (!crtc) + continue; + + funcs = crtc->helper_private; + + if (!funcs || !funcs->atomic_check) + continue; + + ret = funcs->atomic_check(crtc, state->crtc_states[i]); + if (ret) + return ret; + } + + return ret; +} +EXPORT_SYMBOL(drm_atomic_helper_check); + +/** + * drm_atomic_helper_prepare_planes - prepare plane resources after commit + * @dev: DRM device + * @state: atomic state object with old state structures + * + * This function prepares plane state, specifically framebuffers, for the new + * configuration. If any failure is encountered this function will call + * ->cleanup_fb on any already successfully prepared framebuffer. + * + * Returns: + * 0 on success, negative error code on failure. + */ +int drm_atomic_helper_prepare_planes(struct drm_device *dev, + struct drm_atomic_state *state) +{ + int nplanes = dev->mode_config.num_total_plane; + int ret, i; + + for (i = 0; i < nplanes; i++) { + struct drm_plane_helper_funcs *funcs; + struct drm_plane *plane = state->planes[i]; + struct drm_framebuffer *fb; + + if (!plane) + continue; + + funcs = plane->helper_private; + + fb = state->plane_states[i]->fb; + + if (fb && funcs->prepare_fb) { + ret = funcs->prepare_fb(plane, fb); + if (ret) + goto fail; + } + } + + return 0; + +fail: + for (i--; i >= 0; i--) { + struct drm_plane_helper_funcs *funcs; + struct drm_plane *plane = state->planes[i]; + struct drm_framebuffer *fb; + + if (!plane) + continue; + + funcs = plane->helper_private; + + fb = state->plane_states[i]->fb; + + if (fb && funcs->cleanup_fb) + funcs->cleanup_fb(plane, fb); + + } + + return ret; +} +EXPORT_SYMBOL(drm_atomic_helper_prepare_planes); + +/** + * drm_atomic_helper_commit_planes - commit plane state + * @dev: DRM device + * @state: atomic state + * + * This function commits the new plane state using the plane and atomic helper + * functions for planes and crtcs. It assumes that the atomic state has already + * been pushed into the relevant object state pointers, since this step can no + * longer fail. + */ +void drm_atomic_helper_commit_planes(struct drm_device *dev, + struct drm_atomic_state *state) +{ + int nplanes = dev->mode_config.num_total_plane; + int ncrtcs = dev->mode_config.num_crtc; + int i; + + for (i = 0; i < ncrtcs; i++) { + struct drm_crtc_helper_funcs *funcs; + struct drm_crtc *crtc = state->crtcs[i]; + + if (!crtc) + continue; + + funcs = crtc->helper_private; + + if (!funcs || !funcs->atomic_begin) + continue; + + funcs->atomic_begin(crtc); + } + + for (i = 0; i < nplanes; i++) { + struct drm_plane_helper_funcs *funcs; + struct drm_plane *plane = state->planes[i]; + + if (!plane) + continue; + + funcs = plane->helper_private; + + if (!funcs || !funcs->atomic_update) + continue; + + funcs->atomic_update(plane); + } + + for (i = 0; i < ncrtcs; i++) { + struct drm_crtc_helper_funcs *funcs; + struct drm_crtc *crtc = state->crtcs[i]; + + if (!crtc) + continue; + + funcs = crtc->helper_private; + + if (!funcs || !funcs->atomic_flush) + continue; + + funcs->atomic_flush(crtc); + } +} +EXPORT_SYMBOL(drm_atomic_helper_commit_planes); + +/** + * drm_atomic_helper_cleanup_planes - cleanup plane resources after commit + * @dev: DRM device + * @state: atomic state object with old state structures + * + * This function cleans up plane state, specifically framebuffers, from the old + * configuration. Hence the old configuration must be perserved in @old_state to + * be able to call this function. + * + * This function must also be called on the new state when the atomic update + * fails at any point after calling drm_atomic_helper_prepare_planes(). + */ +void drm_atomic_helper_cleanup_planes(struct drm_device *dev, + struct drm_atomic_state *old_state) +{ + int nplanes = dev->mode_config.num_total_plane; + int i; + + for (i = 0; i < nplanes; i++) { + struct drm_plane_helper_funcs *funcs; + struct drm_plane *plane = old_state->planes[i]; + struct drm_framebuffer *old_fb; + + if (!plane) + continue; + + funcs = plane->helper_private; + + old_fb = old_state->plane_states[i]->fb; + + if (old_fb && funcs->cleanup_fb) + funcs->cleanup_fb(plane, old_fb); + } +} +EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes); + +/** + * drm_atomic_helper_swap_state - store atomic state into current sw state + * @dev: DRM device + * @state: atomic state + * + * This function stores the atomic state into the current state pointers in all + * driver objects. It should be called after all failing steps have been done + * and succeeded, but before the actual hardware state is committed. + * + * For cleanup and error recovery the current state for all changed objects will + * be swaped into @state. + * + * With that sequence it fits perfectly into the plane prepare/cleanup sequence: + * + * 1. Call drm_atomic_helper_prepare_planes() with the staged atomic state. + * + * 2. Do any other steps that might fail, call drm_atomic_helper_prepare_planes + * with the staged atomic state. + * + * 3. Put the staged state into the current state pointers with this function. + * + * 4. Actually commit the hardware state. + * + * 5. Call drm_atomic_helper_cleanup_planes with @state, which since step 3 + * contains the old state. Also do any other cleanup required with that state. + */ +void drm_atomic_helper_swap_state(struct drm_device *dev, + struct drm_atomic_state *state) +{ + int i; + + for (i = 0; i < dev->mode_config.num_connector; i++) { + struct drm_connector *connector = state->connectors[i]; + + if (!connector) + continue; + + swap(state->connector_states[i], connector->state); + connector->state->state = NULL; + } + + for (i = 0; i < dev->mode_config.num_crtc; i++) { + struct drm_crtc *crtc = state->crtcs[i]; + + if (!crtc) + continue; + + swap(state->crtc_states[i], crtc->state); + crtc->state->state = NULL; + } + + for (i = 0; i < dev->mode_config.num_total_plane; i++) { + struct drm_plane *plane = state->planes[i]; + + if (!plane) + continue; + + swap(state->plane_states[i], plane->state); + plane->state->state = NULL; + } +} +EXPORT_SYMBOL(drm_atomic_helper_swap_state); diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h new file mode 100644 index 000000000000..79938c62e7ad --- /dev/null +++ b/include/drm/drm_atomic_helper.h @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2014 Red Hat + * Copyright (C) 2014 Intel Corp. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * Authors: + * Rob Clark robdclark@gmail.com + * Daniel Vetter daniel.vetter@ffwll.ch + */ + +#ifndef DRM_ATOMIC_HELPER_H_ +#define DRM_ATOMIC_HELPER_H_ + +int drm_atomic_helper_check(struct drm_device *dev, + struct drm_atomic_state *state); + +int drm_atomic_helper_prepare_planes(struct drm_device *dev, + struct drm_atomic_state *state); +void drm_atomic_helper_commit_planes(struct drm_device *dev, + struct drm_atomic_state *state); +void drm_atomic_helper_cleanup_planes(struct drm_device *dev, + struct drm_atomic_state *old_state); + +void drm_atomic_helper_swap_state(struct drm_device *dev, + struct drm_atomic_state *state); + +#endif /* DRM_ATOMIC_HELPER_H_ */ diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index d79f2e0e8104..ff39aca5c266 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -237,6 +237,9 @@ struct drm_atomic_state; struct drm_crtc_state { bool enable : 1;

+ /* computed state bits used by helpers and drivers */ + bool planes_changed : 1; + struct drm_display_mode mode;

struct drm_pending_vblank_event *event; @@ -757,6 +760,8 @@ struct drm_plane {

enum drm_plane_type type;

+ void *helper_private; + struct drm_plane_state *state; };

diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index a3d75fefd010..adec48b27aa5 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -81,6 +81,12 @@ struct drm_crtc_helper_funcs {

/* disable crtc when not in use - more explicit than dpms off */ void (*disable)(struct drm_crtc *crtc); + + /* atomic helpers */ + int (*atomic_check)(struct drm_crtc *crtc, + struct drm_crtc_state *state); + void (*atomic_begin)(struct drm_crtc *crtc); + void (*atomic_flush)(struct drm_crtc *crtc); };

/** diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h index 52e6870534b2..a6f3a8ed3394 100644 --- a/include/drm/drm_plane_helper.h +++ b/include/drm/drm_plane_helper.h @@ -42,6 +42,28 @@ * planes. */

+/** + * drm_plane_helper_funcs - helper operations for CRTCs + * + * The helper operations are called by the mid-layer CRTC helper. + */ +struct drm_plane_helper_funcs { + int (*prepare_fb)(struct drm_plane *plane, + struct drm_framebuffer *fb); + void (*cleanup_fb)(struct drm_plane *plane, + struct drm_framebuffer *fb); + + int (*atomic_check)(struct drm_plane *plane, + struct drm_plane_state *state); + void (*atomic_update)(struct drm_plane *plane); +}; + +static inline void drm_plane_helper_add(struct drm_plane *plane, + const struct drm_plane_helper_funcs *funcs) +{ + plane->helper_private = (void *)funcs; +} + extern int drm_plane_helper_check_update(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, @@ -68,4 +90,12 @@ extern struct drm_plane *drm_primary_helper_create_plane(struct drm_device *dev, int num_formats);

+int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, + int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h); +int drm_plane_helper_disable(struct drm_plane *plane); + #endif

As usual in both a crtc index and a struct drm_crtc * version.

The function assumes that no one drivers their display below 10Hz, and it will complain if the vblank wait takes longer than that.

v2: Also check dev->max_vblank_counter since some drivers register a fake get_vblank_counter function.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_irq.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 2 ++ 2 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 0de123afdb34..76024fdde452 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -999,6 +999,51 @@ void drm_crtc_vblank_put(struct drm_crtc *crtc) EXPORT_SYMBOL(drm_crtc_vblank_put);

/** + * drm_vblank_wait - wait for one vblank + * @dev: DRM device + * @crtc: crtc index + * + * This waits for one vblank to pass on @crtc, using the irq driver interfaces. + * It is a failure to call this when the vblank irq for @crtc is disable, e.g. + * due to lack of driver support or because the crtc is off. + */ +void drm_vblank_wait(struct drm_device *dev, int crtc) +{ + int ret; + u32 last; + + ret = drm_vblank_get(dev, crtc); + if (WARN_ON(ret)) + return; + + last = drm_vblank_count(dev, crtc); + +#define C (last != drm_vblank_count(dev, crtc)) + ret = wait_event_timeout(dev->vblank[crtc].queue, + C, msecs_to_jiffies(100)); + + WARN_ON(ret == 0); +#undef C + + drm_vblank_put(dev, crtc); +} +EXPORT_SYMBOL(drm_vblank_wait); + +/** + * drm_crtc_vblank_wait - wait for one vblank + * @crtc: DRM crtc + * + * This waits for one vblank to pass on @crtc, using the irq driver interfaces. + * It is a failure to call this when the vblank irq for @crtc is disable, e.g. + * due to lack of driver support or because the crtc is off. + */ +void drm_crtc_vblank_wait(struct drm_crtc *crtc) +{ + drm_vblank_wait(crtc->dev, drm_crtc_index(crtc)); +} +EXPORT_SYMBOL(drm_crtc_vblank_wait); + +/** * drm_vblank_off - disable vblank events on a CRTC * @dev: DRM device * @crtc: CRTC in question diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 06a673894c47..f72e5ef5f7b0 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1355,6 +1355,8 @@ extern int drm_vblank_get(struct drm_device *dev, int crtc); extern void drm_vblank_put(struct drm_device *dev, int crtc); extern int drm_crtc_vblank_get(struct drm_crtc *crtc); extern void drm_crtc_vblank_put(struct drm_crtc *crtc); +extern void drm_vblank_wait(struct drm_device *dev, int crtc); +extern void drm_crtc_vblank_wait(struct drm_crtc *crtc); extern void drm_vblank_off(struct drm_device *dev, int crtc); extern void drm_vblank_on(struct drm_device *dev, int crtc); extern void drm_crtc_vblank_off(struct drm_crtc *crtc);

Converting a driver to the atomic interface can be a daunting undertaking. One of the prerequisites is to have full universal planes support.

To make that transition a bit easier this pathc provides plane helpers which use the new atomic helper callbacks just only for the plane changes. This way the plane update functionality can be tested without being forced to convert everything at once.

Of course a real atomic update capable driver will implement the all plane properties through the atomic interface, so these helpers are mostly transitional. But they can be used to enable proper universal plane support, especially once the crtc helpers have also been adapted.

v2: Use ->atomic_duplicate_state if available.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_plane_helper.c | 181 ++++++++++++++++++++++++++++++++++++- 1 file changed, 180 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index 827ec1a3040b..7befbf017afe 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -27,7 +27,7 @@ #include <drm/drmP.h> #include <drm/drm_plane_helper.h> #include <drm/drm_rect.h> -#include <drm/drm_plane_helper.h> +#include <drm/drm_crtc_helper.h>

#define SUBPIXEL_MASK 0xffff

@@ -369,3 +369,182 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs); } EXPORT_SYMBOL(drm_crtc_init); + +/** + * drm_primary_helper_update() - Helper for primary plane update + * @plane: plane object to update + * @crtc: owning CRTC of owning plane + * @fb: framebuffer to flip onto plane + * @crtc_x: x offset of primary plane on crtc + * @crtc_y: y offset of primary plane on crtc + * @crtc_w: width of primary plane rectangle on crtc + * @crtc_h: height of primary plane rectangle on crtc + * @src_x: x offset of @fb for panning + * @src_y: y offset of @fb for panning + * @src_w: width of source rectangle in @fb + * @src_h: height of source rectangle in @fb + * + * Provides a default plane update handler using the atomic plane update + * functions. It is fully left to the driver to check plane constraints and + * handle corner-cases like a fully occluded or otherwise invisible plane. + * + * This is useful for piecewise transitioning of a driver to the atomic helpers. + * + * RETURNS: + * Zero on success, error code on failure + */ +int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, + int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) +{ + struct drm_plane_state *plane_state; + struct drm_plane_helper_funcs *plane_funcs; + struct drm_crtc_helper_funcs *crtc_funcs; + int ret; + + if (plane->funcs->atomic_duplicate_state) + plane_state = plane->funcs->atomic_duplicate_state(plane); + else if (plane->state) + plane_state = kmemdup(plane->state, sizeof(*plane_state), + GFP_KERNEL); + else + plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL); + if (!plane_state) + return -ENOMEM; + + plane_state->crtc = crtc; + plane_state->fb = fb; + plane_state->crtc_x = crtc_x; + plane_state->crtc_y = crtc_y; + plane_state->crtc_h = crtc_h; + plane_state->crtc_w = crtc_w; + plane_state->src_x = src_x; + plane_state->src_y = src_y; + plane_state->src_h = src_h; + plane_state->src_w = src_w; + + plane_funcs = plane->helper_private; + + if (plane_funcs->atomic_check) { + ret = plane_funcs->atomic_check(plane, plane_state); + if (ret) + goto fail; + } + + if (plane_funcs->prepare_fb) { + ret = plane_funcs->prepare_fb(plane, fb); + if (ret) + goto fail; + } + + /* Point of no return, commit sw state. */ + swap(plane->state, plane_state); + fb = plane_state->fb; + + crtc_funcs = crtc->helper_private; + + if (crtc_funcs && crtc_funcs->atomic_begin) + crtc_funcs->atomic_begin(crtc); + + plane_funcs->atomic_update(plane); + + if (crtc_funcs && crtc_funcs->atomic_flush) + crtc_funcs->atomic_flush(crtc); + + /* There's no way to figure out whether the crtc is running. */ + ret = drm_crtc_vblank_get(crtc); + if (ret == 0) { + drm_crtc_vblank_wait(crtc); + drm_crtc_vblank_put(crtc); + } + + if (plane_funcs->cleanup_fb && fb) + plane_funcs->cleanup_fb(plane, fb); +fail: + kfree(plane_state); + + return ret; +} +EXPORT_SYMBOL(drm_plane_helper_update); + +/** + * drm_primary_helper_disable() - Helper for primary plane disable + * @plane: plane to disable + * + * Provides a default plane disable handler using the atomic plane update + * functions. It is fully left to the driver to check plane constraints and + * handle corner-cases like a fully occluded or otherwise invisible plane. + * + * This is useful for piecewise transitioning of a driver to the atomic helpers. + * + * RETURNS: + * Zero on success, error code on failure + */ +int drm_plane_helper_disable(struct drm_plane *plane) +{ + struct drm_plane_state *plane_state; + struct drm_plane_helper_funcs *plane_funcs; + struct drm_crtc_helper_funcs *crtc_funcs; + struct drm_framebuffer *fb = NULL; + struct drm_crtc *crtc = plane->crtc; + int ret; + + if (WARN_ON(!crtc)) + return -EINVAL; + + if (plane->funcs->atomic_duplicate_state) + plane_state = plane->funcs->atomic_duplicate_state(plane); + else if (plane->state) + plane_state = kmemdup(plane->state, sizeof(*plane_state), + GFP_KERNEL); + else + plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL); + if (!plane_state) + return -ENOMEM; + + plane_funcs = plane->helper_private; + + if (plane_funcs->atomic_check) { + ret = plane_funcs->atomic_check(plane, plane_state); + if (ret) + goto fail; + } + + if (plane_funcs->prepare_fb) { + ret = plane_funcs->prepare_fb(plane, fb); + if (ret) + goto fail; + } + + /* Point of no return, commit sw state. */ + swap(plane->state, plane_state); + fb = plane_state->fb; + + crtc_funcs = crtc->helper_private; + + if (crtc_funcs && crtc_funcs->atomic_begin) + crtc_funcs->atomic_begin(crtc); + + plane_funcs->atomic_update(plane); + + if (crtc_funcs && crtc_funcs->atomic_flush) + crtc_funcs->atomic_flush(crtc); + + /* There's no way to figure out whether the crtc is running. */ + ret = drm_crtc_vblank_get(crtc); + if (ret == 0) { + drm_crtc_vblank_wait(crtc); + drm_crtc_vblank_put(crtc); + } + + if (plane_funcs->cleanup_fb && fb) + plane_funcs->cleanup_fb(plane, fb); +fail: + kfree(plane_state); + + return ret; +} +EXPORT_SYMBOL(drm_plane_helper_disable);

So this is finally the integration of the crtc helper interfaces into the atomic helper functions.

In the check function we now have a few steps:

- First we update the output routing and figure out which crtcs need a full mode set. Suitable encoders are selected using ->best_encoder, with the same semantics as the crtc helpers of implicitly disabling all connectors currently using the encoder.

- Then we pull all other connectors into the state update which feed from a crtc which changes. This must be done do catch mode changes and similar updates - atomic updates are differences on top of the current state.

- Then we call all the various ->mode_fixup to compute the adjusted mode. Note that here we have a slight semantic difference compared to the crtc helpers: We have not yet updated the encoder->crtc link when calling the encoder's ->mode_fixup function. But that's a requirement when converting to atomic since we want to prepare the entire state completely contained with the over drm_atomic_state structure. So this must be carefully checked when converting drivers over to atomic helpers.

- Finally we do call the atomic_check functions on planes and crtcs.

The commit function is also quite a beast:

- The only step that can fail is done first, namely pinning the framebuffers. After that we cross the point of no return, an async commit would push all that into the worker thread.

- The disabling of encoders and connectors is a bit tricky, since depending upon the final state we need to select different crtc helper functions.

- Software tracking is a bit clarified compared to the crtc helpers: We commit the software state before starting to touch the hardware, like crtc helpers. But since we just swap them we still have the old state (i.e. the current hw state) around, which is really handy to write simple disable functions. So no more drm_crtc_helper_disable_all_unused_functions kind of fun because we're leaving unused crtcs/encoders behind. Everything gets shut down in-order now, which is one of the key differences of the i915 helpers compared to crtc helpers and a really nice additional guarantee.

- Like with the plane helpers the atomic commit function waits for one vblank to pass before calling the framebuffer cleanup function.

Compared to Rob's helper approach there's a bunch of upsides:

- All the interfaces which can fail are called in the ->check hook (i.e. ->best_match and the various ->mode_fixup hooks). This means that drivers can just reuse those functions and don't need to move everything into ->atomic_check callbacks. If drivers have no need for additional constraint checking beyong their existing crtc helper callbacks they don't need to do anything.

- The actual commit operation is properly stage: First we prepare framebuffers, which can potentially still fail (due to memory exhausting). This is important for the async case, where this must be done synchronously to correctly return errors.

- The output configuration changes (done with crtc helper functions) and the plane update (using atomic plane helpers) are correctly interleaved: First we shut down any crtcs that need changing, then we update planes and finally we enable everything again. Hardware without GO bits must be more careful with ordering, which this sequence enables.

- Also for hardware with shared output resources (like display PLLs) we first must shut down the old configuration before we can enable the new one. Otherwise we can hit an impossible intermediate state where there's not enough PLLs (which is the point behind atomic updates).

v2: - Ensure that users of ->check update crtc_state->enable correctly. - Update the legacy state in crtc/plane structures. Eventually we want to remove that, but for now the drm core still expects this (especially the plane->fb pointer).

v3: A few changes for better async handling:

- Reorder the software side state commit so that it happens all before we touch the hardware. This way async support becomes very easy since we can punt all the actual hw touching to a worker thread. And as long as we synchronize with that thread (flushing or cancelling, depending upon what the driver can handle) before we commit the next software state there's no need for any locking in the worker thread at all. Which greatly simplifies things.

And as long as we synchronize with all relevant threads we can have a lot of them (e.g. per-crtc for per-crtc updates) running in parallel.

- Expose pre/post plane commit steps separately. We need to expose the actual hw commit step anyway for drivers to be able to implement asynchronous commit workers. But if we expose pre/post and plane commit steps individually we allow drivers to selectively use atomic helpers.

- I've forgotten to call encoder/bridge ->mode_set functions, fix this.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_atomic_helper.c | 592 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc_helper.c | 1 + include/drm/drm_atomic_helper.h | 8 + include/drm/drm_crtc.h | 7 + 4 files changed, 608 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 71e7025068f7..d5d2eb2908cc 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -29,6 +29,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_plane_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_atomic_helper.h>

static void drm_atomic_helper_plane_changed(struct drm_atomic_state *state, @@ -62,6 +63,254 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, } }

+static int +steal_encoder(struct drm_atomic_state *state, + struct drm_encoder *encoder) +{ + struct drm_mode_config *config = &state->dev->mode_config; + struct drm_crtc_state *crtc_state; + struct drm_connector *connector; + struct drm_connector_state *connector_state; + /* + * We can only steal an encoder coming from a connector, which means we + * must already hold the connection_mutex. + */ + WARN_ON(!drm_modeset_is_locked(&config->connection_mutex)); + + if (!encoder->crtc) + return 0; + + crtc_state = drm_atomic_get_crtc_state(state, encoder->crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + crtc_state->mode_changed = true; + + list_for_each_entry(connector, &config->connector_list, head) { + if (connector->state->best_encoder != encoder) + continue; + + connector_state = drm_atomic_get_connector_state(state, + connector); + if (IS_ERR(connector_state)) + return PTR_ERR(connector_state); + + connector_state->crtc = NULL; + connector_state->best_encoder = NULL; + } + + return 0; +} + +static int +update_connector_routing(struct drm_atomic_state *state, int conn_idx) +{ + struct drm_connector_helper_funcs *funcs; + struct drm_encoder *new_encoder; + struct drm_connector *connector; + struct drm_connector_state *connector_state; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int idx, ret; + + connector = state->connectors[conn_idx]; + connector_state = state->connector_states[conn_idx]; + + if (!connector) + return 0; + + if (connector->state->crtc != connector_state->crtc) { + if (connector->state->crtc) { + idx = drm_crtc_index(connector->state->crtc); + + crtc_state = state->crtc_states[idx]; + crtc->state->mode_changed = true; + } + + if (connector_state->crtc) { + idx = drm_crtc_index(connector_state->crtc); + + crtc_state = state->crtc_states[idx]; + crtc->state->mode_changed = true; + } + } + + if (!connector_state->crtc) + return 0; + + funcs = connector->helper_private; + new_encoder = funcs->best_encoder(connector); + + if (!new_encoder) { + DRM_DEBUG_KMS("Now suitable encoder found\n"); + return -EINVAL; + } + + if (new_encoder->crtc) { + ret = steal_encoder(state, new_encoder); + if (ret) { + DRM_DEBUG_KMS("Encoder stealing failed\n"); + return ret; + } + } + + if (new_encoder != connector_state->best_encoder) { + connector_state->best_encoder = new_encoder; + idx = drm_crtc_index(connector_state->crtc); + + crtc_state = state->crtc_states[idx]; + crtc_state->mode_changed = true; + } + + return 0; +} + +static int +mode_fixup(struct drm_atomic_state *state) +{ + int ncrtcs = state->dev->mode_config.num_crtc; + int nconnectors = state->dev->mode_config.num_connector; + struct drm_crtc_state *crtc_state; + struct drm_connector_state *conn_state; + int i; + bool ret; + + for (i = 0; i < ncrtcs; i++) { + crtc_state = state->crtc_states[i]; + + if (!crtc_state || !crtc_state->mode_changed) + continue; + + drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode); + } + + for (i = 0; i < nconnectors; i++) { + struct drm_encoder_helper_funcs *funcs; + struct drm_encoder *encoder; + + conn_state = state->connector_states[i]; + + if (!conn_state || !conn_state->best_encoder) + continue; + + crtc_state = + state->crtc_states[drm_crtc_index(conn_state->crtc)]; + + /* + * Each encoder has at most one connector (since we always steal + * it away), so we won't call ->mode_fixup twice. + */ + encoder = conn_state->best_encoder; + funcs = encoder->helper_private; + + if (encoder->bridge && encoder->bridge->funcs->mode_fixup) { + ret = encoder->bridge->funcs->mode_fixup( + encoder->bridge, &crtc_state->mode, + &crtc_state->adjusted_mode); + if (!ret) { + DRM_DEBUG_KMS("Bridge fixup failed\n"); + return -EINVAL; + } + } + + + ret = funcs->mode_fixup(encoder, &crtc_state->mode, + &crtc_state->adjusted_mode); + if (!ret) { + DRM_DEBUG_KMS("Encoder fixup failed\n"); + return -EINVAL; + } + } + + for (i = 0; i < ncrtcs; i++) { + struct drm_crtc_helper_funcs *funcs; + struct drm_crtc *crtc; + + crtc_state = state->crtc_states[i]; + crtc = state->crtcs[i]; + + if (!crtc_state || !crtc_state->mode_changed) + continue; + + funcs = crtc->helper_private; + ret = funcs->mode_fixup(crtc, &crtc_state->mode, + &crtc_state->adjusted_mode); + if (!ret) { + DRM_DEBUG_KMS("CRTC fixup failed\n"); + return -EINVAL; + } + } + + return 0; +} + +static int +drm_atomic_helper_check_prepare(struct drm_device *dev, + struct drm_atomic_state *state) +{ + int ncrtcs = dev->mode_config.num_crtc; + int nconnectors = dev->mode_config.num_connector; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int i, ret; + + for (i = 0; i < ncrtcs; i++) { + crtc = state->crtcs[i]; + crtc_state = state->crtc_states[i]; + + if (!crtc) + continue; + + if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) + crtc_state->mode_changed = true; + + if (crtc->state->enable != crtc_state->enable) + crtc_state->mode_changed = true; + } + + for (i = 0; i < nconnectors; i++) { + /* + * This only sets crtc->mode_changed for routing changes, + * drivers must set crtc->mode_changed themselves when connector + * properties need to be updated. + */ + ret = update_connector_routing(state, i); + if (ret) + return ret; + } + + /* + * After all the routing has been prepared we need to add in any + * connector which is itself unchanged, but who's crtc changes it's + * configuration. This must be done before calling mode_fixup in case a + * crtc only changed its mode but has the same set of connectors. + */ + for (i = 0; i < ncrtcs; i++) { + crtc = state->crtcs[i]; + crtc_state = state->crtc_states[i]; + + if (!crtc) + continue; + + if (crtc_state->mode_changed) { + bool has_connectors; + + ret = drm_atomic_add_affected_connectors(state, crtc); + if (ret < 0) + return ret; + + has_connectors = drm_atomic_connectors_for_crtc(state, + crtc); + + WARN_ON(crtc_state->enable != has_connectors); + } + } + + ret = mode_fixup(state); + + return 0; +} + /** * drm_atomic_helper_check - validate state object * @dev: DRM device @@ -83,6 +332,10 @@ int drm_atomic_helper_check(struct drm_device *dev, int ncrtcs = dev->mode_config.num_crtc; int i, ret = 0;

+ ret = drm_atomic_helper_check_prepare(dev, state); + if (ret) + return ret; + for (i = 0; i < nplanes; i++) { struct drm_plane_helper_funcs *funcs; struct drm_plane *plane = state->planes[i]; @@ -124,6 +377,344 @@ int drm_atomic_helper_check(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_helper_check);

+static void +disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) +{ + int ncrtcs = old_state->dev->mode_config.num_crtc; + int nconnectors = old_state->dev->mode_config.num_connector; + int i; + + for (i = 0; i < nconnectors; i++) { + struct drm_connector_state *old_conn_state; + struct drm_connector *connector; + struct drm_encoder_helper_funcs *funcs; + struct drm_encoder *encoder; + + old_conn_state = old_state->connector_states[i]; + connector = old_state->connectors[i]; + + /* Shut down everything that's in the changeset and currently + * still on. So need to check the old, saved state. */ + if (!old_conn_state || !old_conn_state->crtc) + continue; + + encoder = connector->state->best_encoder; + funcs = encoder->helper_private; + + if (encoder->bridge) + encoder->bridge->funcs->disable(encoder->bridge); + + /* Right function depends upon target state. */ + if (connector->state->crtc) + funcs->prepare(encoder); + else if (funcs->disable) + funcs->disable(encoder); + else + funcs->dpms(encoder, DRM_MODE_DPMS_OFF); + + if (encoder->bridge) + encoder->bridge->funcs->post_disable(encoder->bridge); + } + + for (i = 0; i < ncrtcs; i++) { + struct drm_crtc_helper_funcs *funcs; + struct drm_crtc *crtc; + + crtc = old_state->crtcs[i]; + + /* Shut down everything that needs a full modeset. */ + if (!crtc || !crtc->state->mode_changed) + continue; + + funcs = crtc->helper_private; + + /* Right function depends upon target state. */ + if (crtc->state->enable) + funcs->prepare(crtc); + else if (funcs->disable) + funcs->disable(crtc); + else + funcs->dpms(crtc, DRM_MODE_DPMS_OFF); + } +} + +static void +set_routing_links(struct drm_device *dev, struct drm_atomic_state *old_state) +{ + int nconnectors = dev->mode_config.num_connector; + int ncrtcs = old_state->dev->mode_config.num_crtc; + int i; + + /* clear out existing links */ + for (i = 0; i < nconnectors; i++) { + struct drm_connector *connector; + + connector = old_state->connectors[i]; + + if (!connector || !connector->encoder) + continue; + + connector->encoder->crtc = NULL; + connector->encoder = NULL; + } + + /* set new links */ + for (i = 0; i < nconnectors; i++) { + struct drm_connector *connector; + + connector = old_state->connectors[i]; + + if (!connector || !connector->state->crtc) + continue; + + connector->encoder = connector->state->best_encoder; + connector->encoder->crtc = connector->state->crtc; + } + + for (i = 0; i < ncrtcs; i++) { + struct drm_crtc *crtc; + + crtc = old_state->crtcs[i]; + + /* Shut down everything that needs a full modeset. */ + if (!crtc) + continue; + + crtc->mode = crtc->state->mode; + crtc->enabled = crtc->state->enable; + crtc->x = crtc->primary->state->src_x >> 16; + crtc->y = crtc->primary->state->src_y >> 16; + } +} + +static void +crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) +{ + int ncrtcs = old_state->dev->mode_config.num_crtc; + int nconnectors = old_state->dev->mode_config.num_connector; + int i; + + for (i = 0; i < ncrtcs; i++) { + struct drm_crtc_helper_funcs *funcs; + struct drm_crtc *crtc; + + crtc = old_state->crtcs[i]; + + if (!crtc || !crtc->state->mode_changed) + continue; + + funcs = crtc->helper_private; + + if (crtc->state->enable) + funcs->mode_set_nofb(crtc); + } + + for (i = 0; i < nconnectors; i++) { + struct drm_connector *connector; + struct drm_crtc_state *new_crtc_state; + struct drm_encoder_helper_funcs *funcs; + struct drm_encoder *encoder; + struct drm_display_mode *mode, *adjusted_mode; + + connector = old_state->connectors[i]; + + if (!connector || !connector->state->best_encoder) + continue; + + encoder = connector->state->best_encoder; + funcs = encoder->helper_private; + new_crtc_state = connector->state->crtc->state; + mode = &new_crtc_state->mode; + adjusted_mode = &new_crtc_state->adjusted_mode; + + funcs->mode_set(encoder, mode, adjusted_mode); + + if (encoder->bridge && encoder->bridge->funcs->mode_set) + encoder->bridge->funcs->mode_set(encoder->bridge, + mode, adjusted_mode); + } +} + +/** + * drm_atomic_helper_commit_planes - modeset commit before plane updates + * @dev: DRM device + * @state: atomic state + * + * This function commits the modeset changes that need to be committed before + * updating planes. It shuts down all the outputs that need to be shut down and + * prepares them (if requried) with the new mode. + */ +void drm_atomic_helper_commit_pre_planes(struct drm_device *dev, + struct drm_atomic_state *state) +{ + disable_outputs(dev, state); + set_routing_links(dev, state); + crtc_set_mode(dev, state); +} +EXPORT_SYMBOL(drm_atomic_helper_commit_pre_planes); + +/** + * drm_atomic_helper_commit_planes - commit plane state + * @dev: DRM device + * @state: atomic state + * + * This function commits the modeset changes that need to be committed after + * updating planes. It enables all the outputs with the new configuration which + * had to be turned off for the update. + */ +void drm_atomic_helper_commit_post_planes(struct drm_device *dev, + struct drm_atomic_state *old_state) +{ + int ncrtcs = old_state->dev->mode_config.num_crtc; + int nconnectors = old_state->dev->mode_config.num_connector; + int i; + + for (i = 0; i < ncrtcs; i++) { + struct drm_crtc_helper_funcs *funcs; + struct drm_crtc *crtc; + + crtc = old_state->crtcs[i]; + + /* Need to filter out CRTCs where only planes change. */ + if (!crtc || !crtc->state->mode_changed) + continue; + + funcs = crtc->helper_private; + + if (crtc->state->enable) + funcs->commit(crtc); + } + + for (i = 0; i < nconnectors; i++) { + struct drm_connector *connector; + struct drm_encoder_helper_funcs *funcs; + struct drm_encoder *encoder; + + connector = old_state->connectors[i]; + + if (!connector || !connector->state->best_encoder) + continue; + + encoder = connector->state->best_encoder; + funcs = encoder->helper_private; + + if (encoder->bridge) + encoder->bridge->funcs->pre_enable(encoder->bridge); + + funcs->commit(encoder); + + if (encoder->bridge) + encoder->bridge->funcs->enable(encoder->bridge); + } +} +EXPORT_SYMBOL(drm_atomic_helper_commit_post_planes); + +static void +wait_for_vblanks(struct drm_device *dev, struct drm_atomic_state *old_state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *old_crtc_state; + int ncrtcs = old_state->dev->mode_config.num_crtc; + int i, ret; + + for (i = 0; i < ncrtcs; i++) { + crtc = old_state->crtcs[i]; + + if (!crtc || !crtc->state->enable) + continue; + + /* no one cares about the old state, so use it for tracking */ + old_crtc_state = old_state->crtc_states[i]; + old_crtc_state->enable = true; + ret = drm_crtc_vblank_get(crtc); + if (ret) { + old_crtc_state->enable = false; + continue; + } + + old_crtc_state->last_vblank_count = drm_vblank_count(dev, i); + } + + for (i = 0; i < ncrtcs; i++) { + crtc = old_state->crtcs[i]; + old_crtc_state = old_state->crtc_states[i]; + + if (!crtc || !old_crtc_state->enable) + continue; + +#define C (old_crtc_state->last_vblank_count != drm_vblank_count(dev, i)) + wait_event(dev->vblank[drm_crtc_index(crtc)].queue, C); + +#undef C + + drm_crtc_vblank_put(crtc); + } +} + +/** + * drm_atomic_helper_check - commit validated state object + * @dev: DRM device + * @state: the driver state object + * @async: asynchronous commit + * + * This function commits a with drm_atomic_helper_check() pre-validated state + * object. This can still fail when e.g. the framebuffer reservation fails. For + * now this doesn't implement asynchronous commits. + * + * RETURNS + * Zero for success or -errno. + */ +int drm_atomic_helper_commit(struct drm_device *dev, + struct drm_atomic_state *state, + bool async) +{ + int ret; + + if (async) + return -EBUSY; + + ret = drm_atomic_helper_prepare_planes(dev, state); + if (ret) + return ret; + + /* + * This is the point of no return - everything below never fails except + * when the hw goes bonghits. Which means we can commit the new state on + * the software side now. + */ + + drm_atomic_helper_swap_state(dev, state); + + /* + * Everything below can be run asynchronously withou the need to grab + * any modeset locks at all under one conditions: It must be guaranteed + * that the asynchronous work has either been cancelled (if the driver + * supports it, which at least requires that the framebuffers get + * cleaned up with drm_atomic_helper_cleanup_planes()) or completed + * before the new state gets committed on the software side with + * drm_atomic_helper_swap_state(). + * + * This scheme allows new atomic state updates to be prepared and + * checked in parallel to the asynchronous completion of the previous + * update. Which is important since compositors need to figure out the + * composition of the next frame right after having submitted the + * current layout. + */ + + drm_atomic_helper_commit_pre_planes(dev, state); + + drm_atomic_helper_commit_planes(dev, state); + + drm_atomic_helper_commit_post_planes(dev, state); + + wait_for_vblanks(dev, state); + + drm_atomic_helper_cleanup_planes(dev, state); + + return 0; +} +EXPORT_SYMBOL(drm_atomic_helper_commit); + /** * drm_atomic_helper_prepare_planes - prepare plane resources after commit * @dev: DRM device @@ -230,6 +821,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, continue;

funcs->atomic_update(plane); + plane->fb = plane->state->fb; }

for (i = 0; i < ncrtcs; i++) { diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 751dcc9ba163..7e838104e72e 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -921,6 +921,7 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod

crtc_state->enable = true; crtc_state->planes_changed = true; + crtc_state->mode_changed = true; drm_mode_copy(&crtc_state->mode, mode); drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode);

diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 79938c62e7ad..9781ce739e10 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -30,6 +30,14 @@

int drm_atomic_helper_check(struct drm_device *dev, struct drm_atomic_state *state); +int drm_atomic_helper_commit(struct drm_device *dev, + struct drm_atomic_state *state, + bool async); + +void drm_atomic_helper_commit_pre_planes(struct drm_device *dev, + struct drm_atomic_state *state); +void drm_atomic_helper_commit_post_planes(struct drm_device *dev, + struct drm_atomic_state *old_state);

int drm_atomic_helper_prepare_planes(struct drm_device *dev, struct drm_atomic_state *state); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 1dec9911abc7..59db2cd34b50 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -239,6 +239,10 @@ struct drm_crtc_state {

/* computed state bits used by helpers and drivers */ bool planes_changed : 1; + bool mode_changed : 1; + + /* last_vblank_count: for vblank waits before cleanup */ + u32 last_vblank_count;

/* adjusted_mode: for use by helpers and drivers */ struct drm_display_mode adjusted_mode; @@ -441,6 +445,9 @@ drm_crtc_destroy_state(struct drm_crtc *crtc, struct drm_connector_state { struct drm_crtc *crtc;

+ /* best_encoder: for use by helpers and drivers */ + struct drm_encoder *best_encoder; + struct drm_atomic_state *state; };

Well, except page_flip since that requires async commit, which isn't there yet.

For the functions which changes planes there's a bit of trickery involved to keep the fb refcounting working. But otherwise fairly straight-forward atomic updates.

The property setting functions are still a bit incomplete. Once we have generic properties (e.g. rotation, but also all the properties needed by the atomic ioctl) we need to filter those out and parse them in the helper. Preferrably with the same function as used by the real atomic ioctl implementation.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_atomic_helper.c | 481 ++++++++++++++++++++++++++++++++++++ include/drm/drm_atomic_helper.h | 21 ++ 2 files changed, 502 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index d5d2eb2908cc..1b13394cf020 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -939,3 +939,484 @@ void drm_atomic_helper_swap_state(struct drm_device *dev, } } EXPORT_SYMBOL(drm_atomic_helper_swap_state); + +/** + * drm_atomic_helper_update_plane - Helper for primary plane update using atomic + * @plane: plane object to update + * @crtc: owning CRTC of owning plane + * @fb: framebuffer to flip onto plane + * @crtc_x: x offset of primary plane on crtc + * @crtc_y: y offset of primary plane on crtc + * @crtc_w: width of primary plane rectangle on crtc + * @crtc_h: height of primary plane rectangle on crtc + * @src_x: x offset of @fb for panning + * @src_y: y offset of @fb for panning + * @src_w: width of source rectangle in @fb + * @src_h: height of source rectangle in @fb + * + * Provides a default plane update handler using the atomic driver interface. + * + * RETURNS: + * Zero on success, error code on failure + */ +int drm_atomic_helper_update_plane(struct drm_plane *plane, + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) +{ + struct drm_atomic_state *state; + struct drm_plane_state *plane_state; + int ret = 0; + + state = drm_atomic_state_alloc(plane->dev); + if (!state) + return -ENOMEM; + + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); +retry: + plane_state = drm_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) { + ret = PTR_ERR(plane_state); + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + plane_state->crtc = crtc; + plane_state->fb = fb; + plane_state->crtc_x = crtc_x; + plane_state->crtc_y = crtc_y; + plane_state->crtc_h = crtc_h; + plane_state->crtc_w = crtc_w; + plane_state->src_x = src_x; + plane_state->src_y = src_y; + plane_state->src_h = src_h; + plane_state->src_w = src_w; + + ret = drm_atomic_commit(state); + if (ret == -EDEADLK) + goto backoff; +fail: + drm_atomic_state_free(state); + + return ret; +backoff: + drm_atomic_legacy_backoff(state); + drm_atomic_state_clear(state); + + /* + * Someone might have exchanged the framebuffer while we dropped locks + * in the backoff code. We need to fix up the fb refcount tracking the + * core does for us. + */ + plane->old_fb = plane->fb; + + goto retry; +} +EXPORT_SYMBOL(drm_atomic_helper_update_plane); + +/** + * drm_atomic_helper_disable_plane - Helper for primary plane disable using * atomic + * @plane: plane to disable + * + * Provides a default plane disablle handler using the atomic driver interface. + * + * RETURNS: + * Zero on success, error code on failure + */ +int drm_atomic_helper_disable_plane(struct drm_plane *plane) +{ + struct drm_atomic_state *state; + struct drm_plane_state *plane_state; + int ret = 0; + + state = drm_atomic_state_alloc(plane->dev); + if (!state) + return -ENOMEM; + + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(plane->crtc); +retry: + plane_state = drm_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) { + ret = PTR_ERR(plane_state); + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + plane_state->crtc = NULL; + plane_state->fb = NULL; + plane_state->crtc_x = 0; + plane_state->crtc_y = 0; + plane_state->crtc_h = 0; + plane_state->crtc_w = 0; + plane_state->src_x = 0; + plane_state->src_y = 0; + plane_state->src_h = 0; + plane_state->src_w = 0; + + ret = drm_atomic_commit(state); + if (ret == -EDEADLK) + goto backoff; +fail: + drm_atomic_state_free(state); + + return ret; +backoff: + drm_atomic_legacy_backoff(state); + drm_atomic_state_clear(state); + + /* + * Someone might have exchanged the framebuffer while we dropped locks + * in the backoff code. We need to fix up the fb refcount tracking the + * core does for us. + */ + plane->old_fb = plane->fb; + + goto retry; +} +EXPORT_SYMBOL(drm_atomic_helper_disable_plane); + +static int update_output_state(struct drm_atomic_state *state, + struct drm_mode_set *set) +{ + struct drm_device *dev = set->crtc->dev; + struct drm_connector_state *conn_state; + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + int nconnectors = state->dev->mode_config.num_connector; + int ncrtcs = state->dev->mode_config.num_crtc; + int ret, i, j; + + /* First grab all affected connector/crtc states. */ + ret = drm_modeset_lock(&dev->mode_config.connection_mutex, + state->acquire_ctx); + if (ret) + return ret; + + for (i = 0; i < set->num_connectors; i++) { + conn_state = drm_atomic_get_connector_state(state, + set->connectors[i]); + if (IS_ERR(conn_state)) + return PTR_ERR(conn_state); + } + + for (i = 0; i < ncrtcs; i++) { + crtc = state->crtcs[i]; + + if (!crtc) + continue; + + ret = drm_atomic_add_affected_connectors(state, crtc); + if (ret) + return ret; + } + + for (i = 0; i < nconnectors; i++) { + struct drm_connector *connector; + + connector = state->connectors[i]; + conn_state = state->connector_states[i]; + + if (!connector) + continue; + + if (conn_state->crtc == crtc) + conn_state->crtc = NULL; + + for (j = 0; j < set->num_connectors; j++) { + if (set->connectors[j] == connector) { + conn_state->crtc = crtc; + break; + } + } + } + + for (i = 0; i < ncrtcs; i++) { + crtc = state->crtcs[i]; + crtc_state = state->crtc_states[i]; + + if (!crtc) + continue; + + crtc_state->enable = + drm_atomic_connectors_for_crtc(state, crtc); + } + + return 0; +} + +/** + * drm_atomic_helper_set_config - set a new config from userspace + * @set: mode set configuration + * + * Provides a default crtc set_config handler using the atomic driver interface. + * + * Returns: + * Returns 0 on success, negative errno numbers on failure. + */ +int drm_atomic_helper_set_config(struct drm_mode_set *set) +{ + struct drm_atomic_state *state; + struct drm_crtc *crtc = set->crtc; + struct drm_crtc_state *crtc_state; + struct drm_plane_state *primary_state; + int ret = 0; + + state = drm_atomic_state_alloc(crtc->dev); + if (!state) + return -ENOMEM; + + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); +retry: + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) { + ret = PTR_ERR(crtc_state); + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + if (!set->mode) { + crtc_state->enable = false; + goto commit; + } + + crtc_state->enable = true; + drm_mode_copy(&crtc_state->mode, set->mode); + + primary_state = drm_atomic_get_plane_state(state, crtc->primary); + if (IS_ERR(primary_state)) { + ret = PTR_ERR(primary_state); + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + primary_state->crtc = crtc; + primary_state->fb = set->fb; + primary_state->crtc_x = 0; + primary_state->crtc_y = 0; + primary_state->crtc_h = set->mode->hdisplay; + primary_state->crtc_w = set->mode->vdisplay; + primary_state->src_x = set->x << 16; + primary_state->src_y = set->y << 16; + primary_state->src_h = set->mode->hdisplay << 16; + primary_state->src_w = set->mode->hdisplay << 16; + + ret = update_output_state(state, set); + if (ret) { + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + +commit: + ret = drm_atomic_commit(state); + if (ret == -EDEADLK) + goto backoff; +fail: + drm_atomic_state_free(state); + + return ret; +backoff: + drm_atomic_legacy_backoff(state); + drm_atomic_state_clear(state); + + /* + * Someone might have exchanged the framebuffer while we dropped locks + * in the backoff code. We need to fix up the fb refcount tracking the + * core does for us. + */ + crtc->primary->old_fb = crtc->primary->fb; + + goto retry; +} + +/** + * drm_atomic_helper_crtc_set_property - helper for crtc prorties + * @crtc: DRM crtc + * @prorty: DRM property + * @val: value of property + * + * Provides a default plane disablle handler using the atomic driver interface. + * + * RETURNS: + * Zero on success, error code on failure + */ +int +drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc, + struct drm_property *property, + uint64_t val) +{ + struct drm_atomic_state *state; + struct drm_crtc_state *crtc_state; + int ret = 0; + + state = drm_atomic_state_alloc(crtc->dev); + if (!state) + return -ENOMEM; + + /* ->set_property is always called with all locks held. */ + state->acquire_ctx = crtc->dev->mode_config.acquire_ctx; +retry: + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) { + ret = PTR_ERR(crtc_state); + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + ret = crtc->funcs->atomic_set_property(crtc, crtc_state, + property, val); + if (ret) { + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + ret = drm_atomic_commit(state); + if (ret == -EDEADLK) + goto backoff; +fail: + drm_atomic_state_free(state); + + return ret; +backoff: + drm_atomic_legacy_backoff(state); + drm_atomic_state_clear(state); + + goto retry; +} +EXPORT_SYMBOL(drm_atomic_helper_crtc_set_property); + +/** + * drm_atomic_helper_plane_set_property - helper for plane prorties + * @plane: DRM plane + * @prorty: DRM property + * @val: value of property + * + * Provides a default plane disablle handler using the atomic driver interface. + * + * RETURNS: + * Zero on success, error code on failure + */ +int +drm_atomic_helper_plane_set_property(struct drm_plane *plane, + struct drm_property *property, + uint64_t val) +{ + struct drm_atomic_state *state; + struct drm_plane_state *plane_state; + int ret = 0; + + state = drm_atomic_state_alloc(plane->dev); + if (!state) + return -ENOMEM; + + /* ->set_property is always called with all locks held. */ + state->acquire_ctx = plane->dev->mode_config.acquire_ctx; +retry: + plane_state = drm_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) { + ret = PTR_ERR(plane_state); + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + ret = plane->funcs->atomic_set_property(plane, plane_state, + property, val); + if (ret) { + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + ret = drm_atomic_commit(state); + if (ret == -EDEADLK) + goto backoff; +fail: + drm_atomic_state_free(state); + + return ret; +backoff: + drm_atomic_legacy_backoff(state); + drm_atomic_state_clear(state); + + goto retry; +} +EXPORT_SYMBOL(drm_atomic_helper_plane_set_property); + +/** + * drm_atomic_helper_connector_set_property - helper for connector prorties + * @connector: DRM connector + * @prorty: DRM property + * @val: value of property + * + * Provides a default plane disablle handler using the atomic driver interface. + * + * RETURNS: + * Zero on success, error code on failure + */ +int +drm_atomic_helper_connector_set_property(struct drm_connector *connector, + struct drm_property *property, + uint64_t val) +{ + struct drm_atomic_state *state; + struct drm_connector_state *connector_state; + int ret = 0; + + state = drm_atomic_state_alloc(connector->dev); + if (!state) + return -ENOMEM; + + /* ->set_property is always called with all locks held. */ + state->acquire_ctx = connector->dev->mode_config.acquire_ctx; +retry: + connector_state = drm_atomic_get_connector_state(state, connector); + if (IS_ERR(connector_state)) { + ret = PTR_ERR(connector_state); + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + ret = connector->funcs->atomic_set_property(connector, connector_state, + property, val); + if (ret) { + if (ret == -EDEADLK) + goto backoff; + else + goto fail; + } + + ret = drm_atomic_commit(state); + if (ret == -EDEADLK) + goto backoff; +fail: + drm_atomic_state_free(state); + + return ret; +backoff: + drm_atomic_legacy_backoff(state); + drm_atomic_state_clear(state); + + goto retry; +} +EXPORT_SYMBOL(drm_atomic_helper_connector_set_property); diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 9781ce739e10..8cd6fe7a48e5 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -49,4 +49,25 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, void drm_atomic_helper_swap_state(struct drm_device *dev, struct drm_atomic_state *state);

+/* implementations for legacy interfaces */ +int drm_atomic_helper_update_plane(struct drm_plane *plane, + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h); +int drm_atomic_helper_disable_plane(struct drm_plane *plane); +int drm_atomic_helper_set_config(struct drm_mode_set *set); + +int drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc, + struct drm_property *property, + uint64_t val); +int drm_atomic_helper_plane_set_property(struct drm_plane *plane, + struct drm_property *property, + uint64_t val); +int drm_atomic_helper_connector_set_property(struct drm_connector *connector, + struct drm_property *property, + uint64_t val); + #endif /* DRM_ATOMIC_HELPER_H_ */

No helper function to do it all yet provided since no driver has support for driver core fences yet. Which we'd need to make the implementation really generic.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_atomic_helper.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 9f7c45b91fe2..423f6bcec362 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -716,6 +716,43 @@ int drm_atomic_helper_commit(struct drm_device *dev, EXPORT_SYMBOL(drm_atomic_helper_commit);

/** + * DOC: Implementing async commit + * + * For now the atomic helpers don't support async commit directly. If there is + * real need it could be added though, using the dma-buf fence infrastructure + * for generic synchronization with outstanding rendering. + * + * For now drivers have to implement async commit themselves, with the following + * sequence being the recommened one: + * + * 1. Run drm_atomic_helper_prepare_planes() first. This is the only function + * which commit needs to call which can fail, so we want to run it first and + * synchronously. + * + * 2. Synchronize with any outstanding async commit worker threads which might + * affect the new state update. This can be done by either cancelling or + * flushing the work items, depending upon whether the driver can deal with + * cancelled updates. Note that it is important to ensure that the framebuffer + * cleanup is still done when cancelling. + * + * For sufficient parallelism it is recommended to have a work item per crtc + * (for updates which don't touch global state) and a global one. Then we only + * need to synchronize with the crtc work items for changed crtcs and the global + * work item, which allows nice concurrent updates on disjoint sets of crtcs. + * + * 3. The software state is updated synchronously with + * drm_atomic_helper_swap_state. Doing this under the protection of all modeset + * locks means concurrent callers never see inconsistent state. And doing this + * while it's guranateed that no relevant async worker runs means that async + * workers do not need grab any locks. Actually they must not grab locks, for + * otherwise the work flushing will deadlock. + * + * 4. Schedule a work item to do all subsequent steps, using the split-out + * commit helpers: a) pre-plane commit b) plane commit c) post-plane commit and + * then cleaning up the framebuffers after one vblank has passed. + */ + +/** * drm_atomic_helper_prepare_planes - prepare plane resources after commit * @dev: DRM device * @state: atomic state object with old state structures

In the fbdev code we want to do trylocks only to avoid deadlocks and other ugly issues. Thus far we've only grabbed the overall modeset lock, but that already failed to exclude a pile of potential concurrent operations. With proper atomic support this will be worse.

So add a trylock mode to the modeset locking code which attempts all locks only with trylocks, if possible. We need to track this in the locking functions themselves and can't restrict this to drivers since driver-private w/w mutexes must be treated the same way.

There's still the issue that other driver private locks aren't handled here at all, but well can't have everything. With this we will at least not regress, even once atomic allows lots of concurrent kms activity.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fb_helper.c | 10 ++++---- drivers/gpu/drm/drm_modeset_lock.c | 51 +++++++++++++++++++++++++++----------- include/drm/drm_modeset_lock.h | 6 +++++ 3 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 3144db9dc0f1..66a438ab4e7f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -419,11 +419,11 @@ static bool drm_fb_helper_force_kernel_mode(void) if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) continue;

- /* NOTE: we use lockless flag below to avoid grabbing other - * modeset locks. So just trylock the underlying mutex - * directly: + /* + * NOTE: Use trylock mode to avoid deadlocks and sleeping in + * panic context. */ - if (!mutex_trylock(&dev->mode_config.mutex)) { + if (!__drm_modeset_lock_all(dev, true)) { error = true; continue; } @@ -432,7 +432,7 @@ static bool drm_fb_helper_force_kernel_mode(void) if (ret) error = true;

- mutex_unlock(&dev->mode_config.mutex); + drm_modeset_unlock_all(dev); } return error; } diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 412b396af740..42ba9ca699ad 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -56,27 +56,27 @@ */

-/** - * drm_modeset_lock_all - take all modeset locks - * @dev: drm device - * - * This function takes all modeset locks, suitable where a more fine-grained - * scheme isn't (yet) implemented. Locks must be dropped with - * drm_modeset_unlock_all. - */ -void drm_modeset_lock_all(struct drm_device *dev) +int __drm_modeset_lock_all(struct drm_device *dev, + bool trylock) { struct drm_mode_config *config = &dev->mode_config; struct drm_modeset_acquire_ctx *ctx; int ret;

- ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); - if (WARN_ON(!ctx)) - return; + ctx = kzalloc(sizeof(*ctx), + trylock ? GFP_ATOMIC : GFP_KERNEL); + if (!ctx) + return -ENOMEM;

- mutex_lock(&config->mutex); + if (trylock) { + if (!mutex_trylock(&config->mutex)) + return -EBUSY; + } else { + mutex_lock(&config->mutex); + }

drm_modeset_acquire_init(ctx, 0); + ctx->trylock_only = trylock;

retry: ret = drm_modeset_lock(&config->connection_mutex, ctx); @@ -95,13 +95,29 @@ retry:

drm_warn_on_modeset_not_all_locked(dev);

- return; + return 0;

fail: if (ret == -EDEADLK) { drm_modeset_backoff(ctx); goto retry; } + + return ret; +} +EXPORT_SYMBOL(__drm_modeset_lock_all); + +/** + * drm_modeset_lock_all - take all modeset locks + * @dev: drm device + * + * This function takes all modeset locks, suitable where a more fine-grained + * scheme isn't (yet) implemented. Locks must be dropped with + * drm_modeset_unlock_all. + */ +void drm_modeset_lock_all(struct drm_device *dev) +{ + WARN_ON(__drm_modeset_lock_all(dev, false) != 0); } EXPORT_SYMBOL(drm_modeset_lock_all);

@@ -287,7 +303,12 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,

WARN_ON(ctx->contended);

- if (interruptible && slow) { + if (ctx->trylock_only) { + if (!ww_mutex_trylock(&lock->mutex)) + return -EBUSY; + else + return 0; + } else if (interruptible && slow) { ret = ww_mutex_lock_slow_interruptible(&lock->mutex, &ctx->ww_ctx); } else if (interruptible) { ret = ww_mutex_lock_interruptible(&lock->mutex, &ctx->ww_ctx); diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index d38e1508f11a..a3f736d24382 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -53,6 +53,11 @@ struct drm_modeset_acquire_ctx { * list of held locks (drm_modeset_lock) */ struct list_head locked; + + /** + * Trylock mode, use only for panic handlers! + */ + bool trylock_only; };

/** @@ -123,6 +128,7 @@ struct drm_device; struct drm_crtc;

void drm_modeset_lock_all(struct drm_device *dev); +int __drm_modeset_lock_all(struct drm_device *dev, bool trylock); void drm_modeset_unlock_all(struct drm_device *dev); void drm_modeset_lock_crtc(struct drm_crtc *crtc); void drm_modeset_unlock_crtc(struct drm_crtc *crtc);