From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm: base prime/dma-buf support Date: Mon, 26 Mar 2012 19:32:15 +0300 Message-ID: <20120326163214.GI4917@intel.com> References: <1332774175-12716-1-git-send-email-airlied@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 51CD5A0A65 for ; Mon, 26 Mar 2012 09:24:15 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1332774175-12716-1-git-send-email-airlied@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Dave Airlie Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Mon, Mar 26, 2012 at 04:02:55PM +0100, Dave Airlie wrote: > +int drm_gem_prime_handle_to_fd(struct drm_device *dev, > + struct drm_file *file_priv, uint32_t handle, uint32_t flags, > + int *prime_fd) > +{ > + struct drm_gem_object *obj; > + > + obj =3D drm_gem_object_lookup(dev, file_priv, handle); > + if (!obj) > + return -ENOENT; > + > + /* don't allow imported buffers to be re-exported */ > + if (obj->import_attach) { > + drm_gem_object_unreference_unlocked(obj); > + return -EINVAL; > + } > + > + if (obj->export_dma_buf) { > + get_file(obj->export_dma_buf->file); > + *prime_fd =3D dma_buf_fd(obj->export_dma_buf, flags); > + drm_gem_object_unreference_unlocked(obj); > + } else { > + obj->export_dma_buf =3D > + dev->driver->gem_prime_export(dev, obj, flags); > + if (IS_ERR_OR_NULL(obj->export_dma_buf)) { > + /* normally the created dma-buf takes ownership of the ref, > + * but if that fails then drop the ref > + */ > + drm_gem_object_unreference_unlocked(obj); > + return PTR_ERR(obj->export_dma_buf); PTR_ERR(NULL) seems like a bad idea. Also, you're accessing 'obj' after dropping the reference. > + } > + *prime_fd =3D dma_buf_fd(obj->export_dma_buf, flags); > + } > + return 0; > +} > +EXPORT_SYMBOL(drm_gem_prime_handle_to_fd); > + > +int drm_gem_prime_fd_to_handle(struct drm_device *dev, > + struct drm_file *file_priv, int prime_fd, uint32_t *handle) > +{ > + struct dma_buf *dma_buf; > + struct drm_gem_object *obj; > + int ret; > + > + dma_buf =3D dma_buf_get(prime_fd); > + if (IS_ERR(dma_buf)) > + return PTR_ERR(dma_buf); > + > + ret =3D drm_prime_lookup_fd_handle_mapping(&file_priv->prime, > + dma_buf, handle); > + if (!ret) { > + dma_buf_put(dma_buf); > + return 0; > + } > + > + /* never seen this one, need to import */ > + obj =3D dev->driver->gem_prime_import(dev, dma_buf); > + if (IS_ERR_OR_NULL(obj)) { > + ret =3D PTR_ERR(obj); PTR_ERR(NULL) again. > + goto fail_put; > + } > + > + ret =3D drm_gem_handle_create(file_priv, obj, handle); > + drm_gem_object_unreference_unlocked(obj); > + if (ret) > + goto fail_put; > + > + ret =3D drm_prime_insert_fd_handle_mapping(&file_priv->prime, > + dma_buf, *handle); > + if (ret) > + goto fail; > + > + return 0; > + > +fail: > + /* hmm, if driver attached, we are relying on the free-object path > + * to detach.. which seems ok.. > + */ > + drm_gem_object_handle_unreference_unlocked(obj); > +fail_put: > + dma_buf_put(dma_buf); > + return ret; > +} > +EXPORT_SYMBOL(drm_gem_prime_fd_to_handle); > + > +int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file_priv) > +{ > + struct drm_prime_handle *args =3D data; > + uint32_t flags; > + > + if (!drm_core_check_feature(dev, DRIVER_PRIME)) > + return -EINVAL; > + > + if (!dev->driver->prime_handle_to_fd) > + return -ENOSYS; > + > + /* we only want to pass DRM_CLOEXEC which is =3D=3D O_CLOEXEC */ > + flags =3D args->flags & DRM_CLOEXEC; Check that the unused flags are 0? If you allow broken userspace to pass uninitialized 'flags' to the kernel, you may have compatibility problems if/when you want to add more flags. > + > + return dev->driver->prime_handle_to_fd(dev, file_priv, > + args->handle, flags, &args->fd); > +} > + > +int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file_priv) > +{ > + struct drm_prime_handle *args =3D data; > + > + if (!drm_core_check_feature(dev, DRIVER_PRIME)) > + return -EINVAL; > + > + if (!dev->driver->prime_fd_to_handle) > + return -ENOSYS; > + > + return dev->driver->prime_fd_to_handle(dev, file_priv, > + args->fd, &args->handle); > +} > + > +/* > + * drm_prime_pages_to_sg > + * > + * this helper creates an sg table object from a set of pages > + * the driver is responsible for mapping the pages into the > + * importers address space > + */ > +struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages) > +{ > + struct sg_table *sg =3D NULL; > + struct scatterlist *iter; > + int i; > + int ret; > + > + sg =3D kzalloc(sizeof(struct sg_table), GFP_KERNEL); kmalloc() would be enough. sg_alloc_table() already does a memset(). > + if (!sg) > + goto out; > + > + ret =3D sg_alloc_table(sg, nr_pages, GFP_KERNEL); > + if (ret) > + goto out; > + > + for_each_sg(sg->sgl, iter, nr_pages, i) > + sg_set_page(iter, pages[i], PAGE_SIZE, 0); > + > + return sg; > +out: > + kfree(sg); > + return NULL; > +} > +EXPORT_SYMBOL(drm_prime_pages_to_sg); -- = Ville Syrj=E4l=E4 Intel OTC