| Message ID | 1432223910-7942-1-git-send-email-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 21, 2015 at 11:58:30AM -0400, Rob Clark wrote: > For actual sharing of buffers with other drivers (ie. actual hardware) > we'll need to pimp things out a bit better to deal w/ caching, multiple > memory domains, etc. See thread: > > http://lists.freedesktop.org/archives/dri-devel/2015-May/083160.html > > But for the llvmpipe use-case this isn't a problem. Nor do we really > need prime/dri3 (dri2 is sufficient). So until the other issues are > sorted lets remove DRIVER_PRIME. > > v2: also drop the dead code > > Signed-off-by: Rob Clark <robdclark@gmail.com> Yeah I'm ok with this too, but it pretty much means vgem is a thing for llvmpipe only. And then abusing dumb buffers is imo ok too, since I don't care too much what kind of horrors drivers pull of internally. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/vgem/Makefile | 2 +- > drivers/gpu/drm/vgem/vgem_dma_buf.c | 94 ------------------------------------- > drivers/gpu/drm/vgem/vgem_drv.c | 11 +---- > drivers/gpu/drm/vgem/vgem_drv.h | 11 ----- > 4 files changed, 2 insertions(+), 116 deletions(-) > delete mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c > > diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile > index 1055cb7..3f4c7b8 100644 > --- a/drivers/gpu/drm/vgem/Makefile > +++ b/drivers/gpu/drm/vgem/Makefile > @@ -1,4 +1,4 @@ > ccflags-y := -Iinclude/drm > -vgem-y := vgem_drv.o vgem_dma_buf.o > +vgem-y := vgem_drv.o > > obj-$(CONFIG_DRM_VGEM) += vgem.o > diff --git a/drivers/gpu/drm/vgem/vgem_dma_buf.c b/drivers/gpu/drm/vgem/vgem_dma_buf.c > deleted file mode 100644 > index 0254438..0000000 > --- a/drivers/gpu/drm/vgem/vgem_dma_buf.c > +++ /dev/null > @@ -1,94 +0,0 @@ > -/* > - * Copyright © 2012 Intel Corporation > - * Copyright © 2014 The Chromium OS Authors > - * > - * Permission is hereby granted, free of charge, to any person obtaining a > - * copy of this software and associated documentation files (the "Software"), > - * to deal in the Software without restriction, including without limitation > - * the rights to use, copy, modify, merge, publish, distribute, sublicense, > - * and/or sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following conditions: > - * > - * The above copyright notice and this permission notice (including the next > - * paragraph) shall be included in all copies or substantial portions of the > - * Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > - * IN THE SOFTWARE. > - * > - * Authors: > - * Ben Widawsky <ben@bwidawsk.net> > - * > - */ > - > -#include <linux/dma-buf.h> > -#include "vgem_drv.h" > - > -struct sg_table *vgem_gem_prime_get_sg_table(struct drm_gem_object *gobj) > -{ > - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); > - BUG_ON(obj->pages == NULL); > - > - return drm_prime_pages_to_sg(obj->pages, obj->base.size / PAGE_SIZE); > -} > - > -int vgem_gem_prime_pin(struct drm_gem_object *gobj) > -{ > - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); > - return vgem_gem_get_pages(obj); > -} > - > -void vgem_gem_prime_unpin(struct drm_gem_object *gobj) > -{ > - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); > - vgem_gem_put_pages(obj); > -} > - > -void *vgem_gem_prime_vmap(struct drm_gem_object *gobj) > -{ > - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); > - BUG_ON(obj->pages == NULL); > - > - return vmap(obj->pages, obj->base.size / PAGE_SIZE, 0, PAGE_KERNEL); > -} > - > -void vgem_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr) > -{ > - vunmap(vaddr); > -} > - > -struct drm_gem_object *vgem_gem_prime_import(struct drm_device *dev, > - struct dma_buf *dma_buf) > -{ > - struct drm_vgem_gem_object *obj = NULL; > - int ret; > - > - obj = kzalloc(sizeof(*obj), GFP_KERNEL); > - if (obj == NULL) { > - ret = -ENOMEM; > - goto fail; > - } > - > - ret = drm_gem_object_init(dev, &obj->base, dma_buf->size); > - if (ret) { > - ret = -ENOMEM; > - goto fail_free; > - } > - > - get_dma_buf(dma_buf); > - > - obj->base.dma_buf = dma_buf; > - obj->use_dma_buf = true; > - > - return &obj->base; > - > -fail_free: > - kfree(obj); > -fail: > - return ERR_PTR(ret); > -} > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c > index cb3b435..7a207ca 100644 > --- a/drivers/gpu/drm/vgem/vgem_drv.c > +++ b/drivers/gpu/drm/vgem/vgem_drv.c > @@ -302,22 +302,13 @@ static const struct file_operations vgem_driver_fops = { > }; > > static struct drm_driver vgem_driver = { > - .driver_features = DRIVER_GEM | DRIVER_PRIME, > + .driver_features = DRIVER_GEM, > .gem_free_object = vgem_gem_free_object, > .gem_vm_ops = &vgem_gem_vm_ops, > .ioctls = vgem_ioctls, > .fops = &vgem_driver_fops, > .dumb_create = vgem_gem_dumb_create, > .dumb_map_offset = vgem_gem_dumb_map, > - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_export = drm_gem_prime_export, > - .gem_prime_import = vgem_gem_prime_import, > - .gem_prime_pin = vgem_gem_prime_pin, > - .gem_prime_unpin = vgem_gem_prime_unpin, > - .gem_prime_get_sg_table = vgem_gem_prime_get_sg_table, > - .gem_prime_vmap = vgem_gem_prime_vmap, > - .gem_prime_vunmap = vgem_gem_prime_vunmap, > .name = DRIVER_NAME, > .desc = DRIVER_DESC, > .date = DRIVER_DATE, > diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h > index 57ab4d8..e9f92f7 100644 > --- a/drivers/gpu/drm/vgem/vgem_drv.h > +++ b/drivers/gpu/drm/vgem/vgem_drv.h > @@ -43,15 +43,4 @@ struct drm_vgem_gem_object { > extern void vgem_gem_put_pages(struct drm_vgem_gem_object *obj); > extern int vgem_gem_get_pages(struct drm_vgem_gem_object *obj); > > -/* vgem_dma_buf.c */ > -extern struct sg_table *vgem_gem_prime_get_sg_table( > - struct drm_gem_object *gobj); > -extern int vgem_gem_prime_pin(struct drm_gem_object *gobj); > -extern void vgem_gem_prime_unpin(struct drm_gem_object *gobj); > -extern void *vgem_gem_prime_vmap(struct drm_gem_object *gobj); > -extern void vgem_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr); > -extern struct drm_gem_object *vgem_gem_prime_import(struct drm_device *dev, > - struct dma_buf *dma_buf); > - > - > #endif > -- > 2.4.1 >
On Thu, May 21, 2015 at 06:07:30PM +0200, Daniel Vetter wrote: > On Thu, May 21, 2015 at 11:58:30AM -0400, Rob Clark wrote: > > For actual sharing of buffers with other drivers (ie. actual hardware) > > we'll need to pimp things out a bit better to deal w/ caching, multiple > > memory domains, etc. See thread: > > > > http://lists.freedesktop.org/archives/dri-devel/2015-May/083160.html > > > > But for the llvmpipe use-case this isn't a problem. Nor do we really > > need prime/dri3 (dri2 is sufficient). So until the other issues are > > sorted lets remove DRIVER_PRIME. > > > > v2: also drop the dead code > > > > Signed-off-by: Rob Clark <robdclark@gmail.com> > > Yeah I'm ok with this too, but it pretty much means vgem is a thing for > llvmpipe only. And then abusing dumb buffers is imo ok too, since I don't > care too much what kind of horrors drivers pull of internally. Ugh. But llvmpipe + dumb buffer is much inferior to SHM. -Chris
On Thu, May 21, 2015 at 12:10 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Thu, May 21, 2015 at 06:07:30PM +0200, Daniel Vetter wrote: >> On Thu, May 21, 2015 at 11:58:30AM -0400, Rob Clark wrote: >> > For actual sharing of buffers with other drivers (ie. actual hardware) >> > we'll need to pimp things out a bit better to deal w/ caching, multiple >> > memory domains, etc. See thread: >> > >> > http://lists.freedesktop.org/archives/dri-devel/2015-May/083160.html >> > >> > But for the llvmpipe use-case this isn't a problem. Nor do we really >> > need prime/dri3 (dri2 is sufficient). So until the other issues are >> > sorted lets remove DRIVER_PRIME. >> > >> > v2: also drop the dead code >> > >> > Signed-off-by: Rob Clark <robdclark@gmail.com> >> >> Yeah I'm ok with this too, but it pretty much means vgem is a thing for >> llvmpipe only. And then abusing dumb buffers is imo ok too, since I don't >> care too much what kind of horrors drivers pull of internally. > > Ugh. But llvmpipe + dumb buffer is much inferior to SHM. it is using shmem for it's dumb buffers.. although I noticed it was mapping writecombine which is probably not what is wanted.. BR, -R > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
On 05/21/2015 06:07 PM, Daniel Vetter wrote: > On Thu, May 21, 2015 at 11:58:30AM -0400, Rob Clark wrote: >> For actual sharing of buffers with other drivers (ie. actual hardware) >> we'll need to pimp things out a bit better to deal w/ caching, multiple >> memory domains, etc. See thread: >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_archives_dri-2Ddevel_2015-2DMay_083160.html&d=AwIDAw&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=5PY5xMHJsYyknpy_IWAegxqACOaf4x60CKosyG0oxCw&s=ciVeRE10840cewxQSgbJ_4YfCH6tU8A1o58k0tHgkD8&e= >> >> But for the llvmpipe use-case this isn't a problem. Nor do we really >> need prime/dri3 (dri2 is sufficient). So until the other issues are >> sorted lets remove DRIVER_PRIME. >> >> v2: also drop the dead code >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> > Yeah I'm ok with this too, but it pretty much means vgem is a thing for > llvmpipe only. And then abusing dumb buffers is imo ok too, since I don't > care too much what kind of horrors drivers pull of internally. > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Well in the end, I'm happy whatever way as long as the root problem is solved. Acked-by: Thomas Hellstrom <thellstrom@vmware.com>
On 22 May 2015 at 02:54, Thomas Hellstrom <thellstrom@vmware.com> wrote: > On 05/21/2015 06:07 PM, Daniel Vetter wrote: >> On Thu, May 21, 2015 at 11:58:30AM -0400, Rob Clark wrote: >>> For actual sharing of buffers with other drivers (ie. actual hardware) >>> we'll need to pimp things out a bit better to deal w/ caching, multiple >>> memory domains, etc. See thread: >>> >>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_archives_dri-2Ddevel_2015-2DMay_083160.html&d=AwIDAw&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=5PY5xMHJsYyknpy_IWAegxqACOaf4x60CKosyG0oxCw&s=ciVeRE10840cewxQSgbJ_4YfCH6tU8A1o58k0tHgkD8&e= >>> >>> But for the llvmpipe use-case this isn't a problem. Nor do we really >>> need prime/dri3 (dri2 is sufficient). So until the other issues are >>> sorted lets remove DRIVER_PRIME. >>> >>> v2: also drop the dead code >>> >>> Signed-off-by: Rob Clark <robdclark@gmail.com> >> Yeah I'm ok with this too, but it pretty much means vgem is a thing for >> llvmpipe only. And then abusing dumb buffers is imo ok too, since I don't >> care too much what kind of horrors drivers pull of internally. >> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> > Well in the end, I'm happy whatever way as long as the root problem is > solved. > > Acked-by: Thomas Hellstrom <thellstrom@vmware.com> > I've applied this, mainly because controversy is bad, so new APIs should never have any, please try and sort out a decent interface on the list, Dave.
diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile index 1055cb7..3f4c7b8 100644 --- a/drivers/gpu/drm/vgem/Makefile +++ b/drivers/gpu/drm/vgem/Makefile @@ -1,4 +1,4 @@ ccflags-y := -Iinclude/drm -vgem-y := vgem_drv.o vgem_dma_buf.o +vgem-y := vgem_drv.o obj-$(CONFIG_DRM_VGEM) += vgem.o diff --git a/drivers/gpu/drm/vgem/vgem_dma_buf.c b/drivers/gpu/drm/vgem/vgem_dma_buf.c deleted file mode 100644 index 0254438..0000000 --- a/drivers/gpu/drm/vgem/vgem_dma_buf.c +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright © 2012 Intel Corporation - * Copyright © 2014 The Chromium OS Authors - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - * - * Authors: - * Ben Widawsky <ben@bwidawsk.net> - * - */ - -#include <linux/dma-buf.h> -#include "vgem_drv.h" - -struct sg_table *vgem_gem_prime_get_sg_table(struct drm_gem_object *gobj) -{ - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); - BUG_ON(obj->pages == NULL); - - return drm_prime_pages_to_sg(obj->pages, obj->base.size / PAGE_SIZE); -} - -int vgem_gem_prime_pin(struct drm_gem_object *gobj) -{ - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); - return vgem_gem_get_pages(obj); -} - -void vgem_gem_prime_unpin(struct drm_gem_object *gobj) -{ - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); - vgem_gem_put_pages(obj); -} - -void *vgem_gem_prime_vmap(struct drm_gem_object *gobj) -{ - struct drm_vgem_gem_object *obj = to_vgem_bo(gobj); - BUG_ON(obj->pages == NULL); - - return vmap(obj->pages, obj->base.size / PAGE_SIZE, 0, PAGE_KERNEL); -} - -void vgem_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr) -{ - vunmap(vaddr); -} - -struct drm_gem_object *vgem_gem_prime_import(struct drm_device *dev, - struct dma_buf *dma_buf) -{ - struct drm_vgem_gem_object *obj = NULL; - int ret; - - obj = kzalloc(sizeof(*obj), GFP_KERNEL); - if (obj == NULL) { - ret = -ENOMEM; - goto fail; - } - - ret = drm_gem_object_init(dev, &obj->base, dma_buf->size); - if (ret) { - ret = -ENOMEM; - goto fail_free; - } - - get_dma_buf(dma_buf); - - obj->base.dma_buf = dma_buf; - obj->use_dma_buf = true; - - return &obj->base; - -fail_free: - kfree(obj); -fail: - return ERR_PTR(ret); -} diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c index cb3b435..7a207ca 100644 --- a/drivers/gpu/drm/vgem/vgem_drv.c +++ b/drivers/gpu/drm/vgem/vgem_drv.c @@ -302,22 +302,13 @@ static const struct file_operations vgem_driver_fops = { }; static struct drm_driver vgem_driver = { - .driver_features = DRIVER_GEM | DRIVER_PRIME, + .driver_features = DRIVER_GEM, .gem_free_object = vgem_gem_free_object, .gem_vm_ops = &vgem_gem_vm_ops, .ioctls = vgem_ioctls, .fops = &vgem_driver_fops, .dumb_create = vgem_gem_dumb_create, .dumb_map_offset = vgem_gem_dumb_map, - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_export = drm_gem_prime_export, - .gem_prime_import = vgem_gem_prime_import, - .gem_prime_pin = vgem_gem_prime_pin, - .gem_prime_unpin = vgem_gem_prime_unpin, - .gem_prime_get_sg_table = vgem_gem_prime_get_sg_table, - .gem_prime_vmap = vgem_gem_prime_vmap, - .gem_prime_vunmap = vgem_gem_prime_vunmap, .name = DRIVER_NAME, .desc = DRIVER_DESC, .date = DRIVER_DATE, diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h index 57ab4d8..e9f92f7 100644 --- a/drivers/gpu/drm/vgem/vgem_drv.h +++ b/drivers/gpu/drm/vgem/vgem_drv.h @@ -43,15 +43,4 @@ struct drm_vgem_gem_object { extern void vgem_gem_put_pages(struct drm_vgem_gem_object *obj); extern int vgem_gem_get_pages(struct drm_vgem_gem_object *obj); -/* vgem_dma_buf.c */ -extern struct sg_table *vgem_gem_prime_get_sg_table( - struct drm_gem_object *gobj); -extern int vgem_gem_prime_pin(struct drm_gem_object *gobj); -extern void vgem_gem_prime_unpin(struct drm_gem_object *gobj); -extern void *vgem_gem_prime_vmap(struct drm_gem_object *gobj); -extern void vgem_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr); -extern struct drm_gem_object *vgem_gem_prime_import(struct drm_device *dev, - struct dma_buf *dma_buf); - - #endif
For actual sharing of buffers with other drivers (ie. actual hardware) we'll need to pimp things out a bit better to deal w/ caching, multiple memory domains, etc. See thread: http://lists.freedesktop.org/archives/dri-devel/2015-May/083160.html But for the llvmpipe use-case this isn't a problem. Nor do we really need prime/dri3 (dri2 is sufficient). So until the other issues are sorted lets remove DRIVER_PRIME. v2: also drop the dead code Signed-off-by: Rob Clark <robdclark@gmail.com> --- drivers/gpu/drm/vgem/Makefile | 2 +- drivers/gpu/drm/vgem/vgem_dma_buf.c | 94 ------------------------------------- drivers/gpu/drm/vgem/vgem_drv.c | 11 +---- drivers/gpu/drm/vgem/vgem_drv.h | 11 ----- 4 files changed, 2 insertions(+), 116 deletions(-) delete mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c