Re: [PATCH v7 04/36] drm: amdgpu: fix common struct sg_table related issues

[Marek Szyprowski]

Hi Christian,

On 22.06.2020 15:27, Christian KÃnig wrote:
> Am 19.06.20 um 12:36 schrieb Marek Szyprowski:
>> The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() 
>> function
>> returns the number of the created entries in the DMA address space.
>> However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
>> dma_unmap_sg must be called with the original number of the entries
>> passed to the dma_map_sg().
>>
>> struct sg_table is a common structure used for describing a 
>> non-contiguous
>> memory buffer, used commonly in the DRM and graphics subsystems. It
>> consists of a scatterlist with memory pages and DMA addresses (sgl 
>> entry),
>> as well as the number of scatterlist entries: CPU pages (orig_nents 
>> entry)
>> and DMA mapped pages (nents entry).
>>
>> It turned out that it was a common mistake to misuse nents and 
>> orig_nents
>> entries, calling DMA-mapping functions with a wrong number of entries or
>> ignoring the number of mapped entries returned by the dma_map_sg()
>> function.
>>
>> To avoid such issues, lets use a common dma-mapping wrappers operating
>> directly on the struct sg_table objects and use scatterlist page
>> iterators where possible. This, almost always, hides references to the
>> nents and orig_nents entries, making the code robust, easier to follow
>> and copy/paste safe.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>> Reviewed-by: Christian KÃnig <christian.koenig@xxxxxxx>
>
> Any objection that we pick this one and the radeon up into our 
> branches for upstreaming?
>
> That should about clashes with other driver changes.

I'm fine. This one and radeon doesn't depend on the prime changes, so it 
should merge fine via your tree. I will try to ask for more review of 
the remaining patches and then try merging via drm-misc.

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