2014-09-18 12:29 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>: > The sw cache clean on BDW is a tempoorary workaround because we cannot > set cache clean on blt ring with risk of hungs. So we are doing the cache clean on sw. > However we are doing much more than needed. Not only when using blt ring. > So, with this extra w/a we minimize the ammount of cache cleans and call it only > on same cases that it was being called on gen7. > > The traditional FBC Cache clean happens over LRI on BLT ring when there is a > frontbuffer touch happening. frontbuffer tracking set fbc_dirty variable > to let BLT flush that it must clean FBC cache. > > fbc.need_sw_cache_clean works in the opposite information direction > of ring->fbc_dirty telling software on frontbuffer tracking to perform > the cache clean on sw side. > > v2: Clean it a little bit and fully check for Broadwell instead of gen8. This is the first time that I try to understand the buffer tracking code, but as far as I could understood - and as far as you explained stuff to me on IRC - the code looks good. But we still could use the review of either Ville or Daniel. With that said: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> And the s/IS_GEN8/IS_BROADWELL/ thing ideally would be a separate patch, but it's fine this way too. > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.h | 10 +++++++++- > drivers/gpu/drm/i915/intel_display.c | 6 ++++-- > drivers/gpu/drm/i915/intel_pm.c | 4 ++-- > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++-- > 4 files changed, 22 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index e3ca8df..a8636e5 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -659,6 +659,14 @@ struct i915_fbc { > > bool false_color; > > + /* On gen8 some rings cannont perform fbc clean operation so for now > + * we are doing this on SW with mmio. > + * This variable works in the opposite information direction > + * of ring->fbc_dirty telling software on frontbuffer tracking > + * to perform the cache clean on sw side. > + */ > + bool need_sw_cache_clean; > + > struct intel_fbc_work { > struct delayed_work work; > struct drm_crtc *crtc; > @@ -2820,7 +2828,7 @@ extern void intel_modeset_setup_hw_state(struct drm_device *dev, > extern void i915_redisable_vga(struct drm_device *dev); > extern void i915_redisable_vga_power_on(struct drm_device *dev); > extern bool intel_fbc_enabled(struct drm_device *dev); > -extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value); > +extern void bdw_fbc_sw_flush(struct drm_device *dev, u32 value); > extern void intel_disable_fbc(struct drm_device *dev); > extern bool ironlake_set_drps(struct drm_device *dev, u8 val); > extern void intel_init_pch_refclk(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index b78f00a..20ba1eb 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9225,8 +9225,10 @@ void intel_frontbuffer_flush(struct drm_device *dev, > * needs to be reworked into a proper frontbuffer tracking scheme like > * psr employs. > */ > - if (IS_BROADWELL(dev)) > - gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN); > + if (dev_priv->fbc.need_sw_cache_clean) { > + dev_priv->fbc.need_sw_cache_clean = false; > + bdw_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN); > + } > } > > /** > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 6f3b94b..40377c7 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -345,11 +345,11 @@ bool intel_fbc_enabled(struct drm_device *dev) > return dev_priv->display.fbc_enabled(dev); > } > > -void gen8_fbc_sw_flush(struct drm_device *dev, u32 value) > +void bdw_fbc_sw_flush(struct drm_device *dev, u32 value) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > - if (!IS_GEN8(dev)) > + if (!IS_BROADWELL(dev)) > return; > > if (!intel_fbc_enabled(dev)) > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 25795f2..c2ddf5f 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2240,6 +2240,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring, > u32 invalidate, u32 flush) > { > struct drm_device *dev = ring->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t cmd; > int ret; > > @@ -2270,8 +2271,12 @@ static int gen6_ring_flush(struct intel_engine_cs *ring, > } > intel_ring_advance(ring); > > - if (IS_GEN7(dev) && !invalidate && flush) > - return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); > + if (!invalidate && flush) { > + if (IS_GEN7(dev)) > + return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); > + else if (IS_BROADWELL(dev)) > + dev_priv->fbc.need_sw_cache_clean = true; > + } > > return 0; > } > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx