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 <[email protected]>
> > Signed-off-by: Paulo Zanoni <[email protected]>
> > ---
> >  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() :)
Could be turned into an inline later.

Reviewed-by: Chris Wilson <[email protected]>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to