Re: [PATCH 0/4] iommu: tegra-gart cleanups

[Robin Murphy <robin.murphy@arm.com>]

On 2023-05-16 16:15, Nicolas Chauvet wrote:
> Le mar. 16 mai 2023 à 14:31, Robin Murphy <robin.murphy@arm.com> a écrit :
>>
>> On 2023-05-16 13:16, Nicolas Chauvet wrote:
>>> Le mar. 16 mai 2023 à 13:45, Robin Murphy <robin.murphy@arm.com> a écrit :
>>>>
>>>> On 2023-05-16 10:53, Nicolas Chauvet wrote:
>>> ...
>>>>> For what it worth, I've tried to test this serie with "grate patches"
>>>>> (1) rebased on top on 6.4-rc2, that would make use of the tegra-gart.
>>>>> That was on PAZ00 (with only 512M of RAM and 96M CMA still allocated).
>>>>> Unfortunately, this lead to the following errors with display problems
>>>>> (no character displayed in lxt-terminal and etc)
>>>>
>>>> Thanks for testing - it's quite possible I've made a silly mistake
>>>> somewhere, but just to double-check, does the same happen with the
>>>> existing driver if you boot with "tegra-gart.gart_debug=1" (or at least
>>>> enable the parameter via sysfs before the DRM driver gets going)?
>>>
>>> Using echo 1 > /sys/module/tegra_gart/parameters/gart_debug shows the
>>> same messages and the same artefacts (missing refreshed window
>>> content).
>>> Using "echo 0 > /sys/module/tegra_gart/parameters/gart_debug" revert
>>> back to normal...
>>
>> OK, cool, so it looks like a pre-existing bug in the caller, but I guess
>> it does mean that unconditionally enabling the checks isn't ideal until
>> that can be sorted out.
> 
> Seems like I had a non-default option with tegra-drm that was
> controlling the way tegra-gart is used:
> https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/grate/gart.c#L19-L29
> 
> With option 4, occurrences of failing allocation are experienced more
> often than the default 0 options when only "scattered BOs are mapped".
> Also with the default option, no noticeable failure is seen.

Oh, I see it now - the logic around tegra_bo_mm_evict_something() 
actually depends on being able to map new PTEs over the top of existing 
ones without an unmap :/

The map/unmap overhead that that's trying to avoid could probably be 
reduced quite significantly anyway by converting GART to the new 
map_pages/unmap_pages callbacks.

Thanks,
Robin.