Quoting Oscar Mateo (2018-02-13 21:56:32)
>
>
> On 02/13/2018 11:18 AM, Daniele Ceraolo Spurio wrote:
> >
> >
> > On 13/02/18 08:37, Mika Kuoppala wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >> +static irqreturn_t
> >> +gen11_gt_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> >> +{
> >> + irqreturn_t ret = IRQ_NONE;
> >> + u16 irq[2][32];
> >> + u32 dw, ident;
> >> + unsigned long tmp;
> >> + unsigned int bank, bit, engine;
> >> + unsigned long wait_start, wait_end;
> >> +
> >> + memset(irq, 0, sizeof(irq));
> >> +
> >> + for (bank = 0; bank < 2; bank++) {
> >> + if (master_ctl & GEN11_GT_DW_IRQ(bank)) {
> >> + dw = I915_READ_FW(GEN11_GT_INTR_DW(bank));
> >> + if (!dw)
> >> + DRM_ERROR("GT_INTR_DW%u blank!\n", bank);
> >> + tmp = dw;
> >> + for_each_set_bit(bit, &tmp, 32) {
> >> + I915_WRITE_FW(GEN11_IIR_REG_SELECTOR(bank), 1 << bit);
> >> + wait_start = local_clock() >> 10;
> >> + /* NB: Specs do not specify how long to spin wait.
> >> + * Taking 100us as an educated guess */
> >> + wait_end = wait_start + 100;
> >> + do {
> >> + ident =
> >> I915_READ_FW(GEN11_INTR_IDENTITY_REG(bank));
> >> + } while (!(ident & GEN11_INTR_DATA_VALID) &&
> >> + !time_after((unsigned long)local_clock() >> 10,
> >> wait_end));
> >
> > Patch 18/20 changes this to time_after64 and wait_end to u64, which
> > looks like the correct way to handle this. Since splitting part of
> > this loop out also helps making things cleaner, would it make sense to
> > squash patch 18/20 into this one?
> >
>
> Makes sense to me. Tvrtko, what do you think?
The squash should be made, but time_after64 is no more correct since the
native 32b/64b wrapped arithmetic is accurate. So what can be done here
is remove the casts and use time_after32() if we truly cared.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
