From 783195ec1cada862d54dee8f312a60bcbba5c0e4 Mon Sep 17 00:00:00 2001 From: Christian König Date: Thu, 8 Nov 2018 09:39:46 +0100 Subject: drm/syncobj: disable the timeline UAPI for now v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until we have sorted out all problems. v2: return -EINVAL during create if flag is set. Signed-off-by: Christian König Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/260937/ --- include/drm/drm_syncobj.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'include/drm') diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 29244cbcd05e..ffd1f4fcf519 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,9 @@ struct drm_syncobj_cb; +/* Move the define here for the moment to avoid exposing the UAPI just yet */ +#define DRM_SYNCOBJ_CREATE_TYPE_TIMELINE (1 << 1) + enum drm_syncobj_type { DRM_SYNCOBJ_TYPE_BINARY, DRM_SYNCOBJ_TYPE_TIMELINE -- cgit v1.2.3 From 131280a162e7fc2a539bb939efd28dd0b964c62c Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 8 Nov 2018 08:04:22 -0800 Subject: drm: Revert syncobj timeline changes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Daniel suggested I submit this, since we're still seeing regressions from it. This is a revert to before 48197bc564c7 ("drm: add syncobj timeline support v9") and its followon fixes. Fixes this on first V3D testcase execution: [ 48.767088] ============================================ [ 48.772410] WARNING: possible recursive locking detected [ 48.777739] 4.19.0-rc6+ #489 Not tainted [ 48.781668] -------------------------------------------- [ 48.786993] shader_runner/3284 is trying to acquire lock: [ 48.792408] ce309d7f (&(&array->lock)->rlock){....}, at: dma_fence_add_callback+0x30/0x23c [ 48.800714] [ 48.800714] but task is already holding lock: [ 48.806559] c5952bd3 (&(&array->lock)->rlock){....}, at: dma_fence_add_callback+0x30/0x23c [ 48.814862] [ 48.814862] other info that might help us debug this: [ 48.821410] Possible unsafe locking scenario: [ 48.821410] [ 48.827338] CPU0 [ 48.829788] ---- [ 48.832239] lock(&(&array->lock)->rlock); [ 48.836434] lock(&(&array->lock)->rlock); [ 48.840640] [ 48.840640] *** DEADLOCK *** [ 48.840640] [ 48.846582] May be due to missing lock nesting notation [ 130.763560] 1 lock held by cts-runner/3270: [ 130.767745] #0: 7834b793 (&(&array->lock)->rlock){-...}, at: dma_fence_add_callback+0x30/0x23c [ 130.776461] stack backtrace: [ 130.780825] CPU: 1 PID: 3270 Comm: cts-runner Not tainted 4.19.0-rc6+ #486 [ 130.787706] Hardware name: Broadcom STB (Flattened Device Tree) [ 130.793645] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 130.801404] [] (show_stack) from [] (dump_stack+0xa8/0xd4) [ 130.808642] [] (dump_stack) from [] (__lock_acquire+0x848/0x1a68) [ 130.816483] [] (__lock_acquire) from [] (lock_acquire+0xd8/0x22c) [ 130.824326] [] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x54/0x68) [ 130.832777] [] (_raw_spin_lock_irqsave) from [] (dma_fence_add_callback+0x30/0x23c) [ 130.842183] [] (dma_fence_add_callback) from [] (dma_fence_array_enable_signaling+0x58/0xec) [ 130.852371] [] (dma_fence_array_enable_signaling) from [] (dma_fence_add_callback+0xe8/0x23c) [ 130.862647] [] (dma_fence_add_callback) from [] (drm_syncobj_wait_ioctl+0x518/0x614) [ 130.872143] [] (drm_syncobj_wait_ioctl) from [] (drm_ioctl_kernel+0xb0/0xf0) [ 130.880940] [] (drm_ioctl_kernel) from [] (drm_ioctl+0x1d8/0x390) [ 130.888782] [] (drm_ioctl) from [] (do_vfs_ioctl+0xb0/0x8ac) [ 130.896187] [] (do_vfs_ioctl) from [] (ksys_ioctl+0x34/0x60) [ 130.903593] [] (ksys_ioctl) from [] (ret_fast_syscall+0x0/0x28) Cc: Chunming Zhou Cc: Christian König Cc: Daniel Vetter Signed-off-by: Eric Anholt Acked-by: Christian König Signed-off-by: Christian König Link: https://patchwork.freedesktop.org/patch/261044/ --- drivers/gpu/drm/drm_syncobj.c | 359 +++++++++--------------------------------- include/drm/drm_syncobj.h | 76 ++++----- 2 files changed, 105 insertions(+), 330 deletions(-) (limited to 'include/drm') diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index da2b85eec6cf..90e0a6dce086 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -56,9 +56,6 @@ #include "drm_internal.h" #include -/* merge normal syncobj to timeline syncobj, the point interval is 1 */ -#define DRM_SYNCOBJ_BINARY_POINT 1 - struct drm_syncobj_stub_fence { struct dma_fence base; spinlock_t lock; @@ -74,29 +71,7 @@ static const struct dma_fence_ops drm_syncobj_stub_fence_ops = { .get_timeline_name = drm_syncobj_stub_fence_get_name, }; -struct drm_syncobj_signal_pt { - struct dma_fence_array *fence_array; - u64 value; - struct list_head list; -}; - -static DEFINE_SPINLOCK(signaled_fence_lock); -static struct dma_fence signaled_fence; -static struct dma_fence *drm_syncobj_get_stub_fence(void) -{ - spin_lock(&signaled_fence_lock); - if (!signaled_fence.ops) { - dma_fence_init(&signaled_fence, - &drm_syncobj_stub_fence_ops, - &signaled_fence_lock, - 0, 0); - dma_fence_signal_locked(&signaled_fence); - } - spin_unlock(&signaled_fence_lock); - - return dma_fence_get(&signaled_fence); -} /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -123,27 +98,6 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, } EXPORT_SYMBOL(drm_syncobj_find); -static struct dma_fence * -drm_syncobj_find_signal_pt_for_point(struct drm_syncobj *syncobj, - uint64_t point) -{ - struct drm_syncobj_signal_pt *signal_pt; - - if ((syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) && - (point <= syncobj->timeline)) - return drm_syncobj_get_stub_fence(); - - list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) { - if (point > signal_pt->value) - continue; - if ((syncobj->type == DRM_SYNCOBJ_TYPE_BINARY) && - (point != signal_pt->value)) - continue; - return dma_fence_get(&signal_pt->fence_array->base); - } - return NULL; -} - static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, struct drm_syncobj_cb *cb, drm_syncobj_func_t func) @@ -152,158 +106,53 @@ static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, list_add_tail(&cb->node, &syncobj->cb_list); } -static void drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj, - struct dma_fence **fence, - struct drm_syncobj_cb *cb, - drm_syncobj_func_t func) +static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj, + struct dma_fence **fence, + struct drm_syncobj_cb *cb, + drm_syncobj_func_t func) { - u64 pt_value = 0; - - WARN_ON(*fence); + int ret; - if (syncobj->type == DRM_SYNCOBJ_TYPE_BINARY) { - /*BINARY syncobj always wait on last pt */ - pt_value = syncobj->signal_point; + *fence = drm_syncobj_fence_get(syncobj); + if (*fence) + return 1; - if (pt_value == 0) - pt_value += DRM_SYNCOBJ_BINARY_POINT; - } - - mutex_lock(&syncobj->cb_mutex); - spin_lock(&syncobj->pt_lock); - *fence = drm_syncobj_find_signal_pt_for_point(syncobj, pt_value); - spin_unlock(&syncobj->pt_lock); - if (!*fence) + spin_lock(&syncobj->lock); + /* We've already tried once to get a fence and failed. Now that we + * have the lock, try one more time just to be sure we don't add a + * callback when a fence has already been set. + */ + if (syncobj->fence) { + *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, + lockdep_is_held(&syncobj->lock))); + ret = 1; + } else { + *fence = NULL; drm_syncobj_add_callback_locked(syncobj, cb, func); - mutex_unlock(&syncobj->cb_mutex); -} - -static void drm_syncobj_remove_callback(struct drm_syncobj *syncobj, - struct drm_syncobj_cb *cb) -{ - mutex_lock(&syncobj->cb_mutex); - list_del_init(&cb->node); - mutex_unlock(&syncobj->cb_mutex); -} + ret = 0; + } + spin_unlock(&syncobj->lock); -static void drm_syncobj_init(struct drm_syncobj *syncobj) -{ - spin_lock(&syncobj->pt_lock); - syncobj->timeline_context = dma_fence_context_alloc(1); - syncobj->timeline = 0; - syncobj->signal_point = 0; - init_waitqueue_head(&syncobj->wq); - - INIT_LIST_HEAD(&syncobj->signal_pt_list); - spin_unlock(&syncobj->pt_lock); + return ret; } -static void drm_syncobj_fini(struct drm_syncobj *syncobj) +void drm_syncobj_add_callback(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb, + drm_syncobj_func_t func) { - struct drm_syncobj_signal_pt *signal_pt = NULL, *tmp; - - spin_lock(&syncobj->pt_lock); - list_for_each_entry_safe(signal_pt, tmp, - &syncobj->signal_pt_list, list) { - list_del(&signal_pt->list); - dma_fence_put(&signal_pt->fence_array->base); - kfree(signal_pt); - } - spin_unlock(&syncobj->pt_lock); + spin_lock(&syncobj->lock); + drm_syncobj_add_callback_locked(syncobj, cb, func); + spin_unlock(&syncobj->lock); } -static int drm_syncobj_create_signal_pt(struct drm_syncobj *syncobj, - struct dma_fence *fence, - u64 point) +void drm_syncobj_remove_callback(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb) { - struct drm_syncobj_signal_pt *signal_pt = - kzalloc(sizeof(struct drm_syncobj_signal_pt), GFP_KERNEL); - struct drm_syncobj_signal_pt *tail_pt; - struct dma_fence **fences; - int num_fences = 0; - int ret = 0, i; - - if (!signal_pt) - return -ENOMEM; - if (!fence) - goto out; - - fences = kmalloc_array(sizeof(void *), 2, GFP_KERNEL); - if (!fences) { - ret = -ENOMEM; - goto out; - } - fences[num_fences++] = dma_fence_get(fence); - /* timeline syncobj must take this dependency */ - if (syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) { - spin_lock(&syncobj->pt_lock); - if (!list_empty(&syncobj->signal_pt_list)) { - tail_pt = list_last_entry(&syncobj->signal_pt_list, - struct drm_syncobj_signal_pt, list); - fences[num_fences++] = - dma_fence_get(&tail_pt->fence_array->base); - } - spin_unlock(&syncobj->pt_lock); - } - signal_pt->fence_array = dma_fence_array_create(num_fences, fences, - syncobj->timeline_context, - point, false); - if (!signal_pt->fence_array) { - ret = -ENOMEM; - goto fail; - } - - spin_lock(&syncobj->pt_lock); - if (syncobj->signal_point >= point) { - DRM_WARN("A later signal is ready!"); - spin_unlock(&syncobj->pt_lock); - goto exist; - } - signal_pt->value = point; - list_add_tail(&signal_pt->list, &syncobj->signal_pt_list); - syncobj->signal_point = point; - spin_unlock(&syncobj->pt_lock); - wake_up_all(&syncobj->wq); - - return 0; -exist: - dma_fence_put(&signal_pt->fence_array->base); -fail: - for (i = 0; i < num_fences; i++) - dma_fence_put(fences[i]); - kfree(fences); -out: - kfree(signal_pt); - return ret; + spin_lock(&syncobj->lock); + list_del_init(&cb->node); + spin_unlock(&syncobj->lock); } -static void drm_syncobj_garbage_collection(struct drm_syncobj *syncobj) -{ - struct drm_syncobj_signal_pt *signal_pt, *tmp, *tail_pt; - - spin_lock(&syncobj->pt_lock); - tail_pt = list_last_entry(&syncobj->signal_pt_list, - struct drm_syncobj_signal_pt, - list); - list_for_each_entry_safe(signal_pt, tmp, - &syncobj->signal_pt_list, list) { - if (syncobj->type == DRM_SYNCOBJ_TYPE_BINARY && - signal_pt == tail_pt) - continue; - if (dma_fence_is_signaled(&signal_pt->fence_array->base)) { - syncobj->timeline = signal_pt->value; - list_del(&signal_pt->list); - dma_fence_put(&signal_pt->fence_array->base); - kfree(signal_pt); - } else { - /*signal_pt is in order in list, from small to big, so - * the later must not be signal either */ - break; - } - } - - spin_unlock(&syncobj->pt_lock); -} /** * drm_syncobj_replace_fence - replace fence in a sync object. * @syncobj: Sync object to replace fence in @@ -316,30 +165,28 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, u64 point, struct dma_fence *fence) { - u64 pt_value = point; - - drm_syncobj_garbage_collection(syncobj); - if (syncobj->type == DRM_SYNCOBJ_TYPE_BINARY) { - if (!fence) { - drm_syncobj_fini(syncobj); - drm_syncobj_init(syncobj); - return; - } - pt_value = syncobj->signal_point + - DRM_SYNCOBJ_BINARY_POINT; - } - drm_syncobj_create_signal_pt(syncobj, fence, pt_value); - if (fence) { - struct drm_syncobj_cb *cur, *tmp; - LIST_HEAD(cb_list); + struct dma_fence *old_fence; + struct drm_syncobj_cb *cur, *tmp; + + if (fence) + dma_fence_get(fence); + + spin_lock(&syncobj->lock); + + old_fence = rcu_dereference_protected(syncobj->fence, + lockdep_is_held(&syncobj->lock)); + rcu_assign_pointer(syncobj->fence, fence); - mutex_lock(&syncobj->cb_mutex); + if (fence != old_fence) { list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { list_del_init(&cur->node); cur->func(syncobj, cur); } - mutex_unlock(&syncobj->cb_mutex); } + + spin_unlock(&syncobj->lock); + + dma_fence_put(old_fence); } EXPORT_SYMBOL(drm_syncobj_replace_fence); @@ -362,64 +209,6 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) return 0; } -static int -drm_syncobj_point_get(struct drm_syncobj *syncobj, u64 point, u64 flags, - struct dma_fence **fence) -{ - int ret = 0; - - if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { - ret = wait_event_interruptible(syncobj->wq, - point <= syncobj->signal_point); - if (ret < 0) - return ret; - } - spin_lock(&syncobj->pt_lock); - *fence = drm_syncobj_find_signal_pt_for_point(syncobj, point); - if (!*fence) - ret = -EINVAL; - spin_unlock(&syncobj->pt_lock); - return ret; -} - -/** - * drm_syncobj_search_fence - lookup and reference the fence in a sync object or - * in a timeline point - * @syncobj: sync object pointer - * @point: timeline point - * @flags: DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT or not - * @fence: out parameter for the fence - * - * if flags is DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT, the function will block - * here until specific timeline points is reached. - * if not, you need a submit thread and block in userspace until all future - * timeline points have materialized, only then you can submit to the kernel, - * otherwise, function will fail to return fence. - * - * Returns 0 on success or a negative error value on failure. On success @fence - * contains a reference to the fence, which must be released by calling - * dma_fence_put(). - */ -int drm_syncobj_search_fence(struct drm_syncobj *syncobj, u64 point, - u64 flags, struct dma_fence **fence) -{ - u64 pt_value = point; - - if (!syncobj) - return -ENOENT; - - drm_syncobj_garbage_collection(syncobj); - if (syncobj->type == DRM_SYNCOBJ_TYPE_BINARY) { - /*BINARY syncobj always wait on last pt */ - pt_value = syncobj->signal_point; - - if (pt_value == 0) - pt_value += DRM_SYNCOBJ_BINARY_POINT; - } - return drm_syncobj_point_get(syncobj, pt_value, flags, fence); -} -EXPORT_SYMBOL(drm_syncobj_search_fence); - /** * drm_syncobj_find_fence - lookup and reference the fence in a sync object * @file_private: drm file private pointer @@ -429,7 +218,7 @@ EXPORT_SYMBOL(drm_syncobj_search_fence); * @fence: out parameter for the fence * * This is just a convenience function that combines drm_syncobj_find() and - * drm_syncobj_lookup_fence(). + * drm_syncobj_fence_get(). * * Returns 0 on success or a negative error value on failure. On success @fence * contains a reference to the fence, which must be released by calling @@ -440,11 +229,16 @@ int drm_syncobj_find_fence(struct drm_file *file_private, struct dma_fence **fence) { struct drm_syncobj *syncobj = drm_syncobj_find(file_private, handle); - int ret; + int ret = 0; - ret = drm_syncobj_search_fence(syncobj, point, flags, fence); - if (syncobj) - drm_syncobj_put(syncobj); + if (!syncobj) + return -ENOENT; + + *fence = drm_syncobj_fence_get(syncobj); + if (!*fence) { + ret = -EINVAL; + } + drm_syncobj_put(syncobj); return ret; } EXPORT_SYMBOL(drm_syncobj_find_fence); @@ -460,7 +254,7 @@ void drm_syncobj_free(struct kref *kref) struct drm_syncobj *syncobj = container_of(kref, struct drm_syncobj, refcount); - drm_syncobj_fini(syncobj); + drm_syncobj_replace_fence(syncobj, 0, NULL); kfree(syncobj); } EXPORT_SYMBOL(drm_syncobj_free); @@ -493,13 +287,7 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags, kref_init(&syncobj->refcount); INIT_LIST_HEAD(&syncobj->cb_list); - spin_lock_init(&syncobj->pt_lock); - mutex_init(&syncobj->cb_mutex); - if (flags & DRM_SYNCOBJ_CREATE_TYPE_TIMELINE) - syncobj->type = DRM_SYNCOBJ_TYPE_TIMELINE; - else - syncobj->type = DRM_SYNCOBJ_TYPE_BINARY; - drm_syncobj_init(syncobj); + spin_lock_init(&syncobj->lock); if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) { ret = drm_syncobj_assign_null_handle(syncobj); @@ -782,8 +570,7 @@ drm_syncobj_create_ioctl(struct drm_device *dev, void *data, return -EOPNOTSUPP; /* no valid flags yet */ - if (args->flags & ~(DRM_SYNCOBJ_CREATE_SIGNALED | - DRM_SYNCOBJ_CREATE_TYPE_TIMELINE)) + if (args->flags & ~DRM_SYNCOBJ_CREATE_SIGNALED) return -EINVAL; return drm_syncobj_create_as_handle(file_private, @@ -876,8 +663,9 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, struct syncobj_wait_entry *wait = container_of(cb, struct syncobj_wait_entry, syncobj_cb); - drm_syncobj_search_fence(syncobj, 0, 0, &wait->fence); - + /* This happens inside the syncobj lock */ + wait->fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, + lockdep_is_held(&syncobj->lock))); wake_up_process(wait->task); } @@ -903,8 +691,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, signaled_count = 0; for (i = 0; i < count; ++i) { entries[i].task = current; - drm_syncobj_search_fence(syncobjs[i], 0, 0, - &entries[i].fence); + entries[i].fence = drm_syncobj_fence_get(syncobjs[i]); if (!entries[i].fence) { if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { continue; @@ -935,9 +722,6 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { for (i = 0; i < count; ++i) { - if (entries[i].fence) - continue; - drm_syncobj_fence_get_or_add_callback(syncobjs[i], &entries[i].fence, &entries[i].syncobj_cb, @@ -1169,13 +953,12 @@ drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, if (ret < 0) return ret; - for (i = 0; i < args->count_handles; i++) { - drm_syncobj_fini(syncobjs[i]); - drm_syncobj_init(syncobjs[i]); - } + for (i = 0; i < args->count_handles; i++) + drm_syncobj_replace_fence(syncobjs[i], 0, NULL); + drm_syncobj_array_free(syncobjs, args->count_handles); - return ret; + return 0; } int diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index ffd1f4fcf519..2eda44def639 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,18 +30,10 @@ struct drm_syncobj_cb; -/* Move the define here for the moment to avoid exposing the UAPI just yet */ -#define DRM_SYNCOBJ_CREATE_TYPE_TIMELINE (1 << 1) - -enum drm_syncobj_type { - DRM_SYNCOBJ_TYPE_BINARY, - DRM_SYNCOBJ_TYPE_TIMELINE -}; - /** * struct drm_syncobj - sync object. * - * This structure defines a generic sync object which is timeline based. + * This structure defines a generic sync object which wraps a &dma_fence. */ struct drm_syncobj { /** @@ -49,42 +41,21 @@ struct drm_syncobj { */ struct kref refcount; /** - * @type: indicate syncobj type - */ - enum drm_syncobj_type type; - /** - * @wq: wait signal operation work queue - */ - wait_queue_head_t wq; - /** - * @timeline_context: fence context used by timeline - */ - u64 timeline_context; - /** - * @timeline: syncobj timeline value, which indicates point is signaled. + * @fence: + * NULL or a pointer to the fence bound to this object. + * + * This field should not be used directly. Use drm_syncobj_fence_get() + * and drm_syncobj_replace_fence() instead. */ - u64 timeline; + struct dma_fence __rcu *fence; /** - * @signal_point: which indicates the latest signaler point. + * @cb_list: List of callbacks to call when the &fence gets replaced. */ - u64 signal_point; - /** - * @signal_pt_list: signaler point list. - */ - struct list_head signal_pt_list; - - /** - * @cb_list: List of callbacks to call when the &fence gets replaced. - */ struct list_head cb_list; /** - * @pt_lock: Protects pt list. - */ - spinlock_t pt_lock; - /** - * @cb_mutex: Protects syncobj cb list. + * @lock: Protects &cb_list and write-locks &fence. */ - struct mutex cb_mutex; + spinlock_t lock; /** * @file: A file backing for this syncobj. */ @@ -97,7 +68,7 @@ typedef void (*drm_syncobj_func_t)(struct drm_syncobj *syncobj, /** * struct drm_syncobj_cb - callback for drm_syncobj_add_callback * @node: used by drm_syncob_add_callback to append this struct to - * &drm_syncobj.cb_list + * &drm_syncobj.cb_list * @func: drm_syncobj_func_t to call * * This struct will be initialized by drm_syncobj_add_callback, additional @@ -135,6 +106,29 @@ drm_syncobj_put(struct drm_syncobj *obj) kref_put(&obj->refcount, drm_syncobj_free); } +/** + * drm_syncobj_fence_get - get a reference to a fence in a sync object + * @syncobj: sync object. + * + * This acquires additional reference to &drm_syncobj.fence contained in @obj, + * if not NULL. It is illegal to call this without already holding a reference. + * No locks required. + * + * Returns: + * Either the fence of @obj or NULL if there's none. + */ +static inline struct dma_fence * +drm_syncobj_fence_get(struct drm_syncobj *syncobj) +{ + struct dma_fence *fence; + + rcu_read_lock(); + fence = dma_fence_get_rcu_safe(&syncobj->fence); + rcu_read_unlock(); + + return fence; +} + struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, u32 handle); void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, u64 point, @@ -148,7 +142,5 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags, int drm_syncobj_get_handle(struct drm_file *file_private, struct drm_syncobj *syncobj, u32 *handle); int drm_syncobj_get_fd(struct drm_syncobj *syncobj, int *p_fd); -int drm_syncobj_search_fence(struct drm_syncobj *syncobj, u64 point, u64 flags, - struct dma_fence **fence); #endif -- cgit v1.2.3 From f001488112c43fea6b7558e15f71d64ee7495286 Mon Sep 17 00:00:00 2001 From: Noralf Trønnes Date: Sat, 10 Nov 2018 15:56:43 +0100 Subject: drm/driver: Add defaults for .gem_prime_export/import callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The majority of drivers use drm_gem_prime_export() and drm_gem_prime_import() for these callbacks so let's make them the default. Signed-off-by: Noralf Trønnes Acked-by: Daniel Vetter Acked-by: Christian König Link: https://patchwork.freedesktop.org/patch/msgid/20181110145647.17580-2-noralf@tronnes.org --- Documentation/gpu/todo.rst | 7 +++++++ drivers/gpu/drm/drm_prime.c | 10 ++++++++-- include/drm/drm_drv.h | 4 ++++ 3 files changed, 19 insertions(+), 2 deletions(-) (limited to 'include/drm') diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 5c9d86c962af..abc31956ef15 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -234,6 +234,13 @@ efficient. Contact: Daniel Vetter +Defaults for .gem_prime_import and export +----------------------------------------- + +Most drivers don't need to set drm_driver->gem_prime_import and +->gem_prime_export now that drm_gem_prime_import() and drm_gem_prime_export() +are the default. + Core refactorings ================= diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 8d54d51a6b6b..ba6c7e02a2ae 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -529,7 +529,10 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, return dmabuf; } - dmabuf = dev->driver->gem_prime_export(dev, obj, flags); + if (dev->driver->gem_prime_export) + dmabuf = dev->driver->gem_prime_export(dev, obj, flags); + else + dmabuf = drm_gem_prime_export(dev, obj, flags); if (IS_ERR(dmabuf)) { /* normally the created dma-buf takes ownership of the ref, * but if that fails then drop the ref @@ -762,7 +765,10 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, /* never seen this one, need to import */ mutex_lock(&dev->object_name_lock); - obj = dev->driver->gem_prime_import(dev, dma_buf); + if (dev->driver->gem_prime_import) + obj = dev->driver->gem_prime_import(dev, dma_buf); + else + obj = drm_gem_prime_import(dev, dma_buf); if (IS_ERR(obj)) { ret = PTR_ERR(obj); goto out_unlock; diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 3199ef70c007..dbb2f6ad184a 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -471,6 +471,8 @@ struct drm_driver { * @gem_prime_export: * * export GEM -> dmabuf + * + * This defaults to drm_gem_prime_export() if not set. */ struct dma_buf * (*gem_prime_export)(struct drm_device *dev, struct drm_gem_object *obj, int flags); @@ -478,6 +480,8 @@ struct drm_driver { * @gem_prime_import: * * import dmabuf -> GEM + * + * This defaults to drm_gem_prime_import() if not set. */ struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev, struct dma_buf *dma_buf); -- cgit v1.2.3 From 7698799f9554deeb0f3abb22857c3deb41c10db8 Mon Sep 17 00:00:00 2001 From: Noralf Trønnes Date: Sat, 10 Nov 2018 15:56:44 +0100 Subject: drm/prime: Add drm_gem_prime_mmap() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a generic PRIME GEM mmap function. v2: Fix link in docs (Daniel Vetter) Suggested-by: Daniel Vetter Signed-off-by: Noralf Trønnes Reviewed-by: Daniel Vetter Acked-by: Christian König Link: https://patchwork.freedesktop.org/patch/msgid/20181110145647.17580-3-noralf@tronnes.org --- drivers/gpu/drm/drm_prime.c | 37 +++++++++++++++++++++++++++++++++++++ include/drm/drm_prime.h | 1 + 2 files changed, 38 insertions(+) (limited to 'include/drm') diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index ba6c7e02a2ae..6a4fe0216e27 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -651,6 +651,43 @@ out_unlock: } EXPORT_SYMBOL(drm_gem_prime_handle_to_fd); +/** + * drm_gem_prime_mmap - PRIME mmap function for GEM drivers + * @obj: GEM object + * @vma: Virtual address range + * + * This function sets up a userspace mapping for PRIME exported buffers using + * the same codepath that is used for regular GEM buffer mapping on the DRM fd. + * The fake GEM offset is added to vma->vm_pgoff and &drm_driver->fops->mmap is + * called to set up the mapping. + * + * Drivers can use this as their &drm_driver.gem_prime_mmap callback. + */ +int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) +{ + /* Used by drm_gem_mmap() to lookup the GEM object */ + struct drm_file priv = { + .minor = obj->dev->primary, + }; + struct file fil = { + .private_data = &priv, + }; + int ret; + + ret = drm_vma_node_allow(&obj->vma_node, &priv); + if (ret) + return ret; + + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node); + + ret = obj->dev->driver->fops->mmap(&fil, vma); + + drm_vma_node_revoke(&obj->vma_node, &priv); + + return ret; +} +EXPORT_SYMBOL(drm_gem_prime_mmap); + /** * drm_gem_prime_import_dev - core implementation of the import callback * @dev: drm_device to import into diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h index e2032fbc0f08..b03731a3f079 100644 --- a/include/drm/drm_prime.h +++ b/include/drm/drm_prime.h @@ -70,6 +70,7 @@ struct dma_buf *drm_gem_prime_export(struct drm_device *dev, int drm_gem_prime_handle_to_fd(struct drm_device *dev, struct drm_file *file_priv, uint32_t handle, uint32_t flags, int *prime_fd); +int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma); struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf); -- cgit v1.2.3 From b39b5394fabc79acbaafb26b777fd348c868bf7e Mon Sep 17 00:00:00 2001 From: Noralf Trønnes Date: Sat, 10 Nov 2018 15:56:45 +0100 Subject: drm/gem: Add drm_gem_object_funcs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds an optional function table on GEM objects. The main benefit is for drivers that support more than one type of memory (shmem,vram,cma) for their buffers depending on the hardware it runs on. With the callbacks attached to the GEM object itself, it is easier to have core helpers for the the various buffer types. The driver only has to make the decision about buffer type on GEM object creation and all other callbacks can be handled by the chosen helper. drm_driver->gem_prime_res_obj has not been added since there's a todo to put a reservation_object into drm_gem_object. v3: Add todo entry v2: Drop drm_gem_object_funcs->prime_mmap in favour of drm_gem_prime_mmap() (Daniel Vetter) v1: - drm_gem_object_funcs.map -> .prime_map let it only do PRIME mmap like the function it superseeds (Daniel Vetter) - Flip around the if ladders and make obj->funcs the first choice highlighting the fact that this the new default way of doing it (Daniel Vetter) Signed-off-by: Noralf Trønnes Acked-by: Daniel Vetter Acked-by: Christian König Link: https://patchwork.freedesktop.org/patch/msgid/20181110145647.17580-4-noralf@tronnes.org --- Documentation/gpu/todo.rst | 6 ++ drivers/gpu/drm/drm_client.c | 12 ++-- drivers/gpu/drm/drm_gem.c | 109 ++++++++++++++++++++++++++++++++--- drivers/gpu/drm/drm_prime.c | 34 ++++++----- include/drm/drm_gem.h | 131 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 258 insertions(+), 34 deletions(-) (limited to 'include/drm') diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index abc31956ef15..31ef4adc91c9 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -241,6 +241,12 @@ Most drivers don't need to set drm_driver->gem_prime_import and ->gem_prime_export now that drm_gem_prime_import() and drm_gem_prime_export() are the default. +struct drm_gem_object_funcs +--------------------------- + +GEM objects can now have a function table instead of having the callbacks on the +DRM driver struct. This is now the preferred way and drivers can be moved over. + Core refactorings ================= diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index fc03d26fcacc..9b2bd28dde0a 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -81,8 +81,7 @@ int drm_client_init(struct drm_device *dev, struct drm_client_dev *client, { int ret; - if (!drm_core_check_feature(dev, DRIVER_MODESET) || - !dev->driver->dumb_create || !dev->driver->gem_prime_vmap) + if (!drm_core_check_feature(dev, DRIVER_MODESET) || !dev->driver->dumb_create) return -EOPNOTSUPP; if (funcs && !try_module_get(funcs->owner)) @@ -229,8 +228,7 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer) { struct drm_device *dev = buffer->client->dev; - if (buffer->vaddr && dev->driver->gem_prime_vunmap) - dev->driver->gem_prime_vunmap(buffer->gem, buffer->vaddr); + drm_gem_vunmap(buffer->gem, buffer->vaddr); if (buffer->gem) drm_gem_object_put_unlocked(buffer->gem); @@ -283,9 +281,9 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u * fd_install step out of the driver backend hooks, to make that * final step optional for internal users. */ - vaddr = dev->driver->gem_prime_vmap(obj); - if (!vaddr) { - ret = -ENOMEM; + vaddr = drm_gem_vmap(obj); + if (IS_ERR(vaddr)) { + ret = PTR_ERR(vaddr); goto err_delete; } diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 512078ebd97b..8b55ece97967 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -257,7 +257,9 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) struct drm_gem_object *obj = ptr; struct drm_device *dev = obj->dev; - if (dev->driver->gem_close_object) + if (obj->funcs && obj->funcs->close) + obj->funcs->close(obj, file_priv); + else if (dev->driver->gem_close_object) dev->driver->gem_close_object(obj, file_priv); if (drm_core_check_feature(dev, DRIVER_PRIME)) @@ -410,7 +412,11 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, if (ret) goto err_remove; - if (dev->driver->gem_open_object) { + if (obj->funcs && obj->funcs->open) { + ret = obj->funcs->open(obj, file_priv); + if (ret) + goto err_revoke; + } else if (dev->driver->gem_open_object) { ret = dev->driver->gem_open_object(obj, file_priv); if (ret) goto err_revoke; @@ -835,7 +841,9 @@ drm_gem_object_free(struct kref *kref) container_of(kref, struct drm_gem_object, refcount); struct drm_device *dev = obj->dev; - if (dev->driver->gem_free_object_unlocked) { + if (obj->funcs) { + obj->funcs->free(obj); + } else if (dev->driver->gem_free_object_unlocked) { dev->driver->gem_free_object_unlocked(obj); } else if (dev->driver->gem_free_object) { WARN_ON(!mutex_is_locked(&dev->struct_mutex)); @@ -864,13 +872,13 @@ drm_gem_object_put_unlocked(struct drm_gem_object *obj) dev = obj->dev; - if (dev->driver->gem_free_object_unlocked) { - kref_put(&obj->refcount, drm_gem_object_free); - } else { + if (dev->driver->gem_free_object) { might_lock(&dev->struct_mutex); if (kref_put_mutex(&obj->refcount, drm_gem_object_free, &dev->struct_mutex)) mutex_unlock(&dev->struct_mutex); + } else { + kref_put(&obj->refcount, drm_gem_object_free); } } EXPORT_SYMBOL(drm_gem_object_put_unlocked); @@ -960,11 +968,14 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL; - if (!dev->driver->gem_vm_ops) + if (obj->funcs && obj->funcs->vm_ops) + vma->vm_ops = obj->funcs->vm_ops; + else if (dev->driver->gem_vm_ops) + vma->vm_ops = dev->driver->gem_vm_ops; + else return -EINVAL; vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; - vma->vm_ops = dev->driver->gem_vm_ops; vma->vm_private_data = obj; vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); @@ -1066,6 +1077,86 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent, drm_printf_indent(p, indent, "imported=%s\n", obj->import_attach ? "yes" : "no"); - if (obj->dev->driver->gem_print_info) + if (obj->funcs && obj->funcs->print_info) + obj->funcs->print_info(p, indent, obj); + else if (obj->dev->driver->gem_print_info) obj->dev->driver->gem_print_info(p, indent, obj); } + +/** + * drm_gem_pin - Pin backing buffer in memory + * @obj: GEM object + * + * Make sure the backing buffer is pinned in memory. + * + * Returns: + * 0 on success or a negative error code on failure. + */ +int drm_gem_pin(struct drm_gem_object *obj) +{ + if (obj->funcs && obj->funcs->pin) + return obj->funcs->pin(obj); + else if (obj->dev->driver->gem_prime_pin) + return obj->dev->driver->gem_prime_pin(obj); + else + return 0; +} +EXPORT_SYMBOL(drm_gem_pin); + +/** + * drm_gem_unpin - Unpin backing buffer from memory + * @obj: GEM object + * + * Relax the requirement that the backing buffer is pinned in memory. + */ +void drm_gem_unpin(struct drm_gem_object *obj) +{ + if (obj->funcs && obj->funcs->unpin) + obj->funcs->unpin(obj); + else if (obj->dev->driver->gem_prime_unpin) + obj->dev->driver->gem_prime_unpin(obj); +} +EXPORT_SYMBOL(drm_gem_unpin); + +/** + * drm_gem_vmap - Map buffer into kernel virtual address space + * @obj: GEM object + * + * Returns: + * A virtual pointer to a newly created GEM object or an ERR_PTR-encoded negative + * error code on failure. + */ +void *drm_gem_vmap(struct drm_gem_object *obj) +{ + void *vaddr; + + if (obj->funcs && obj->funcs->vmap) + vaddr = obj->funcs->vmap(obj); + else if (obj->dev->driver->gem_prime_vmap) + vaddr = obj->dev->driver->gem_prime_vmap(obj); + else + vaddr = ERR_PTR(-EOPNOTSUPP); + + if (!vaddr) + vaddr = ERR_PTR(-ENOMEM); + + return vaddr; +} +EXPORT_SYMBOL(drm_gem_vmap); + +/** + * drm_gem_vunmap - Remove buffer mapping from kernel virtual address space + * @obj: GEM object + * @vaddr: Virtual address (can be NULL) + */ +void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr) +{ + if (!vaddr) + return; + + if (obj->funcs && obj->funcs->vunmap) + obj->funcs->vunmap(obj, vaddr); + else if (obj->dev->driver->gem_prime_vunmap) + obj->dev->driver->gem_prime_vunmap(obj, vaddr); +} +EXPORT_SYMBOL(drm_gem_vunmap); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 6a4fe0216e27..5737cb8c6f03 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -199,7 +199,6 @@ int drm_gem_map_attach(struct dma_buf *dma_buf, { struct drm_prime_attachment *prime_attach; struct drm_gem_object *obj = dma_buf->priv; - struct drm_device *dev = obj->dev; prime_attach = kzalloc(sizeof(*prime_attach), GFP_KERNEL); if (!prime_attach) @@ -208,10 +207,7 @@ int drm_gem_map_attach(struct dma_buf *dma_buf, prime_attach->dir = DMA_NONE; attach->priv = prime_attach; - if (!dev->driver->gem_prime_pin) - return 0; - - return dev->driver->gem_prime_pin(obj); + return drm_gem_pin(obj); } EXPORT_SYMBOL(drm_gem_map_attach); @@ -228,7 +224,6 @@ void drm_gem_map_detach(struct dma_buf *dma_buf, { struct drm_prime_attachment *prime_attach = attach->priv; struct drm_gem_object *obj = dma_buf->priv; - struct drm_device *dev = obj->dev; if (prime_attach) { struct sg_table *sgt = prime_attach->sgt; @@ -247,8 +242,7 @@ void drm_gem_map_detach(struct dma_buf *dma_buf, attach->priv = NULL; } - if (dev->driver->gem_prime_unpin) - dev->driver->gem_prime_unpin(obj); + drm_gem_unpin(obj); } EXPORT_SYMBOL(drm_gem_map_detach); @@ -310,7 +304,10 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, if (WARN_ON(prime_attach->dir != DMA_NONE)) return ERR_PTR(-EBUSY); - sgt = obj->dev->driver->gem_prime_get_sg_table(obj); + if (obj->funcs) + sgt = obj->funcs->get_sg_table(obj); + else + sgt = obj->dev->driver->gem_prime_get_sg_table(obj); if (!IS_ERR(sgt)) { if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, @@ -406,12 +403,13 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release); void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf) { struct drm_gem_object *obj = dma_buf->priv; - struct drm_device *dev = obj->dev; + void *vaddr; - if (dev->driver->gem_prime_vmap) - return dev->driver->gem_prime_vmap(obj); - else - return NULL; + vaddr = drm_gem_vmap(obj); + if (IS_ERR(vaddr)) + vaddr = NULL; + + return vaddr; } EXPORT_SYMBOL(drm_gem_dmabuf_vmap); @@ -426,10 +424,8 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap); void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) { struct drm_gem_object *obj = dma_buf->priv; - struct drm_device *dev = obj->dev; - if (dev->driver->gem_prime_vunmap) - dev->driver->gem_prime_vunmap(obj, vaddr); + drm_gem_vunmap(obj, vaddr); } EXPORT_SYMBOL(drm_gem_dmabuf_vunmap); @@ -529,7 +525,9 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, return dmabuf; } - if (dev->driver->gem_prime_export) + if (obj->funcs && obj->funcs->export) + dmabuf = obj->funcs->export(obj, flags); + else if (dev->driver->gem_prime_export) dmabuf = dev->driver->gem_prime_export(dev, obj, flags); else dmabuf = drm_gem_prime_export(dev, obj, flags); diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 3583b98a1718..f466ce5bde0e 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -38,6 +38,121 @@ #include +struct drm_gem_object; + +/** + * struct drm_gem_object_funcs - GEM object functions + */ +struct drm_gem_object_funcs { + /** + * @free: + * + * Deconstructor for drm_gem_objects. + * + * This callback is mandatory. + */ + void (*free)(struct drm_gem_object *obj); + + /** + * @open: + * + * Called upon GEM handle creation. + * + * This callback is optional. + */ + int (*open)(struct drm_gem_object *obj, struct drm_file *file); + + /** + * @close: + * + * Called upon GEM handle release. + * + * This callback is optional. + */ + void (*close)(struct drm_gem_object *obj, struct drm_file *file); + + /** + * @print_info: + * + * If driver subclasses struct &drm_gem_object, it can implement this + * optional hook for printing additional driver specific info. + * + * drm_printf_indent() should be used in the callback passing it the + * indent argument. + * + * This callback is called from drm_gem_print_info(). + * + * This callback is optional. + */ + void (*print_info)(struct drm_printer *p, unsigned int indent, + const struct drm_gem_object *obj); + + /** + * @export: + * + * Export backing buffer as a &dma_buf. + * If this is not set drm_gem_prime_export() is used. + * + * This callback is optional. + */ + struct dma_buf *(*export)(struct drm_gem_object *obj, int flags); + + /** + * @pin: + * + * Pin backing buffer in memory. + * + * This callback is optional. + */ + int (*pin)(struct drm_gem_object *obj); + + /** + * @unpin: + * + * Unpin backing buffer. + * + * This callback is optional. + */ + void (*unpin)(struct drm_gem_object *obj); + + /** + * @get_sg_table: + * + * Returns a Scatter-Gather table representation of the buffer. + * Used when exporting a buffer. + * + * This callback is mandatory if buffer export is supported. + */ + struct sg_table *(*get_sg_table)(struct drm_gem_object *obj); + + /** + * @vmap: + * + * Returns a virtual address for the buffer. + * + * This callback is optional. + */ + void *(*vmap)(struct drm_gem_object *obj); + + /** + * @vunmap: + * + * Releases the the address previously returned by @vmap. + * + * This callback is optional. + */ + void (*vunmap)(struct drm_gem_object *obj, void *vaddr); + + /** + * @vm_ops: + * + * Virtual memory operations used with mmap. + * + * This is optional but necessary for mmap support. + */ + const struct vm_operations_struct *vm_ops; +}; + /** * struct drm_gem_object - GEM buffer object * @@ -146,6 +261,17 @@ struct drm_gem_object { * simply leave it as NULL. */ struct dma_buf_attachment *import_attach; + + /** + * @funcs: + * + * Optional GEM object functions. If this is set, it will be used instead of the + * corresponding &drm_driver GEM callbacks. + * + * New drivers should use this. + * + */ + const struct drm_gem_object_funcs *funcs; }; /** @@ -293,4 +419,9 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, uint32_t handle); +int drm_gem_pin(struct drm_gem_object *obj); +void drm_gem_unpin(struct drm_gem_object *obj); +void *drm_gem_vmap(struct drm_gem_object *obj); +void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr); + #endif /* __DRM_GEM_H__ */ -- cgit v1.2.3 From b9068cde51eea189c2c3560a9a8ae83f7b213c9c Mon Sep 17 00:00:00 2001 From: Noralf Trønnes Date: Sat, 10 Nov 2018 15:56:46 +0100 Subject: drm/cma-helper: Add DRM_GEM_CMA_VMAP_DRIVER_OPS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds functionality to the CMA helper which ensures that the kernel virtual address is set on the CMA GEM object also for imported buffers. The drivers have been audited to ensure that none set ->vaddr on imported buffers, making the conditional dma_buf_vunmap() call in drm_gem_cma_free_object() safe. Signed-off-by: Noralf Trønnes Acked-by: Christian König Reviewed-by: Eric Anholt Link: https://patchwork.freedesktop.org/patch/msgid/20181110145647.17580-5-noralf@tronnes.org --- drivers/gpu/drm/drm_gem_cma_helper.c | 86 ++++++++++++++++++++++++++++++++++++ include/drm/drm_gem_cma_helper.h | 24 ++++++++++ 2 files changed, 110 insertions(+) (limited to 'include/drm') diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 1d2ced882b66..cc26625b4b33 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -176,6 +176,7 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv, * * This function frees the backing memory of the CMA GEM object, cleans up the * GEM object state and frees the memory used to store the object itself. + * If the buffer is imported and the virtual address is set, it is released. * Drivers using the CMA helpers should set this as their * &drm_driver.gem_free_object_unlocked callback. */ @@ -189,6 +190,8 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, cma_obj->vaddr, cma_obj->paddr); } else if (gem_obj->import_attach) { + if (cma_obj->vaddr) + dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr); drm_prime_gem_destroy(gem_obj, cma_obj->sgt); } @@ -575,3 +578,86 @@ void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr) /* Nothing to do */ } EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vunmap); + +static const struct drm_gem_object_funcs drm_cma_gem_default_funcs = { + .free = drm_gem_cma_free_object, + .print_info = drm_gem_cma_print_info, + .get_sg_table = drm_gem_cma_prime_get_sg_table, + .vmap = drm_gem_cma_prime_vmap, + .vm_ops = &drm_gem_cma_vm_ops, +}; + +/** + * drm_cma_gem_create_object_default_funcs - Create a CMA GEM object with a + * default function table + * @dev: DRM device + * @size: Size of the object to allocate + * + * This sets the GEM object functions to the default CMA helper functions. + * This function can be used as the &drm_driver.gem_create_object callback. + * + * Returns: + * A pointer to a allocated GEM object or an error pointer on failure. + */ +struct drm_gem_object * +drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size) +{ + struct drm_gem_cma_object *cma_obj; + + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL); + if (!cma_obj) + return NULL; + + cma_obj->base.funcs = &drm_cma_gem_default_funcs; + + return &cma_obj->base; +} +EXPORT_SYMBOL(drm_cma_gem_create_object_default_funcs); + +/** + * drm_gem_cma_prime_import_sg_table_vmap - PRIME import another driver's + * scatter/gather table and get the virtual address of the buffer + * @dev: DRM device + * @attach: DMA-BUF attachment + * @sgt: Scatter/gather table of pinned pages + * + * This function imports a scatter/gather table using + * drm_gem_cma_prime_import_sg_table() and uses dma_buf_vmap() to get the kernel + * virtual address. This ensures that a CMA GEM object always has its virtual + * address set. This address is released when the object is freed. + * + * This function can be used as the &drm_driver.gem_prime_import_sg_table + * callback. The DRM_GEM_CMA_VMAP_DRIVER_OPS() macro provides a shortcut to set + * the necessary DRM driver operations. + * + * Returns: + * A pointer to a newly created GEM object or an ERR_PTR-encoded negative + * error code on failure. + */ +struct drm_gem_object * +drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *dev, + struct dma_buf_attachment *attach, + struct sg_table *sgt) +{ + struct drm_gem_cma_object *cma_obj; + struct drm_gem_object *obj; + void *vaddr; + + vaddr = dma_buf_vmap(attach->dmabuf); + if (!vaddr) { + DRM_ERROR("Failed to vmap PRIME buffer\n"); + return ERR_PTR(-ENOMEM); + } + + obj = drm_gem_cma_prime_import_sg_table(dev, attach, sgt); + if (IS_ERR(obj)) { + dma_buf_vunmap(attach->dmabuf, vaddr); + return obj; + } + + cma_obj = to_drm_gem_cma_obj(obj); + cma_obj->vaddr = vaddr; + + return obj; +} +EXPORT_SYMBOL(drm_gem_cma_prime_import_sg_table_vmap); diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h index 19777145cf8e..07c504940ba1 100644 --- a/include/drm/drm_gem_cma_helper.h +++ b/include/drm/drm_gem_cma_helper.h @@ -103,4 +103,28 @@ int drm_gem_cma_prime_mmap(struct drm_gem_object *obj, void *drm_gem_cma_prime_vmap(struct drm_gem_object *obj); void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr); +struct drm_gem_object * +drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size); + +/** + * DRM_GEM_CMA_VMAP_DRIVER_OPS - CMA GEM driver operations ensuring a virtual + * address on the buffer + * + * This macro provides a shortcut for setting the default GEM operations in the + * &drm_driver structure for drivers that need the virtual address also on + * imported buffers. + */ +#define DRM_GEM_CMA_VMAP_DRIVER_OPS \ + .gem_create_object = drm_cma_gem_create_object_default_funcs, \ + .dumb_create = drm_gem_cma_dumb_create, \ + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, \ + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, \ + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table_vmap, \ + .gem_prime_mmap = drm_gem_prime_mmap + +struct drm_gem_object * +drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *drm, + struct dma_buf_attachment *attach, + struct sg_table *sgt); + #endif /* __DRM_GEM_CMA_HELPER_H__ */ -- cgit v1.2.3 From 3db8d37dd84e93f352b8a8d34a713415cd1a6d0f Mon Sep 17 00:00:00 2001 From: Noralf Trønnes Date: Sat, 10 Nov 2018 15:56:47 +0100 Subject: drm/tinydrm: Use DRM_GEM_CMA_VMAP_DRIVER_OPS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CMA helper now has the functionality to ensure a virtual address on imported buffer so use that. While touching all tinydrm drivers, remove the unnecessary inclusion of drm_fb_helper.h in some drivers. Cc: David Lechner Cc: Eric Anholt Signed-off-by: Noralf Trønnes Acked-by: Christian König Reviewed-by: Eric Anholt Link: https://patchwork.freedesktop.org/patch/msgid/20181110145647.17580-6-noralf@tronnes.org --- drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 71 -------------------------- drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 6 +++ drivers/gpu/drm/tinydrm/hx8357d.c | 4 +- drivers/gpu/drm/tinydrm/ili9225.c | 5 +- drivers/gpu/drm/tinydrm/ili9341.c | 4 +- drivers/gpu/drm/tinydrm/mi0283qt.c | 6 +-- drivers/gpu/drm/tinydrm/mipi-dbi.c | 10 ++-- drivers/gpu/drm/tinydrm/repaper.c | 4 +- drivers/gpu/drm/tinydrm/st7586.c | 5 +- drivers/gpu/drm/tinydrm/st7735r.c | 4 +- include/drm/tinydrm/tinydrm.h | 35 +++---------- 11 files changed, 38 insertions(+), 116 deletions(-) (limited to 'include/drm') diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c index 9af51d982a33..01a6f2d42440 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c @@ -36,77 +36,6 @@ * and registers the DRM device using devm_tinydrm_register(). */ -/** - * tinydrm_gem_cma_prime_import_sg_table - Produce a CMA GEM object from - * another driver's scatter/gather table of pinned pages - * @drm: DRM device to import into - * @attach: DMA-BUF attachment - * @sgt: Scatter/gather table of pinned pages - * - * This function imports a scatter/gather table exported via DMA-BUF by - * another driver using drm_gem_cma_prime_import_sg_table(). It sets the - * kernel virtual address on the CMA object. Drivers should use this as their - * &drm_driver->gem_prime_import_sg_table callback if they need the virtual - * address. tinydrm_gem_cma_free_object() should be used in combination with - * this function. - * - * Returns: - * A pointer to a newly created GEM object or an ERR_PTR-encoded negative - * error code on failure. - */ -struct drm_gem_object * -tinydrm_gem_cma_prime_import_sg_table(struct drm_device *drm, - struct dma_buf_attachment *attach, - struct sg_table *sgt) -{ - struct drm_gem_cma_object *cma_obj; - struct drm_gem_object *obj; - void *vaddr; - - vaddr = dma_buf_vmap(attach->dmabuf); - if (!vaddr) { - DRM_ERROR("Failed to vmap PRIME buffer\n"); - return ERR_PTR(-ENOMEM); - } - - obj = drm_gem_cma_prime_import_sg_table(drm, attach, sgt); - if (IS_ERR(obj)) { - dma_buf_vunmap(attach->dmabuf, vaddr); - return obj; - } - - cma_obj = to_drm_gem_cma_obj(obj); - cma_obj->vaddr = vaddr; - - return obj; -} -EXPORT_SYMBOL(tinydrm_gem_cma_prime_import_sg_table); - -/** - * tinydrm_gem_cma_free_object - Free resources associated with a CMA GEM - * object - * @gem_obj: GEM object to free - * - * This function frees the backing memory of the CMA GEM object, cleans up the - * GEM object state and frees the memory used to store the object itself using - * drm_gem_cma_free_object(). It also handles PRIME buffers which has the kernel - * virtual address set by tinydrm_gem_cma_prime_import_sg_table(). Drivers - * can use this as their &drm_driver->gem_free_object_unlocked callback. - */ -void tinydrm_gem_cma_free_object(struct drm_gem_object *gem_obj) -{ - if (gem_obj->import_attach) { - struct drm_gem_cma_object *cma_obj; - - cma_obj = to_drm_gem_cma_obj(gem_obj); - dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr); - cma_obj->vaddr = NULL; - } - - drm_gem_cma_free_object(gem_obj); -} -EXPORT_SYMBOL_GPL(tinydrm_gem_cma_free_object); - static struct drm_framebuffer * tinydrm_fb_create(struct drm_device *drm, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd) diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index dcd390163a4a..bf6bfbc5d412 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -9,12 +9,18 @@ #include #include +#include #include #include #include +#include +#include +#include +#include #include #include +#include static unsigned int spi_max; module_param(spi_max, uint, 0400); diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c index c3e51c2baebc..81a2bbeb25d4 100644 --- a/drivers/gpu/drm/tinydrm/hx8357d.c +++ b/drivers/gpu/drm/tinydrm/hx8357d.c @@ -16,7 +16,7 @@ #include #include -#include +#include #include #include #include @@ -188,7 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops); static struct drm_driver hx8357d_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, .fops = &hx8357d_fops, - TINYDRM_GEM_DRIVER_OPS, + DRM_GEM_CMA_VMAP_DRIVER_OPS, .debugfs_init = mipi_dbi_debugfs_init, .name = "hx8357d", .desc = "HX8357D", diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c index 455fefe012f5..78f7c2d1b449 100644 --- a/drivers/gpu/drm/tinydrm/ili9225.c +++ b/drivers/gpu/drm/tinydrm/ili9225.c @@ -20,7 +20,8 @@ #include #include