Re: [PATCH v5 2/4] drm/amdgpu: Create helper to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC

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

 



Am 26.07.19 um 16:53 schrieb Michel Dänzer:
On 2019-07-26 1:55 p.m., Christian König wrote:
Am 26.07.19 um 10:54 schrieb Michel Dänzer:
On 2019-07-26 9:11 a.m., Christian König wrote:
Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:
Move the logic to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC in
amdgpu_bo_do_create into standalone helper so it can be reused
in other functions.

v4:
Switch to return bool.

v5: Fix typos.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
---
    drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 61
+++++++++++++++++-------------
    drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +
    2 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 989b7b5..8702062 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -413,6 +413,40 @@ static bool amdgpu_bo_validate_size(struct
amdgpu_device *adev,
        return false;
    }
    +bool amdgpu_bo_support_uswc(u64 bo_flags)
+{
+
+#ifdef CONFIG_X86_32
+    /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
+     * See https://bugs.freedesktop.org/show_bug.cgi?id=84627
+     */
+    return false;
+#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
+    /* Don't try to enable write-combining when it can't work, or
things
+     * may be slow
+     * See https://bugs.freedesktop.org/show_bug.cgi?id=88758
+     */
+
+#ifndef CONFIG_COMPILE_TEST
+#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better
performance \
+     thanks to write-combining
+#endif
+
+    if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
+        DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT
for "
+                  "better performance thanks to write-combining\n");
I don't think this message belongs here.

[...]
@@ -466,33 +500,8 @@ static int amdgpu_bo_do_create(struct
[...]
+    if (!amdgpu_bo_support_uswc(bo->flags))
            bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
Rather here we should do "if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC
&& !amdgpu_bo_support_uswc())" and then clear the flag and also print
the warning.
That would require duplicating the CONFIG_X86_PAT related logic here as
well, which is a bit ugly.
Actually I would say we should drop this extra check and always emit a
message that USWC is disabled for this platform.

We now need it for more than just better performance and it should be
explicitly noted that this is not available in the logs.
A log message which doesn't explain why it's disabled / how to enable it
would probably cause us user support pain.

Mhm, sounds like you didn't got what I wanted to say.

No log message was fine as long as USWC was only a performance optimization, but now it becomes mandatory for correct operation in some settings.

In other words in very low VRAM configurations it can be that we can't enable higher resolution because the kernel is not compiled with the necessary flags for USWC support.

Printing that when the driver loads sounds like the best place to me.

Regards,
Christian.
_______________________________________________
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