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

[Paulo Zanoni]

Em Qua, 2016-10-26 às 11:22 -0700, Rodrigo Vivi escreveu:
> 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.

NAK.

What is said above has nothing to do with what the code below does. The
amount of stolen memory is decided by i915_gem_init_stolen() and its
friends, not by the FBC code.

What this patch does is that it potentially reduces the amount of
usable stolen memory available to FBC by up to 8mb. That's totally not
going to help making FBC save more power. It can only do the opposite.

Also, the 1x compression threshold is the best thing.

The (unpatched) code below implements the restriction documented in
FBC_CFB_BASE, and it's not applicable to BXT (the spec explicitly says
that).

Also, we have a history of people who try to measure how much power FBC
saves, but they forget to do things such as properly setting the SATA
link power management or other stuff, leaving the system stuck in PC3
state or worse, and then they toggle FBC and conclude that FBC does not
save power. Power testing should really be done by the power management
team. 

Due to the past situations we had, I'd like to see data on the
measurements done before I can believe that the problem is really FBC.

> 
> 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.
>  	 * 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)
>  		end = ggtt->stolen_size - 8 * 1024 * 1024;
>  	else
>  		end = ggtt->stolen_usable_size;