| Message ID | 1411054170-2411-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-09-18 12:29 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>: > 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@intel.com> 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@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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; }
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. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- 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(-)