Re: [PATCH] android: fix warning when releasing active sync point
2015-12-14 Dmitry Torokhov : > Userspace can close the sync device while there are still active fence > points, in which case kernel produces the following warning: > > [ 43.853176] [ cut here ] > [ 43.857834] WARNING: CPU: 0 PID: 892 at > /mnt/host/source/src/third_party/kernel/v3.18/drivers/staging/android/sync.c:439 > android_fence_release+0x88/0x104() > [ 43.871741] CPU: 0 PID: 892 Comm: Binder_5 Tainted: G U > 3.18.0-07661-g0550ce9 #1 > [ 43.880176] Hardware name: Google Tegra210 Smaug Rev 1+ (DT) > [ 43.885834] Call trace: > [ 43.888294] [] dump_backtrace+0x0/0x10c > [ 43.893697] [] show_stack+0x10/0x1c > [ 43.898756] [] dump_stack+0x74/0xb8 > [ 43.903814] [] warn_slowpath_common+0x84/0xb0 > [ 43.909736] [] warn_slowpath_null+0x14/0x20 > [ 43.915482] [] android_fence_release+0x84/0x104 > [ 43.921582] [] fence_release+0x104/0x134 > [ 43.927066] [] sync_fence_free+0x74/0x9c > [ 43.932552] [] sync_fence_release+0x34/0x48 > [ 43.938304] [] __fput+0x100/0x1b8 > [ 43.943185] [] fput+0x8/0x14 > [ 43.947982] [] task_work_run+0xb0/0xe4 > [ 43.953297] [] do_notify_resume+0x44/0x5c > [ 43.958867] ---[ end trace 5a2aa4027cc5d171 ]--- This crash report seems to be for a 3.18 kernel. Can you reproduce it on upstream kernel as well? Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: fix warning when releasing active sync point
2015-12-15 Daniel Vetter : > On Mon, Dec 14, 2015 at 05:29:55PM -0800, Dmitry Torokhov wrote: > > Userspace can close the sync device while there are still active fence > > points, in which case kernel produces the following warning: > > > > [ 43.853176] [ cut here ] > > [ 43.857834] WARNING: CPU: 0 PID: 892 at > > /mnt/host/source/src/third_party/kernel/v3.18/drivers/staging/android/sync.c:439 > > android_fence_release+0x88/0x104() > > [ 43.871741] CPU: 0 PID: 892 Comm: Binder_5 Tainted: G U > > 3.18.0-07661-g0550ce9 #1 > > [ 43.880176] Hardware name: Google Tegra210 Smaug Rev 1+ (DT) > > [ 43.885834] Call trace: > > [ 43.888294] [] dump_backtrace+0x0/0x10c > > [ 43.893697] [] show_stack+0x10/0x1c > > [ 43.898756] [] dump_stack+0x74/0xb8 > > [ 43.903814] [] warn_slowpath_common+0x84/0xb0 > > [ 43.909736] [] warn_slowpath_null+0x14/0x20 > > [ 43.915482] [] android_fence_release+0x84/0x104 > > [ 43.921582] [] fence_release+0x104/0x134 > > [ 43.927066] [] sync_fence_free+0x74/0x9c > > [ 43.932552] [] sync_fence_release+0x34/0x48 > > [ 43.938304] [] __fput+0x100/0x1b8 > > [ 43.943185] [] fput+0x8/0x14 > > [ 43.947982] [] task_work_run+0xb0/0xe4 > > [ 43.953297] [] do_notify_resume+0x44/0x5c > > [ 43.958867] ---[ end trace 5a2aa4027cc5d171 ]--- > > > > Let's fix it by introducing a new optional callback (disable_signaling) > > to fence operations so that drivers can do proper clean ups when we > > remove last callback for given fence. > > > > Reviewed-by: Andrew Bresticker > > Signed-off-by: Dmitry Torokhov > > --- > > drivers/dma-buf/fence.c| 6 +- > > drivers/staging/android/sync.c | 8 > > include/linux/fence.h | 2 ++ > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c > > index 7b05dbe..0ed73ad 100644 > > --- a/drivers/dma-buf/fence.c > > +++ b/drivers/dma-buf/fence.c > > @@ -304,8 +304,12 @@ fence_remove_callback(struct fence *fence, struct > > fence_cb *cb) > > spin_lock_irqsave(fence->lock, flags); > > > > ret = !list_empty(&cb->node); > > - if (ret) > > + if (ret) { > > list_del_init(&cb->node); > > + if (list_empty(&fence->cb_list)) > > + if (fence->ops->disable_signaling) > > + fence->ops->disable_signaling(fence); > > What exactly is the bug here? A fence with no callbacks registered any > more shouldn't have any problem. Why exactly does this blow up? The WARN_ON is probably this one: https://android.googlesource.com/kernel/common/+/android-3.18/drivers/staging/android/sync.c#433 I've been wondering in the last few days if this warning is really necessary. If the user is closing a sync_timeline that has unsignalled fences it should probably be aware of that already. Then I think it is okay to remove the the sync_pt from the active_list at the release-time. In fact I've already prepared a patch doing that. Thoughts? Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v8 2/2] staging/android: refactor SYNC IOCTLs
From: Gustavo Padovan Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid future API breaks and optimize buffer allocation. Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences. However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file. Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed. Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file. info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues. And a flags member was added. An example userspace code for the later would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } Finally the IOCTLs numbers were changed to avoid any potential old userspace running the old API to get weird errors. Changing the opcodes will make them fail right away. This is just a precaution, there no upstream users of these interfaces yet and the only user is Android, but we don't expect anyone trying to run android userspace and all it dependencies on top of upstream kernels. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Acked-by: Greg Hackmann Acked-by: Rob Clark Acked-by: Daniel Vetter --- v2: fix fence_info memory leak v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info() Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size Comments from Dan Carpenter - fix info->sync_fence_info documentation v4: remove allocated struct sync_file_info (comment from Maarten) v5: merge all commits that were changing the ABI --- drivers/staging/android/sync.c | 76 - drivers/staging/android/uapi/sync.h | 36 +- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..ae81c95 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,9 @@ err_put_fd: return err; } -static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) + if (info.flags || info.pad) return -EINVAL; - if (size
[PATCH v8 1/2] staging/android: remove redundant comments on sync_merge_data
From: Gustavo Padovan struct sync_merge_data already have documentation on top of the struct definition. No need to duplicate it. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst --- drivers/staging/android/uapi/sync.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index a0cf357..4467c76 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -21,9 +21,9 @@ * @fence: returns the fd of the new fence to userspace */ struct sync_merge_data { - __s32 fd2; /* fd of second fence */ - charname[32]; /* name of new fence */ - __s32 fence; /* fd on newly created fence */ + __s32 fd2; + charname[32]; + __s32 fence; }; /** -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] de-stage android sync framework
From: Gustavo Padovan Hi, This is the first step and the most important one of the de-stage of the the sync framework, it de-stage the sync_file part which is used to send/receive fence file descriptors with the userspace. These patches sits on top of the sync ABI changes that I sent earlier today: https://www.spinics.net/lists/dri-devel/msg102795.html Comments are welcome! Gustavo Gustavo Padovan (1): dma-buf/sync_file: de-stage sync_file drivers/Kconfig| 2 + drivers/dma-buf/Kconfig| 11 + drivers/dma-buf/Makefile | 1 + drivers/dma-buf/sync_file.c| 382 + drivers/staging/android/Kconfig| 1 + drivers/staging/android/sync.c | 362 --- drivers/staging/android/sync.h | 92 + drivers/staging/android/sync_debug.c | 1 + include/linux/sync_file.h | 106 ++ .../uapi/sync.h => include/uapi/linux/sync_file.h | 0 10 files changed, 506 insertions(+), 452 deletions(-) create mode 100644 drivers/dma-buf/Kconfig create mode 100644 drivers/dma-buf/sync_file.c create mode 100644 include/linux/sync_file.h rename drivers/staging/android/uapi/sync.h => include/uapi/linux/sync_file.h (100%) -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v9 2/3] kernel.h: add to_user_ptr()
2016-03-17 Joe Perches : > On Thu, 2016-03-17 at 18:19 -0300, Gustavo Padovan wrote: > > 2016-03-17 Joe Perches : > > > On Thu, 2016-03-17 at 16:50 -0400, Rob Clark wrote: > > > > On Thu, Mar 17, 2016 at 4:40 PM, Joe Perches wrote: > > > [] > > > > > It's a name that seems like it should be a straightforward > > > > > cast of a kernel pointer to a __user pointer like: > > > > > > > > > > static inline void __user *to_user_ptr(void *p) > > > > > { > > > > > return (void __user *)p; > > > > > } > > > > ahh, ok. I guess I was used to using it in the context of ioctl > > > > structs.. in that context u64 -> (void __user *) made more sense. > > > > > > > > Maybe uapi_to_ptr()? (ok, not super-creative.. maybe someone has a > > > > better idea) > > > Maybe u64_to_user_ptr? > > That is a good name. If everyone agrees I can resend this patch > > changing it to u64_to_user_ptr. Then should we still keep it on > > kernel.h? > > I've no particular opinion about location, > but maybe compat.h might be appropriate. I don't think this is really related to compat. I'd keep kernel.h. The problem I'm trying to solve here is: CC drivers/dma-buf/sync_file.o drivers/dma-buf/sync_file.c: In function ‘sync_file_ioctl_fence_info’: drivers/dma-buf/sync_file.c:341:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] if (copy_to_user((void __user *)info.sync_fence_info, fence_info, where info.sync_fence_info is __u64. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v10 3/3] staging/android: refactor SYNC IOCTLs
From: Gustavo Padovan Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid future API breaks and optimize buffer allocation. Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences. However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file. Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed. Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file. info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues. And a flags member was added. An example userspace code for the later would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } Finally the IOCTLs numbers were changed to avoid any potential old userspace running the old API to get weird errors. Changing the opcodes will make them fail right away. This is just a precaution, there no upstream users of these interfaces yet and the only user is Android, but we don't expect anyone trying to run android userspace and all it dependencies on top of upstream kernels. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Acked-by: Greg Hackmann Acked-by: Rob Clark Acked-by: Daniel Vetter --- v2: fix fence_info memory leak v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info() Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size Comments from Dan Carpenter - fix info->sync_fence_info documentation v4: remove allocated struct sync_file_info (comment from Maarten) v5: merge all commits that were changing the ABI v6: fix -Wint-to-pointer-cast error on info.sync_fence_info --- drivers/staging/android/sync.c | 76 - drivers/staging/android/uapi/sync.h | 36 +- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..f9c6094 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,9 @@ err_put_fd: return err; } -static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) + if (inf
Re: [PATCH v9 2/3] kernel.h: add to_user_ptr()
2016-03-17 Joe Perches : > On Thu, 2016-03-17 at 16:50 -0400, Rob Clark wrote: > > On Thu, Mar 17, 2016 at 4:40 PM, Joe Perches wrote: > [] > > > It's a name that seems like it should be a straightforward > > > cast of a kernel pointer to a __user pointer like: > > > > > > static inline void __user *to_user_ptr(void *p) > > > { > > > return (void __user *)p; > > > } > > ahh, ok. I guess I was used to using it in the context of ioctl > > structs.. in that context u64 -> (void __user *) made more sense. > > > > Maybe uapi_to_ptr()? (ok, not super-creative.. maybe someone has a > > better idea) > > Maybe u64_to_user_ptr? That is a good name. If everyone agrees I can resend this patch changing it to u64_to_user_ptr. Then should we still keep it on kernel.h? Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 1/3] staging/android: remove redundant comments on sync_merge_data
From: Gustavo Padovan struct sync_merge_data already have documentation on top of the struct definition. No need to duplicate it. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst --- drivers/staging/android/uapi/sync.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index a0cf357..4467c76 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -21,9 +21,9 @@ * @fence: returns the fd of the new fence to userspace */ struct sync_merge_data { - __s32 fd2; /* fd of second fence */ - charname[32]; /* name of new fence */ - __s32 fence; /* fd on newly created fence */ + __s32 fd2; + charname[32]; + __s32 fence; }; /** -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v9 2/3] kernel.h: add to_user_ptr()
2016-03-17 Gustavo Padovan : > 2016-03-17 Joe Perches : > > > On Thu, 2016-03-17 at 14:30 -0300, Gustavo Padovan wrote: > > > This function had copies in 3 different files. Unify them in > > > kernel.h. > > > > This is only used by gpu/drm. > > > > I think this is a poor name for a generic function > > that would be in kernel.h. > > > > Isn't there an include file in linux/drm that's > > appropriate for this. Maybe drmP.h > > > > Maybe prefix this function name with drm_ too. > > No, the next patch adds a user to drivers/staging (which will be moved > to drivers/dma-buf) soon. Maybe move to a different header in > include/linux/? not sure which one. > > > Also, there's this that might conflict: > > > > arch/powerpc/kernel/signal_32.c:#define to_user_ptr(p) > > ptr_to_compat(p) > > arch/powerpc/kernel/signal_32.c:#define to_user_ptr(p) ((unsigned > > long)(p)) > > Right, I'll figure out how to replace these two too. The powerpc to_user_ptr has a different meaning from the one I'm adding in this patch. I propose we just rename powerpc's to_user_ptr to __to_user_ptr and leave the rest as is. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v9 2/3] kernel.h: add to_user_ptr()
2016-03-17 Joe Perches : > On Thu, 2016-03-17 at 14:30 -0300, Gustavo Padovan wrote: > > This function had copies in 3 different files. Unify them in > > kernel.h. > > This is only used by gpu/drm. > > I think this is a poor name for a generic function > that would be in kernel.h. > > Isn't there an include file in linux/drm that's > appropriate for this. Maybe drmP.h > > Maybe prefix this function name with drm_ too. No, the next patch adds a user to drivers/staging (which will be moved to drivers/dma-buf) soon. Maybe move to a different header in include/linux/? not sure which one. > Also, there's this that might conflict: > > arch/powerpc/kernel/signal_32.c:#define to_user_ptr(p) > ptr_to_compat(p) > arch/powerpc/kernel/signal_32.c:#define to_user_ptr(p) ((unsigned > long)(p)) Right, I'll figure out how to replace these two too. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v10 1/3] staging/android: remove redundant comments on sync_merge_data
From: Gustavo Padovan struct sync_merge_data already have documentation on top of the struct definition. No need to duplicate it. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst --- drivers/staging/android/uapi/sync.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index a0cf357..4467c76 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -21,9 +21,9 @@ * @fence: returns the fd of the new fence to userspace */ struct sync_merge_data { - __s32 fd2; /* fd of second fence */ - charname[32]; /* name of new fence */ - __s32 fence; /* fd on newly created fence */ + __s32 fd2; + charname[32]; + __s32 fence; }; /** -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 3/3] staging/android: refactor SYNC IOCTLs
From: Gustavo Padovan Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid future API breaks and optimize buffer allocation. Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences. However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file. Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed. Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file. info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues. And a flags member was added. An example userspace code for the later would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } Finally the IOCTLs numbers were changed to avoid any potential old userspace running the old API to get weird errors. Changing the opcodes will make them fail right away. This is just a precaution, there no upstream users of these interfaces yet and the only user is Android, but we don't expect anyone trying to run android userspace and all it dependencies on top of upstream kernels. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Acked-by: Greg Hackmann Acked-by: Rob Clark Acked-by: Daniel Vetter --- v2: fix fence_info memory leak v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info() Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size Comments from Dan Carpenter - fix info->sync_fence_info documentation v4: remove allocated struct sync_file_info (comment from Maarten) v5: merge all commits that were changing the ABI v6: fix -Wint-to-pointer-cast error on info.sync_fence_info --- drivers/staging/android/sync.c | 75 - drivers/staging/android/uapi/sync.h | 36 +- 2 files changed, 66 insertions(+), 45 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..1d9c530 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,9 @@ err_put_fd: return err; } -static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,58 +496,62 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) + if (inf
[PATCH v10 2/3] kernel.h: add u64_to_user_ptr()
From: Gustavo Padovan This function had copies in 3 different files. Unify them in kernel.h. Cc: Joe Perches Cc: Andrew Morton Cc: David Airlie Cc: Daniel Vetter Cc: Rob Clark Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 5 - drivers/gpu/drm/i915/i915_gem.c | 14 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++ include/linux/kernel.h | 5 + 6 files changed, 25 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 1aba01a..5420bc5 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -28,11 +28,6 @@ #define BO_LOCKED 0x4000 #define BO_PINNED 0x2000 -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, struct etnaviv_gpu *gpu, size_t nr) { @@ -351,21 +346,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, cmdbuf->exec_state = args->exec_state; cmdbuf->ctx = file->driver_priv; - ret = copy_from_user(bos, to_user_ptr(args->bos), + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), args->nr_bos * sizeof(*bos)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(relocs, to_user_ptr(args->relocs), + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), args->nr_relocs * sizeof(*relocs)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(stream, to_user_ptr(args->stream), + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), args->stream_size); if (ret) { ret = -EFAULT; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b0847b9..c446895 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3564,11 +3564,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev) return VGACNTRL; } -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) { unsigned long j = msecs_to_jiffies(m); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bb44bad..cd11790 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; void *vaddr = obj->phys_handle->vaddr + args->offset; - char __user *user_data = to_user_ptr(args->data_ptr); + char __user *user_data = u64_to_user_ptr(args->data_ptr); int ret = 0; /* We manually control the domain here and pretend that it @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, int needs_clflush = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_WRITE, - to_user_ptr(args->data_ptr), + u64_to_user_ptr(args->data_ptr), args->size)) return -EFAULT; @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, if (ret) goto out_unpin; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; offset = i915_gem_obj_ggtt_offset(obj) + args->offset; @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev, int needs_clflush_before = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -1036,12 +1036,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_READ, - to_user_ptr(args->data_ptr), + u64_to_user_ptr(args-&
[PATCH v9 2/3] kernel.h: add to_user_ptr()
From: Gustavo Padovan This function had copies in 3 different files. Unify them in kernel.h. Cc: Andrew Morton Cc: David Airlie Cc: Daniel Vetter Cc: Rob Clark Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 5 - drivers/gpu/drm/i915/i915_drv.h | 5 - drivers/gpu/drm/msm/msm_gem_submit.c | 5 - include/linux/kernel.h | 5 + 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 1aba01a..b1fafb6 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -28,11 +28,6 @@ #define BO_LOCKED 0x4000 #define BO_PINNED 0x2000 -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, struct etnaviv_gpu *gpu, size_t nr) { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b0847b9..c446895 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3564,11 +3564,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev) return VGACNTRL; } -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) { unsigned long j = msecs_to_jiffies(m); diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 6d7cd3f..e9c8b96 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -28,11 +28,6 @@ #define BO_LOCKED 0x4000 #define BO_PINNED 0x2000 -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static struct msm_gem_submit *submit_create(struct drm_device *dev, struct msm_gpu *gpu, int nr) { diff --git a/include/linux/kernel.h b/include/linux/kernel.h index f31638c..c0a6001 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -53,6 +53,11 @@ #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) +static inline void __user *to_user_ptr(u64 address) +{ + return (void __user *)(uintptr_t)address; +} + /* * This looks more complex than it should be. But we need to * get the type for the ~ right in round_down (it needs to be -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] dma-buf/sync_file: de-stage sync_file
From: Gustavo Padovan sync_file is useful to connect one or more fences to the file. The file is used by userspace to track fences. Signed-off-by: Gustavo Padovan --- drivers/Kconfig| 2 + drivers/dma-buf/Kconfig| 11 + drivers/dma-buf/Makefile | 1 + drivers/dma-buf/sync_file.c| 382 + drivers/staging/android/Kconfig| 1 + drivers/staging/android/sync.c | 362 --- drivers/staging/android/sync.h | 92 + drivers/staging/android/sync_debug.c | 1 + include/linux/sync_file.h | 106 ++ .../uapi/sync.h => include/uapi/linux/sync_file.h | 0 10 files changed, 506 insertions(+), 452 deletions(-) create mode 100644 drivers/dma-buf/Kconfig create mode 100644 drivers/dma-buf/sync_file.c create mode 100644 include/linux/sync_file.h rename drivers/staging/android/uapi/sync.h => include/uapi/linux/sync_file.h (100%) diff --git a/drivers/Kconfig b/drivers/Kconfig index d2ac339..430f761 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -114,6 +114,8 @@ source "drivers/rtc/Kconfig" source "drivers/dma/Kconfig" +source "drivers/dma-buf/Kconfig" + source "drivers/dca/Kconfig" source "drivers/auxdisplay/Kconfig" diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig new file mode 100644 index 000..9824bc4 --- /dev/null +++ b/drivers/dma-buf/Kconfig @@ -0,0 +1,11 @@ +menu "DMABUF options" + +config SYNC_FILE + bool "sync_file support for fences" + default n + select ANON_INODES + select DMA_SHARED_BUFFER + ---help--- + This option enables the fence framework synchronization to export + sync_files to userspace that can represent one or more fences. +endmenu diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 57a675f..4a424ec 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1 +1,2 @@ obj-y := dma-buf.o fence.o reservation.o seqno-fence.o +obj-$(CONFIG_SYNC_FILE)+= sync_file.o diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c new file mode 100644 index 000..df786a2 --- /dev/null +++ b/drivers/dma-buf/sync_file.c @@ -0,0 +1,382 @@ +/* + * drivers/dma-buf/sync_file.c + * + * Copyright (C) 2012 Google, Inc. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static const struct file_operations sync_file_fops; + +static struct sync_file *sync_file_alloc(int size, const char *name) +{ + struct sync_file *sync_file; + + sync_file = kzalloc(size, GFP_KERNEL); + if (!sync_file) + return NULL; + + sync_file->file = anon_inode_getfile("sync_file", &sync_file_fops, +sync_file, 0); + if (IS_ERR(sync_file->file)) + goto err; + + kref_init(&sync_file->kref); + strlcpy(sync_file->name, name, sizeof(sync_file->name)); + + init_waitqueue_head(&sync_file->wq); + + return sync_file; + +err: + kfree(sync_file); + return NULL; +} + +static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) +{ + struct sync_file_cb *check; + struct sync_file *sync_file; + + check = container_of(cb, struct sync_file_cb, cb); + sync_file = check->sync_file; + + if (atomic_dec_and_test(&sync_file->status)) + wake_up_all(&sync_file->wq); +} + +/* TODO: implement a create which takes more that one fence */ +struct sync_file *sync_file_create(const char *name, struct fence *fence) +{ + struct sync_file *sync_file; + + sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1]), + name); + if (!sync_file) + return NULL; + + sync_file->num_fences = 1; + atomic_set(&sync_file->status, 1); + + sync_file->cbs[0].fence = fence; + sync_file->cbs[0].sync_file = sync_file; + if (fence_add_callback(fence, &sync_file->cbs[0].cb, + fence_check_cb_func)) + atomic_dec(&sync_file->status); + + return sync_file; +} +EXPO
Re: [PATCH] dma-buf/sync_file: de-stage sync_file
Hi Sumit, 2016-03-21 Sumit Semwal : > Thanks for the patch, Gustavo! > > On 18 March 2016 at 19:49, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > sync_file is useful to connect one or more fences to the file. The file is > > used by userspace to track fences. > > > I think it is worthwhile to add relevant bits to the Documentation as > well - care to add relevant stuff to either Documentation/dma_buf.txt, > or to a file of its own? (I'd prefer the former, but would leave it > upto you.) Sure, that is part of my plan. I'm currently trying to have a working version of fences on DRM then I think I'll be able to add Documentation for everything I have done. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 1/5] staging/android: add num_fences field to struct sync_file_info
Hi Greg, 2016-03-30 Greg Kroah-Hartman : > On Thu, Mar 03, 2016 at 04:40:42PM -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Gustavo, can you resend both series of your android patches so I know I > have the latest ones to work with? Please also collect the acks that > people have provided so far. I have resent it already. The lastest patches on this series is v10, it contain the acks https://lkml.org/lkml/2016/3/18/298 Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10 3/3] staging/android: refactor SYNC IOCTLs
Hi Greg, Any comments on this? Thanks, Gustavo 2016-03-18 Gustavo Padovan : > From: Gustavo Padovan > > Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid > future API breaks and optimize buffer allocation. > > Now num_fences can be filled by the caller to inform how many fences it > wants to retrieve from the kernel. If the num_fences passed is greater > than zero info->sync_fence_info should point to a buffer with enough space > to fit all fences. > > However if num_fences passed to the kernel is 0, the kernel will reply > with number of fences of the sync_file. > > Sending first an ioctl with num_fences = 0 can optimize buffer allocation, > in a first call with num_fences = 0 userspace will receive the actual > number of fences in the num_fences filed. > > Then it can allocate a buffer with the correct size on sync_fence_info and > call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences > in the sync_file. > > info->sync_fence_info was converted to __u64 pointer to prevent 32bit > compatibility issues. And a flags member was added. > > An example userspace code for the later would be: > > struct sync_file_info *info; > int err, size, num_fences; > > info = malloc(sizeof(*info)); > > info.flags = 0; > err = ioctl(fd, SYNC_IOC_FILE_INFO, info); > num_fences = info->num_fences; > > if (num_fences) { > info.flags = 0; > size = sizeof(struct sync_fence_info) * num_fences; > info->num_fences = num_fences; > info->sync_fence_info = (uint64_t) calloc(num_fences, > sizeof(struct > sync_fence_info)); > > err = ioctl(fd, SYNC_IOC_FILE_INFO, info); > } > > Finally the IOCTLs numbers were changed to avoid any potential old > userspace running the old API to get weird errors. Changing the opcodes > will make them fail right away. This is just a precaution, there no > upstream users of these interfaces yet and the only user is Android, but > we don't expect anyone trying to run android userspace and all it > dependencies on top of upstream kernels. > > Signed-off-by: Gustavo Padovan > Reviewed-by: Maarten Lankhorst > Acked-by: Greg Hackmann > Acked-by: Rob Clark > Acked-by: Daniel Vetter > > --- > v2: fix fence_info memory leak > > v3: Comments from Emil Velikov > - improve commit message > - remove __u64 cast > - remove check for output fields in file_info > - clean up sync_fill_fence_info() > > Comments from Maarten Lankhorst > - remove in.num_fences && !in.sync_fence_info check > - remove info->len and use only num_fences to calculate size > > Comments from Dan Carpenter > - fix info->sync_fence_info documentation > > v4: remove allocated struct sync_file_info (comment from Maarten) > > v5: merge all commits that were changing the ABI > > v6: fix -Wint-to-pointer-cast error on info.sync_fence_info > --- > drivers/staging/android/sync.c | 76 > - > drivers/staging/android/uapi/sync.h | 36 +- > 2 files changed, 67 insertions(+), 45 deletions(-) > > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c > index 3a8f210..f9c6094 100644 > --- a/drivers/staging/android/sync.c > +++ b/drivers/staging/android/sync.c > @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file > *sync_file, > goto err_put_fd; > } > > + if (data.flags || data.pad) { > + err = -EINVAL; > + goto err_put_fd; > + } > + > fence2 = sync_file_fdget(data.fd2); > if (!fence2) { > err = -ENOENT; > @@ -479,13 +484,9 @@ err_put_fd: > return err; > } > > -static int sync_fill_fence_info(struct fence *fence, void *data, int size) > +static void sync_fill_fence_info(struct fence *fence, > + struct sync_fence_info *info) > { > - struct sync_fence_info *info = data; > - > - if (size < sizeof(*info)) > - return -ENOMEM; > - > strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), > sizeof(info->obj_name)); > strlcpy(info->driver_name, fence->ops->get_driver_name(fence), > @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, > void *data, int size) > else > info->status = 0; > info->timestamp_ns = ktime_to_ns(fenc
Re: [PATCH v10 3/3] staging/android: refactor SYNC IOCTLs
Hi Greg, Any comment on this? Thanks, Gustavo 2016-03-18 Gustavo Padovan : > From: Gustavo Padovan > > Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid > future API breaks and optimize buffer allocation. > > Now num_fences can be filled by the caller to inform how many fences it > wants to retrieve from the kernel. If the num_fences passed is greater > than zero info->sync_fence_info should point to a buffer with enough space > to fit all fences. > > However if num_fences passed to the kernel is 0, the kernel will reply > with number of fences of the sync_file. > > Sending first an ioctl with num_fences = 0 can optimize buffer allocation, > in a first call with num_fences = 0 userspace will receive the actual > number of fences in the num_fences filed. > > Then it can allocate a buffer with the correct size on sync_fence_info and > call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences > in the sync_file. > > info->sync_fence_info was converted to __u64 pointer to prevent 32bit > compatibility issues. And a flags member was added. > > An example userspace code for the later would be: > > struct sync_file_info *info; > int err, size, num_fences; > > info = malloc(sizeof(*info)); > > info.flags = 0; > err = ioctl(fd, SYNC_IOC_FILE_INFO, info); > num_fences = info->num_fences; > > if (num_fences) { > info.flags = 0; > size = sizeof(struct sync_fence_info) * num_fences; > info->num_fences = num_fences; > info->sync_fence_info = (uint64_t) calloc(num_fences, > sizeof(struct > sync_fence_info)); > > err = ioctl(fd, SYNC_IOC_FILE_INFO, info); > } > > Finally the IOCTLs numbers were changed to avoid any potential old > userspace running the old API to get weird errors. Changing the opcodes > will make them fail right away. This is just a precaution, there no > upstream users of these interfaces yet and the only user is Android, but > we don't expect anyone trying to run android userspace and all it > dependencies on top of upstream kernels. > > Signed-off-by: Gustavo Padovan > Reviewed-by: Maarten Lankhorst > Acked-by: Greg Hackmann > Acked-by: Rob Clark > Acked-by: Daniel Vetter > > --- > v2: fix fence_info memory leak > > v3: Comments from Emil Velikov > - improve commit message > - remove __u64 cast > - remove check for output fields in file_info > - clean up sync_fill_fence_info() > > Comments from Maarten Lankhorst > - remove in.num_fences && !in.sync_fence_info check > - remove info->len and use only num_fences to calculate size > > Comments from Dan Carpenter > - fix info->sync_fence_info documentation > > v4: remove allocated struct sync_file_info (comment from Maarten) > > v5: merge all commits that were changing the ABI > > v6: fix -Wint-to-pointer-cast error on info.sync_fence_info > --- > drivers/staging/android/sync.c | 76 > - > drivers/staging/android/uapi/sync.h | 36 +- > 2 files changed, 67 insertions(+), 45 deletions(-) > > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c > index 3a8f210..f9c6094 100644 > --- a/drivers/staging/android/sync.c > +++ b/drivers/staging/android/sync.c > @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file > *sync_file, > goto err_put_fd; > } > > + if (data.flags || data.pad) { > + err = -EINVAL; > + goto err_put_fd; > + } > + > fence2 = sync_file_fdget(data.fd2); > if (!fence2) { > err = -ENOENT; > @@ -479,13 +484,9 @@ err_put_fd: > return err; > } > > -static int sync_fill_fence_info(struct fence *fence, void *data, int size) > +static void sync_fill_fence_info(struct fence *fence, > + struct sync_fence_info *info) > { > - struct sync_fence_info *info = data; > - > - if (size < sizeof(*info)) > - return -ENOMEM; > - > strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), > sizeof(info->obj_name)); > strlcpy(info->driver_name, fence->ops->get_driver_name(fence), > @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, > void *data, int size) > else > info->status = 0; > info->timestamp_ns = ktime_to_ns(fence->timesta
[PATCH v10 RESEND 1/3] staging/android: remove redundant comments on sync_merge_data
From: Gustavo Padovan struct sync_merge_data already have documentation on top of the struct definition. No need to duplicate it. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst --- drivers/staging/android/uapi/sync.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index a0cf357..4467c76 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -21,9 +21,9 @@ * @fence: returns the fd of the new fence to userspace */ struct sync_merge_data { - __s32 fd2; /* fd of second fence */ - charname[32]; /* name of new fence */ - __s32 fence; /* fd on newly created fence */ + __s32 fd2; + charname[32]; + __s32 fence; }; /** -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v10 RESEND 2/3] kernel.h: add u64_to_user_ptr()
From: Gustavo Padovan This function had copies in 3 different files. Unify them in kernel.h. Cc: Joe Perches Cc: Andrew Morton Cc: David Airlie Cc: Daniel Vetter Cc: Rob Clark Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 5 - drivers/gpu/drm/i915/i915_gem.c | 14 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++ include/linux/kernel.h | 5 + 6 files changed, 25 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 236ada9..afdd55d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -28,11 +28,6 @@ #define BO_LOCKED 0x4000 #define BO_PINNED 0x2000 -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, struct etnaviv_gpu *gpu, size_t nr) { @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, cmdbuf->exec_state = args->exec_state; cmdbuf->ctx = file->driver_priv; - ret = copy_from_user(bos, to_user_ptr(args->bos), + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), args->nr_bos * sizeof(*bos)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(relocs, to_user_ptr(args->relocs), + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), args->nr_relocs * sizeof(*relocs)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(stream, to_user_ptr(args->stream), + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), args->stream_size); if (ret) { ret = -EFAULT; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1048093..bb624cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev) return VGACNTRL; } -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) { unsigned long j = msecs_to_jiffies(m); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dabc089..2889716 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; void *vaddr = obj->phys_handle->vaddr + args->offset; - char __user *user_data = to_user_ptr(args->data_ptr); + char __user *user_data = u64_to_user_ptr(args->data_ptr); int ret = 0; /* We manually control the domain here and pretend that it @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, int needs_clflush = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_WRITE, - to_user_ptr(args->data_ptr), + u64_to_user_ptr(args->data_ptr), args->size)) return -EFAULT; @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, if (ret) goto out_unpin; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; offset = i915_gem_obj_ggtt_offset(obj) + args->offset; @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev, int needs_clflush_before = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -1036,12 +1036,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_READ, - to_user_ptr(args->data_ptr), + u64_to_user_ptr(args-&
[PATCH v10 RESEND 3/3] staging/android: refactor SYNC IOCTLs
From: Gustavo Padovan Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid future API breaks and optimize buffer allocation. Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences. However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file. Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed. Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file. info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues. And a flags member was added. An example userspace code for the later would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } Finally the IOCTLs numbers were changed to avoid any potential old userspace running the old API to get weird errors. Changing the opcodes will make them fail right away. This is just a precaution, there no upstream users of these interfaces yet and the only user is Android, but we don't expect anyone trying to run android userspace and all it dependencies on top of upstream kernels. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Acked-by: Greg Hackmann Acked-by: Rob Clark Acked-by: Daniel Vetter --- v2: fix fence_info memory leak v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info() Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size Comments from Dan Carpenter - fix info->sync_fence_info documentation v4: remove allocated struct sync_file_info (comment from Maarten) v5: merge all commits that were changing the ABI v6: fix -Wint-to-pointer-cast error on info.sync_fence_info --- drivers/staging/android/sync.c | 76 - drivers/staging/android/uapi/sync.h | 36 +- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..f9c6094 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,9 @@ err_put_fd: return err; } -static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) + if (inf
Re: [PATCH v10 RESEND 2/3] kernel.h: add u64_to_user_ptr()
2016-04-20 Maarten Lankhorst : > Op 19-04-16 om 22:42 schreef Gustavo Padovan: > > From: Gustavo Padovan > > > > This function had copies in 3 different files. Unify them in kernel.h. > > > > Cc: Joe Perches > > Cc: Andrew Morton > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Rob Clark > > Signed-off-by: Gustavo Padovan > > --- > > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++ > > drivers/gpu/drm/i915/i915_drv.h | 5 - > > drivers/gpu/drm/i915/i915_gem.c | 14 +++--- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++--- > > drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++ > > include/linux/kernel.h | 5 + > > 6 files changed, 25 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > > b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > > index 236ada9..afdd55d 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > > @@ -28,11 +28,6 @@ > > #define BO_LOCKED 0x4000 > > #define BO_PINNED 0x2000 > > > > -static inline void __user *to_user_ptr(u64 address) > > -{ > > - return (void __user *)(uintptr_t)address; > > -} > > - > > static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, > > struct etnaviv_gpu *gpu, size_t nr) > > { > > @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, > > void *data, > > cmdbuf->exec_state = args->exec_state; > > cmdbuf->ctx = file->driver_priv; > > > > - ret = copy_from_user(bos, to_user_ptr(args->bos), > > + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), > > args->nr_bos * sizeof(*bos)); > > if (ret) { > > ret = -EFAULT; > > goto err_submit_cmds; > > } > > > > - ret = copy_from_user(relocs, to_user_ptr(args->relocs), > > + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), > > args->nr_relocs * sizeof(*relocs)); > > if (ret) { > > ret = -EFAULT; > > goto err_submit_cmds; > > } > > > > - ret = copy_from_user(stream, to_user_ptr(args->stream), > > + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), > > args->stream_size); > > if (ret) { > > ret = -EFAULT; > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 1048093..bb624cc 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct > > drm_device *dev) > > return VGACNTRL; > > } > > > > -static inline void __user *to_user_ptr(u64 address) > > -{ > > - return (void __user *)(uintptr_t)address; > > -} > > - > > static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) > > { > > unsigned long j = msecs_to_jiffies(m); > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index dabc089..2889716 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, > > { > > struct drm_device *dev = obj->base.dev; > > void *vaddr = obj->phys_handle->vaddr + args->offset; > > - char __user *user_data = to_user_ptr(args->data_ptr); > > + char __user *user_data = u64_to_user_ptr(args->data_ptr); > > int ret = 0; > > > > /* We manually control the domain here and pretend that it > > @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, > > int needs_clflush = 0; > > struct sg_page_iter sg_iter; > > > > - user_data = to_user_ptr(args->data_ptr); > > + user_data = u64_to_user_ptr(args->data_ptr); > > remain = args->size; > > > > obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); > > @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, > > return 0; > > > > if (!access_ok(VERIFY_WRITE, > > - to_user_ptr(args->data_ptr), > > + u64_to_user_ptr(args->da
[PATCH 2/3] kernel.h: add u64_to_user_ptr()
From: Gustavo Padovan This function had copies in 3 different files. Unify them in kernel.h. Cc: Joe Perches Cc: Andrew Morton Cc: David Airlie Cc: Daniel Vetter Cc: Rob Clark Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 5 - drivers/gpu/drm/i915/i915_gem.c | 14 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++ include/linux/kernel.h | 6 ++ 6 files changed, 26 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 236ada9..afdd55d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -28,11 +28,6 @@ #define BO_LOCKED 0x4000 #define BO_PINNED 0x2000 -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, struct etnaviv_gpu *gpu, size_t nr) { @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, cmdbuf->exec_state = args->exec_state; cmdbuf->ctx = file->driver_priv; - ret = copy_from_user(bos, to_user_ptr(args->bos), + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), args->nr_bos * sizeof(*bos)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(relocs, to_user_ptr(args->relocs), + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), args->nr_relocs * sizeof(*relocs)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(stream, to_user_ptr(args->stream), + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), args->stream_size); if (ret) { ret = -EFAULT; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1048093..bb624cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev) return VGACNTRL; } -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) { unsigned long j = msecs_to_jiffies(m); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dabc089..2889716 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; void *vaddr = obj->phys_handle->vaddr + args->offset; - char __user *user_data = to_user_ptr(args->data_ptr); + char __user *user_data = u64_to_user_ptr(args->data_ptr); int ret = 0; /* We manually control the domain here and pretend that it @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, int needs_clflush = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_WRITE, - to_user_ptr(args->data_ptr), + u64_to_user_ptr(args->data_ptr), args->size)) return -EFAULT; @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, if (ret) goto out_unpin; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; offset = i915_gem_obj_ggtt_offset(obj) + args->offset; @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev, int needs_clflush_before = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -1036,12 +1036,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_READ, - to_user_ptr(args->data_ptr), + u64_to_user_pt
[PATCH 3/3] staging/android: refactor SYNC IOCTLs
From: Gustavo Padovan Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid future API breaks and optimize buffer allocation. Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences. However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file. Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed. Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file. info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues. And a flags member was added. An example userspace code for the later would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } Finally the IOCTLs numbers were changed to avoid any potential old userspace running the old API to get weird errors. Changing the opcodes will make them fail right away. This is just a precaution, there no upstream users of these interfaces yet and the only user is Android, but we don't expect anyone trying to run android userspace and all it dependencies on top of upstream kernels. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Acked-by: Greg Hackmann Acked-by: Rob Clark Acked-by: Daniel Vetter --- v2: fix fence_info memory leak v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info() Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size Comments from Dan Carpenter - fix info->sync_fence_info documentation v4: remove allocated struct sync_file_info (comment from Maarten) v5: merge all commits that were changing the ABI v6: fix -Wint-to-pointer-cast error on info.sync_fence_info --- drivers/staging/android/sync.c | 76 - drivers/staging/android/uapi/sync.h | 36 +- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..f9c6094 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,9 @@ err_put_fd: return err; } -static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) + if (inf
[PATCH 1/3] staging/android: remove redundant comments on sync_merge_data
From: Gustavo Padovan struct sync_merge_data already have documentation on top of the struct definition. No need to duplicate it. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst --- drivers/staging/android/uapi/sync.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index a0cf357..4467c76 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -21,9 +21,9 @@ * @fence: returns the fd of the new fence to userspace */ struct sync_merge_data { - __s32 fd2; /* fd of second fence */ - charname[32]; /* name of new fence */ - __s32 fence; /* fd on newly created fence */ + __s32 fd2; + charname[32]; + __s32 fence; }; /** -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging/android: remove redundant comments on sync_merge_data
I messed up with the subject prefix, but this is v11, adds typecheck() to patch 2. 2016-04-20 Gustavo Padovan : > From: Gustavo Padovan > > struct sync_merge_data already have documentation on top of the > struct definition. No need to duplicate it. > > Signed-off-by: Gustavo Padovan > Reviewed-by: Maarten Lankhorst > --- > drivers/staging/android/uapi/sync.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/android/uapi/sync.h > b/drivers/staging/android/uapi/sync.h > index a0cf357..4467c76 100644 > --- a/drivers/staging/android/uapi/sync.h > +++ b/drivers/staging/android/uapi/sync.h > @@ -21,9 +21,9 @@ > * @fence: returns the fd of the new fence to userspace > */ > struct sync_merge_data { > - __s32 fd2; /* fd of second fence */ > - charname[32]; /* name of new fence */ > - __s32 fence; /* fd on newly created fence */ > + __s32 fd2; > + charname[32]; > + __s32 fence; > }; > > /** > -- > 2.5.5 > Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] kernel.h: add u64_to_user_ptr()
2016-04-20 Joe Perches : > On Wed, 2016-04-20 at 16:18 -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > This function had copies in 3 different files. Unify them in kernel.h. > [] > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > [] > > @@ -53,6 +53,12 @@ > > > > #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + > > __must_be_array(arr)) > > > > +static inline void __user *u64_to_user_ptr(u64 address) > > +{ > > + typecheck(u64, address); > > + return (void __user *)(uintptr_t)address; > > +} > > + > > This won't work because by the time address is checked > address is already u64 > > This would need to be something like > > #define u64_to_user_ptr(x)\ > ({\ > typecheck(u64, x); \ > u64_to_user_ptr(x); \ > }) Indeed, thanks for noting and for the suggestion. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v12 0/2] staging/android: Sync ABI rework
From: Gustavo Padovan Hi, Here we clean up the Sync ABI and then improve in to a more optimized version. Also it is now less likely to need changes in the future. This is not breaking any upstream user of the sync framework, as no driver wired support for it, so far Android is the only user. A patch to AOSP will be provided to fix it there. We've made the changes in a way that userspace can figure out if the new versions are present and if not fallback to the older ABI version. More information on patch 2 description. To accomplish that we had to create a u64_to_user_ptr() macro so patch 1 adds that and also fixes some places in the kernel that were using (void __user *)(uintptr_t) cast directly. Patch 2 is the actual rework and has Ack from the people interested in it, including Android folks. Gustavo Padovan (2): kernel.h: add u64_to_user_ptr() staging/android: refactor SYNC IOCTLs drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 ++-- drivers/gpu/drm/i915/i915_drv.h | 5 -- drivers/gpu/drm/i915/i915_gem.c | 14 ++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 ++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 ++-- drivers/staging/android/sync.c | 76 +++- drivers/staging/android/uapi/sync.h | 36 + include/linux/kernel.h | 7 +++ 8 files changed, 94 insertions(+), 80 deletions(-) -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v12 1/2] kernel.h: add u64_to_user_ptr()
From: Gustavo Padovan This function had copies in 3 different files. Unify them in kernel.h. Cc: Joe Perches Cc: Andrew Morton Cc: David Airlie Cc: Daniel Vetter Cc: Rob Clark Signed-off-by: Gustavo Padovan --- v2: add typecheck() (comment from Maarten Lankhorst) v3: make u64_to_user_ptr() a macro (comment from Joe Perches) --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 5 - drivers/gpu/drm/i915/i915_gem.c | 14 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++ include/linux/kernel.h | 7 +++ 6 files changed, 27 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 236ada9..afdd55d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -28,11 +28,6 @@ #define BO_LOCKED 0x4000 #define BO_PINNED 0x2000 -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, struct etnaviv_gpu *gpu, size_t nr) { @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, cmdbuf->exec_state = args->exec_state; cmdbuf->ctx = file->driver_priv; - ret = copy_from_user(bos, to_user_ptr(args->bos), + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), args->nr_bos * sizeof(*bos)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(relocs, to_user_ptr(args->relocs), + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), args->nr_relocs * sizeof(*relocs)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(stream, to_user_ptr(args->stream), + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), args->stream_size); if (ret) { ret = -EFAULT; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1048093..bb624cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev) return VGACNTRL; } -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) { unsigned long j = msecs_to_jiffies(m); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dabc089..2889716 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; void *vaddr = obj->phys_handle->vaddr + args->offset; - char __user *user_data = to_user_ptr(args->data_ptr); + char __user *user_data = u64_to_user_ptr(args->data_ptr); int ret = 0; /* We manually control the domain here and pretend that it @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, int needs_clflush = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_WRITE, - to_user_ptr(args->data_ptr), + u64_to_user_ptr(args->data_ptr), args->size)) return -EFAULT; @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, if (ret) goto out_unpin; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; offset = i915_gem_obj_ggtt_offset(obj) + args->offset; @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev, int needs_clflush_before = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -1036,12 +1036,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, r
[PATCH v12 2/2] staging/android: refactor SYNC IOCTLs
From: Gustavo Padovan Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid future API breaks and optimize buffer allocation. Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences. However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file. Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed. Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file. info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues. And a flags member was added. An example userspace code for the later would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } Finally the IOCTLs numbers were changed to avoid any potential old userspace running the old API to get weird errors. Changing the opcodes will make them fail right away. This is just a precaution, there no upstream users of these interfaces yet and the only user is Android, but we don't expect anyone trying to run android userspace and all it dependencies on top of upstream kernels. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Acked-by: Greg Hackmann Acked-by: Rob Clark Acked-by: Daniel Vetter --- v2: fix fence_info memory leak v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info() Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size Comments from Dan Carpenter - fix info->sync_fence_info documentation v4: remove allocated struct sync_file_info (comment from Maarten) v5: merge all commits that were changing the ABI v6: fix -Wint-to-pointer-cast error on info.sync_fence_info --- drivers/staging/android/sync.c | 76 - drivers/staging/android/uapi/sync.h | 36 +- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..f9c6094 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,9 @@ err_put_fd: return err; } -static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) + if (inf
Re: [PATCH v12 1/2] kernel.h: add u64_to_user_ptr()
2016-04-22 Daniel Vetter : > On Thu, Apr 21, 2016 at 12:38:49PM -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > This function had copies in 3 different files. Unify them in kernel.h. > > > > Cc: Joe Perches > > Cc: Andrew Morton > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Rob Clark > > Signed-off-by: Gustavo Padovan > > Ack for i915 parts for merging through any suitable tree. But since this > mostly touches drm I'd propose we pull this in through drm-misc. Joe? Patch 2 needs this change and it is meant to be applied against the staging tree. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC v2 7/8] drm/fence: add fence timeline to drm_crtc
From: Gustavo Padovan Create one timeline context for each CRTC to be able to handle out-fences and signal them. It adds a few members to struct drm_crtc: fence_context, where we store the context we get from fence_context_alloc(), the fence seqno and the fence lock, that we pass in fence_init() to be used by the fence. Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/drm_crtc.c | 29 + include/drm/drm_crtc.h | 19 +++ 2 files changed, 48 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 65212ce..cf9750a 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -659,6 +659,32 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) return num; } +static const char *drm_crtc_fence_get_driver_name(struct fence *fence) +{ + struct drm_crtc *crtc = fence_to_crtc(fence); + + return crtc->dev->driver->name; +} + +static const char *drm_crtc_fence_get_timeline_name(struct fence *fence) +{ + struct drm_crtc *crtc = fence_to_crtc(fence); + + return crtc->name; +} + +static bool drm_crtc_fence_enable_signaling(struct fence *fence) +{ + return true; +} + +const struct fence_ops drm_crtc_fence_ops = { + .get_driver_name = drm_crtc_fence_get_driver_name, + .get_timeline_name = drm_crtc_fence_get_timeline_name, + .enable_signaling = drm_crtc_fence_enable_signaling, + .wait = fence_default_wait, +}; + /** * drm_crtc_init_with_planes - Initialise a new CRTC object with *specified primary and cursor planes. @@ -709,6 +735,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, return -ENOMEM; } + crtc->fence_context = fence_context_alloc(1); + spin_lock_init(&crtc->fence_lock); + crtc->base.properties = &crtc->properties; list_add_tail(&crtc->head, &config->crtc_list); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 5ba3cda..d8c32c8 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -715,6 +716,9 @@ struct drm_crtc_funcs { * @helper_private: mid-layer private data * @properties: property tracking for this CRTC * @state: current atomic state for this CRTC + * @fence_context: context for fence signalling + * @fence_lock: fence lock for the fence context + * @fence_seqno: seqno variable to create fences * @acquire_ctx: per-CRTC implicit acquire context used by atomic drivers for * legacy IOCTLs * @@ -771,6 +775,11 @@ struct drm_crtc { struct drm_crtc_state *state; + /* fence timelines info for DRM out-fences */ + unsigned int fence_context; + spinlock_t fence_lock; + unsigned long fence_seqno; + /* * For legacy crtc IOCTLs so that atomic drivers can get at the locking * acquire context. @@ -778,6 +787,16 @@ struct drm_crtc { struct drm_modeset_acquire_ctx *acquire_ctx; }; +extern const struct fence_ops drm_crtc_fence_ops; + +static inline struct drm_crtc *fence_to_crtc(struct fence *fence) +{ + if (fence->ops != &drm_crtc_fence_ops) + return NULL; + + return container_of(fence->lock, struct drm_crtc, fence_lock); +} + /** * struct drm_connector_state - mutable connector state * @connector: backpointer to the connector -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC v2 1/8] dma-buf/fence: add fence_collection fences
From: Gustavo Padovan struct fence_collection inherits from struct fence and carries a collection of fences that needs to be waited together. It is useful to translate a sync_file to a fence to remove the complexity of dealing with sync_files on DRM drivers. So even if there are many fences in the sync_file that needs to waited for a commit to happen, they all get added to the fence_collection and passed for DRM use as a standard struct fence. That means that no changes needed to any driver besides supporting fences. fence_collection's fence doesn't belong to any timeline context, so fence_is_later() and fence_later() are not meant to be called with fence_collections fences. v2: Comments by Daniel Vetter: - merge fence_collection_init() and fence_collection_add() - only add callbacks at ->enable_signalling() - remove fence_collection_put() - check for type on to_fence_collection() - adjust fence_is_later() and fence_later() to WARN_ON() if they are used with collection fences. Signed-off-by: Gustavo Padovan --- drivers/dma-buf/Makefile | 2 +- drivers/dma-buf/fence-collection.c | 159 + drivers/dma-buf/fence.c| 2 +- include/linux/fence-collection.h | 73 + include/linux/fence.h | 9 +++ 5 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 drivers/dma-buf/fence-collection.c create mode 100644 include/linux/fence-collection.h diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 4a424ec..52f818f 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,2 +1,2 @@ -obj-y := dma-buf.o fence.o reservation.o seqno-fence.o +obj-y := dma-buf.o fence.o reservation.o seqno-fence.o fence-collection.o obj-$(CONFIG_SYNC_FILE)+= sync_file.o diff --git a/drivers/dma-buf/fence-collection.c b/drivers/dma-buf/fence-collection.c new file mode 100644 index 000..88872e5 --- /dev/null +++ b/drivers/dma-buf/fence-collection.c @@ -0,0 +1,159 @@ +/* + * fence-collection: aggregate fences to be waited together + * + * Copyright (C) 2016 Collabora Ltd + * Authors: + * Gustavo Padovan + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include + +static const char *fence_collection_get_driver_name(struct fence *fence) +{ + struct fence_collection *collection = to_fence_collection(fence); + struct fence *f = collection->fences[0].fence; + + return f->ops->get_driver_name(fence); +} + +static const char *fence_collection_get_timeline_name(struct fence *fence) +{ + return "no context"; +} + +static void collection_check_cb_func(struct fence *fence, struct fence_cb *cb) +{ + struct fence_collection_cb *f_cb; + struct fence_collection *collection; + + f_cb = container_of(cb, struct fence_collection_cb, cb); + collection = f_cb->collection; + + if (atomic_dec_and_test(&collection->num_pending_fences)) + fence_signal(&collection->base); +} + +static bool fence_collection_enable_signaling(struct fence *fence) +{ + struct fence_collection *collection = to_fence_collection(fence); + int i; + + for (i = 0 ; i < collection->num_fences ; i++) { + if (fence_add_callback(collection->fences[i].fence, + &collection->fences[i].cb, + collection_check_cb_func)) { + atomic_dec(&collection->num_pending_fences); + return false; + } + } + + return !!atomic_read(&collection->num_pending_fences); +} + +static bool fence_collection_signaled(struct fence *fence) +{ + struct fence_collection *collection = to_fence_collection(fence); + + return (atomic_read(&collection->num_pending_fences) == 0); +} + +static void fence_collection_release(struct fence *fence) +{ + struct fence_collection *collection = to_fence_collection(fence); + int i; + + for (i = 0 ; i < collection->num_fences ; i++) + fence_put(collection->fences[i].fence); + + kfree(collection->fences); + fence_free(fence); +} + +static signed long fence_collection_wait(struct fence *fence, bool intr, +signed long timeout) +{ + struct fence_collection *collection = to_fence_collection(fence); + int
[RFC v2 4/8] drm/fence: allow fence waiting to be interrupted by userspace
From: Gustavo Padovan If userspace is running an synchronously atomic commit and interrupts the atomic operation during fence_wait() it will hang until the timer expires, so here we change the wait to be interruptible so it stop immediately when userspace wants to quit. Also adds the necessary error checking for fence_wait(). v2: Comment by Daniel Vetter - Add error checking for fence_wait() Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/drm_atomic_helper.c | 21 +++-- include/drm/drm_atomic_helper.h | 4 ++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 7bf678e..f1cfcce 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -993,13 +993,15 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables); * incoming fb's and stash it in the drm_plane_state. This is called after * drm_atomic_helper_swap_state() so it uses the current plane state (and * just uses the atomic state to find the changed planes) + * + * Return 0 on sucess or < 0 on error. */ -void drm_atomic_helper_wait_for_fences(struct drm_device *dev, - struct drm_atomic_state *state) +int drm_atomic_helper_wait_for_fences(struct drm_device *dev, + struct drm_atomic_state *state) { struct drm_plane *plane; struct drm_plane_state *plane_state; - int i; + int i, ret; for_each_plane_in_state(state, plane, plane_state, i) { if (!plane->state->fence) @@ -1007,10 +1009,14 @@ void drm_atomic_helper_wait_for_fences(struct drm_device *dev, WARN_ON(!plane->state->fb); - fence_wait(plane->state->fence, false); + ret = fence_wait(plane->state->fence, true); + if (ret) + return ret; fence_put(plane->state->fence); plane->state->fence = NULL; } + + return 0; } EXPORT_SYMBOL(drm_atomic_helper_wait_for_fences); @@ -1174,7 +1180,9 @@ int drm_atomic_helper_commit(struct drm_device *dev, * current layout. */ - drm_atomic_helper_wait_for_fences(dev, state); + ret = drm_atomic_helper_wait_for_fences(dev, state); + if (ret) + goto out; drm_atomic_helper_commit_modeset_disables(dev, state); @@ -1184,11 +1192,12 @@ int drm_atomic_helper_commit(struct drm_device *dev, drm_atomic_helper_wait_for_vblanks(dev, state); +out: drm_atomic_helper_cleanup_planes(dev, state); drm_atomic_state_free(state); - return 0; + return ret; } EXPORT_SYMBOL(drm_atomic_helper_commit); diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index fe9d89c..3a5d2e5 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -42,8 +42,8 @@ int drm_atomic_helper_commit(struct drm_device *dev, struct drm_atomic_state *state, bool async); -void drm_atomic_helper_wait_for_fences(struct drm_device *dev, - struct drm_atomic_state *state); +int drm_atomic_helper_wait_for_fences(struct drm_device *dev, + struct drm_atomic_state *state); bool drm_atomic_helper_framebuffer_changed(struct drm_device *dev, struct drm_atomic_state *old_state, struct drm_crtc *crtc); -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC v2 2/8] Documentation: add fence-collection to kernel DocBook
From: Gustavo Padovan Include fence-collection files in the DocBook. Signed-off-by: Gustavo Padovan --- Documentation/DocBook/device-drivers.tmpl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl index 509a187..68539e5 100644 --- a/Documentation/DocBook/device-drivers.tmpl +++ b/Documentation/DocBook/device-drivers.tmpl @@ -132,8 +132,10 @@ X!Edrivers/base/interface.c !Edrivers/dma-buf/dma-buf.c !Edrivers/dma-buf/fence.c !Edrivers/dma-buf/seqno-fence.c +!Edrivers/dma-buf/fence-collection.c !Iinclude/linux/fence.h !Iinclude/linux/seqno-fence.h +!Iinclude/linux/fence-collection.h !Edrivers/dma-buf/reservation.c !Iinclude/linux/reservation.h !Edrivers/dma-buf/sync_file.c -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC v2 5/8] drm/fence: add in-fences support
From: Gustavo Padovan There is now a new property called FENCE_FD attached to every plane state that receives the sync_file fd from userspace via the atomic commit IOCTL. The fd is then translated to a fence (that may be a fence_collection subclass or just a normal fence) and then used by DRM to fence_wait() for all fences in the sync_file to signal. So it only commits when all framebuffers are ready to scanout. Signed-off-by: Gustavo Padovan v2: Comments by Daniel Vetter: - remove set state->fence = NULL in destroy phase - accept fence -1 as valid and just return 0 - do not call fence_get() - sync_file_fences_get() already calls it - fence_put() if state->fence is already set, in case userspace set the property more than once. --- drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c| 14 ++ drivers/gpu/drm/drm_atomic_helper.c | 3 +++ drivers/gpu/drm/drm_crtc.c | 7 +++ include/drm/drm_crtc.h | 1 + 5 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f2a74d0..3c987e3 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -12,6 +12,7 @@ menuconfig DRM select I2C select I2C_ALGOBIT select DMA_SHARED_BUFFER + select SYNC_FILE help Kernel-level support for the Direct Rendering Infrastructure (DRI) introduced in XFree86 4.0. If you say Y here, you need to select diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 8ee1db8..13674c7 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,6 +30,7 @@ #include #include #include +#include /** * drm_atomic_state_default_release - @@ -680,6 +681,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, drm_atomic_set_fb_for_plane(state, fb); if (fb) drm_framebuffer_unreference(fb); + } else if (property == config->prop_fence_fd) { + if (U642I64(val) == -1) + return 0; + + if (state->fence) + fence_put(state->fence); + + state->fence = sync_file_fences_get(val); + if (!state->fence) + return -EINVAL; + } else if (property == config->prop_crtc_id) { struct drm_crtc *crtc = drm_crtc_find(dev, val); return drm_atomic_set_crtc_for_plane(state, crtc); @@ -737,6 +749,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, if (property == config->prop_fb_id) { *val = (state->fb) ? state->fb->base.id : 0; + } else if (property == config->prop_fence_fd) { + *val = -1; } else if (property == config->prop_crtc_id) { *val = (state->crtc) ? state->crtc->base.id : 0; } else if (property == config->prop_crtc_x) { diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index f1cfcce..866f332 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2696,6 +2696,9 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane *plane, { if (state->fb) drm_framebuffer_unreference(state->fb); + + if (state->fence) + fence_put(state->fence); } EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 55ffde5..65212ce 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1278,6 +1278,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(&plane->base, config->prop_fb_id, 0); + drm_object_attach_property(&plane->base, config->prop_fence_fd, -1); drm_object_attach_property(&plane->base, config->prop_crtc_id, 0); drm_object_attach_property(&plane->base, config->prop_crtc_x, 0); drm_object_attach_property(&plane->base, config->prop_crtc_y, 0); @@ -1533,6 +1534,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_fb_id = prop; + prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC, + "FENCE_FD", -1, INT_MAX); + if (!prop) + return -ENOMEM; + dev->mode_config.prop_fence_fd = prop; + prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC, "CRTC_ID", DRM_MODE_OBJECT_CRTC); if (!prop) diff --git a/include/drm/drm_crtc.h b/include/drm/d
[RFC v2 8/8] drm/fence: add out-fences support
From: Gustavo Padovan Support DRM out-fences creating a sync_file with a fence for each crtc update with the DRM_MODE_ATOMIC_OUT_FENCE flag. We then send an struct drm_out_fences array with the out-fences fds back in the drm_atomic_ioctl() as an out arg in the out_fences_ptr field. struct drm_out_fences { __u32 crtc_id; __u32 fd; }; v2: Comment by Rob Clark: - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here. Comment by Daniel Vetter: - Add clean up code for out_fences Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/drm_atomic.c | 163 +-- include/drm/drm_crtc.h | 10 +++ include/uapi/drm/drm_mode.h | 11 ++- 3 files changed, 179 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 5f9d434..06c6007 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1566,6 +1566,133 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_clean_old_fb); +static struct drm_out_fence_state *get_out_fence(struct drm_device *dev, +struct drm_atomic_state *state, +uint32_t __user *out_fences_ptr, +uint64_t count_out_fences, +uint64_t user_data) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + struct drm_out_fences *out_fences; + struct drm_out_fence_state *fence_state; + int num_fences = 0; + int i, ret; + + if (count_out_fences > dev->mode_config.num_crtc) + return ERR_PTR(-EINVAL); + + out_fences = kcalloc(count_out_fences, sizeof(*out_fences), +GFP_KERNEL); + if (!out_fences) + return ERR_PTR(-ENOMEM); + + fence_state = kcalloc(count_out_fences, sizeof(*fence_state), +GFP_KERNEL); + if (!fence_state) { + kfree(out_fences); + return ERR_PTR(-ENOMEM); + } + + for (i = 0 ; i < count_out_fences ; i++) + fence_state[i].fd = -1; + + for_each_crtc_in_state(state, crtc, crtc_state, i) { + struct drm_pending_vblank_event *e; + struct fence *fence; + char name[32]; + + fence = kzalloc(sizeof(*fence), GFP_KERNEL); + if (!fence) { + ret = -ENOMEM; + goto out; + } + + fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, + crtc->fence_context, crtc->fence_seqno); + + snprintf(name, sizeof(name), "crtc-%d_%lu", +drm_crtc_index(crtc), crtc->fence_seqno++); + + fence_state[i].fd = get_unused_fd_flags(O_CLOEXEC); + if (fence_state[i].fd < 0) { + fence_put(fence); + ret = fence_state[i].fd; + goto out; + } + + fence_state[i].sync_file = sync_file_create(name, fence); + if(!fence_state[i].sync_file) { + fence_put(fence); + ret = -ENOMEM; + goto out; + } + + if (crtc_state->event) { + crtc_state->event->base.fence = fence; + } else { + e = create_vblank_event(dev, NULL, fence, user_data); + if (!e) { + ret = -ENOMEM; + goto out; + } + + crtc_state->event = e; + } + + out_fences[num_fences].crtc_id = crtc->base.id; + out_fences[num_fences].fd = fence_state[i].fd; + num_fences++; + } + + if (copy_to_user(out_fences_ptr, out_fences, +num_fences * sizeof(*out_fences))) { + ret = -EFAULT; + goto out; + } + + kfree(out_fences); + + return fence_state; + +out: + for (i = 0 ; i < count_out_fences ; i++) { + if (fence_state[i].sync_file) + sync_file_put(fence_state[i].sync_file); + if (fence_state[i].fd >= 0) + put_unused_fd(fence_state[i].fd); + } + + kfree(fence_state); + kfree(out_fences); + + return ERR_PTR(ret); +} + +static void install_out_fence(uint64_t count_out_fences, + struct drm_out_fence_state *state) +{ + int i; + + for (i = 0 ; i < count_out_fences ; i++) { + if (state[i].sync_file) + sync_file_ins
[RFC v2 0/8] drm: explicit fencing support
From: Gustavo Padovan Hi, Currently the Linux Kernel only have an implicit fencing mechanism where the fence are attached directly to buffers and userspace is unaware of what is happening. On the other hand explicit fencing which is not supported yet by Linux but it expose fences to the userspace to handle fencing between producer/consumer explicitely. For that we use the Android Sync Framework[1], a explicit fencing mechanism that help the userspace handles fences directly. It has the concept of sync_file (called sync_fence in Android) that expose the driver's fences to userspace via file descriptors. File descriptors are useful because we can pass them around between process. The Sync Framework is currently in the staging tree and on the process to be de-staged[2]. With explicit fencing we have a global mechanism that optimizes the flow of buffers between consumers and producers, avoid a lot of waiting. So instead of waiting for a buffer to be processed by the GPU before sending it to DRM in an Atomic IOCTL we can get a sync_file fd from the GPU driver at the moment we submit the buffer processing. The compositor then passes these fds to DRM in a atomic commit request, that will not be displayed until the fences signal, i.e, the GPU finished processing the buffer and it is ready to display. In DRM the fences we wait on before displaying a buffer are called in-fences. Vice-versa, we have out-fences, to sychronize the return of buffers to GPU (producer) to be processed again. When DRM receives an atomic request with a special flag set it generates one fence per-crtc and attach it to a per-crtc sync_file. It then returns the array of sync_file fds to userspace as an atomic_ioctl out arg. With the fences available userspace can forward these fences to the GPU, where it will wait the fence to signal before starting to process on buffer again. Explicit fencing with Sync Framework allows buffer suballocation. Userspace get a large buffer and divides it into small ones and submit requests to process them, each subbuffer gets and sync_file fd and can be processed in parallel. This is not even possible with implicit fencing. While these are out-fences in DRM (the consumer) they become in-fences once they get to the GPU (the producer). DRM explicit fences are opt-in, as the default will still be implicit fencing. To enable explicit in-fences one just need to pass a sync_file fd in the FENCE_FD plane property. *In-fences are per-plane*, i.e., per framebuffer. For out-fences, just enabling DRM_MODE_ATOMIC_OUT_FENCE flag is enough. *Out-fences are per-crtc*. In-fences - In the first discussions on #dri-devel on IRC we decided to hide the Sync Framework from DRM drivers to reduce complexity, so as soon we get the fd via FENCE_FD plane property we convert the sync_file fd to a struct fence. However a sync_file might contain more than one fence, so we created the fence_collection concept. struct fence_collection is a subclass of struct fence and stores a group of fences that needs to be waited together, in other words, all the fences in the sync_file. Then we just use the already in place fence support to wait on those fences. Once the producer calls fence_signal() for all fences on wait we can proceed with the atomic commit and display the framebuffers. DRM drivers only needs to be converted to struct fence to make use of this feature. Out-fences -- Passing the DRM_MODE_ATOMIC_OUT_FENCE flag to an atomic request enables out-fences. The kernel then creates a fence, attach it to a sync_file and install this file on a unused fd for each crtc. Userspace get the fence back as an array of per-crtc sync_file fds. DRM core use the already in place drm_event infrastructure to help signal fences, we've added a fence pointer to struct drm_pending_event. If the atomic update received requested an PAGE_FLIP_EVENT we just use the same drm_pending_event and set our fence there, otherwise we just create an event with a NULL file_priv to set our fence. On vblank we just call fence_signal() to signal that the buffer related to this fence is *now* on the screen. Note that this is exactly the opposite behaviour from Android, where the fences are signaled when they are not on display anymore, so free to be reused. No changes are required to DRM drivers to have out-fences support, apart from atomic support of course. Kernel tree --- For those who want all patches on this RFC are in my tree. The tree includes all sync frameworks patches needed at the moment: https://git.kernel.org/cgit/linux/kernel/git/padovan/linux.git/log/?h=fences I also hacked some poor some fake fences support to modetest here: https://git.collabora.com/cgit/user/padovan/libdrm.git/log/?h=atomic v2 changes -- The changes from RFC v1 to RFC v2 are in the patches description. TODO * upstream Android Sync ABI changes * de-stage Android Sync Framework * Upstream sync selftests (Emilio Lopez)
[RFC v2 3/8] dma-buf/sync_file: add sync_file_fences_get()
From: Gustavo Padovan Creates a function that given an sync file descriptor returns a fence_collection containing all fences in the sync_file. If there is only one fence in the sync_file this fence itself is returned, however if there is more than one, a fence_collection fence is returned. v2: Comments by Daniel Vetter - Adapt to new version of fence_collection_init() - Hold a reference for the fence we return Signed-off-by: Gustavo Padovan --- drivers/dma-buf/sync_file.c | 60 + include/linux/sync_file.h | 3 +++ 2 files changed, 63 insertions(+) diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 40ee098..5dd76c0 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -147,6 +148,62 @@ void sync_file_install(struct sync_file *sync_file, int fd) } EXPORT_SYMBOL(sync_file_install); +/** + * sync_file_fences_get - get the fence(s) related to the sync_file fd + * @fd:sync_file fd to get the fence(s) from + * + * Ensures @fd references a valid sync_file and returns a fence that + * represents all fence in the sync_file. + * + * If there is only one fence in the sync_file, this fence is returned. + * If there is more than one, a fence_collection containing all fences + * is created and its base fence object is returned. + * On both cases a reference to the returned fence is held. On error + * NULL is returned. + */ +struct fence *sync_file_fences_get(int fd) +{ + struct sync_file *sync_file; + struct fence_collection *collection; + struct fence_collection_cb *cb; + int i; + + sync_file = sync_file_fdget(fd); + if (!sync_file) + return NULL; + + if (sync_file->num_fences == 1) { + struct fence *fence = sync_file->cbs[0].fence; + + fence_get(fence); + sync_file_put(sync_file); + return fence; + } + + cb = kcalloc(sync_file->num_fences, sizeof(*cb), GFP_KERNEL); + if (!cb) { + sync_file_put(sync_file); + return NULL; + } + + for (i = 0 ; i < sync_file->num_fences ; i++) + cb[i].fence = sync_file->cbs[i].fence; + + collection = fence_collection_create(sync_file->num_fences, cb); + if (!collection) { + kfree(cb); + sync_file_put(sync_file); + return NULL; + } + + sync_file->collection = collection; + fence_get(&collection->base); + sync_file_put(sync_file); + + return &collection->base; +} +EXPORT_SYMBOL(sync_file_fences_get); + static void sync_file_add_pt(struct sync_file *sync_file, int *i, struct fence *fence) { @@ -234,6 +291,9 @@ static void sync_file_free(struct kref *kref) kref); int i; + if (sync_file->collection) + fence_put(&sync_file->collection->base); + for (i = 0; i < sync_file->num_fences; ++i) { fence_remove_callback(sync_file->cbs[i].fence, &sync_file->cbs[i].cb); diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h index 900fa0c..3eb3aad 100644 --- a/include/linux/sync_file.h +++ b/include/linux/sync_file.h @@ -35,6 +35,7 @@ struct sync_file_cb { * @num_fences:number of sync_pts in the fence * @wq:wait queue for fence signaling * @status:0: signaled, >0:active, <0: error + * @collection:collection with the fences in the sync_file * @cbs: sync_pts callback information */ struct sync_file { @@ -49,6 +50,7 @@ struct sync_file { wait_queue_head_t wq; atomic_tstatus; + struct fence_collection *collection; struct sync_file_cb cbs[]; }; @@ -59,4 +61,5 @@ void sync_file_install(struct sync_file *sync_file, int fd); struct sync_file *sync_file_merge(const char *name, struct sync_file *a, struct sync_file *b); +struct fence *sync_file_fences_get(int fd); #endif /* _LINUX_SYNC_H */ -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC v2 6/8] drm/fence: add fence to drm_pending_event
From: Gustavo Padovan Now a drm_pending_event can either send a real drm_event or signal a fence, or both. It allow us to signal via fences when the buffer is displayed on the screen. Which in turn means that the previous buffer is not in use anymore and can be freed or sent back to another driver for processing. v2: Comments from Daniel Vetter - call fence_signal in drm_send_event_locked() - remove unneeded !e->event check Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/drm_atomic.c | 19 +-- drivers/gpu/drm/drm_fops.c | 8 +++- include/drm/drmP.h | 2 ++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 13674c7..5f9d434 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1437,7 +1437,8 @@ EXPORT_SYMBOL(drm_atomic_async_commit); */ static struct drm_pending_vblank_event *create_vblank_event( - struct drm_device *dev, struct drm_file *file_priv, uint64_t user_data) + struct drm_device *dev, struct drm_file *file_priv, + struct fence *fence, uint64_t user_data) { struct drm_pending_vblank_event *e = NULL; int ret; @@ -1450,12 +1451,17 @@ static struct drm_pending_vblank_event *create_vblank_event( e->event.base.length = sizeof(e->event); e->event.user_data = user_data; - ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base); - if (ret) { - kfree(e); - return NULL; + if (file_priv) { + ret = drm_event_reserve_init(dev, file_priv, &e->base, +&e->event.base); + if (ret) { + kfree(e); + return NULL; + } } + e->base.fence = fence; + return e; } @@ -1682,7 +1688,8 @@ retry: for_each_crtc_in_state(state, crtc, crtc_state, i) { struct drm_pending_vblank_event *e; - e = create_vblank_event(dev, file_priv, arg->user_data); + e = create_vblank_event(dev, file_priv, NULL, + arg->user_data); if (!e) { ret = -ENOMEM; goto out; diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index aeef58e..7872635 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -801,8 +801,14 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) { assert_spin_locked(&dev->event_lock); + if (e->fence) { + fence_signal(e->fence); + fence_put(e->fence); + } + if (!e->file_priv) { - e->destroy(e); + if (e->destroy) + e->destroy(e); return; } diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 3c8422c..2ae6d7e 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -56,6 +56,7 @@ #include #include #include +#include #include #include @@ -282,6 +283,7 @@ struct drm_ioctl_desc { /* Event queued up for userspace to read */ struct drm_pending_event { struct drm_event *event; + struct fence *fence; struct list_head link; struct list_head pending_link; struct drm_file *file_priv; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v12 1/2] kernel.h: add u64_to_user_ptr()
2016-04-26 Lucas Stach : > Am Donnerstag, den 21.04.2016, 12:38 -0300 schrieb Gustavo Padovan: > > From: Gustavo Padovan > > > > This function had copies in 3 different files. Unify them in kernel.h. > > > > Cc: Joe Perches > > Cc: Andrew Morton > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Rob Clark > > Signed-off-by: Gustavo Padovan > > > Though I normally prefer static inline functions, I see the benefits of > using the macro form here. > > For the etnaviv part: > Acked-by: Lucas Stach Thank you all! I'll collect the Acks and send a new version, so it is easier for Greg to pick it up. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v13 0/2] staging/android: Sync ABI rework
From: Gustavo Padovan Hi Greg, This patchset clean up the Sync ABI and then improve in to a more optimized version. Also it is now less likely to need changes in the future. This is not breaking any upstream user of the sync framework, as no driver wired support for it, so far Android is the only user. A patch to AOSP will be provided to fix it there. We've made the changes in a way that userspace can figure out if the new versions are present and if not fallback to the older ABI version. More information on patch 2 description. To accomplish that we had to create a u64_to_user_ptr() macro so patch 1 adds that and also fixes some places in the kernel that were using (void __user *)(uintptr_t) cast directly. It already has Acks from maintainers of drm drivers it changes. Patch 2 is the actual rework and has Ack from the people interested in it, including Android folks. Gustavo Padovan (2): kernel.h: add u64_to_user_ptr() staging/android: refactor SYNC IOCTLs drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 ++-- drivers/gpu/drm/i915/i915_drv.h | 5 -- drivers/gpu/drm/i915/i915_gem.c | 14 ++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 ++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 ++-- drivers/staging/android/sync.c | 76 +++- drivers/staging/android/uapi/sync.h | 36 + include/linux/kernel.h | 7 +++ 8 files changed, 94 insertions(+), 80 deletions(-) -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v13 2/2] staging/android: refactor SYNC IOCTLs
From: Gustavo Padovan Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid future API breaks and optimize buffer allocation. Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences. However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file. Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed. Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file. info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues. And a flags member was added. An example userspace code for the later would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } Finally the IOCTLs numbers were changed to avoid any potential old userspace running the old API to get weird errors. Changing the opcodes will make them fail right away. This is just a precaution, there no upstream users of these interfaces yet and the only user is Android, but we don't expect anyone trying to run android userspace and all it dependencies on top of upstream kernels. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Acked-by: Greg Hackmann Acked-by: Rob Clark Acked-by: Daniel Vetter --- v2: fix fence_info memory leak v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info() Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size Comments from Dan Carpenter - fix info->sync_fence_info documentation v4: remove allocated struct sync_file_info (comment from Maarten) v5: merge all commits that were changing the ABI v6: fix -Wint-to-pointer-cast error on info.sync_fence_info --- drivers/staging/android/sync.c | 76 - drivers/staging/android/uapi/sync.h | 36 +- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..f9c6094 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,9 @@ err_put_fd: return err; } -static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) + if (inf
[PATCH v13 1/2] kernel.h: add u64_to_user_ptr()
From: Gustavo Padovan This function had copies in 3 different files. Unify them in kernel.h. Cc: Joe Perches Cc: Andrew Morton Cc: David Airlie Cc: Daniel Vetter Cc: Rob Clark Signed-off-by: Gustavo Padovan Acked-by: Daniel Vetter[drm/i915/] Acked-by: Rob Clark[drm/msm/] Acked-by: Lucas Stach [drm/etinav/] Acked-by: Maarten Lankhorst --- v2: add typecheck() (comment from Maarten Lankhorst) v3: make u64_to_user_ptr() a macro (comment from Joe Perches) v4: collect all Acked-bys --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 5 - drivers/gpu/drm/i915/i915_gem.c | 14 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++ include/linux/kernel.h | 7 +++ 6 files changed, 27 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 236ada9..afdd55d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -28,11 +28,6 @@ #define BO_LOCKED 0x4000 #define BO_PINNED 0x2000 -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, struct etnaviv_gpu *gpu, size_t nr) { @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, cmdbuf->exec_state = args->exec_state; cmdbuf->ctx = file->driver_priv; - ret = copy_from_user(bos, to_user_ptr(args->bos), + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), args->nr_bos * sizeof(*bos)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(relocs, to_user_ptr(args->relocs), + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), args->nr_relocs * sizeof(*relocs)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(stream, to_user_ptr(args->stream), + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), args->stream_size); if (ret) { ret = -EFAULT; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1048093..bb624cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev) return VGACNTRL; } -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) { unsigned long j = msecs_to_jiffies(m); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dabc089..2889716 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; void *vaddr = obj->phys_handle->vaddr + args->offset; - char __user *user_data = to_user_ptr(args->data_ptr); + char __user *user_data = u64_to_user_ptr(args->data_ptr); int ret = 0; /* We manually control the domain here and pretend that it @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, int needs_clflush = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_WRITE, - to_user_ptr(args->data_ptr), + u64_to_user_ptr(args->data_ptr), args->size)) return -EFAULT; @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, if (ret) goto out_unpin; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; offset = i915_gem_obj_ggtt_offset(obj) + args->offset; @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev, int needs_clflush_before = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size;
[PATCH 01/12] staging/android: remove redundant comments on sync_merge_data
From: Gustavo Padovan struct sync_merge_data already have documentation on top of the struct definition. No need to duplicate it. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst --- drivers/staging/android/uapi/sync.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index 7de5d6a..413303d 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -23,9 +23,9 @@ * @pad: padding for 64-bit alignment, should always be zero */ struct sync_merge_data { - charname[32]; /* name of new fence */ - __s32 fd2; /* fd of second fence */ - __s32 fence; /* fd on newly created fence */ + charname[32]; + __s32 fd2; + __s32 fence; __u32 flags; __u32 pad; }; @@ -33,8 +33,8 @@ struct sync_merge_data { /** * struct sync_fence_info - detailed fence information * @obj_name: name of parent sync_timeline - * @driver_name: name of driver implementing the parent - * @status:status of the fence 0:active 1:signaled <0:error +* @driver_name:name of driver implementing the parent +* @status: status of the fence 0:active 1:signaled <0:error * @flags: fence_info flags * @timestamp_ns: timestamp of status change in nanoseconds */ -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/12] staging/android: move sync_file functions comments to sync.c
From: Gustavo Padovan To keep comments in line with drivers/dma-buf/ move all sync_file comments to sync.c. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 26 +- drivers/staging/android/sync.h | 31 --- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index b965e2a..a89ded0 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -173,7 +173,14 @@ static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) wake_up_all(&sync_file->wq); } -/* TODO: implement a create which takes more that one fence */ +/** + * sync_fence_create() - creates a sync fence + * @name: name of fence to create + * @fence: fence to add to the sync_fence + * + * Creates a sync_file containg @fence. Once this is called, the sync_file + * takes ownership of @fence. + */ struct sync_file *sync_file_create(const char *name, struct fence *fence) { struct sync_file *sync_file; @@ -198,6 +205,13 @@ struct sync_file *sync_file_create(const char *name, struct fence *fence) } EXPORT_SYMBOL(sync_file_create); +/** + * sync_file_fdget() - get a sync_file from an fd + * @fd:fd referencing a fence + * + * Ensures @fd references a valid sync_file, increments the refcount of the + * backing file. Returns the sync_file or NULL in case of error. + */ struct sync_file *sync_file_fdget(int fd) { struct file *file = fget(fd); @@ -229,6 +243,16 @@ static void sync_file_add_pt(struct sync_file *sync_file, int *i, } } +/** + * sync_file_merge() - merge two sync_files + * @name: name of new fence + * @a: sync_file a + * @b: sync_file b + * + * Creates a new sync_file which contains copies of all the fences in both + * @a and @b. @a and @b remain valid, independent sync_file. Returns the + * new merged sync_file or NULL in case of error. + */ struct sync_file *sync_file_merge(const char *name, struct sync_file *a, struct sync_file *b) { diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index c45cc7b..925fba5 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -167,40 +167,9 @@ void sync_timeline_signal(struct sync_timeline *obj); */ struct fence *sync_pt_create(struct sync_timeline *parent, int size); -/** - * sync_fence_create() - creates a sync fence - * @name: name of fence to create - * @fence: fence to add to the sync_fence - * - * Creates a sync_file containg @fence. Once this is called, the sync_file - * takes ownership of @fence. - */ struct sync_file *sync_file_create(const char *name, struct fence *fence); - -/* - * API for sync_file consumers - */ - -/** - * sync_file_merge() - merge two sync_files - * @name: name of new fence - * @a: sync_file a - * @b: sync_file b - * - * Creates a new sync_file which contains copies of all the fences in both - * @a and @b. @a and @b remain valid, independent sync_file. Returns the - * new merged sync_file or NULL in case of error. - */ struct sync_file *sync_file_merge(const char *name, struct sync_file *a, struct sync_file *b); - -/** - * sync_file_fdget() - get a sync_file from an fd - * @fd:fd referencing a fence - * - * Ensures @fd references a valid sync_file, increments the refcount of the - * backing file. Returns the sync_file or NULL in case of error. - */ struct sync_file *sync_file_fdget(int fd); #ifdef CONFIG_DEBUG_FS -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/12] staging/android: drop sync_file_install() and sync_file_put()
From: Gustavo Padovan These two functions are just wrappers for one line functions, they call fd_install() and fput() respectively, so just get rid of them and use fd_install() and fput() directly for more simplicity. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 20 drivers/staging/android/sync.h | 19 --- drivers/staging/android/sync_debug.c | 4 ++-- 3 files changed, 6 insertions(+), 37 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index f9c6094..b965e2a 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -216,18 +216,6 @@ err: } EXPORT_SYMBOL(sync_file_fdget); -void sync_file_put(struct sync_file *sync_file) -{ - fput(sync_file->file); -} -EXPORT_SYMBOL(sync_file_put); - -void sync_file_install(struct sync_file *sync_file, int fd) -{ - fd_install(fd, sync_file->file); -} -EXPORT_SYMBOL(sync_file_install); - static void sync_file_add_pt(struct sync_file *sync_file, int *i, struct fence *fence) { @@ -469,15 +457,15 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fence3; } - sync_file_install(fence3, fd); - sync_file_put(fence2); + fd_install(fd, fence3->file); + fput(fence2->file); return 0; err_put_fence3: - sync_file_put(fence3); + fput(fence3->file); err_put_fence2: - sync_file_put(fence2); + fput(fence2->file); err_put_fd: put_unused_fd(fd); diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index d2a1734..c45cc7b 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -203,25 +203,6 @@ struct sync_file *sync_file_merge(const char *name, */ struct sync_file *sync_file_fdget(int fd); -/** - * sync_file_put() - puts a reference of a sync_file - * @sync_file: sync_file to put - * - * Puts a reference on @sync_fence. If this is the last reference, the - * sync_fil and all it's sync_pts will be freed - */ -void sync_file_put(struct sync_file *sync_file); - -/** - * sync_file_install() - installs a sync_file into a file descriptor - * @sync_file: sync_file to install - * @fd:file descriptor in which to install the fence - * - * Installs @sync_file into @fd. @fd's should be acquired through - * get_unused_fd_flags(O_CLOEXEC). - */ -void sync_file_install(struct sync_file *sync_file, int fd); - #ifdef CONFIG_DEBUG_FS void sync_timeline_debug_add(struct sync_timeline *obj); diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 5a7ec58..e4b0e41 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -272,12 +272,12 @@ static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj, data.fence = fd; if (copy_to_user((void __user *)arg, &data, sizeof(data))) { - sync_file_put(sync_file); + fput(sync_file->file); err = -EFAULT; goto err; } - sync_file_install(sync_file, fd); + fd_install(fd, sync_file->file); return 0; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/12] De-stage Sync File Framework
From: Gustavo Padovan Hi, This patchset sits on top of Sync ABI Rework v13: https://www.spinics.net/lists/dri-devel/msg105667.html The first eight clean up and prepare sync_file for de-staging. The last four patches do the de-staging, moving files to drivers/dma-buf/ and include/linux/ plus adding Documentation. As the de-stage depends upon many changes on the staging tree it would be good to get all the patches merged through the staging tree if Sumit agrees with that. The next step on the Sync de-stage is clean up the remaining bits of the Sync Framework, mainly SW_SYNC, which is only used for testing. Thanks, Gustavo --- Gustavo Padovan (12): staging/android: remove redundant comments on sync_merge_data staging/android: drop sync_file_install() and sync_file_put() staging/android: move sync_file functions comments to sync.c staging/android: make sync_file_merge() static staging/android: make sync_file_fdget() static staging/android: prepare sync_file for de-staging staging/android: improve documentation for sync_file staging/android: style fix: alignment to match the open parenthesis dma-buf/sync_file: de-stage sync_file headers dma-buf/sync_file: de-stage sync_file Documentation: include sync_file into DocBook Documentation: add Sync File doc Documentation/DocBook/device-drivers.tmpl | 2 + Documentation/dma-buf-sync_file.txt | 65 + drivers/Kconfig | 2 + drivers/dma-buf/Kconfig | 11 + drivers/dma-buf/Makefile | 1 + drivers/dma-buf/sync_file.c | 394 ++ drivers/staging/android/Kconfig | 1 + drivers/staging/android/sync.c| 362 --- drivers/staging/android/sync.h| 91 +-- drivers/staging/android/sync_debug.c | 5 +- drivers/staging/android/uapi/sync.h | 100 include/linux/sync_file.h | 57 + include/uapi/linux/sync_file.h| 100 13 files changed, 638 insertions(+), 553 deletions(-) create mode 100644 Documentation/dma-buf-sync_file.txt create mode 100644 drivers/dma-buf/Kconfig create mode 100644 drivers/dma-buf/sync_file.c delete mode 100644 drivers/staging/android/uapi/sync.h create mode 100644 include/linux/sync_file.h create mode 100644 include/uapi/linux/sync_file.h -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/12] staging/android: make sync_file_merge() static
From: Gustavo Padovan There is no plan in the near future to use this function outside of this file so keep it as static. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 4 ++-- drivers/staging/android/sync.h | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index a89ded0..c441fde 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -253,8 +253,8 @@ static void sync_file_add_pt(struct sync_file *sync_file, int *i, * @a and @b. @a and @b remain valid, independent sync_file. Returns the * new merged sync_file or NULL in case of error. */ -struct sync_file *sync_file_merge(const char *name, - struct sync_file *a, struct sync_file *b) +static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, +struct sync_file *b) { int num_fences = a->num_fences + b->num_fences; struct sync_file *sync_file; diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 925fba5..ffc6df6 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -168,8 +168,6 @@ void sync_timeline_signal(struct sync_timeline *obj); struct fence *sync_pt_create(struct sync_timeline *parent, int size); struct sync_file *sync_file_create(const char *name, struct fence *fence); -struct sync_file *sync_file_merge(const char *name, - struct sync_file *a, struct sync_file *b); struct sync_file *sync_file_fdget(int fd); #ifdef CONFIG_DEBUG_FS -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/12] staging/android: prepare sync_file for de-staging
From: Gustavo Padovan Move its functions and structs to their own file. Also moves function's docs to the .c file. Signed-off-by: Gustavo Padovan --- drivers/staging/android/Makefile | 2 +- drivers/staging/android/sync.c | 373 --- drivers/staging/android/sync.h | 38 +- drivers/staging/android/sync_debug.c | 1 + drivers/staging/android/sync_file.c| 393 + drivers/staging/android/sync_file.h| 57 +++ .../staging/android/uapi/{sync.h => sync_file.h} | 0 7 files changed, 454 insertions(+), 410 deletions(-) create mode 100644 drivers/staging/android/sync_file.c create mode 100644 drivers/staging/android/sync_file.h rename drivers/staging/android/uapi/{sync.h => sync_file.h} (100%) diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index 980d6dc..ebc2df1 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -4,5 +4,5 @@ obj-y += ion/ obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)+= lowmemorykiller.o -obj-$(CONFIG_SYNC) += sync.o sync_debug.o +obj-$(CONFIG_SYNC) += sync_file.o sync.o sync_debug.o obj-$(CONFIG_SW_SYNC) += sw_sync.o diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 1239684..1d14c83 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -16,10 +16,7 @@ #include #include -#include -#include #include -#include #include #include #include @@ -32,7 +29,6 @@ #include "trace/sync.h" static const struct fence_ops android_fence_ops; -static const struct file_operations sync_file_fops; struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, int size, const char *name) @@ -136,181 +132,6 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size) } EXPORT_SYMBOL(sync_pt_create); -static struct sync_file *sync_file_alloc(int size, const char *name) -{ - struct sync_file *sync_file; - - sync_file = kzalloc(size, GFP_KERNEL); - if (!sync_file) - return NULL; - - sync_file->file = anon_inode_getfile("sync_file", &sync_file_fops, -sync_file, 0); - if (IS_ERR(sync_file->file)) - goto err; - - kref_init(&sync_file->kref); - strlcpy(sync_file->name, name, sizeof(sync_file->name)); - - init_waitqueue_head(&sync_file->wq); - - return sync_file; - -err: - kfree(sync_file); - return NULL; -} - -static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) -{ - struct sync_file_cb *check; - struct sync_file *sync_file; - - check = container_of(cb, struct sync_file_cb, cb); - sync_file = check->sync_file; - - if (atomic_dec_and_test(&sync_file->status)) - wake_up_all(&sync_file->wq); -} - -/** - * sync_fence_create() - creates a sync fence - * @name: name of fence to create - * @fence: fence to add to the sync_fence - * - * Creates a sync_file containg @fence. Once this is called, the sync_file - * takes ownership of @fence. - */ -struct sync_file *sync_file_create(const char *name, struct fence *fence) -{ - struct sync_file *sync_file; - - sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1]), - name); - if (!sync_file) - return NULL; - - sync_file->num_fences = 1; - atomic_set(&sync_file->status, 1); - - sync_file->cbs[0].fence = fence; - sync_file->cbs[0].sync_file = sync_file; - if (fence_add_callback(fence, &sync_file->cbs[0].cb, - fence_check_cb_func)) - atomic_dec(&sync_file->status); - - sync_file_debug_add(sync_file); - - return sync_file; -} -EXPORT_SYMBOL(sync_file_create); - -/** - * sync_file_fdget() - get a sync_file from an fd - * @fd:fd referencing a fence - * - * Ensures @fd references a valid sync_file, increments the refcount of the - * backing file. Returns the sync_file or NULL in case of error. - */ -static struct sync_file *sync_file_fdget(int fd) -{ - struct file *file = fget(fd); - - if (!file) - return NULL; - - if (file->f_op != &sync_file_fops) - goto err; - - return file->private_data; - -err: - fput(file); - return NULL; -} - -static void sync_file_add_pt(struct sync_file *sync_file, int *i, -struct fence *fence) -{ - sync_file->cbs[*i].fence =
[PATCH 09/12] dma-buf/sync_file: de-stage sync_file headers
From: Gustavo Padovan Move sync_file headers file to include/ dir. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.h | 4 ++-- drivers/staging/android/sync_debug.c | 2 +- drivers/staging/android/sync_file.c | 4 ++-- {drivers/staging/android => include/linux}/sync_file.h | 0 {drivers/staging/android/uapi => include/uapi/linux}/sync_file.h | 0 5 files changed, 5 insertions(+), 5 deletions(-) rename {drivers/staging/android => include/linux}/sync_file.h (100%) rename {drivers/staging/android/uapi => include/uapi/linux}/sync_file.h (100%) diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index df44abb..b56885c 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -20,8 +20,8 @@ #include #include -#include "sync_file.h" -#include "uapi/sync_file.h" +#include +#include struct sync_timeline; diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 6589aba..9ce3c5d 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -26,7 +26,7 @@ #include #include #include -#include "sync_file.h" +#include #include "sw_sync.h" #ifdef CONFIG_DEBUG_FS diff --git a/drivers/staging/android/sync_file.c b/drivers/staging/android/sync_file.c index 0862c7f..c4eec1c 100644 --- a/drivers/staging/android/sync_file.c +++ b/drivers/staging/android/sync_file.c @@ -23,8 +23,8 @@ #include #include #include -#include "sync_file.h" -#include "uapi/sync_file.h" +#include +#include static const struct file_operations sync_file_fops; diff --git a/drivers/staging/android/sync_file.h b/include/linux/sync_file.h similarity index 100% rename from drivers/staging/android/sync_file.h rename to include/linux/sync_file.h diff --git a/drivers/staging/android/uapi/sync_file.h b/include/uapi/linux/sync_file.h similarity index 100% rename from drivers/staging/android/uapi/sync_file.h rename to include/uapi/linux/sync_file.h -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/12] Documentation: include sync_file into DocBook
From: Gustavo Padovan Add entry in device-drivers.tmpl for sync_file documentation. Signed-off-by: Gustavo Padovan --- Documentation/DocBook/device-drivers.tmpl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl index 184f3c7..509a187 100644 --- a/Documentation/DocBook/device-drivers.tmpl +++ b/Documentation/DocBook/device-drivers.tmpl @@ -136,6 +136,8 @@ X!Edrivers/base/interface.c !Iinclude/linux/seqno-fence.h !Edrivers/dma-buf/reservation.c !Iinclude/linux/reservation.h +!Edrivers/dma-buf/sync_file.c +!Iinclude/linux/sync_file.h !Edrivers/base/dma-coherent.c !Edrivers/base/dma-mapping.c -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/12] staging/android: style fix: alignment to match the open parenthesis
From: Gustavo Padovan Fix checks reported by checkpatch.pl. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync_file.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/sync_file.c b/drivers/staging/android/sync_file.c index 5d77c08..0862c7f 100644 --- a/drivers/staging/android/sync_file.c +++ b/drivers/staging/android/sync_file.c @@ -241,7 +241,7 @@ static unsigned int sync_file_poll(struct file *file, poll_table *wait) } static long sync_file_ioctl_merge(struct sync_file *sync_file, - unsigned long arg) + unsigned long arg) { int fd = get_unused_fd_flags(O_CLOEXEC); int err; @@ -296,7 +296,7 @@ err_put_fd: } static void sync_fill_fence_info(struct fence *fence, - struct sync_fence_info *info) +struct sync_fence_info *info) { strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); @@ -310,7 +310,7 @@ static void sync_fill_fence_info(struct fence *fence, } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, - unsigned long arg) + unsigned long arg) { struct sync_file_info info; struct sync_fence_info *fence_info = NULL; @@ -369,7 +369,7 @@ out: } static long sync_file_ioctl(struct file *file, unsigned int cmd, -unsigned long arg) + unsigned long arg) { struct sync_file *sync_file = file->private_data; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 12/12] Documentation: add Sync File doc
From: Gustavo Padovan Add sync_file documentation on dma-buf-sync_file.txt --- Documentation/dma-buf-sync_file.txt | 65 + 1 file changed, 65 insertions(+) create mode 100644 Documentation/dma-buf-sync_file.txt diff --git a/Documentation/dma-buf-sync_file.txt b/Documentation/dma-buf-sync_file.txt new file mode 100644 index 000..aa7320f --- /dev/null +++ b/Documentation/dma-buf-sync_file.txt @@ -0,0 +1,65 @@ +DMA Buffer Sync File API Guide +~~ + + Gustavo Padovan + + +This document serves as a guide for device drivers writers on what is the +dma-buf sync_file API, and how drivers can support it. Sync file is the +carrier of the fences(struct fence) that needs to synchronized between drivers. + +The sync_file API is meant to be used to send and receive fence information +to/from userspace. It enables userspace to do explicit fencing, where instead +of attaching a fence to the buffer a Producer driver (such as GPU or V4L +driver) it sends the fence related to the buffer to userspace. The fence then +can be sent to the Consumer (DRM driver for example), that will not use the +buffer for anything before the fence signals, i.e., the driver that issued the +fence is not using/processing the buffer anymore, so it signals that the buffer +is ready to use. And vice-versa for the Consumer -> Producer part of the cycle. +Sync files allows userspace awareness on the DMA buffer sharing synchronization +between drivers. + +Sync file was originally added in the Android kernel but current Linux Desktop +can benefit a lot from it. + +in-fences and out-fences + + +Sync files can go either to or from userspace. When a sync_file is sent from +the driver to userspace we call the fences it contains 'out-fences'. They are +related to a buffer that the driver is processing or is going to process, so +the driver create out-fences to be able to notify, through fence_signal(), when +it has finished using (or processing) that buffer. Out-fences are fences that +the driver creates. + +On the other hand if the driver receives fence(s) through a sync_file from +userspace we call these fence(s) 'in-fences'. Receiveing in-fences means that +we need to wait for the fence(s) to signal before using any buffer related to +the in-fences. + +Creating Sync Files +--- + +When a driver needs to send an out-fence userspace it creates a sync_file. + +Interface: + struct sync_file *sync_file_create(const char *name, struct fence *fence); + +The caller pass the name and the out-fence and gets back the sync_file. That is +just the first step, next it needs to install an fd on sync_file->file. So it +gets an fd: + + fd = get_unused_fd_flags(O_CLOEXEC); + +and installs it on sync_file->file: + + fd_install(fd, sync_file->file); + +The sync_file fd now can be sent to userspace. + +If the creation process fail, or the sync_file needs to be released by any +other reason fput(sync_file->file) should be used. + +References: +[1] struct sync_file in include/linux/sync_file.h +[2] All interfaces mentioned above defined in include/linux/sync_file.h -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/12] staging/android: make sync_file_fdget() static
From: Gustavo Padovan There is no plan in the near future to use this function outside of this file so keep it as static. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 3 +-- drivers/staging/android/sync.h | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index c441fde..1239684 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -212,7 +212,7 @@ EXPORT_SYMBOL(sync_file_create); * Ensures @fd references a valid sync_file, increments the refcount of the * backing file. Returns the sync_file or NULL in case of error. */ -struct sync_file *sync_file_fdget(int fd) +static struct sync_file *sync_file_fdget(int fd) { struct file *file = fget(fd); @@ -228,7 +228,6 @@ err: fput(file); return NULL; } -EXPORT_SYMBOL(sync_file_fdget); static void sync_file_add_pt(struct sync_file *sync_file, int *i, struct fence *fence) diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index ffc6df6..1f164df 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -168,7 +168,6 @@ void sync_timeline_signal(struct sync_timeline *obj); struct fence *sync_pt_create(struct sync_timeline *parent, int size); struct sync_file *sync_file_create(const char *name, struct fence *fence); -struct sync_file *sync_file_fdget(int fd); #ifdef CONFIG_DEBUG_FS -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/12] dma-buf/sync_file: de-stage sync_file
From: Gustavo Padovan sync_file is useful to connect one or more fences to the file. The file is used by userspace to track fences between drivers that share DMA bufs. Signed-off-by: Gustavo Padovan --- drivers/Kconfig | 2 ++ drivers/dma-buf/Kconfig | 11 +++ drivers/dma-buf/Makefile | 1 + drivers/{staging/android => dma-buf}/sync_file.c | 0 drivers/staging/android/Kconfig | 1 + drivers/staging/android/Makefile | 2 +- 6 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/Kconfig rename drivers/{staging/android => dma-buf}/sync_file.c (100%) diff --git a/drivers/Kconfig b/drivers/Kconfig index d2ac339..430f761 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -114,6 +114,8 @@ source "drivers/rtc/Kconfig" source "drivers/dma/Kconfig" +source "drivers/dma-buf/Kconfig" + source "drivers/dca/Kconfig" source "drivers/auxdisplay/Kconfig" diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig new file mode 100644 index 000..9824bc4 --- /dev/null +++ b/drivers/dma-buf/Kconfig @@ -0,0 +1,11 @@ +menu "DMABUF options" + +config SYNC_FILE + bool "sync_file support for fences" + default n + select ANON_INODES + select DMA_SHARED_BUFFER + ---help--- + This option enables the fence framework synchronization to export + sync_files to userspace that can represent one or more fences. +endmenu diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 57a675f..4a424ec 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1 +1,2 @@ obj-y := dma-buf.o fence.o reservation.o seqno-fence.o +obj-$(CONFIG_SYNC_FILE)+= sync_file.o diff --git a/drivers/staging/android/sync_file.c b/drivers/dma-buf/sync_file.c similarity index 100% rename from drivers/staging/android/sync_file.c rename to drivers/dma-buf/sync_file.c diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig index 4244821..7a3a77e 100644 --- a/drivers/staging/android/Kconfig +++ b/drivers/staging/android/Kconfig @@ -38,6 +38,7 @@ config SW_SYNC bool "Software synchronization objects" default n depends on SYNC + depends on SYNC_FILE ---help--- A sync object driver that uses a 32bit counter to coordinate synchronization. Useful when there is no hardware primitive backing diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index ebc2df1..980d6dc 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -4,5 +4,5 @@ obj-y += ion/ obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)+= lowmemorykiller.o -obj-$(CONFIG_SYNC) += sync_file.o sync.o sync_debug.o +obj-$(CONFIG_SYNC) += sync.o sync_debug.o obj-$(CONFIG_SW_SYNC) += sw_sync.o -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/12] staging/android: improve documentation for sync_file
From: Gustavo Padovan num_fences was missing a colon mark and sync_file_create() now have better description. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync_file.c | 3 ++- drivers/staging/android/sync_file.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/sync_file.c b/drivers/staging/android/sync_file.c index 3902538..5d77c08 100644 --- a/drivers/staging/android/sync_file.c +++ b/drivers/staging/android/sync_file.c @@ -71,7 +71,8 @@ static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) * @fence: fence to add to the sync_fence * * Creates a sync_file containg @fence. Once this is called, the sync_file - * takes ownership of @fence. + * takes ownership of @fence. The sync_file can be released with + * fput(sync_file->file). Returns the sync_file or NULL in case of error. */ struct sync_file *sync_file_create(const char *name, struct fence *fence) { diff --git a/drivers/staging/android/sync_file.h b/drivers/staging/android/sync_file.h index a53259c..003db09 100644 --- a/drivers/staging/android/sync_file.h +++ b/drivers/staging/android/sync_file.h @@ -32,7 +32,7 @@ struct sync_file_cb { * @kref: reference count on fence. * @name: name of sync_file. Useful for debugging * @sync_file_list:membership in global file list - * @num_fences number of sync_pts in the fence + * @num_fences:number of sync_pts in the fence * @wq:wait queue for fence signaling * @status:0: signaled, >0:active, <0: error * @cbs: sync_pts callback information -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/12] De-stage Sync File Framework
2016-04-27 Daniel Vetter : > On Wed, Apr 27, 2016 at 01:27:07PM -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Hi, > > > > This patchset sits on top of Sync ABI Rework v13: > > > > https://www.spinics.net/lists/dri-devel/msg105667.html > > > > The first eight clean up and prepare sync_file for de-staging. The last four > > patches do the de-staging, moving files to drivers/dma-buf/ and > > include/linux/ > > plus adding Documentation. > > > > As the de-stage depends upon many changes on the staging tree it would > > be good to get all the patches merged through the staging tree if Sumit > > agrees with that. > > > > The next step on the Sync de-stage is clean up the remaining bits > > of the Sync Framework, mainly SW_SYNC, which is only used for testing. > > Ok I looked once more at all this stuff, and there's some nitpicks I > discussed with Gustavo on irc. But really their small, and I think > perfectly ok to address them once sync_file is destaged. Especially since > there's a lot more work pending on top of this, so we really want to get > sync_file.[hc] destaged in 4.7. It'll take us a few iterations and a few > drivers using this in anger in upstream to perfect the internal interfaces > anyway, but let's get this started. > > Had some real nitpicks on the docs patch, but that can also be merged > later on imo. Except for that patch, on the series: > > Reviewed-by: Daniel Vetter > Thanks for the review, I'm sending v2 fixing the nitpicks you pointed out. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 04/13] staging/android: make sync_file_merge() static
From: Gustavo Padovan There is no plan in the near future to use this function outside of this file so keep it as static. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/sync.c | 5 ++--- drivers/staging/android/sync.h | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index a89ded0..e9bf251 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -253,8 +253,8 @@ static void sync_file_add_pt(struct sync_file *sync_file, int *i, * @a and @b. @a and @b remain valid, independent sync_file. Returns the * new merged sync_file or NULL in case of error. */ -struct sync_file *sync_file_merge(const char *name, - struct sync_file *a, struct sync_file *b) +static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, +struct sync_file *b) { int num_fences = a->num_fences + b->num_fences; struct sync_file *sync_file; @@ -310,7 +310,6 @@ struct sync_file *sync_file_merge(const char *name, sync_file_debug_add(sync_file); return sync_file; } -EXPORT_SYMBOL(sync_file_merge); static const char *android_fence_get_driver_name(struct fence *fence) { diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 925fba5..ffc6df6 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -168,8 +168,6 @@ void sync_timeline_signal(struct sync_timeline *obj); struct fence *sync_pt_create(struct sync_timeline *parent, int size); struct sync_file *sync_file_create(const char *name, struct fence *fence); -struct sync_file *sync_file_merge(const char *name, - struct sync_file *a, struct sync_file *b); struct sync_file *sync_file_fdget(int fd); #ifdef CONFIG_DEBUG_FS -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 03/13] staging/android: move sync_file functions comments to sync.c
From: Gustavo Padovan To keep comments in line with drivers/dma-buf/ move all sync_file comments to sync.c. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/sync.c | 26 +- drivers/staging/android/sync.h | 31 --- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index b965e2a..a89ded0 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -173,7 +173,14 @@ static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) wake_up_all(&sync_file->wq); } -/* TODO: implement a create which takes more that one fence */ +/** + * sync_fence_create() - creates a sync fence + * @name: name of fence to create + * @fence: fence to add to the sync_fence + * + * Creates a sync_file containg @fence. Once this is called, the sync_file + * takes ownership of @fence. + */ struct sync_file *sync_file_create(const char *name, struct fence *fence) { struct sync_file *sync_file; @@ -198,6 +205,13 @@ struct sync_file *sync_file_create(const char *name, struct fence *fence) } EXPORT_SYMBOL(sync_file_create); +/** + * sync_file_fdget() - get a sync_file from an fd + * @fd:fd referencing a fence + * + * Ensures @fd references a valid sync_file, increments the refcount of the + * backing file. Returns the sync_file or NULL in case of error. + */ struct sync_file *sync_file_fdget(int fd) { struct file *file = fget(fd); @@ -229,6 +243,16 @@ static void sync_file_add_pt(struct sync_file *sync_file, int *i, } } +/** + * sync_file_merge() - merge two sync_files + * @name: name of new fence + * @a: sync_file a + * @b: sync_file b + * + * Creates a new sync_file which contains copies of all the fences in both + * @a and @b. @a and @b remain valid, independent sync_file. Returns the + * new merged sync_file or NULL in case of error. + */ struct sync_file *sync_file_merge(const char *name, struct sync_file *a, struct sync_file *b) { diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index c45cc7b..925fba5 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -167,40 +167,9 @@ void sync_timeline_signal(struct sync_timeline *obj); */ struct fence *sync_pt_create(struct sync_timeline *parent, int size); -/** - * sync_fence_create() - creates a sync fence - * @name: name of fence to create - * @fence: fence to add to the sync_fence - * - * Creates a sync_file containg @fence. Once this is called, the sync_file - * takes ownership of @fence. - */ struct sync_file *sync_file_create(const char *name, struct fence *fence); - -/* - * API for sync_file consumers - */ - -/** - * sync_file_merge() - merge two sync_files - * @name: name of new fence - * @a: sync_file a - * @b: sync_file b - * - * Creates a new sync_file which contains copies of all the fences in both - * @a and @b. @a and @b remain valid, independent sync_file. Returns the - * new merged sync_file or NULL in case of error. - */ struct sync_file *sync_file_merge(const char *name, struct sync_file *a, struct sync_file *b); - -/** - * sync_file_fdget() - get a sync_file from an fd - * @fd:fd referencing a fence - * - * Ensures @fd references a valid sync_file, increments the refcount of the - * backing file. Returns the sync_file or NULL in case of error. - */ struct sync_file *sync_file_fdget(int fd); #ifdef CONFIG_DEBUG_FS -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 01/13] staging/android: remove redundant comments on sync_merge_data
From: Gustavo Padovan struct sync_merge_data already have documentation on top of the struct definition. No need to duplicate it. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Reviewed-by: Daniel Vetter --- drivers/staging/android/uapi/sync.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index 7de5d6a..413303d 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -23,9 +23,9 @@ * @pad: padding for 64-bit alignment, should always be zero */ struct sync_merge_data { - charname[32]; /* name of new fence */ - __s32 fd2; /* fd of second fence */ - __s32 fence; /* fd on newly created fence */ + charname[32]; + __s32 fd2; + __s32 fence; __u32 flags; __u32 pad; }; @@ -33,8 +33,8 @@ struct sync_merge_data { /** * struct sync_fence_info - detailed fence information * @obj_name: name of parent sync_timeline - * @driver_name: name of driver implementing the parent - * @status:status of the fence 0:active 1:signaled <0:error +* @driver_name:name of driver implementing the parent +* @status: status of the fence 0:active 1:signaled <0:error * @flags: fence_info flags * @timestamp_ns: timestamp of status change in nanoseconds */ -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 06/13] staging/android: remove name arg from sync_file_create()
From: Gustavo Padovan Simplifies the API to only receive the fence it needs to add to the sync and create a name for the sync_file based on the fence context and seqno. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/sync.c | 16 +--- drivers/staging/android/sync.h | 2 +- drivers/staging/android/sync_debug.c | 3 +-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 7e0fa20..5470ae9 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -136,7 +136,7 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size) } EXPORT_SYMBOL(sync_pt_create); -static struct sync_file *sync_file_alloc(int size, const char *name) +static struct sync_file *sync_file_alloc(int size) { struct sync_file *sync_file; @@ -150,7 +150,6 @@ static struct sync_file *sync_file_alloc(int size, const char *name) goto err; kref_init(&sync_file->kref); - strlcpy(sync_file->name, name, sizeof(sync_file->name)); init_waitqueue_head(&sync_file->wq); @@ -175,23 +174,25 @@ static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) /** * sync_fence_create() - creates a sync fence - * @name: name of fence to create * @fence: fence to add to the sync_fence * * Creates a sync_file containg @fence. Once this is called, the sync_file * takes ownership of @fence. */ -struct sync_file *sync_file_create(const char *name, struct fence *fence) +struct sync_file *sync_file_create(struct fence *fence) { struct sync_file *sync_file; - sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1]), - name); + sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1])); if (!sync_file) return NULL; sync_file->num_fences = 1; atomic_set(&sync_file->status, 1); + snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%d-%d", +fence->ops->get_driver_name(fence), +fence->ops->get_timeline_name(fence), fence->context, +fence->seqno); sync_file->cbs[0].fence = fence; sync_file->cbs[0].sync_file = sync_file; @@ -260,7 +261,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, int i, i_a, i_b; unsigned long size = offsetof(struct sync_file, cbs[num_fences]); - sync_file = sync_file_alloc(size, name); + sync_file = sync_file_alloc(size); if (!sync_file) return NULL; @@ -306,6 +307,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, atomic_sub(num_fences - i, &sync_file->status); sync_file->num_fences = i; + strlcpy(sync_file->name, name, sizeof(sync_file->name)); sync_file_debug_add(sync_file); return sync_file; } diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 1f164df..7dee444 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -167,7 +167,7 @@ void sync_timeline_signal(struct sync_timeline *obj); */ struct fence *sync_pt_create(struct sync_timeline *parent, int size); -struct sync_file *sync_file_create(const char *name, struct fence *fence); +struct sync_file *sync_file_create(struct fence *fence); #ifdef CONFIG_DEBUG_FS diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index e4b0e41..2cab40d 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -262,8 +262,7 @@ static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj, goto err; } - data.name[sizeof(data.name) - 1] = '\0'; - sync_file = sync_file_create(data.name, fence); + sync_file = sync_file_create(fence); if (!sync_file) { fence_put(fence); err = -ENOMEM; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 02/13] staging/android: drop sync_file_install() and sync_file_put()
From: Gustavo Padovan These two functions are just wrappers for one line functions, they call fd_install() and fput() respectively, so just get rid of them and use fd_install() and fput() directly for more simplicity. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/sync.c | 20 drivers/staging/android/sync.h | 19 --- drivers/staging/android/sync_debug.c | 4 ++-- 3 files changed, 6 insertions(+), 37 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index f9c6094..b965e2a 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -216,18 +216,6 @@ err: } EXPORT_SYMBOL(sync_file_fdget); -void sync_file_put(struct sync_file *sync_file) -{ - fput(sync_file->file); -} -EXPORT_SYMBOL(sync_file_put); - -void sync_file_install(struct sync_file *sync_file, int fd) -{ - fd_install(fd, sync_file->file); -} -EXPORT_SYMBOL(sync_file_install); - static void sync_file_add_pt(struct sync_file *sync_file, int *i, struct fence *fence) { @@ -469,15 +457,15 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fence3; } - sync_file_install(fence3, fd); - sync_file_put(fence2); + fd_install(fd, fence3->file); + fput(fence2->file); return 0; err_put_fence3: - sync_file_put(fence3); + fput(fence3->file); err_put_fence2: - sync_file_put(fence2); + fput(fence2->file); err_put_fd: put_unused_fd(fd); diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index d2a1734..c45cc7b 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -203,25 +203,6 @@ struct sync_file *sync_file_merge(const char *name, */ struct sync_file *sync_file_fdget(int fd); -/** - * sync_file_put() - puts a reference of a sync_file - * @sync_file: sync_file to put - * - * Puts a reference on @sync_fence. If this is the last reference, the - * sync_fil and all it's sync_pts will be freed - */ -void sync_file_put(struct sync_file *sync_file); - -/** - * sync_file_install() - installs a sync_file into a file descriptor - * @sync_file: sync_file to install - * @fd:file descriptor in which to install the fence - * - * Installs @sync_file into @fd. @fd's should be acquired through - * get_unused_fd_flags(O_CLOEXEC). - */ -void sync_file_install(struct sync_file *sync_file, int fd); - #ifdef CONFIG_DEBUG_FS void sync_timeline_debug_add(struct sync_timeline *obj); diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 5a7ec58..e4b0e41 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -272,12 +272,12 @@ static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj, data.fence = fd; if (copy_to_user((void __user *)arg, &data, sizeof(data))) { - sync_file_put(sync_file); + fput(sync_file->file); err = -EFAULT; goto err; } - sync_file_install(sync_file, fd); + fd_install(fd, sync_file->file); return 0; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 00/13] De-stage Sync File Framework
From: Gustavo Padovan Hi, This patchset sits on top of Sync ABI Rework v13: https://www.spinics.net/lists/dri-devel/msg105667.html The first eight clean up and prepare sync_file for de-staging. The last four patches do the de-staging, moving files to drivers/dma-buf/ and include/linux/ plus adding Documentation. As the de-stage depends upon many changes on the staging tree it would be good to get all the patches merged through the staging tree if Sumit agrees with that. The next step on the Sync de-stage is clean up the remaining bits of the Sync Framework, mainly SW_SYNC, which is only used for testing. v2: - Add Reviewed-by: tag from Daniel Vetter to all patches. - Take in sugestions for the Sync File Documentation (Daniel) - Remove name arg from sync_file_crate() (Daniel) - Revome leftover EXPORT_SYMBOL(sync_file_merge) (Daniel) Thanks, Gustavo Gustavo Padovan (13): staging/android: remove redundant comments on sync_merge_data staging/android: drop sync_file_install() and sync_file_put() staging/android: move sync_file functions comments to sync.c staging/android: make sync_file_merge() static staging/android: make sync_file_fdget() static staging/android: remove name arg from sync_file_create() staging/android: prepare sync_file for de-staging staging/android: improve documentation for sync_file staging/android: style fix: alignment to match the open parenthesis dma-buf/sync_file: de-stage sync_file headers dma-buf/sync_file: de-stage sync_file Documentation: include sync_file into DocBook Documentation: add Sync File doc Documentation/DocBook/device-drivers.tmpl | 2 + Documentation/sync_file.txt | 69 ++ drivers/Kconfig | 2 + drivers/dma-buf/Kconfig | 11 + drivers/dma-buf/Makefile | 1 + drivers/dma-buf/sync_file.c | 395 ++ drivers/staging/android/Kconfig | 1 + drivers/staging/android/sync.c| 362 --- drivers/staging/android/sync.h| 91 +-- drivers/staging/android/sync_debug.c | 8 +- drivers/staging/android/uapi/sync.h | 100 include/linux/sync_file.h | 57 + include/uapi/linux/sync_file.h| 100 13 files changed, 644 insertions(+), 555 deletions(-) create mode 100644 Documentation/sync_file.txt create mode 100644 drivers/dma-buf/Kconfig create mode 100644 drivers/dma-buf/sync_file.c delete mode 100644 drivers/staging/android/uapi/sync.h create mode 100644 include/linux/sync_file.h create mode 100644 include/uapi/linux/sync_file.h -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 07/13] staging/android: prepare sync_file for de-staging
From: Gustavo Padovan Move its functions and structs to their own file. Also moves function's docs to the .c file. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/Makefile | 2 +- drivers/staging/android/sync.c | 374 --- drivers/staging/android/sync.h | 38 +- drivers/staging/android/sync_debug.c | 1 + drivers/staging/android/sync_file.c| 394 + drivers/staging/android/sync_file.h| 57 +++ .../staging/android/uapi/{sync.h => sync_file.h} | 0 7 files changed, 455 insertions(+), 411 deletions(-) create mode 100644 drivers/staging/android/sync_file.c create mode 100644 drivers/staging/android/sync_file.h rename drivers/staging/android/uapi/{sync.h => sync_file.h} (100%) diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index 980d6dc..ebc2df1 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -4,5 +4,5 @@ obj-y += ion/ obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)+= lowmemorykiller.o -obj-$(CONFIG_SYNC) += sync.o sync_debug.o +obj-$(CONFIG_SYNC) += sync_file.o sync.o sync_debug.o obj-$(CONFIG_SW_SYNC) += sw_sync.o diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 5470ae9..1d14c83 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -16,10 +16,7 @@ #include #include -#include -#include #include -#include #include #include #include @@ -32,7 +29,6 @@ #include "trace/sync.h" static const struct fence_ops android_fence_ops; -static const struct file_operations sync_file_fops; struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, int size, const char *name) @@ -136,182 +132,6 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size) } EXPORT_SYMBOL(sync_pt_create); -static struct sync_file *sync_file_alloc(int size) -{ - struct sync_file *sync_file; - - sync_file = kzalloc(size, GFP_KERNEL); - if (!sync_file) - return NULL; - - sync_file->file = anon_inode_getfile("sync_file", &sync_file_fops, -sync_file, 0); - if (IS_ERR(sync_file->file)) - goto err; - - kref_init(&sync_file->kref); - - init_waitqueue_head(&sync_file->wq); - - return sync_file; - -err: - kfree(sync_file); - return NULL; -} - -static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) -{ - struct sync_file_cb *check; - struct sync_file *sync_file; - - check = container_of(cb, struct sync_file_cb, cb); - sync_file = check->sync_file; - - if (atomic_dec_and_test(&sync_file->status)) - wake_up_all(&sync_file->wq); -} - -/** - * sync_fence_create() - creates a sync fence - * @fence: fence to add to the sync_fence - * - * Creates a sync_file containg @fence. Once this is called, the sync_file - * takes ownership of @fence. - */ -struct sync_file *sync_file_create(struct fence *fence) -{ - struct sync_file *sync_file; - - sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1])); - if (!sync_file) - return NULL; - - sync_file->num_fences = 1; - atomic_set(&sync_file->status, 1); - snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%d-%d", -fence->ops->get_driver_name(fence), -fence->ops->get_timeline_name(fence), fence->context, -fence->seqno); - - sync_file->cbs[0].fence = fence; - sync_file->cbs[0].sync_file = sync_file; - if (fence_add_callback(fence, &sync_file->cbs[0].cb, - fence_check_cb_func)) - atomic_dec(&sync_file->status); - - sync_file_debug_add(sync_file); - - return sync_file; -} -EXPORT_SYMBOL(sync_file_create); - -/** - * sync_file_fdget() - get a sync_file from an fd - * @fd:fd referencing a fence - * - * Ensures @fd references a valid sync_file, increments the refcount of the - * backing file. Returns the sync_file or NULL in case of error. - */ -static struct sync_file *sync_file_fdget(int fd) -{ - struct file *file = fget(fd); - - if (!file) - return NULL; - - if (file->f_op != &sync_file_fops) - goto err; - - return file->private_data; - -err: - fput(file); - return NULL; -} - -static void sync_file_add_pt(struct sync_file *sync_file, int *i, -
[PATCH v2 05/13] staging/android: make sync_file_fdget() static
From: Gustavo Padovan There is no plan in the near future to use this function outside of this file so keep it as static. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/sync.c | 3 +-- drivers/staging/android/sync.h | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index e9bf251..7e0fa20 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -212,7 +212,7 @@ EXPORT_SYMBOL(sync_file_create); * Ensures @fd references a valid sync_file, increments the refcount of the * backing file. Returns the sync_file or NULL in case of error. */ -struct sync_file *sync_file_fdget(int fd) +static struct sync_file *sync_file_fdget(int fd) { struct file *file = fget(fd); @@ -228,7 +228,6 @@ err: fput(file); return NULL; } -EXPORT_SYMBOL(sync_file_fdget); static void sync_file_add_pt(struct sync_file *sync_file, int *i, struct fence *fence) diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index ffc6df6..1f164df 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -168,7 +168,6 @@ void sync_timeline_signal(struct sync_timeline *obj); struct fence *sync_pt_create(struct sync_timeline *parent, int size); struct sync_file *sync_file_create(const char *name, struct fence *fence); -struct sync_file *sync_file_fdget(int fd); #ifdef CONFIG_DEBUG_FS -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 11/13] dma-buf/sync_file: de-stage sync_file
From: Gustavo Padovan sync_file is useful to connect one or more fences to the file. The file is used by userspace to track fences between drivers that share DMA bufs. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/Kconfig | 2 ++ drivers/dma-buf/Kconfig | 11 +++ drivers/dma-buf/Makefile | 1 + drivers/{staging/android => dma-buf}/sync_file.c | 0 drivers/staging/android/Kconfig | 1 + drivers/staging/android/Makefile | 2 +- 6 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/Kconfig rename drivers/{staging/android => dma-buf}/sync_file.c (100%) diff --git a/drivers/Kconfig b/drivers/Kconfig index d2ac339..430f761 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -114,6 +114,8 @@ source "drivers/rtc/Kconfig" source "drivers/dma/Kconfig" +source "drivers/dma-buf/Kconfig" + source "drivers/dca/Kconfig" source "drivers/auxdisplay/Kconfig" diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig new file mode 100644 index 000..9824bc4 --- /dev/null +++ b/drivers/dma-buf/Kconfig @@ -0,0 +1,11 @@ +menu "DMABUF options" + +config SYNC_FILE + bool "sync_file support for fences" + default n + select ANON_INODES + select DMA_SHARED_BUFFER + ---help--- + This option enables the fence framework synchronization to export + sync_files to userspace that can represent one or more fences. +endmenu diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 57a675f..4a424ec 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1 +1,2 @@ obj-y := dma-buf.o fence.o reservation.o seqno-fence.o +obj-$(CONFIG_SYNC_FILE)+= sync_file.o diff --git a/drivers/staging/android/sync_file.c b/drivers/dma-buf/sync_file.c similarity index 100% rename from drivers/staging/android/sync_file.c rename to drivers/dma-buf/sync_file.c diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig index 4244821..7a3a77e 100644 --- a/drivers/staging/android/Kconfig +++ b/drivers/staging/android/Kconfig @@ -38,6 +38,7 @@ config SW_SYNC bool "Software synchronization objects" default n depends on SYNC + depends on SYNC_FILE ---help--- A sync object driver that uses a 32bit counter to coordinate synchronization. Useful when there is no hardware primitive backing diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index ebc2df1..980d6dc 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -4,5 +4,5 @@ obj-y += ion/ obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)+= lowmemorykiller.o -obj-$(CONFIG_SYNC) += sync_file.o sync.o sync_debug.o +obj-$(CONFIG_SYNC) += sync.o sync_debug.o obj-$(CONFIG_SW_SYNC) += sw_sync.o -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 13/13] Documentation: add Sync File doc
From: Gustavo Padovan Add sync_file documentation on dma-buf-sync_file.txt Reviewed-by: Daniel Vetter --- Documentation/sync_file.txt | 69 + 1 file changed, 69 insertions(+) create mode 100644 Documentation/sync_file.txt diff --git a/Documentation/sync_file.txt b/Documentation/sync_file.txt new file mode 100644 index 000..eaf8297 --- /dev/null +++ b/Documentation/sync_file.txt @@ -0,0 +1,69 @@ + Sync File API Guide + ~~~ + + Gustavo Padovan + + +This document serves as a guide for device drivers writers on what the +sync_file API is, and how drivers can support it. Sync file is the carrier of +the fences(struct fence) that needs to synchronized between drivers or across +process boundaries. + +The sync_file API is meant to be used to send and receive fence information +to/from userspace. It enables userspace to do explicit fencing, where instead +of attaching a fence to the buffer a producer driver (such as a GPU or V4L +driver) sends the fence related to the buffer to userspace via a sync_file. + +The sync_file then can be sent to the consumer (DRM driver for example), that +will not use the buffer for anything before the fence(s) signals, i.e., the +driver that issued the fence is not using/processing the buffer anymore, so it +signals that the buffer is ready to use. And vice-versa for the consumer -> +producer part of the cycle. + +Sync files allows userspace awareness on buffer sharing synchronization between +drivers. + +Sync file was originally added in the Android kernel but current Linux Desktop +can benefit a lot from it. + +in-fences and out-fences + + +Sync files can go either to or from userspace. When a sync_file is sent from +the driver to userspace we call the fences it contains 'out-fences'. They are +related to a buffer that the driver is processing or is going to process, so +the driver an create out-fence to be able to notify, through fence_signal(), +when it has finished using (or processing) that buffer. Out-fences are fences +that the driver creates. + +On the other hand if the driver receives fence(s) through a sync_file from +userspace we call these fence(s) 'in-fences'. Receiveing in-fences means that +we need to wait for the fence(s) to signal before using any buffer related to +the in-fences. + +Creating Sync Files +--- + +When a driver needs to send an out-fence userspace it creates a sync_file. + +Interface: + struct sync_file *sync_file_create(struct fence *fence); + +The caller pass the out-fence and gets back the sync_file. That is just the +first step, next it needs to install an fd on sync_file->file. So it gets an +fd: + + fd = get_unused_fd_flags(O_CLOEXEC); + +and installs it on sync_file->file: + + fd_install(fd, sync_file->file); + +The sync_file fd now can be sent to userspace. + +If the creation process fail, or the sync_file needs to be released by any +other reason fput(sync_file->file) should be used. + +References: +[1] struct sync_file in include/linux/sync_file.h +[2] All interfaces mentioned above defined in include/linux/sync_file.h -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 10/13] dma-buf/sync_file: de-stage sync_file headers
From: Gustavo Padovan Move sync_file headers file to include/ dir. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/sync.h | 4 ++-- drivers/staging/android/sync_debug.c | 2 +- drivers/staging/android/sync_file.c | 4 ++-- {drivers/staging/android => include/linux}/sync_file.h | 0 {drivers/staging/android/uapi => include/uapi/linux}/sync_file.h | 0 5 files changed, 5 insertions(+), 5 deletions(-) rename {drivers/staging/android => include/linux}/sync_file.h (100%) rename {drivers/staging/android/uapi => include/uapi/linux}/sync_file.h (100%) diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index df44abb..b56885c 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -20,8 +20,8 @@ #include #include -#include "sync_file.h" -#include "uapi/sync_file.h" +#include +#include struct sync_timeline; diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 8b55218..5f57499 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -26,7 +26,7 @@ #include #include #include -#include "sync_file.h" +#include #include "sw_sync.h" #ifdef CONFIG_DEBUG_FS diff --git a/drivers/staging/android/sync_file.c b/drivers/staging/android/sync_file.c index eabf90d..f08cf2d 100644 --- a/drivers/staging/android/sync_file.c +++ b/drivers/staging/android/sync_file.c @@ -23,8 +23,8 @@ #include #include #include -#include "sync_file.h" -#include "uapi/sync_file.h" +#include +#include static const struct file_operations sync_file_fops; diff --git a/drivers/staging/android/sync_file.h b/include/linux/sync_file.h similarity index 100% rename from drivers/staging/android/sync_file.h rename to include/linux/sync_file.h diff --git a/drivers/staging/android/uapi/sync_file.h b/include/uapi/linux/sync_file.h similarity index 100% rename from drivers/staging/android/uapi/sync_file.h rename to include/uapi/linux/sync_file.h -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 09/13] staging/android: style fix: alignment to match the open parenthesis
From: Gustavo Padovan Fix checks reported by checkpatch.pl. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/sync_file.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/sync_file.c b/drivers/staging/android/sync_file.c index d9da3a4..eabf90d 100644 --- a/drivers/staging/android/sync_file.c +++ b/drivers/staging/android/sync_file.c @@ -242,7 +242,7 @@ static unsigned int sync_file_poll(struct file *file, poll_table *wait) } static long sync_file_ioctl_merge(struct sync_file *sync_file, - unsigned long arg) + unsigned long arg) { int fd = get_unused_fd_flags(O_CLOEXEC); int err; @@ -297,7 +297,7 @@ err_put_fd: } static void sync_fill_fence_info(struct fence *fence, - struct sync_fence_info *info) +struct sync_fence_info *info) { strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); @@ -311,7 +311,7 @@ static void sync_fill_fence_info(struct fence *fence, } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, - unsigned long arg) + unsigned long arg) { struct sync_file_info info; struct sync_fence_info *fence_info = NULL; @@ -370,7 +370,7 @@ out: } static long sync_file_ioctl(struct file *file, unsigned int cmd, -unsigned long arg) + unsigned long arg) { struct sync_file *sync_file = file->private_data; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 08/13] staging/android: improve documentation for sync_file
From: Gustavo Padovan num_fences was missing a colon mark and sync_file_create() now have better description. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/staging/android/sync_file.c | 5 +++-- drivers/staging/android/sync_file.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/sync_file.c b/drivers/staging/android/sync_file.c index 2c724ec..d9da3a4 100644 --- a/drivers/staging/android/sync_file.c +++ b/drivers/staging/android/sync_file.c @@ -65,11 +65,12 @@ static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) } /** - * sync_fence_create() - creates a sync fence + * sync_file_create() - creates a sync file * @fence: fence to add to the sync_fence * * Creates a sync_file containg @fence. Once this is called, the sync_file - * takes ownership of @fence. + * takes ownership of @fence. The sync_file can be released with + * fput(sync_file->file). Returns the sync_file or NULL in case of error. */ struct sync_file *sync_file_create(struct fence *fence) { diff --git a/drivers/staging/android/sync_file.h b/drivers/staging/android/sync_file.h index 8a1b546..c6ffe8b 100644 --- a/drivers/staging/android/sync_file.h +++ b/drivers/staging/android/sync_file.h @@ -32,7 +32,7 @@ struct sync_file_cb { * @kref: reference count on fence. * @name: name of sync_file. Useful for debugging * @sync_file_list:membership in global file list - * @num_fences number of sync_pts in the fence + * @num_fences:number of sync_pts in the fence * @wq:wait queue for fence signaling * @status:0: signaled, >0:active, <0: error * @cbs: sync_pts callback information -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 12/13] Documentation: include sync_file into DocBook
From: Gustavo Padovan Add entry in device-drivers.tmpl for sync_file documentation. Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- Documentation/DocBook/device-drivers.tmpl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl index 184f3c7..509a187 100644 --- a/Documentation/DocBook/device-drivers.tmpl +++ b/Documentation/DocBook/device-drivers.tmpl @@ -136,6 +136,8 @@ X!Edrivers/base/interface.c !Iinclude/linux/seqno-fence.h !Edrivers/dma-buf/reservation.c !Iinclude/linux/reservation.h +!Edrivers/dma-buf/sync_file.c +!Iinclude/linux/sync_file.h !Edrivers/base/dma-coherent.c !Edrivers/base/dma-mapping.c -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v2 1/8] dma-buf/fence: add fence_collection fences
2016-04-26 Chris Wilson : > On Mon, Apr 25, 2016 at 07:33:21PM -0300, Gustavo Padovan wrote: > > +static const char *fence_collection_get_timeline_name(struct fence *fence) > > +{ > > + return "no context"; > > "unbound" to distinguish from fence contexts within a timeline? > > > +static bool fence_collection_enable_signaling(struct fence *fence) > > +{ > > + struct fence_collection *collection = to_fence_collection(fence); > > + int i; > > + > > + for (i = 0 ; i < collection->num_fences ; i++) { > > + if (fence_add_callback(collection->fences[i].fence, > > + &collection->fences[i].cb, > > + collection_check_cb_func)) { > > + atomic_dec(&collection->num_pending_fences); > > + return false; > > Don't stop, we need to enable all the others! > > > + } > > + } > > + > > + return !!atomic_read(&collection->num_pending_fences); > > Redundant !! > > > +} > > + > > +static bool fence_collection_signaled(struct fence *fence) > > +{ > > + struct fence_collection *collection = to_fence_collection(fence); > > + > > + return (atomic_read(&collection->num_pending_fences) == 0); > > Redundant () > > > +static signed long fence_collection_wait(struct fence *fence, bool intr, > > +signed long timeout) > > +{ > > What advantage does this have over fence_default_wait? You enable > signaling on all, then wait sequentially. The code looks redundant and > could just use fence_default_wait instead. None actually, I'll just replace it with fence_default_wait(). Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v2 8/8] drm/fence: add out-fences support
2016-04-26 Daniel Vetter : > On Mon, Apr 25, 2016 at 07:33:28PM -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Support DRM out-fences creating a sync_file with a fence for each crtc > > update with the DRM_MODE_ATOMIC_OUT_FENCE flag. > > > > We then send an struct drm_out_fences array with the out-fences fds back in > > the drm_atomic_ioctl() as an out arg in the out_fences_ptr field. > > > > struct drm_out_fences { > > __u32 crtc_id; > > __u32 fd; > > }; > > > > v2: Comment by Rob Clark: > > - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here. > > > > Comment by Daniel Vetter: > > - Add clean up code for out_fences > > > > Signed-off-by: Gustavo Padovan > > --- > > drivers/gpu/drm/drm_atomic.c | 163 > > +-- > > include/drm/drm_crtc.h | 10 +++ > > include/uapi/drm/drm_mode.h | 11 ++- > > 3 files changed, 179 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index 5f9d434..06c6007 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -1566,6 +1566,133 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, > > } > > EXPORT_SYMBOL(drm_atomic_clean_old_fb); > > > > +static struct drm_out_fence_state *get_out_fence(struct drm_device *dev, > > +struct drm_atomic_state *state, > > +uint32_t __user > > *out_fences_ptr, > > +uint64_t count_out_fences, > > +uint64_t user_data) > > +{ > > + struct drm_crtc *crtc; > > + struct drm_crtc_state *crtc_state; > > + struct drm_out_fences *out_fences; > > + struct drm_out_fence_state *fence_state; > > + int num_fences = 0; > > + int i, ret; > > + > > + if (count_out_fences > dev->mode_config.num_crtc) > > + return ERR_PTR(-EINVAL); > > + > > + out_fences = kcalloc(count_out_fences, sizeof(*out_fences), > > +GFP_KERNEL); > > + if (!out_fences) > > + return ERR_PTR(-ENOMEM); > > A bit tricky, but the above kcalloc is the only thing that catches integer > overflows in count_out_fences. Needs a comment imo since this could be a > security exploit if we accidentally screw it up. The check above makes sure that count_out_fences is not bigger than num_crtc. Don't that fix this? > > Also needs a testcase imo. > > > + > > + fence_state = kcalloc(count_out_fences, sizeof(*fence_state), > > +GFP_KERNEL); > > + if (!fence_state) { > > + kfree(out_fences); > > + return ERR_PTR(-ENOMEM); > > + } > > + > > + for (i = 0 ; i < count_out_fences ; i++) > > + fence_state[i].fd = -1; > > + > > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > > + struct drm_pending_vblank_event *e; > > + struct fence *fence; > > + char name[32]; > > + > > + fence = kzalloc(sizeof(*fence), GFP_KERNEL); > > + if (!fence) { > > + ret = -ENOMEM; > > + goto out; > > + } > > + > > + fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, > > + crtc->fence_context, crtc->fence_seqno); > > + > > + snprintf(name, sizeof(name), "crtc-%d_%lu", > > +drm_crtc_index(crtc), crtc->fence_seqno++); > > Hm ... fence_init_with_name? I'm kinda confused why we only name fences > that are exported though, and why not all of them. Debugging fence > deadlocks is real hard, so giving them all names might be a good idea. > > Anyway, seems like more room for a bit more sync_file/struct fence > merging. We just removed name from sync_file_create() so snprintf() is not even necessary here anymore. > > > + > > + fence_state[i].fd = get_unused_fd_flags(O_CLOEXEC); > > + if (fence_state[i].fd < 0) { > > + fence_put(fence); > > + ret = fence_state[i].fd; > > + goto out; > > + } > > + > > + fence_state[i].sync_file = sync_file_create(name, fence); > > +
[PATCH 03/13] staging/android: remove struct sync_timeline_ops
From: Gustavo Padovan Move drv_name, the last field of sync_timeline_ops, to sync_timeline and remove sync_timeline_ops. struct sync_timeline_ops was just an extra abstraction on top of fence_ops, and in the last few commits we removed all it ops in favor of cleaner fence_ops. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c| 9 ++--- drivers/staging/android/sync.c | 8 drivers/staging/android/sync.h | 28 +--- drivers/staging/android/sync_debug.c | 3 +-- 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 4200b12..c5e92c6 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -38,16 +38,11 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) } EXPORT_SYMBOL(sw_sync_pt_create); -static struct sync_timeline_ops sw_sync_timeline_ops = { - .driver_name = "sw_sync", -}; - struct sw_sync_timeline *sw_sync_timeline_create(const char *name) { struct sw_sync_timeline *obj = (struct sw_sync_timeline *) - sync_timeline_create(&sw_sync_timeline_ops, -sizeof(struct sw_sync_timeline), -name); + sync_timeline_create(sizeof(struct sw_sync_timeline), +"sw_sync", name); return obj; } diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index c75d1e6..b3efcaa 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -30,8 +30,8 @@ static const struct fence_ops android_fence_ops; -struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, - int size, const char *name) +struct sync_timeline *sync_timeline_create(int size, const char *drv_name, + const char *name) { struct sync_timeline *obj; @@ -43,9 +43,9 @@ struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, return NULL; kref_init(&obj->kref); - obj->ops = ops; obj->context = fence_context_alloc(1); strlcpy(obj->name, name, sizeof(obj->name)); + strlcpy(obj->drv_name, drv_name, sizeof(obj->drv_name)); INIT_LIST_HEAD(&obj->child_list_head); INIT_LIST_HEAD(&obj->active_list_head); @@ -139,7 +139,7 @@ static const char *android_fence_get_driver_name(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); - return parent->ops->driver_name; + return parent->drv_name; } static const char *android_fence_get_timeline_name(struct fence *fence) diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 29f8c19..f003e97 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -23,20 +23,10 @@ #include #include -struct sync_timeline; - -/** - * struct sync_timeline_ops - sync object implementation ops - * @driver_name: name of the implementation - */ -struct sync_timeline_ops { - const char *driver_name; -}; - /** * struct sync_timeline - sync object * @kref: reference count on fence. - * @ops: ops that define the implementation of the sync_timeline + * @drv_name: drv_name of the driver using the sync_timeline * @name: name of the sync_timeline. Useful for debugging * @destroyed: set when sync_timeline is destroyed * @child_list_head: list of children sync_pts for this sync_timeline @@ -47,7 +37,7 @@ struct sync_timeline_ops { */ struct sync_timeline { struct kref kref; - const struct sync_timeline_ops *ops; + chardrv_name[32]; charname[32]; /* protected by child_list_lock */ @@ -76,17 +66,17 @@ static inline struct sync_timeline *fence_parent(struct fence *fence) /** * sync_timeline_create() - creates a sync object - * @ops: specifies the implementation ops for the object * @size: size to allocate for this obj + * @drv_name: sync_timeline driver name * @name: sync_timeline name * - * Creates a new sync_timeline which will use the implementation specified by - * @ops. @size bytes will be allocated allowing for implementation specific - * data to be kept after the generic sync_timeline struct. Returns the - * sync_timeline object or NULL in case of error. + * Creates a new sync_timeline. @size bytes will be allocated allowing + * for implementation specific data to be kept after the generic + * sync_timeline struct. Returns the sync_timeline object or NULL in + * case of error. */ -struct sync_timeline *sync_timeline_create(const struct sync_timeline_o
[PATCH 00/13] staging/android: clean up SW_SYNC
From: Gustavo Padovan Hi, The following patches do a clean up on the sw_sync inteface. It starts by removing struct sync_timeline_ops, which was creating unecessary wrappers in the code and the start to organize the sync_timeline and sw_sync code better. sw_sync interface was moved to sw_sync.c along with sync_timeline - which is now internal to sw_sync. The next step after this work is the actual de-stage of SW_SYNC and the upstreaming of selftests for sw_sync and sync_file. Please review! Gustavo --- Gustavo Padovan (13): staging/android: store last signaled value on sync timeline staging/android: remove .{fence,timeline}_value_str() from timeline_ops staging/android: remove struct sync_timeline_ops staging/android: remove sw_sync_timeline and sw_sync_pt staging/android: remove sw_sync.[ch] files staging/android: rename android_fence to timeline_fence staging/android: remove unnecessary check for fence staging/android: remove size arg of sync_timeline_create() staging/android: bring struct sync_pt back staging/android: move sw_sync related code to sw_sync.c staging/android: clean up #includes in the sync framework staging/android: make sync_timeline internal to sw_sync staging/android: make sw_ioctl info internal to sw_sync.c drivers/staging/android/Kconfig| 16 +- drivers/staging/android/Makefile | 3 +- drivers/staging/android/sw_sync.c | 355 + drivers/staging/android/sw_sync.h | 59 -- drivers/staging/android/sync.c | 221 drivers/staging/android/sync.h | 95 ++--- drivers/staging/android/sync_debug.c | 152 +- drivers/staging/android/trace/sync.h | 12 +- drivers/staging/android/uapi/sw_sync.h | 32 --- include/linux/fence.h | 2 - 10 files changed, 350 insertions(+), 597 deletions(-) delete mode 100644 drivers/staging/android/sw_sync.h delete mode 100644 drivers/staging/android/sync.c delete mode 100644 drivers/staging/android/uapi/sw_sync.h -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 13/13] staging/android: make sw_ioctl info internal to sw_sync.c
From: Gustavo Padovan We don't want to export this from the kernel. This is interface is only for testing and debug. So testers shall copy the ioctl info in their own projects. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c | 13 - drivers/staging/android/uapi/sw_sync.h | 32 2 files changed, 12 insertions(+), 33 deletions(-) delete mode 100644 drivers/staging/android/uapi/sw_sync.h diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 4922233..7a7acc1 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -20,12 +20,23 @@ #include #include -#include "uapi/sw_sync.h" #include "sync.h" #define CREATE_TRACE_POINTS #include "trace/sync.h" +struct sw_sync_create_fence_data { + __u32 value; + charname[32]; + __s32 fence; /* fd of new fence */ +}; + +#define SW_SYNC_IOC_MAGIC 'W' + +#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\ + struct sw_sync_create_fence_data) +#define SW_SYNC_IOC_INC_IOW(SW_SYNC_IOC_MAGIC, 1, __u32) + /** * sync_timeline_create() - creates a sync object * @drv_name: sync_timeline driver name diff --git a/drivers/staging/android/uapi/sw_sync.h b/drivers/staging/android/uapi/sw_sync.h deleted file mode 100644 index 9b5d486..000 --- a/drivers/staging/android/uapi/sw_sync.h +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright (C) 2012 Google, Inc. - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - */ - -#ifndef _UAPI_LINUX_SW_SYNC_H -#define _UAPI_LINUX_SW_SYNC_H - -#include - -struct sw_sync_create_fence_data { - __u32 value; - charname[32]; - __s32 fence; /* fd of new fence */ -}; - -#define SW_SYNC_IOC_MAGIC 'W' - -#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\ - struct sw_sync_create_fence_data) -#define SW_SYNC_IOC_INC_IOW(SW_SYNC_IOC_MAGIC, 1, __u32) - -#endif /* _UAPI_LINUX_SW_SYNC_H */ -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/13] staging/android: bring struct sync_pt back
From: Gustavo Padovan Move the list_head members from sync_pt to struct fence was a mistake, they will not be used by struct fence as planned before, so here we create sync_pt again to bring the list heads back. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 40 ++-- drivers/staging/android/sync.h | 29 ++ drivers/staging/android/sync_debug.c | 16 +++ include/linux/fence.h| 2 -- 4 files changed, 53 insertions(+), 34 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index c83a599..aab80ec 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -28,8 +28,6 @@ #define CREATE_TRACE_POINTS #include "trace/sync.h" -static const struct fence_ops timeline_fence_ops; - struct sync_timeline *sync_timeline_create(const char *drv_name, const char *name) { @@ -90,7 +88,7 @@ EXPORT_SYMBOL(sync_timeline_destroy); void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) { unsigned long flags; - struct fence *fence, *next; + struct sync_pt *pt, *next; trace_sync_timeline(obj); @@ -98,37 +96,37 @@ void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) obj->value += inc; - list_for_each_entry_safe(fence, next, &obj->active_list_head, + list_for_each_entry_safe(pt, next, &obj->active_list_head, active_list) { - if (fence_is_signaled_locked(fence)) - list_del_init(&fence->active_list); + if (fence_is_signaled_locked(&pt->base)) + list_del_init(&pt->active_list); } spin_unlock_irqrestore(&obj->child_list_lock, flags); } EXPORT_SYMBOL(sync_timeline_signal); -struct fence *sync_pt_create(struct sync_timeline *obj, int size, +struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size, unsigned int value) { unsigned long flags; - struct fence *fence; + struct sync_pt *pt; - if (size < sizeof(*fence)) + if (size < sizeof(*pt)) return NULL; - fence = kzalloc(size, GFP_KERNEL); - if (!fence) + pt = kzalloc(size, GFP_KERNEL); + if (!pt) return NULL; spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); - fence_init(fence, &timeline_fence_ops, &obj->child_list_lock, + fence_init(&pt->base, &timeline_fence_ops, &obj->child_list_lock, obj->context, value); - list_add_tail(&fence->child_list, &obj->child_list_head); - INIT_LIST_HEAD(&fence->active_list); + list_add_tail(&pt->child_list, &obj->child_list_head); + INIT_LIST_HEAD(&pt->active_list); spin_unlock_irqrestore(&obj->child_list_lock, flags); - return fence; + return pt; } EXPORT_SYMBOL(sync_pt_create); @@ -148,13 +146,14 @@ static const char *timeline_fence_get_timeline_name(struct fence *fence) static void timeline_fence_release(struct fence *fence) { + struct sync_pt *pt = fence_to_sync_pt(fence); struct sync_timeline *parent = fence_parent(fence); unsigned long flags; spin_lock_irqsave(fence->lock, flags); - list_del(&fence->child_list); - if (WARN_ON_ONCE(!list_empty(&fence->active_list))) - list_del(&fence->active_list); + list_del(&pt->child_list); + if (WARN_ON_ONCE(!list_empty(&pt->active_list))) + list_del(&pt->active_list); spin_unlock_irqrestore(fence->lock, flags); sync_timeline_put(parent); @@ -170,12 +169,13 @@ static bool timeline_fence_signaled(struct fence *fence) static bool timeline_fence_enable_signaling(struct fence *fence) { + struct sync_pt *pt = fence_to_sync_pt(fence); struct sync_timeline *parent = fence_parent(fence); if (timeline_fence_signaled(fence)) return false; - list_add_tail(&fence->active_list, &parent->active_list_head); + list_add_tail(&pt->active_list, &parent->active_list_head); return true; } @@ -193,7 +193,7 @@ static void timeline_fence_timeline_value_str(struct fence *fence, snprintf(str, size, "%d", parent->value); } -static const struct fence_ops timeline_fence_ops = { +const struct fence_ops timeline_fence_ops = { .get_driver_name = timeline_fence_get_driver_name, .get_timeline_name = timeline_fence_get_timeline_name, .enable_signaling = timeline_fence_enable_signaling, diff --git a/drivers/staging/an
[PATCH 07/13] staging/android: remove unnecessary check for fence
From: Gustavo Padovan When we call sync_print_fence() fence is always valid. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync_debug.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index dc85d5f..6282046 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -109,7 +109,7 @@ static void sync_print_fence(struct seq_file *s, struct fence *fence, bool show) seq_printf(s, "@%lld.%09ld", (s64)ts64.tv_sec, ts64.tv_nsec); } - if ((!fence || fence->ops->timeline_value_str) && + if (fence->ops->timeline_value_str && fence->ops->fence_value_str) { char value[64]; bool success; @@ -117,10 +117,9 @@ static void sync_print_fence(struct seq_file *s, struct fence *fence, bool show) fence->ops->fence_value_str(fence, value, sizeof(value)); success = strlen(value); - if (success) + if (success) { seq_printf(s, ": %s", value); - if (success && fence) { fence->ops->timeline_value_str(fence, value, sizeof(value)); -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/13] staging/android: clean up #includes in the sync framework
From: Gustavo Padovan Most of the includes there are not necessary anymore. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 6 -- drivers/staging/android/sync.h | 3 --- drivers/staging/android/sync_debug.c | 16 3 files changed, 25 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index aab80ec..bb12d86 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -14,14 +14,8 @@ * */ -#include #include -#include -#include -#include #include -#include -#include #include "sync.h" diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 02ecf44..54c515b 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -13,9 +13,6 @@ #ifndef _LINUX_SYNC_H #define _LINUX_SYNC_H -#include -#include -#include #include #include #include diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 2733cc3..864ad01 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -15,22 +15,6 @@ */ #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "uapi/sw_sync.h" #include "sync.h" #ifdef CONFIG_DEBUG_FS -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 12/13] staging/android: make sync_timeline internal to sw_sync
From: Gustavo Padovan The only use sync_timeline will have in upstream kernel is for debugging through the SW_SYNC interface. So make it internal to SW_SYNC to avoid people use it in the future. Signed-off-by: Gustavo Padovan --- drivers/staging/android/Kconfig | 16 +-- drivers/staging/android/Makefile | 3 +- drivers/staging/android/sw_sync.c | 211 ++ drivers/staging/android/sync.c| 199 --- drivers/staging/android/sync.h| 49 - 5 files changed, 216 insertions(+), 262 deletions(-) delete mode 100644 drivers/staging/android/sync.c diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig index 6480f60..f52c682 100644 --- a/drivers/staging/android/Kconfig +++ b/drivers/staging/android/Kconfig @@ -24,26 +24,18 @@ config ANDROID_LOW_MEMORY_KILLER scripts (/init.rc), and it defines priority values with minimum free memory size for each priority. -config SYNC - bool "Synchronization framework" - default n - select ANON_INODES - select DMA_SHARED_BUFFER - ---help--- - This option enables the framework for synchronization between multiple - drivers. Sync implementations can take advantage of hardware - synchronization built into devices like GPUs. - config SW_SYNC - bool "Software synchronization objects" + bool "Software synchronization framework" default n - depends on SYNC depends on SYNC_FILE ---help--- A sync object driver that uses a 32bit counter to coordinate synchronization. Useful when there is no hardware primitive backing the synchronization. + WARNING: improper use of this can result in deadlocking kernel + drivers from userspace. Intended for test and debug only. + source "drivers/staging/android/ion/Kconfig" endif # if ANDROID diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index 980d6dc..7ca61b7 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -4,5 +4,4 @@ obj-y += ion/ obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)+= lowmemorykiller.o -obj-$(CONFIG_SYNC) += sync.o sync_debug.o -obj-$(CONFIG_SW_SYNC) += sw_sync.o +obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 90e3ee5..4922233 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -17,11 +17,222 @@ #include #include #include +#include #include #include "uapi/sw_sync.h" #include "sync.h" +#define CREATE_TRACE_POINTS +#include "trace/sync.h" + +/** + * sync_timeline_create() - creates a sync object + * @drv_name: sync_timeline driver name + * @name: sync_timeline name + * + * Creates a new sync_timeline. Returns the sync_timeline object or NULL in + * case of error. + */ +struct sync_timeline *sync_timeline_create(const char *drv_name, + const char *name) +{ + struct sync_timeline *obj; + + obj = kzalloc(sizeof(*obj), GFP_KERNEL); + if (!obj) + return NULL; + + kref_init(&obj->kref); + obj->context = fence_context_alloc(1); + strlcpy(obj->name, name, sizeof(obj->name)); + strlcpy(obj->drv_name, drv_name, sizeof(obj->drv_name)); + + INIT_LIST_HEAD(&obj->child_list_head); + INIT_LIST_HEAD(&obj->active_list_head); + spin_lock_init(&obj->child_list_lock); + + sync_timeline_debug_add(obj); + + return obj; +} + +static void sync_timeline_free(struct kref *kref) +{ + struct sync_timeline *obj = + container_of(kref, struct sync_timeline, kref); + + sync_timeline_debug_remove(obj); + + kfree(obj); +} + +static void sync_timeline_get(struct sync_timeline *obj) +{ + kref_get(&obj->kref); +} + +static void sync_timeline_put(struct sync_timeline *obj) +{ + kref_put(&obj->kref, sync_timeline_free); +} + +/** + * sync_timeline_destroy() - destroys a sync object + * @obj: sync_timeline to destroy + * + * A sync implementation should call this when the @obj is going away + * (i.e. module unload.) @obj won't actually be freed until all its children + * fences are freed. + */ +static void sync_timeline_destroy(struct sync_timeline *obj) +{ + obj->destroyed = true; + /* +* Ensure timeline is marked as destroyed before +* changing timeline's fences status. +*/ + smp_wmb(); + + sync_timeline_put(obj); +} + +/** + * sync_timeline_signal() - signal a status change
[PATCH 06/13] staging/android: rename android_fence to timeline_fence
From: Gustavo Padovan We are moving out of staging/android so rename it to a name that is not related to android anymore. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index b3efcaa..442d808 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -28,7 +28,7 @@ #define CREATE_TRACE_POINTS #include "trace/sync.h" -static const struct fence_ops android_fence_ops; +static const struct fence_ops timeline_fence_ops; struct sync_timeline *sync_timeline_create(int size, const char *drv_name, const char *name) @@ -126,7 +126,7 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size, spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); - fence_init(fence, &android_fence_ops, &obj->child_list_lock, + fence_init(fence, &timeline_fence_ops, &obj->child_list_lock, obj->context, value); list_add_tail(&fence->child_list, &obj->child_list_head); INIT_LIST_HEAD(&fence->active_list); @@ -135,21 +135,21 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size, } EXPORT_SYMBOL(sync_pt_create); -static const char *android_fence_get_driver_name(struct fence *fence) +static const char *timeline_fence_get_driver_name(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); return parent->drv_name; } -static const char *android_fence_get_timeline_name(struct fence *fence) +static const char *timeline_fence_get_timeline_name(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); return parent->name; } -static void android_fence_release(struct fence *fence) +static void timeline_fence_release(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); unsigned long flags; @@ -164,31 +164,31 @@ static void android_fence_release(struct fence *fence) fence_free(fence); } -static bool android_fence_signaled(struct fence *fence) +static bool timeline_fence_signaled(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); return (fence->seqno > parent->value) ? false : true; } -static bool android_fence_enable_signaling(struct fence *fence) +static bool timeline_fence_enable_signaling(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); - if (android_fence_signaled(fence)) + if (timeline_fence_signaled(fence)) return false; list_add_tail(&fence->active_list, &parent->active_list_head); return true; } -static void android_fence_value_str(struct fence *fence, +static void timeline_fence_value_str(struct fence *fence, char *str, int size) { snprintf(str, size, "%d", fence->seqno); } -static void android_fence_timeline_value_str(struct fence *fence, +static void timeline_fence_timeline_value_str(struct fence *fence, char *str, int size) { struct sync_timeline *parent = fence_parent(fence); @@ -196,13 +196,13 @@ static void android_fence_timeline_value_str(struct fence *fence, snprintf(str, size, "%d", parent->value); } -static const struct fence_ops android_fence_ops = { - .get_driver_name = android_fence_get_driver_name, - .get_timeline_name = android_fence_get_timeline_name, - .enable_signaling = android_fence_enable_signaling, - .signaled = android_fence_signaled, +static const struct fence_ops timeline_fence_ops = { + .get_driver_name = timeline_fence_get_driver_name, + .get_timeline_name = timeline_fence_get_timeline_name, + .enable_signaling = timeline_fence_enable_signaling, + .signaled = timeline_fence_signaled, .wait = fence_default_wait, - .release = android_fence_release, - .fence_value_str = android_fence_value_str, - .timeline_value_str = android_fence_timeline_value_str, + .release = timeline_fence_release, + .fence_value_str = timeline_fence_value_str, + .timeline_value_str = timeline_fence_timeline_value_str, }; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/13] staging/android: remove sw_sync.[ch] files
From: Gustavo Padovan We can glue the sw_sync file operations directly on the sync framework without the need to pass through sw_sync wrappers. It only builds sw_sync debugfs file support if CONFIG_SW_SYNC is enabled. Signed-off-by: Gustavo Padovan --- drivers/staging/android/Makefile | 1 - drivers/staging/android/sw_sync.c| 45 -- drivers/staging/android/sw_sync.h| 47 drivers/staging/android/sync_debug.c | 17 ++--- 4 files changed, 13 insertions(+), 97 deletions(-) delete mode 100644 drivers/staging/android/sw_sync.c delete mode 100644 drivers/staging/android/sw_sync.h diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index 980d6dc..bf45967 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -5,4 +5,3 @@ obj-y += ion/ obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)+= lowmemorykiller.o obj-$(CONFIG_SYNC) += sync.o sync_debug.o -obj-$(CONFIG_SW_SYNC) += sw_sync.o diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c deleted file mode 100644 index 461dbd9..000 --- a/drivers/staging/android/sw_sync.c +++ /dev/null @@ -1,45 +0,0 @@ -/* - * drivers/base/sw_sync.c - * - * Copyright (C) 2012 Google, Inc. - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include "sw_sync.h" - -struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value) -{ - return sync_pt_create(obj, sizeof(struct fence), value); -} -EXPORT_SYMBOL(sw_sync_pt_create); - -struct sync_timeline *sw_sync_timeline_create(const char *name) -{ - return sync_timeline_create(sizeof(struct sync_timeline), - "sw_sync", name); -} -EXPORT_SYMBOL(sw_sync_timeline_create); - -void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc) -{ - sync_timeline_signal(obj, inc); -} -EXPORT_SYMBOL(sw_sync_timeline_inc); diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h deleted file mode 100644 index 9f26c62..000 --- a/drivers/staging/android/sw_sync.h +++ /dev/null @@ -1,47 +0,0 @@ -/* - * include/linux/sw_sync.h - * - * Copyright (C) 2012 Google, Inc. - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - */ - -#ifndef _LINUX_SW_SYNC_H -#define _LINUX_SW_SYNC_H - -#include -#include -#include "sync.h" -#include "uapi/sw_sync.h" - -#if IS_ENABLED(CONFIG_SW_SYNC) -struct sync_timeline *sw_sync_timeline_create(const char *name); -void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc); - -struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value); -#else -static inline struct sync_timeline *sw_sync_timeline_create(const char *name) -{ - return NULL; -} - -static inline void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc) -{ -} - -static inline struct fence *sw_sync_pt_create(struct sync_timeline *obj, - u32 value) -{ - return NULL; -} -#endif /* IS_ENABLED(CONFIG_SW_SYNC) */ - -#endif /* _LINUX_SW_SYNC_H */ diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index e207a4d..dc85d5f 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -27,7 +27,11 @@ #include #include #include -#include "sw_sync.h" +#include +#include + +#include "uapi/sw_sync.h" +#include "sync.h" #ifdef CONFIG_DEBUG_FS @@ -200,6 +204,7 @@ static const struct file_operations sync_info_debugfs_fops = { .release= single_release, }; +#if IS_ENABLED(CONFIG_SW_SYNC) /* * *WARNING* * @@ -214,7 +219,7 @@ static int sw_sync_debugfs_open(struct inode *inode, struct file *file) get_task_comm(task_comm, current); - obj = sw_sync_timeline_create(task_comm); + obj = sync
[PATCH 01/13] staging/android: store last signaled value on sync timeline
From: Gustavo Padovan Now fence timeline is aware of the last signaled fence, as it receives the increment to the current value in sync_timeline_signal(). That allow us to remove .has_signaled() from timeline_ops as we can directly compare using timeline->value and fence->seqno in sync.c Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c | 16 ++-- drivers/staging/android/sync.c| 15 +++ drivers/staging/android/sync.h| 14 +- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index af39ff5..428e22c 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -30,7 +30,7 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) struct sw_sync_pt *pt; pt = (struct sw_sync_pt *) - sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt)); + sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt), value); pt->value = value; @@ -38,15 +38,6 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) } EXPORT_SYMBOL(sw_sync_pt_create); -static int sw_sync_fence_has_signaled(struct fence *fence) -{ - struct sw_sync_pt *pt = (struct sw_sync_pt *)fence; - struct sw_sync_timeline *obj = - (struct sw_sync_timeline *)fence_parent(fence); - - return (pt->value > obj->value) ? 0 : 1; -} - static void sw_sync_timeline_value_str(struct sync_timeline *sync_timeline, char *str, int size) { @@ -64,7 +55,6 @@ static void sw_sync_fence_value_str(struct fence *fence, char *str, int size) static struct sync_timeline_ops sw_sync_timeline_ops = { .driver_name = "sw_sync", - .has_signaled = sw_sync_fence_has_signaled, .timeline_value_str = sw_sync_timeline_value_str, .fence_value_str = sw_sync_fence_value_str, }; @@ -82,8 +72,6 @@ EXPORT_SYMBOL(sw_sync_timeline_create); void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc) { - obj->value += inc; - - sync_timeline_signal(&obj->obj); + sync_timeline_signal(&obj->obj, inc); } EXPORT_SYMBOL(sw_sync_timeline_inc); diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 1d14c83..8dd2181 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -90,7 +90,7 @@ void sync_timeline_destroy(struct sync_timeline *obj) } EXPORT_SYMBOL(sync_timeline_destroy); -void sync_timeline_signal(struct sync_timeline *obj) +void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) { unsigned long flags; struct fence *fence, *next; @@ -99,6 +99,8 @@ void sync_timeline_signal(struct sync_timeline *obj) spin_lock_irqsave(&obj->child_list_lock, flags); + obj->value += inc; + list_for_each_entry_safe(fence, next, &obj->active_list_head, active_list) { if (fence_is_signaled_locked(fence)) @@ -109,7 +111,8 @@ void sync_timeline_signal(struct sync_timeline *obj) } EXPORT_SYMBOL(sync_timeline_signal); -struct fence *sync_pt_create(struct sync_timeline *obj, int size) +struct fence *sync_pt_create(struct sync_timeline *obj, int size, +unsigned int value) { unsigned long flags; struct fence *fence; @@ -124,7 +127,7 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size) spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); fence_init(fence, &android_fence_ops, &obj->child_list_lock, - obj->context, ++obj->value); + obj->context, value); list_add_tail(&fence->child_list, &obj->child_list_head); INIT_LIST_HEAD(&fence->active_list); spin_unlock_irqrestore(&obj->child_list_lock, flags); @@ -164,12 +167,8 @@ static void android_fence_release(struct fence *fence) static bool android_fence_signaled(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); - int ret; - ret = parent->ops->has_signaled(fence); - if (ret < 0) - fence->status = ret; - return ret; + return (fence->seqno > parent->value) ? false : true; } static bool android_fence_enable_signaling(struct fence *fence) diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index b56885c..627525c 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -28,19 +28,12 @@ struct sync_timeline; /** * struct sync_timeline_ops - sync object implementation ops * @driver_name: name of the implementation - * @has_signaled: returns: - *
[PATCH 02/13] staging/android: remove .{fence, timeline}_value_str() from timeline_ops
From: Gustavo Padovan Now that the value of fence and the timeline are not stored by sw_sync anymore we can remove this extra abstraction to retrieve this data. This patch changes both fence_ops (.fence_value_str and .timeline_value_str) to return the str directly. It also clean up struct sync_timeline_ops by removing both ops from there. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c| 17 - drivers/staging/android/sync.c | 16 ++-- drivers/staging/android/sync.h | 9 - drivers/staging/android/sync_debug.c | 12 ++-- drivers/staging/android/trace/sync.h | 12 +++- 5 files changed, 7 insertions(+), 59 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 428e22c..4200b12 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -38,25 +38,8 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) } EXPORT_SYMBOL(sw_sync_pt_create); -static void sw_sync_timeline_value_str(struct sync_timeline *sync_timeline, - char *str, int size) -{ - struct sw_sync_timeline *timeline = - (struct sw_sync_timeline *)sync_timeline; - snprintf(str, size, "%d", timeline->value); -} - -static void sw_sync_fence_value_str(struct fence *fence, char *str, int size) -{ - struct sw_sync_pt *pt = (struct sw_sync_pt *)fence; - - snprintf(str, size, "%d", pt->value); -} - static struct sync_timeline_ops sw_sync_timeline_ops = { .driver_name = "sw_sync", - .timeline_value_str = sw_sync_timeline_value_str, - .fence_value_str = sw_sync_fence_value_str, }; struct sw_sync_timeline *sw_sync_timeline_create(const char *name) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 8dd2181..c75d1e6 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -185,14 +185,7 @@ static bool android_fence_enable_signaling(struct fence *fence) static void android_fence_value_str(struct fence *fence, char *str, int size) { - struct sync_timeline *parent = fence_parent(fence); - - if (!parent->ops->fence_value_str) { - if (size) - *str = 0; - return; - } - parent->ops->fence_value_str(fence, str, size); + snprintf(str, size, "%d", fence->seqno); } static void android_fence_timeline_value_str(struct fence *fence, @@ -200,12 +193,7 @@ static void android_fence_timeline_value_str(struct fence *fence, { struct sync_timeline *parent = fence_parent(fence); - if (!parent->ops->timeline_value_str) { - if (size) - *str = 0; - return; - } - parent->ops->timeline_value_str(parent, str, size); + snprintf(str, size, "%d", parent->value); } static const struct fence_ops android_fence_ops = { diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 627525c..29f8c19 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -28,18 +28,9 @@ struct sync_timeline; /** * struct sync_timeline_ops - sync object implementation ops * @driver_name: name of the implementation - * @timeline_value_str: fill str with the value of the sync_timeline's counter - * @fence_value_str: fill str with the value of the fence */ struct sync_timeline_ops { const char *driver_name; - - /* optional */ - void (*timeline_value_str)(struct sync_timeline *timeline, char *str, - int size); - - /* optional */ - void (*fence_value_str)(struct fence *fence, char *str, int size); }; /** diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 5f57499..c532457 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -133,16 +133,8 @@ static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj) struct list_head *pos; unsigned long flags; - seq_printf(s, "%s %s", obj->name, obj->ops->driver_name); - - if (obj->ops->timeline_value_str) { - char value[64]; - - obj->ops->timeline_value_str(obj, value, sizeof(value)); - seq_printf(s, ": %s", value); - } - - seq_puts(s, "\n"); + seq_printf(s, "%s %s: %d\n", obj->name, obj->ops->driver_name, + obj->value); spin_lock_irqsave(&obj->child_list_lock, flags); list_for_each(pos, &obj->child_list_head) { diff --git a/drivers/staging/android/trace/sync.h b/drivers/staging/andro
[PATCH 04/13] staging/android: remove sw_sync_timeline and sw_sync_pt
From: Gustavo Padovan As we moved value storage to sync_timeline and fence those two structs became useless and can be removed now. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c| 24 +++- drivers/staging/android/sw_sync.h| 24 ++-- drivers/staging/android/sync_debug.c | 12 ++-- 3 files changed, 19 insertions(+), 41 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index c5e92c6..461dbd9 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -25,31 +25,21 @@ #include "sw_sync.h" -struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) +struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value) { - struct sw_sync_pt *pt; - - pt = (struct sw_sync_pt *) - sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt), value); - - pt->value = value; - - return (struct fence *)pt; + return sync_pt_create(obj, sizeof(struct fence), value); } EXPORT_SYMBOL(sw_sync_pt_create); -struct sw_sync_timeline *sw_sync_timeline_create(const char *name) +struct sync_timeline *sw_sync_timeline_create(const char *name) { - struct sw_sync_timeline *obj = (struct sw_sync_timeline *) - sync_timeline_create(sizeof(struct sw_sync_timeline), -"sw_sync", name); - - return obj; + return sync_timeline_create(sizeof(struct sync_timeline), + "sw_sync", name); } EXPORT_SYMBOL(sw_sync_timeline_create); -void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc) +void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc) { - sync_timeline_signal(&obj->obj, inc); + sync_timeline_signal(obj, inc); } EXPORT_SYMBOL(sw_sync_timeline_inc); diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h index e18667b..9f26c62 100644 --- a/drivers/staging/android/sw_sync.h +++ b/drivers/staging/android/sw_sync.h @@ -22,34 +22,22 @@ #include "sync.h" #include "uapi/sw_sync.h" -struct sw_sync_timeline { - struct sync_timeline obj; - - u32 value; -}; - -struct sw_sync_pt { - struct fencept; - - u32 value; -}; - #if IS_ENABLED(CONFIG_SW_SYNC) -struct sw_sync_timeline *sw_sync_timeline_create(const char *name); -void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc); +struct sync_timeline *sw_sync_timeline_create(const char *name); +void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc); -struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value); +struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value); #else -static inline struct sw_sync_timeline *sw_sync_timeline_create(const char *name) +static inline struct sync_timeline *sw_sync_timeline_create(const char *name) { return NULL; } -static inline void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc) +static inline void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc) { } -static inline struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, +static inline struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value) { return NULL; diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index e5634f2..e207a4d 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -209,7 +209,7 @@ static const struct file_operations sync_info_debugfs_fops = { /* opening sw_sync create a new sync obj */ static int sw_sync_debugfs_open(struct inode *inode, struct file *file) { - struct sw_sync_timeline *obj; + struct sync_timeline *obj; char task_comm[TASK_COMM_LEN]; get_task_comm(task_comm, current); @@ -225,13 +225,13 @@ static int sw_sync_debugfs_open(struct inode *inode, struct file *file) static int sw_sync_debugfs_release(struct inode *inode, struct file *file) { - struct sw_sync_timeline *obj = file->private_data; + struct sync_timeline *obj = file->private_data; - sync_timeline_destroy(&obj->obj); + sync_timeline_destroy(obj); return 0; } -static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj, +static long sw_sync_ioctl_create_fence(struct sync_timeline *obj, unsigned long arg) { int fd = get_unused_fd_flags(O_CLOEXEC); @@ -277,7 +277,7 @@ err: return err; } -static long sw_sync_ioctl_inc(struct sw_sync_timeline *obj, unsigned long arg) +static long sw_sync_ioctl_inc(struct sync_timeline *obj, unsigned long arg) { u32 value; @@ -292,7 +292,7 @@ static long sw_sync_ioctl_inc(struct sw_sync
[PATCH 10/13] staging/android: move sw_sync related code to sw_sync.c
From: Gustavo Padovan Split sync_debug and sw_sync in two different files. Signed-off-by: Gustavo Padovan --- drivers/staging/android/Makefile | 1 + drivers/staging/android/sw_sync.c| 136 +++ drivers/staging/android/sync.h | 2 + drivers/staging/android/sync_debug.c | 115 - 4 files changed, 139 insertions(+), 115 deletions(-) create mode 100644 drivers/staging/android/sw_sync.c diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index bf45967..980d6dc 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -5,3 +5,4 @@ obj-y += ion/ obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)+= lowmemorykiller.o obj-$(CONFIG_SYNC) += sync.o sync_debug.o +obj-$(CONFIG_SW_SYNC) += sw_sync.o diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c new file mode 100644 index 000..90e3ee5 --- /dev/null +++ b/drivers/staging/android/sw_sync.c @@ -0,0 +1,136 @@ +/* + * drivers/dma-buf/sw_sync.c + * + * Copyright (C) 2012 Google, Inc. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include +#include +#include +#include + +#include "uapi/sw_sync.h" +#include "sync.h" + +/* + * *WARNING* + * + * improper use of this can result in deadlocking kernel drivers from userspace. + */ + +/* opening sw_sync create a new sync obj */ +static int sw_sync_debugfs_open(struct inode *inode, struct file *file) +{ + struct sync_timeline *obj; + char task_comm[TASK_COMM_LEN]; + + get_task_comm(task_comm, current); + + obj = sync_timeline_create("sw_sync", task_comm); + if (!obj) + return -ENOMEM; + + file->private_data = obj; + + return 0; +} + +static int sw_sync_debugfs_release(struct inode *inode, struct file *file) +{ + struct sync_timeline *obj = file->private_data; + + sync_timeline_destroy(obj); + return 0; +} + +static long sw_sync_ioctl_create_fence(struct sync_timeline *obj, + unsigned long arg) +{ + int fd = get_unused_fd_flags(O_CLOEXEC); + int err; + struct sync_pt *pt; + struct sync_file *sync_file; + struct sw_sync_create_fence_data data; + + if (fd < 0) + return fd; + + if (copy_from_user(&data, (void __user *)arg, sizeof(data))) { + err = -EFAULT; + goto err; + } + + pt = sync_pt_create(obj, sizeof(*pt), data.value); + if (!pt) { + err = -ENOMEM; + goto err; + } + + sync_file = sync_file_create(&pt->base); + if (!sync_file) { + fence_put(&pt->base); + err = -ENOMEM; + goto err; + } + + data.fence = fd; + if (copy_to_user((void __user *)arg, &data, sizeof(data))) { + fput(sync_file->file); + err = -EFAULT; + goto err; + } + + fd_install(fd, sync_file->file); + + return 0; + +err: + put_unused_fd(fd); + return err; +} + +static long sw_sync_ioctl_inc(struct sync_timeline *obj, unsigned long arg) +{ + u32 value; + + if (copy_from_user(&value, (void __user *)arg, sizeof(value))) + return -EFAULT; + + sync_timeline_signal(obj, value); + + return 0; +} + +static long sw_sync_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct sync_timeline *obj = file->private_data; + + switch (cmd) { + case SW_SYNC_IOC_CREATE_FENCE: + return sw_sync_ioctl_create_fence(obj, arg); + + case SW_SYNC_IOC_INC: + return sw_sync_ioctl_inc(obj, arg); + + default: + return -ENOTTY; + } +} + +const struct file_operations sw_sync_debugfs_fops = { + .open = sw_sync_debugfs_open, + .release= sw_sync_debugfs_release, + .unlocked_ioctl = sw_sync_ioctl, + .compat_ioctl = sw_sync_ioctl, +}; diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 14b61cb..02ecf44 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -132,6 +132,8 @@ struct sync_pt *sync_pt_create(struct sync_timeline *parent, int size, #ifdef CONFIG_DEB
[PATCH 08/13] staging/android: remove size arg of sync_timeline_create()
From: Gustavo Padovan After we removed sw_sync_timeline this arg has not been really used by anyone, all its users pass the size of struct sync_timeline there. So simplify this function but not requiring the size anymore. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 7 ++- drivers/staging/android/sync.h | 7 ++- drivers/staging/android/sync_debug.c | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 442d808..c83a599 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -30,15 +30,12 @@ static const struct fence_ops timeline_fence_ops; -struct sync_timeline *sync_timeline_create(int size, const char *drv_name, +struct sync_timeline *sync_timeline_create(const char *drv_name, const char *name) { struct sync_timeline *obj; - if (size < sizeof(struct sync_timeline)) - return NULL; - - obj = kzalloc(size, GFP_KERNEL); + obj = kzalloc(sizeof(*obj), GFP_KERNEL); if (!obj) return NULL; diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index f003e97..f2fbf98 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -66,16 +66,13 @@ static inline struct sync_timeline *fence_parent(struct fence *fence) /** * sync_timeline_create() - creates a sync object - * @size: size to allocate for this obj * @drv_name: sync_timeline driver name * @name: sync_timeline name * - * Creates a new sync_timeline. @size bytes will be allocated allowing - * for implementation specific data to be kept after the generic - * sync_timeline struct. Returns the sync_timeline object or NULL in + * Creates a new sync_timeline. Returns the sync_timeline object or NULL in * case of error. */ -struct sync_timeline *sync_timeline_create(int size, const char *drv_name, +struct sync_timeline *sync_timeline_create(const char *drv_name, const char *name); /** diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 6282046..cb0f888 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -218,7 +218,7 @@ static int sw_sync_debugfs_open(struct inode *inode, struct file *file) get_task_comm(task_comm, current); - obj = sync_timeline_create(sizeof(*obj), "sw_sync", task_comm); + obj = sync_timeline_create("sw_sync", task_comm); if (!obj) return -ENOMEM; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/12] staging/android: remove redundant comments on sync_merge_data
2016-05-02 Pavel Machek : > On Wed 2016-04-27 13:27:08, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > struct sync_merge_data already have documentation on top of the > > struct definition. No need to duplicate it. > > > > Signed-off-by: Gustavo Padovan > > Reviewed-by: Maarten Lankhorst > > > @@ -33,8 +33,8 @@ struct sync_merge_data { > > /** > > * struct sync_fence_info - detailed fence information > > * @obj_name: name of parent sync_timeline > > - * @driver_name: name of driver implementing the parent > > - * @status:status of the fence 0:active 1:signaled <0:error > > +* @driver_name:name of driver implementing the parent > > +* @status: status of the fence 0:active 1:signaled <0:error > > The whitespace (or mail client configuration?) looks wrong here. this has been fixed in v2 already and Greg pulled everything into staging next already. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 06/12] staging/android: prepare sync_file for de-staging
Hi Pavel, 2016-05-02 Pavel Machek : > Hi! > > > > -} > > -EXPORT_SYMBOL(sync_file_merge); > > - > > static const char *android_fence_get_driver_name(struct fence *fence) > > { > > struct sync_timeline *parent = fence_parent(fence); > > if this is meant to be used outside android, should it select some > better prefix than android_fence_? Sure, This patchset doesn't touch this code, but my latest patchset changes them to timeline_*. Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] MAINTAINERS: add entry for the Sync File Framework
From: Gustavo Padovan Add Gustavo as maintainer for the Sync File Framework. Sumit is co-maintainer as he maintains drivers/dma-buf/. It also uses Sumit's tree as base. Cc: Sumit Semwal Signed-off-by: Gustavo Padovan --- MAINTAINERS | 10 ++ 1 file changed, 10 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8c10b4c..0abc9c3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3677,6 +3677,16 @@ F: include/linux/*fence.h F: Documentation/dma-buf-sharing.txt T: git git://git.linaro.org/people/sumitsemwal/linux-dma-buf.git +SYNC FILE FRAMEWORK +M: Gustavo Padovan +S: Maintained +L: linux-me...@vger.kernel.org +L: dri-de...@lists.freedesktop.org +F: drivers/dma-buf/sync_file.c +F: include/linux/sync_file.h +F: Documentation/sync_file.txt +T: git git://git.linaro.org/people/sumitsemwal/linux-dma-buf.git + DMA GENERIC OFFLOAD ENGINE SUBSYSTEM M: Vinod Koul L: dmaeng...@vger.kernel.org -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] MAINTAINERS: add entry for the Sync File Framework
From: Gustavo Padovan Add Gustavo as maintainer for the Sync File Framework. Sumit is co-maintainer as he maintains drivers/dma-buf/. It also uses Sumit's tree as base. Signed-off-by: Gustavo Padovan Acked-by: Sumit Semwal Acked-by: Maarten Lankhorst --- MAINTAINERS | 11 +++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8c10b4c..71c4e7f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3677,6 +3677,17 @@ F: include/linux/*fence.h F: Documentation/dma-buf-sharing.txt T: git git://git.linaro.org/people/sumitsemwal/linux-dma-buf.git +SYNC FILE FRAMEWORK +M: Sumit Semwal +R: Gustavo Padovan +S: Maintained +L: linux-me...@vger.kernel.org +L: dri-de...@lists.freedesktop.org +F: drivers/dma-buf/sync_file.c +F: include/linux/sync_file.h +F: Documentation/sync_file.txt +T: git git://git.linaro.org/people/sumitsemwal/linux-dma-buf.git + DMA GENERIC OFFLOAD ENGINE SUBSYSTEM M: Vinod Koul L: dmaeng...@vger.kernel.org -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/18] staging/android: store last signaled value on sync timeline
From: Gustavo Padovan Now fence timeline is aware of the last signaled fence, as it receives the increment to the current value in sync_timeline_signal(). That allow us to remove .has_signaled() from timeline_ops as we can directly compare using timeline->value and fence->seqno in sync.c Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c | 16 ++-- drivers/staging/android/sync.c| 15 +++ drivers/staging/android/sync.h| 14 +- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index af39ff5..428e22c 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -30,7 +30,7 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) struct sw_sync_pt *pt; pt = (struct sw_sync_pt *) - sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt)); + sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt), value); pt->value = value; @@ -38,15 +38,6 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) } EXPORT_SYMBOL(sw_sync_pt_create); -static int sw_sync_fence_has_signaled(struct fence *fence) -{ - struct sw_sync_pt *pt = (struct sw_sync_pt *)fence; - struct sw_sync_timeline *obj = - (struct sw_sync_timeline *)fence_parent(fence); - - return (pt->value > obj->value) ? 0 : 1; -} - static void sw_sync_timeline_value_str(struct sync_timeline *sync_timeline, char *str, int size) { @@ -64,7 +55,6 @@ static void sw_sync_fence_value_str(struct fence *fence, char *str, int size) static struct sync_timeline_ops sw_sync_timeline_ops = { .driver_name = "sw_sync", - .has_signaled = sw_sync_fence_has_signaled, .timeline_value_str = sw_sync_timeline_value_str, .fence_value_str = sw_sync_fence_value_str, }; @@ -82,8 +72,6 @@ EXPORT_SYMBOL(sw_sync_timeline_create); void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc) { - obj->value += inc; - - sync_timeline_signal(&obj->obj); + sync_timeline_signal(&obj->obj, inc); } EXPORT_SYMBOL(sw_sync_timeline_inc); diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 1d14c83..8dd2181 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -90,7 +90,7 @@ void sync_timeline_destroy(struct sync_timeline *obj) } EXPORT_SYMBOL(sync_timeline_destroy); -void sync_timeline_signal(struct sync_timeline *obj) +void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) { unsigned long flags; struct fence *fence, *next; @@ -99,6 +99,8 @@ void sync_timeline_signal(struct sync_timeline *obj) spin_lock_irqsave(&obj->child_list_lock, flags); + obj->value += inc; + list_for_each_entry_safe(fence, next, &obj->active_list_head, active_list) { if (fence_is_signaled_locked(fence)) @@ -109,7 +111,8 @@ void sync_timeline_signal(struct sync_timeline *obj) } EXPORT_SYMBOL(sync_timeline_signal); -struct fence *sync_pt_create(struct sync_timeline *obj, int size) +struct fence *sync_pt_create(struct sync_timeline *obj, int size, +unsigned int value) { unsigned long flags; struct fence *fence; @@ -124,7 +127,7 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size) spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); fence_init(fence, &android_fence_ops, &obj->child_list_lock, - obj->context, ++obj->value); + obj->context, value); list_add_tail(&fence->child_list, &obj->child_list_head); INIT_LIST_HEAD(&fence->active_list); spin_unlock_irqrestore(&obj->child_list_lock, flags); @@ -164,12 +167,8 @@ static void android_fence_release(struct fence *fence) static bool android_fence_signaled(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); - int ret; - ret = parent->ops->has_signaled(fence); - if (ret < 0) - fence->status = ret; - return ret; + return (fence->seqno > parent->value) ? false : true; } static bool android_fence_enable_signaling(struct fence *fence) diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index b56885c..627525c 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -28,19 +28,12 @@ struct sync_timeline; /** * struct sync_timeline_ops - sync object implementation ops * @driver_name: name of the implementation - * @has_signaled: returns: - *
[PATCH 00/18] staging/android: clean up SW_SYNC
From: Gustavo Padovan Hi, The following patches do a clean up on the sw_sync inteface. It starts by removing struct sync_timeline_ops, which was creating unecessary wrappers in the code and the start to organize the sync_timeline and sw_sync code better. sw_sync interface was moved to sw_sync.c along with sync_timeline - which is now internal to sw_sync. The next step after this work is the actual de-stage of SW_SYNC and the upstreaming of selftests for sw_sync and sync_file. Please review! Gustavo --- Gustavo Padovan (18): staging/android: store last signaled value on sync timeline staging/android: remove .{fence,timeline}_value_str() from timeline_ops staging/android: remove struct sync_timeline_ops staging/android: remove sw_sync_timeline and sw_sync_pt staging/android: remove sw_sync.[ch] files staging/android: rename android_fence to timeline_fence staging/android: remove unnecessary check for fence staging/android: remove size arg of sync_timeline_create() staging/android: bring struct sync_pt back staging/android: move sw_sync related code to sw_sync.c staging/android: clean up #includes in the sync framework staging/android: make sync_timeline internal to sw_sync staging/android: make sw_ioctl info internal to sw_sync.c staging/android: remove 'destroyed' member from struct sync_timeline staging/android: remove sync_timeline_destroy() staging/android: remove drv_name from sync_timeline staging/android: rename sync.h to sync_debug.h staging/android: add DEBUG_FS dependence on Kconfig drivers/staging/android/Kconfig| 17 +- drivers/staging/android/Makefile | 3 +- drivers/staging/android/sw_sync.c | 341 - drivers/staging/android/sw_sync.h | 59 -- drivers/staging/android/sync.c | 221 - drivers/staging/android/sync.h | 154 --- drivers/staging/android/sync_debug.c | 154 +-- drivers/staging/android/sync_debug.h | 83 drivers/staging/android/trace/sync.h | 14 +- drivers/staging/android/uapi/sw_sync.h | 32 include/linux/fence.h | 2 - 11 files changed, 399 insertions(+), 681 deletions(-) delete mode 100644 drivers/staging/android/sw_sync.h delete mode 100644 drivers/staging/android/sync.c delete mode 100644 drivers/staging/android/sync.h create mode 100644 drivers/staging/android/sync_debug.h delete mode 100644 drivers/staging/android/uapi/sw_sync.h -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/18] staging/android: remove .{fence, timeline}_value_str() from timeline_ops
From: Gustavo Padovan Now that the value of fence and the timeline are not stored by sw_sync anymore we can remove this extra abstraction to retrieve this data. This patch changes both fence_ops (.fence_value_str and .timeline_value_str) to return the str directly. It also clean up struct sync_timeline_ops by removing both ops from there. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c| 17 - drivers/staging/android/sync.c | 16 ++-- drivers/staging/android/sync.h | 9 - drivers/staging/android/sync_debug.c | 12 ++-- drivers/staging/android/trace/sync.h | 12 +++- 5 files changed, 7 insertions(+), 59 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 428e22c..4200b12 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -38,25 +38,8 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) } EXPORT_SYMBOL(sw_sync_pt_create); -static void sw_sync_timeline_value_str(struct sync_timeline *sync_timeline, - char *str, int size) -{ - struct sw_sync_timeline *timeline = - (struct sw_sync_timeline *)sync_timeline; - snprintf(str, size, "%d", timeline->value); -} - -static void sw_sync_fence_value_str(struct fence *fence, char *str, int size) -{ - struct sw_sync_pt *pt = (struct sw_sync_pt *)fence; - - snprintf(str, size, "%d", pt->value); -} - static struct sync_timeline_ops sw_sync_timeline_ops = { .driver_name = "sw_sync", - .timeline_value_str = sw_sync_timeline_value_str, - .fence_value_str = sw_sync_fence_value_str, }; struct sw_sync_timeline *sw_sync_timeline_create(const char *name) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 8dd2181..c75d1e6 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -185,14 +185,7 @@ static bool android_fence_enable_signaling(struct fence *fence) static void android_fence_value_str(struct fence *fence, char *str, int size) { - struct sync_timeline *parent = fence_parent(fence); - - if (!parent->ops->fence_value_str) { - if (size) - *str = 0; - return; - } - parent->ops->fence_value_str(fence, str, size); + snprintf(str, size, "%d", fence->seqno); } static void android_fence_timeline_value_str(struct fence *fence, @@ -200,12 +193,7 @@ static void android_fence_timeline_value_str(struct fence *fence, { struct sync_timeline *parent = fence_parent(fence); - if (!parent->ops->timeline_value_str) { - if (size) - *str = 0; - return; - } - parent->ops->timeline_value_str(parent, str, size); + snprintf(str, size, "%d", parent->value); } static const struct fence_ops android_fence_ops = { diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 627525c..29f8c19 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -28,18 +28,9 @@ struct sync_timeline; /** * struct sync_timeline_ops - sync object implementation ops * @driver_name: name of the implementation - * @timeline_value_str: fill str with the value of the sync_timeline's counter - * @fence_value_str: fill str with the value of the fence */ struct sync_timeline_ops { const char *driver_name; - - /* optional */ - void (*timeline_value_str)(struct sync_timeline *timeline, char *str, - int size); - - /* optional */ - void (*fence_value_str)(struct fence *fence, char *str, int size); }; /** diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 5f57499..c532457 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -133,16 +133,8 @@ static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj) struct list_head *pos; unsigned long flags; - seq_printf(s, "%s %s", obj->name, obj->ops->driver_name); - - if (obj->ops->timeline_value_str) { - char value[64]; - - obj->ops->timeline_value_str(obj, value, sizeof(value)); - seq_printf(s, ": %s", value); - } - - seq_puts(s, "\n"); + seq_printf(s, "%s %s: %d\n", obj->name, obj->ops->driver_name, + obj->value); spin_lock_irqsave(&obj->child_list_lock, flags); list_for_each(pos, &obj->child_list_head) { diff --git a/drivers/staging/android/trace/sync.h b/drivers/staging/andro
[PATCH 04/18] staging/android: remove sw_sync_timeline and sw_sync_pt
From: Gustavo Padovan As we moved value storage to sync_timeline and fence those two structs became useless and can be removed now. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sw_sync.c| 24 +++- drivers/staging/android/sw_sync.h| 24 ++-- drivers/staging/android/sync_debug.c | 12 ++-- 3 files changed, 19 insertions(+), 41 deletions(-) diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index c5e92c6..461dbd9 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -25,31 +25,21 @@ #include "sw_sync.h" -struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) +struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value) { - struct sw_sync_pt *pt; - - pt = (struct sw_sync_pt *) - sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt), value); - - pt->value = value; - - return (struct fence *)pt; + return sync_pt_create(obj, sizeof(struct fence), value); } EXPORT_SYMBOL(sw_sync_pt_create); -struct sw_sync_timeline *sw_sync_timeline_create(const char *name) +struct sync_timeline *sw_sync_timeline_create(const char *name) { - struct sw_sync_timeline *obj = (struct sw_sync_timeline *) - sync_timeline_create(sizeof(struct sw_sync_timeline), -"sw_sync", name); - - return obj; + return sync_timeline_create(sizeof(struct sync_timeline), + "sw_sync", name); } EXPORT_SYMBOL(sw_sync_timeline_create); -void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc) +void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc) { - sync_timeline_signal(&obj->obj, inc); + sync_timeline_signal(obj, inc); } EXPORT_SYMBOL(sw_sync_timeline_inc); diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h index e18667b..9f26c62 100644 --- a/drivers/staging/android/sw_sync.h +++ b/drivers/staging/android/sw_sync.h @@ -22,34 +22,22 @@ #include "sync.h" #include "uapi/sw_sync.h" -struct sw_sync_timeline { - struct sync_timeline obj; - - u32 value; -}; - -struct sw_sync_pt { - struct fencept; - - u32 value; -}; - #if IS_ENABLED(CONFIG_SW_SYNC) -struct sw_sync_timeline *sw_sync_timeline_create(const char *name); -void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc); +struct sync_timeline *sw_sync_timeline_create(const char *name); +void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc); -struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value); +struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value); #else -static inline struct sw_sync_timeline *sw_sync_timeline_create(const char *name) +static inline struct sync_timeline *sw_sync_timeline_create(const char *name) { return NULL; } -static inline void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc) +static inline void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc) { } -static inline struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, +static inline struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value) { return NULL; diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index e5634f2..e207a4d 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -209,7 +209,7 @@ static const struct file_operations sync_info_debugfs_fops = { /* opening sw_sync create a new sync obj */ static int sw_sync_debugfs_open(struct inode *inode, struct file *file) { - struct sw_sync_timeline *obj; + struct sync_timeline *obj; char task_comm[TASK_COMM_LEN]; get_task_comm(task_comm, current); @@ -225,13 +225,13 @@ static int sw_sync_debugfs_open(struct inode *inode, struct file *file) static int sw_sync_debugfs_release(struct inode *inode, struct file *file) { - struct sw_sync_timeline *obj = file->private_data; + struct sync_timeline *obj = file->private_data; - sync_timeline_destroy(&obj->obj); + sync_timeline_destroy(obj); return 0; } -static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj, +static long sw_sync_ioctl_create_fence(struct sync_timeline *obj, unsigned long arg) { int fd = get_unused_fd_flags(O_CLOEXEC); @@ -277,7 +277,7 @@ err: return err; } -static long sw_sync_ioctl_inc(struct sw_sync_timeline *obj, unsigned long arg) +static long sw_sync_ioctl_inc(struct sync_timeline *obj, unsigned long arg) { u32 value; @@ -292,7 +292,7 @@ static long sw_sync_ioctl_inc(struct sw_sync
[PATCH 06/18] staging/android: rename android_fence to timeline_fence
From: Gustavo Padovan We are moving out of staging/android so rename it to a name that is not related to android anymore. Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index b3efcaa..442d808 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -28,7 +28,7 @@ #define CREATE_TRACE_POINTS #include "trace/sync.h" -static const struct fence_ops android_fence_ops; +static const struct fence_ops timeline_fence_ops; struct sync_timeline *sync_timeline_create(int size, const char *drv_name, const char *name) @@ -126,7 +126,7 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size, spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); - fence_init(fence, &android_fence_ops, &obj->child_list_lock, + fence_init(fence, &timeline_fence_ops, &obj->child_list_lock, obj->context, value); list_add_tail(&fence->child_list, &obj->child_list_head); INIT_LIST_HEAD(&fence->active_list); @@ -135,21 +135,21 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size, } EXPORT_SYMBOL(sync_pt_create); -static const char *android_fence_get_driver_name(struct fence *fence) +static const char *timeline_fence_get_driver_name(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); return parent->drv_name; } -static const char *android_fence_get_timeline_name(struct fence *fence) +static const char *timeline_fence_get_timeline_name(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); return parent->name; } -static void android_fence_release(struct fence *fence) +static void timeline_fence_release(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); unsigned long flags; @@ -164,31 +164,31 @@ static void android_fence_release(struct fence *fence) fence_free(fence); } -static bool android_fence_signaled(struct fence *fence) +static bool timeline_fence_signaled(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); return (fence->seqno > parent->value) ? false : true; } -static bool android_fence_enable_signaling(struct fence *fence) +static bool timeline_fence_enable_signaling(struct fence *fence) { struct sync_timeline *parent = fence_parent(fence); - if (android_fence_signaled(fence)) + if (timeline_fence_signaled(fence)) return false; list_add_tail(&fence->active_list, &parent->active_list_head); return true; } -static void android_fence_value_str(struct fence *fence, +static void timeline_fence_value_str(struct fence *fence, char *str, int size) { snprintf(str, size, "%d", fence->seqno); } -static void android_fence_timeline_value_str(struct fence *fence, +static void timeline_fence_timeline_value_str(struct fence *fence, char *str, int size) { struct sync_timeline *parent = fence_parent(fence); @@ -196,13 +196,13 @@ static void android_fence_timeline_value_str(struct fence *fence, snprintf(str, size, "%d", parent->value); } -static const struct fence_ops android_fence_ops = { - .get_driver_name = android_fence_get_driver_name, - .get_timeline_name = android_fence_get_timeline_name, - .enable_signaling = android_fence_enable_signaling, - .signaled = android_fence_signaled, +static const struct fence_ops timeline_fence_ops = { + .get_driver_name = timeline_fence_get_driver_name, + .get_timeline_name = timeline_fence_get_timeline_name, + .enable_signaling = timeline_fence_enable_signaling, + .signaled = timeline_fence_signaled, .wait = fence_default_wait, - .release = android_fence_release, - .fence_value_str = android_fence_value_str, - .timeline_value_str = android_fence_timeline_value_str, + .release = timeline_fence_release, + .fence_value_str = timeline_fence_value_str, + .timeline_value_str = timeline_fence_timeline_value_str, }; -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/18] staging/android: remove sw_sync.[ch] files
From: Gustavo Padovan We can glue the sw_sync file operations directly on the sync framework without the need to pass through sw_sync wrappers. It only builds sw_sync debugfs file support if CONFIG_SW_SYNC is enabled. Signed-off-by: Gustavo Padovan --- drivers/staging/android/Makefile | 1 - drivers/staging/android/sw_sync.c| 45 -- drivers/staging/android/sw_sync.h| 47 drivers/staging/android/sync_debug.c | 17 ++--- 4 files changed, 13 insertions(+), 97 deletions(-) delete mode 100644 drivers/staging/android/sw_sync.c delete mode 100644 drivers/staging/android/sw_sync.h diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index 980d6dc..bf45967 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -5,4 +5,3 @@ obj-y += ion/ obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)+= lowmemorykiller.o obj-$(CONFIG_SYNC) += sync.o sync_debug.o -obj-$(CONFIG_SW_SYNC) += sw_sync.o diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c deleted file mode 100644 index 461dbd9..000 --- a/drivers/staging/android/sw_sync.c +++ /dev/null @@ -1,45 +0,0 @@ -/* - * drivers/base/sw_sync.c - * - * Copyright (C) 2012 Google, Inc. - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include "sw_sync.h" - -struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value) -{ - return sync_pt_create(obj, sizeof(struct fence), value); -} -EXPORT_SYMBOL(sw_sync_pt_create); - -struct sync_timeline *sw_sync_timeline_create(const char *name) -{ - return sync_timeline_create(sizeof(struct sync_timeline), - "sw_sync", name); -} -EXPORT_SYMBOL(sw_sync_timeline_create); - -void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc) -{ - sync_timeline_signal(obj, inc); -} -EXPORT_SYMBOL(sw_sync_timeline_inc); diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h deleted file mode 100644 index 9f26c62..000 --- a/drivers/staging/android/sw_sync.h +++ /dev/null @@ -1,47 +0,0 @@ -/* - * include/linux/sw_sync.h - * - * Copyright (C) 2012 Google, Inc. - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - */ - -#ifndef _LINUX_SW_SYNC_H -#define _LINUX_SW_SYNC_H - -#include -#include -#include "sync.h" -#include "uapi/sw_sync.h" - -#if IS_ENABLED(CONFIG_SW_SYNC) -struct sync_timeline *sw_sync_timeline_create(const char *name); -void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc); - -struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value); -#else -static inline struct sync_timeline *sw_sync_timeline_create(const char *name) -{ - return NULL; -} - -static inline void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc) -{ -} - -static inline struct fence *sw_sync_pt_create(struct sync_timeline *obj, - u32 value) -{ - return NULL; -} -#endif /* IS_ENABLED(CONFIG_SW_SYNC) */ - -#endif /* _LINUX_SW_SYNC_H */ diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index e207a4d..dc85d5f 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -27,7 +27,11 @@ #include #include #include -#include "sw_sync.h" +#include +#include + +#include "uapi/sw_sync.h" +#include "sync.h" #ifdef CONFIG_DEBUG_FS @@ -200,6 +204,7 @@ static const struct file_operations sync_info_debugfs_fops = { .release= single_release, }; +#if IS_ENABLED(CONFIG_SW_SYNC) /* * *WARNING* * @@ -214,7 +219,7 @@ static int sw_sync_debugfs_open(struct inode *inode, struct file *file) get_task_comm(task_comm, current); - obj = sw_sync_timeline_create(task_comm); + obj = sync