From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luis Chamberlain Date: Tue, 12 Nov 2019 22:17:44 +0000 Subject: Re: [PATCH] video: fbdev: atyfb: only use ioremap_uc() on i386 and ia64 Message-Id: <20191112221744.GN11244@42.do-not-panic.com> List-Id: References: <20191111192258.2234502-1-arnd@arndb.de> <20191112105507.GA7122@lst.de> <20191112140631.GA10922@lst.de> In-Reply-To: <20191112140631.GA10922@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Christoph Hellwig , Juergen Gross Cc: Fenghua Yu , Linux Fbdev development list , Tony Luck , linux-ia64@vger.kernel.org, Arnd Bergmann , Bartlomiej Zolnierkiewicz , Daniel Vetter , X86 ML , Linux Kernel Mailing List , dri-devel , Ingo Molnar , Borislav Petkov , Souptick Joarder , Tuowen Zhao , "H. Peter Anvin" , Thomas Gleixner , Andy Shevchenko , Mika Westerberg On Tue, Nov 12, 2019 at 03:06:31PM +0100, Christoph Hellwig wrote: > On Tue, Nov 12, 2019 at 02:04:16PM +0100, Daniel Vetter wrote: > > Wut ... Maybe I'm missing something, but from how we use mtrr in other > > gpu drivers it's a) either you use MTRR because that's all you got or > > b) you use pat. Mixing both sounds like a pretty bad idea, You misread the patch. And indeed there is a bit of complexity involved here folks should be aware of as .. well, its been a while. A mix of both MTRR and PAT is not effectively done on the code patch for the atyb driver. If you have PAT only PAT is used. If you don't have PAT a solution is provided to use MTRR. The goal of the patch really was to help finally avoid direct calls to MTRR. *This* driver was the *one* crazy exception where we needed to adddress this with a solution which would work effectively for both non-PAT and PAT world which had crazy constraints. So with this out of the way, no direct calls of MTRRs was possible and there are future possible gains with this for x86. The biggest two were: 1) Xen didn't have to implement MTRR hypervisor calls out for Linux guests. This means Xen guests don't have to enable MTRRs. Any code path avoiding such craziness as stop_machine() on each CPU during bootup, resume, CPU online and whenever an MTRR is set is a blessing. 2) We may be closer in the future to getting ioremap_nocache to use UC isntead of UC-, this would be a win. x86 ioremap_nocache() does not use UC (strong UC), it just uses UC-. Note though that BIOSes can *only* enable UC by using MTRR directly, fan control for a system was one use case example that can come up, just as an example. Ideally your BIOS won't need this. When and how this is done is platform and BIOS specific though. So effectively, if a BIOS enables MTRRs the Linux must keep them enabled. If the BIOS disables MTRRs the kernel keeps them disabled. > Can you take a look at "mfd: intel-lpss: Use devm_ioremap_uc for MMIO" > in linux-next, which also looks rather fishy to me? Can't we use > the MTRR APIs to override the broken BIOS MTRR setup there as well? The call there was put to allow precisely for such work around but also allow the code to work on PAT / non-PAT systems by using the same API. > With that we could kill ioremap_uc entirely. ioremap_uc() is a compromise to avoid direct calls to MTRRs, since ioremap_nocache() is not effectively yet using UC. Whether or not other archs carry it. Luis From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luis Chamberlain Date: Tue, 12 Nov 2019 22:17:44 +0000 Subject: Re: [PATCH] video: fbdev: atyfb: only use ioremap_uc() on i386 and ia64 Message-Id: <20191112221744.GN11244@42.do-not-panic.com> List-Id: References: <20191111192258.2234502-1-arnd@arndb.de> <20191112105507.GA7122@lst.de> <20191112140631.GA10922@lst.de> In-Reply-To: <20191112140631.GA10922@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Christoph Hellwig , Juergen Gross Cc: Daniel Vetter , Arnd Bergmann , Bartlomiej Zolnierkiewicz , X86 ML , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , linux-ia64@vger.kernel.org, Tony Luck , Fenghua Yu , Maarten Lankhorst , Souptick Joarder , dri-devel , Linux Fbdev development list , Linux Kernel Mailing List , Tuowen Zhao , Mika Westerberg , Andy Shevchenko On Tue, Nov 12, 2019 at 03:06:31PM +0100, Christoph Hellwig wrote: > On Tue, Nov 12, 2019 at 02:04:16PM +0100, Daniel Vetter wrote: > > Wut ... Maybe I'm missing something, but from how we use mtrr in other > > gpu drivers it's a) either you use MTRR because that's all you got or > > b) you use pat. Mixing both sounds like a pretty bad idea, You misread the patch. And indeed there is a bit of complexity involved here folks should be aware of as .. well, its been a while. A mix of both MTRR and PAT is not effectively done on the code patch for the atyb driver. If you have PAT only PAT is used. If you don't have PAT a solution is provided to use MTRR. The goal of the patch really was to help finally avoid direct calls to MTRR. *This* driver was the *one* crazy exception where we needed to adddress this with a solution which would work effectively for both non-PAT and PAT world which had crazy constraints. So with this out of the way, no direct calls of MTRRs was possible and there are future possible gains with this for x86. The biggest two were: 1) Xen didn't have to implement MTRR hypervisor calls out for Linux guests. This means Xen guests don't have to enable MTRRs. Any code path avoiding such craziness as stop_machine() on each CPU during bootup, resume, CPU online and whenever an MTRR is set is a blessing. 2) We may be closer in the future to getting ioremap_nocache to use UC isntead of UC-, this would be a win. x86 ioremap_nocache() does not use UC (strong UC), it just uses UC-. Note though that BIOSes can *only* enable UC by using MTRR directly, fan control for a system was one use case example that can come up, just as an example. Ideally your BIOS won't need this. When and how this is done is platform and BIOS specific though. So effectively, if a BIOS enables MTRRs the Linux must keep them enabled. If the BIOS disables MTRRs the kernel keeps them disabled. > Can you take a look at "mfd: intel-lpss: Use devm_ioremap_uc for MMIO" > in linux-next, which also looks rather fishy to me? Can't we use > the MTRR APIs to override the broken BIOS MTRR setup there as well? The call there was put to allow precisely for such work around but also allow the code to work on PAT / non-PAT systems by using the same API. > With that we could kill ioremap_uc entirely. ioremap_uc() is a compromise to avoid direct calls to MTRRs, since ioremap_nocache() is not effectively yet using UC. Whether or not other archs carry it. Luis From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luis Chamberlain Subject: Re: [PATCH] video: fbdev: atyfb: only use ioremap_uc() on i386 and ia64 Date: Tue, 12 Nov 2019 22:17:44 +0000 Message-ID: <20191112221744.GN11244@42.do-not-panic.com> References: <20191111192258.2234502-1-arnd@arndb.de> <20191112105507.GA7122@lst.de> <20191112140631.GA10922@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-pf1-f195.google.com (mail-pf1-f195.google.com [209.85.210.195]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3320D6EC01 for ; Tue, 12 Nov 2019 22:17:47 +0000 (UTC) Received: by mail-pf1-f195.google.com with SMTP id n13so99348pff.1 for ; Tue, 12 Nov 2019 14:17:47 -0800 (PST) Content-Disposition: inline In-Reply-To: <20191112140631.GA10922@lst.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Christoph Hellwig , Juergen Gross Cc: Fenghua Yu , Linux Fbdev development list , Tony Luck , linux-ia64@vger.kernel.org, Arnd Bergmann , Bartlomiej Zolnierkiewicz , Daniel Vetter , X86 ML , Linux Kernel Mailing List , dri-devel , Ingo Molnar , Borislav Petkov , Souptick Joarder , Tuowen Zhao , "H. Peter Anvin" , Thomas Gleixner , Andy Shevchenko , Mika Westerberg List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBOb3YgMTIsIDIwMTkgYXQgMDM6MDY6MzFQTSArMDEwMCwgQ2hyaXN0b3BoIEhlbGx3 aWcgd3JvdGU6Cj4gT24gVHVlLCBOb3YgMTIsIDIwMTkgYXQgMDI6MDQ6MTZQTSArMDEwMCwgRGFu aWVsIFZldHRlciB3cm90ZToKPiA+IFd1dCAuLi4gTWF5YmUgSSdtIG1pc3Npbmcgc29tZXRoaW5n LCBidXQgZnJvbSBob3cgd2UgdXNlIG10cnIgaW4gb3RoZXIKPiA+IGdwdSBkcml2ZXJzIGl0J3Mg YSkgZWl0aGVyIHlvdSB1c2UgTVRSUiBiZWNhdXNlIHRoYXQncyBhbGwgeW91IGdvdCBvcgo+ID4g YikgeW91IHVzZSBwYXQuIE1peGluZyBib3RoIHNvdW5kcyBsaWtlIGEgcHJldHR5IGJhZCBpZGVh LAoKWW91IG1pc3JlYWQgdGhlIHBhdGNoLiBBbmQgaW5kZWVkIHRoZXJlIGlzIGEgYml0IG9mIGNv bXBsZXhpdHkgaW52b2x2ZWQKaGVyZSBmb2xrcyBzaG91bGQgYmUgYXdhcmUgb2YgYXMgLi4gd2Vs bCwgaXRzIGJlZW4gYSB3aGlsZS4KCkEgbWl4IG9mIGJvdGggTVRSUiBhbmQgUEFUIGlzIG5vdCBl ZmZlY3RpdmVseSBkb25lIG9uIHRoZSBjb2RlIHBhdGNoIGZvcgp0aGUgYXR5YiBkcml2ZXIuIElm IHlvdSBoYXZlIFBBVCBvbmx5IFBBVCBpcyB1c2VkLiAgSWYgeW91IGRvbid0IGhhdmUKUEFUIGEg c29sdXRpb24gaXMgcHJvdmlkZWQgdG8gdXNlIE1UUlIuCgpUaGUgZ29hbCBvZiB0aGUgcGF0Y2gg cmVhbGx5IHdhcyB0byBoZWxwIGZpbmFsbHkgYXZvaWQgZGlyZWN0IGNhbGxzCnRvIE1UUlIuICpU aGlzKiBkcml2ZXIgd2FzIHRoZSAqb25lKiBjcmF6eSBleGNlcHRpb24gd2hlcmUgd2UgbmVlZGVk CnRvIGFkZGRyZXNzIHRoaXMgd2l0aCBhIHNvbHV0aW9uIHdoaWNoIHdvdWxkIHdvcmsgZWZmZWN0 aXZlbHkgZm9yIGJvdGgKbm9uLVBBVCBhbmQgUEFUIHdvcmxkIHdoaWNoIGhhZCBjcmF6eSBjb25z dHJhaW50cy4KClNvIHdpdGggdGhpcyBvdXQgb2YgdGhlIHdheSwgbm8gZGlyZWN0IGNhbGxzIG9m IE1UUlJzIHdhcyBwb3NzaWJsZSBhbmQKdGhlcmUgYXJlIGZ1dHVyZSBwb3NzaWJsZSBnYWlucyB3 aXRoIHRoaXMgZm9yIHg4Ni4gVGhlIGJpZ2dlc3QgdHdvIHdlcmU6CgogIDEpIFhlbiBkaWRuJ3Qg aGF2ZSB0byBpbXBsZW1lbnQgTVRSUiBoeXBlcnZpc29yIGNhbGxzIG91dCBmb3IgTGludXgKICAg ICBndWVzdHMuIFRoaXMgbWVhbnMgWGVuIGd1ZXN0cyBkb24ndCBoYXZlIHRvIGVuYWJsZSBNVFJS cy4gQW55IGNvZGUKICAgICBwYXRoIGF2b2lkaW5nIHN1Y2ggY3JhemluZXNzIGFzIHN0b3BfbWFj aGluZSgpIG9uIGVhY2ggQ1BVIGR1cmluZwogICAgIGJvb3R1cCwgcmVzdW1lLCBDUFUgb25saW5l IGFuZCB3aGVuZXZlciBhbiBNVFJSIGlzIHNldCBpcyBhIGJsZXNzaW5nLgoKICAyKSBXZSBtYXkg YmUgY2xvc2VyIGluIHRoZSBmdXR1cmUgdG8gZ2V0dGluZyBpb3JlbWFwX25vY2FjaGUgdG8gdXNl CiAgICAgVUMgaXNudGVhZCBvZiBVQy0sIHRoaXMgd291bGQgYmUgYSB3aW4uIHg4NiBpb3JlbWFw X25vY2FjaGUoKSBkb2VzCiAgICAgbm90IHVzZSBVQyAoc3Ryb25nIFVDKSwgaXQganVzdCB1c2Vz IFVDLS4KCk5vdGUgdGhvdWdoIHRoYXQgQklPU2VzIGNhbiAqb25seSogZW5hYmxlIFVDIGJ5IHVz aW5nIE1UUlIgZGlyZWN0bHksIGZhbgpjb250cm9sIGZvciBhIHN5c3RlbSB3YXMgb25lIHVzZSBj YXNlIGV4YW1wbGUgdGhhdCBjYW4gY29tZSB1cCwganVzdCBhcwphbiBleGFtcGxlLiBJZGVhbGx5 IHlvdXIgQklPUyB3b24ndCBuZWVkIHRoaXMuIFdoZW4gYW5kIGhvdyB0aGlzIGlzIGRvbmUKaXMg cGxhdGZvcm0gYW5kIEJJT1Mgc3BlY2lmaWMgdGhvdWdoLiBTbyBlZmZlY3RpdmVseSwgaWYgYSBC SU9TIGVuYWJsZXMKTVRSUnMgdGhlIExpbnV4IG11c3Qga2VlcCB0aGVtIGVuYWJsZWQuIElmIHRo ZSBCSU9TIGRpc2FibGVzIE1UUlJzIHRoZQprZXJuZWwga2VlcHMgdGhlbSBkaXNhYmxlZC4KCj4g Q2FuIHlvdSB0YWtlIGEgbG9vayBhdCAibWZkOiBpbnRlbC1scHNzOiBVc2UgZGV2bV9pb3JlbWFw X3VjIGZvciBNTUlPIgo+IGluIGxpbnV4LW5leHQsIHdoaWNoIGFsc28gbG9va3MgcmF0aGVyIGZp c2h5IHRvIG1lPyAgQ2FuJ3Qgd2UgdXNlCj4gdGhlIE1UUlIgQVBJcyB0byBvdmVycmlkZSB0aGUg YnJva2VuIEJJT1MgTVRSUiBzZXR1cCB0aGVyZSBhcyB3ZWxsPwoKVGhlIGNhbGwgdGhlcmUgd2Fz IHB1dCB0byBhbGxvdyBwcmVjaXNlbHkgZm9yIHN1Y2ggd29yayBhcm91bmQgYnV0IGFsc28KYWxs b3cgdGhlIGNvZGUgdG8gd29yayBvbiBQQVQgLyBub24tUEFUIHN5c3RlbXMgYnkgdXNpbmcgdGhl IHNhbWUgQVBJLgoKPiBXaXRoIHRoYXQgd2UgY291bGQga2lsbCBpb3JlbWFwX3VjIGVudGlyZWx5 LgoKaW9yZW1hcF91YygpIGlzIGEgY29tcHJvbWlzZSB0byBhdm9pZCBkaXJlY3QgY2FsbHMgdG8g TVRSUnMsIHNpbmNlCmlvcmVtYXBfbm9jYWNoZSgpIGlzIG5vdCBlZmZlY3RpdmVseSB5ZXQgdXNp bmcgVUMuIFdoZXRoZXIgb3Igbm90Cm90aGVyIGFyY2hzIGNhcnJ5IGl0LgoKICBMdWlzCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWls aW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZy ZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbA==