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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@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?

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@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

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux