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

[Daniel Vetter]

On Wed, Jul 08, 2015 at 05:58:58PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> 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.
> 
> This may fix kms_frontbuffer_tracking failures depending on your
> monitor configuration.
> 
> 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 | 10 ++++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 52d07fb..4a4b2a9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -936,6 +936,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 0a24e96..7b65b00 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";
> @@ -804,6 +806,14 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
>  		goto out_disable;
>  	}
>  
> +	/* TODO: we need to figure out what are the stride restrictions for the
> +	 * older platforms. */
> +	if (fb->pitches[0] < 512 || fb->pitches[0] > 16384 ||

< 512 is already take care of by the X-tiling restriction (requires a
multiple of 512 bytes stride). > 16kb might be the general stride limit,
we should probably reject that in intel_framebuffer_init. 64b alignment is
strange since X-tiled is stricter, and we that already in
intel_fb_stride_alignment.

So summary: Sounds like we need to add per-platform stride limit checks to
fb create code. Plus igt testcases to make sure we check for this (since
right now it seems like we don't). Additional checks here in the fbc code
don't seem required. But if you want to I guess you could convert them to
WARN_ON (without bailing out).
-Daniel

> +	    (fb->pitches[0] & (64 - 1)) != 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.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch