Re: [PATCH] drm/i915: Put all permanent stolen allocations together

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 11, 2018 at 04:47:42PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-09-11 16:31:56)
> > On Tue, Sep 11, 2018 at 04:14:39PM +0100, Chris Wilson wrote:
> > > Whilst reviewing another new user of stolen memory, Ville made the
> > > observation that we should try to ensure that all permanent allocations
> > > within stolen memory are clustered together at either end of the stolen
> > > region, in order to reduce fragmentation. In the depths of
> > > i915_gem_stolen.c it is not always clear what manner of allocation we
> > > need, so expose the drm_mm search parameter and push the decision to our
> > > callers.
> > > 
> > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h         | 15 +++++++++------
> > >  drivers/gpu/drm/i915/i915_gem_stolen.c  | 19 ++++++++++++-------
> > >  drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++-
> > >  drivers/gpu/drm/i915/intel_fbc.c        | 13 ++++++++-----
> > >  drivers/gpu/drm/i915/intel_fbdev.c      |  3 ++-
> > >  drivers/gpu/drm/i915/intel_overlay.c    |  3 ++-
> > >  drivers/gpu/drm/i915/intel_pm.c         |  3 ++-
> > >  drivers/gpu/drm/i915/intel_ringbuffer.c |  2 +-
> > >  8 files changed, 38 insertions(+), 23 deletions(-)
> > > 
> > <snip>
> > > diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> > > index 72eb7e48e8bc..b134b9cabf93 100644
> > > --- a/drivers/gpu/drm/i915/intel_overlay.c
> > > +++ b/drivers/gpu/drm/i915/intel_overlay.c
> > > @@ -1306,7 +1306,8 @@ static int get_registers(struct intel_overlay *overlay, bool use_phys)
> > >       struct i915_vma *vma;
> > >       int err;
> > >  
> > > -     obj = i915_gem_object_create_stolen(overlay->i915, PAGE_SIZE);
> > > +     obj = i915_gem_object_create_stolen(overlay->i915,
> > > +                                         PAGE_SIZE, DRM_MM_INSERT_LOW);
> > 
> > Won't these early INSERT_LOWs create problems for the BIOS fb takeover?
> 
> The BIOS reservation is the very first thing we do, right? The sequence
> is meant to be init the drm_mm, probe all the HW state (BIOS fb, vlv
> pctx, anything else?) and make the reservations, then finish
> initialising the unreserved portions before allowing ourselves to
> allocate from it.

Hmm. Yeah, I think you're right. Somehow I got it into my head that at
least the overlay setup would be done before the state readout. In
theory that would be the correct order if we had to eg. turn off the
overlay while sanitizing the display state in case the BIOS left the
overlay on for some reason. But that is likely to never happen so
I guess we're fine with the current order.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx