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 07:18:31 +0200 Message-ID: <561C9427.8020001@vmware.com> References: <20151012223527.34143.87313.stgit@dwillia2-desk3.jf.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from smtp-outbound-2.vmware.com (smtp-outbound-2.vmware.com [208.91.2.13]) by gabe.freedesktop.org (Postfix) with ESMTPS id 59F786E535 for ; Mon, 12 Oct 2015 22:18:36 -0700 (PDT) In-Reply-To: <20151012223527.34143.87313.stgit@dwillia2-desk3.jf.intel.com> 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 SGkhCgpPbiAxMC8xMy8yMDE1IDEyOjM1IEFNLCBEYW4gV2lsbGlhbXMgd3JvdGU6Cj4gUGVyIGNv bW1pdCAyZTU4NmE3ZTAxN2EgImRybS92bXdnZng6IE1hcCB0aGUgZmlmbyBhcyBjYWNoZWQiIHRo ZSBkcml2ZXIKPiBleHBlY3RzIHRoZSBmaWZvIHJlZ2lzdGVycyB0byBiZSBjYWNoZWFibGUuICBJ biBwcmVwYXJhdGlvbiBmb3IKPiBkZXByZWNhdGluZyBpb3JlbWFwX2NhY2hlKCkgY29udmVydCBp dHMgdXNhZ2UgaW4gdm13Z2Z4IHRvIG1lbXJlbWFwKCkuCj4KPiBDYzogRGF2aWQgQWlybGllIDxh aXJsaWVkQGxpbnV4LmllPgo+IENjOiBUaG9tYXMgSGVsbHN0cm9tIDx0aGVsbHN0cm9tQHZtd2Fy ZS5jb20+Cj4gQ2M6IFNpbmNsYWlyIFllaCA8c3llaEB2bXdhcmUuY29tPgo+IENjOiBkcmktZGV2 ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4gU2lnbmVkLW9mZi1ieTogRGFuIFdpbGxpYW1zIDxk YW4uai53aWxsaWFtc0BpbnRlbC5jb20+CgpXaGlsZSBJIGhhdmUgbm90aGluZyBhZ2FpbnN0IHRo ZSBjb252ZXJzaW9uLCB3aGF0J3Mgc3RvcHBpbmcgdGhlCmNvbXBpbGVyIGZyb20gcmVvcmRlcmlu ZyB3cml0ZXMgb24gYSBnZW5lcmljIGFyY2hpdGVjdHVyZSBhbmQgY2FjaGluZwphbmQgcmVvcmRl cmluZyByZWFkcyBvbiB4ODYgaW4gcGFydGljdWxhcj8gQXQgdGhlIHZlcnkgbGVhc3QgaXQgbG9v a3MgdG8KbWUgbGlrZSB0aGUgbWVtb3J5IGFjY2Vzc2VzIG9mIHRoZSBtZW1yZW1hcCdkIG1lbW9y eSBuZWVkcyB0byBiZQplbmNhcHN1bGF0ZWQgd2l0aGluIFJFQURfT05DRSBhbmQgV1JJVEVfT05D RS4KCkhvdyBpcyB0aGlzIGhhbmRsZWQgaW4gdGhlIG90aGVyIGNvbnZlcnNpb25zPwoKVGhhbmtz LApUaG9tYXMKCgoKCgo+IC0tLQo+Cj4gIFRoaXMgaXMgcGFydCBvZiB0aGUgdHJlZS13aWRlIGNv bnZlcnNpb24gb2YgaW9yZW1hcF9jYWNoZSgpIHRvCj4gIG1lbXJlbWFwKCkgdGFyZ2V0ZWQgZm9y IHY0LjQgWzFdCj4gIAo+ICBBcyBub3RlZCBpbiB0aGF0IGNvdmVyIGxldHRlciBmZWVsIGZyZWUg dG8gYXBwbHkgb3IgYWNrLiAgVGhlc2UKPiAgaW5kaXZpZHVhbCBjb252ZXJzaW9ucyBjYW4gZ28g aW4gaW5kZXBlbmRlbnRseSBnaXZlbiB0aGUgYmFzZSBtZW1yZW1hcCgpCj4gIGltcGxlbWVudGF0 aW9uIGlzIGFscmVhZHkgdXBzdHJlYW0gaW4gdjQuMy1yYzEuCj4gIAo+ICBUaGlzIHBhc3NlcyBh IGNsZWFuIHJ1biBvZiBzcGFyc2UsIGJ1dCBJIGhhdmUgbm90IHRyaWVkIHRvIHJ1biB0aGUKPiAg cmVzdWx0Lgo+ICAKPiAgWzFdOiBodHRwczovL3VybGRlZmVuc2UucHJvb2Zwb2ludC5jb20vdjIv dXJsP3U9aHR0cHMtM0FfX2xrbWwub3JnX2xrbWxfMjAxNV8xMF85XzY5OSZkPUJRSUNhUSZjPVNx Y2wwRXo2TTBYOGFlTTY3TEtJaURKQVhWZUF3LVlpaFZNTnRYdC11RXMmcj12cHVrUGtCdHBvTlFw MklVS3VGdmlPbVBOWVdWS21lbjNKZWV1NTV6bUVBJm09WHVWdFEzXzI4amluNWhkSzZ3QmNMaWdF aVJjLTFUdWVsWWEzem05NG00NCZzPWtOMy0yWFN0V1dVMGYyMHdOR3BtUWl3WlRUaUJCendENExT aHZmb2t3a1EmZT0gCj4KPiAgZHJpdmVycy9ncHUvZHJtL3Ztd2dmeC92bXdnZnhfZHJ2LmMgICB8 ICAgIDggKy0tCj4gIGRyaXZlcnMvZ3B1L2RybS92bXdnZngvdm13Z2Z4X2Rydi5oICAgfCAgICAy IC0KPiAgZHJpdmVycy9ncHUvZHJtL3Ztd2dmeC92bXdnZnhfZmVuY2UuYyB8ICAgMjMgKysrKy0t LQo+ICBkcml2ZXJzL2dwdS9kcm0vdm13Z2Z4L3Ztd2dmeF9maWZvLmMgIHwgIDEwMiArKysrKysr KysrKysrKysrLS0tLS0tLS0tLS0tLS0tLS0KPiAgZHJpdmVycy9ncHUvZHJtL3Ztd2dmeC92bXdn ZnhfaW9jdGwuYyB8ICAgIDkgKy0tCj4gIGRyaXZlcnMvZ3B1L2RybS92bXdnZngvdm13Z2Z4X2ly cS5jICAgfCAgICA4ICstLQo+ICBkcml2ZXJzL2dwdS9kcm0vdm13Z2Z4L3Ztd2dmeF9rbXMuYyAg IHwgICAyNCArKysrLS0tLQo+ICA3IGZpbGVzIGNoYW5nZWQsIDg0IGluc2VydGlvbnMoKyksIDky IGRlbGV0aW9ucygtKQo+Cj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS92bXdnZngvdm13 Z2Z4X2Rydi5jIGIvZHJpdmVycy9ncHUvZHJtL3Ztd2dmeC92bXdnZnhfZHJ2LmMKPiBpbmRleCAy YzdhMjVjNzFhZjIuLmQ2MTUyY2Q3YzYzNCAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0v dm13Z2Z4L3Ztd2dmeF9kcnYuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS92bXdnZngvdm13Z2Z4 X2Rydi5jCj4gQEAgLTc1Miw4ICs3NTIsOCBAQCBzdGF0aWMgaW50IHZtd19kcml2ZXJfbG9hZChz dHJ1Y3QgZHJtX2RldmljZSAqZGV2LCB1bnNpZ25lZCBsb25nIGNoaXBzZXQpCj4gIAl0dG1fbG9j a19zZXRfa2lsbCgmZGV2X3ByaXYtPmZiZGV2X21hc3Rlci5sb2NrLCBmYWxzZSwgU0lHVEVSTSk7 Cj4gIAlkZXZfcHJpdi0+YWN0aXZlX21hc3RlciA9ICZkZXZfcHJpdi0+ZmJkZXZfbWFzdGVyOwo+ ICAKPiAtCWRldl9wcml2LT5tbWlvX3ZpcnQgPSBpb3JlbWFwX2NhY2hlKGRldl9wcml2LT5tbWlv X3N0YXJ0LAo+IC0JCQkJCSAgICBkZXZfcHJpdi0+bW1pb19zaXplKTsKPiArCWRldl9wcml2LT5t bWlvX3ZpcnQgPSBtZW1yZW1hcChkZXZfcHJpdi0+bW1pb19zdGFydCwKPiArCQkJCSAgICAgICBk ZXZfcHJpdi0+bW1pb19zaXplLCBNRU1SRU1BUF9XQik7Cj4gIAo+ICAJaWYgKHVubGlrZWx5KGRl dl9wcml2LT5tbWlvX3ZpcnQgPT0gTlVMTCkpIHsKPiAgCQlyZXQgPSAtRU5PTUVNOwo+IEBAIC05 MDcsNyArOTA3LDcgQEAgb3V0X25vX2lycToKPiAgb3V0X25vX2RldmljZToKPiAgCXR0bV9vYmpl Y3RfZGV2aWNlX3JlbGVhc2UoJmRldl9wcml2LT50ZGV2KTsKPiAgb3V0X2VycjQ6Cj4gLQlpb3Vu bWFwKGRldl9wcml2LT5tbWlvX3ZpcnQpOwo+ICsJbWVtdW5tYXAoZGV2X3ByaXYtPm1taW9fdmly dCk7Cj4gIG91dF9lcnIzOgo+ICAJdm13X3R0bV9nbG9iYWxfcmVsZWFzZShkZXZfcHJpdik7Cj4g IG91dF9lcnIwOgo+IEBAIC05NTgsNyArOTU4LDcgQEAgc3RhdGljIGludCB2bXdfZHJpdmVyX3Vu bG9hZChzdHJ1Y3QgZHJtX2RldmljZSAqZGV2KQo+ICAJCXBjaV9yZWxlYXNlX3JlZ2lvbnMoZGV2 LT5wZGV2KTsKPiAgCj4gIAl0dG1fb2JqZWN0X2RldmljZV9yZWxlYXNlKCZkZXZfcHJpdi0+dGRl dik7Cj4gLQlpb3VubWFwKGRldl9wcml2LT5tbWlvX3ZpcnQpOwo+ICsJbWVtdW5tYXAoZGV2X3By aXYtPm1taW9fdmlydCk7Cj4gIAlpZiAoZGV2X3ByaXYtPmN0eC5zdGFnZWRfYmluZGluZ3MpCj4g IAkJdm13X2JpbmRpbmdfc3RhdGVfZnJlZShkZXZfcHJpdi0+Y3R4LnN0YWdlZF9iaW5kaW5ncyk7 Cj4gIAl2bXdfdHRtX2dsb2JhbF9yZWxlYXNlKGRldl9wcml2KTsKPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy9ncHUvZHJtL3Ztd2dmeC92bXdnZnhfZHJ2LmggYi9kcml2ZXJzL2dwdS9kcm0vdm13Z2Z4 L3Ztd2dmeF9kcnYuaAo+IGluZGV4IGYxOWZkMzliNDNlMS4uN2ZmMWRiMjNkZTgwIDEwMDY0NAo+ IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS92bXdnZngvdm13Z2Z4X2Rydi5oCj4gKysrIGIvZHJpdmVy cy9ncHUvZHJtL3Ztd2dmeC92bXdnZnhfZHJ2LmgKPiBAQCAtMzc1LDcgKzM3NSw3IEBAIHN0cnVj dCB2bXdfcHJpdmF0ZSB7Cj4gIAl1aW50MzJfdCBzdGR1X21heF9oZWlnaHQ7Cj4gIAl1aW50MzJf dCBpbml0aWFsX3dpZHRoOwo+ICAJdWludDMyX3QgaW5pdGlhbF9oZWlnaHQ7Cj4gLQl1MzIgX19p b21lbSAqbW1pb192aXJ0Owo+ICsJdTMyICptbWlvX3ZpcnQ7Cj4gIAl1aW50MzJfdCBjYXBhYmls aXRpZXM7Cj4gIAl1aW50MzJfdCBtYXhfZ21yX2lkczsKPiAgCXVpbnQzMl90IG1heF9nbXJfcGFn ZXM7Cj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS92bXdnZngvdm13Z2Z4X2ZlbmNlLmMg Yi9kcml2ZXJzL2dwdS9kcm0vdm13Z2Z4L3Ztd2dmeF9mZW5jZS5jCj4gaW5kZXggNTY3ZGRlZGU1 MWQxLi45N2Y3NWFkYzA4MGQgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL3Ztd2dmeC92 bXdnZnhfZmVuY2UuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS92bXdnZngvdm13Z2Z4X2ZlbmNl LmMKPiBAQCAtMTQxLDkgKzE0MSw5IEBAIHN0YXRpYyBib29sIHZtd19mZW5jZV9lbmFibGVfc2ln bmFsaW5nKHN0cnVjdCBmZW5jZSAqZikKPiAgCj4gIAlzdHJ1Y3Qgdm13X2ZlbmNlX21hbmFnZXIg KmZtYW4gPSBmbWFuX2Zyb21fZmVuY2UoZmVuY2UpOwo+ICAJc3RydWN0IHZtd19wcml2YXRlICpk ZXZfcHJpdiA9IGZtYW4tPmRldl9wcml2Owo+ICsJdTMyICpmaWZvX21lbSA9IGRldl9wcml2LT5t bWlvX3ZpcnQ7Cj4gKwl1MzIgc2Vxbm8gPSAqKGZpZm9fbWVtICsgU1ZHQV9GSUZPX0ZFTkNFKTsK PiAgCj4gLQl1MzIgX19pb21lbSAqZmlmb19tZW0gPSBkZXZfcHJpdi0+bW1pb192aXJ0Owo+IC0J dTMyIHNlcW5vID0gaW9yZWFkMzIoZmlmb19tZW0gKyBTVkdBX0ZJRk9fRkVOQ0UpOwo+ICAJaWYg KHNlcW5vIC0gZmVuY2UtPmJhc2Uuc2Vxbm8gPCBWTVdfRkVOQ0VfV1JBUCkKPiAgCQlyZXR1cm4g ZmFsc2U7Cj4gIAo+CkhlcmUsIGZvciBleGFtcGxlLgoKL1Rob21hcwoKCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QK ZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Au b3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932454AbbJMFSh (ORCPT ); Tue, 13 Oct 2015 01:18:37 -0400 Received: from smtp-outbound-2.vmware.com ([208.91.2.13]:39456 "EHLO smtp-outbound-2.vmware.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752345AbbJMFSg convert rfc822-to-8bit (ORCPT ); Tue, 13 Oct 2015 01:18:36 -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> CC: David Airlie , , , Sinclair Yeh From: Thomas Hellstrom X-Enigmail-Draft-Status: N1110 Message-ID: <561C9427.8020001@vmware.com> Date: Tue, 13 Oct 2015 07:18:31 +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: <20151012223527.34143.87313.stgit@dwillia2-desk3.jf.intel.com> 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 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. How is this handled in the other conversions? Thanks, Thomas > --- > > This is part of the tree-wide conversion of ioremap_cache() to > memremap() targeted for v4.4 [1] > > As noted in that cover letter feel free to apply or ack. These > individual conversions can go in independently given the base memremap() > implementation is already upstream in v4.3-rc1. > > This passes a clean run of sparse, but I have not tried to run the > result. > > [1]: https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2015_10_9_699&d=BQICaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=XuVtQ3_28jin5hdK6wBcLigEiRc-1TuelYa3zm94m44&s=kN3-2XStWWU0f20wNGpmQiwZTTiBBzwD4LShvfokwkQ&e= > > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 8 +-- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 - > drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 23 ++++--- > drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c | 102 ++++++++++++++++----------------- > drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 9 +-- > drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8 +-- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 24 ++++---- > 7 files changed, 84 insertions(+), 92 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index 2c7a25c71af2..d6152cd7c634 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -752,8 +752,8 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) > ttm_lock_set_kill(&dev_priv->fbdev_master.lock, false, SIGTERM); > dev_priv->active_master = &dev_priv->fbdev_master; > > - dev_priv->mmio_virt = ioremap_cache(dev_priv->mmio_start, > - dev_priv->mmio_size); > + dev_priv->mmio_virt = memremap(dev_priv->mmio_start, > + dev_priv->mmio_size, MEMREMAP_WB); > > if (unlikely(dev_priv->mmio_virt == NULL)) { > ret = -ENOMEM; > @@ -907,7 +907,7 @@ out_no_irq: > out_no_device: > ttm_object_device_release(&dev_priv->tdev); > out_err4: > - iounmap(dev_priv->mmio_virt); > + memunmap(dev_priv->mmio_virt); > out_err3: > vmw_ttm_global_release(dev_priv); > out_err0: > @@ -958,7 +958,7 @@ static int vmw_driver_unload(struct drm_device *dev) > pci_release_regions(dev->pdev); > > ttm_object_device_release(&dev_priv->tdev); > - iounmap(dev_priv->mmio_virt); > + memunmap(dev_priv->mmio_virt); > if (dev_priv->ctx.staged_bindings) > vmw_binding_state_free(dev_priv->ctx.staged_bindings); > vmw_ttm_global_release(dev_priv); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index f19fd39b43e1..7ff1db23de80 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -375,7 +375,7 @@ struct vmw_private { > uint32_t stdu_max_height; > uint32_t initial_width; > uint32_t initial_height; > - u32 __iomem *mmio_virt; > + u32 *mmio_virt; > uint32_t capabilities; > uint32_t max_gmr_ids; > uint32_t max_gmr_pages; > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > index 567ddede51d1..97f75adc080d 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > @@ -141,9 +141,9 @@ static bool vmw_fence_enable_signaling(struct fence *f) > > struct vmw_fence_manager *fman = fman_from_fence(fence); > struct vmw_private *dev_priv = fman->dev_priv; > + u32 *fifo_mem = dev_priv->mmio_virt; > + u32 seqno = *(fifo_mem + SVGA_FIFO_FENCE); > > - u32 __iomem *fifo_mem = dev_priv->mmio_virt; > - u32 seqno = ioread32(fifo_mem + SVGA_FIFO_FENCE); > if (seqno - fence->base.seqno < VMW_FENCE_WRAP) > return false; > > Here, for example. /Thomas