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

Chris Wilson
2015-09-23T16:58:27Z

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];

Compressing the plane or the scanout? Would be nice to have a comment
here explaining why src_h is correct (as opposed to say crtc_h, or the
old fb height).
-Chris

--
Chris Wilson, Intel Open Source Technology Centre