[PATCH] drm/amdgpu: Clear VRAM for DRM dumb_create buffers

[Christian König]

Am 11.03.19 um 18:53 schrieb Kazlauskas, Nicholas:
> On 3/11/19 1:31 PM, Koenig, Christian wrote:
>> Am 11.03.19 um 18:04 schrieb Kazlauskas, Nicholas:
>>> On 3/11/19 12:58 PM, Christian König wrote:
>>>> Am 08.03.19 um 17:47 schrieb Alex Deucher:
>>>>> On Fri, Mar 8, 2019 at 10:38 AM Nicholas Kazlauskas
>>>>> <nicholas.kazlauskas at amd.com> wrote:
>>>>>> The dumb_create API isn't intended for high performance rendering
>>>>>> and it's more useful for userspace (ie. IGT) to have them precleared.
>>>>>>
>>>>>> The bonus here is that we also won't needlessly leak whatever was
>>>>>> previously in VRAM, but it also probably wasn't sensitive if it was
>>>>>> going through this API.
>>>>>>
>>>>>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>>>> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
>>>> NAK, IIRC we used to have this bit here before.
>>>>
>>>> In general I agree that this would be a good idea, but the problem is
>>>> the that first dump buffer is sometimes created before the engine
>>>> (usually SDMA) to clear VRAM is initialized.
>>>>
>>>> So you can run into a nice crash on suspend/resume.
>>>>
>>>> Christian.
>>> FWIW I never hit it myself during any suspend/resume tests I did but
>>> I'll take your word for it.
>>>
>>> How about doing a map/memset/unmap immediately after buffer creation
>>> here? It'd be much slower, but it's also what we'd be doing in userspace
>>> anyway.
>> And history repeat itself :) Yeah, that was also suggested before and
>> didn't worked either for some reason.
>>
>> But that was ~2 years ago and I really don't remember the details.
>>
>> What could work is maybe to add a check if the engine is available if
>> the flag is given and ignore it when it isn't.
>>
>> Christian.
> The function amdgpu_fill_buffer is used to clear memory, and it checks
>
> if (!adev->mman.buffer_funcs_enabled)
>
> at the start. And buffer_funcs_enabled is only set after SDMA has been
> initialized following resume. In the case where the SDMA engine is off
> it'll just throw that DRM_ERROR and return -EINVAL.
>
> So if we still want to continue with buffer creation in the case where
> the engine is off then this flag could also be checked in
> dumb_buffer_create and cleared in the case where buffer_funcs_enabled =
> false.
>
> How does that sound to you?

Yeah, that sounds like a plan to me.

Christian.

>
> Nicholas Kazlauskas
>
>>> Nicholas Kazlauskas
>>>
>>>
>>>>>> ---
>>>>>>      drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 3 ++-
>>>>>>      1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>>> index fcaaac30e84b..a58072bbc9b8 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>>> @@ -743,7 +743,8 @@ int amdgpu_mode_dumb_create(struct drm_file
>>>>>> *file_priv,
>>>>>>             domain = amdgpu_bo_get_preferred_pin_domain(adev,
>>>>>>                                     
>>>>>> amdgpu_display_supported_domains(adev));
>>>>>>             r = amdgpu_gem_object_create(adev, args->size, 0, domain,
>>>>>> -
>>>>>> AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED,
>>>>>> +
>>>>>> AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
>>>>>> +                                    AMDGPU_GEM_CREATE_VRAM_CLEARED,
>>>>>>                                          ttm_bo_type_device, NULL, &gobj);
>>>>>>             if (r)
>>>>>>                     return -ENOMEM;
>>>>>> -- 
>>>>>> 2.17.1
>>>>>>
>>>>>> _______________________________________________
>>>>>> amd-gfx mailing list
>>>>>> amd-gfx at lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx