Quoting Joonas Lahtinen (2017-09-08 10:29:35)
> Convert to use the freshly available made INTEL_GEN_MASK for easier
> grepping and improve function readability and clarify the UABI
> documentation.
>
> No functional changes.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 81 ++++++++++++++++++-------------------
> include/uapi/drm/i915_drm.h | 6 ++-
> 2 files changed, 44 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 1b38eb94d461..74f135d247a1 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1292,72 +1292,71 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv)
> intel_uncore_forcewake_reset(dev_priv, false);
> }
>
> -#define GEN_RANGE(l, h) GENMASK((h) - 1, (l) - 1)
> -
> -static const struct register_whitelist {
> - i915_reg_t offset_ldw, offset_udw;
> - uint32_t size;
> - /* supported gens, 0x10 for 4, 0x30 for 4 and 5, etc. */
> - uint32_t gen_bitmask;
> -} whitelist[] = {
> - { .offset_ldw = RING_TIMESTAMP(RENDER_RING_BASE),
> - .offset_udw = RING_TIMESTAMP_UDW(RENDER_RING_BASE),
> - .size = 8, .gen_bitmask = GEN_RANGE(4, 10) },
> -};
> +static const struct reg_whitelist {
> + i915_reg_t offset_ldw;
> + i915_reg_t offset_udw;
> + unsigned long gen_mask;
> + u8 size;
> +} reg_read_whitelist[] = {{
Hmm, Won't {{ look unusual if we ever say add all the other ring
timestamps to the white list? Or problem for another day?
> + .offset_ldw = RING_TIMESTAMP(RENDER_RING_BASE),
> + .offset_udw = RING_TIMESTAMP_UDW(RENDER_RING_BASE),
> + .gen_mask = INTEL_GEN_MASK(4, 10),
> + .size = 8
> +}};
>
> int i915_reg_read_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_i915_reg_read *reg = data;
> - struct register_whitelist const *entry = whitelist;
> - unsigned size;
> - i915_reg_t offset_ldw, offset_udw;
> - int i, ret = 0;
> -
> - for (i = 0; i < ARRAY_SIZE(whitelist); i++, entry++) {
> - if (i915_mmio_reg_offset(entry->offset_ldw) == (reg->offset & -entry->size) &&
> - (INTEL_INFO(dev_priv)->gen_mask & entry->gen_bitmask))
> + struct reg_whitelist const *entry;
> + unsigned flags;
> + int remain;
> + int ret = 0;
> +
> + entry = reg_read_whitelist;
> + remain = ARRAY_SIZE(reg_read_whitelist);
> + while (remain) {
> + 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);
> + GEM_BUG_ON(hweight8(entry->size) != 1);
> + GEM_BUG_ON(entry->size > 8);
Sensible assertions, but we already depending on entry->size being well
defined to get to here. So move it up. Also hweight8(x) != 1 is
!is_power_of_2(x)
>
> - intel_runtime_pm_get(dev_priv);
> + flags = reg->offset & ~i915_mmio_reg_offset(entry->offset_ldw);
>
> - switch (size) {
> - case 8 | 1:
> - reg->val = I915_READ64_2x32(offset_ldw, offset_udw);
> - break;
> + intel_runtime_pm_get(dev_priv);
> + switch (entry->size) {
> case 8:
> - reg->val = I915_READ64(offset_ldw);
> + if (flags & I915_REG_READ_8B_WA)
We're losing -EINVAL for the invalid flag combinations. Can I tempt you
to use (entry->size | flags)?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
