Quoting Chris Wilson (2017-09-11 10:18:51)
> Quoting Joonas Lahtinen (2017-09-11 08:57:19)
> > + entry = reg_read_whitelist;
> > + remain = ARRAY_SIZE(reg_read_whitelist);
> > + while (remain) {
> > + GEM_BUG_ON(!is_power_of_2(entry->size));
> > + GEM_BUG_ON(entry->size > 8);
> > +
> > + if (INTEL_INFO(dev_priv)->gen_mask & entry->gen_mask &&
> > + i915_mmio_reg_offset(entry->offset_ldw) ==
> > + (reg->offset & -entry->size))
> > break;
> > + entry++;
> > + remain--;
> > }
> >
> > - if (i == ARRAY_SIZE(whitelist))
> > + if (!remain)
> > return -EINVAL;
> >
> > - /* We use the low bits to encode extra flags as the register should
> > - * be naturally aligned (and those that are not so aligned merely
> > - * limit the available flags for that register).
> > - */
> > - offset_ldw = entry->offset_ldw;
> > - offset_udw = entry->offset_udw;
> > - size = entry->size;
> > - size |= reg->offset ^ i915_mmio_reg_offset(offset_ldw);
> > + flags = reg->offset & ~i915_mmio_reg_offset(entry->offset_ldw);
>
> The mmio offset is not a mask, so ~not_a_mask is an interesting mix of
> bits. So I still think ^ is clearer to extract the bits that differ in
> the user offset. If you want to enunciate the interface clearly then
> use flags = reg->offset & (entry->size - 1);
With a GEM_BUG_ON(entry->offset_ldw & -entry->size); for sanity.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
