[Intel-gfx] [PATCH 05/16] drm/i915: check for the supported strides on HSW+ FBC

[Ville Syrjälä]

On Fri, Aug 14, 2015 at 06:34:10PM -0300, Paulo Zanoni wrote:
> I could only find the restrictions for HSW+, but I think it's safe to
> assume that the older platforms also can't support the configurations
> HSW can't support. The older platforms probably have additional
> restrictions, so we need to figure out those and implement them later.
> Let's not block HSW+ FBC based on missing information for the older
> platforms.

gen2/3	4K or 8K
gen4	2K - 16K
g4x/ilk	not really documented, I'd just go with gen6+ limits since it's fbc2 already
gen5+	512 - 16K

Which means we don't really need another upper limit besides 16K since
all the older platforms are already limited at that or below for tiled
buffers. But on old platforms we do need to check the lower limits.

> 
> v2:
>   - Just WARN_ON() the strides that should have been caught earlier
>     (Daniel)
>   - Make it a new function since I expect this to grow more.
> v3:
>   - Document which IGT test is exercised by this.
> 
> Testcase: igt/kms_frontbuffer_tracking/fbc-badstride
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e0f3f05..4fd7858 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -940,6 +940,7 @@ struct i915_fbc {
>  		FBC_CHIP_DEFAULT, /* disabled by default on this chip */
>  		FBC_ROTATION, /* rotation is not supported */
>  		FBC_IN_DBG_MASTER, /* kernel debugger is active */
> +		FBC_BAD_STRIDE, /* stride is not supported */
>  	} no_fbc_reason;
>  
>  	bool (*fbc_enabled)(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index f7be9ab8..73bd559 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -501,6 +501,8 @@ const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
>  		return "rotation unsupported";
>  	case FBC_IN_DBG_MASTER:
>  		return "Kernel debugger is active";
> +	case FBC_BAD_STRIDE:
> +		return "framebuffer stride not supported";
>  	default:
>  		MISSING_CASE(reason);
>  		return "unknown reason";
> @@ -694,6 +696,22 @@ static int intel_fbc_setup_cfb(struct drm_i915_private *dev_priv, int size,
>  	return intel_fbc_alloc_cfb(dev_priv, size, fb_cpp);
>  }
>  
> +static bool stride_is_valid(unsigned int stride)
> +{
> +	/* TODO: we need to figure out what are the stride restrictions for the
> +	 * older platforms. */
> +
> +	/* These should have been caught earlier. */
> +	WARN_ON(stride < 512);
> +	WARN_ON((stride & (64 - 1)) != 0);

This is rather weird thing to check since we need an X-tiled buffer. I
do see it mentioned in the SNB+ PM guides though.

> +
> +	/* These are additional FBC restrictions. */
> +	if (stride > 16385)

Off by one?

> +		return false;
> +
> +	return true;
> +}
> +
>  /**
>   * __intel_fbc_update - enable/disable FBC as needed, unlocked
>   * @dev_priv: i915 device instance
> @@ -804,6 +822,11 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
>  		goto out_disable;
>  	}
>  
> +	if (!stride_is_valid(fb->pitches[0])) {
> +		set_no_fbc_reason(dev_priv, FBC_BAD_STRIDE);
> +		goto out_disable;
> +	}
> +
>  	/* If the kernel debugger is active, always disable compression */
>  	if (in_dbg_master()) {
>  		set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER);
> -- 
> 2.4.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC