diff mbox

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

Message ID 20170908114906.9915-2-joonas.lahtinen@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joonas Lahtinen Sept. 8, 2017, 11:49 a.m. UTC
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(-)

Comments

Chris Wilson Sept. 8, 2017, 2:16 p.m. UTC | #1
Quoting Joonas Lahtinen (2017-09-08 12:49:06)
> 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)

I think you missed git add.
-Chris
>
Joonas Lahtinen Sept. 11, 2017, 7:59 a.m. UTC | #2
On Fri, 2017-09-08 at 15:16 +0100, Chris Wilson wrote:
> Quoting Joonas Lahtinen (2017-09-08 12:49:06)
> > 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)
> 
> I think you missed git add.

Oh well, should not try to submit patches on Friday (or Monday for that
matter). Just did the git add and sent with -3 instead of -v3... But
the correct version is en route, too.

Regards, Joonas
diff mbox

Patch

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.
  *
  */