On Thu, 10 May 2012 15:11:15 +0200, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> This adds handle->fd and fd->handle support to i915, this is to allow
> for offloading of rendering in one direction and outputs in the other.
>
> v2 from Daniel Vetter:
> - fixup conflicts with the prepare/finish gtt prep work.
> - implement ppgtt binding support.
>
> Note that we have squat i-g-t testcoverage for any of the lifetime and
> access rules dma_buf/prime support brings along. And there are quite a
> few intricate situations here.
>
> Also note that the integration with the existing code is a bit
> hackish, especially around get_gtt_pages and put_gtt_pages. It imo
> would be easier with the prep code from Chris Wilson's unbound series,
> but that is for 3.6.
>
> Also note that I didn't bother to put the new prepare/finish gtt hooks
> to good use by moving the dma_buf_map/unmap_attachment calls in there
> (like we've originally planned for).
>
> Last but not least this patch is only compile-tested, but I've changed
> very little compared to Dave Airlie's version. So there's a decent
> chance v2 on drm-next works as well as v1 on 3.4-rc.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/i915_drv.c | 8 +-
> drivers/gpu/drm/i915/i915_drv.h | 11 +++
> drivers/gpu/drm/i915/i915_gem.c | 12 ++-
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 149 ++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +++-
> 6 files changed, 193 insertions(+), 7 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/i915_gem_dmabuf.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 8b8bbc7..7b7ecb8 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -37,7 +37,8 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
> dvo_ch7017.o \
> dvo_ivch.o \
> dvo_tfp410.o \
> - dvo_sil164.o
> + dvo_sil164.o \
> + i915_gem_dmabuf.o
>
> i915-$(CONFIG_COMPAT) += i915_ioc32.o
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ccfc23..ac13c2c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1034,7 +1034,7 @@ static struct drm_driver driver = {
> */
> .driver_features =
> DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | /* DRIVER_USE_MTRR |*/
> - DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM,
> + DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME,
> .load = i915_driver_load,
> .unload = i915_driver_unload,
> .open = i915_driver_open,
> @@ -1057,6 +1057,12 @@ static struct drm_driver driver = {
> .gem_init_object = i915_gem_init_object,
> .gem_free_object = i915_gem_free_object,
> .gem_vm_ops = &i915_gem_vm_ops,
> +
> + .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> + .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> + .gem_prime_export = i915_gem_prime_export,
> + .gem_prime_import = i915_gem_prime_import,
Maybe a .gem_prime_ops = &i915_gem_prime_ops.
> +
> .dumb_create = i915_gem_dumb_create,
> .dumb_map_offset = i915_gem_mmap_gtt,
> .dumb_destroy = i915_gem_dumb_destroy,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e03a4f8..751f25c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -935,6 +935,8 @@ struct drm_i915_gem_object {
> struct scatterlist *sg_list;
> int num_sg;
>
> + /* prime dma-buf support */
> + struct sg_table *sg_table;
This looks like a kludge which with a little more work could integrate
neatly into the existing sg code without having to add aditional special
cases.
> +struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachment,
> + enum dma_data_direction dir)
> +{
> + struct drm_i915_gem_object *obj = attachment->dmabuf->priv;
> + struct drm_device *dev = obj->base.dev;
> + int npages = obj->base.size / PAGE_SIZE;
> + struct sg_table *sg = NULL;
> + int ret;
> + int nents;
> +
> + ret = i915_mutex_lock_interruptible(dev);
> + if (ret)
> + return NULL;
Not impressed by the lack of error reporting through this interface.
> +struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + struct dma_buf_attachment *attach;
> + struct sg_table *sg;
> + struct drm_i915_gem_object *obj;
> + int npages;
> + int size;
> + int ret;
> +
> + /* is this one of own objects? */
> + if (dma_buf->ops == &i915_dmabuf_ops) {
> + obj = dma_buf->priv;
> + /* is it from our device? */
> + if (obj->base.dev == dev) {
> + drm_gem_object_reference(&obj->base);
> + return &obj->base;
> + }
> + }
> +
> + /* need to attach */
> + attach = dma_buf_attach(dma_buf, dev->dev);
> + if (IS_ERR(attach))
> + return ERR_PTR(-EINVAL);
Return the original error.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
