[PATCH 2/3] drm/exynos: gem: Rework scatter-list contiguity check on Prime import

[Marek Szyprowski]

Hi Inki,

On 21.04.2020 09:38, Inki Dae wrote:
> 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.szyprowski at 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. */

Okay.

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

This is not the same. My check for != 1 is intended. Checking contiguity 
of the scatterlist if it has only 1 element doesn't have much sense.

>> +		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.

Well, it is a grey area. Some code incorrectly sets nents as orig_nents, 
thus this version is simply safe for both approaches. sg entries unused 
for DMA chunks have sg_dma_len() == 0.

The above check is more or less copied from 
drm_gem_cma_prime_import_sg_table() (drivers/gpu/drm/drm_gem_cma_helper.c).

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland