Quoting Mika Kuoppala (2017-10-30 12:27:07)
> There is a possibility on gen9 hardware to miss the forcewake ack
> message. The recommended workaround is to use another free
> bit and toggle it until original bit is successfully acknowledged.
>
> Some future gen9 revs might or might not fix the underlying issue but
> the fallback to reserve bit dance can be considered as harmless:
> without the ack timeout we never reach the reserve bit forcewake.
> Thus as of now we adopt a blanket approach for all gen9 and leave
> the bypassing the reserve bit approach for future patches if
> corresponding hw revisions do appear.
>
> Commit 83e3337204b2 ("drm/i915: Increase maximum polling time to 50ms
> for forcewake request/clear ack") did increase the forcewake timeout.
> If the issue was a delayed ack, future work could include finding
> a suitable timeout value both for primary ack and reserve toggle
> to reduce the worst case latency.
>
> v2: use bit 15, naming, comment (Chris), only wait fallback ack
> v3: fix return on fallback, backoff after fallback write (Chris)
>
> References: HSDES #1604254524
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102051
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> ---
> +static int
> +fw_domain_wait_ack_with_fallback(const struct drm_i915_private *i915,
> + const struct intel_uncore_forcewake_domain *d,
> + const enum ack_type type)
> +{
> + const u32 ack_bit = FORCEWAKE_KERNEL;
> + const u32 value = type == ACK_SET ? ack_bit : 0;
> + unsigned int pass = 0;
> + bool ack_detected;
> +
> + /*
> + * There is a possibility of driver's wake request colliding
> + * with hardware's own wake requests and that can cause
> + * hardware to not deliver the driver's ack message.
> + *
> + * Use a fallback bit toggle to kick the gpu state machine
> + * in hopes that the original ack will be delivered along with
> + * the fallback ack.
s/in hopes/in the hope/
> + *
> + * This workaround is described in HSDES #1604254524
> + */
> +
> + do {
> + wait_ack_clear(i915, d, FORCEWAKE_KERNEL_FALLBACK);
> +
> + __raw_i915_write32(i915, d->reg_set,
> + _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL_FALLBACK));
> + /* Give gt some time to relax before the polling frenzy */
> + udelay(10 * pass);
> + wait_ack_set(i915, d, FORCEWAKE_KERNEL_FALLBACK);
I would have started from pass=1 (i.e. udelay(10)) as we already have a
0-delay for the primary wait_ack before we hit the fallback.
> +
> + ack_detected = (__raw_i915_read32(i915, d->reg_ack) & ack_bit) == value;
> +
> + __raw_i915_write32(i915, d->reg_set,
> + _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL_FALLBACK));
> + pass++;
> + } while (!ack_detected && pass < 10);
unsigned int pass = 1;
do {
...
} while (!ack_detected && pass++ < 10);
> +
> + DRM_DEBUG_DRIVER("%s had to use fallback to %s ack, 0x%x (passes %u)\n",
> + intel_uncore_forcewake_domain_to_str(d->id),
> + type == ACK_SET ? "set" : "clear",
> + __raw_i915_read32(i915, d->reg_ack),
> + pass);
> +
> + return ack_detected ? 0 : -ETIMEDOUT;
> +}
I was going to say a-b, but given the state machine we've deduced that
explains why this w/a has any chance of succeeding, I feel a
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
is justified. (If it's wrong, I'm equally culpable ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
