On Thu, Dec 15, 2016 at 01:40:15PM +0000, Chris Wilson wrote: > On Thu, Dec 15, 2016 at 03:26:59PM +0200, Ville Syrjälä wrote: > > On Thu, Dec 15, 2016 at 11:23:55AM -0200, Paulo Zanoni wrote: > > > Don't even tell the mm allocator to handle the first page of stolen on > > > the affected platforms. 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. > > > > > > Notice that now ggtt->stolen_usable_size won't reflect the ending > > > address of the stolen usable range anymore, so we have to fix the > > > places that rely on this. To simplify, we'll just use U64_MAX. > > > > > > v2: don't even put the first page on the mm (Chris) > > > v3: drm_mm_init() takes size instead of end as argument (Ville) > > > v4: add a comment explaining the reserved ranges (Chris) > > > use 0 for start and U64_MAX for end when possible (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_gtt.h | 10 +++++++++- > > > drivers/gpu/drm/i915/i915_gem_stolen.c | 36 ++++++++++++---------------------- > > > drivers/gpu/drm/i915/intel_fbc.c | 2 +- > > > 3 files changed, 22 insertions(+), 26 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > > > index 8965bbb..0055b85 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > > > @@ -315,8 +315,16 @@ struct i915_ggtt { > > > struct i915_address_space base; > > > struct io_mapping mappable; /* Mapping to our CPU mappable region */ > > > > > > + /* Stolen memory is segmented in hardware with different portions > > > + * offlimits to certain functions. > > > + * > > > + * The drm_mm is initialised to the total accessible range, as found > > > + * from the PCI config. On Broadwell+, this is further restricted to > > > + * avoid the first page! The upper end of stolen memory is reserved for > > > + * hardware functions and similarly removed from the accessible range. > > > + */ > > > size_t stolen_size; /* Total size of stolen memory */ > > > - size_t stolen_usable_size; /* Total size minus BIOS reserved */ > > > + size_t stolen_usable_size; /* Total size minus reserved ranges */ > > > size_t stolen_reserved_base; > > > size_t stolen_reserved_size; > > > u64 mappable_end; /* End offset that we can CPU map */ > > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > > > index b1c8897..cbbfc64 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, > > > @@ -73,11 +67,8 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv, > > > struct drm_mm_node *node, u64 size, > > > unsigned alignment) > > > { > > > - struct i915_ggtt *ggtt = &dev_priv->ggtt; > > > - > > > return i915_gem_stolen_insert_node_in_range(dev_priv, node, size, > > > - alignment, 0, > > > - ggtt->stolen_usable_size); > > > + alignment, 0, U64_MAX); > > > > Just i915_gem_stolen_insert_node() then? > > This is stolen_insert_node() :) /me goes get some coffee... > Could be turned into an inline later. > > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
