From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luis Chamberlain Date: Tue, 12 Nov 2019 22:24:23 +0000 Subject: Re: [PATCH] video: fbdev: atyfb: only use ioremap_uc() on i386 and ia64 Message-Id: <20191112222423.GO11244@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: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Daniel Vetter , Juergen Gross Cc: Linux Fbdev development list , linux-ia64@vger.kernel.org, dri-devel , "H. Peter Anvin" , Lee Jones , Christoph Hellwig , X86 ML , Ingo Molnar , Tuowen Zhao , Fenghua Yu , Arnd Bergmann , Bartlomiej Zolnierkiewicz , "Luis R. Rodriguez" , Borislav Petkov , Thomas Gleixner , Andy Shevchenko , Mika Westerberg , Tony Luck , Linux Kernel Mailing List , AceLan Kao , Souptick Joarder , Roman Gilg On Tue, Nov 12, 2019 at 03:26:35PM +0100, Daniel Vetter wrote: > On Tue, Nov 12, 2019 at 3:06 PM 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, since if > > > you need MTRR for performance (because you dont have PAT) then you > > > can't fix the wc with the PAT-based ioremap_uc. And if you have PAT, > > > then you don't really need an MTRR to get wc. > > > > > > So I'd revert this patch from Luis and ... > > > > Sounds great to me.. > > > > > ... apply this one. Since the same reasoning should apply to anything > > > that's running on any cpu with PAT. > > > > 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? > > Hm so that's way out of my knowledge, but I think mtrr_cleanup() was > supposed to fix up messy/broken MTRR setups by the bios. So maybe they > simply didn't enable that in their .config with CONFIG_MTRR_SANITIZER. I had originally suggested to just make the driver build on x86, but an atlternative was to provide the call for the missing architecture. > An explicit cleanup is currently not possible for drivers, since the > only interface exported to drivers is arch_phys_wc_add/del (which > short-circuits if pat works since you don't need mtrr in that case). Right, the goal was to not call MTRR directly. > Adding everyone from that commit, plus Luis. Drivers really shouldn't > assume/work around the bios setting up superflous/wrong MTRR. Such things are needed, otherwise some systems may not boot... > > With that we could kill ioremap_uc entirely. > > So yeah removing that seems definitely like the right thing. I think this would be possible if we could flop ioremap_nocache() to UC instead of UC- on x86. Otherwise, I can't see how we can remove this by still not allowing direct MTRR calls. Luis From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luis Chamberlain Date: Tue, 12 Nov 2019 22:24:23 +0000 Subject: Re: [PATCH] video: fbdev: atyfb: only use ioremap_uc() on i386 and ia64 Message-Id: <20191112222423.GO11244@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: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Daniel Vetter , Juergen Gross Cc: Christoph Hellwig , Tuowen Zhao , AceLan Kao , Mika Westerberg , Andy Shevchenko , Roman Gilg , Lee Jones , "Luis R. Rodriguez" , 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 On Tue, Nov 12, 2019 at 03:26:35PM +0100, Daniel Vetter wrote: > On Tue, Nov 12, 2019 at 3:06 PM 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, since if > > > you need MTRR for performance (because you dont have PAT) then you > > > can't fix the wc with the PAT-based ioremap_uc. And if you have PAT, > > > then you don't really need an MTRR to get wc. > > > > > > So I'd revert this patch from Luis and ... > > > > Sounds great to me.. > > > > > ... apply this one. Since the same reasoning should apply to anything > > > that's running on any cpu with PAT. > > > > 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? > > Hm so that's way out of my knowledge, but I think mtrr_cleanup() was > supposed to fix up messy/broken MTRR setups by the bios. So maybe they > simply didn't enable that in their .config with CONFIG_MTRR_SANITIZER. I had originally suggested to just make the driver build on x86, but an atlternative was to provide the call for the missing architecture. > An explicit cleanup is currently not possible for drivers, since the > only interface exported to drivers is arch_phys_wc_add/del (which > short-circuits if pat works since you don't need mtrr in that case). Right, the goal was to not call MTRR directly. > Adding everyone from that commit, plus Luis. Drivers really shouldn't > assume/work around the bios setting up superflous/wrong MTRR. Such things are needed, otherwise some systems may not boot... > > With that we could kill ioremap_uc entirely. > > So yeah removing that seems definitely like the right thing. I think this would be possible if we could flop ioremap_nocache() to UC instead of UC- on x86. Otherwise, I can't see how we can remove this by still not allowing direct MTRR calls. 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:24:23 +0000 Message-ID: <20191112222423.GO11244@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 93CFE6EC06 for ; Tue, 12 Nov 2019 22:24:25 +0000 (UTC) Received: by mail-pf1-f195.google.com with SMTP id q26so78063pfn.11 for ; Tue, 12 Nov 2019 14:24:25 -0800 (PST) Content-Disposition: inline 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: Daniel Vetter , Juergen Gross Cc: Linux Fbdev development list , linux-ia64@vger.kernel.org, dri-devel , "H. Peter Anvin" , Lee Jones , Christoph Hellwig , X86 ML , Ingo Molnar , Tuowen Zhao , Fenghua Yu , Arnd Bergmann , Bartlomiej Zolnierkiewicz , "Luis R. Rodriguez" , Borislav Petkov , Thomas Gleixner , Andy Shevchenko , Mika Westerberg , Tony Luck , Linux Kernel Mailing List , AceLan Kao , Souptick Joarder , Roman Gilg List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBOb3YgMTIsIDIwMTkgYXQgMDM6MjY6MzVQTSArMDEwMCwgRGFuaWVsIFZldHRlciB3 cm90ZToKPiBPbiBUdWUsIE5vdiAxMiwgMjAxOSBhdCAzOjA2IFBNIENocmlzdG9waCBIZWxsd2ln IDxoY2hAbHN0LmRlPiB3cm90ZToKPiA+IE9uIFR1ZSwgTm92IDEyLCAyMDE5IGF0IDAyOjA0OjE2 UE0gKzAxMDAsIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4gPiA+IFd1dCAuLi4gTWF5YmUgSSdtIG1p c3Npbmcgc29tZXRoaW5nLCBidXQgZnJvbSBob3cgd2UgdXNlIG10cnIgaW4gb3RoZXIKPiA+ID4g Z3B1IGRyaXZlcnMgaXQncyBhKSBlaXRoZXIgeW91IHVzZSBNVFJSIGJlY2F1c2UgdGhhdCdzIGFs bCB5b3UgZ290IG9yCj4gPiA+IGIpIHlvdSB1c2UgcGF0LiBNaXhpbmcgYm90aCBzb3VuZHMgbGlr ZSBhIHByZXR0eSBiYWQgaWRlYSwgc2luY2UgaWYKPiA+ID4geW91IG5lZWQgTVRSUiBmb3IgcGVy Zm9ybWFuY2UgKGJlY2F1c2UgeW91IGRvbnQgaGF2ZSBQQVQpIHRoZW4geW91Cj4gPiA+IGNhbid0 IGZpeCB0aGUgd2Mgd2l0aCB0aGUgUEFULWJhc2VkIGlvcmVtYXBfdWMuIEFuZCBpZiB5b3UgaGF2 ZSBQQVQsCj4gPiA+IHRoZW4geW91IGRvbid0IHJlYWxseSBuZWVkIGFuIE1UUlIgdG8gZ2V0IHdj Lgo+ID4gPgo+ID4gPiBTbyBJJ2QgcmV2ZXJ0IHRoaXMgcGF0Y2ggZnJvbSBMdWlzIGFuZCAuLi4K PiA+Cj4gPiBTb3VuZHMgZ3JlYXQgdG8gbWUuLgo+ID4KPiA+ID4gLi4uIGFwcGx5IHRoaXMgb25l LiBTaW5jZSB0aGUgc2FtZSByZWFzb25pbmcgc2hvdWxkIGFwcGx5IHRvIGFueXRoaW5nCj4gPiA+ IHRoYXQncyBydW5uaW5nIG9uIGFueSBjcHUgd2l0aCBQQVQuCj4gPgo+ID4gQ2FuIHlvdSB0YWtl IGEgbG9vayBhdCAibWZkOiBpbnRlbC1scHNzOiBVc2UgZGV2bV9pb3JlbWFwX3VjIGZvciBNTUlP Igo+ID4gaW4gbGludXgtbmV4dCwgd2hpY2ggYWxzbyBsb29rcyByYXRoZXIgZmlzaHkgdG8gbWU/ ICBDYW4ndCB3ZSB1c2UKPiA+IHRoZSBNVFJSIEFQSXMgdG8gb3ZlcnJpZGUgdGhlIGJyb2tlbiBC SU9TIE1UUlIgc2V0dXAgdGhlcmUgYXMgd2VsbD8KPiAKPiBIbSBzbyB0aGF0J3Mgd2F5IG91dCBv ZiBteSBrbm93bGVkZ2UsIGJ1dCBJIHRoaW5rIG10cnJfY2xlYW51cCgpIHdhcwo+IHN1cHBvc2Vk IHRvIGZpeCB1cCBtZXNzeS9icm9rZW4gTVRSUiBzZXR1cHMgYnkgdGhlIGJpb3MuIFNvIG1heWJl IHRoZXkKPiBzaW1wbHkgZGlkbid0IGVuYWJsZSB0aGF0IGluIHRoZWlyIC5jb25maWcgd2l0aCBD T05GSUdfTVRSUl9TQU5JVElaRVIuCgpJIGhhZCBvcmlnaW5hbGx5IHN1Z2dlc3RlZCB0byBqdXN0 IG1ha2UgdGhlIGRyaXZlciBidWlsZCBvbiB4ODYsIGJ1dCBhbgphdGx0ZXJuYXRpdmUgd2FzIHRv IHByb3ZpZGUgdGhlIGNhbGwgZm9yIHRoZSBtaXNzaW5nIGFyY2hpdGVjdHVyZS4KCj4gQW4gZXhw bGljaXQgY2xlYW51cCBpcyBjdXJyZW50bHkgbm90IHBvc3NpYmxlIGZvciBkcml2ZXJzLCBzaW5j ZSB0aGUKPiBvbmx5IGludGVyZmFjZSBleHBvcnRlZCB0byBkcml2ZXJzIGlzIGFyY2hfcGh5c193 Y19hZGQvZGVsICh3aGljaAo+IHNob3J0LWNpcmN1aXRzIGlmIHBhdCB3b3JrcyBzaW5jZSB5b3Ug ZG9uJ3QgbmVlZCBtdHJyIGluIHRoYXQgY2FzZSkuCgpSaWdodCwgdGhlIGdvYWwgd2FzIHRvIG5v dCBjYWxsIE1UUlIgZGlyZWN0bHkuCgo+IEFkZGluZyBldmVyeW9uZSBmcm9tIHRoYXQgY29tbWl0 LCBwbHVzIEx1aXMuIERyaXZlcnMgcmVhbGx5IHNob3VsZG4ndAo+IGFzc3VtZS93b3JrIGFyb3Vu ZCB0aGUgYmlvcyBzZXR0aW5nIHVwIHN1cGVyZmxvdXMvd3JvbmcgTVRSUi4KClN1Y2ggdGhpbmdz IGFyZSBuZWVkZWQsIG90aGVyd2lzZSBzb21lIHN5c3RlbXMgbWF5IG5vdCBib290Li4uCgo+ID4g V2l0aCB0aGF0IHdlIGNvdWxkIGtpbGwgaW9yZW1hcF91YyBlbnRpcmVseS4KPiAKPiBTbyB5ZWFo IHJlbW92aW5nIHRoYXQgc2VlbXMgZGVmaW5pdGVseSBsaWtlIHRoZSByaWdodCB0aGluZy4KCkkg dGhpbmsgdGhpcyB3b3VsZCBiZSBwb3NzaWJsZSBpZiB3ZSBjb3VsZCBmbG9wIGlvcmVtYXBfbm9j YWNoZSgpIHRvIFVDCmluc3RlYWQgb2YgVUMtIG9uIHg4Ni4gT3RoZXJ3aXNlLCBJIGNhbid0IHNl ZSBob3cgd2UgY2FuIHJlbW92ZSB0aGlzIGJ5CnN0aWxsIG5vdCBhbGxvd2luZyBkaXJlY3QgTVRS UiBjYWxscy4KCiAgTHVpcwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3Rv cC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmkt ZGV2ZWw=