Hi Marek,
20. 4. 21. 오후 5:09에 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@xxxxxxxxxxx>
>>> ---
>>> 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.
Oops sorry. My intention was 'if (sgt->nents > 1)' because if (sgt->nents != 1) allows
- sgt->nents < 1
- sgt->nents > 1
I think the checking would be valid only in case of having multiple entries - sgt->nents > 1.
Thanks,
Inki Dae
>
>>> + 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
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
