| Message ID | 1477471926-15796-3-git-send-email-thierry.escande@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I'm going through old patches in patchwork that fell through the cracks, and this is one of them. If this is still desired, please rebase and repost. I'm marking this series as Obsoleted in patchwork, since it no longer applies anyway. Regards, Hans On 10/26/2016 10:52 AM, Thierry Escande wrote: > From: Heng-Ruey Hsu <henryhsu@chromium.org> > > DMA allocations for MMAP type are uncached by default. But for > some cases, CPU has to access the buffers. ie: memcpy for format > converter. Supporting cacheable MMAP improves huge performance. > > This patch enables cacheable memory for DMA coherent allocator in mmap > buffer allocation if non-consistent DMA attribute is set and kernel > mapping is present. Even if userspace doesn't mmap the buffer, sync > still should be happening if kernel mapping is present. > If not done in allocation, it is enabled when memory is mapped from > userspace (if non-consistent DMA attribute is set). > > Signed-off-by: Heng-Ruey Hsu <henryhsu@chromium.org> > Tested-by: Heng-ruey Hsu <henryhsu@chromium.org> > Reviewed-by: Tomasz Figa <tfiga@chromium.org> > Signed-off-by: Thierry Escande <thierry.escande@collabora.com> > --- > drivers/media/v4l2-core/videobuf2-dma-contig.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c > index 0d9665d..89b534a 100644 > --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c > +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c > @@ -151,6 +151,10 @@ static void vb2_dc_put(void *buf_priv) > sg_free_table(buf->sgt_base); > kfree(buf->sgt_base); > } > + if (buf->dma_sgt) { > + sg_free_table(buf->dma_sgt); > + kfree(buf->dma_sgt); > + } > dma_free_attrs(buf->dev, buf->size, buf->cookie, buf->dma_addr, > buf->attrs); > put_device(buf->dev); > @@ -192,6 +196,14 @@ static void *vb2_dc_alloc(struct device *dev, unsigned long attrs, > buf->handler.put = vb2_dc_put; > buf->handler.arg = buf; > > + /* > + * Enable cache maintenance. Even if userspace doesn't mmap the buffer, > + * sync still should be happening if kernel mapping is present. > + */ > + if (!(buf->attrs & DMA_ATTR_NO_KERNEL_MAPPING) && > + buf->attrs & DMA_ATTR_NON_CONSISTENT) > + buf->dma_sgt = vb2_dc_get_base_sgt(buf); > + > atomic_inc(&buf->refcount); > > return buf; > @@ -227,6 +239,10 @@ static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma) > > vma->vm_ops->open(vma); > > + /* Enable cache maintenance if not enabled in allocation. */ > + if (!buf->dma_sgt && buf->attrs & DMA_ATTR_NON_CONSISTENT) > + buf->dma_sgt = vb2_dc_get_base_sgt(buf); > + > pr_debug("%s: mapped dma addr 0x%08lx at 0x%08lx, size %ld\n", > __func__, (unsigned long)buf->dma_addr, vma->vm_start, > buf->size); >
Hi Hans, On Mon, Sep 17, 2018 at 11:41 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: > > I'm going through old patches in patchwork that fell through the > cracks, and this is one of them. > > If this is still desired, please rebase and repost. > > I'm marking this series as Obsoleted in patchwork, since it no longer > applies anyway. The ability to have cached mappings of MMAP buffers is strongly desired, but I'm afraid not the way this patch does it. First of all, it's not a decision for the driver to make, but for the user space, depending on the access pattern it does. It also isn't something specific to vb2-dma-contig only. I remember Sakari had a series that attempted to solve this in a more comprehensive way[1]. I remember it had some minor problems when I reviewed it, but generally the idea seemed sane. Sakari, do you have any plans to revive that work? [1] https://www.mail-archive.com/linux-media@vger.kernel.org/msg112459.html Best regards, Tomasz > > Regards, > > Hans > > > On 10/26/2016 10:52 AM, Thierry Escande wrote: > > From: Heng-Ruey Hsu <henryhsu@chromium.org> > > > > DMA allocations for MMAP type are uncached by default. But for > > some cases, CPU has to access the buffers. ie: memcpy for format > > converter. Supporting cacheable MMAP improves huge performance. > > > > This patch enables cacheable memory for DMA coherent allocator in mmap > > buffer allocation if non-consistent DMA attribute is set and kernel > > mapping is present. Even if userspace doesn't mmap the buffer, sync > > still should be happening if kernel mapping is present. > > If not done in allocation, it is enabled when memory is mapped from > > userspace (if non-consistent DMA attribute is set). > > > > Signed-off-by: Heng-Ruey Hsu <henryhsu@chromium.org> > > Tested-by: Heng-ruey Hsu <henryhsu@chromium.org> > > Reviewed-by: Tomasz Figa <tfiga@chromium.org> > > Signed-off-by: Thierry Escande <thierry.escande@collabora.com> > > --- > > drivers/media/v4l2-core/videobuf2-dma-contig.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c > > index 0d9665d..89b534a 100644 > > --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c > > +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c > > @@ -151,6 +151,10 @@ static void vb2_dc_put(void *buf_priv) > > sg_free_table(buf->sgt_base); > > kfree(buf->sgt_base); > > } > > + if (buf->dma_sgt) { > > + sg_free_table(buf->dma_sgt); > > + kfree(buf->dma_sgt); > > + } > > dma_free_attrs(buf->dev, buf->size, buf->cookie, buf->dma_addr, > > buf->attrs); > > put_device(buf->dev); > > @@ -192,6 +196,14 @@ static void *vb2_dc_alloc(struct device *dev, unsigned long attrs, > > buf->handler.put = vb2_dc_put; > > buf->handler.arg = buf; > > > > + /* > > + * Enable cache maintenance. Even if userspace doesn't mmap the buffer, > > + * sync still should be happening if kernel mapping is present. > > + */ > > + if (!(buf->attrs & DMA_ATTR_NO_KERNEL_MAPPING) && > > + buf->attrs & DMA_ATTR_NON_CONSISTENT) > > + buf->dma_sgt = vb2_dc_get_base_sgt(buf); > > + > > atomic_inc(&buf->refcount); > > > > return buf; > > @@ -227,6 +239,10 @@ static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma) > > > > vma->vm_ops->open(vma); > > > > + /* Enable cache maintenance if not enabled in allocation. */ > > + if (!buf->dma_sgt && buf->attrs & DMA_ATTR_NON_CONSISTENT) > > + buf->dma_sgt = vb2_dc_get_base_sgt(buf); > > + > > pr_debug("%s: mapped dma addr 0x%08lx at 0x%08lx, size %ld\n", > > __func__, (unsigned long)buf->dma_addr, vma->vm_start, > > buf->size); > > >
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 0d9665d..89b534a 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -151,6 +151,10 @@ static void vb2_dc_put(void *buf_priv) sg_free_table(buf->sgt_base); kfree(buf->sgt_base); } + if (buf->dma_sgt) { + sg_free_table(buf->dma_sgt); + kfree(buf->dma_sgt); + } dma_free_attrs(buf->dev, buf->size, buf->cookie, buf->dma_addr, buf->attrs); put_device(buf->dev); @@ -192,6 +196,14 @@ static void *vb2_dc_alloc(struct device *dev, unsigned long attrs, buf->handler.put = vb2_dc_put; buf->handler.arg = buf; + /* + * Enable cache maintenance. Even if userspace doesn't mmap the buffer, + * sync still should be happening if kernel mapping is present. + */ + if (!(buf->attrs & DMA_ATTR_NO_KERNEL_MAPPING) && + buf->attrs & DMA_ATTR_NON_CONSISTENT) + buf->dma_sgt = vb2_dc_get_base_sgt(buf); + atomic_inc(&buf->refcount); return buf; @@ -227,6 +239,10 @@ static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma) vma->vm_ops->open(vma); + /* Enable cache maintenance if not enabled in allocation. */ + if (!buf->dma_sgt && buf->attrs & DMA_ATTR_NON_CONSISTENT) + buf->dma_sgt = vb2_dc_get_base_sgt(buf); + pr_debug("%s: mapped dma addr 0x%08lx at 0x%08lx, size %ld\n", __func__, (unsigned long)buf->dma_addr, vma->vm_start, buf->size);