On Thu, Apr 06, 2017 at 11:44:19AM +0300, Mika Kuoppala wrote: > Replace the handcrafter loop when checking for fifo slots > with atomic wait for. This brings this wait in line with > the other waits on register access. We also get a readable > timeout constraint, so make it to fail after 10ms. > > Chris suggested that we should fail silently as the fifo debug > handler, now attached to unclaimed mmio handling, will take care of the > possible errors at later stage. > > Note that the decision to wait was changed so that we avoid > allocating the first reserved entry. Nor do we reduce the count > if we fail the wait, removing the possiblity to wrap the > count if the hw fifo returned zero. Otoh, we don't abort the write so the slot is still taken. Nor does it update the last known fifo_count along that path. However, we leave it set to a value that will cause us to reread the counter on the next call, so it comes out in the wash. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=100247 > Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_uncore.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index a129a73..df744a8 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -29,6 +29,7 @@ > #include <linux/pm_runtime.h> > > #define FORCEWAKE_ACK_TIMEOUT_MS 50 > +#define GT_FIFO_TIMEOUT_MS 10 > > #define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__)) > > @@ -181,28 +182,27 @@ static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv) > > static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) > { > - int ret = 0; > + u32 n; > > /* 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_priv->uncore.fifo_count = fifo_free_entries(dev_priv); > - > - if (dev_priv->uncore.fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) { > - int loop = 500; > - u32 fifo = fifo_free_entries(dev_priv); > - > - while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) { > - udelay(10); > - fifo = fifo_free_entries(dev_priv); > + n = fifo_free_entries(dev_priv); > + else > + n = dev_priv->uncore.fifo_count; > + > + if (n <= GT_FIFO_NUM_RESERVED_ENTRIES) { > + if (wait_for_atomic((n = fifo_free_entries(dev_priv)) > > + GT_FIFO_NUM_RESERVED_ENTRIES, > + GT_FIFO_TIMEOUT_MS)) { > + DRM_DEBUG("GT_FIFO timeout, entries: %u, unclaimed: %d\n", n, > + intel_uncore_unclaimed_mmio(dev_priv)); What's the connection here between unclaimed mmio and the fifo count? i.e. what information do we glean? Espcially as this information is part of the generic mmio framework already. > + return -EBUSY; > } > - if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES)) > - ++ret; > - dev_priv->uncore.fifo_count = fifo; > } > - dev_priv->uncore.fifo_count--; Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
