Re: [PATCH] drm/i915: Call intel_fbc_pre_update() after pinning the new pageflip

["Zanoni, Paulo R" <paulo.r.zanoni@intel.com>]

Em Ter, 2016-08-16 às 08:43 +0100, Chris Wilson escreveu:
> intel_fbc_pre_update() depends upon the new state being already
> pinned
> in place in the Global GTT (primarily for both fencing which wants
> both
> an offset and a fence register, if assigned). This requires the call
> to
> intel_fbc_pre_update() be after intel_pin_and_fence_fb() - but commit
> 5a21b6650a23 ("drm/i915: Revert async unpin and nonblocking atomic
> commit") seems to have returned the code to a different state.
> 
> Fixes 5a21b6650a23 ("drm/i915: Revert async unpin and
> nonblocking...")

I think it actually fixes e8216e502acaad129210c3c8b30cb4ab41e70239
("drm/i915/fbc: call intel_fbc_pre_update earlier during page flips").

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Cc: drm-intel-fixes@lists.freedesktop.org
> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 37d71d5d2369..916ce7b2d4d7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12061,9 +12061,6 @@ static int intel_crtc_page_flip(struct
> drm_crtc *crtc,
>  	crtc->primary->fb = fb;
>  	update_state_fb(crtc->primary);
>  
> -	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
> -			     to_intel_plane_state(primary->state));
> -
>  	work->pending_flip_obj = i915_gem_object_get(obj);
>  
>  	ret = i915_mutex_lock_interruptible(dev);
> @@ -12109,6 +12106,9 @@ static int intel_crtc_page_flip(struct
> drm_crtc *crtc,
>  	work->gtt_offset += intel_crtc->dspaddr_offset;
>  	work->rotation = crtc->primary->state->rotation;
>  
> +	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
> +			     to_intel_plane_state(primary->state));
> +

After you reported the problem yesterday I wrote almost the exact same
patch and just didn't submit it because I didn't have time to test it.
I also added a little comment on top of the call to try to prevent us
from further regressions:

/* There's the potential that the next frame will not be compatible
with FBC, so we want to call pre_update() before the actual page flip.
The problem is that pre_update() caches some information about the fb
object, so we want to do this only after the object is pinned. Let's be
on the safe side and do this immediately before scheduling the
flip. */

With or without this or some other comment:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>  	if (mmio_flip) {
>  		INIT_WORK(&work->mmio_work,
> intel_mmio_flip_work_func);
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx