[Intel-gfx] [PATCH 3/7] drm/i915: don't allocate fbcon from stolen memory if it's too big

[Daniel Vetter]

On Thu, Oct 08, 2015 at 01:19:27PM -0700, Jesse Barnes wrote:
> On 09/23/2015 08:52 AM, Paulo Zanoni wrote:
> > Technology has evolved and now we have eDP panels with 3200x1800
> > resolution. In the meantime, the BIOS guys didn't change the default
> > 32mb for stolen memory. On top of that, we can't assume our users will
> > be able to increase the default stolen memory size to more than 32mb -
> > I'm not even sure all BIOSes allow that.
> > 
> > So just the fbcon buffer alone eats 22mb of my stolen memroy, and due
> > to the BDW/SKL restriction of not using the last 8mb of stolen memory,
> > all that's left for FBC is 2mb! Since fbcon is not the coolest feature
> > ever, I think it's better to save our precious stolen resource to FBC
> > and the other guys.
> > 
> > On the other hand, we really want to use as much stolen memory as
> > possible, since on some older systems the stolen memory may be a
> > considerable percentage of the total available memory.
> > 
> > This patch tries to achieve a little balance using a simple heuristic:
> > if the fbcon wants more than half of the available stolen memory,
> > don't use stolen memory in order to leave some for FBC and the other
> > features.
> > 
> > The long term plan should be to implement a way to set priorities for
> > stolen memory allocation and then evict low priority users when the
> > high priority ones need the memory. While we still don't have that,
> > let's try to make FBC usable with the simple solution.
> > 
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |  7 +++++++
> >  drivers/gpu/drm/i915/intel_fbdev.c   | 10 ++++++++--
> >  2 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 2a1fab3..24b8a72 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2486,6 +2486,7 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
> >  			      struct intel_initial_plane_config *plane_config)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct drm_i915_gem_object *obj = NULL;
> >  	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> >  	struct drm_framebuffer *fb = &plane_config->fb->base;
> > @@ -2498,6 +2499,12 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
> >  	if (plane_config->size == 0)
> >  		return false;
> >  
> > +	/* If the FB is too big, just don't use it since fbdev is not very
> > +	 * important and we should probably use that space with FBC or other
> > +	 * features. */
> > +	if (size_aligned * 2 > dev_priv->gtt.stolen_usable_size)
> > +		return false;
> > +
> >  	obj = i915_gem_object_create_stolen_for_preallocated(dev,
> >  							     base_aligned,
> >  							     base_aligned,
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 6532912..4fd5fdf 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -121,8 +121,9 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> >  		container_of(helper, struct intel_fbdev, helper);
> >  	struct drm_framebuffer *fb;
> >  	struct drm_device *dev = helper->dev;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct drm_mode_fb_cmd2 mode_cmd = {};
> > -	struct drm_i915_gem_object *obj;
> > +	struct drm_i915_gem_object *obj = NULL;
> >  	int size, ret;
> >  
> >  	/* we don't do packed 24bpp */
> > @@ -139,7 +140,12 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> >  
> >  	size = mode_cmd.pitches[0] * mode_cmd.height;
> >  	size = PAGE_ALIGN(size);
> > -	obj = i915_gem_object_create_stolen(dev, size);
> > +
> > +	/* If the FB is too big, just don't use it since fbdev is not very
> > +	 * important and we should probably use that space with FBC or other
> > +	 * features. */
> > +	if (size * 2 < dev_priv->gtt.stolen_usable_size)
> > +		obj = i915_gem_object_create_stolen(dev, size);
> >  	if (obj == NULL)
> >  		obj = i915_gem_alloc_object(dev, size);
> >  	if (!obj) {
> > 
> 
> I agree with Chris that we should make this smarter too, but I don't
> think this hurts in the meantime, so:

Maybe I'm mixing things up again, but with the evict-stolen-to-shmem
patch we have in the stolen series the tooling should be there already.
Tricky bit will be to make it race-free, which is probably impossible
(within a reasonable amount of effort).

> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch