Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86

From: "Michel Dänzer" <michel@daenzer.net>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Christoph Hellwig <hch@infradead.org>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Will Deacon <will.deacon@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	amd-gfx list <amd-gfx@lists.freedesktop.org>,
	Junwei Zhang <Jerry.Zhang@amd.com>,
	David Airlie <airlied@linux.ie>, Huang Rui <ray.huang@amd.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Alex Deucher <alexander.deucher@amd.com>,
	Sean Paul <sean@poorly.run>,
	Christian Koenig <christian.koenig@amd.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
Date: Thu, 24 Jan 2019 10:31:06 +0100	[thread overview]
Message-ID: <ee261b46-1a83-6af2-31fe-fb9f30894c1e@daenzer.net> (raw)
In-Reply-To: <CAKv+Gu-9vEKu8mG-o+-4M1ZcgZmTTJVfEdNS9R27AFX8P5sqyQ@mail.gmail.com>

On 2019-01-23 5:52 p.m., Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 17:44, Christoph Hellwig <hch@infradead.org> wrote:
>>
>> I think we just want a driver-local check for those combinations
>> where we know this hack actually works, which really just seems
>> to be x86-64 with PAT. Something like the patch below, but maybe with
>> even more strong warnings to not do something like this elsewhere:
> 
> I agree that your patch seems like the right way to ensure that the WC
> optimization hack is only used where we know it works.
> 
> But my concern is that it seems likely that non-cache coherent
> implementations are relying on this hack as well.

I've been trying to tell you they can't rely on that, because the amdgpu
driver doesn't use this functionality for fundamentals such as ring
buffers used for feeding the hardware with commands. Instead, for those
it relies on snooped PCIe transfers being coherent with the CPU caches.

>> -#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");

FWIW, please don't drop these compile and build time warnings where we
continue to take advantage of PAT.

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