From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752428AbbC0VWA (ORCPT ); Fri, 27 Mar 2015 17:22:00 -0400 Received: from mail-lb0-f174.google.com ([209.85.217.174]:33763 "EHLO mail-lb0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751284AbbC0VV5 (ORCPT ); Fri, 27 Mar 2015 17:21:57 -0400 MIME-Version: 1.0 In-Reply-To: <20150327201201.GL5622@wotan.suse.de> References: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com> <1426893517-2511-10-git-send-email-mcgrof@do-not-panic.com> <20150327201201.GL5622@wotan.suse.de> From: Andy Lutomirski Date: Fri, 27 Mar 2015 14:21:34 -0700 Message-ID: Subject: Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around To: "Luis R. Rodriguez" Cc: "Luis R. Rodriguez" , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , Juergen Gross , Jan Beulich , Borislav Petkov , Suresh Siddha , venkatesh.pallipadi@intel.com, Dave Airlie , "linux-kernel@vger.kernel.org" , Linux Fbdev development list , X86 ML , "xen-devel@lists.xenproject.org" , Ingo Molnar , Linus Torvalds , Daniel Vetter , Antonino Daplas , Jean-Christophe Plagniol-Villard , Tomi Valkeinen Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 27, 2015 at 1:12 PM, Luis R. Rodriguez wrote: > On Fri, Mar 20, 2015 at 04:52:18PM -0700, Andy Lutomirski wrote: >> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez >> wrote: >> > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c >> > index 8025624..8875e56 100644 >> > --- a/drivers/video/fbdev/aty/atyfb_base.c >> > +++ b/drivers/video/fbdev/aty/atyfb_base.c >> > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info) >> > >> > #ifdef CONFIG_MTRR >> > par->mtrr_aper = -1; >> > - par->mtrr_reg = -1; >> > if (!nomtrr) { >> > - /* Cover the whole resource. */ >> > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size, >> > + par->mtrr_aper = mtrr_add(info->fix.smem_start, >> > + info->fix.smem_len, >> > MTRR_TYPE_WRCOMB, 1); >> > - if (par->mtrr_aper >= 0 && !par->aux_start) { >> > - /* Make a hole for mmio. */ >> > - par->mtrr_reg = mtrr_add(par->res_start + 0x800000 - >> > - GUI_RESERVE, GUI_RESERVE, >> > - MTRR_TYPE_UNCACHABLE, 1); >> > - if (par->mtrr_reg < 0) { >> > - mtrr_del(par->mtrr_aper, 0, 0); >> > - par->mtrr_aper = -1; >> > - } >> > - } >> > } >> > #endif >> > >> > @@ -2776,10 +2765,6 @@ aty_init_exit: >> > par->pll_ops->set_pll(info, &par->saved_pll); >> > >> > #ifdef CONFIG_MTRR >> > - if (par->mtrr_reg >= 0) { >> > - mtrr_del(par->mtrr_reg, 0, 0); >> > - par->mtrr_reg = -1; >> > - } >> > if (par->mtrr_aper >= 0) { >> > mtrr_del(par->mtrr_aper, 0, 0); >> > par->mtrr_aper = -1; >> > @@ -3466,7 +3451,7 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info, >> > } >> > >> > info->fix.mmio_start = raddr; >> > - par->ati_regbase = ioremap(info->fix.mmio_start, 0x1000); >> > + par->ati_regbase = ioremap_nocache(info->fix.mmio_start, 0x1000); >> >> Double-check me, but I think that ioremap_nocache + WC MTRR = WC. > > Precicely, in this case the WC hole was obtained by using MTRR WC. This > patch removes that WC hole trick and now we can be explciit about > only wanting ioremap_nocache() on the registers, that is WC is not > desired here and is not used. The patch does not highlight the fact > that there was left in place another ioremap() call for the framebuffer: > > info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len); > > That is the one that later after this patch we use ioremap_wc() for. > This patch just removes the hole solution. That's all. > I don't understand. If I read it right, there's a 2^n byte BAR. You're requesting WC for the whole think using arch_phys_wc_add. On a PAT system that has no effect and all is well. On a non-PAT system, it adds an MTRR. That means that you need to override the MTRR somehow for the mmio regs, and UC- won't do the trick. Or am I missing something here? --Andy > Luis -- Andy Lutomirski AMA Capital Management, LLC From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Date: Fri, 27 Mar 2015 21:21:34 +0000 Subject: Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around Message-Id: List-Id: References: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com> <1426893517-2511-10-git-send-email-mcgrof@do-not-panic.com> <20150327201201.GL5622@wotan.suse.de> In-Reply-To: <20150327201201.GL5622@wotan.suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: "Luis R. Rodriguez" Cc: "Luis R. Rodriguez" , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , Juergen Gross , Jan Beulich , Borislav Petkov , Suresh Siddha , venkatesh.pallipadi@intel.com, Dave Airlie , "linux-kernel@vger.kernel.org" , Linux Fbdev development list , X86 ML , "xen-devel@lists.xenproject.org" , Ingo Molnar , Linus Torvalds , Daniel Vetter , Antonino Daplas , Jean-Christophe Plagniol-Villard , Tomi Valkeinen On Fri, Mar 27, 2015 at 1:12 PM, Luis R. Rodriguez wrote: > On Fri, Mar 20, 2015 at 04:52:18PM -0700, Andy Lutomirski wrote: >> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez >> wrote: >> > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c >> > index 8025624..8875e56 100644 >> > --- a/drivers/video/fbdev/aty/atyfb_base.c >> > +++ b/drivers/video/fbdev/aty/atyfb_base.c >> > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info) >> > >> > #ifdef CONFIG_MTRR >> > par->mtrr_aper = -1; >> > - par->mtrr_reg = -1; >> > if (!nomtrr) { >> > - /* Cover the whole resource. */ >> > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size, >> > + par->mtrr_aper = mtrr_add(info->fix.smem_start, >> > + info->fix.smem_len, >> > MTRR_TYPE_WRCOMB, 1); >> > - if (par->mtrr_aper >= 0 && !par->aux_start) { >> > - /* Make a hole for mmio. */ >> > - par->mtrr_reg = mtrr_add(par->res_start + 0x800000 - >> > - GUI_RESERVE, GUI_RESERVE, >> > - MTRR_TYPE_UNCACHABLE, 1); >> > - if (par->mtrr_reg < 0) { >> > - mtrr_del(par->mtrr_aper, 0, 0); >> > - par->mtrr_aper = -1; >> > - } >> > - } >> > } >> > #endif >> > >> > @@ -2776,10 +2765,6 @@ aty_init_exit: >> > par->pll_ops->set_pll(info, &par->saved_pll); >> > >> > #ifdef CONFIG_MTRR >> > - if (par->mtrr_reg >= 0) { >> > - mtrr_del(par->mtrr_reg, 0, 0); >> > - par->mtrr_reg = -1; >> > - } >> > if (par->mtrr_aper >= 0) { >> > mtrr_del(par->mtrr_aper, 0, 0); >> > par->mtrr_aper = -1; >> > @@ -3466,7 +3451,7 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info, >> > } >> > >> > info->fix.mmio_start = raddr; >> > - par->ati_regbase = ioremap(info->fix.mmio_start, 0x1000); >> > + par->ati_regbase = ioremap_nocache(info->fix.mmio_start, 0x1000); >> >> Double-check me, but I think that ioremap_nocache + WC MTRR = WC. > > Precicely, in this case the WC hole was obtained by using MTRR WC. This > patch removes that WC hole trick and now we can be explciit about > only wanting ioremap_nocache() on the registers, that is WC is not > desired here and is not used. The patch does not highlight the fact > that there was left in place another ioremap() call for the framebuffer: > > info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len); > > That is the one that later after this patch we use ioremap_wc() for. > This patch just removes the hole solution. That's all. > I don't understand. If I read it right, there's a 2^n byte BAR. You're requesting WC for the whole think using arch_phys_wc_add. On a PAT system that has no effect and all is well. On a non-PAT system, it adds an MTRR. That means that you need to override the MTRR somehow for the mmio regs, and UC- won't do the trick. Or am I missing something here? --Andy > Luis -- Andy Lutomirski AMA Capital Management, LLC