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.d...@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

Reply via email to