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

[Michel Dänzer]

On 2019-07-24 5:48 p.m., Grodzovsky, Andrey wrote:
> On 7/24/19 11:40 AM, Michel Dänzer wrote:
>> On 2019-07-24 5:27 p.m., Andrey Grodzovsky wrote:
>>> 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.
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
>>> ---
>>>   [...]
>>>
>>> +#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");
>> The AMDGPU_GEM_CREATE_CPU_GTT_USWC check will always pass at some point
>> due to patch 1, so passing in the flags is kind of pointless. Just do
>> the DRM_INFO_ONCE unconditionally here.
> 
> AMDGPU_GEM_CREATE_CPU_GTT_USWC is set unconditionally only for FB console BOs in patch 1, for user mode BOs this
> flag might not be set in user mode and so this warning shouldn't pop in this case.

You're right in theory, but in practice this is pretty unlikely at this
point, other than maybe a headless machine which only runs compute
workloads (in which case PAT might be desirable anyway for other reasons?).

Anyway, it's just my opinion that no flags need to be passed in, not a
blocker.


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