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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

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@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@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:

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

Might be nice to macro-ize the size comparison too, both for readability
and to change our threshold in one place if we ever need to.

Thanks,
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx