| Message ID | 1481659046-30739-2-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 13, 2016 at 05:57:26PM -0200, Paulo Zanoni wrote: > Reserve the first page of stolen memory right after initializing the > mm allocator. This means that we won't inherit the FB in case the BIOS > decides to put it at the start of stolen. IMHO, avoiding constant > screen flickering is more important than BIOS framebuffer inheritance. > > TODO: the goal is to ask everybody with screen flickering problems to > test this patch. Hopefully we'll be able to add a bunch of Bugzilla > tags here before merging it. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605 > Cc: stable@vger.kernel.org > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/i915_gem_stolen.c | 30 +++++++++++++----------------- > 2 files changed, 16 insertions(+), 17 deletions(-) > > > Time to spam bugzilla again with links to patchwork. The tester on > fd.o bug 94605 suggests this may be the fix we've been looking for. > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 0e11ed7..0868a88 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1531,6 +1531,9 @@ struct i915_gem_mm { > /** LRU list of objects with fence regs on them. */ > struct list_head fence_list; > > + /** First page of stolen, for platforms where it's reserved. */ > + struct drm_mm_node first_page; > + > /** > * Are we in a non-interruptible section of code like > * modesetting? > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index b1c8897..9b3ae58 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv, > if (!drm_mm_initialized(&dev_priv->mm.stolen)) > return -ENODEV; > > - /* See the comment at the drm_mm_init() call for more about this check. > - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete) > - */ > - if (start < 4096 && INTEL_GEN(dev_priv) >= 8) > - start = 4096; > - > mutex_lock(&dev_priv->mm.stolen_lock); > ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size, > alignment, start, end, > @@ -286,6 +280,9 @@ void i915_gem_cleanup_stolen(struct drm_device *dev) > if (!drm_mm_initialized(&dev_priv->mm.stolen)) > return; > > + if (drm_mm_node_allocated(&dev_priv->mm.first_page)) > + drm_mm_remove_node(&dev_priv->mm.first_page); > + > drm_mm_takedown(&dev_priv->mm.stolen); > } > > @@ -491,19 +488,18 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv) > > ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total; > > - /* > - * Basic memrange allocator for stolen space. > - * > - * TODO: Notice that some platforms require us to not use the first page > - * of the stolen memory but their BIOSes may still put the framebuffer > - * on the first page. So we don't reserve this page for now because of > - * that. Our current solution is to just prevent new nodes from being > - * inserted on the first page - see the check we have at > - * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon > - * problem later. > - */ > + /* Basic memrange allocator for stolen space. */ > drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size); > > + /* WaSkipStolenMemoryFirstPage:bdw+ */ > + if (INTEL_GEN(dev_priv) >= 8) { > + dev_priv->mm.first_page.start = 0; > + dev_priv->mm.first_page.size = 4096; > + if (drm_mm_reserve_node(&dev_priv->mm.stolen, > + &dev_priv->mm.first_page)) > + DRM_DEBUG_KMS("Failed to reserve the first page of stolen memory.\n"); > + } If you are saying that the first page can never be used, then just don't put into the range allocator. -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0e11ed7..0868a88 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1531,6 +1531,9 @@ struct i915_gem_mm { /** LRU list of objects with fence regs on them. */ struct list_head fence_list; + /** First page of stolen, for platforms where it's reserved. */ + struct drm_mm_node first_page; + /** * Are we in a non-interruptible section of code like * modesetting? diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index b1c8897..9b3ae58 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv, if (!drm_mm_initialized(&dev_priv->mm.stolen)) return -ENODEV; - /* See the comment at the drm_mm_init() call for more about this check. - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete) - */ - if (start < 4096 && INTEL_GEN(dev_priv) >= 8) - start = 4096; - mutex_lock(&dev_priv->mm.stolen_lock); ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size, alignment, start, end, @@ -286,6 +280,9 @@ void i915_gem_cleanup_stolen(struct drm_device *dev) if (!drm_mm_initialized(&dev_priv->mm.stolen)) return; + if (drm_mm_node_allocated(&dev_priv->mm.first_page)) + drm_mm_remove_node(&dev_priv->mm.first_page); + drm_mm_takedown(&dev_priv->mm.stolen); } @@ -491,19 +488,18 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv) ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total; - /* - * Basic memrange allocator for stolen space. - * - * TODO: Notice that some platforms require us to not use the first page - * of the stolen memory but their BIOSes may still put the framebuffer - * on the first page. So we don't reserve this page for now because of - * that. Our current solution is to just prevent new nodes from being - * inserted on the first page - see the check we have at - * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon - * problem later. - */ + /* Basic memrange allocator for stolen space. */ drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size); + /* WaSkipStolenMemoryFirstPage:bdw+ */ + if (INTEL_GEN(dev_priv) >= 8) { + dev_priv->mm.first_page.start = 0; + dev_priv->mm.first_page.size = 4096; + if (drm_mm_reserve_node(&dev_priv->mm.stolen, + &dev_priv->mm.first_page)) + DRM_DEBUG_KMS("Failed to reserve the first page of stolen memory.\n"); + } + return 0; }
Reserve the first page of stolen memory right after initializing the mm allocator. This means that we won't inherit the FB in case the BIOS decides to put it at the start of stolen. IMHO, avoiding constant screen flickering is more important than BIOS framebuffer inheritance. TODO: the goal is to ask everybody with screen flickering problems to test this patch. Hopefully we'll be able to add a bunch of Bugzilla tags here before merging it. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605 Cc: stable@vger.kernel.org Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_gem_stolen.c | 30 +++++++++++++----------------- 2 files changed, 16 insertions(+), 17 deletions(-) Time to spam bugzilla again with links to patchwork. The tester on fd.o bug 94605 suggests this may be the fix we've been looking for.