Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

14 participants
3243 discussions

Re: [Linaro-mm-sig] dynamic DMA-buf sharing between devices

by Christian König

On top of those I have 6 more patches in the pipeline to enable VRAM P2P with DMA-buf. So that is not the end of the patch set :) Christian. Am 17.04.19 um 15:52 schrieb Chunming Zhou: > Thanks Christian, great job. I will verify it this week when I finish my > current work on hand. > > -David > > 在 2019/4/17 2:38, Christian König wrote: >> Hi everybody, >> >> core idea in this patch set is that DMA-buf importers can now provide an optional invalidate callback. Using this callback and the reservation object exporters can now avoid pinning DMA-buf memory for a long time while sharing it between devices. >> >> I've already send out an older version roughly a year ago, but didn't had time to further look into cleaning this up. >> >> The last time a major problem was that we would had to fix up all drivers implementing DMA-buf at once. >> >> Now I avoid this by allowing mappings to be cached in the DMA-buf attachment and so driver can optionally move over to the new interface one by one. >> >> This is also a prerequisite to my patchset enabling sharing of device memory with DMA-buf. >> >> Please review and/or comment, >> Christian. >> >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel(a)lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel

6 years, 8 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use

by Laura Abbott

On 3/29/19 7:26 PM, Zengtao (B) wrote: > Hi laura: > >> -----Original Message----- >> From: Laura Abbott [mailto:labbott@redhat.com] >> Sent: Friday, March 29, 2019 9:27 PM >> To: Zengtao (B) <prime.zeng(a)hisilicon.com>; sumit.semwal(a)linaro.org >> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>; Arve Hjønnevåg >> <arve(a)android.com>; Todd Kjos <tkjos(a)android.com>; Martijn Coenen >> <maco(a)android.com>; Joel Fernandes <joel(a)joelfernandes.org>; >> Christian Brauner <christian(a)brauner.io>; devel(a)driverdev.osuosl.org; >> dri-devel(a)lists.freedesktop.org; linaro-mm-sig(a)lists.linaro.org; >> linux-kernel(a)vger.kernel.org >> Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel >> driver use >> >> On 3/29/19 11:40 AM, Zeng Tao wrote: >>> There are two reasons for this patch: >>> 1. There are some potential requirements for ion_alloc in kernel >>> space, some media drivers need to allocate media buffers from ion >>> instead of buddy or dma framework, this is more convient and clean >>> very for media drivers. And In that case, ion is the only media buffer >>> provider, it's more easier to maintain. >>> 2. Fd is only needed by user processes, not the kernel space, so >>> dma_buf should be returned instead of fd for kernel space, and >>> dma_buf_fd should be called only for userspace api. >>> >> >> I really want to just NAK this because it doesn't seem like something >> that's necessary. The purpose of Ion is to provide buffers to userspace >> because there's no other way for userspace to get access to the memory. >> The kernel already has other APIs to access the memory. This also >> complicates the re-work that's been happening where the requirement is >> only userspace. >> >> Can you be more detailed about which media drivers you are referring to >> and why they can't just use other APIs? >> > > I think I 've got your point, the ION is designed for usespace, but for kernel > space, we are really lacking of someone which plays the same role,(allocate > media memory, share the memory using dma_buf, provide debug and statistics > for media memory). > > In fact, for kernel space, we have the dma framework, dma-buf, etc.. > And we can work on top of such apis, but some duplicate jobs(everyone has > to maintain its own buffer sharing, debug and statistics). > So we need to have some to do the common things(ION's the best choice now) > Keep in mind that Ion is a thin shell of what it was as most of the debugging and statistics was removed because it was buggy. Most of that should end up going at the dma_buf layer since it's really a dma_buf allocation API. > When the ION was introduced, a lot of media memory frameworks existed, the > dma framework was not so good, so ION heaps, integrated buffer sharing, statistics > and usespace api were the required features, but now dma framework is more powerful, > we don't even need ION heaps now, but the userspace api, buffer sharing, statistics are > still needed, and the buffer sharing, statistics can be re-worked and export to kernel space, > not only used by userspace, , and that is my point. > I see what you are getting at but I don't think the same thing applies to the kernel as it does userspace. We can enforce a single way of using the dma_buf fd in userspace but the kernel has a variety of ways to use dma_buf because each driver and framework has its own needs. I'm still not convinced that adding Ion APIs in the kernel is the right option since as you point out we don't really need the heaps. That mostly leaves Ion as a wrapper to handle doing the export. Maybe we could benefit from that but I think it might require more thought. I'd rather see a proposal in the media API itself showing what you think is necessary but without using Ion. That would be a good start so we could fully review what might make sense to pull out of Ion into something common. Thanks, Laura >> >>> Signed-off-by: Zeng Tao <prime.zeng(a)hisilicon.com> >>> --- >>> drivers/staging/android/ion/ion.c | 32 >> +++++++++++++++++--------------- >>> 1 file changed, 17 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/staging/android/ion/ion.c >>> b/drivers/staging/android/ion/ion.c >>> index 92c2914..e93fb49 100644 >>> --- a/drivers/staging/android/ion/ion.c >>> +++ b/drivers/staging/android/ion/ion.c >>> @@ -387,13 +387,13 @@ static const struct dma_buf_ops >> dma_buf_ops = { >>> .unmap = ion_dma_buf_kunmap, >>> }; >>> >>> -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned >>> int flags) >>> +struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask, >>> + unsigned int flags) >>> { >>> struct ion_device *dev = internal_dev; >>> struct ion_buffer *buffer = NULL; >>> struct ion_heap *heap; >>> DEFINE_DMA_BUF_EXPORT_INFO(exp_info); >>> - int fd; >>> struct dma_buf *dmabuf; >>> >>> pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__, >> @@ >>> -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int >> heap_id_mask, unsigned int flags) >>> len = PAGE_ALIGN(len); >>> >>> if (!len) >>> - return -EINVAL; >>> + return ERR_PTR(-EINVAL); >>> >>> down_read(&dev->lock); >>> plist_for_each_entry(heap, &dev->heaps, node) { @@ -421,10 >> +421,10 >>> @@ static int ion_alloc(size_t len, unsigned int heap_id_mask, >> unsigned int flags) >>> up_read(&dev->lock); >>> >>> if (!buffer) >>> - return -ENODEV; >>> + return ERR_PTR(-ENODEV); >>> >>> if (IS_ERR(buffer)) >>> - return PTR_ERR(buffer); >>> + return ERR_PTR(PTR_ERR(buffer)); >>> >>> exp_info.ops = &dma_buf_ops; >>> exp_info.size = buffer->size; >>> @@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int >> heap_id_mask, unsigned int flags) >>> exp_info.priv = buffer; >>> >>> dmabuf = dma_buf_export(&exp_info); >>> - if (IS_ERR(dmabuf)) { >>> + if (IS_ERR(dmabuf)) >>> _ion_buffer_destroy(buffer); >>> - return PTR_ERR(dmabuf); >>> - } >>> >>> - fd = dma_buf_fd(dmabuf, O_CLOEXEC); >>> - if (fd < 0) >>> - dma_buf_put(dmabuf); >>> - >>> - return fd; >>> + return dmabuf; >>> } >>> +EXPORT_SYMBOL(ion_alloc); >>> >>> static int ion_query_heaps(struct ion_heap_query *query) >>> { >>> @@ -539,12 +534,19 @@ static long ion_ioctl(struct file *filp, unsigned >> int cmd, unsigned long arg) >>> case ION_IOC_ALLOC: >>> { >>> int fd; >>> + struct dma_buf *dmabuf; >>> >>> - fd = ion_alloc(data.allocation.len, >>> + dmabuf = ion_alloc(data.allocation.len, >>> data.allocation.heap_id_mask, >>> data.allocation.flags); >>> - if (fd < 0) >>> + if (IS_ERR(dmabuf)) >>> + return PTR_ERR(dmabuf); >>> + >>> + fd = dma_buf_fd(dmabuf, O_CLOEXEC); >>> + if (fd < 0) { >>> + dma_buf_put(dmabuf); >>> return fd; >>> + } >>> >>> data.allocation.fd = fd; >>> >>> >

6 years, 8 months

[PATCH] staging: android: ion: fix err_cast.cocci warnings

by kbuild test robot

From: kbuild test robot <lkp(a)intel.com> drivers/staging/android/ion/ion.c:427:9-16: WARNING: ERR_CAST can be used with buffer Use ERR_CAST inlined function instead of ERR_PTR(PTR_ERR(...)) Generated by: scripts/coccinelle/api/err_cast.cocci Fixes: d51f182d188a ("staging: android: ion: refactory ion_alloc for kernel driver use") CC: Zeng Tao <prime.zeng(a)hisilicon.com> Signed-off-by: kbuild test robot <lkp(a)intel.com> --- url: https://github.com/0day-ci/linux/commits/Zeng-Tao/staging-android-ion-refac… ion.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -424,7 +424,7 @@ struct dma_buf *ion_alloc(size_t len, un return ERR_PTR(-ENODEV); if (IS_ERR(buffer)) - return ERR_PTR(PTR_ERR(buffer)); + return ERR_CAST(buffer); exp_info.ops = &dma_buf_ops; exp_info.size = buffer->size;

6 years, 8 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use

by kbuild test robot

Hi Zeng, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on staging/staging-testing] [also build test WARNING on v5.1-rc2 next-20190329] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Zeng-Tao/staging-android-ion-refac… coccinelle warnings: (new ones prefixed by >>) >> drivers/staging/android/ion/ion.c:427:9-16: WARNING: ERR_CAST can be used with buffer Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation

6 years, 8 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use

by Greg Kroah-Hartman

On Sat, Mar 30, 2019 at 02:32:35AM +0000, Zengtao (B) wrote: > >-----Original Message----- > >From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org] > >Sent: Saturday, March 30, 2019 12:04 AM > >To: Zengtao (B) <prime.zeng(a)hisilicon.com> > >Cc: labbott(a)redhat.com; sumit.semwal(a)linaro.org; > >devel(a)driverdev.osuosl.org; Todd Kjos <tkjos(a)android.com>; > >linux-kernel(a)vger.kernel.org; dri-devel(a)lists.freedesktop.org; > >linaro-mm-sig(a)lists.linaro.org; Arve Hjønnevåg <arve(a)android.com>; > >Joel Fernandes <joel(a)joelfernandes.org>; Martijn Coenen > ><maco(a)android.com>; Christian Brauner <christian(a)brauner.io> > >Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel > >driver use > > > >On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote: > >> There are two reasons for this patch: > >> 1. There are some potential requirements for ion_alloc in kernel > >> space, some media drivers need to allocate media buffers from ion > >> instead of buddy or dma framework, this is more convient and clean > >> very for media drivers. And In that case, ion is the only media buffer > >> provider, it's more easier to maintain. > > > >As this really is just DMA, what is wrong with the existing dma framework > >that makes it hard to use? You have seen all of the changes recently to it, > >right? > > The current dma framework is powerful enough(to me, and more complex ^_^) > , CMA, IOMMU are all integrated, it's good. But buffer sharing, statistics, debug, > are not so friendly for media drivers(each driver has to do all, but duplicate jobs). Then go add statistics and debugging to the dma code so that everyone benefits! thanks, greg k-h

6 years, 8 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use

by Greg Kroah-Hartman

6 years, 8 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use

by Greg Kroah-Hartman

On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote: > There are two reasons for this patch: > 1. There are some potential requirements for ion_alloc in kernel space, > some media drivers need to allocate media buffers from ion instead of > buddy or dma framework, this is more convient and clean very for media > drivers. And In that case, ion is the only media buffer provider, it's > more easier to maintain. > 2. Fd is only needed by user processes, not the kernel space, so dma_buf > should be returned instead of fd for kernel space, and dma_buf_fd should > be called only for userspace api. > > Signed-off-by: Zeng Tao <prime.zeng(a)hisilicon.com> > --- > drivers/staging/android/ion/ion.c | 32 +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index 92c2914..e93fb49 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -387,13 +387,13 @@ static const struct dma_buf_ops dma_buf_ops = { > .unmap = ion_dma_buf_kunmap, > }; > > -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > +struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask, > + unsigned int flags) > { > struct ion_device *dev = internal_dev; > struct ion_buffer *buffer = NULL; > struct ion_heap *heap; > DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > - int fd; > struct dma_buf *dmabuf; > > pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__, > @@ -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > len = PAGE_ALIGN(len); > > if (!len) > - return -EINVAL; > + return ERR_PTR(-EINVAL); > > down_read(&dev->lock); > plist_for_each_entry(heap, &dev->heaps, node) { > @@ -421,10 +421,10 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > up_read(&dev->lock); > > if (!buffer) > - return -ENODEV; > + return ERR_PTR(-ENODEV); > > if (IS_ERR(buffer)) > - return PTR_ERR(buffer); > + return ERR_PTR(PTR_ERR(buffer)); > > exp_info.ops = &dma_buf_ops; > exp_info.size = buffer->size; > @@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > exp_info.priv = buffer; > > dmabuf = dma_buf_export(&exp_info); > - if (IS_ERR(dmabuf)) { > + if (IS_ERR(dmabuf)) > _ion_buffer_destroy(buffer); > - return PTR_ERR(dmabuf); > - } > > - fd = dma_buf_fd(dmabuf, O_CLOEXEC); > - if (fd < 0) > - dma_buf_put(dmabuf); > - > - return fd; > + return dmabuf; > } > +EXPORT_SYMBOL(ion_alloc); If you are going to do this (and personally I'm with Laura in that I don't think you need it) this should be EXPORT_SYMBOL_GPL() please. thanks, greg k-h

6 years, 8 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use

by Laura Abbott

On 3/29/19 11:40 AM, Zeng Tao wrote: > There are two reasons for this patch: > 1. There are some potential requirements for ion_alloc in kernel space, > some media drivers need to allocate media buffers from ion instead of > buddy or dma framework, this is more convient and clean very for media > drivers. And In that case, ion is the only media buffer provider, it's > more easier to maintain. > 2. Fd is only needed by user processes, not the kernel space, so dma_buf > should be returned instead of fd for kernel space, and dma_buf_fd should > be called only for userspace api. > I really want to just NAK this because it doesn't seem like something that's necessary. The purpose of Ion is to provide buffers to userspace because there's no other way for userspace to get access to the memory. The kernel already has other APIs to access the memory. This also complicates the re-work that's been happening where the requirement is only userspace. Can you be more detailed about which media drivers you are referring to and why they can't just use other APIs? > Signed-off-by: Zeng Tao <prime.zeng(a)hisilicon.com> > --- > drivers/staging/android/ion/ion.c | 32 +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index 92c2914..e93fb49 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -387,13 +387,13 @@ static const struct dma_buf_ops dma_buf_ops = { > .unmap = ion_dma_buf_kunmap, > }; > > -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > +struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask, > + unsigned int flags) > { > struct ion_device *dev = internal_dev; > struct ion_buffer *buffer = NULL; > struct ion_heap *heap; > DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > - int fd; > struct dma_buf *dmabuf; > > pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__, > @@ -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > len = PAGE_ALIGN(len); > > if (!len) > - return -EINVAL; > + return ERR_PTR(-EINVAL); > > down_read(&dev->lock); > plist_for_each_entry(heap, &dev->heaps, node) { > @@ -421,10 +421,10 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > up_read(&dev->lock); > > if (!buffer) > - return -ENODEV; > + return ERR_PTR(-ENODEV); > > if (IS_ERR(buffer)) > - return PTR_ERR(buffer); > + return ERR_PTR(PTR_ERR(buffer)); > > exp_info.ops = &dma_buf_ops; > exp_info.size = buffer->size; > @@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > exp_info.priv = buffer; > > dmabuf = dma_buf_export(&exp_info); > - if (IS_ERR(dmabuf)) { > + if (IS_ERR(dmabuf)) > _ion_buffer_destroy(buffer); > - return PTR_ERR(dmabuf); > - } > > - fd = dma_buf_fd(dmabuf, O_CLOEXEC); > - if (fd < 0) > - dma_buf_put(dmabuf); > - > - return fd; > + return dmabuf; > } > +EXPORT_SYMBOL(ion_alloc); > > static int ion_query_heaps(struct ion_heap_query *query) > { > @@ -539,12 +534,19 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > case ION_IOC_ALLOC: > { > int fd; > + struct dma_buf *dmabuf; > > - fd = ion_alloc(data.allocation.len, > + dmabuf = ion_alloc(data.allocation.len, > data.allocation.heap_id_mask, > data.allocation.flags); > - if (fd < 0) > + if (IS_ERR(dmabuf)) > + return PTR_ERR(dmabuf); > + > + fd = dma_buf_fd(dmabuf, O_CLOEXEC); > + if (fd < 0) { > + dma_buf_put(dmabuf); > return fd; > + } > > data.allocation.fd = fd; > >

6 years, 8 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use

by Dan Carpenter

If we're going to export ion_alloc() for other drivers to use then let's make an ion_free() helper function as well. void ion_free(struct dma_buf *dmabuf) { dma_buf_put(dmabuf); } regards, dan carpenter

6 years, 8 months

Re: [Linaro-mm-sig] [PATCH RFC 1/1] uio: Add dma-buf import ioctls

by Daniel Vetter

On Thu, Feb 28, 2019 at 04:18:57PM -0800, Hyun Kwon wrote: > Hi Daniel, > > On Thu, 2019-02-28 at 02:01:46 -0800, Daniel Vetter wrote: > > On Wed, Feb 27, 2019 at 04:36:06PM -0800, Hyun Kwon wrote: > > > Hi Daniel, > > > > > > On Wed, 2019-02-27 at 06:13:45 -0800, Daniel Vetter wrote: > > > > On Tue, Feb 26, 2019 at 11:20 PM Hyun Kwon <hyun.kwon(a)xilinx.com> wrote: > > > > > > > > > > Hi Daniel, > > > > > > > > > > Thanks for the comment. > > > > > > > > > > On Tue, 2019-02-26 at 04:06:13 -0800, Daniel Vetter wrote: > > > > > > On Tue, Feb 26, 2019 at 12:53 PM Greg Kroah-Hartman > > > > > > <gregkh(a)linuxfoundation.org> wrote: > > > > > > > > > > > > > > On Sat, Feb 23, 2019 at 12:28:17PM -0800, Hyun Kwon wrote: > > > > > > > > Add the dmabuf map / unmap interfaces. This allows the user driver > > > > > > > > to be able to import the external dmabuf and use it from user space. > > > > > > > > > > > > > > > > Signed-off-by: Hyun Kwon <hyun.kwon(a)xilinx.com> > > > > > > > > --- > > > > > > > > drivers/uio/Makefile | 2 +- > > > > > > > > drivers/uio/uio.c | 43 +++++++++ > > > > > > > > drivers/uio/uio_dmabuf.c | 210 +++++++++++++++++++++++++++++++++++++++++++ > > > > > > > > drivers/uio/uio_dmabuf.h | 26 ++++++ > > > > > > > > include/uapi/linux/uio/uio.h | 33 +++++++ > > > > > > > > 5 files changed, 313 insertions(+), 1 deletion(-) > > > > > > > > create mode 100644 drivers/uio/uio_dmabuf.c > > > > > > > > create mode 100644 drivers/uio/uio_dmabuf.h > > > > > > > > create mode 100644 include/uapi/linux/uio/uio.h > > > > > > > > > > > > > > > > diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile > > > > > > > > index c285dd2..5da16c7 100644 > > > > > > > > --- a/drivers/uio/Makefile > > > > > > > > +++ b/drivers/uio/Makefile > > > > > > > > @@ -1,5 +1,5 @@ > > [snip] > > > > > > > Frankly looks like a ploy to sidestep review by graphics folks. We'd > > > > > > ask for the userspace first :-) > > > > > > > > > > Please refer to pull request [1]. > > > > > > > > > > For any interest in more details, the libmetal is the abstraction layer > > > > > which provides platform independent APIs. The backend implementation > > > > > can be selected per different platforms: ex, rtos, linux, > > > > > standalone (xilinx),,,. For Linux, it supports UIO / vfio as of now. > > > > > The actual user space drivers sit on top of libmetal. Such drivers can be > > > > > found in [2]. This is why I try to avoid any device specific code in > > > > > Linux kernel. > > > > > > > > > > > > > > > > > Also, exporting dma_addr to userspace is considered a very bad idea. > > > > > > > > > > I agree, hence the RFC to pick some brains. :-) Would it make sense > > > > > if this call doesn't export the physicall address, but instead takes > > > > > only the dmabuf fd and register offsets to be programmed? > > > > > > > > > > > If you want to do this properly, you need a minimal in-kernel memory > > > > > > manager, and those tend to be based on top of drm_gem.c and merged > > > > > > through the gpu tree. The last place where we accidentally leaked a > > > > > > dma addr for gpu buffers was in the fbdev code, and we plugged that > > > > > > one with > > > > > > > > > > Could you please help me understand how having a in-kernel memory manager > > > > > helps? Isn't it just moving same dmabuf import / paddr export functionality > > > > > in different modules: kernel memory manager vs uio. In fact, Xilinx does have > > > > > such memory manager based on drm gem in downstream. But for this time we took > > > > > the approach of implementing this through generic dmabuf allocator, ION, and > > > > > enabling the import capability in the UIO infrastructure instead. > > > > > > > > There's a group of people working on upstreaming a xilinx drm driver > > > > already. Which driver are we talking about? Can you pls provide a link > > > > to that xilinx drm driver? > > > > > > > > > > The one I was pushing [1] is implemented purely for display, and not > > > intended for anything other than that as of now. What I'm refering to above > > > is part of Xilinx FPGA (acceleration) runtime [2]. As far as I know, > > > it's planned to be upstreamed, but not yet started. The Xilinx runtime > > > software has its own in-kernel memory manager based on drm_cma_gem with > > > its own ioctls [3]. > > > > > > Thanks, > > > -hyun > > > > > > [1] https://patchwork.kernel.org/patch/10513001/ > > > [2] https://github.com/Xilinx/XRT > > > [3] https://github.com/Xilinx/XRT/tree/master/src/runtime_src/driver/zynq/drm > > > > I've done a very quick look only, and yes this is kinda what I'd expect. > > Doing a small drm gem driver for an fpga/accelarator that needs lots of > > memories is the right architecture, since at the low level of kernel > > interfaces a gpu really isn't anything else than an accelarater. > > > > And from a very cursory look the gem driver you mentioned (I only scrolled > > through the ioctl handler quickly) looks reasonable. > > Thanks for taking time to look and share input. But still I'd like to > understand why it's more reasonable if the similar ioctl exists with drm > than with uio. Is it because such drm ioctl is vendor specific? We do have quite a pile of shared infrastructure in drm beyond just the vendor specific ioctl. So putting accelerator drivers there makes sense, whether the programming is a GPU, some neural network folder, an FPGA or something else. The one issue is that we require open source userspace together with your driver, since just the accelerator shim in the kernel alone is fairly useless (both for review and for doing anything with it). But there's also some kernel maintainers who disagree and happily take drivers originally written for drm and then rewritten for non-drm for upstream to avoid the drm folks (or at least it very much looks like that, and happens fairly regularly). Cheers, Daniel > > Thanks, > -hyun > > > -Daniel > > > > > > > Thanks, Daniel > > > > > > > > > Thanks, > > > > > -hyun > > > > > > > > > > [1] https://github.com/OpenAMP/libmetal/pull/82/commits/951e2762bd487c98919ad12… > > > > > [2] https://github.com/Xilinx/embeddedsw/tree/master/XilinxProcessorIPLib/drive… > > > > > > > > > > > > > > > > > commit 4be9bd10e22dfc7fc101c5cf5969ef2d3a042d8a (tag: > > > > > > drm-misc-next-fixes-2018-10-03) > > > > > > Author: Neil Armstrong <narmstrong(a)baylibre.com> > > > > > > Date: Fri Sep 28 14:05:55 2018 +0200 > > > > > > > > > > > > drm/fb_helper: Allow leaking fbdev smem_start > > > > > > > > > > > > Together with cuse the above patch should be enough to implement a drm > > > > > > driver entirely in userspace at least. > > > > > > > > > > > > Cheers, Daniel > > > > > > -- > > > > > > Daniel Vetter > > > > > > Software Engineer, Intel Corporation > > > > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > > > > > > > > > > > > > -- > > > > Daniel Vetter > > > > Software Engineer, Intel Corporation > > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

6 years, 9 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig