Re: [PATCH 1/7] drm/i915: fix CFB size calculation

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

On Wed, Sep 23, 2015 at 12:52:21PM -0300, Paulo Zanoni wrote:
> We were considering the whole framebuffer height, but the spec clearly
> says that we should only consider the active display height size.
> 
> On my current testing machine, this moves us from 124 successes and
> 502 skips to 209 successes and 417 skips on "kms_frontbuffer_tracking
> --fbc-only". The high amount of skips is due to the --fbc-only
> argument. We had those skips due to not enough stolen memory for the
> tests. We're now passing the maximum possible amount: 209.
> 
> Note: when this patch was written, the amount of tests we had for FBC
> was different than what we have now.
> 
> v2: Use the clipped src size instead of pipe_src_h (Ville).
> 
> Testcase: igt/kms_frontbuffer_tracking/fbc-*
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index d38f464..0a6b454 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -693,9 +693,15 @@ void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv)
>  	mutex_unlock(&dev_priv->fbc.lock);
>  }
>  
> -static int intel_fbc_setup_cfb(struct drm_i915_private *dev_priv, int size,
> -			       int fb_cpp)
> +static int intel_fbc_setup_cfb(struct intel_crtc *crtc)
>  {
> +	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +	struct drm_framebuffer *fb = crtc->base.primary->fb;
> +	int size, fb_cpp;
> +
> +	size = (crtc->base.primary->state->src_h >> 16) * fb->pitches[0];

That's what the user asked for. The clipped size is drm_rect_height(&state->src)

As mentioned in my last review, the docs aren't really clear on whether we
need to consider the plane src height or dst height, or if we're even
allowed to use FBC with scaled planes.

> +	fb_cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> +
>  	if (size <= dev_priv->fbc.uncompressed_size)
>  		return 0;
>  
> @@ -883,8 +889,7 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
>  		goto out_disable;
>  	}
>  
> -	if (intel_fbc_setup_cfb(dev_priv, obj->base.size,
> -				drm_format_plane_cpp(fb->pixel_format, 0))) {
> +	if (intel_fbc_setup_cfb(intel_crtc)) {
>  		set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL);
>  		goto out_disable;
>  	}
> -- 
> 2.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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