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@xxxxxxx> 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@xxxxxxx> >>>> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx> >>> 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? 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@xxxxxxxxxxxxxxxxxxxxx >>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>>> _______________________________________________ >>>> amd-gfx mailing list >>>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx >>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > _______________________________________________ > amd-gfx mailing list > amd-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx