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

[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 at amd.com>
>>>> ---
>>>>    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.


-- 
Earthling Michel Dänzer               |              https://www.amd.com
Libre software enthusiast             |             Mesa and X developer