Re: [PATCH] drm/i915: Enable FBC for non X-tiled FBs

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Mar 15, 2017 at 12:29:35PM +0530, Praveen Paneri wrote:
> FBC is only enabled for X-tiled framebuffers but there are
> quite a few cases where we tend to use Y-tiled framebuffers.
> So enabling it for non X-tiled framebuffers.

This patch doesn't actuall enable anything. In fact to me it looks like
we're already allowing Y tiling + FBC, so the commit message/subject should
likely say that you're fixing something.

Unless I'm missing some magic check somewhere that still prevents
Y tiled + FBC. But then this patch wouldn't be sufficient either.

> 
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 2 ++
>  drivers/gpu/drm/i915/intel_fbc.c | 8 ++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc843f9..9d7a376 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2506,6 +2506,8 @@ enum skl_disp_power_wells {
>  #define  BDW_FBC_COMPRESSION_MASK	0xfff
>  
>  #define FBC_LL_SIZE		(1536)
> +#define FBC_YSTRIDE		_MMIO(0x4208c)
> +#define   FBC_STRIDE_OVERRIDE	(1<<13)

These defines shouldn't be in the middle of the FBC1 register defines.
The register should also be called CHICKEN_MISC_1, so the defines
should live next to the other CHICKEN_MISC register defines.

>  
>  #define FBC_LLC_READ_CTRL	_MMIO(0x9044)
>  #define   FBC_LLC_FULLY_OPEN	(1<<30)
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 17d418b..0ac9889 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -301,6 +301,14 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  	u32 dpfc_ctl;
>  	int threshold = dev_priv->fbc.threshold;
>  
> +	if (INTEL_GEN(dev_priv) >= 9 &&

AFAICS IS_GEN9() would be better.

> +	    i915_gem_object_get_tiling(cache->vma->obj) != I915_TILING_X) {
> +		struct intel_fbc_state_cache *cache = &dev_priv->fbc.state_cache;
> +		int cfb_stride = DIV_ROUND_UP(cache->plane.src_w,
> +					      (32 * threshold)) * 8;

Are we taking this compressed stride alignment requirement into account
when we allocate the cfb?

> +		I915_WRITE(FBC_YSTRIDE, FBC_STRIDE_OVERRIDE | cfb_stride);
> +	}
> +
>  	dpfc_ctl = 0;
>  	if (IS_IVYBRIDGE(dev_priv))
>  		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx