From: Brian Starkey Date: Wed, 29 Mar 2017 17:42:33 +0100 Subject: drm: writeback: Add out-fences for writeback connectors Git-commit: b13cc8dd588434e2aec781e6d12224e4c408ac18 Patch-mainline: v4.19-rc1 References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166 Add the WRITEBACK_OUT_FENCE_PTR property to writeback connectors, to enable userspace to get a fence which will signal once the writeback is complete. It is not allowed to request an out-fence without a framebuffer attached to the connector. A timeline is added to drm_writeback_connector for use by the writeback out-fences. In the case of a commit failure or DRM_MODE_ATOMIC_TEST_ONLY, the fence is set to -1. Changes from v2: - Rebase onto Gustavo Padovan's v9 explicit sync series - Change out_fence_ptr type to s32 __user * - Set *out_fence_ptr to -1 in drm_atomic_connector_set_property - Store fence in drm_writeback_job Gustavo Padovan: - Move out_fence_ptr out of connector_state - Signal fence from drm_writeback_signal_completion instead of in driver directly Changes from v3: - Rebase onto commit 7e9081c5aac7 ("drm/fence: fix memory overwrite when setting out_fence fd") (change out_fence_ptr to s32 __user *, for real this time.) - Update documentation around WRITEBACK_OUT_FENCE_PTR Signed-off-by: Brian Starkey [rebased and fixed conflicts] Signed-off-by: Mihail Atanassov Signed-off-by: Liviu Dudau Reviewed-by: Eric Anholt Reviewed-by: Sean Paul Link: https://patchwork.freedesktop.org/patch/229036/ Acked-by: Petr Tesarik --- drivers/gpu/drm/drm_atomic.c | 99 +++++++++++++++++++++++++++++++++--- drivers/gpu/drm/drm_writeback.c | 109 +++++++++++++++++++++++++++++++++++++++- include/drm/drm_atomic.h | 8 ++ include/drm/drm_connector.h | 8 +- include/drm/drm_mode_config.h | 8 ++ include/drm/drm_writeback.h | 41 ++++++++++++++- 6 files changed, 257 insertions(+), 16 deletions(-) --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -326,6 +326,35 @@ static s32 __user *get_out_fence_for_crt return fence_ptr; } +static int set_out_fence_for_connector(struct drm_atomic_state *state, + struct drm_connector *connector, + s32 __user *fence_ptr) +{ + unsigned int index = drm_connector_index(connector); + + if (!fence_ptr) + return 0; + + if (put_user(-1, fence_ptr)) + return -EFAULT; + + state->connectors[index].out_fence_ptr = fence_ptr; + + return 0; +} + +static s32 __user *get_out_fence_for_connector(struct drm_atomic_state *state, + struct drm_connector *connector) +{ + unsigned int index = drm_connector_index(connector); + s32 __user *fence_ptr; + + fence_ptr = state->connectors[index].out_fence_ptr; + state->connectors[index].out_fence_ptr = NULL; + + return fence_ptr; +} + /** * drm_atomic_set_mode_for_crtc - set mode for CRTC * @state: the CRTC whose incoming state to update @@ -735,6 +764,12 @@ static int drm_atomic_connector_check(st return -EINVAL; } + if (writeback_job->out_fence && !writeback_job->fb) { + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] requesting out-fence without framebuffer\n", + connector->base.id, connector->name); + return -EINVAL; + } + return 0; } @@ -1375,6 +1410,11 @@ static int drm_atomic_connector_set_prop if (fb) drm_framebuffer_put(fb); return ret; + } else if (property == config->writeback_out_fence_ptr_property) { + s32 __user *fence_ptr = u64_to_user_ptr(val); + + return set_out_fence_for_connector(state->state, connector, + fence_ptr); } else if (connector->funcs->atomic_set_property) { return connector->funcs->atomic_set_property(connector, state, property, val); @@ -1464,6 +1504,8 @@ drm_atomic_connector_get_property(struct } else if (property == config->writeback_fb_id_property) { /* Writeback framebuffer is one-shot, write and forget */ *val = 0; + } else if (property == config->writeback_out_fence_ptr_property) { + *val = 0; } else if (connector->funcs->atomic_get_property) { return connector->funcs->atomic_get_property(connector, state, property, val); @@ -2304,7 +2346,7 @@ static int setup_out_fence(struct drm_ou return 0; } -static int prepare_crtc_signaling(struct drm_device *dev, +static int prepare_signaling(struct drm_device *dev, struct drm_atomic_state *state, struct drm_mode_atomic *arg, struct drm_file *file_priv, @@ -2313,6 +2355,8 @@ static int prepare_crtc_signaling(struct { struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; + struct drm_connector *conn; + struct drm_connector_state *conn_state; int i, c = 0, ret; if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) @@ -2378,6 +2422,43 @@ static int prepare_crtc_signaling(struct c++; } + for_each_new_connector_in_state(state, conn, conn_state, i) { + struct drm_writeback_job *job; + struct drm_out_fence_state *f; + struct dma_fence *fence; + s32 __user *fence_ptr; + + fence_ptr = get_out_fence_for_connector(state, conn); + if (!fence_ptr) + continue; + + job = drm_atomic_get_writeback_job(conn_state); + if (!job) + return -ENOMEM; + + f = krealloc(*fence_state, sizeof(**fence_state) * + (*num_fences + 1), GFP_KERNEL); + if (!f) + return -ENOMEM; + + memset(&f[*num_fences], 0, sizeof(*f)); + + f[*num_fences].out_fence_ptr = fence_ptr; + *fence_state = f; + + fence = drm_writeback_get_out_fence((struct drm_writeback_connector *)conn); + if (!fence) + return -ENOMEM; + + ret = setup_out_fence(&f[(*num_fences)++], fence); + if (ret) { + dma_fence_put(fence); + return ret; + } + + job->out_fence = fence; + } + /* * Having this flag means user mode pends on event which will never * reach due to lack of at least one CRTC for signaling @@ -2388,11 +2469,11 @@ static int prepare_crtc_signaling(struct return 0; } -static void complete_crtc_signaling(struct drm_device *dev, - struct drm_atomic_state *state, - struct drm_out_fence_state *fence_state, - unsigned int num_fences, - bool install_fds) +static void complete_signaling(struct drm_device *dev, + struct drm_atomic_state *state, + struct drm_out_fence_state *fence_state, + unsigned int num_fences, + bool install_fds) { struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; @@ -2562,8 +2643,8 @@ retry: drm_mode_object_put(obj); } - ret = prepare_crtc_signaling(dev, state, arg, file_priv, &fence_state, - &num_fences); + ret = prepare_signaling(dev, state, arg, file_priv, &fence_state, + &num_fences); if (ret) goto out; @@ -2579,7 +2660,7 @@ retry: } out: - complete_crtc_signaling(dev, state, fence_state, num_fences, !ret); + complete_signaling(dev, state, fence_state, num_fences, !ret); if (ret == -EDEADLK) { drm_atomic_state_clear(state); --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -14,6 +14,7 @@ #include #include #include +#include /** * DOC: overview @@ -31,6 +32,16 @@ * framebuffer applies only to a single commit (see below). A framebuffer may * not be attached while the CRTC is off. * + * Unlike with planes, when a writeback framebuffer is removed by userspace DRM + * makes no attempt to remove it from active use by the connector. This is + * because no method is provided to abort a writeback operation, and in any + * case making a new commit whilst a writeback is ongoing is undefined (see + * WRITEBACK_OUT_FENCE_PTR below). As soon as the current writeback is finished, + * the framebuffer will automatically no longer be in active use. As it will + * also have already been removed from the framebuffer list, there will be no + * way for any userspace application to retrieve a reference to it in the + * intervening period. + * * Writeback connectors have some additional properties, which userspace * can use to query and control them: * @@ -47,8 +58,54 @@ * data is an array of u32 DRM_FORMAT_* fourcc values. * Userspace can use this blob to find out what pixel formats are supported * by the connector's writeback engine. + * + * "WRITEBACK_OUT_FENCE_PTR": + * Userspace can use this property to provide a pointer for the kernel to + * fill with a sync_file file descriptor, which will signal once the + * writeback is finished. The value should be the address of a 32-bit + * signed integer, cast to a u64. + * Userspace should wait for this fence to signal before making another + * commit affecting any of the same CRTCs, Planes or Connectors. + * **Failure to do so will result in undefined behaviour.** + * For this reason it is strongly recommended that all userspace + * applications making use of writeback connectors *always* retrieve an + * out-fence for the commit and use it appropriately. + * From userspace, this property will always read as zero. */ +#define fence_to_wb_connector(x) container_of(x->lock, \ + struct drm_writeback_connector, \ + fence_lock) + +static const char *drm_writeback_fence_get_driver_name(struct dma_fence *fence) +{ + struct drm_writeback_connector *wb_connector = + fence_to_wb_connector(fence); + + return wb_connector->base.dev->driver->name; +} + +static const char * +drm_writeback_fence_get_timeline_name(struct dma_fence *fence) +{ + struct drm_writeback_connector *wb_connector = + fence_to_wb_connector(fence); + + return wb_connector->timeline_name; +} + +static bool drm_writeback_fence_enable_signaling(struct dma_fence *fence) +{ + return true; +} + +static const struct dma_fence_ops drm_writeback_fence_ops = { + .get_driver_name = drm_writeback_fence_get_driver_name, + .get_timeline_name = drm_writeback_fence_get_timeline_name, + .enable_signaling = drm_writeback_fence_enable_signaling, + .wait = dma_fence_default_wait, +}; + static int create_writeback_properties(struct drm_device *dev) { struct drm_property *prop; @@ -72,6 +129,15 @@ static int create_writeback_properties(s dev->mode_config.writeback_pixel_formats_property = prop; } + if (!dev->mode_config.writeback_out_fence_ptr_property) { + prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC, + "WRITEBACK_OUT_FENCE_PTR", 0, + U64_MAX); + if (!prop) + return -ENOMEM; + dev->mode_config.writeback_out_fence_ptr_property = prop; + } + return 0; } @@ -141,6 +207,15 @@ int drm_writeback_connector_init(struct INIT_LIST_HEAD(&wb_connector->job_queue); spin_lock_init(&wb_connector->job_lock); + wb_connector->fence_context = dma_fence_context_alloc(1); + spin_lock_init(&wb_connector->fence_lock); + snprintf(wb_connector->timeline_name, + sizeof(wb_connector->timeline_name), + "CONNECTOR:%d-%s", connector->base.id, connector->name); + + drm_object_attach_property(&connector->base, + config->writeback_out_fence_ptr_property, 0); + drm_object_attach_property(&connector->base, config->writeback_fb_id_property, 0); @@ -210,6 +285,7 @@ static void cleanup_work(struct work_str /** * drm_writeback_signal_completion - Signal the completion of a writeback job * @wb_connector: The writeback connector whose job is complete + * @status: Status code to set in the writeback out_fence (0 for success) * * Drivers should call this to signal the completion of a previously queued * writeback job. It should be called as soon as possible after the hardware @@ -223,7 +299,8 @@ static void cleanup_work(struct work_str * See also: drm_writeback_queue_job() */ void -drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector) +drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector, + int status) { unsigned long flags; struct drm_writeback_job *job; @@ -232,8 +309,15 @@ drm_writeback_signal_completion(struct d job = list_first_entry_or_null(&wb_connector->job_queue, struct drm_writeback_job, list_entry); - if (job) + if (job) { list_del(&job->list_entry); + if (job->out_fence) { + if (status) + dma_fence_set_error(job->out_fence, status); + dma_fence_signal(job->out_fence); + dma_fence_put(job->out_fence); + } + } spin_unlock_irqrestore(&wb_connector->job_lock, flags); if (WARN_ON(!job)) @@ -243,3 +327,24 @@ drm_writeback_signal_completion(struct d queue_work(system_long_wq, &job->cleanup_work); } EXPORT_SYMBOL(drm_writeback_signal_completion); + +struct dma_fence * +drm_writeback_get_out_fence(struct drm_writeback_connector *wb_connector) +{ + struct dma_fence *fence; + + if (WARN_ON(wb_connector->base.connector_type != + DRM_MODE_CONNECTOR_WRITEBACK)) + return NULL; + + fence = kzalloc(sizeof(*fence), GFP_KERNEL); + if (!fence) + return NULL; + + dma_fence_init(fence, &drm_writeback_fence_ops, + &wb_connector->fence_lock, wb_connector->fence_context, + ++wb_connector->fence_seqno); + + return fence; +} +EXPORT_SYMBOL(drm_writeback_get_out_fence); --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -160,6 +160,14 @@ struct __drm_crtcs_state { struct __drm_connnectors_state { struct drm_connector *ptr; struct drm_connector_state *state, *old_state, *new_state; + /** + * @out_fence_ptr: + * + * User-provided pointer which the kernel uses to return a sync_file + * file descriptor. Used by writeback connectors to signal completion of + * the writeback. + */ + s32 __user *out_fence_ptr; }; struct drm_private_obj; --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -441,10 +441,10 @@ struct drm_connector_state { /** * @writeback_job: Writeback job for writeback connectors * - * Holds the framebuffer for a writeback connector. As the writeback - * completion may be asynchronous to the normal commit cycle, the - * writeback job lifetime is managed separately from the normal atomic - * state by this object. + * Holds the framebuffer and out-fence for a writeback connector. As + * the writeback completion may be asynchronous to the normal commit + * cycle, the writeback job lifetime is managed separately from the + * normal atomic state by this object. * * See also: drm_writeback_queue_job() and * drm_writeback_signal_completion() --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -798,6 +798,14 @@ struct drm_mode_config { * See also: drm_writeback_connector_init() */ struct drm_property *writeback_pixel_formats_property; + /** + * @writeback_out_fence_ptr_property: Property for writeback connectors, + * fd pointer representing the outgoing fences for a writeback + * connector. Userspace should provide a pointer to a value of type s32, + * and then cast that pointer to u64. + * See also: drm_writeback_connector_init() + */ + struct drm_property *writeback_out_fence_ptr_property; /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow; --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -50,6 +50,32 @@ struct drm_writeback_connector { * drm_writeback_signal_completion() */ struct list_head job_queue; + + /** + * @fence_context: + * + * timeline context used for fence operations. + */ + unsigned int fence_context; + /** + * @fence_lock: + * + * spinlock to protect the fences in the fence_context. + */ + spinlock_t fence_lock; + /** + * @fence_seqno: + * + * Seqno variable used as monotonic counter for the fences + * created on the connector's timeline. + */ + unsigned long fence_seqno; + /** + * @timeline_name: + * + * The name of the connector's fence timeline. + */ + char timeline_name[32]; }; struct drm_writeback_job { @@ -75,6 +101,13 @@ struct drm_writeback_job { * directly, use drm_atomic_set_writeback_fb_for_connector() */ struct drm_framebuffer *fb; + + /** + * @out_fence: + * + * Fence which will signal once the writeback has completed + */ + struct dma_fence *out_fence; }; int drm_writeback_connector_init(struct drm_device *dev, @@ -87,5 +120,11 @@ void drm_writeback_queue_job(struct drm_ struct drm_writeback_job *job); void drm_writeback_cleanup_job(struct drm_writeback_job *job); -void drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector); + +void +drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector, + int status); + +struct dma_fence * +drm_writeback_get_out_fence(struct drm_writeback_connector *wb_connector); #endif