[Intel-gfx] [PATCH 1/2] drm/i915: Try to allocate more memory for FBC on Ironlake or GM45

[ykzhao]

On Thu, 2010-03-25 at 04:31 +0800, Jesse Barnes wrote:
> On Wed, 24 Mar 2010 12:57:41 -0700
> Eric Anholt <eric at anholt.net> wrote:
> 
> > On Tue, 23 Mar 2010 11:12:32 +0800, Zhenyu Wang <zhenyuw at linux.intel.com> wrote:
> > > From: Zhao Yakui <yakui.zhao at intel.com>
> > > 
> > > The framebuffer compression can support up to 2K lines and 4K pixels, which
> > > means that it can support up to 32M memory space. But now it is only limited
> > > to 8M.
> > > 
> > > Try to allocate more memory for FBC from the stolen memory on Ironlake or
> > > GM45.
> > > 
> > > Signed-off-by: Zhao Yakui <yakui.zhao at intel.com>
> > > Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_dma.c      |   16 +++++++++++++---
> > >  drivers/gpu/drm/i915/intel_display.c |   10 ++++++++--
> > >  2 files changed, 21 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > index 902d286..61fb967 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -1451,9 +1451,19 @@ static int i915_load_modeset_init(struct drm_device *dev,
> > >  		int cfb_size;
> > >  
> > >  		/* Try to get an 8M buffer... */
> > > -		if (prealloc_size > (9*1024*1024))
> > > -			cfb_size = 8*1024*1024;
> > > -		else /* fall back to 7/8 of the stolen space */
> > > +		if (prealloc_size > (9*1024*1024)) {
> > > +			if (IS_GM45(dev) || IS_IRONLAKE_M(dev)) {
> > > +				if (prealloc_size >= (64 * 1024 * 1024))
> > > +					cfb_size = 32 * 1024 * 1024;
> > > +				else if (prealloc_size >= (32 * 1024 * 1024))
> > > +					cfb_size = 24 * 1024 * 1024;
> > > +				else if (prealloc_size >= (16 * 1024 * 1024))
> > > +					cfb_size = 12 * 1024 * 1024;
> > > +				else
> > > +					cfb_size = 8 * 1024 * 1024;
> > > +			} else
> > > +				cfb_size = 8*1024*1024;
> > > +		} else /* fall back to 7/8 of the stolen space */
> > >  			cfb_size = prealloc_size * 7 / 8;
> > >  		i915_setup_compression(dev, cfb_size);
> > 
> > The previous code was "use 7/8 of stolen space, unless we've got a ton
> > in which case limit it to 8MB."  Now your code has a ton of cases to use
> > 4/8MB increments.  Why 4/8MB increments?  From the commit message, I
> > would have expected:
> > 
> > cfb_size = prealloc_size * 7 / 8;
> > if (IS_GM45(dev) || IS_IRONLAKE_M(dev)) {
> >    if (cfb_size > (32 * 1024 * 1024))
> >       cfb_size = 32 * 1024 * 1024;
> > } else {
> >    if (cfb_size > (8 * 1024 * 1024))
> >       cfb_size = 8 * 1024 * 1024;
> > }
> 
> Yeah, that logic looks better.  We could even make it a module param to
> allow override (e.g. if a user only needed an 8M buffer and wanted to
> let the OS have the rest).

The FBC on Ironlake or Cantiga can support up to 32M compression
framebuffer. But how much stolen memory can be allocated for FBC depends
on the amount of total stolen memory space. The purpose of the logic is
only to allocate more memory for FBC if possible.

   >If the stolen memory is 32M, we can allocate about 24M for FBC.
   >If the stolen memory is 16M, we can allocate about 12M for FBC.
   >If the stolen memory is below 9M, it will fall back to the original
logic.

At the same time it seems that some stolen memory is still left from the
current logic. As we don't know how much left stolen memory is
appropriate, the conservative allocation policy is used.


>