From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: kick any firmware framebuffers before claiming the gtt Date: Mon, 11 Jun 2012 16:43:15 -0700 Message-ID: <20120611164315.529edb0b@bwidawsk.net> References: <20120611161344.GF4695@phenom.ffwll.local> <1339432092-30126-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id CD3BA9F075 for ; Mon, 11 Jun 2012 16:43:24 -0700 (PDT) In-Reply-To: <1339432092-30126-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org We could probably fix this with a kernel command line too: video=vesafb:mtrr:3 On Mon, 11 Jun 2012 18:28:12 +0200 Daniel Vetter wrote: > Especially vesafb likes to map everything as uc- (yikes), and if that > mapping hangs around still while we try to map the gtt as wc the > kernel will downgrade our request to uc-, resulting in abyssal > performance. Probably every graphics device running on x86 wants this, should we consider trying to generalize it a bit more with a DRM helper? At the very least, I think we should consider some Kconfig changes (comments, excluding vesafb when i915 is selected, or something). Just to satisfy my curiousity if vesafb is loaded after, we'll run into the same problem (like GPU offloading with vesa display)? s/abyssal/abysmal > > Unfortunately we can't do this as early as readon does (i.e. as the > first thing we do when initializing the hw) because our fb/mmio space > region moves around on a per-gen basis. So I've had to move it below > the gtt initialization, but that seems to work, too. The important > thing is that we do this before we set up the gtt wc mapping. > > Now an altogether different question is why people compile their > kernels with vesafb enabled, but I guess making things just work isn't > bad per se ... The kernel comments recommend turning it on if unsure. As above, I think that should be corrected. > > v2: > - s/radeondrmfb/inteldrmfb/ > - fix up error handling > > Reported-and-tested-by: "Kilarski, Bernard R" > Signed-Off-by: Daniel Vetter Reviewed-by: Ben Widawsky still, a couple comments below > --- > drivers/gpu/drm/i915/i915_dma.c | 35 ++++++++++++++++++++++++++++------- > 1 file changed, 28 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 262a74d..379cb14 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1401,6 +1401,25 @@ i915_mtrr_setup(struct drm_i915_private *dev_priv, unsigned long base, > } > } > > +static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) > +{ > + struct apertures_struct *ap; > + struct pci_dev *pdev = dev_priv->dev->pdev; > + bool primary = false; > + > + ap = alloc_apertures(1); > + ap->ranges[0].base = dev_priv->dev->agp->base; > + ap->ranges[0].size = > + dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT; > + > +#ifdef CONFIG_X86 > + primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW; > +#endif I don't really understand the point of this bit, but I guess other drivers do it, so meh. Also, the ifdef CONFIG_X86 is sort of pointless in the i915 driver, but no biggie. > + > + remove_conflicting_framebuffers(ap, "inteldrmfb", primary); > + kfree(ap); > +} > + > /** > * i915_driver_load - setup chip and create an initial config > * @dev: DRM device > @@ -1446,6 +1465,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > goto free_priv; > } > > + dev_priv->mm.gtt = intel_gtt_get(); > + if (!dev_priv->mm.gtt) { > + DRM_ERROR("Failed to initialize GTT\n"); > + ret = -ENODEV; > + goto put_bridge; > + } > + > + i915_kick_out_firmware_fb(dev_priv); > + > pci_set_master(dev->pdev); > > /* overlay on gen2 is broken and can't address above 1G */ > @@ -1471,13 +1499,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > goto put_bridge; > } > > - dev_priv->mm.gtt = intel_gtt_get(); > - if (!dev_priv->mm.gtt) { > - DRM_ERROR("Failed to initialize GTT\n"); > - ret = -ENODEV; > - goto out_rmmap; > - } > - > aperture_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT; > > dev_priv->mm.gtt_mapping = -- Ben Widawsky, Intel Open Source Technology Center