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:52:46 +0200 Message-ID: <561D52FE.5080304@vmware.com> References: <20151012223527.34143.87313.stgit@dwillia2-desk3.jf.intel.com> <561C9427.8020001@vmware.com> <561D4F65.5080908@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 E9F8F6E0D1 for ; Tue, 13 Oct 2015 11:52:51 -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 T24gMTAvMTMvMjAxNSAwODo0OCBQTSwgRGFuIFdpbGxpYW1zIHdyb3RlOgo+IE9uIFR1ZSwgT2N0 IDEzLCAyMDE1IGF0IDExOjM3IEFNLCBUaG9tYXMgSGVsbHN0cm9tCj4gPHRoZWxsc3Ryb21Adm13 YXJlLmNvbT4gd3JvdGU6Cj4+IE9uIDEwLzEzLzIwMTUgMDY6MzUgUE0sIERhbiBXaWxsaWFtcyB3 cm90ZToKPj4+IE9uIE1vbiwgT2N0IDEyLCAyMDE1IGF0IDEwOjE4IFBNLCBUaG9tYXMgSGVsbHN0 cm9tCj4+PiA8dGhlbGxzdHJvbUB2bXdhcmUuY29tPiB3cm90ZToKPj4+PiBIaSEKPj4+Pgo+Pj4+ IE9uIDEwLzEzLzIwMTUgMTI6MzUgQU0sIERhbiBXaWxsaWFtcyB3cm90ZToKPj4+Pj4gUGVyIGNv bW1pdCAyZTU4NmE3ZTAxN2EgImRybS92bXdnZng6IE1hcCB0aGUgZmlmbyBhcyBjYWNoZWQiIHRo ZSBkcml2ZXIKPj4+Pj4gZXhwZWN0cyB0aGUgZmlmbyByZWdpc3RlcnMgdG8gYmUgY2FjaGVhYmxl LiAgSW4gcHJlcGFyYXRpb24gZm9yCj4+Pj4+IGRlcHJlY2F0aW5nIGlvcmVtYXBfY2FjaGUoKSBj b252ZXJ0IGl0cyB1c2FnZSBpbiB2bXdnZnggdG8gbWVtcmVtYXAoKS4KPj4+Pj4KPj4+Pj4gQ2M6 IERhdmlkIEFpcmxpZSA8YWlybGllZEBsaW51eC5pZT4KPj4+Pj4gQ2M6IFRob21hcyBIZWxsc3Ry b20gPHRoZWxsc3Ryb21Adm13YXJlLmNvbT4KPj4+Pj4gQ2M6IFNpbmNsYWlyIFllaCA8c3llaEB2 bXdhcmUuY29tPgo+Pj4+PiBDYzogZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwo+Pj4+ PiBTaWduZWQtb2ZmLWJ5OiBEYW4gV2lsbGlhbXMgPGRhbi5qLndpbGxpYW1zQGludGVsLmNvbT4K Pj4+PiBXaGlsZSBJIGhhdmUgbm90aGluZyBhZ2FpbnN0IHRoZSBjb252ZXJzaW9uLCB3aGF0J3Mg c3RvcHBpbmcgdGhlCj4+Pj4gY29tcGlsZXIgZnJvbSByZW9yZGVyaW5nIHdyaXRlcyBvbiBhIGdl bmVyaWMgYXJjaGl0ZWN0dXJlIGFuZCBjYWNoaW5nCj4+Pj4gYW5kIHJlb3JkZXJpbmcgcmVhZHMg b24geDg2IGluIHBhcnRpY3VsYXI/IEF0IHRoZSB2ZXJ5IGxlYXN0IGl0IGxvb2tzIHRvCj4+Pj4g bWUgbGlrZSB0aGUgbWVtb3J5IGFjY2Vzc2VzIG9mIHRoZSBtZW1yZW1hcCdkIG1lbW9yeSBuZWVk cyB0byBiZQo+Pj4+IGVuY2Fwc3VsYXRlZCB3aXRoaW4gUkVBRF9PTkNFIGFuZCBXUklURV9PTkNF Lgo+Pj4gSG1tLCBjdXJyZW50bHkgdGhlIGNvZGUgaXMgdXNpbmcgaW9yZWFkMzIvaW93cml0ZTMy IHdoaWNoIG9ubHkgZG8KPj4+IHZvbGF0aWxlIGFjY2Vzc2VzLCB3aGVyZWFzIFJFQURfT05DRSAv IFdSSVRFX09OQ0UgaGF2ZSBhIG1lbW9yeQo+Pj4gY2xvYmJlciBvbiBlbnRyeSBhbmQgZXhpdC4g IFNvLCBJJ20gYXNzdW1pbmcgYWxsIHlvdSBuZWVkIGlzIHRoZQo+Pj4gZ3VhcmFudGVlIG9mICJu byBjb21waWxlciByZS1vcmRlcmluZyIgYW5kIG5vdCB0aGUgc3Ryb25nZXIKPj4+IFJFQURfT05D RS9XUklURV9PTkNFIGd1YXJhbnRlZXMsIGJ1dCB0aGF0IHN0aWxsIHNlZW1zIGJyb2tlbiBjb21w YXJlZAo+Pj4gdG8gZXhwbGljaXQgZmVuY2luZyB3aGVyZSBpdCBtYXR0ZXJzLgo+PiBJJ20gbm90 IHF1aXRlIHN1cmUgSSBmb2xsb3cgeW91IGhlcmUsIGl0IGxvb2tzIHRvIG1lIGxpa2UgUkVBRF9P TkNFKCkKPj4gYW5kIFdSSVRFX09OQ0UoKSBhcmUgaW1wbGVtZW50ZWQgYXMKPj4gdm9sYXRpbGUg YWNjZXNzZXMsCj4gQWgsIHNvcnJ5LCBJIHdhcyBsb29raW5nIGF0IHRoZSBkZWZhdWx0IGNhc2Uu Li4KPgo+PiBodHRwczovL3VybGRlZmVuc2UucHJvb2Zwb2ludC5jb20vdjIvdXJsP3U9aHR0cC0z QV9fbHhyLmZyZWUtMkRlbGVjdHJvbnMuY29tX3NvdXJjZV9pbmNsdWRlX2xpbnV4X2NvbXBpbGVy LmgtMjNMMjE1JmQ9QlFJQmFRJmM9U3FjbDBFejZNMFg4YWVNNjdMS0lpREpBWFZlQXctWWloVk1O dFh0LXVFcyZyPXZwdWtQa0J0cG9OUXAySVVLdUZ2aU9tUE5ZV1ZLbWVuM0plZXU1NXptRUEmbT1K UnhlYm1qY1I0Si15aEQwd1JPaktyQUt5dG81T2VldEl2cXQ3TXFWX1dBJnM9em43WW1uUzc0empN M1NkNURwOW1ablNMMjdqcWVsNmN3Ukh3WVY2Z1UzVSZlPSAKPj4KPj4ganVzdCBsaWtlIGlvcmVh ZDMyIGFuZCBpb3dyaXRlMzIKPj4KPj4gaHR0cHM6Ly91cmxkZWZlbnNlLnByb29mcG9pbnQuY29t L3YyL3VybD91PWh0dHAtM0FfX2x4ci5mcmVlLTJEZWxlY3Ryb25zLmNvbV9zb3VyY2VfaW5jbHVk ZV9hc20tMkRnZW5lcmljX2lvLmgtMjNMNTQmZD1CUUlCYVEmYz1TcWNsMEV6Nk0wWDhhZU02N0xL SWlESkFYVmVBdy1ZaWhWTU50WHQtdUVzJnI9dnB1a1BrQnRwb05RcDJJVUt1RnZpT21QTllXVktt ZW4zSmVldTU1em1FQSZtPUpSeGVibWpjUjRKLXloRDB3Uk9qS3JBS3l0bzVPZWV0SXZxdDdNcVZf V0Emcz15NGREMkdVcGNaVkhsam5UaFl1Z0YtWUxUZ2VQNkVuNEp3b09ua2FMZzdBJmU9IAo+Pgo+ PiB3aGljaCB3b3VsZCBtaW5pbWl6ZSBhbnkgcG90ZW50aWFsIGltcGFjdCBvZiB0aGlzIGNoYW5n ZS4KPj4gSU1PIG9wdGltaXppbmcgdGhlIG1lbW9yeSBhY2Nlc3NlcyBjYW4gYmUgZG9uZSBhcyBh IGxhdGVyIHN0ZXAuCj4+Cj4gT2ssIEknbGwgbWFrZSBsb2NhbCByZWFkX2ZpZm8oKSBhbmQgd3Jp dGVfZmlmbygpIG1hY3JvcyB0byBtYWtlIHRoaXMKPiBleHBsaWNpdC4gIEFyZSB0aGVzZSBuYW1l cyBvayB3aXRoIHlvdT8KClN1cmUuCgpUaGFua3MsClRob21hcwoKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmkt ZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcv bWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752187AbbJMSwy (ORCPT ); Tue, 13 Oct 2015 14:52:54 -0400 Received: from smtp-outbound-1.vmware.com ([208.91.2.12]:50513 "EHLO smtp-outbound-1.vmware.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752105AbbJMSww (ORCPT ); Tue, 13 Oct 2015 14:52:52 -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> <561D4F65.5080908@vmware.com> CC: David Airlie , "linux-kernel@vger.kernel.org" , , Sinclair Yeh From: Thomas Hellstrom Message-ID: <561D52FE.5080304@vmware.com> Date: Tue, 13 Oct 2015 20:52:46 +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: 7bit 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 08:48 PM, Dan Williams wrote: > On Tue, Oct 13, 2015 at 11:37 AM, Thomas Hellstrom > wrote: >> 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, > Ah, sorry, I was looking at the default case... > >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lxr.free-2Delectrons.com_source_include_linux_compiler.h-23L215&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=JRxebmjcR4J-yhD0wROjKrAKyto5OeetIvqt7MqV_WA&s=zn7YmnS74zjM3Sd5Dp9mZnSL27jqel6cwRHwYV6gU3U&e= >> >> just like ioread32 and iowrite32 >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lxr.free-2Delectrons.com_source_include_asm-2Dgeneric_io.h-23L54&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=JRxebmjcR4J-yhD0wROjKrAKyto5OeetIvqt7MqV_WA&s=y4dD2GUpcZVHljnThYugF-YLTgeP6En4JwoOnkaLg7A&e= >> >> which would minimize any potential impact of this change. >> IMO optimizing the memory accesses can be done as a later step. >> > Ok, I'll make local read_fifo() and write_fifo() macros to make this > explicit. Are these names ok with you? Sure. Thanks, Thomas