| Message ID | 1454456238-18742-1-git-send-email-zachr@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org> On Tue, Feb 2, 2016 at 3:37 PM, Zach Reizner <zachr@google.com> wrote: > The prime fd to handle ioctl was not used with rockchip before. Support > was added in order to support potential uses (e.g. zero-copy video > decode, camera). > > Signed-off-by: Zach Reizner <zachr@google.com> > --- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 + > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 92 ++++++++++++++++++++++++++--- > drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 7 ++- > 3 files changed, 91 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index dbfec29..4a01d92 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -307,6 +307,7 @@ static struct drm_driver rockchip_drm_driver = { > .gem_prime_import = drm_gem_prime_import, > .gem_prime_export = drm_gem_prime_export, > .gem_prime_get_sg_table = rockchip_gem_prime_get_sg_table, > + .gem_prime_import_sg_table = rockchip_gem_prime_import_sg_table, > .gem_prime_vmap = rockchip_gem_prime_vmap, > .gem_prime_vunmap = rockchip_gem_prime_vunmap, > .gem_prime_mmap = rockchip_gem_mmap_buf, > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index a5c512e..cf6b50b 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -19,6 +19,7 @@ > #include <drm/rockchip_drm.h> > > #include <linux/dma-attrs.h> > +#include <linux/dma-buf.h> > > #include "rockchip_drm_drv.h" > #include "rockchip_drm_gem.h" > @@ -105,13 +106,11 @@ int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > return rockchip_drm_gem_object_mmap(obj, vma); > } > > -struct rockchip_gem_object * > - rockchip_gem_create_object(struct drm_device *drm, unsigned int size, > - bool alloc_kmap) > +static struct rockchip_gem_object * > + rockchip_gem_alloc_object(struct drm_device *drm, unsigned int size) > { > struct rockchip_gem_object *rk_obj; > struct drm_gem_object *obj; > - int ret; > > size = round_up(size, PAGE_SIZE); > > @@ -123,6 +122,20 @@ struct rockchip_gem_object * > > drm_gem_private_object_init(drm, obj, size); > > + return rk_obj; > +} > + > +struct rockchip_gem_object * > + rockchip_gem_create_object(struct drm_device *drm, unsigned int size, > + bool alloc_kmap) > +{ > + struct rockchip_gem_object *rk_obj; > + int ret; > + > + rk_obj = rockchip_gem_alloc_object(drm, size); > + if (IS_ERR(rk_obj)) > + return rk_obj; > + > ret = rockchip_gem_alloc_buf(rk_obj, alloc_kmap); > if (ret) > goto err_free_rk_obj; > @@ -140,13 +153,18 @@ err_free_rk_obj: > */ > void rockchip_gem_free_object(struct drm_gem_object *obj) > { > - struct rockchip_gem_object *rk_obj; > + struct drm_device *drm = obj->dev; > + struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); > > drm_gem_free_mmap_offset(obj); > > - rk_obj = to_rockchip_obj(obj); > - > - rockchip_gem_free_buf(rk_obj); > + if (rk_obj->sg) { > + dma_unmap_sg(drm->dev, rk_obj->sg->sgl, rk_obj->sg->nents, > + DMA_BIDIRECTIONAL); > + drm_prime_gem_destroy(obj, rk_obj->sg); > + } else { > + rockchip_gem_free_buf(rk_obj); > + } > > kfree(rk_obj); > } > @@ -289,6 +307,64 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj) > return sgt; > } > > +static unsigned long rockchip_sg_get_contiguous_size(struct sg_table *sgt, > + int count) > +{ > + struct scatterlist *s; > + dma_addr_t expected = sg_dma_address(sgt->sgl); > + unsigned int i; > + unsigned long size = 0; > + > + for_each_sg(sgt->sgl, s, count, i) { > + if (sg_dma_address(s) != expected) > + break; > + expected = sg_dma_address(s) + sg_dma_len(s); > + size += sg_dma_len(s); > + } > + return size; > +} > + > + > +struct drm_gem_object * > +rockchip_gem_prime_import_sg_table(struct drm_device *drm, > + struct dma_buf_attachment *attach, > + struct sg_table *sg) > +{ > + struct rockchip_gem_object *rk_obj; > + size_t size = attach->dmabuf->size; > + int count; > + int ret; > + > + rk_obj = rockchip_gem_alloc_object(drm, size); > + if (IS_ERR(rk_obj)) > + return ERR_CAST(rk_obj); > + > + count = dma_map_sg(drm->dev, sg->sgl, sg->nents, > + DMA_BIDIRECTIONAL); > + > + if (!count) { > + ret = -EINVAL; > + goto err_free_rk_obj; > + } > + > + if (rockchip_sg_get_contiguous_size(sg, count) < size) { > + DRM_ERROR("failed to map sg_table to contiguous linear address.\n"); > + dma_unmap_sg(drm->dev, sg->sgl, sg->nents, > + DMA_BIDIRECTIONAL); > + ret = -EINVAL; > + goto err_free_rk_obj; > + } > + > + rk_obj->dma_addr = sg_dma_address(sg->sgl); > + rk_obj->sg = sg; > + > + return &rk_obj->base; > + > +err_free_rk_obj: > + kfree(rk_obj); > + return ERR_PTR(ret); > +} > + > void *rockchip_gem_prime_vmap(struct drm_gem_object *obj) > { > struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h > index d5d8010..b6dc3b1 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h > @@ -17,6 +17,8 @@ > > #define to_rockchip_obj(x) container_of(x, struct rockchip_gem_object, base) > > +struct sg_table; > + > struct rockchip_gem_object { > struct drm_gem_object base; > unsigned int flags; > @@ -24,11 +26,14 @@ struct rockchip_gem_object { > void *kvaddr; > dma_addr_t dma_addr; > struct dma_attrs dma_attrs; > + > + struct sg_table *sg; > }; > > struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj); > struct drm_gem_object * > -rockchip_gem_prime_import_sg_table(struct drm_device *dev, size_t size, > +rockchip_gem_prime_import_sg_table(struct drm_device *dev, > + struct dma_buf_attachment *attach, > struct sg_table *sgt); > void *rockchip_gem_prime_vmap(struct drm_gem_object *obj); > void rockchip_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr); > -- > 2.7.0.rc3.207.g0ac5344 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Zach, On 2 February 2016 at 23:37, Zach Reizner <zachr@google.com> wrote: > The prime fd to handle ioctl was not used with rockchip before. Support > was added in order to support potential uses (e.g. zero-copy video > decode, camera). > Similar patch came around a few months ago and got this reply [1]. If the situation has changed (there is an open-source driver/user for this) it should be clearly mentioned in the commit message, as opposed to "in order to support potential uses". Regards, Emil [1] https://lists.freedesktop.org/archives/dri-devel/2015-November/094568.html
On Tue, Feb 23, 2016 at 6:29 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > Hi Zach, > > On 2 February 2016 at 23:37, Zach Reizner <zachr@google.com> wrote: >> The prime fd to handle ioctl was not used with rockchip before. Support >> was added in order to support potential uses (e.g. zero-copy video >> decode, camera). >> > Similar patch came around a few months ago and got this reply [1]. If > the situation has changed (there is an open-source driver/user for > this) it should be clearly mentioned in the commit message, as opposed > to "in order to support potential uses". hmm, well it is not driver specific uabi, and we have let several other mali/img users do prime.. I'm not sure, maybe those platforms can do a basic v4l <-> display thing w/ prime. Although upstream tends to hurt a bit for camera support.. BR, -R > Regards, > Emil > > [1] https://lists.freedesktop.org/archives/dri-devel/2015-November/094568.html > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Feb 23, 2016 at 3:56 PM, Rob Clark <robdclark@gmail.com> wrote: > On Tue, Feb 23, 2016 at 6:29 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> Hi Zach, >> >> On 2 February 2016 at 23:37, Zach Reizner <zachr@google.com> wrote: >>> The prime fd to handle ioctl was not used with rockchip before. Support >>> was added in order to support potential uses (e.g. zero-copy video >>> decode, camera). >>> >> Similar patch came around a few months ago and got this reply [1]. If >> the situation has changed (there is an open-source driver/user for >> this) it should be clearly mentioned in the commit message, as opposed >> to "in order to support potential uses". > > hmm, well it is not driver specific uabi, and we have let several > other mali/img users do prime.. > > I'm not sure, maybe those platforms can do a basic v4l <-> display > thing w/ prime. Although upstream tends to hurt a bit for camera > support.. There used to be vgem, but that was reverted... Stéphane > > BR, > -R > >> Regards, >> Emil >> >> [1] https://lists.freedesktop.org/archives/dri-devel/2015-November/094568.html >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 23 February 2016 at 23:56, Rob Clark <robdclark@gmail.com> wrote: > On Tue, Feb 23, 2016 at 6:29 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> Hi Zach, >> >> On 2 February 2016 at 23:37, Zach Reizner <zachr@google.com> wrote: >>> The prime fd to handle ioctl was not used with rockchip before. Support >>> was added in order to support potential uses (e.g. zero-copy video >>> decode, camera). >>> >> Similar patch came around a few months ago and got this reply [1]. If >> the situation has changed (there is an open-source driver/user for >> this) it should be clearly mentioned in the commit message, as opposed >> to "in order to support potential uses". > > hmm, well it is not driver specific uabi, and we have let several > other mali/img users do prime.. > I'm suspecting that's because we missed the patches fly by, as opposed to people being particularly happy with the idea. If the "don't break userspace" kernel rule did not exist and a few more people to kept an eye open for these misuses [I believe is the correct word], things could easily be fixed/reverted ;-) If the majority feels like we can/should allow these - so be it. I'm just pointing/reminding what was said previously. > I'm not sure, maybe those platforms can do a basic v4l <-> display > thing w/ prime. Although upstream tends to hurt a bit for camera > support.. > It a bit unfortunate indeed. I wonder if companies cannot put more pressure on vendors to get things open/upstream - hint, hint ;-) Wanted: Dead or alive - [upstream] open-source users :-P Regards Emil
Hi, On 24 February 2016 at 16:01, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 23 February 2016 at 23:56, Rob Clark <robdclark@gmail.com> wrote: >> On Tue, Feb 23, 2016 at 6:29 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 2 February 2016 at 23:37, Zach Reizner <zachr@google.com> wrote: >>>> The prime fd to handle ioctl was not used with rockchip before. Support >>>> was added in order to support potential uses (e.g. zero-copy video >>>> decode, camera). >>>> >>> Similar patch came around a few months ago and got this reply [1]. If >>> the situation has changed (there is an open-source driver/user for >>> this) it should be clearly mentioned in the commit message, as opposed >>> to "in order to support potential uses". >>> >> I'm not sure, maybe those platforms can do a basic v4l <-> display >> thing w/ prime. Although upstream tends to hurt a bit for camera >> support.. >> > It a bit unfortunate indeed. I wonder if companies cannot put more > pressure on vendors to get things open/upstream - hint, hint ;-) > > Wanted: Dead or alive - [upstream] open-source users :-P V4L (media decode rather than camera IIRC) is indeed possible; there's a driver in the chromeos-3.18 downstream tree, and there's active work going on to get that included in mainline. So that would be a good user. Cheers, Daniel
On 23 February 2016 at 23:56, Rob Clark <robdclark@gmail.com> wrote: > On Tue, Feb 23, 2016 at 6:29 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> Hi Zach, >> >> On 2 February 2016 at 23:37, Zach Reizner <zachr@google.com> wrote: >>> The prime fd to handle ioctl was not used with rockchip before. Support >>> was added in order to support potential uses (e.g. zero-copy video >>> decode, camera). >>> >> Similar patch came around a few months ago and got this reply [1]. If >> the situation has changed (there is an open-source driver/user for >> this) it should be clearly mentioned in the commit message, as opposed >> to "in order to support potential uses". > > hmm, well it is not driver specific uabi, and we have let several > other mali/img users do prime.. > > I'm not sure, maybe those platforms can do a basic v4l <-> display > thing w/ prime. Although upstream tends to hurt a bit for camera > support.. > Actually... one possible user is vgem. With the series from Tiago "Direct userspace dma-buf mmap" hitting upstream, we can actually reapply the problematic parts of vgem, and get this in as well. Hopefully echoing the slogan "upstream kernel needs users for interfaces", did not come too much for people. If it did, sorry but it's a rule which was set before I came along. Regards, Emil
On 4 March 2016 at 17:45, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 23 February 2016 at 23:56, Rob Clark <robdclark@gmail.com> wrote: >> On Tue, Feb 23, 2016 at 6:29 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> Hi Zach, >>> >>> On 2 February 2016 at 23:37, Zach Reizner <zachr@google.com> wrote: >>>> The prime fd to handle ioctl was not used with rockchip before. Support >>>> was added in order to support potential uses (e.g. zero-copy video >>>> decode, camera). >>>> >>> Similar patch came around a few months ago and got this reply [1]. If >>> the situation has changed (there is an open-source driver/user for >>> this) it should be clearly mentioned in the commit message, as opposed >>> to "in order to support potential uses". >> >> hmm, well it is not driver specific uabi, and we have let several >> other mali/img users do prime.. >> >> I'm not sure, maybe those platforms can do a basic v4l <-> display >> thing w/ prime. Although upstream tends to hurt a bit for camera >> support.. >> > Actually... one possible user is vgem. With the series from Tiago > "Direct userspace dma-buf mmap" hitting upstream, we can actually > reapply the problematic parts of vgem, and get this in as well. > > Hopefully echoing the slogan "upstream kernel needs users for > interfaces", did not come too much for people. > If it did, sorry but it's a rule which was set before I came along. > Gents, just realised that this hasn't landed yet. As mentioned above, my earlier 'rant' is no longer applicable so feel free to check with the Rockchip maintainer and get this upstream. Regards, Emil
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index dbfec29..4a01d92 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -307,6 +307,7 @@ static struct drm_driver rockchip_drm_driver = { .gem_prime_import = drm_gem_prime_import, .gem_prime_export = drm_gem_prime_export, .gem_prime_get_sg_table = rockchip_gem_prime_get_sg_table, + .gem_prime_import_sg_table = rockchip_gem_prime_import_sg_table, .gem_prime_vmap = rockchip_gem_prime_vmap, .gem_prime_vunmap = rockchip_gem_prime_vunmap, .gem_prime_mmap = rockchip_gem_mmap_buf, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index a5c512e..cf6b50b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -19,6 +19,7 @@ #include <drm/rockchip_drm.h> #include <linux/dma-attrs.h> +#include <linux/dma-buf.h> #include "rockchip_drm_drv.h" #include "rockchip_drm_gem.h" @@ -105,13 +106,11 @@ int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) return rockchip_drm_gem_object_mmap(obj, vma); } -struct rockchip_gem_object * - rockchip_gem_create_object(struct drm_device *drm, unsigned int size, - bool alloc_kmap) +static struct rockchip_gem_object * + rockchip_gem_alloc_object(struct drm_device *drm, unsigned int size) { struct rockchip_gem_object *rk_obj; struct drm_gem_object *obj; - int ret; size = round_up(size, PAGE_SIZE); @@ -123,6 +122,20 @@ struct rockchip_gem_object * drm_gem_private_object_init(drm, obj, size); + return rk_obj; +} + +struct rockchip_gem_object * + rockchip_gem_create_object(struct drm_device *drm, unsigned int size, + bool alloc_kmap) +{ + struct rockchip_gem_object *rk_obj; + int ret; + + rk_obj = rockchip_gem_alloc_object(drm, size); + if (IS_ERR(rk_obj)) + return rk_obj; + ret = rockchip_gem_alloc_buf(rk_obj, alloc_kmap); if (ret) goto err_free_rk_obj; @@ -140,13 +153,18 @@ err_free_rk_obj: */ void rockchip_gem_free_object(struct drm_gem_object *obj) { - struct rockchip_gem_object *rk_obj; + struct drm_device *drm = obj->dev; + struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); drm_gem_free_mmap_offset(obj); - rk_obj = to_rockchip_obj(obj); - - rockchip_gem_free_buf(rk_obj); + if (rk_obj->sg) { + dma_unmap_sg(drm->dev, rk_obj->sg->sgl, rk_obj->sg->nents, + DMA_BIDIRECTIONAL); + drm_prime_gem_destroy(obj, rk_obj->sg); + } else { + rockchip_gem_free_buf(rk_obj); + } kfree(rk_obj); } @@ -289,6 +307,64 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj) return sgt; } +static unsigned long rockchip_sg_get_contiguous_size(struct sg_table *sgt, + int count) +{ + struct scatterlist *s; + dma_addr_t expected = sg_dma_address(sgt->sgl); + unsigned int i; + unsigned long size = 0; + + for_each_sg(sgt->sgl, s, count, i) { + if (sg_dma_address(s) != expected) + break; + expected = sg_dma_address(s) + sg_dma_len(s); + size += sg_dma_len(s); + } + return size; +} + + +struct drm_gem_object * +rockchip_gem_prime_import_sg_table(struct drm_device *drm, + struct dma_buf_attachment *attach, + struct sg_table *sg) +{ + struct rockchip_gem_object *rk_obj; + size_t size = attach->dmabuf->size; + int count; + int ret; + + rk_obj = rockchip_gem_alloc_object(drm, size); + if (IS_ERR(rk_obj)) + return ERR_CAST(rk_obj); + + count = dma_map_sg(drm->dev, sg->sgl, sg->nents, + DMA_BIDIRECTIONAL); + + if (!count) { + ret = -EINVAL; + goto err_free_rk_obj; + } + + if (rockchip_sg_get_contiguous_size(sg, count) < size) { + DRM_ERROR("failed to map sg_table to contiguous linear address.\n"); + dma_unmap_sg(drm->dev, sg->sgl, sg->nents, + DMA_BIDIRECTIONAL); + ret = -EINVAL; + goto err_free_rk_obj; + } + + rk_obj->dma_addr = sg_dma_address(sg->sgl); + rk_obj->sg = sg; + + return &rk_obj->base; + +err_free_rk_obj: + kfree(rk_obj); + return ERR_PTR(ret); +} + void *rockchip_gem_prime_vmap(struct drm_gem_object *obj) { struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h index d5d8010..b6dc3b1 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h @@ -17,6 +17,8 @@ #define to_rockchip_obj(x) container_of(x, struct rockchip_gem_object, base) +struct sg_table; + struct rockchip_gem_object { struct drm_gem_object base; unsigned int flags; @@ -24,11 +26,14 @@ struct rockchip_gem_object { void *kvaddr; dma_addr_t dma_addr; struct dma_attrs dma_attrs; + + struct sg_table *sg; }; struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj); struct drm_gem_object * -rockchip_gem_prime_import_sg_table(struct drm_device *dev, size_t size, +rockchip_gem_prime_import_sg_table(struct drm_device *dev, + struct dma_buf_attachment *attach, struct sg_table *sgt); void *rockchip_gem_prime_vmap(struct drm_gem_object *obj); void rockchip_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
The prime fd to handle ioctl was not used with rockchip before. Support was added in order to support potential uses (e.g. zero-copy video decode, camera). Signed-off-by: Zach Reizner <zachr@google.com> --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 + drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 92 ++++++++++++++++++++++++++--- drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 7 ++- 3 files changed, 91 insertions(+), 9 deletions(-)