On Tue, Jan 19, 2016 at 11:35:44AM -0200, Paulo Zanoni wrote:
> We'll now call intel_fbc_pre_update instead of intel_fbc_deactivate
> during atomic commits. This will continue to guarantee that we
> deactivate FBC and it will also update the state checking structures
> at the correct time. Then, later, at the point where we were calling
> intel_fbc_update, we'll only need to call intel_fbc_post_update.
>
> Also add the proper warnings in case we don't have the appropriate
> locks. Daniel mentioned the warnings will have to be removed for async
> commits, but let's keep them here while we can.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zan...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 11 +++++------
> drivers/gpu/drm/i915/intel_drv.h | 8 +++++---
> drivers/gpu/drm/i915/intel_fbc.c | 33 ++++++++++++++++++---------------
> 3 files changed, 28 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index baab41046..baa4cc9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11733,7 +11733,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> to_intel_plane(primary)->frontbuffer_bit);
> mutex_unlock(&dev->struct_mutex);
>
> - intel_fbc_deactivate(intel_crtc);
> + intel_fbc_pre_update(intel_crtc);
> intel_frontbuffer_flip_prepare(dev,
>
> to_intel_plane(primary)->frontbuffer_bit);
>
> +void intel_fbc_pre_update(struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> struct intel_fbc *fbc = &dev_priv->fbc;
>
> - WARN_ON(!mutex_is_locked(&fbc->lock));
> + if (!fbc_supported(dev_priv))
> + return;
> +
> + mutex_lock(&fbc->lock);
>
> if (!multiple_pipes_ok(dev_priv)) {
> set_no_fbc_reason(dev_priv, "more than one pipe active");
> @@ -907,15 +915,17 @@ static void intel_fbc_pre_update(struct intel_crtc
> *crtc)
> }
>
> if (!fbc->enabled || fbc->crtc != crtc)
> - return;
> + goto unlock;
>
> intel_fbc_update_state_cache(crtc);
>
> deactivate:
> __intel_fbc_deactivate(dev_priv);
> +unlock:
> + mutex_unlock(&fbc->lock);
> }
So this change is broken as intel_fbc_pre_update() depends upon state
derived from the pinned framebuffer object but is being called by
intel_crtc_page_flip() before that object is pinned with
intel_pin_and_fence_fb - i.e. state such as the gtt_offset or the fence
for the object are incorrect, and even trying to look at them can cause
an oops.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
