20. 4. 7. 오후 10:42에 Marek Szyprowski 이(가) 쓴 글:
> Explicitly check if the imported buffer has been mapped as contiguous in
> the DMA address space, what is required by all Exynos DRM CRTC drivers.
> While touching this, set buffer flags depending on the availability of
> the IOMMU.
> 
> Signed-off-by: Marek Szyprowski <m.szyprow...@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 36 +++++++++++++++++--------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c 
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 40514d3dcf60..9d4e4d321bda 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -458,6 +458,23 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device 
> *dev,
>       int npages;
>       int ret;
>  

(Optional) could you leave one comment here?
i.e., /* Check if DMA memory region from a exporter are mapped contiguously or 
not. */ 

> +     if (sgt->nents != 1) {

How about using below condition instead?
if (sgt->nents > 0) {

> +             dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> +             struct scatterlist *s;
> +             unsigned int i;
> +
> +             for_each_sg(sgt->sgl, s, sgt->nents, i) {
> +                     if (!sg_dma_len(s))
> +                             continue;

Isn't it an error case if sg_dma_len(s) is 0? I think length of s is 0 then it 
would be invalid because all entries of sgt should be mapped before 
sg_dma_len() is called.

Thanks,
Inki Dae

> +                     if (sg_dma_address(s) != next_addr) {
> +                             DRM_ERROR("buffer chunks must be mapped 
> contiguously");
> +                             return ERR_PTR(-EINVAL);
> +                     }
> +                     next_addr = sg_dma_address(s) + sg_dma_len(s);
> +             }
> +             return ERR_PTR(-EINVAL);
> +     }
> +
>       exynos_gem = exynos_drm_gem_init(dev, attach->dmabuf->size);
>       if (IS_ERR(exynos_gem)) {
>               ret = PTR_ERR(exynos_gem);
> @@ -480,18 +497,15 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device 
> *dev,
>  
>       exynos_gem->sgt = sgt;
>  
> -     if (sgt->nents == 1) {
> -             /* always physically continuous memory if sgt->nents is 1. */
> -             exynos_gem->flags |= EXYNOS_BO_CONTIG;
> -     } else {
> -             /*
> -              * this case could be CONTIG or NONCONTIG type but for now
> -              * sets NONCONTIG.
> -              * TODO. we have to find a way that exporter can notify
> -              * the type of its own buffer to importer.
> -              */
> +     /*
> +      * Buffer has been mapped as contiguous into DMA address space,
> +      * but if there is IOMMU, it can be either CONTIG or NONCONTIG.
> +      * We assume a simplified logic below:
> +      */
> +     if (is_drm_iommu_supported(dev))
>               exynos_gem->flags |= EXYNOS_BO_NONCONTIG;
> -     }
> +     else
> +             exynos_gem->flags |= EXYNOS_BO_CONTIG;
>  
>       return &exynos_gem->base;
>  
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to