On Fri, 06 Apr 2018, José Roberto de Souza <jose.souza@xxxxxxxxx> wrote: > A simple page flip can cause the CFB required size to increase and > if it is bigger than the currently allocated CFB it needs to be > resized to activate FBC again. > > Until now this case was not being handled but CI is starting to > get some of this errors. > > So here it will free the old CFB and a try to allocate the required > CFB in the schedule activation work, it will happen after one vblank > so is guarantee that FBC was completed disabled and is not using CFB. > > Also in case that there is no enough stolen memory to allocate the > new CFB it will try 3 times per full modeset as the CFB requirement > could be reduced in the next non-full modeset. > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105683 > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_fbc.c | 46 +++++++++++++++++++++----------- > 2 files changed, 33 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 5373b171bb96..4ce19b45f67d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -566,6 +566,9 @@ struct intel_fbc { > } work; > > const char *no_fbc_reason; > + > + bool cfb_try_resize; > + u8 cfb_resize_tries_left; > }; > > /* > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > index 573b034a02fd..7d77936db3ec 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -41,6 +41,9 @@ > #include "intel_drv.h" > #include "i915_drv.h" > > +static void __intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv); > +static int intel_fbc_alloc_cfb(struct intel_crtc *crtc); > + > static inline bool fbc_supported(struct drm_i915_private *dev_priv) > { > return HAS_FBC(dev_priv); > @@ -446,6 +449,15 @@ static void intel_fbc_work_fn(struct work_struct *__work) > goto retry; > } > > + if (fbc->cfb_try_resize && fbc->cfb_resize_tries_left) { > + __intel_fbc_cleanup_cfb(dev_priv); > + if (intel_fbc_alloc_cfb(crtc)) { > + fbc->no_fbc_reason = "not enough stolen memory"; > + fbc->cfb_resize_tries_left--; > + goto out; > + } > + } > + > intel_fbc_hw_activate(dev_priv); > > work->scheduled = false; > @@ -850,22 +862,6 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) > return false; > } > > - /* It is possible for the required CFB size change without a > - * crtc->disable + crtc->enable since it is possible to change the > - * stride without triggering a full modeset. Since we try to > - * over-allocate the CFB, there's a chance we may keep FBC enabled even > - * if this happens, but if we exceed the current CFB size we'll have to > - * disable FBC. Notice that it would be possible to disable FBC, wait > - * for a frame, free the stolen node, then try to reenable FBC in case > - * we didn't get any invalidate/deactivate calls, but this would require > - * a lot of tracking just for a specific case. If we conclude it's an > - * important case, we can implement it later. */ > - if (intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) > > - fbc->compressed_fb.size * fbc->threshold) { > - fbc->no_fbc_reason = "CFB requirements changed"; > - return false; > - } > - > /* > * Work around a problem on GEN9+ HW, where enabling FBC on a plane > * having a Y offset that isn't divisible by 4 causes FIFO underrun > @@ -877,6 +873,23 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) > return false; > } > > + /* It is possible for the required CFB size change without a > + * crtc->disable + crtc->enable since it is possible to change the > + * stride without triggering a full modeset. Since we try to > + * over-allocate the CFB, there's a chance we may keep FBC enabled even > + * if this happens, but if we exceed the current CFB size we'll have to > + * resize CFB. > + */ > + if (!drm_mm_node_allocated(&fbc->compressed_fb) || > + (intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) > > + fbc->compressed_fb.size * fbc->threshold)) { > + fbc->cfb_try_resize = true; > + DRM_DEBUG_KMS("CFB requirements have changed, activation \ > + work will try to resize it"); Either the string entirely on one line, or the string split to two, but please never line continuation within the string. BR, Jani. > + } else { > + fbc->cfb_try_resize = false; > + } > + > return true; > } > > @@ -1208,6 +1221,7 @@ void intel_fbc_enable(struct intel_crtc *crtc, > > fbc->enabled = true; > fbc->crtc = crtc; > + fbc->cfb_resize_tries_left = 3; > out: > mutex_unlock(&fbc->lock); > } -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
