[Intel-gfx] [PATCH v3 2/2] drm/i915: Simplify i915_reg_read_ioctl

[Chris Wilson]

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