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@xxxxxxxxxxxx>
> ---
> .../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
