Re: [PATCH v2 4/7] scatterlist: add sg_alloc_table_from_buf() helper

[Dave Gordon]

On 30/03/16 19:18, Boris Brezillon wrote:

On Wed, 30 Mar 2016 09:51:43 -0700
Mark Brown  wrote:


On Wed, Mar 30, 2016 at 05:39:51PM +0200, Boris Brezillon wrote:

sg_alloc_table_from_buf() provides an easy solution to create an sg_table
from a virtual address pointer. This function takes care of dealing with
vmallocated buffers, buffer alignment, or DMA engine limitations (maximum
DMA transfer size).


This seems nice.  Should we also have a further helper on top of this
which will get constraints from a dmaengine, it seems like it'd be a
common need?


Yep, we could create a wrapper extracting dma_slave caps info,
converting it to sg_constraints and calling sg_alloc_table_from_buf().
But let's try to get this function accepted first, and I'll send another
patch providing this wrapper.

BTW, do you see other things that should be added in sg_constraints?



You could compare with the things Solaris uses to describe the 
restrictions on a DMA binding ...


http://docs.oracle.com/cd/E23824_01/html/821-1478/ddi-dma-attr-9s.html#REFMAN9Sddi-dma-attr-9s

.Dave.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/7] scatterlist: add sg_alloc_table_from_buf() helper

[Mark Brown]

On Wed, Mar 30, 2016 at 08:18:31PM +0200, Boris Brezillon wrote:

> BTW, do you see other things that should be added in sg_constraints?

It looked to do everything SPI does which is everything I know about.


signature.asc
Description: PGP signature

Re: [PATCH v2 4/7] scatterlist: add sg_alloc_table_from_buf() helper

[Boris Brezillon]

On Wed, 30 Mar 2016 09:51:43 -0700
Mark Brown  wrote:

> On Wed, Mar 30, 2016 at 05:39:51PM +0200, Boris Brezillon wrote:
> > sg_alloc_table_from_buf() provides an easy solution to create an sg_table
> > from a virtual address pointer. This function takes care of dealing with
> > vmallocated buffers, buffer alignment, or DMA engine limitations (maximum
> > DMA transfer size).
> 
> This seems nice.  Should we also have a further helper on top of this
> which will get constraints from a dmaengine, it seems like it'd be a
> common need?

Yep, we could create a wrapper extracting dma_slave caps info,
converting it to sg_constraints and calling sg_alloc_table_from_buf().
But let's try to get this function accepted first, and I'll send another
patch providing this wrapper.

BTW, do you see other things that should be added in sg_constraints?

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/7] scatterlist: add sg_alloc_table_from_buf() helper

[Mark Brown]

On Wed, Mar 30, 2016 at 05:39:51PM +0200, Boris Brezillon wrote:
> sg_alloc_table_from_buf() provides an easy solution to create an sg_table
> from a virtual address pointer. This function takes care of dealing with
> vmallocated buffers, buffer alignment, or DMA engine limitations (maximum
> DMA transfer size).

This seems nice.  Should we also have a further helper on top of this
which will get constraints from a dmaengine, it seems like it'd be a
common need?


signature.asc
Description: PGP signature

[PATCH v2 4/7] scatterlist: add sg_alloc_table_from_buf() helper

[Boris Brezillon]

sg_alloc_table_from_buf() provides an easy solution to create an sg_table
from a virtual address pointer. This function takes care of dealing with
vmallocated buffers, buffer alignment, or DMA engine limitations (maximum
DMA transfer size).

Signed-off-by: Boris Brezillon 
---
 include/linux/scatterlist.h |  24 +++
 lib/scatterlist.c   | 161 
 2 files changed, 185 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 556ec1e..4a75362 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -41,6 +41,27 @@ struct sg_table {
unsigned int orig_nents;/* original size of list */
 };
 
+/**
+ * struct sg_constraints - SG constraints structure
+ *
+ * @max_chunk_len: maximum chunk buffer length. Each SG entry has to be smaller
+ *than this value. Zero means no constraint.
+ * @required_alignment: minimum alignment. Is used for both size and pointer
+ * alignment. If this constraint is not met, the function
+ * should return -EINVAL.
+ * @preferred_alignment: preferred alignment. Mainly used to optimize
+ *  throughput when the DMA engine performs better when
+ *  doing aligned accesses.
+ *
+ * This structure is here to help sg_alloc_table_from_buf() create the optimal
+ * SG list based on DMA engine constraints.
+ */
+struct sg_constraints {
+   size_t max_chunk_len;
+   size_t required_alignment;
+   size_t preferred_alignment;
+};
+
 /*
  * Notes on SG table design.
  *
@@ -265,6 +286,9 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
struct page **pages, unsigned int n_pages,
unsigned long offset, unsigned long size,
gfp_t gfp_mask);
+int sg_alloc_table_from_buf(struct sg_table *sgt, const void *buf, size_t len,
+   const struct sg_constraints *constraints,
+   gfp_t gfp_mask);
 
 size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
  size_t buflen, off_t skip, bool to_buffer);
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 004fc70..94776ff 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -433,6 +433,167 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
 }
 EXPORT_SYMBOL(sg_alloc_table_from_pages);
 
+static size_t sg_buf_chunk_len(const void *buf, size_t len,
+  const struct sg_constraints *cons)
+{
+   size_t chunk_len = len;
+
+   if (cons->max_chunk_len)
+   chunk_len = min_t(size_t, chunk_len, cons->max_chunk_len);
+
+   if (is_vmalloc_addr(buf)) {
+   unsigned long offset_in_page = offset_in_page(buf);
+   size_t contig_len = PAGE_SIZE - offset_in_page;
+   unsigned long phys = vmalloc_to_pfn(buf) - offset_in_page;
+   const void *contig_ptr = buf + contig_len;
+
+   /*
+* Vmalloced buffer might be composed of several physically
+* contiguous pages. Avoid extra scattergather entries in
+* this case.
+*/
+   while (contig_len < chunk_len) {
+   if (phys + PAGE_SIZE != vmalloc_to_pfn(contig_ptr))
+   break;
+
+   contig_len += PAGE_SIZE;
+   contig_ptr += PAGE_SIZE;
+   phys += PAGE_SIZE;
+   }
+
+   chunk_len = min_t(size_t, chunk_len, contig_len);
+   }
+
+   if (!IS_ALIGNED((unsigned long)buf, cons->preferred_alignment)) {
+   const void *aligned_buf = PTR_ALIGN(buf,
+   cons->preferred_alignment);
+   size_t unaligned_len = (unsigned long)(aligned_buf - buf);
+
+   chunk_len = min_t(size_t, chunk_len, unaligned_len);
+   } else if (chunk_len > cons->preferred_alignment) {
+   chunk_len &= ~(cons->preferred_alignment - 1);
+   }
+
+   return chunk_len;
+}
+
+#define sg_for_each_chunk_in_buf(buf, len, chunk_len, constraints) \
+   for (chunk_len = sg_buf_chunk_len(buf, len, constraints);   \
+len;   \
+len -= chunk_len, buf += chunk_len,\
+chunk_len = sg_buf_chunk_len(buf, len, constraints))
+
+static int sg_check_constraints(struct sg_constraints *cons,
+   const void *buf, size_t len)
+{
+   if (!cons->required_alignment)
+   cons->required_alignment = 1;
+
+   if (!cons->preferred_alignment)
+   cons->preferred_alignment = cons->required_alignment;
+
+   /* Test if buf and len are properly aligned. */
+   if (!IS_ALIGNED((unsigned long)buf, cons->required_alignment) ||