diff mbox

[15/16] Revert "drm/i915: Allocate fbcon from stolen memory"

Message ID 1439588061-18064-16-git-send-email-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Aug. 14, 2015, 9:34 p.m. UTC
This reverts commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb.

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. And 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.

Besides, neither the original commit message nor the review comments
showed any arguments in favor of actually allocating fbcon from
stolen.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Chris Wilson Aug. 15, 2015, 8:24 a.m. UTC | #1
On Fri, Aug 14, 2015 at 06:34:20PM -0300, Paulo Zanoni wrote:
> This reverts commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb.
> 
> 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. And 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.
> 
> Besides, neither the original commit message nor the review comments
> showed any arguments in favor of actually allocating fbcon from
> stolen.

Pardon?
-Chris
Zanoni, Paulo R Aug. 18, 2015, 9:54 p.m. UTC | #2
Em Sáb, 2015-08-15 às 09:24 +0100, Chris Wilson escreveu:
> On Fri, Aug 14, 2015 at 06:34:20PM -0300, Paulo Zanoni wrote:

> > This reverts commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb.

> > 

> > 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. And 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.

> > 

> > Besides, neither the original commit message nor the review 

> > comments

> > showed any arguments in favor of actually allocating fbcon from

> > stolen.

> 

> Pardon?



pzanoni@panetone:~/git/kernel/kernel$ git show
0ffb0ff283cca16f72caf29c44496d83b0c291fb | head -n 12
commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Nov 15 11:32:27 2012 +0000

    drm/i915: Allocate fbcon from stolen memory
    
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

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

    Acked-by: Ben Widawsky <ben@bwidawsk.net>

    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>


diff --git a/drivers/gpu/drm/i915/intel_fb.c
b/drivers/gpu/drm/i915/intel_fb.c
pzanoni@panetone:~/git/kernel/kernel$ 

And the only review I could find was: 

http://lists.freedesktop.org/archives/intel-gfx/2012
-October/021025.html

> -Chris

>
Chris Wilson Aug. 19, 2015, 8:16 a.m. UTC | #3
On Tue, Aug 18, 2015 at 09:54:57PM +0000, Zanoni, Paulo R wrote:
> Em Sáb, 2015-08-15 às 09:24 +0100, Chris Wilson escreveu:
> > On Fri, Aug 14, 2015 at 06:34:20PM -0300, Paulo Zanoni wrote:
> > > This reverts commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb.
> > > 
> > > 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. And 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.
> > > 
> > > Besides, neither the original commit message nor the review 
> > > comments
> > > showed any arguments in favor of actually allocating fbcon from
> > > stolen.
> > 
> > Pardon?
> 
> 
> pzanoni@panetone:~/git/kernel/kernel$ git show
> 0ffb0ff283cca16f72caf29c44496d83b0c291fb | head -n 12
> commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Thu Nov 15 11:32:27 2012 +0000
> 
>     drm/i915: Allocate fbcon from stolen memory
>     
>     Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>     Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>     Acked-by: Ben Widawsky <ben@bwidawsk.net>
>     Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> diff --git a/drivers/gpu/drm/i915/intel_fb.c
> b/drivers/gpu/drm/i915/intel_fb.c
> pzanoni@panetone:~/git/kernel/kernel$ 

Sorry, I though it was pretty self explanatory that we want to reuse as
much as stolen as is possible on all machines, epecially those where
even 8MiB reserved is about 10% of available memory. So not using it for
pinned memory is a double whammy.

That would have been said in the preceeding patches to enable stolen
memory.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 96476d7..f90bf72 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -139,9 +139,7 @@  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 (obj == NULL)
-		obj = i915_gem_alloc_object(dev, size);
+	obj = i915_gem_alloc_object(dev, size);
 	if (!obj) {
 		DRM_ERROR("failed to allocate framebuffer\n");
 		ret = -ENOMEM;