From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH] drm/vmwgfx: switch from ioremap_cache to memremap Date: Tue, 13 Oct 2015 20:37:25 +0200 Message-ID: <561D4F65.5080908@vmware.com> References: <20151012223527.34143.87313.stgit@dwillia2-desk3.jf.intel.com> <561C9427.8020001@vmware.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from smtp-outbound-1.vmware.com (smtp-outbound-1.vmware.com [208.91.2.12]) by gabe.freedesktop.org (Postfix) with ESMTPS id 131D66E66C for ; Tue, 13 Oct 2015 11:37:32 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Dan Williams Cc: "linux-kernel@vger.kernel.org" , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gMTAvMTMvMjAxNSAwNjozNSBQTSwgRGFuIFdpbGxpYW1zIHdyb3RlOgo+IE9uIE1vbiwgT2N0 IDEyLCAyMDE1IGF0IDEwOjE4IFBNLCBUaG9tYXMgSGVsbHN0cm9tCj4gPHRoZWxsc3Ryb21Adm13 YXJlLmNvbT4gd3JvdGU6Cj4+IEhpIQo+Pgo+PiBPbiAxMC8xMy8yMDE1IDEyOjM1IEFNLCBEYW4g V2lsbGlhbXMgd3JvdGU6Cj4+PiBQZXIgY29tbWl0IDJlNTg2YTdlMDE3YSAiZHJtL3Ztd2dmeDog TWFwIHRoZSBmaWZvIGFzIGNhY2hlZCIgdGhlIGRyaXZlcgo+Pj4gZXhwZWN0cyB0aGUgZmlmbyBy ZWdpc3RlcnMgdG8gYmUgY2FjaGVhYmxlLiAgSW4gcHJlcGFyYXRpb24gZm9yCj4+PiBkZXByZWNh dGluZyBpb3JlbWFwX2NhY2hlKCkgY29udmVydCBpdHMgdXNhZ2UgaW4gdm13Z2Z4IHRvIG1lbXJl bWFwKCkuCj4+Pgo+Pj4gQ2M6IERhdmlkIEFpcmxpZSA8YWlybGllZEBsaW51eC5pZT4KPj4+IENj OiBUaG9tYXMgSGVsbHN0cm9tIDx0aGVsbHN0cm9tQHZtd2FyZS5jb20+Cj4+PiBDYzogU2luY2xh aXIgWWVoIDxzeWVoQHZtd2FyZS5jb20+Cj4+PiBDYzogZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNr dG9wLm9yZwo+Pj4gU2lnbmVkLW9mZi1ieTogRGFuIFdpbGxpYW1zIDxkYW4uai53aWxsaWFtc0Bp bnRlbC5jb20+Cj4+IFdoaWxlIEkgaGF2ZSBub3RoaW5nIGFnYWluc3QgdGhlIGNvbnZlcnNpb24s IHdoYXQncyBzdG9wcGluZyB0aGUKPj4gY29tcGlsZXIgZnJvbSByZW9yZGVyaW5nIHdyaXRlcyBv biBhIGdlbmVyaWMgYXJjaGl0ZWN0dXJlIGFuZCBjYWNoaW5nCj4+IGFuZCByZW9yZGVyaW5nIHJl YWRzIG9uIHg4NiBpbiBwYXJ0aWN1bGFyPyBBdCB0aGUgdmVyeSBsZWFzdCBpdCBsb29rcyB0bwo+ PiBtZSBsaWtlIHRoZSBtZW1vcnkgYWNjZXNzZXMgb2YgdGhlIG1lbXJlbWFwJ2QgbWVtb3J5IG5l ZWRzIHRvIGJlCj4+IGVuY2Fwc3VsYXRlZCB3aXRoaW4gUkVBRF9PTkNFIGFuZCBXUklURV9PTkNF Lgo+IEhtbSwgY3VycmVudGx5IHRoZSBjb2RlIGlzIHVzaW5nIGlvcmVhZDMyL2lvd3JpdGUzMiB3 aGljaCBvbmx5IGRvCj4gdm9sYXRpbGUgYWNjZXNzZXMsIHdoZXJlYXMgUkVBRF9PTkNFIC8gV1JJ VEVfT05DRSBoYXZlIGEgbWVtb3J5Cj4gY2xvYmJlciBvbiBlbnRyeSBhbmQgZXhpdC4gIFNvLCBJ J20gYXNzdW1pbmcgYWxsIHlvdSBuZWVkIGlzIHRoZQo+IGd1YXJhbnRlZSBvZiAibm8gY29tcGls ZXIgcmUtb3JkZXJpbmciIGFuZCBub3QgdGhlIHN0cm9uZ2VyCj4gUkVBRF9PTkNFL1dSSVRFX09O Q0UgZ3VhcmFudGVlcywgYnV0IHRoYXQgc3RpbGwgc2VlbXMgYnJva2VuIGNvbXBhcmVkCj4gdG8g ZXhwbGljaXQgZmVuY2luZyB3aGVyZSBpdCBtYXR0ZXJzLgoKSSdtIG5vdCBxdWl0ZSBzdXJlIEkg Zm9sbG93IHlvdSBoZXJlLCBpdCBsb29rcyB0byBtZSBsaWtlIFJFQURfT05DRSgpCmFuZCBXUklU RV9PTkNFKCkgYXJlIGltcGxlbWVudGVkIGFzCnZvbGF0aWxlIGFjY2Vzc2VzLAoKaHR0cDovL2x4 ci5mcmVlLWVsZWN0cm9ucy5jb20vc291cmNlL2luY2x1ZGUvbGludXgvY29tcGlsZXIuaCNMMjE1 CgpqdXN0IGxpa2UgaW9yZWFkMzIgYW5kIGlvd3JpdGUzMgoKaHR0cDovL2x4ci5mcmVlLWVsZWN0 cm9ucy5jb20vc291cmNlL2luY2x1ZGUvYXNtLWdlbmVyaWMvaW8uaCNMNTQKCndoaWNoIHdvdWxk IG1pbmltaXplIGFueSBwb3RlbnRpYWwgaW1wYWN0IG9mIHRoaXMgY2hhbmdlLgpJTU8gb3B0aW1p emluZyB0aGUgbWVtb3J5IGFjY2Vzc2VzIGNhbiBiZSBkb25lIGFzIGEgbGF0ZXIgc3RlcC4KCi9U aG9tYXMKCgoKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f XwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcK aHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751546AbbJMShc (ORCPT ); Tue, 13 Oct 2015 14:37:32 -0400 Received: from smtp-outbound-1.vmware.com ([208.91.2.12]:49700 "EHLO smtp-outbound-1.vmware.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751039AbbJMShb convert rfc822-to-8bit (ORCPT ); Tue, 13 Oct 2015 14:37:31 -0400 Subject: Re: [PATCH] drm/vmwgfx: switch from ioremap_cache to memremap To: Dan Williams References: <20151012223527.34143.87313.stgit@dwillia2-desk3.jf.intel.com> <561C9427.8020001@vmware.com> CC: David Airlie , "linux-kernel@vger.kernel.org" , , Sinclair Yeh From: Thomas Hellstrom X-Enigmail-Draft-Status: N1110 Message-ID: <561D4F65.5080908@vmware.com> Date: Tue, 13 Oct 2015 20:37:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8BIT X-Originating-IP: [10.113.160.246] X-ClientProxiedBy: EX13-CAS-013.vmware.com (10.113.191.65) To EX13-MBX-024.vmware.com (10.113.191.44) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/13/2015 06:35 PM, Dan Williams wrote: > On Mon, Oct 12, 2015 at 10:18 PM, Thomas Hellstrom > wrote: >> Hi! >> >> On 10/13/2015 12:35 AM, Dan Williams wrote: >>> Per commit 2e586a7e017a "drm/vmwgfx: Map the fifo as cached" the driver >>> expects the fifo registers to be cacheable. In preparation for >>> deprecating ioremap_cache() convert its usage in vmwgfx to memremap(). >>> >>> Cc: David Airlie >>> Cc: Thomas Hellstrom >>> Cc: Sinclair Yeh >>> Cc: dri-devel@lists.freedesktop.org >>> Signed-off-by: Dan Williams >> While I have nothing against the conversion, what's stopping the >> compiler from reordering writes on a generic architecture and caching >> and reordering reads on x86 in particular? At the very least it looks to >> me like the memory accesses of the memremap'd memory needs to be >> encapsulated within READ_ONCE and WRITE_ONCE. > Hmm, currently the code is using ioread32/iowrite32 which only do > volatile accesses, whereas READ_ONCE / WRITE_ONCE have a memory > clobber on entry and exit. So, I'm assuming all you need is the > guarantee of "no compiler re-ordering" and not the stronger > READ_ONCE/WRITE_ONCE guarantees, but that still seems broken compared > to explicit fencing where it matters. I'm not quite sure I follow you here, it looks to me like READ_ONCE() and WRITE_ONCE() are implemented as volatile accesses, http://lxr.free-electrons.com/source/include/linux/compiler.h#L215 just like ioread32 and iowrite32 http://lxr.free-electrons.com/source/include/asm-generic/io.h#L54 which would minimize any potential impact of this change. IMO optimizing the memory accesses can be done as a later step. /Thomas