[PATCH v2 2/2] drm/i915: Simplify i915_reg_read_ioctl

Joonas Lahtinen
2017-09-08T11:49:46Z

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.

v2:
- Lift GEM_BUG_ONs and use is_power_of_2 (Chris)
- Retain -EINVAL on bad flags behavior (Chris)

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
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[] = {{
+ .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);

- 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)
+ reg->val = I915_READ64_2x32(entry->offset_ldw,
+ entry->offset_udw);
+ else
+ reg->val = I915_READ64(entry->offset_ldw);
break;
case 4:
- reg->val = I915_READ(offset_ldw);
+ reg->val = I915_READ(entry->offset_ldw);
break;
case 2:
- reg->val = I915_READ16(offset_ldw);
+ reg->val = I915_READ16(entry->offset_ldw);
break;
case 1:
- reg->val = I915_READ8(offset_ldw);
+ reg->val = I915_READ8(entry->offset_ldw);
break;
default:
ret = -EINVAL;
- goto out;
+ break;
}
-
-out:
intel_runtime_pm_put(dev_priv);
+
return ret;
}

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d8d10d932759..b4505d55990d 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1308,14 +1308,16 @@ struct drm_i915_reg_read {
* be specified
*/
__u64 offset;
+#define I915_REG_READ_8B_WA BIT(0)
+
__u64 val; /* Return value */
};
/* Known registers:
*
* Render engine timestamp - 0x2358 + 64bit - gen7+
* - Note this register returns an invalid value if using the default
- * single instruction 8byte read, in order to workaround that use
- * offset (0x2538 | 1) instead.
+ * single instruction 8byte read, in order to workaround that pass
+ * flag I915_REG_READ_8B_WA in offset field.
*
*/

--
2.13.5