[v7,03/28] media: videobuf2: calculate restricted memory size
Commit Message
Getting the physical address with sg_dma_address for restricted memory,
only return the first physical address size since sg may not be physical
continuous, then leading to the dmabuf size is small than buf size. Need
to bypass continuous checking for restricted memory.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../common/videobuf2/videobuf2-dma-contig.c | 34 +++++++++++++++----
1 file changed, 28 insertions(+), 6 deletions(-)
Comments
On 20/07/2024 09:15, Yunfei Dong wrote:
> Getting the physical address with sg_dma_address for restricted memory,
> only return the first physical address size since sg may not be physical
> continuous, then leading to the dmabuf size is small than buf size. Need
> to bypass continuous checking for restricted memory.
>
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
> .../common/videobuf2/videobuf2-dma-contig.c | 34 +++++++++++++++----
> 1 file changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> index 3d4fd4ef5310..f0e4652b615f 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> @@ -66,6 +66,22 @@ static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
> return size;
> }
>
> +/**************************************************/
> +/* restricted mem scatterlist table functions */
> +/**************************************************/
> +
> +static unsigned long vb2_dc_get_res_mem_contiguous_size(struct sg_table *sgt)
> +{
> + struct scatterlist *s;
> + unsigned int i;
> + unsigned long size = 0;
> +
> + for_each_sgtable_dma_sg(sgt, s, i)
> + size += sg_dma_len(s);
> +
> + return size;
> +}
I think it is better to add a 'bool restricted' argument to vb2_dc_get_contiguous_size.
If true, then skip the 'expected' check there.
> +
> /*********************************************/
> /* callbacks for all buffers */
> /*********************************************/
> @@ -648,10 +664,13 @@ static void *vb2_dc_get_userptr(struct vb2_buffer *vb, struct device *dev,
> goto fail_sgt_init;
> }
>
> - contig_size = vb2_dc_get_contiguous_size(sgt);
> + if (buf->vb->vb2_queue->restricted_mem)
I think it is better to do the same as with buf->non_coherent_mem,
so add a 'bool restricted_mem' to struct vb2_dc_buf and set it in
vb2_dc_alloc(). It makes this code easier to read.
> + contig_size = vb2_dc_get_res_mem_contiguous_size(sgt);
> + else
> + contig_size = vb2_dc_get_contiguous_size(sgt);
> if (contig_size < size) {
> - pr_err("contiguous mapping is too small %lu/%lu\n",
> - contig_size, size);
> + pr_err("contiguous mapping is too small %lu/%lu/%u\n",
> + contig_size, size, buf->vb->vb2_queue->restricted_mem);
Rather than add a "/%u", which is not easy to understand, perhaps do this:
pr_err("%scontiguous mapping is too small %lu/%lu\n",
buf->vb->vb2_queue->restricted_mem ? "restricted " : "",
contig_size, size);
> ret = -EFAULT;
> goto fail_map_sg;
> }
> @@ -711,10 +730,13 @@ static int vb2_dc_map_dmabuf(void *mem_priv)
> }
>
> /* checking if dmabuf is big enough to store contiguous chunk */
> - contig_size = vb2_dc_get_contiguous_size(sgt);
> + if (buf->vb->vb2_queue->restricted_mem)
> + contig_size = vb2_dc_get_res_mem_contiguous_size(sgt);
> + else
> + contig_size = vb2_dc_get_contiguous_size(sgt);
> if (contig_size < buf->size) {
> - pr_err("contiguous chunk is too small %lu/%lu\n",
> - contig_size, buf->size);
> + pr_err("contiguous chunk is too small %lu/%lu/%u\n",
> + contig_size, buf->size, buf->vb->vb2_queue->restricted_mem);
Ditto.
> dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt,
> buf->dma_dir);
> return -EFAULT;
Regards,
Hans
@@ -66,6 +66,22 @@ static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
return size;
}
+/**************************************************/
+/* restricted mem scatterlist table functions */
+/**************************************************/
+
+static unsigned long vb2_dc_get_res_mem_contiguous_size(struct sg_table *sgt)
+{
+ struct scatterlist *s;
+ unsigned int i;
+ unsigned long size = 0;
+
+ for_each_sgtable_dma_sg(sgt, s, i)
+ size += sg_dma_len(s);
+
+ return size;
+}
+
/*********************************************/
/* callbacks for all buffers */
/*********************************************/
@@ -648,10 +664,13 @@ static void *vb2_dc_get_userptr(struct vb2_buffer *vb, struct device *dev,
goto fail_sgt_init;
}
- contig_size = vb2_dc_get_contiguous_size(sgt);
+ if (buf->vb->vb2_queue->restricted_mem)
+ contig_size = vb2_dc_get_res_mem_contiguous_size(sgt);
+ else
+ contig_size = vb2_dc_get_contiguous_size(sgt);
if (contig_size < size) {
- pr_err("contiguous mapping is too small %lu/%lu\n",
- contig_size, size);
+ pr_err("contiguous mapping is too small %lu/%lu/%u\n",
+ contig_size, size, buf->vb->vb2_queue->restricted_mem);
ret = -EFAULT;
goto fail_map_sg;
}
@@ -711,10 +730,13 @@ static int vb2_dc_map_dmabuf(void *mem_priv)
}
/* checking if dmabuf is big enough to store contiguous chunk */
- contig_size = vb2_dc_get_contiguous_size(sgt);
+ if (buf->vb->vb2_queue->restricted_mem)
+ contig_size = vb2_dc_get_res_mem_contiguous_size(sgt);
+ else
+ contig_size = vb2_dc_get_contiguous_size(sgt);
if (contig_size < buf->size) {
- pr_err("contiguous chunk is too small %lu/%lu\n",
- contig_size, buf->size);
+ pr_err("contiguous chunk is too small %lu/%lu/%u\n",
+ contig_size, buf->size, buf->vb->vb2_queue->restricted_mem);
dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt,
buf->dma_dir);
return -EFAULT;