On Wed, Apr 26, 2017 at 01:28:29PM +1000, Dave Airlie wrote:
> +#include <drm/drmP.h>
I wonder if Daniel has already split everything used here into its own
headers?
> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/anon_inodes.h>
> +
> +#include "drm_internal.h"
> +#include <drm/drm_syncobj.h>
> +
> +static struct drm_syncobj *drm_syncobj_get(struct drm_file *file_private,
> + u32 handle)
I would like this exposed to the driver as well, so I can do handle to
syncobj conversion once. (drm_syncobj_find() if we go with kref_get style
naming.)
> +static struct drm_syncobj *drm_syncobj_fdget(int fd)
It will aslo be useful to export the fd -> syncobj function for drivers. In
the interface Jason has for execbuf, we can substitute the handle for an
fd + flag, which may be more convenient for the user.
> +static int drm_syncobj_handle_to_fd(struct drm_file *file_private,
> + u32 handle, int *p_fd)
> +{
> + struct drm_syncobj *syncobj = drm_syncobj_get(file_private, handle);
> + int ret;
> + int fd;
> +
> + if (!syncobj)
> + return -EINVAL;
> +
> + fd = get_unused_fd_flags(O_CLOEXEC);
> + if (fd < 0) {
> + drm_syncobj_unreference(syncobj);
> + return fd;
> + }
> +
> + mutex_lock(&syncobj->file_lock);
> + if (!syncobj->file) {
Mutex only being used to ensure syncobj->file is set once? Is the race
likely? If not, we can spare the mutex and just use a try instead:
if (!syncobj->file) {
struct file *file;
file = anon_inode_getfile("syncobj_file",
&drm_syncobj_file_fops,
syncobj, 0);
if (IS_ERR(file))
return PTR_ERR(file);
drm_syncobj_get(syncobj);
if (cmpxchg(&syncobj->file, NULL, file) {
/* lost the race */
fput(file);
}
}
> + fd_install(fd, syncobj->file);
> + mutex_unlock(&syncobj->file_lock);
> + drm_syncobj_unreference(syncobj);
> + *p_fd = fd;
> + return 0;
> +out_unlock:
> + put_unused_fd(fd);
> + mutex_unlock(&syncobj->file_lock);
> + drm_syncobj_unreference(syncobj);
> + return ret;
> +}
> +
> +/**
> + * drm_gem_object_reference - acquire a GEM BO reference
> + * @obj: GEM buffer object
> + *
> + * This acquires additional reference to @obj. It is illegal to call this
> + * without already holding a reference. No locks required.
> + */
> +static inline void
> +drm_syncobj_reference(struct drm_syncobj *obj)
> +{
> + kref_get(&obj->refcount);
We've been steadily converting to the kref_get style of nomenclature for
drm object reference handling (i.e. drm_syncobj_get, drm_syncobj_put)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
