On Wed, Dec 14, 2016 at 06:39:08PM +0200, Ville Syrjälä wrote: > On Wed, Dec 14, 2016 at 12:55:38PM -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. But the BIOS should not be > > putting it at the start of stolen since it's going to get corrupted. I > > suppose the bug here is that some pixels at the very top of the screen > > will be corrupted, so it's not exactly easy to notice. > > > > We have confirmation that the first page of stolen does actually get > > corrupted, so I really think we should do this in order to avoid any > > possible future headaches, even if that means losing BIOS framebuffer > > inheritance. Let's not use the HW in a way it's not supposed to be > > used. > > > > v2: don't even put the first page on the mm (Chris). > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605 > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_gem_stolen.c | 27 ++++++++------------------- > > 1 file changed, 8 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > > index b1c8897..7435540 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, > > @@ -410,7 +404,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv) > > { > > struct i915_ggtt *ggtt = &dev_priv->ggtt; > > unsigned long reserved_total, reserved_base = 0, reserved_size; > > - unsigned long stolen_top; > > + unsigned long stolen_start = 0, stolen_top; > > > > mutex_init(&dev_priv->mm.stolen_lock); > > > > @@ -491,18 +485,13 @@ 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. > > - */ > > - drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size); > > + /* WaSkipStolenMemoryFirstPage:bdw+ */ > > + if (INTEL_GEN(dev_priv) >= 8) > > + stolen_start = 4096; > > + > > + /* Basic memrange allocator for stolen space. */ > > + drm_mm_init(&dev_priv->mm.stolen, stolen_start, > > + ggtt->stolen_usable_size); > > Don't you need to reduce the size by 4k as well? Sigh. Yes. No more review for me. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
