[Intel-gfx] [PATCH] drm/i915/fbc: Assume maximum 8mb of stolen is used for gen8+

[David Weinehall]

On Wed, Oct 26, 2016 at 09:18:59PM +0000, Vivi, Rodrigo wrote:
> On Wed, 2016-10-26 at 22:45 +0200, David Weinehall wrote:
> > On Wed, Oct 26, 2016 at 11:22:00AM -0700, Rodrigo Vivi wrote:
> > > Since Broxton has same FBC block as BDW+ let's assume it also
> > > don't have access to the stolen usable range.
> > > 
> > > FBC is currently not saving power on Broxton and I believe
> > > the compression threshold is limited to 1x.
> > > 
> > > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > > Cc: Marc Herbert <marc.herbert at intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_fbc.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > > index cbe2ebd..640db67 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -530,12 +530,11 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
> > >  	int ret;
> > >  	u64 end;
> > >  
> > > -	/* The FBC hardware for BDW/SKL doesn't have access to the stolen
> > > +	/* The FBC hardware for gen8+ doesn't have access to the stolen
> > >  	 * reserved range size, so it always assumes the maximum (8mb) is used.
> > 
> > Might I suggest correcting the comment to say megabyte instead of
> > millibit while at it?
> 
> you mean s/8mb/8MB right? I can change it in a v3 for sure.

Indeed.

> > 
> > >  	 * If we enable FBC using a CFB on that memory range we'll get FIFO
> > >  	 * underruns, even if that range is not reserved by the BIOS. */
> > > -	if (IS_BROADWELL(dev_priv) ||
> > > -	    IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> > > +	if (INTEL_INFO(dev_priv)->gen <= 8)
> > 
> > INTEL_GEN(dev_priv)
> 
> yeap, good point. will change that later .
> 
> Although for Marc the old style has more chance to get applied on his
> tree for now.

Fair enough.

> Let's wait for his confirmation before the v3 anyway.
> 
> > 
> > Also, shouldn't this be >= 8? Also, 
> 
> duh! sorry!
> Marc already pointed me this mistake... v2 was sent already.

Yeah, I noticed just after I pressed send :)

> > remember that gen8 also includes
> > CherryView -- is the behaviour the same there?
> 
> chv doesn't have fbc so I hope it never reaches this path...

Goodie.


Kind regards, David