From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH] drm/agp/i915: trim stolen space to 32M Date: Thu, 8 Jul 2010 09:01:44 -0700 Message-ID: <20100708090144.22b5cb28@virtuousgeek.org> References: <20100707144000.3ce0c490@virtuousgeek.org> <201007081058.22193.simon.farnsworth@onelan.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cpoproxy3-pub.bluehost.com (cpoproxy3-pub.bluehost.com [67.222.54.6]) by gabe.freedesktop.org (Postfix) with SMTP id AD3DF9EBCD for ; Thu, 8 Jul 2010 09:02:03 -0700 (PDT) In-Reply-To: <201007081058.22193.simon.farnsworth@onelan.com> 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: Simon Farnsworth Cc: intel-gfx@lists.freedesktop.org, t.artem@mailcity.com List-Id: intel-gfx@lists.freedesktop.org On Thu, 8 Jul 2010 10:58:21 +0100 Simon Farnsworth wrote: > On Wednesday 7 July 2010, Jesse Barnes wrote: > > Some BIOSes will claim a large chunk of stolen space. Unless we > > reclaim it, our aperture for remapping buffer objects will be > > constrained. So clamp the stolen space to 32M and ignore the rest. > > > I'm not sure that this changelog fits the patch - if I'm understanding the code > correctly, you're clamping to 16M, not 32M. > > Apart from that, the code looks sensible. Oops updated to 32M for Ironlake but didn't update the changelog or comments. > > > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > > index f97122a..54ed0e1 100644 > > --- a/drivers/char/agp/intel-gtt.c > > +++ b/drivers/char/agp/intel-gtt.c > > @@ -25,6 +25,10 @@ > > #define USE_PCI_DMA_API 1 > > #endif > > > > +/* Max amount of stolen space, anything above will be returned to Linux */ > > +int intel_max_stolen = 16 * 1024 * 1024; > > This is 16M, not 32M > > > +EXPORT_SYMBOL(intel_max_stolen); > > + > > static const struct aper_size_info_fixed intel_i810_sizes[] = > > { > > {64, 16384, 4}, > > @@ -710,7 +714,12 @@ static void intel_i830_init_gtt_entries(void) > > break; > > } > > } > > - if (gtt_entries > 0) { > > + if (!local && gtt_entries > intel_max_stolen) { > > + dev_info(&agp_bridge->dev->dev, > > + "detected %dK stolen memory, trimming to %dK\n", > > + gtt_entries / KB(1), intel_max_stolen / KB(1)); > > + gtt_entries = intel_max_stolen / KB(4); > > This appears to limit to intel_max_stolen bytes, not intel_max_stolen * 2 > bytes. I think that's what I want to do, so I'm not sure what you mean? Each GTT entry covers a 4k page so we have to convert the total size into the number of entries... > > > + } else if (gtt_entries > 0) { > > dev_info(&agp_bridge->dev->dev, "detected %dK %s memory\n", > > gtt_entries / KB(1), local ? "local" : "stolen"); > > gtt_entries /= KB(4); > > diff --git a/drivers/gpu/drm/i915/i915_dma.c > > b/drivers/gpu/drm/i915/i915_dma.c index e2dd903..69e25ab 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -40,6 +40,8 @@ > > #include > > #include > > > > +extern int intel_max_stolen; /* from AGP driver */ > > + > > /** > > * Sets up the hardware status page for devices that need a physical > > address * in the register. > > @@ -2105,6 +2107,12 @@ int i915_driver_load(struct drm_device *dev, > > unsigned long flags) if (ret) > > goto out_iomapfree; > > > > + if (prealloc_size > intel_max_stolen) { > > + DRM_INFO("detected %dM stolen memory, trimming to %dM\n", > > + prealloc_size >> 20, intel_max_stolen >> 20); > > + prealloc_size = intel_max_stolen; > > And again here, you appear to limit to intel_max_stolen, not to twice that. > > > + } > > + > > dev_priv->wq = create_singlethread_workqueue("i915"); > > if (dev_priv->wq == NULL) { > > DRM_ERROR("Failed to create our workqueue.\n"); -- Jesse Barnes, Intel Open Source Technology Center