On Wed, May 14, 2014 at 04:18:02PM +0300, Mika Kuoppala wrote: > If we get the final value of zero as a count of free > entries available, we will underflow our own fifo_count > and then it will take a long time before we check things again. > Admittedly we are in trouble already if we get into this situation, > but prevent the underflow by returning early. > > v2: Less convoluted control flow (Daniel Vetter) > > Signed-off-by: Mika Kuoppala <mika.kuopp...@intel.com> > --- > drivers/gpu/drm/i915/intel_uncore.c | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index 76dc185..bf1b661 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -154,10 +154,8 @@ static void __gen7_gt_force_wake_mt_put(struct > drm_i915_private *dev_priv, > gen6_gt_check_fifodbg(dev_priv); > } > > -static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) > +static bool __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) > { > - int ret = 0; > - > /* On VLV, FIFO will be shared by both SW and HW. > * So, we need to read the FREE_ENTRIES everytime */ > if (IS_VALLEYVIEW(dev_priv->dev)) > @@ -173,12 +171,12 @@ static int __gen6_gt_wait_for_fifo(struct > drm_i915_private *dev_priv) > fifo = __raw_i915_read32(dev_priv, GTFIFOCTL) & > GT_FIFO_FREE_ENTRIES_MASK; > } > if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
Maybe kill the 'loop<0' check while at it. It's redundant and IMO just makes things less obvious. Also I don't get why we first check for 'fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES', but then the while loop checks for 'fifo <= GT_FIFO_NUM_RESERVED_ENTRIES'. > - ++ret; > + return true; > dev_priv->uncore.fifo_count = fifo; We no longer update fifo_count on failure. Not really a problem, but since we've already done all the work maybe we should still update it. > } > dev_priv->uncore.fifo_count--; > > - return ret; > + return false; > } > > static void vlv_force_wake_reset(struct drm_i915_private *dev_priv) > @@ -642,13 +640,13 @@ gen5_write##x(struct drm_i915_private *dev_priv, off_t > reg, u##x val, bool trace > #define __gen6_write(x) \ > static void \ > gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool > trace) { \ > - u32 __fifo_ret = 0; \ > + bool __fifo_failed = false; \ > REG_WRITE_HEADER; \ > if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ > - __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ > + __fifo_failed = __gen6_gt_wait_for_fifo(dev_priv); \ > } \ > __raw_i915_write##x(dev_priv, reg, val); \ > - if (unlikely(__fifo_ret)) { \ > + if (unlikely(__fifo_failed)) { \ > gen6_gt_check_fifodbg(dev_priv); \ > } \ > REG_WRITE_FOOTER; \ > @@ -657,14 +655,14 @@ gen6_write##x(struct drm_i915_private *dev_priv, off_t > reg, u##x val, bool trace > #define __hsw_write(x) \ > static void \ > hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool > trace) { \ > - u32 __fifo_ret = 0; \ > + bool __fifo_failed = false; \ > REG_WRITE_HEADER; \ > if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ > - __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ > + __fifo_failed = __gen6_gt_wait_for_fifo(dev_priv); \ > } \ > hsw_unclaimed_reg_clear(dev_priv, reg); \ > __raw_i915_write##x(dev_priv, reg, val); \ > - if (unlikely(__fifo_ret)) { \ > + if (unlikely(__fifo_failed)) { \ > gen6_gt_check_fifodbg(dev_priv); \ > } \ > hsw_unclaimed_reg_check(dev_priv, reg); \ > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx