On Thu, 2018-04-12 at 18:07 +0200, Maarten Lankhorst wrote: > There is a small race window in which FBC can be enabled after > pre_plane_update is called, but before the page flip has been > queued or completed. I don't think there is such window, intel_fbc_deactivate() that is called from intel_fbc_pre_update() will set fbc->work.scheduled = false; so the FBC will not be enabled in intel_fbc_work_fn() > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103167 > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_fbc.c | 35 +++++++++++------------------- > ----- > 2 files changed, 12 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index a0b8db3db141..2e2f24c2db9e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -492,6 +492,7 @@ struct intel_fbc { > > bool enabled; > bool active; > + bool flip_pending; > > bool underrun_detected; > struct work_struct underrun_work; > diff --git a/drivers/gpu/drm/i915/intel_fbc.c > b/drivers/gpu/drm/i915/intel_fbc.c > index b431b6733cc1..4770dd7dad5c 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -924,13 +924,6 @@ static void intel_fbc_get_reg_params(struct > intel_crtc *crtc, > 32 * fbc->threshold) > * 8; > } > > -static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params > *params1, > - struct intel_fbc_reg_params > *params2) > -{ > - /* We can use this since intel_fbc_get_reg_params() does a > memset. */ > - return memcmp(params1, params2, sizeof(*params1)) == 0; > -} > - > void intel_fbc_pre_update(struct intel_crtc *crtc, > struct intel_crtc_state *crtc_state, > struct intel_plane_state *plane_state) > @@ -952,6 +945,7 @@ void intel_fbc_pre_update(struct intel_crtc > *crtc, > if (!fbc->enabled || fbc->crtc != crtc) > goto unlock; > > + fbc->flip_pending = true; Also this is not a good name, other actions can cause this function to be executed other than a flip. > intel_fbc_update_state_cache(crtc, crtc_state, plane_state); > > deactivate: > @@ -988,13 +982,15 @@ static void __intel_fbc_post_update(struct > intel_crtc *crtc) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > struct intel_fbc *fbc = &dev_priv->fbc; > - struct intel_fbc_reg_params old_params; > > WARN_ON(!mutex_is_locked(&fbc->lock)); > > if (!fbc->enabled || fbc->crtc != crtc) > return; > > + fbc->flip_pending = false; > + WARN_ON(fbc->active); > + > if (!i915_modparams.enable_fbc) { > intel_fbc_deactivate(dev_priv, "disabled at runtime > per module param"); > __intel_fbc_disable(dev_priv); > @@ -1002,25 +998,16 @@ static void __intel_fbc_post_update(struct > intel_crtc *crtc) > return; > } > > - if (!intel_fbc_can_activate(crtc)) { > - WARN_ON(fbc->active); > + if (!intel_fbc_can_activate(crtc)) > return; > - } > > - old_params = fbc->params; > intel_fbc_get_reg_params(crtc, &fbc->params); > > - /* If the scanout has not changed, don't modify the FBC > settings. > - * Note that we make the fundamental assumption that the fb- > >obj > - * cannot be unpinned (and have its GTT offset and fence > revoked) > - * without first being decoupled from the scanout and FBC > disabled. > - */ > - if (fbc->active && > - intel_fbc_reg_params_equal(&old_params, &fbc->params)) > - return; > - > - intel_fbc_deactivate(dev_priv, "FBC enabled (active or > scheduled)"); > - intel_fbc_schedule_activation(crtc); > + if (!fbc->busy_bits) { I guess this 'if' the line that is fixing the issue. > + intel_fbc_deactivate(dev_priv, "FBC enabled (active > or scheduled)"); > + intel_fbc_schedule_activation(crtc); > + } else > + intel_fbc_deactivate(dev_priv, "frontbuffer write"); > } > > void intel_fbc_post_update(struct intel_crtc *crtc) > @@ -1085,7 +1072,7 @@ void intel_fbc_flush(struct drm_i915_private > *dev_priv, > (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) > { > if (fbc->active) > intel_fbc_recompress(dev_priv); > - else > + else if (!fbc->flip_pending) > __intel_fbc_post_update(fbc->crtc); > } > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
