Re: [PATCH v2 2/3] drm/exynos: gem: rework scatter-list contiguity check on prime import

[Inki Dae <inki.dae@samsung.com>]

20. 4. 22. 오후 8:40에 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.

Picked it up.

Thanks,
Inki Dae

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> v2:
> - reworked a check for sgt->nents
> - added a comment about a scatter-list contiguity check
> - removed additional 'return ERR_PTR(-EINVAL)' accidentaly left after debugging
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 42 ++++++++++++++++++++++++---------
>  1 file changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 40514d3..f136efb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -458,6 +458,29 @@ struct drm_gem_object *
>  	int npages;
>  	int ret;
>  
> +	if (sgt->nents < 1)
> +		return ERR_PTR(-EINVAL);
> +
> +	/*
> +	 * Check if the provided buffer has been mapped as contiguous
> +	 * into DMA address space.
> +	 */
> +	if (sgt->nents > 1) {
> +		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))
> +				break;
> +			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);
> +		}
> +	}
> +
>  	exynos_gem = exynos_drm_gem_init(dev, attach->dmabuf->size);
>  	if (IS_ERR(exynos_gem)) {
>  		ret = PTR_ERR(exynos_gem);
> @@ -480,18 +503,15 @@ struct drm_gem_object *
>  
>  	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;
>  
>