Re: [PATCH 1/4] drm/i915/fbc: update busy_bits even for GTT and flip flushes

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Mar 21, 2016 at 04:26:54PM -0300, Paulo Zanoni wrote:
> We ignore ORIGIN_GTT because the hardware tracking can recognize GTT
> writes and take care of them. We also ignore ORIGIN_FLIP because we
> deal with flips without relying on the frontbuffer tracking
> infrastructure. On the other hand, a flush is a flush and means we're
> good to go, so we need to update busy_bits in order to reflect that,
> even if we're not going to do anything else about it.
> 
> How to reproduce the bug fixed by this patch:
>  - boot SKL up to the desktop environment
>  - stop the display manager
>  - run any of the igt/kms_frontbuffer_tracking/*fbc*onoff* subtests
>  - the tests will fail
> 
> The steps above will create the right conditions for us to lose track
> of busy_bits. If you, for example, run the full set of FBC tests, the
> onoff subtests will succeed.
> 
> Also notice that the "bug" is that we'll just keep FBC disabled on
> cases where it could be enabled, so it's not something the users can
> perceive, it just affects power consumption numbers on properly
> configured machines.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Is this covered by your nasty igt test suite? Kernel side looks good, so
with appropriate Testcase: tag added:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 2e571f5..b8ba79c 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -996,13 +996,13 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>  	if (!fbc_supported(dev_priv))
>  		return;
>  
> -	if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
> -		return;
> -
>  	mutex_lock(&fbc->lock);
>  
>  	fbc->busy_bits &= ~frontbuffer_bits;
>  
> +	if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
> +		goto out;
> +
>  	if (!fbc->busy_bits && fbc->enabled &&
>  	    (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) {
>  		if (fbc->active)
> @@ -1011,6 +1011,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>  			__intel_fbc_post_update(fbc->crtc);
>  	}
>  
> +out:
>  	mutex_unlock(&fbc->lock);
>  }
>  
> -- 
> 2.7.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx