All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 00/12] Legacy mmio accessor macro pruning
@ 2019-06-07 12:08 Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 01/12] drm/i915: Eliminate unused mmio accessors Tvrtko Ursulin
                   ` (14 more replies)
  0 siblings, 15 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Since we want to stop using the legacy mmio accessors (the ones which create a
hidden dependency to having a dev_priv local) this series removes all of the
more exotic ones by converting the call sites to use the new uncore accessors.

The ones left are only the standard 32-bit ones and the _FW suffix flavour. It
will be a much bigger (more invasive) task to get rid of those.

But even though this series only results in a partial conversion, benefit is
that with removed macros there is no chance to use them by accident in new code.

Tvrtko Ursulin (12):
  drm/i915: Eliminate unused mmio accessors
  drm/i915: Convert i915_reg_read_ioctl to use explicit mmio accessors
  drm/i915: Convert icl_get_stolen_reserved to uncore mmio accessors
  drm/i915: Convert gem_record_fences to uncore mmio accessors
  drm/i915: Convert intel_read_wm_latency to uncore mmio accessors
  drm/i915: Remove I915_READ64 and I915_READ64_32x2
  drm/i915: Remove I915_READ8
  drm/i915: Remove I915_POSTING_READ_FW
  drm/i915: Remove POSTING_READ16
  drm/i915: Remove I915_WRITE_NOTRACE
  drm/i915: Remove I915_READ_NOTRACE
  drm/i915: Remove I915_READ16 and I915_WRITE16

 drivers/gpu/drm/i915/gem/i915_gem_stolen.c  |   4 +-
 drivers/gpu/drm/i915/gt/intel_engine.h      |   1 +
 drivers/gpu/drm/i915/gt/intel_ringbuffer.c  |  14 +-
 drivers/gpu/drm/i915/gvt/debugfs.c          |   4 +-
 drivers/gpu/drm/i915/gvt/firmware.c         |   5 +-
 drivers/gpu/drm/i915/i915_debugfs.c         |  37 ++---
 drivers/gpu/drm/i915/i915_drv.c             |   6 +-
 drivers/gpu/drm/i915/i915_drv.h             |  31 ----
 drivers/gpu/drm/i915/i915_gem.c             |   9 +-
 drivers/gpu/drm/i915/i915_gpu_error.c       | 116 ++++++++-------
 drivers/gpu/drm/i915/i915_irq.c             |  42 +++---
 drivers/gpu/drm/i915/i915_pmu.c             |   8 +-
 drivers/gpu/drm/i915/intel_crt.c            |  41 +++---
 drivers/gpu/drm/i915/intel_dp.c             |  43 +++---
 drivers/gpu/drm/i915/intel_gmbus.c          |  53 ++++---
 drivers/gpu/drm/i915/intel_guc_submission.c |   4 +-
 drivers/gpu/drm/i915/intel_pm.c             | 149 +++++++++++---------
 drivers/gpu/drm/i915/intel_uncore.c         |  23 +--
 18 files changed, 322 insertions(+), 268 deletions(-)

-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RFC 01/12] drm/i915: Eliminate unused mmio accessors
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 02/12] drm/i915: Convert i915_reg_read_ioctl to use explicit " Tvrtko Ursulin
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

On the path of removing mmio accessors with implicit dev_priv, easy first
step is to remove all such unused macros.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dfe4b11ee423..8da1541546ee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2853,12 +2853,9 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
 	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
 
 #define I915_READ8(reg__)	  __I915_REG_OP(read8, dev_priv, (reg__))
-#define I915_WRITE8(reg__, val__) __I915_REG_OP(write8, dev_priv, (reg__), (val__))
 
 #define I915_READ16(reg__)	   __I915_REG_OP(read16, dev_priv, (reg__))
 #define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
-#define I915_READ16_NOTRACE(reg__)	   __I915_REG_OP(read16_notrace, dev_priv, (reg__))
-#define I915_WRITE16_NOTRACE(reg__, val__) __I915_REG_OP(write16_notrace, dev_priv, (reg__), (val__))
 
 #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
 #define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
@@ -2914,7 +2911,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
  */
 #define I915_READ_FW(reg__) __I915_REG_OP(read_fw, dev_priv, (reg__))
 #define I915_WRITE_FW(reg__, val__) __I915_REG_OP(write_fw, dev_priv, (reg__), (val__))
-#define I915_WRITE64_FW(reg__, val__) __I915_REG_OP(write64_fw, dev_priv, (reg__), (val__))
 #define POSTING_READ_FW(reg__) __I915_REG_OP(posting_read_fw, dev_priv, (reg__))
 
 /* "Broadcast RGB" property */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 02/12] drm/i915: Convert i915_reg_read_ioctl to use explicit mmio accessors
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 01/12] drm/i915: Eliminate unused mmio accessors Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 03/12] drm/i915: Convert icl_get_stolen_reserved to uncore " Tvrtko Ursulin
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

No excuse for code located in intel_uncore.c to not use intel_uncore_
helpers.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f78668123f02..85171a8b866a 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1672,7 +1672,8 @@ static const struct reg_whitelist {
 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_private *i915 = to_i915(dev);
+	struct intel_uncore *uncore = &i915->uncore;
 	struct drm_i915_reg_read *reg = data;
 	struct reg_whitelist const *entry;
 	intel_wakeref_t wakeref;
@@ -1689,7 +1690,7 @@ int i915_reg_read_ioctl(struct drm_device *dev,
 		GEM_BUG_ON(entry->size > 8);
 		GEM_BUG_ON(entry_offset & (entry->size - 1));
 
-		if (INTEL_INFO(dev_priv)->gen_mask & entry->gen_mask &&
+		if (INTEL_INFO(i915)->gen_mask & entry->gen_mask &&
 		    entry_offset == (reg->offset & -entry->size))
 			break;
 		entry++;
@@ -1701,18 +1702,22 @@ int i915_reg_read_ioctl(struct drm_device *dev,
 
 	flags = reg->offset & (entry->size - 1);
 
-	with_intel_runtime_pm(dev_priv, wakeref) {
+	with_intel_runtime_pm(i915, wakeref) {
 		if (entry->size == 8 && flags == I915_REG_READ_8B_WA)
-			reg->val = I915_READ64_2x32(entry->offset_ldw,
-						    entry->offset_udw);
+			reg->val = intel_uncore_read64_2x32(uncore,
+							    entry->offset_ldw,
+							    entry->offset_udw);
 		else if (entry->size == 8 && flags == 0)
-			reg->val = I915_READ64(entry->offset_ldw);
+			reg->val = intel_uncore_read64(uncore,
+						       entry->offset_ldw);
 		else if (entry->size == 4 && flags == 0)
-			reg->val = I915_READ(entry->offset_ldw);
+			reg->val = intel_uncore_read(uncore, entry->offset_ldw);
 		else if (entry->size == 2 && flags == 0)
-			reg->val = I915_READ16(entry->offset_ldw);
+			reg->val = intel_uncore_read16(uncore,
+						       entry->offset_ldw);
 		else if (entry->size == 1 && flags == 0)
-			reg->val = I915_READ8(entry->offset_ldw);
+			reg->val = intel_uncore_read8(uncore,
+						      entry->offset_ldw);
 		else
 			ret = -EINVAL;
 	}
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 03/12] drm/i915: Convert icl_get_stolen_reserved to uncore mmio accessors
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 01/12] drm/i915: Eliminate unused mmio accessors Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 02/12] drm/i915: Convert i915_reg_read_ioctl to use explicit " Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 04/12] drm/i915: Convert gem_record_fences " Tvrtko Ursulin
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

More removal of implicit dev_priv.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 84d4f549eb21..432037e5d0a5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -325,11 +325,11 @@ static void bdw_get_stolen_reserved(struct drm_i915_private *dev_priv,
 	*size = stolen_top - *base;
 }
 
-static void icl_get_stolen_reserved(struct drm_i915_private *dev_priv,
+static void icl_get_stolen_reserved(struct drm_i915_private *i915,
 				    resource_size_t *base,
 				    resource_size_t *size)
 {
-	u64 reg_val = I915_READ64(GEN6_STOLEN_RESERVED);
+	u64 reg_val = intel_uncore_read64(&i915->uncore, GEN6_STOLEN_RESERVED);
 
 	DRM_DEBUG_DRIVER("GEN6_STOLEN_RESERVED = 0x%016llx\n", reg_val);
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 04/12] drm/i915: Convert gem_record_fences to uncore mmio accessors
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 03/12] drm/i915: Convert icl_get_stolen_reserved to uncore " Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 05/12] drm/i915: Convert intel_read_wm_latency " Tvrtko Ursulin
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

More implicit dev_priv removal.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 193a93857d99..a523bf050a25 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1123,17 +1123,23 @@ static u32 i915_error_generate_code(struct i915_gpu_state *error,
 static void gem_record_fences(struct i915_gpu_state *error)
 {
 	struct drm_i915_private *dev_priv = error->i915;
+	struct intel_uncore *uncore = &dev_priv->uncore;
 	int i;
 
 	if (INTEL_GEN(dev_priv) >= 6) {
 		for (i = 0; i < dev_priv->num_fence_regs; i++)
-			error->fence[i] = I915_READ64(FENCE_REG_GEN6_LO(i));
+			error->fence[i] =
+				intel_uncore_read64(uncore,
+						    FENCE_REG_GEN6_LO(i));
 	} else if (INTEL_GEN(dev_priv) >= 4) {
 		for (i = 0; i < dev_priv->num_fence_regs; i++)
-			error->fence[i] = I915_READ64(FENCE_REG_965_LO(i));
+			error->fence[i] =
+				intel_uncore_read64(uncore,
+						    FENCE_REG_965_LO(i));
 	} else {
 		for (i = 0; i < dev_priv->num_fence_regs; i++)
-			error->fence[i] = I915_READ(FENCE_REG(i));
+			error->fence[i] =
+				intel_uncore_read(uncore, FENCE_REG(i));
 	}
 	error->nfence = i;
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 05/12] drm/i915: Convert intel_read_wm_latency to uncore mmio accessors
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 04/12] drm/i915: Convert gem_record_fences " Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:08 ` [RFC 06/12] drm/i915: Remove I915_READ64 and I915_READ64_32x2 Tvrtko Ursulin
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

More implicit dev_priv removal.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8f82cb72d3a6..d7272d4ff258 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2813,6 +2813,8 @@ hsw_compute_linetime_wm(const struct intel_crtc_state *cstate)
 static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
 				  u16 wm[8])
 {
+	struct intel_uncore *uncore = &dev_priv->uncore;
+
 	if (INTEL_GEN(dev_priv) >= 9) {
 		u32 val;
 		int ret, i;
@@ -2894,7 +2896,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
 			wm[0] += 1;
 
 	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		u64 sskpd = I915_READ64(MCH_SSKPD);
+		u64 sskpd = intel_uncore_read64(uncore, MCH_SSKPD);
 
 		wm[0] = (sskpd >> 56) & 0xFF;
 		if (wm[0] == 0)
@@ -2904,14 +2906,14 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
 		wm[3] = (sskpd >> 20) & 0x1FF;
 		wm[4] = (sskpd >> 32) & 0x1FF;
 	} else if (INTEL_GEN(dev_priv) >= 6) {
-		u32 sskpd = I915_READ(MCH_SSKPD);
+		u32 sskpd = intel_uncore_read(uncore, MCH_SSKPD);
 
 		wm[0] = (sskpd >> SSKPD_WM0_SHIFT) & SSKPD_WM_MASK;
 		wm[1] = (sskpd >> SSKPD_WM1_SHIFT) & SSKPD_WM_MASK;
 		wm[2] = (sskpd >> SSKPD_WM2_SHIFT) & SSKPD_WM_MASK;
 		wm[3] = (sskpd >> SSKPD_WM3_SHIFT) & SSKPD_WM_MASK;
 	} else if (INTEL_GEN(dev_priv) >= 5) {
-		u32 mltr = I915_READ(MLTR_ILK);
+		u32 mltr = intel_uncore_read(uncore, MLTR_ILK);
 
 		/* ILK primary LP0 latency is 700 ns */
 		wm[0] = 7;
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 06/12] drm/i915: Remove I915_READ64 and I915_READ64_32x2
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (4 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 05/12] drm/i915: Convert intel_read_wm_latency " Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:44   ` Jani Nikula
  2019-06-07 12:08 ` [RFC 07/12] drm/i915: Remove I915_READ8 Tvrtko Ursulin
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Now that all their users are gone we can remove the macros and
accompanying duplicated comment.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8da1541546ee..b2763721b76d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2862,24 +2862,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
 #define I915_READ_NOTRACE(reg__)	 __I915_REG_OP(read_notrace, dev_priv, (reg__))
 #define I915_WRITE_NOTRACE(reg__, val__) __I915_REG_OP(write_notrace, dev_priv, (reg__), (val__))
 
-/* Be very careful with read/write 64-bit values. On 32-bit machines, they
- * will be implemented using 2 32-bit writes in an arbitrary order with
- * an arbitrary delay between them. This can cause the hardware to
- * act upon the intermediate value, possibly leading to corruption and
- * machine death. For this reason we do not support I915_WRITE64, or
- * dev_priv->uncore.funcs.mmio_writeq.
- *
- * When reading a 64-bit value as two 32-bit values, the delay may cause
- * the two reads to mismatch, e.g. a timestamp overflowing. Also note that
- * occasionally a 64-bit register does not actualy support a full readq
- * and must be read using two 32-bit reads.
- *
- * You have been warned.
- */
-#define I915_READ64(reg__)	__I915_REG_OP(read64, dev_priv, (reg__))
-#define I915_READ64_2x32(lower_reg__, upper_reg__) \
-	__I915_REG_OP(read64_2x32, dev_priv, (lower_reg__), (upper_reg__))
-
 #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
 #define POSTING_READ16(reg__)	__I915_REG_OP(posting_read16, dev_priv, (reg__))
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 07/12] drm/i915: Remove I915_READ8
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (5 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 06/12] drm/i915: Remove I915_READ64 and I915_READ64_32x2 Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 13:11   ` Jani Nikula
  2019-06-11  8:58   ` Jani Nikula
  2019-06-07 12:08 ` [RFC 08/12] drm/i915: Remove I915_POSTING_READ_FW Tvrtko Ursulin
                   ` (7 subsequent siblings)
  14 siblings, 2 replies; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Only a few call sites remain which have been converted to uncore mmio
accessors and so the macro can be removed.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 --
 drivers/gpu/drm/i915/intel_crt.c | 41 ++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_pm.c  |  6 ++---
 3 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b2763721b76d..13815795e197 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2852,8 +2852,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
 #define __I915_REG_OP(op__, dev_priv__, ...) \
 	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
 
-#define I915_READ8(reg__)	  __I915_REG_OP(read8, dev_priv, (reg__))
-
 #define I915_READ16(reg__)	   __I915_REG_OP(read16, dev_priv, (reg__))
 #define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
 
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index bb56518576a1..3fcf2f84bcce 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -643,6 +643,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
 {
 	struct drm_device *dev = crt->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 save_bclrpat;
 	u32 save_vtotal;
 	u32 vtotal, vactive;
@@ -663,9 +664,9 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
 	pipeconf_reg = PIPECONF(pipe);
 	pipe_dsl_reg = PIPEDSL(pipe);
 
-	save_bclrpat = I915_READ(bclrpat_reg);
-	save_vtotal = I915_READ(vtotal_reg);
-	vblank = I915_READ(vblank_reg);
+	save_bclrpat = intel_uncore_read(uncore, bclrpat_reg);
+	save_vtotal = intel_uncore_read(uncore, vtotal_reg);
+	vblank = intel_uncore_read(uncore, vblank_reg);
 
 	vtotal = ((save_vtotal >> 16) & 0xfff) + 1;
 	vactive = (save_vtotal & 0x7ff) + 1;
@@ -674,21 +675,23 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
 	vblank_end = ((vblank >> 16) & 0xfff) + 1;
 
 	/* Set the border color to purple. */
-	I915_WRITE(bclrpat_reg, 0x500050);
+	intel_uncore_write(uncore, bclrpat_reg, 0x500050);
 
 	if (!IS_GEN(dev_priv, 2)) {
-		u32 pipeconf = I915_READ(pipeconf_reg);
-		I915_WRITE(pipeconf_reg, pipeconf | PIPECONF_FORCE_BORDER);
-		POSTING_READ(pipeconf_reg);
+		u32 pipeconf = intel_uncore_read(uncore, pipeconf_reg);
+		intel_uncore_write(uncore,
+				   pipeconf_reg,
+				   pipeconf | PIPECONF_FORCE_BORDER);
+		intel_uncore_posting_read(uncore, pipeconf_reg);
 		/* Wait for next Vblank to substitue
 		 * border color for Color info */
 		intel_wait_for_vblank(dev_priv, pipe);
-		st00 = I915_READ8(_VGA_MSR_WRITE);
+		st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
 		status = ((st00 & (1 << 4)) != 0) ?
 			connector_status_connected :
 			connector_status_disconnected;
 
-		I915_WRITE(pipeconf_reg, pipeconf);
+		intel_uncore_write(uncore, pipeconf_reg, pipeconf);
 	} else {
 		bool restore_vblank = false;
 		int count, detect;
@@ -702,9 +705,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
 			u32 vsync_start = (vsync & 0xffff) + 1;
 
 			vblank_start = vsync_start;
-			I915_WRITE(vblank_reg,
-				   (vblank_start - 1) |
-				   ((vblank_end - 1) << 16));
+			intel_uncore_write(uncore,
+					   vblank_reg,
+					   (vblank_start - 1) |
+					   ((vblank_end - 1) << 16));
 			restore_vblank = true;
 		}
 		/* sample in the vertical border, selecting the larger one */
@@ -716,9 +720,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
 		/*
 		 * Wait for the border to be displayed
 		 */
-		while (I915_READ(pipe_dsl_reg) >= vactive)
+		while (intel_uncore_read(uncore, pipe_dsl_reg) >= vactive)
 			;
-		while ((dsl = I915_READ(pipe_dsl_reg)) <= vsample)
+		while ((dsl = intel_uncore_read(uncore, pipe_dsl_reg)) <=
+		       vsample)
 			;
 		/*
 		 * Watch ST00 for an entire scanline
@@ -728,14 +733,14 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
 		do {
 			count++;
 			/* Read the ST00 VGA status register */
-			st00 = I915_READ8(_VGA_MSR_WRITE);
+			st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
 			if (st00 & (1 << 4))
 				detect++;
-		} while ((I915_READ(pipe_dsl_reg) == dsl));
+		} while ((intel_uncore_read(uncore, pipe_dsl_reg) == dsl));
 
 		/* restore vblank if necessary */
 		if (restore_vblank)
-			I915_WRITE(vblank_reg, vblank);
+			intel_uncore_write(uncore, vblank_reg, vblank);
 		/*
 		 * If more than 3/4 of the scanline detected a monitor,
 		 * then it is assumed to be present. This works even on i830,
@@ -748,7 +753,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
 	}
 
 	/* Restore previous settings */
-	I915_WRITE(bclrpat_reg, save_bclrpat);
+	intel_uncore_write(uncore, bclrpat_reg, save_bclrpat);
 
 	return status;
 }
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d7272d4ff258..93e411e6ad19 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -8160,15 +8160,15 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
 	return val;
 }
 
-unsigned long i915_mch_val(struct drm_i915_private *dev_priv)
+unsigned long i915_mch_val(struct drm_i915_private *i915)
 {
 	unsigned long m, x, b;
 	u32 tsfs;
 
-	tsfs = I915_READ(TSFS);
+	tsfs = intel_uncore_read(&i915->uncore, TSFS);
 
 	m = ((tsfs & TSFS_SLOPE_MASK) >> TSFS_SLOPE_SHIFT);
-	x = I915_READ8(TR1);
+	x = intel_uncore_read8(&i915->uncore, TR1);
 
 	b = tsfs & TSFS_INTR_MASK;
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 08/12] drm/i915: Remove I915_POSTING_READ_FW
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (6 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 07/12] drm/i915: Remove I915_READ8 Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:48   ` Jani Nikula
  2019-06-07 12:08 ` [RFC 09/12] drm/i915: Remove POSTING_READ16 Tvrtko Ursulin
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Only a few call sites remain which have been converted to uncore mmio
accessors and so the macro can be removed.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h             |  1 -
 drivers/gpu/drm/i915/i915_gem.c             |  9 +++---
 drivers/gpu/drm/i915/i915_irq.c             |  2 +-
 drivers/gpu/drm/i915/intel_guc_submission.c |  4 +--
 drivers/gpu/drm/i915/intel_pm.c             | 31 +++++++++++----------
 5 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 13815795e197..9dcec93426de 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2891,7 +2891,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
  */
 #define I915_READ_FW(reg__) __I915_REG_OP(read_fw, dev_priv, (reg__))
 #define I915_WRITE_FW(reg__, val__) __I915_REG_OP(write_fw, dev_priv, (reg__), (val__))
-#define POSTING_READ_FW(reg__) __I915_REG_OP(posting_read_fw, dev_priv, (reg__))
 
 /* "Broadcast RGB" property */
 #define INTEL_BROADCAST_RGB_AUTO 0
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9f2e213c6046..94d85b0fb860 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -263,11 +263,12 @@ void i915_gem_flush_ggtt_writes(struct drm_i915_private *dev_priv)
 	i915_gem_chipset_flush(dev_priv);
 
 	with_intel_runtime_pm(dev_priv, wakeref) {
-		spin_lock_irq(&dev_priv->uncore.lock);
+		struct intel_uncore *uncore = &dev_priv->uncore;
 
-		POSTING_READ_FW(RING_HEAD(RENDER_RING_BASE));
-
-		spin_unlock_irq(&dev_priv->uncore.lock);
+		spin_lock_irq(&uncore->lock);
+		intel_uncore_posting_read_fw(uncore,
+					     RING_HEAD(RENDER_RING_BASE));
+		spin_unlock_irq(&uncore->lock);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ca8f4226e598..0763ffffea53 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -386,7 +386,7 @@ static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
 void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, u32 mask)
 {
 	ilk_update_gt_irq(dev_priv, mask, mask);
-	POSTING_READ_FW(GTIMR);
+	intel_uncore_posting_read_fw(&dev_priv->uncore, GTIMR);
 }
 
 void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, u32 mask)
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 89592ef778b8..97f6970d8da8 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -557,10 +557,10 @@ static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
  */
 static void flush_ggtt_writes(struct i915_vma *vma)
 {
-	struct drm_i915_private *dev_priv = vma->vm->i915;
+	struct drm_i915_private *i915 = vma->vm->i915;
 
 	if (i915_vma_is_map_and_fenceable(vma))
-		POSTING_READ_FW(GUC_STATUS);
+		intel_uncore_posting_read_fw(&i915->uncore, GUC_STATUS);
 }
 
 static void inject_preempt_context(struct work_struct *work)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 93e411e6ad19..84588ff8732f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1949,6 +1949,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_uncore *uncore = &dev_priv->uncore;
 	const struct vlv_fifo_state *fifo_state =
 		&crtc_state->wm.vlv.fifo_state;
 	int sprite0_start, sprite1_start, fifo_size;
@@ -1974,13 +1975,13 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 	 * intel_pipe_update_start() has already disabled interrupts
 	 * for us, so a plain spin_lock() is sufficient here.
 	 */
-	spin_lock(&dev_priv->uncore.lock);
+	spin_lock(&uncore->lock);
 
 	switch (crtc->pipe) {
 		u32 dsparb, dsparb2, dsparb3;
 	case PIPE_A:
-		dsparb = I915_READ_FW(DSPARB);
-		dsparb2 = I915_READ_FW(DSPARB2);
+		dsparb = intel_uncore_read_fw(uncore, DSPARB);
+		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
 
 		dsparb &= ~(VLV_FIFO(SPRITEA, 0xff) |
 			    VLV_FIFO(SPRITEB, 0xff));
@@ -1992,12 +1993,12 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEA_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITEB_HI, sprite1_start >> 8));
 
-		I915_WRITE_FW(DSPARB, dsparb);
-		I915_WRITE_FW(DSPARB2, dsparb2);
+		intel_uncore_write_fw(uncore, DSPARB, dsparb);
+		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
 		break;
 	case PIPE_B:
-		dsparb = I915_READ_FW(DSPARB);
-		dsparb2 = I915_READ_FW(DSPARB2);
+		dsparb = intel_uncore_read_fw(uncore, DSPARB);
+		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
 
 		dsparb &= ~(VLV_FIFO(SPRITEC, 0xff) |
 			    VLV_FIFO(SPRITED, 0xff));
@@ -2009,12 +2010,12 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEC_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITED_HI, sprite1_start >> 8));
 
-		I915_WRITE_FW(DSPARB, dsparb);
-		I915_WRITE_FW(DSPARB2, dsparb2);
+		intel_uncore_write_fw(uncore, DSPARB, dsparb);
+		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
 		break;
 	case PIPE_C:
-		dsparb3 = I915_READ_FW(DSPARB3);
-		dsparb2 = I915_READ_FW(DSPARB2);
+		dsparb3 = intel_uncore_read_fw(uncore, DSPARB3);
+		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
 
 		dsparb3 &= ~(VLV_FIFO(SPRITEE, 0xff) |
 			     VLV_FIFO(SPRITEF, 0xff));
@@ -2026,16 +2027,16 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEE_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITEF_HI, sprite1_start >> 8));
 
-		I915_WRITE_FW(DSPARB3, dsparb3);
-		I915_WRITE_FW(DSPARB2, dsparb2);
+		intel_uncore_write_fw(uncore, DSPARB3, dsparb3);
+		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
 		break;
 	default:
 		break;
 	}
 
-	POSTING_READ_FW(DSPARB);
+	intel_uncore_posting_read_fw(uncore, DSPARB);
 
-	spin_unlock(&dev_priv->uncore.lock);
+	spin_unlock(&uncore->lock);
 }
 
 #undef VLV_FIFO
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 09/12] drm/i915: Remove POSTING_READ16
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (7 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 08/12] drm/i915: Remove I915_POSTING_READ_FW Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:49   ` Jani Nikula
  2019-06-07 12:08 ` [RFC 10/12] drm/i915: Remove I915_WRITE_NOTRACE Tvrtko Ursulin
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Only a few call sites remain which have been converted to uncore mmio
accessors and so the macro can be removed.

ENGINE_POSTING_READ16 is added to replace one engine->mmio_base relative
call site.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine.h     |  1 +
 drivers/gpu/drm/i915/gt/intel_ringbuffer.c |  8 ++++----
 drivers/gpu/drm/i915/i915_drv.h            |  1 -
 drivers/gpu/drm/i915/intel_pm.c            | 11 ++++++-----
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 201bbd2a4faf..1439fa4093ac 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -52,6 +52,7 @@ struct drm_printer;
 #define ENGINE_READ(...)	__ENGINE_READ_OP(read, __VA_ARGS__)
 #define ENGINE_READ_FW(...)	__ENGINE_READ_OP(read_fw, __VA_ARGS__)
 #define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read, __VA_ARGS__)
+#define ENGINE_POSTING_READ16(...) __ENGINE_READ_OP(posting_read16, __VA_ARGS__)
 
 #define ENGINE_READ64(engine__, lower_reg__, upper_reg__) \
 	__ENGINE_REG_OP(read64_2x32, (engine__), \
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index ff58d658e3e2..0373af648e72 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -976,11 +976,11 @@ i9xx_irq_disable(struct intel_engine_cs *engine)
 static void
 i8xx_irq_enable(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
+	struct drm_i915_private *i915 = engine->i915;
 
-	dev_priv->irq_mask &= ~engine->irq_enable_mask;
-	I915_WRITE16(GEN2_IMR, dev_priv->irq_mask);
-	POSTING_READ16(RING_IMR(engine->mmio_base));
+	i915->irq_mask &= ~engine->irq_enable_mask;
+	intel_uncore_write16(&i915->uncore, GEN2_IMR, i915->irq_mask);
+	ENGINE_POSTING_READ16(engine, RING_IMR);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9dcec93426de..2fcf6eb8ab3f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2861,7 +2861,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
 #define I915_WRITE_NOTRACE(reg__, val__) __I915_REG_OP(write_notrace, dev_priv, (reg__), (val__))
 
 #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
-#define POSTING_READ16(reg__)	__I915_REG_OP(posting_read16, dev_priv, (reg__))
 
 /* These are untraced mmio-accessors that are only valid to be used inside
  * critical sections, such as inside IRQ handlers, where forcewake is explicitly
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 84588ff8732f..5a6679e2b6ee 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6406,13 +6406,14 @@ void intel_init_ipc(struct drm_i915_private *dev_priv)
  */
 DEFINE_SPINLOCK(mchdev_lock);
 
-bool ironlake_set_drps(struct drm_i915_private *dev_priv, u8 val)
+bool ironlake_set_drps(struct drm_i915_private *i915, u8 val)
 {
+	struct intel_uncore *uncore = &i915->uncore;
 	u16 rgvswctl;
 
 	lockdep_assert_held(&mchdev_lock);
 
-	rgvswctl = I915_READ16(MEMSWCTL);
+	rgvswctl = intel_uncore_read16(uncore, MEMSWCTL);
 	if (rgvswctl & MEMCTL_CMD_STS) {
 		DRM_DEBUG("gpu busy, RCS change rejected\n");
 		return false; /* still busy with another command */
@@ -6420,11 +6421,11 @@ bool ironlake_set_drps(struct drm_i915_private *dev_priv, u8 val)
 
 	rgvswctl = (MEMCTL_CMD_CHFREQ << MEMCTL_CMD_SHIFT) |
 		(val << MEMCTL_FREQ_SHIFT) | MEMCTL_SFCAVM;
-	I915_WRITE16(MEMSWCTL, rgvswctl);
-	POSTING_READ16(MEMSWCTL);
+	intel_uncore_write16(uncore, MEMSWCTL, rgvswctl);
+	intel_uncore_posting_read16(uncore, MEMSWCTL);
 
 	rgvswctl |= MEMCTL_CMD_STS;
-	I915_WRITE16(MEMSWCTL, rgvswctl);
+	intel_uncore_write16(uncore, MEMSWCTL, rgvswctl);
 
 	return true;
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 10/12] drm/i915: Remove I915_WRITE_NOTRACE
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (8 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 09/12] drm/i915: Remove POSTING_READ16 Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:50   ` Jani Nikula
  2019-06-07 12:08 ` [RFC 11/12] drm/i915: Remove I915_READ_NOTRACE Tvrtko Ursulin
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Only a few call sites remain which have been converted to uncore mmio
accessors and so the macro can be removed.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    |  1 -
 drivers/gpu/drm/i915/intel_gmbus.c | 42 +++++++++++++++++++-----------
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2fcf6eb8ab3f..1ea2d5f42e52 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2858,7 +2858,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
 #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
 #define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
 #define I915_READ_NOTRACE(reg__)	 __I915_REG_OP(read_notrace, dev_priv, (reg__))
-#define I915_WRITE_NOTRACE(reg__, val__) __I915_REG_OP(write_notrace, dev_priv, (reg__), (val__))
 
 #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
 
diff --git a/drivers/gpu/drm/i915/intel_gmbus.c b/drivers/gpu/drm/i915/intel_gmbus.c
index 969ce8b71e32..5e4c543e82e8 100644
--- a/drivers/gpu/drm/i915/intel_gmbus.c
+++ b/drivers/gpu/drm/i915/intel_gmbus.c
@@ -201,27 +201,37 @@ static u32 get_reserved(struct intel_gmbus *bus)
 static int get_clock(void *data)
 {
 	struct intel_gmbus *bus = data;
-	struct drm_i915_private *dev_priv = bus->dev_priv;
+	struct intel_uncore *uncore = &bus->dev_priv->uncore;
 	u32 reserved = get_reserved(bus);
-	I915_WRITE_NOTRACE(bus->gpio_reg, reserved | GPIO_CLOCK_DIR_MASK);
-	I915_WRITE_NOTRACE(bus->gpio_reg, reserved);
-	return (I915_READ_NOTRACE(bus->gpio_reg) & GPIO_CLOCK_VAL_IN) != 0;
+
+	intel_uncore_write_notrace(uncore,
+				   bus->gpio_reg,
+				   reserved | GPIO_CLOCK_DIR_MASK);
+	intel_uncore_write_notrace(uncore, bus->gpio_reg, reserved);
+
+	return (intel_uncore_read_notrace(uncore, bus->gpio_reg) &
+		GPIO_CLOCK_VAL_IN) != 0;
 }
 
 static int get_data(void *data)
 {
 	struct intel_gmbus *bus = data;
-	struct drm_i915_private *dev_priv = bus->dev_priv;
+	struct intel_uncore *uncore = &bus->dev_priv->uncore;
 	u32 reserved = get_reserved(bus);
-	I915_WRITE_NOTRACE(bus->gpio_reg, reserved | GPIO_DATA_DIR_MASK);
-	I915_WRITE_NOTRACE(bus->gpio_reg, reserved);
-	return (I915_READ_NOTRACE(bus->gpio_reg) & GPIO_DATA_VAL_IN) != 0;
+
+	intel_uncore_write_notrace(uncore,
+				   bus->gpio_reg,
+				   reserved | GPIO_DATA_DIR_MASK);
+	intel_uncore_write_notrace(uncore, bus->gpio_reg, reserved);
+
+	return (intel_uncore_read_notrace(uncore, bus->gpio_reg) &
+		GPIO_DATA_VAL_IN) != 0;
 }
 
 static void set_clock(void *data, int state_high)
 {
 	struct intel_gmbus *bus = data;
-	struct drm_i915_private *dev_priv = bus->dev_priv;
+	struct intel_uncore *uncore = &bus->dev_priv->uncore;
 	u32 reserved = get_reserved(bus);
 	u32 clock_bits;
 
@@ -229,16 +239,18 @@ static void set_clock(void *data, int state_high)
 		clock_bits = GPIO_CLOCK_DIR_IN | GPIO_CLOCK_DIR_MASK;
 	else
 		clock_bits = GPIO_CLOCK_DIR_OUT | GPIO_CLOCK_DIR_MASK |
-			GPIO_CLOCK_VAL_MASK;
+			     GPIO_CLOCK_VAL_MASK;
 
-	I915_WRITE_NOTRACE(bus->gpio_reg, reserved | clock_bits);
-	POSTING_READ(bus->gpio_reg);
+	intel_uncore_write_notrace(uncore,
+				   bus->gpio_reg,
+				   reserved | clock_bits);
+	intel_uncore_posting_read(uncore, bus->gpio_reg);
 }
 
 static void set_data(void *data, int state_high)
 {
 	struct intel_gmbus *bus = data;
-	struct drm_i915_private *dev_priv = bus->dev_priv;
+	struct intel_uncore *uncore = &bus->dev_priv->uncore;
 	u32 reserved = get_reserved(bus);
 	u32 data_bits;
 
@@ -248,8 +260,8 @@ static void set_data(void *data, int state_high)
 		data_bits = GPIO_DATA_DIR_OUT | GPIO_DATA_DIR_MASK |
 			GPIO_DATA_VAL_MASK;
 
-	I915_WRITE_NOTRACE(bus->gpio_reg, reserved | data_bits);
-	POSTING_READ(bus->gpio_reg);
+	intel_uncore_write_notrace(uncore, bus->gpio_reg, reserved | data_bits);
+	intel_uncore_posting_read(uncore, bus->gpio_reg);
 }
 
 static int
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 11/12] drm/i915: Remove I915_READ_NOTRACE
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (9 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 10/12] drm/i915: Remove I915_WRITE_NOTRACE Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-11  8:57   ` Jani Nikula
  2019-06-07 12:08 ` [RFC 12/12] drm/i915: Remove I915_READ16 and I915_WRITE16 Tvrtko Ursulin
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Only a few call sites remain which have been converted to uncore mmio
accessors and so the macro can be removed.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gvt/debugfs.c  |  4 +--
 drivers/gpu/drm/i915/gvt/firmware.c |  5 ++--
 drivers/gpu/drm/i915/i915_drv.c     |  6 ++--
 drivers/gpu/drm/i915/i915_drv.h     |  1 -
 drivers/gpu/drm/i915/i915_pmu.c     |  8 ++++--
 drivers/gpu/drm/i915/intel_dp.c     | 43 +++++++++++++++--------------
 drivers/gpu/drm/i915/intel_gmbus.c  | 11 ++++----
 7 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/debugfs.c b/drivers/gpu/drm/i915/gvt/debugfs.c
index 8a9606f91e68..2fb7b73b260d 100644
--- a/drivers/gpu/drm/i915/gvt/debugfs.c
+++ b/drivers/gpu/drm/i915/gvt/debugfs.c
@@ -58,12 +58,12 @@ static int mmio_offset_compare(void *priv,
 static inline int mmio_diff_handler(struct intel_gvt *gvt,
 				    u32 offset, void *data)
 {
-	struct drm_i915_private *dev_priv = gvt->dev_priv;
+	struct drm_i915_private *i915 = gvt->dev_priv;
 	struct mmio_diff_param *param = data;
 	struct diff_mmio *node;
 	u32 preg, vreg;
 
-	preg = I915_READ_NOTRACE(_MMIO(offset));
+	preg = intel_uncore_read_notrace(&i915->uncore, _MMIO(offset));
 	vreg = vgpu_vreg(param->vgpu, offset);
 
 	if (preg != vreg) {
diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c
index 4ac18b447247..049775e8e350 100644
--- a/drivers/gpu/drm/i915/gvt/firmware.c
+++ b/drivers/gpu/drm/i915/gvt/firmware.c
@@ -68,9 +68,10 @@ static struct bin_attribute firmware_attr = {
 
 static int mmio_snapshot_handler(struct intel_gvt *gvt, u32 offset, void *data)
 {
-	struct drm_i915_private *dev_priv = gvt->dev_priv;
+	struct drm_i915_private *i915 = gvt->dev_priv;
 
-	*(u32 *)(data + offset) = I915_READ_NOTRACE(_MMIO(offset));
+	*(u32 *)(data + offset) = intel_uncore_read_notrace(&i915->uncore,
+							    _MMIO(offset));
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1af6751e1b36..81ff2c78fd55 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2708,7 +2708,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)
 	I915_WRITE(VLV_GUNIT_CLOCK_GATE2,	s->clock_gate_dis2);
 }
 
-static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
+static int vlv_wait_for_pw_status(struct drm_i915_private *i915,
 				  u32 mask, u32 val)
 {
 	i915_reg_t reg = VLV_GTLC_PW_STATUS;
@@ -2722,7 +2722,9 @@ static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
 	 * Transitioning between RC6 states should be at most 2ms (see
 	 * valleyview_enable_rps) so use a 3ms timeout.
 	 */
-	ret = wait_for(((reg_value = I915_READ_NOTRACE(reg)) & mask) == val, 3);
+	ret = wait_for(((reg_value =
+			 intel_uncore_read_notrace(&i915->uncore, reg)) & mask)
+		       == val, 3);
 
 	/* just trace the final value */
 	trace_i915_reg_rw(false, reg, reg_value, sizeof(reg_value), true);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1ea2d5f42e52..9f0208e410ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2857,7 +2857,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
 
 #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
 #define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
-#define I915_READ_NOTRACE(reg__)	 __I915_REG_OP(read_notrace, dev_priv, (reg__))
 
 #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
 
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 1ccda0ee4ff5..eb9c0e0e545c 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -227,9 +227,11 @@ frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns)
 		if (dev_priv->gt.awake) {
 			intel_wakeref_t wakeref;
 
-			with_intel_runtime_pm_if_in_use(dev_priv, wakeref)
-				val = intel_get_cagf(dev_priv,
-						     I915_READ_NOTRACE(GEN6_RPSTAT1));
+			with_intel_runtime_pm_if_in_use(dev_priv, wakeref) {
+				val = intel_uncore_read_notrace(&dev_priv->uncore,
+								GEN6_RPSTAT1);
+				val = intel_get_cagf(dev_priv, val);
+			}
 		}
 
 		add_sample_mult(&dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT],
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b099a9dc28fd..0862678e68b9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1082,13 +1082,13 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
 static u32
 intel_dp_aux_wait_done(struct intel_dp *intel_dp)
 {
-	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
 	u32 status;
 	bool done;
 
-#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
-	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
+#define C (((status = intel_uncore_read_notrace(&i915->uncore, ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
+	done = wait_event_timeout(i915->gmbus_wait_queue, C,
 				  msecs_to_jiffies_timeout(10));
 
 	/* just trace the final value */
@@ -1221,8 +1221,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
 		  u32 aux_send_ctl_flags)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv =
+	struct drm_i915_private *i915 =
 			to_i915(intel_dig_port->base.base.dev);
+	struct intel_uncore *uncore = &i915->uncore;
 	i915_reg_t ch_ctl, ch_data[5];
 	u32 aux_clock_divider;
 	enum intel_display_power_domain aux_domain =
@@ -1238,7 +1239,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
 	for (i = 0; i < ARRAY_SIZE(ch_data); i++)
 		ch_data[i] = intel_dp->aux_ch_data_reg(intel_dp, i);
 
-	aux_wakeref = intel_display_power_get(dev_priv, aux_domain);
+	aux_wakeref = intel_display_power_get(i915, aux_domain);
 	pps_wakeref = pps_lock(intel_dp);
 
 	/*
@@ -1253,13 +1254,13 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
 	 * lowest possible wakeup latency and so prevent the cpu from going into
 	 * deep sleep states.
 	 */
-	pm_qos_update_request(&dev_priv->pm_qos, 0);
+	pm_qos_update_request(&i915->pm_qos, 0);
 
 	intel_dp_check_edp(intel_dp);
 
 	/* Try to wait for any previous AUX channel activity */
 	for (try = 0; try < 3; try++) {
-		status = I915_READ_NOTRACE(ch_ctl);
+		status = intel_uncore_read_notrace(uncore, ch_ctl);
 		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 			break;
 		msleep(1);
@@ -1269,7 +1270,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
 
 	if (try == 3) {
 		static u32 last_status = -1;
-		const u32 status = I915_READ(ch_ctl);
+		const u32 status = intel_uncore_read(uncore, ch_ctl);
 
 		if (status != last_status) {
 			WARN(1, "dp_aux_ch not started status 0x%08x\n",
@@ -1298,21 +1299,23 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
 		for (try = 0; try < 5; try++) {
 			/* Load the send data into the aux channel data registers */
 			for (i = 0; i < send_bytes; i += 4)
-				I915_WRITE(ch_data[i >> 2],
-					   intel_dp_pack_aux(send + i,
-							     send_bytes - i));
+				intel_uncore_write(uncore,
+						   ch_data[i >> 2],
+						   intel_dp_pack_aux(send + i,
+								     send_bytes - i));
 
 			/* Send the command and wait for it to complete */
-			I915_WRITE(ch_ctl, send_ctl);
+			intel_uncore_write(uncore, ch_ctl, send_ctl);
 
 			status = intel_dp_aux_wait_done(intel_dp);
 
 			/* Clear done status and any errors */
-			I915_WRITE(ch_ctl,
-				   status |
-				   DP_AUX_CH_CTL_DONE |
-				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
-				   DP_AUX_CH_CTL_RECEIVE_ERROR);
+			intel_uncore_write(uncore,
+					   ch_ctl,
+					   status |
+					   DP_AUX_CH_CTL_DONE |
+					   DP_AUX_CH_CTL_TIME_OUT_ERROR |
+					   DP_AUX_CH_CTL_RECEIVE_ERROR);
 
 			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
 			 *   400us delay required for errors and timeouts
@@ -1375,18 +1378,18 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
 		recv_bytes = recv_size;
 
 	for (i = 0; i < recv_bytes; i += 4)
-		intel_dp_unpack_aux(I915_READ(ch_data[i >> 2]),
+		intel_dp_unpack_aux(intel_uncore_read(uncore, ch_data[i >> 2]),
 				    recv + i, recv_bytes - i);
 
 	ret = recv_bytes;
 out:
-	pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
+	pm_qos_update_request(&i915->pm_qos, PM_QOS_DEFAULT_VALUE);
 
 	if (vdd)
 		edp_panel_vdd_off(intel_dp, false);
 
 	pps_unlock(intel_dp, pps_wakeref);
-	intel_display_power_put_async(dev_priv, aux_domain, aux_wakeref);
+	intel_display_power_put_async(i915, aux_domain, aux_wakeref);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_gmbus.c b/drivers/gpu/drm/i915/intel_gmbus.c
index 5e4c543e82e8..aa88e6e7cc65 100644
--- a/drivers/gpu/drm/i915/intel_gmbus.c
+++ b/drivers/gpu/drm/i915/intel_gmbus.c
@@ -186,14 +186,15 @@ static void bxt_gmbus_clock_gating(struct drm_i915_private *dev_priv,
 
 static u32 get_reserved(struct intel_gmbus *bus)
 {
-	struct drm_i915_private *dev_priv = bus->dev_priv;
+	struct drm_i915_private *i915 = bus->dev_priv;
+	struct intel_uncore *uncore = &i915->uncore;
 	u32 reserved = 0;
 
 	/* On most chips, these bits must be preserved in software. */
-	if (!IS_I830(dev_priv) && !IS_I845G(dev_priv))
-		reserved = I915_READ_NOTRACE(bus->gpio_reg) &
-					     (GPIO_DATA_PULLUP_DISABLE |
-					      GPIO_CLOCK_PULLUP_DISABLE);
+	if (!IS_I830(i915) && !IS_I845G(i915))
+		reserved = intel_uncore_read_notrace(uncore, bus->gpio_reg) &
+			   (GPIO_DATA_PULLUP_DISABLE |
+			    GPIO_CLOCK_PULLUP_DISABLE);
 
 	return reserved;
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC 12/12] drm/i915: Remove I915_READ16 and I915_WRITE16
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (10 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 11/12] drm/i915: Remove I915_READ_NOTRACE Tvrtko Ursulin
@ 2019-06-07 12:08 ` Tvrtko Ursulin
  2019-06-07 12:59   ` Jani Nikula
  2019-06-07 14:36 ` ✗ Fi.CI.CHECKPATCH: warning for Legacy mmio accessor macro pruning Patchwork
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 12:08 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Jani Nikula

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Remove call sites in favour of uncore mmio accessors and remove the old
macros.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_ringbuffer.c |   6 +-
 drivers/gpu/drm/i915/i915_debugfs.c        |  37 ++++----
 drivers/gpu/drm/i915/i915_drv.h            |   3 -
 drivers/gpu/drm/i915/i915_gpu_error.c      | 104 ++++++++++++---------
 drivers/gpu/drm/i915/i915_irq.c            |  40 ++++----
 drivers/gpu/drm/i915/intel_pm.c            |  93 ++++++++++--------
 6 files changed, 161 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index 0373af648e72..be0ba0b742f7 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -986,10 +986,10 @@ i8xx_irq_enable(struct intel_engine_cs *engine)
 static void
 i8xx_irq_disable(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
+	struct drm_i915_private *i915 = engine->i915;
 
-	dev_priv->irq_mask |= engine->irq_enable_mask;
-	I915_WRITE16(GEN2_IMR, dev_priv->irq_mask);
+	i915->irq_mask |= engine->irq_enable_mask;
+	intel_uncore_write16(&i915->uncore, GEN2_IMR, i915->irq_mask);
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f212241a2758..a11e06c21165 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -998,6 +998,7 @@ static const struct file_operations i915_error_state_fops = {
 static int i915_frequency_info(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
+	struct intel_uncore *uncore = &dev_priv->uncore;
 	struct intel_rps *rps = &dev_priv->gt_pm.rps;
 	intel_wakeref_t wakeref;
 	int ret = 0;
@@ -1005,8 +1006,8 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 	wakeref = intel_runtime_pm_get(dev_priv);
 
 	if (IS_GEN(dev_priv, 5)) {
-		u16 rgvswctl = I915_READ16(MEMSWCTL);
-		u16 rgvstat = I915_READ16(MEMSTAT_ILK);
+		u16 rgvswctl = intel_uncore_read16(uncore, MEMSWCTL);
+		u16 rgvstat = intel_uncore_read16(uncore, MEMSTAT_ILK);
 
 		seq_printf(m, "Requested P-state: %d\n", (rgvswctl >> 8) & 0xf);
 		seq_printf(m, "Requested VID: %d\n", rgvswctl & 0x3f);
@@ -1328,13 +1329,14 @@ static int i915_reset_info(struct seq_file *m, void *unused)
 
 static int ironlake_drpc_info(struct seq_file *m)
 {
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
+	struct drm_i915_private *i915 = node_to_i915(m->private);
+	struct intel_uncore *uncore = &i915->uncore;
 	u32 rgvmodectl, rstdbyctl;
 	u16 crstandvid;
 
-	rgvmodectl = I915_READ(MEMMODECTL);
-	rstdbyctl = I915_READ(RSTDBYCTL);
-	crstandvid = I915_READ16(CRSTANDVID);
+	rgvmodectl = intel_uncore_read(uncore, MEMMODECTL);
+	rstdbyctl = intel_uncore_read(uncore, RSTDBYCTL);
+	crstandvid = intel_uncore_read16(uncore, CRSTANDVID);
 
 	seq_printf(m, "HD boost: %s\n", yesno(rgvmodectl & MEMMODE_BOOST_EN));
 	seq_printf(m, "Boost freq: %d\n",
@@ -1917,6 +1919,7 @@ static const char *swizzle_string(unsigned swizzle)
 static int i915_swizzle_info(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
+	struct intel_uncore *uncore = &dev_priv->uncore;
 	intel_wakeref_t wakeref;
 
 	wakeref = intel_runtime_pm_get(dev_priv);
@@ -1928,30 +1931,30 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
 
 	if (IS_GEN_RANGE(dev_priv, 3, 4)) {
 		seq_printf(m, "DDC = 0x%08x\n",
-			   I915_READ(DCC));
+			   intel_uncore_read(uncore, DCC));
 		seq_printf(m, "DDC2 = 0x%08x\n",
-			   I915_READ(DCC2));
+			   intel_uncore_read(uncore, DCC2));
 		seq_printf(m, "C0DRB3 = 0x%04x\n",
-			   I915_READ16(C0DRB3));
+			   intel_uncore_read16(uncore, C0DRB3));
 		seq_printf(m, "C1DRB3 = 0x%04x\n",
-			   I915_READ16(C1DRB3));
+			   intel_uncore_read16(uncore, C1DRB3));
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n",
-			   I915_READ(MAD_DIMM_C0));
+			   intel_uncore_read(uncore, MAD_DIMM_C0));
 		seq_printf(m, "MAD_DIMM_C1 = 0x%08x\n",
-			   I915_READ(MAD_DIMM_C1));
+			   intel_uncore_read(uncore, MAD_DIMM_C1));
 		seq_printf(m, "MAD_DIMM_C2 = 0x%08x\n",
-			   I915_READ(MAD_DIMM_C2));
+			   intel_uncore_read(uncore, MAD_DIMM_C2));
 		seq_printf(m, "TILECTL = 0x%08x\n",
-			   I915_READ(TILECTL));
+			   intel_uncore_read(uncore, TILECTL));
 		if (INTEL_GEN(dev_priv) >= 8)
 			seq_printf(m, "GAMTARBMODE = 0x%08x\n",
-				   I915_READ(GAMTARBMODE));
+				   intel_uncore_read(uncore, GAMTARBMODE));
 		else
 			seq_printf(m, "ARB_MODE = 0x%08x\n",
-				   I915_READ(ARB_MODE));
+				   intel_uncore_read(uncore, ARB_MODE));
 		seq_printf(m, "DISP_ARB_CTL = 0x%08x\n",
-			   I915_READ(DISP_ARB_CTL));
+			   intel_uncore_read(uncore, DISP_ARB_CTL));
 	}
 
 	if (dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9f0208e410ed..261b65caaf80 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2852,9 +2852,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
 #define __I915_REG_OP(op__, dev_priv__, ...) \
 	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
 
-#define I915_READ16(reg__)	   __I915_REG_OP(read16, dev_priv, (reg__))
-#define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
-
 #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
 #define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
 
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index a523bf050a25..9caf91d98b27 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1576,7 +1576,8 @@ static void capture_uc_state(struct i915_gpu_state *error)
 /* Capture all registers which don't fit into another category. */
 static void capture_reg_state(struct i915_gpu_state *error)
 {
-	struct drm_i915_private *dev_priv = error->i915;
+	struct drm_i915_private *i915 = error->i915;
+	struct intel_uncore *uncore = &i915->uncore;
 	int i;
 
 	/* General organization
@@ -1588,71 +1589,84 @@ static void capture_reg_state(struct i915_gpu_state *error)
 	 */
 
 	/* 1: Registers specific to a single generation */
-	if (IS_VALLEYVIEW(dev_priv)) {
-		error->gtier[0] = I915_READ(GTIER);
-		error->ier = I915_READ(VLV_IER);
-		error->forcewake = I915_READ_FW(FORCEWAKE_VLV);
+	if (IS_VALLEYVIEW(i915)) {
+		error->gtier[0] = intel_uncore_read(uncore, GTIER);
+		error->ier = intel_uncore_read(uncore, VLV_IER);
+		error->forcewake = intel_uncore_read_fw(uncore, FORCEWAKE_VLV);
 	}
 
-	if (IS_GEN(dev_priv, 7))
-		error->err_int = I915_READ(GEN7_ERR_INT);
+	if (IS_GEN(i915, 7))
+		error->err_int = intel_uncore_read(uncore, GEN7_ERR_INT);
 
-	if (INTEL_GEN(dev_priv) >= 8) {
-		error->fault_data0 = I915_READ(GEN8_FAULT_TLB_DATA0);
-		error->fault_data1 = I915_READ(GEN8_FAULT_TLB_DATA1);
+	if (INTEL_GEN(i915) >= 8) {
+		error->fault_data0 = intel_uncore_read(uncore,
+						       GEN8_FAULT_TLB_DATA0);
+		error->fault_data1 = intel_uncore_read(uncore,
+						       GEN8_FAULT_TLB_DATA1);
 	}
 
-	if (IS_GEN(dev_priv, 6)) {
-		error->forcewake = I915_READ_FW(FORCEWAKE);
-		error->gab_ctl = I915_READ(GAB_CTL);
-		error->gfx_mode = I915_READ(GFX_MODE);
+	if (IS_GEN(i915, 6)) {
+		error->forcewake = intel_uncore_read_fw(uncore, FORCEWAKE);
+		error->gab_ctl = intel_uncore_read(uncore, GAB_CTL);
+		error->gfx_mode = intel_uncore_read(uncore, GFX_MODE);
 	}
 
 	/* 2: Registers which belong to multiple generations */
-	if (INTEL_GEN(dev_priv) >= 7)
-		error->forcewake = I915_READ_FW(FORCEWAKE_MT);
+	if (INTEL_GEN(i915) >= 7)
+		error->forcewake = intel_uncore_read_fw(uncore, FORCEWAKE_MT);
 
-	if (INTEL_GEN(dev_priv) >= 6) {
-		error->derrmr = I915_READ(DERRMR);
-		error->error = I915_READ(ERROR_GEN6);
-		error->done_reg = I915_READ(DONE_REG);
+	if (INTEL_GEN(i915) >= 6) {
+		error->derrmr = intel_uncore_read(uncore, DERRMR);
+		error->error = intel_uncore_read(uncore, ERROR_GEN6);
+		error->done_reg = intel_uncore_read(uncore, DONE_REG);
 	}
 
-	if (INTEL_GEN(dev_priv) >= 5)
-		error->ccid = I915_READ(CCID(RENDER_RING_BASE));
+	if (INTEL_GEN(i915) >= 5)
+		error->ccid = intel_uncore_read(uncore, CCID(RENDER_RING_BASE));
 
 	/* 3: Feature specific registers */
-	if (IS_GEN_RANGE(dev_priv, 6, 7)) {
-		error->gam_ecochk = I915_READ(GAM_ECOCHK);
-		error->gac_eco = I915_READ(GAC_ECO_BITS);
+	if (IS_GEN_RANGE(i915, 6, 7)) {
+		error->gam_ecochk = intel_uncore_read(uncore, GAM_ECOCHK);
+		error->gac_eco = intel_uncore_read(uncore, GAC_ECO_BITS);
 	}
 
 	/* 4: Everything else */
-	if (INTEL_GEN(dev_priv) >= 11) {
-		error->ier = I915_READ(GEN8_DE_MISC_IER);
-		error->gtier[0] = I915_READ(GEN11_RENDER_COPY_INTR_ENABLE);
-		error->gtier[1] = I915_READ(GEN11_VCS_VECS_INTR_ENABLE);
-		error->gtier[2] = I915_READ(GEN11_GUC_SG_INTR_ENABLE);
-		error->gtier[3] = I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE);
-		error->gtier[4] = I915_READ(GEN11_CRYPTO_RSVD_INTR_ENABLE);
-		error->gtier[5] = I915_READ(GEN11_GUNIT_CSME_INTR_ENABLE);
+	if (INTEL_GEN(i915) >= 11) {
+		error->ier = intel_uncore_read(uncore, GEN8_DE_MISC_IER);
+		error->gtier[0] =
+			intel_uncore_read(uncore,
+					  GEN11_RENDER_COPY_INTR_ENABLE);
+		error->gtier[1] =
+			intel_uncore_read(uncore, GEN11_VCS_VECS_INTR_ENABLE);
+		error->gtier[2] =
+			intel_uncore_read(uncore, GEN11_GUC_SG_INTR_ENABLE);
+		error->gtier[3] =
+			intel_uncore_read(uncore,
+					  GEN11_GPM_WGBOXPERF_INTR_ENABLE);
+		error->gtier[4] =
+			intel_uncore_read(uncore,
+					  GEN11_CRYPTO_RSVD_INTR_ENABLE);
+		error->gtier[5] =
+			intel_uncore_read(uncore,
+					  GEN11_GUNIT_CSME_INTR_ENABLE);
 		error->ngtier = 6;
-	} else if (INTEL_GEN(dev_priv) >= 8) {
-		error->ier = I915_READ(GEN8_DE_MISC_IER);
+	} else if (INTEL_GEN(i915) >= 8) {
+		error->ier = intel_uncore_read(uncore, GEN8_DE_MISC_IER);
 		for (i = 0; i < 4; i++)
-			error->gtier[i] = I915_READ(GEN8_GT_IER(i));
+			error->gtier[i] = intel_uncore_read(uncore,
+							    GEN8_GT_IER(i));
 		error->ngtier = 4;
-	} else if (HAS_PCH_SPLIT(dev_priv)) {
-		error->ier = I915_READ(DEIER);
-		error->gtier[0] = I915_READ(GTIER);
+	} else if (HAS_PCH_SPLIT(i915)) {
+		error->ier = intel_uncore_read(uncore, DEIER);
+		error->gtier[0] = intel_uncore_read(uncore, GTIER);
 		error->ngtier = 1;
-	} else if (IS_GEN(dev_priv, 2)) {
-		error->ier = I915_READ16(GEN2_IER);
-	} else if (!IS_VALLEYVIEW(dev_priv)) {
-		error->ier = I915_READ(GEN2_IER);
+	} else if (IS_GEN(i915, 2)) {
+		error->ier = intel_uncore_read16(uncore, GEN2_IER);
+	} else if (!IS_VALLEYVIEW(i915)) {
+		error->ier = intel_uncore_read(uncore, GEN2_IER);
 	}
-	error->eir = I915_READ(EIR);
-	error->pgtbl_er = I915_READ(PGTBL_ER);
+	error->eir = intel_uncore_read(uncore, EIR);
+	error->pgtbl_er = intel_uncore_read(uncore, PGTBL_ER);
 }
 
 static const char *
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0763ffffea53..9680f1376ac5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1231,20 +1231,23 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
 
 static void ironlake_rps_change_irq_handler(struct drm_i915_private *dev_priv)
 {
+	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 busy_up, busy_down, max_avg, min_avg;
 	u8 new_delay;
 
 	spin_lock(&mchdev_lock);
 
-	I915_WRITE16(MEMINTRSTS, I915_READ(MEMINTRSTS));
+	intel_uncore_write16(uncore,
+			     MEMINTRSTS,
+			     intel_uncore_read(uncore, MEMINTRSTS));
 
 	new_delay = dev_priv->ips.cur_delay;
 
-	I915_WRITE16(MEMINTRSTS, MEMINT_EVAL_CHG);
-	busy_up = I915_READ(RCPREVBSYTUPAVG);
-	busy_down = I915_READ(RCPREVBSYTDNAVG);
-	max_avg = I915_READ(RCBMAXAVG);
-	min_avg = I915_READ(RCBMINAVG);
+	intel_uncore_write16(uncore, MEMINTRSTS, MEMINT_EVAL_CHG);
+	busy_up = intel_uncore_read(uncore, RCPREVBSYTUPAVG);
+	busy_down = intel_uncore_read(uncore, RCPREVBSYTDNAVG);
+	max_avg = intel_uncore_read(uncore, RCBMAXAVG);
+	min_avg = intel_uncore_read(uncore, RCBMINAVG);
 
 	/* Handle RCS change request from hw */
 	if (busy_up > max_avg) {
@@ -4323,8 +4326,10 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u16 enable_mask;
 
-	I915_WRITE16(EMR, ~(I915_ERROR_PAGE_TABLE |
-			    I915_ERROR_MEMORY_REFRESH));
+	intel_uncore_write16(uncore,
+			     EMR,
+			     ~(I915_ERROR_PAGE_TABLE |
+			       I915_ERROR_MEMORY_REFRESH));
 
 	/* Unmask the interrupts that we always want on. */
 	dev_priv->irq_mask =
@@ -4350,17 +4355,18 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-static void i8xx_error_irq_ack(struct drm_i915_private *dev_priv,
+static void i8xx_error_irq_ack(struct drm_i915_private *i915,
 			       u16 *eir, u16 *eir_stuck)
 {
+	struct intel_uncore *uncore = &i915->uncore;
 	u16 emr;
 
-	*eir = I915_READ16(EIR);
+	*eir = intel_uncore_read16(uncore, EIR);
 
 	if (*eir)
-		I915_WRITE16(EIR, *eir);
+		intel_uncore_write16(uncore, EIR, *eir);
 
-	*eir_stuck = I915_READ16(EIR);
+	*eir_stuck = intel_uncore_read16(uncore, EIR);
 	if (*eir_stuck == 0)
 		return;
 
@@ -4374,9 +4380,9 @@ static void i8xx_error_irq_ack(struct drm_i915_private *dev_priv,
 	 * (or by a GPU reset) so we mask any bit that
 	 * remains set.
 	 */
-	emr = I915_READ16(EMR);
-	I915_WRITE16(EMR, 0xffff);
-	I915_WRITE16(EMR, emr | *eir_stuck);
+	emr = intel_uncore_read16(uncore, EMR);
+	intel_uncore_write16(uncore, EMR, 0xffff);
+	intel_uncore_write16(uncore, EMR, emr | *eir_stuck);
 }
 
 static void i8xx_error_irq_handler(struct drm_i915_private *dev_priv,
@@ -4442,7 +4448,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 		u16 eir = 0, eir_stuck = 0;
 		u16 iir;
 
-		iir = I915_READ16(GEN2_IIR);
+		iir = intel_uncore_read16(&dev_priv->uncore, GEN2_IIR);
 		if (iir == 0)
 			break;
 
@@ -4455,7 +4461,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 		if (iir & I915_MASTER_ERROR_INTERRUPT)
 			i8xx_error_irq_ack(dev_priv, &eir, &eir_stuck);
 
-		I915_WRITE16(GEN2_IIR, iir);
+		intel_uncore_write16(&dev_priv->uncore, GEN2_IIR, iir);
 
 		if (iir & I915_USER_INTERRUPT)
 			intel_engine_breadcrumbs_irq(dev_priv->engine[RCS0]);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5a6679e2b6ee..2c7f3ebc0117 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -191,8 +191,8 @@ static void i915_ironlake_get_mem_freq(struct drm_i915_private *dev_priv)
 {
 	u16 ddrpll, csipll;
 
-	ddrpll = I915_READ16(DDRMPLL1);
-	csipll = I915_READ16(CSIPLL0);
+	ddrpll = intel_uncore_read16(&dev_priv->uncore, DDRMPLL1);
+	csipll = intel_uncore_read16(&dev_priv->uncore, CSIPLL0);
 
 	switch (ddrpll & 0xff) {
 	case 0xc:
@@ -6432,26 +6432,27 @@ bool ironlake_set_drps(struct drm_i915_private *i915, u8 val)
 
 static void ironlake_enable_drps(struct drm_i915_private *dev_priv)
 {
+	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 rgvmodectl;
 	u8 fmax, fmin, fstart, vstart;
 
 	spin_lock_irq(&mchdev_lock);
 
-	rgvmodectl = I915_READ(MEMMODECTL);
+	rgvmodectl = intel_uncore_read(uncore, MEMMODECTL);
 
 	/* Enable temp reporting */
-	I915_WRITE16(PMMISC, I915_READ(PMMISC) | MCPPCE_EN);
-	I915_WRITE16(TSC1, I915_READ(TSC1) | TSE);
+	intel_uncore_write16(uncore, PMMISC, I915_READ(PMMISC) | MCPPCE_EN);
+	intel_uncore_write16(uncore, TSC1, I915_READ(TSC1) | TSE);
 
 	/* 100ms RC evaluation intervals */
-	I915_WRITE(RCUPEI, 100000);
-	I915_WRITE(RCDNEI, 100000);
+	intel_uncore_write(uncore, RCUPEI, 100000);
+	intel_uncore_write(uncore, RCDNEI, 100000);
 
 	/* Set max/min thresholds to 90ms and 80ms respectively */
-	I915_WRITE(RCBMAXAVG, 90000);
-	I915_WRITE(RCBMINAVG, 80000);
+	intel_uncore_write(uncore, RCBMAXAVG, 90000);
+	intel_uncore_write(uncore, RCBMINAVG, 80000);
 
-	I915_WRITE(MEMIHYST, 1);
+	intel_uncore_write(uncore, MEMIHYST, 1);
 
 	/* Set up min, max, and cur for interrupt handling */
 	fmax = (rgvmodectl & MEMMODE_FMAX_MASK) >> MEMMODE_FMAX_SHIFT;
@@ -6459,8 +6460,8 @@ static void ironlake_enable_drps(struct drm_i915_private *dev_priv)
 	fstart = (rgvmodectl & MEMMODE_FSTART_MASK) >>
 		MEMMODE_FSTART_SHIFT;
 
-	vstart = (I915_READ(PXVFREQ(fstart)) & PXVFREQ_PX_MASK) >>
-		PXVFREQ_PX_SHIFT;
+	vstart = (intel_uncore_read(uncore, PXVFREQ(fstart)) &
+		  PXVFREQ_PX_MASK) >> PXVFREQ_PX_SHIFT;
 
 	dev_priv->ips.fmax = fmax; /* IPS callback will increase this */
 	dev_priv->ips.fstart = fstart;
@@ -6472,53 +6473,66 @@ static void ironlake_enable_drps(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_DRIVER("fmax: %d, fmin: %d, fstart: %d\n",
 			 fmax, fmin, fstart);
 
-	I915_WRITE(MEMINTREN, MEMINT_CX_SUPR_EN | MEMINT_EVAL_CHG_EN);
+	intel_uncore_write(uncore,
+			   MEMINTREN,
+			   MEMINT_CX_SUPR_EN | MEMINT_EVAL_CHG_EN);
 
 	/*
 	 * Interrupts will be enabled in ironlake_irq_postinstall
 	 */
 
-	I915_WRITE(VIDSTART, vstart);
-	POSTING_READ(VIDSTART);
+	intel_uncore_write(uncore, VIDSTART, vstart);
+	intel_uncore_posting_read(uncore, VIDSTART);
 
 	rgvmodectl |= MEMMODE_SWMODE_EN;
-	I915_WRITE(MEMMODECTL, rgvmodectl);
+	intel_uncore_write(uncore, MEMMODECTL, rgvmodectl);
 
-	if (wait_for_atomic((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10))
+	if (wait_for_atomic((intel_uncore_read(uncore, MEMSWCTL) &
+			     MEMCTL_CMD_STS) == 0, 10))
 		DRM_ERROR("stuck trying to change perf mode\n");
 	mdelay(1);
 
 	ironlake_set_drps(dev_priv, fstart);
 
-	dev_priv->ips.last_count1 = I915_READ(DMIEC) +
-		I915_READ(DDREC) + I915_READ(CSIEC);
+	dev_priv->ips.last_count1 =
+		intel_uncore_read(uncore, DMIEC) +
+		intel_uncore_read(uncore, DDREC) +
+		intel_uncore_read(uncore, CSIEC);
 	dev_priv->ips.last_time1 = jiffies_to_msecs(jiffies);
-	dev_priv->ips.last_count2 = I915_READ(GFXEC);
+	dev_priv->ips.last_count2 = intel_uncore_read(uncore, GFXEC);
 	dev_priv->ips.last_time2 = ktime_get_raw_ns();
 
 	spin_unlock_irq(&mchdev_lock);
 }
 
-static void ironlake_disable_drps(struct drm_i915_private *dev_priv)
+static void ironlake_disable_drps(struct drm_i915_private *i915)
 {
+	struct intel_uncore *uncore = &i915->uncore;
 	u16 rgvswctl;
 
 	spin_lock_irq(&mchdev_lock);
 
-	rgvswctl = I915_READ16(MEMSWCTL);
+	rgvswctl = intel_uncore_read16(uncore, MEMSWCTL);
 
 	/* Ack interrupts, disable EFC interrupt */
-	I915_WRITE(MEMINTREN, I915_READ(MEMINTREN) & ~MEMINT_EVAL_CHG_EN);
-	I915_WRITE(MEMINTRSTS, MEMINT_EVAL_CHG);
-	I915_WRITE(DEIER, I915_READ(DEIER) & ~DE_PCU_EVENT);
-	I915_WRITE(DEIIR, DE_PCU_EVENT);
-	I915_WRITE(DEIMR, I915_READ(DEIMR) | DE_PCU_EVENT);
+	intel_uncore_write(uncore,
+			   MEMINTREN,
+			   intel_uncore_read(uncore, MEMINTREN) &
+			   ~MEMINT_EVAL_CHG_EN);
+	intel_uncore_write(uncore, MEMINTRSTS, MEMINT_EVAL_CHG);
+	intel_uncore_write(uncore,
+			   DEIER,
+			   intel_uncore_read(uncore, DEIER) & ~DE_PCU_EVENT);
+	intel_uncore_write(uncore, DEIIR, DE_PCU_EVENT);
+	intel_uncore_write(uncore,
+			   DEIMR,
+			   intel_uncore_read(uncore, DEIMR) | DE_PCU_EVENT);
 
 	/* Go back to the starting frequency */
-	ironlake_set_drps(dev_priv, dev_priv->ips.fstart);
+	ironlake_set_drps(i915, i915->ips.fstart);
 	mdelay(1);
 	rgvswctl |= MEMCTL_CMD_STS;
-	I915_WRITE(MEMSWCTL, rgvswctl);
+	intel_uncore_write(uncore, MEMSWCTL, rgvswctl);
 	mdelay(1);
 
 	spin_unlock_irq(&mchdev_lock);
@@ -9504,16 +9518,21 @@ static void g4x_init_clock_gating(struct drm_i915_private *dev_priv)
 
 static void i965gm_init_clock_gating(struct drm_i915_private *dev_priv)
 {
-	I915_WRITE(RENCLK_GATE_D1, I965_RCC_CLOCK_GATE_DISABLE);
-	I915_WRITE(RENCLK_GATE_D2, 0);
-	I915_WRITE(DSPCLK_GATE_D, 0);
-	I915_WRITE(RAMCLK_GATE_D, 0);
-	I915_WRITE16(DEUC, 0);
-	I915_WRITE(MI_ARB_STATE,
-		   _MASKED_BIT_ENABLE(MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE));
+	struct intel_uncore *uncore = &dev_priv->uncore;
+
+	intel_uncore_write(uncore, RENCLK_GATE_D1, I965_RCC_CLOCK_GATE_DISABLE);
+	intel_uncore_write(uncore, RENCLK_GATE_D2, 0);
+	intel_uncore_write(uncore, DSPCLK_GATE_D, 0);
+	intel_uncore_write(uncore, RAMCLK_GATE_D, 0);
+	intel_uncore_write16(uncore, DEUC, 0);
+	intel_uncore_write(uncore,
+			   MI_ARB_STATE,
+			   _MASKED_BIT_ENABLE(MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE));
 
 	/* WaDisable_RenderCache_OperationalFlush:gen4 */
-	I915_WRITE(CACHE_MODE_0, _MASKED_BIT_DISABLE(RC_OP_FLUSH_ENABLE));
+	intel_uncore_write(uncore,
+			   CACHE_MODE_0,
+			   _MASKED_BIT_DISABLE(RC_OP_FLUSH_ENABLE));
 }
 
 static void i965g_init_clock_gating(struct drm_i915_private *dev_priv)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RFC 06/12] drm/i915: Remove I915_READ64 and I915_READ64_32x2
  2019-06-07 12:08 ` [RFC 06/12] drm/i915: Remove I915_READ64 and I915_READ64_32x2 Tvrtko Ursulin
@ 2019-06-07 12:44   ` Jani Nikula
  0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2019-06-07 12:44 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Now that all their users are gone we can remove the macros and
> accompanying duplicated comment.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Suggested-by: Jani Nikula <jani.nikula@intel.com>

On patches 1-6,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 18 ------------------
>  1 file changed, 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8da1541546ee..b2763721b76d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2862,24 +2862,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>  #define I915_READ_NOTRACE(reg__)	 __I915_REG_OP(read_notrace, dev_priv, (reg__))
>  #define I915_WRITE_NOTRACE(reg__, val__) __I915_REG_OP(write_notrace, dev_priv, (reg__), (val__))
>  
> -/* Be very careful with read/write 64-bit values. On 32-bit machines, they
> - * will be implemented using 2 32-bit writes in an arbitrary order with
> - * an arbitrary delay between them. This can cause the hardware to
> - * act upon the intermediate value, possibly leading to corruption and
> - * machine death. For this reason we do not support I915_WRITE64, or
> - * dev_priv->uncore.funcs.mmio_writeq.
> - *
> - * When reading a 64-bit value as two 32-bit values, the delay may cause
> - * the two reads to mismatch, e.g. a timestamp overflowing. Also note that
> - * occasionally a 64-bit register does not actualy support a full readq
> - * and must be read using two 32-bit reads.
> - *
> - * You have been warned.
> - */
> -#define I915_READ64(reg__)	__I915_REG_OP(read64, dev_priv, (reg__))
> -#define I915_READ64_2x32(lower_reg__, upper_reg__) \
> -	__I915_REG_OP(read64_2x32, dev_priv, (lower_reg__), (upper_reg__))
> -
>  #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
>  #define POSTING_READ16(reg__)	__I915_REG_OP(posting_read16, dev_priv, (reg__))

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 08/12] drm/i915: Remove I915_POSTING_READ_FW
  2019-06-07 12:08 ` [RFC 08/12] drm/i915: Remove I915_POSTING_READ_FW Tvrtko Ursulin
@ 2019-06-07 12:48   ` Jani Nikula
  0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2019-06-07 12:48 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Only a few call sites remain which have been converted to uncore mmio
> accessors and so the macro can be removed.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h             |  1 -
>  drivers/gpu/drm/i915/i915_gem.c             |  9 +++---
>  drivers/gpu/drm/i915/i915_irq.c             |  2 +-
>  drivers/gpu/drm/i915/intel_guc_submission.c |  4 +--
>  drivers/gpu/drm/i915/intel_pm.c             | 31 +++++++++++----------
>  5 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 13815795e197..9dcec93426de 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2891,7 +2891,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>   */
>  #define I915_READ_FW(reg__) __I915_REG_OP(read_fw, dev_priv, (reg__))
>  #define I915_WRITE_FW(reg__, val__) __I915_REG_OP(write_fw, dev_priv, (reg__), (val__))
> -#define POSTING_READ_FW(reg__) __I915_REG_OP(posting_read_fw, dev_priv, (reg__))
>  
>  /* "Broadcast RGB" property */
>  #define INTEL_BROADCAST_RGB_AUTO 0
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9f2e213c6046..94d85b0fb860 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -263,11 +263,12 @@ void i915_gem_flush_ggtt_writes(struct drm_i915_private *dev_priv)
>  	i915_gem_chipset_flush(dev_priv);
>  
>  	with_intel_runtime_pm(dev_priv, wakeref) {
> -		spin_lock_irq(&dev_priv->uncore.lock);
> +		struct intel_uncore *uncore = &dev_priv->uncore;
>  
> -		POSTING_READ_FW(RING_HEAD(RENDER_RING_BASE));
> -
> -		spin_unlock_irq(&dev_priv->uncore.lock);
> +		spin_lock_irq(&uncore->lock);
> +		intel_uncore_posting_read_fw(uncore,
> +					     RING_HEAD(RENDER_RING_BASE));
> +		spin_unlock_irq(&uncore->lock);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ca8f4226e598..0763ffffea53 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -386,7 +386,7 @@ static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
>  void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, u32 mask)
>  {
>  	ilk_update_gt_irq(dev_priv, mask, mask);
> -	POSTING_READ_FW(GTIMR);
> +	intel_uncore_posting_read_fw(&dev_priv->uncore, GTIMR);
>  }
>  
>  void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, u32 mask)
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 89592ef778b8..97f6970d8da8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -557,10 +557,10 @@ static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
>   */
>  static void flush_ggtt_writes(struct i915_vma *vma)
>  {
> -	struct drm_i915_private *dev_priv = vma->vm->i915;
> +	struct drm_i915_private *i915 = vma->vm->i915;
>  
>  	if (i915_vma_is_map_and_fenceable(vma))
> -		POSTING_READ_FW(GUC_STATUS);
> +		intel_uncore_posting_read_fw(&i915->uncore, GUC_STATUS);
>  }
>  
>  static void inject_preempt_context(struct work_struct *work)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 93e411e6ad19..84588ff8732f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1949,6 +1949,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	const struct vlv_fifo_state *fifo_state =
>  		&crtc_state->wm.vlv.fifo_state;
>  	int sprite0_start, sprite1_start, fifo_size;
> @@ -1974,13 +1975,13 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  	 * intel_pipe_update_start() has already disabled interrupts
>  	 * for us, so a plain spin_lock() is sufficient here.
>  	 */
> -	spin_lock(&dev_priv->uncore.lock);
> +	spin_lock(&uncore->lock);
>  
>  	switch (crtc->pipe) {
>  		u32 dsparb, dsparb2, dsparb3;
>  	case PIPE_A:
> -		dsparb = I915_READ_FW(DSPARB);
> -		dsparb2 = I915_READ_FW(DSPARB2);
> +		dsparb = intel_uncore_read_fw(uncore, DSPARB);
> +		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
>  
>  		dsparb &= ~(VLV_FIFO(SPRITEA, 0xff) |
>  			    VLV_FIFO(SPRITEB, 0xff));
> @@ -1992,12 +1993,12 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  		dsparb2 |= (VLV_FIFO(SPRITEA_HI, sprite0_start >> 8) |
>  			   VLV_FIFO(SPRITEB_HI, sprite1_start >> 8));
>  
> -		I915_WRITE_FW(DSPARB, dsparb);
> -		I915_WRITE_FW(DSPARB2, dsparb2);
> +		intel_uncore_write_fw(uncore, DSPARB, dsparb);
> +		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
>  		break;
>  	case PIPE_B:
> -		dsparb = I915_READ_FW(DSPARB);
> -		dsparb2 = I915_READ_FW(DSPARB2);
> +		dsparb = intel_uncore_read_fw(uncore, DSPARB);
> +		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
>  
>  		dsparb &= ~(VLV_FIFO(SPRITEC, 0xff) |
>  			    VLV_FIFO(SPRITED, 0xff));
> @@ -2009,12 +2010,12 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  		dsparb2 |= (VLV_FIFO(SPRITEC_HI, sprite0_start >> 8) |
>  			   VLV_FIFO(SPRITED_HI, sprite1_start >> 8));
>  
> -		I915_WRITE_FW(DSPARB, dsparb);
> -		I915_WRITE_FW(DSPARB2, dsparb2);
> +		intel_uncore_write_fw(uncore, DSPARB, dsparb);
> +		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
>  		break;
>  	case PIPE_C:
> -		dsparb3 = I915_READ_FW(DSPARB3);
> -		dsparb2 = I915_READ_FW(DSPARB2);
> +		dsparb3 = intel_uncore_read_fw(uncore, DSPARB3);
> +		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
>  
>  		dsparb3 &= ~(VLV_FIFO(SPRITEE, 0xff) |
>  			     VLV_FIFO(SPRITEF, 0xff));
> @@ -2026,16 +2027,16 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  		dsparb2 |= (VLV_FIFO(SPRITEE_HI, sprite0_start >> 8) |
>  			   VLV_FIFO(SPRITEF_HI, sprite1_start >> 8));
>  
> -		I915_WRITE_FW(DSPARB3, dsparb3);
> -		I915_WRITE_FW(DSPARB2, dsparb2);
> +		intel_uncore_write_fw(uncore, DSPARB3, dsparb3);
> +		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
>  		break;
>  	default:
>  		break;
>  	}
>  
> -	POSTING_READ_FW(DSPARB);
> +	intel_uncore_posting_read_fw(uncore, DSPARB);
>  
> -	spin_unlock(&dev_priv->uncore.lock);
> +	spin_unlock(&uncore->lock);
>  }
>  
>  #undef VLV_FIFO

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 09/12] drm/i915: Remove POSTING_READ16
  2019-06-07 12:08 ` [RFC 09/12] drm/i915: Remove POSTING_READ16 Tvrtko Ursulin
@ 2019-06-07 12:49   ` Jani Nikula
  0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2019-06-07 12:49 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Only a few call sites remain which have been converted to uncore mmio
> accessors and so the macro can be removed.
>
> ENGINE_POSTING_READ16 is added to replace one engine->mmio_base relative
> call site.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_engine.h     |  1 +
>  drivers/gpu/drm/i915/gt/intel_ringbuffer.c |  8 ++++----
>  drivers/gpu/drm/i915/i915_drv.h            |  1 -
>  drivers/gpu/drm/i915/intel_pm.c            | 11 ++++++-----
>  4 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 201bbd2a4faf..1439fa4093ac 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -52,6 +52,7 @@ struct drm_printer;
>  #define ENGINE_READ(...)	__ENGINE_READ_OP(read, __VA_ARGS__)
>  #define ENGINE_READ_FW(...)	__ENGINE_READ_OP(read_fw, __VA_ARGS__)
>  #define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read, __VA_ARGS__)
> +#define ENGINE_POSTING_READ16(...) __ENGINE_READ_OP(posting_read16, __VA_ARGS__)
>  
>  #define ENGINE_READ64(engine__, lower_reg__, upper_reg__) \
>  	__ENGINE_REG_OP(read64_2x32, (engine__), \
> diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> index ff58d658e3e2..0373af648e72 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> @@ -976,11 +976,11 @@ i9xx_irq_disable(struct intel_engine_cs *engine)
>  static void
>  i8xx_irq_enable(struct intel_engine_cs *engine)
>  {
> -	struct drm_i915_private *dev_priv = engine->i915;
> +	struct drm_i915_private *i915 = engine->i915;
>  
> -	dev_priv->irq_mask &= ~engine->irq_enable_mask;
> -	I915_WRITE16(GEN2_IMR, dev_priv->irq_mask);
> -	POSTING_READ16(RING_IMR(engine->mmio_base));
> +	i915->irq_mask &= ~engine->irq_enable_mask;
> +	intel_uncore_write16(&i915->uncore, GEN2_IMR, i915->irq_mask);
> +	ENGINE_POSTING_READ16(engine, RING_IMR);
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9dcec93426de..2fcf6eb8ab3f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2861,7 +2861,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>  #define I915_WRITE_NOTRACE(reg__, val__) __I915_REG_OP(write_notrace, dev_priv, (reg__), (val__))
>  
>  #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
> -#define POSTING_READ16(reg__)	__I915_REG_OP(posting_read16, dev_priv, (reg__))
>  
>  /* These are untraced mmio-accessors that are only valid to be used inside
>   * critical sections, such as inside IRQ handlers, where forcewake is explicitly
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 84588ff8732f..5a6679e2b6ee 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6406,13 +6406,14 @@ void intel_init_ipc(struct drm_i915_private *dev_priv)
>   */
>  DEFINE_SPINLOCK(mchdev_lock);
>  
> -bool ironlake_set_drps(struct drm_i915_private *dev_priv, u8 val)
> +bool ironlake_set_drps(struct drm_i915_private *i915, u8 val)
>  {
> +	struct intel_uncore *uncore = &i915->uncore;
>  	u16 rgvswctl;
>  
>  	lockdep_assert_held(&mchdev_lock);
>  
> -	rgvswctl = I915_READ16(MEMSWCTL);
> +	rgvswctl = intel_uncore_read16(uncore, MEMSWCTL);
>  	if (rgvswctl & MEMCTL_CMD_STS) {
>  		DRM_DEBUG("gpu busy, RCS change rejected\n");
>  		return false; /* still busy with another command */
> @@ -6420,11 +6421,11 @@ bool ironlake_set_drps(struct drm_i915_private *dev_priv, u8 val)
>  
>  	rgvswctl = (MEMCTL_CMD_CHFREQ << MEMCTL_CMD_SHIFT) |
>  		(val << MEMCTL_FREQ_SHIFT) | MEMCTL_SFCAVM;
> -	I915_WRITE16(MEMSWCTL, rgvswctl);
> -	POSTING_READ16(MEMSWCTL);
> +	intel_uncore_write16(uncore, MEMSWCTL, rgvswctl);
> +	intel_uncore_posting_read16(uncore, MEMSWCTL);
>  
>  	rgvswctl |= MEMCTL_CMD_STS;
> -	I915_WRITE16(MEMSWCTL, rgvswctl);
> +	intel_uncore_write16(uncore, MEMSWCTL, rgvswctl);
>  
>  	return true;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 10/12] drm/i915: Remove I915_WRITE_NOTRACE
  2019-06-07 12:08 ` [RFC 10/12] drm/i915: Remove I915_WRITE_NOTRACE Tvrtko Ursulin
@ 2019-06-07 12:50   ` Jani Nikula
  0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2019-06-07 12:50 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Only a few call sites remain which have been converted to uncore mmio
> accessors and so the macro can be removed.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h    |  1 -
>  drivers/gpu/drm/i915/intel_gmbus.c | 42 +++++++++++++++++++-----------
>  2 files changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2fcf6eb8ab3f..1ea2d5f42e52 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2858,7 +2858,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>  #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
>  #define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
>  #define I915_READ_NOTRACE(reg__)	 __I915_REG_OP(read_notrace, dev_priv, (reg__))
> -#define I915_WRITE_NOTRACE(reg__, val__) __I915_REG_OP(write_notrace, dev_priv, (reg__), (val__))
>  
>  #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
>  
> diff --git a/drivers/gpu/drm/i915/intel_gmbus.c b/drivers/gpu/drm/i915/intel_gmbus.c
> index 969ce8b71e32..5e4c543e82e8 100644
> --- a/drivers/gpu/drm/i915/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/intel_gmbus.c
> @@ -201,27 +201,37 @@ static u32 get_reserved(struct intel_gmbus *bus)
>  static int get_clock(void *data)
>  {
>  	struct intel_gmbus *bus = data;
> -	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	struct intel_uncore *uncore = &bus->dev_priv->uncore;
>  	u32 reserved = get_reserved(bus);
> -	I915_WRITE_NOTRACE(bus->gpio_reg, reserved | GPIO_CLOCK_DIR_MASK);
> -	I915_WRITE_NOTRACE(bus->gpio_reg, reserved);
> -	return (I915_READ_NOTRACE(bus->gpio_reg) & GPIO_CLOCK_VAL_IN) != 0;
> +
> +	intel_uncore_write_notrace(uncore,
> +				   bus->gpio_reg,
> +				   reserved | GPIO_CLOCK_DIR_MASK);
> +	intel_uncore_write_notrace(uncore, bus->gpio_reg, reserved);
> +
> +	return (intel_uncore_read_notrace(uncore, bus->gpio_reg) &
> +		GPIO_CLOCK_VAL_IN) != 0;
>  }
>  
>  static int get_data(void *data)
>  {
>  	struct intel_gmbus *bus = data;
> -	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	struct intel_uncore *uncore = &bus->dev_priv->uncore;
>  	u32 reserved = get_reserved(bus);
> -	I915_WRITE_NOTRACE(bus->gpio_reg, reserved | GPIO_DATA_DIR_MASK);
> -	I915_WRITE_NOTRACE(bus->gpio_reg, reserved);
> -	return (I915_READ_NOTRACE(bus->gpio_reg) & GPIO_DATA_VAL_IN) != 0;
> +
> +	intel_uncore_write_notrace(uncore,
> +				   bus->gpio_reg,
> +				   reserved | GPIO_DATA_DIR_MASK);
> +	intel_uncore_write_notrace(uncore, bus->gpio_reg, reserved);
> +
> +	return (intel_uncore_read_notrace(uncore, bus->gpio_reg) &
> +		GPIO_DATA_VAL_IN) != 0;
>  }
>  
>  static void set_clock(void *data, int state_high)
>  {
>  	struct intel_gmbus *bus = data;
> -	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	struct intel_uncore *uncore = &bus->dev_priv->uncore;
>  	u32 reserved = get_reserved(bus);
>  	u32 clock_bits;
>  
> @@ -229,16 +239,18 @@ static void set_clock(void *data, int state_high)
>  		clock_bits = GPIO_CLOCK_DIR_IN | GPIO_CLOCK_DIR_MASK;
>  	else
>  		clock_bits = GPIO_CLOCK_DIR_OUT | GPIO_CLOCK_DIR_MASK |
> -			GPIO_CLOCK_VAL_MASK;
> +			     GPIO_CLOCK_VAL_MASK;
>  
> -	I915_WRITE_NOTRACE(bus->gpio_reg, reserved | clock_bits);
> -	POSTING_READ(bus->gpio_reg);
> +	intel_uncore_write_notrace(uncore,
> +				   bus->gpio_reg,
> +				   reserved | clock_bits);
> +	intel_uncore_posting_read(uncore, bus->gpio_reg);
>  }
>  
>  static void set_data(void *data, int state_high)
>  {
>  	struct intel_gmbus *bus = data;
> -	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	struct intel_uncore *uncore = &bus->dev_priv->uncore;
>  	u32 reserved = get_reserved(bus);
>  	u32 data_bits;
>  
> @@ -248,8 +260,8 @@ static void set_data(void *data, int state_high)
>  		data_bits = GPIO_DATA_DIR_OUT | GPIO_DATA_DIR_MASK |
>  			GPIO_DATA_VAL_MASK;
>  
> -	I915_WRITE_NOTRACE(bus->gpio_reg, reserved | data_bits);
> -	POSTING_READ(bus->gpio_reg);
> +	intel_uncore_write_notrace(uncore, bus->gpio_reg, reserved | data_bits);
> +	intel_uncore_posting_read(uncore, bus->gpio_reg);
>  }
>  
>  static int

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 12/12] drm/i915: Remove I915_READ16 and I915_WRITE16
  2019-06-07 12:08 ` [RFC 12/12] drm/i915: Remove I915_READ16 and I915_WRITE16 Tvrtko Ursulin
@ 2019-06-07 12:59   ` Jani Nikula
  0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2019-06-07 12:59 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Remove call sites in favour of uncore mmio accessors and remove the old
> macros.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This patch really starts to show the ugly side of switching to
intel_uncore_* functions, with one-liners turning into three.

Yet, I think this is fine for the special case variants READ16 and
WRITE16.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/gt/intel_ringbuffer.c |   6 +-
>  drivers/gpu/drm/i915/i915_debugfs.c        |  37 ++++----
>  drivers/gpu/drm/i915/i915_drv.h            |   3 -
>  drivers/gpu/drm/i915/i915_gpu_error.c      | 104 ++++++++++++---------
>  drivers/gpu/drm/i915/i915_irq.c            |  40 ++++----
>  drivers/gpu/drm/i915/intel_pm.c            |  93 ++++++++++--------
>  6 files changed, 161 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> index 0373af648e72..be0ba0b742f7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> @@ -986,10 +986,10 @@ i8xx_irq_enable(struct intel_engine_cs *engine)
>  static void
>  i8xx_irq_disable(struct intel_engine_cs *engine)
>  {
> -	struct drm_i915_private *dev_priv = engine->i915;
> +	struct drm_i915_private *i915 = engine->i915;
>  
> -	dev_priv->irq_mask |= engine->irq_enable_mask;
> -	I915_WRITE16(GEN2_IMR, dev_priv->irq_mask);
> +	i915->irq_mask |= engine->irq_enable_mask;
> +	intel_uncore_write16(&i915->uncore, GEN2_IMR, i915->irq_mask);
>  }
>  
>  static int
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f212241a2758..a11e06c21165 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -998,6 +998,7 @@ static const struct file_operations i915_error_state_fops = {
>  static int i915_frequency_info(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	struct intel_rps *rps = &dev_priv->gt_pm.rps;
>  	intel_wakeref_t wakeref;
>  	int ret = 0;
> @@ -1005,8 +1006,8 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>  	wakeref = intel_runtime_pm_get(dev_priv);
>  
>  	if (IS_GEN(dev_priv, 5)) {
> -		u16 rgvswctl = I915_READ16(MEMSWCTL);
> -		u16 rgvstat = I915_READ16(MEMSTAT_ILK);
> +		u16 rgvswctl = intel_uncore_read16(uncore, MEMSWCTL);
> +		u16 rgvstat = intel_uncore_read16(uncore, MEMSTAT_ILK);
>  
>  		seq_printf(m, "Requested P-state: %d\n", (rgvswctl >> 8) & 0xf);
>  		seq_printf(m, "Requested VID: %d\n", rgvswctl & 0x3f);
> @@ -1328,13 +1329,14 @@ static int i915_reset_info(struct seq_file *m, void *unused)
>  
>  static int ironlake_drpc_info(struct seq_file *m)
>  {
> -	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> +	struct drm_i915_private *i915 = node_to_i915(m->private);
> +	struct intel_uncore *uncore = &i915->uncore;
>  	u32 rgvmodectl, rstdbyctl;
>  	u16 crstandvid;
>  
> -	rgvmodectl = I915_READ(MEMMODECTL);
> -	rstdbyctl = I915_READ(RSTDBYCTL);
> -	crstandvid = I915_READ16(CRSTANDVID);
> +	rgvmodectl = intel_uncore_read(uncore, MEMMODECTL);
> +	rstdbyctl = intel_uncore_read(uncore, RSTDBYCTL);
> +	crstandvid = intel_uncore_read16(uncore, CRSTANDVID);
>  
>  	seq_printf(m, "HD boost: %s\n", yesno(rgvmodectl & MEMMODE_BOOST_EN));
>  	seq_printf(m, "Boost freq: %d\n",
> @@ -1917,6 +1919,7 @@ static const char *swizzle_string(unsigned swizzle)
>  static int i915_swizzle_info(struct seq_file *m, void *data)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	intel_wakeref_t wakeref;
>  
>  	wakeref = intel_runtime_pm_get(dev_priv);
> @@ -1928,30 +1931,30 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
>  
>  	if (IS_GEN_RANGE(dev_priv, 3, 4)) {
>  		seq_printf(m, "DDC = 0x%08x\n",
> -			   I915_READ(DCC));
> +			   intel_uncore_read(uncore, DCC));
>  		seq_printf(m, "DDC2 = 0x%08x\n",
> -			   I915_READ(DCC2));
> +			   intel_uncore_read(uncore, DCC2));
>  		seq_printf(m, "C0DRB3 = 0x%04x\n",
> -			   I915_READ16(C0DRB3));
> +			   intel_uncore_read16(uncore, C0DRB3));
>  		seq_printf(m, "C1DRB3 = 0x%04x\n",
> -			   I915_READ16(C1DRB3));
> +			   intel_uncore_read16(uncore, C1DRB3));
>  	} else if (INTEL_GEN(dev_priv) >= 6) {
>  		seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n",
> -			   I915_READ(MAD_DIMM_C0));
> +			   intel_uncore_read(uncore, MAD_DIMM_C0));
>  		seq_printf(m, "MAD_DIMM_C1 = 0x%08x\n",
> -			   I915_READ(MAD_DIMM_C1));
> +			   intel_uncore_read(uncore, MAD_DIMM_C1));
>  		seq_printf(m, "MAD_DIMM_C2 = 0x%08x\n",
> -			   I915_READ(MAD_DIMM_C2));
> +			   intel_uncore_read(uncore, MAD_DIMM_C2));
>  		seq_printf(m, "TILECTL = 0x%08x\n",
> -			   I915_READ(TILECTL));
> +			   intel_uncore_read(uncore, TILECTL));
>  		if (INTEL_GEN(dev_priv) >= 8)
>  			seq_printf(m, "GAMTARBMODE = 0x%08x\n",
> -				   I915_READ(GAMTARBMODE));
> +				   intel_uncore_read(uncore, GAMTARBMODE));
>  		else
>  			seq_printf(m, "ARB_MODE = 0x%08x\n",
> -				   I915_READ(ARB_MODE));
> +				   intel_uncore_read(uncore, ARB_MODE));
>  		seq_printf(m, "DISP_ARB_CTL = 0x%08x\n",
> -			   I915_READ(DISP_ARB_CTL));
> +			   intel_uncore_read(uncore, DISP_ARB_CTL));
>  	}
>  
>  	if (dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9f0208e410ed..261b65caaf80 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2852,9 +2852,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>  #define __I915_REG_OP(op__, dev_priv__, ...) \
>  	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
>  
> -#define I915_READ16(reg__)	   __I915_REG_OP(read16, dev_priv, (reg__))
> -#define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
> -
>  #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
>  #define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
>  
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index a523bf050a25..9caf91d98b27 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1576,7 +1576,8 @@ static void capture_uc_state(struct i915_gpu_state *error)
>  /* Capture all registers which don't fit into another category. */
>  static void capture_reg_state(struct i915_gpu_state *error)
>  {
> -	struct drm_i915_private *dev_priv = error->i915;
> +	struct drm_i915_private *i915 = error->i915;
> +	struct intel_uncore *uncore = &i915->uncore;
>  	int i;
>  
>  	/* General organization
> @@ -1588,71 +1589,84 @@ static void capture_reg_state(struct i915_gpu_state *error)
>  	 */
>  
>  	/* 1: Registers specific to a single generation */
> -	if (IS_VALLEYVIEW(dev_priv)) {
> -		error->gtier[0] = I915_READ(GTIER);
> -		error->ier = I915_READ(VLV_IER);
> -		error->forcewake = I915_READ_FW(FORCEWAKE_VLV);
> +	if (IS_VALLEYVIEW(i915)) {
> +		error->gtier[0] = intel_uncore_read(uncore, GTIER);
> +		error->ier = intel_uncore_read(uncore, VLV_IER);
> +		error->forcewake = intel_uncore_read_fw(uncore, FORCEWAKE_VLV);
>  	}
>  
> -	if (IS_GEN(dev_priv, 7))
> -		error->err_int = I915_READ(GEN7_ERR_INT);
> +	if (IS_GEN(i915, 7))
> +		error->err_int = intel_uncore_read(uncore, GEN7_ERR_INT);
>  
> -	if (INTEL_GEN(dev_priv) >= 8) {
> -		error->fault_data0 = I915_READ(GEN8_FAULT_TLB_DATA0);
> -		error->fault_data1 = I915_READ(GEN8_FAULT_TLB_DATA1);
> +	if (INTEL_GEN(i915) >= 8) {
> +		error->fault_data0 = intel_uncore_read(uncore,
> +						       GEN8_FAULT_TLB_DATA0);
> +		error->fault_data1 = intel_uncore_read(uncore,
> +						       GEN8_FAULT_TLB_DATA1);
>  	}
>  
> -	if (IS_GEN(dev_priv, 6)) {
> -		error->forcewake = I915_READ_FW(FORCEWAKE);
> -		error->gab_ctl = I915_READ(GAB_CTL);
> -		error->gfx_mode = I915_READ(GFX_MODE);
> +	if (IS_GEN(i915, 6)) {
> +		error->forcewake = intel_uncore_read_fw(uncore, FORCEWAKE);
> +		error->gab_ctl = intel_uncore_read(uncore, GAB_CTL);
> +		error->gfx_mode = intel_uncore_read(uncore, GFX_MODE);
>  	}
>  
>  	/* 2: Registers which belong to multiple generations */
> -	if (INTEL_GEN(dev_priv) >= 7)
> -		error->forcewake = I915_READ_FW(FORCEWAKE_MT);
> +	if (INTEL_GEN(i915) >= 7)
> +		error->forcewake = intel_uncore_read_fw(uncore, FORCEWAKE_MT);
>  
> -	if (INTEL_GEN(dev_priv) >= 6) {
> -		error->derrmr = I915_READ(DERRMR);
> -		error->error = I915_READ(ERROR_GEN6);
> -		error->done_reg = I915_READ(DONE_REG);
> +	if (INTEL_GEN(i915) >= 6) {
> +		error->derrmr = intel_uncore_read(uncore, DERRMR);
> +		error->error = intel_uncore_read(uncore, ERROR_GEN6);
> +		error->done_reg = intel_uncore_read(uncore, DONE_REG);
>  	}
>  
> -	if (INTEL_GEN(dev_priv) >= 5)
> -		error->ccid = I915_READ(CCID(RENDER_RING_BASE));
> +	if (INTEL_GEN(i915) >= 5)
> +		error->ccid = intel_uncore_read(uncore, CCID(RENDER_RING_BASE));
>  
>  	/* 3: Feature specific registers */
> -	if (IS_GEN_RANGE(dev_priv, 6, 7)) {
> -		error->gam_ecochk = I915_READ(GAM_ECOCHK);
> -		error->gac_eco = I915_READ(GAC_ECO_BITS);
> +	if (IS_GEN_RANGE(i915, 6, 7)) {
> +		error->gam_ecochk = intel_uncore_read(uncore, GAM_ECOCHK);
> +		error->gac_eco = intel_uncore_read(uncore, GAC_ECO_BITS);
>  	}
>  
>  	/* 4: Everything else */
> -	if (INTEL_GEN(dev_priv) >= 11) {
> -		error->ier = I915_READ(GEN8_DE_MISC_IER);
> -		error->gtier[0] = I915_READ(GEN11_RENDER_COPY_INTR_ENABLE);
> -		error->gtier[1] = I915_READ(GEN11_VCS_VECS_INTR_ENABLE);
> -		error->gtier[2] = I915_READ(GEN11_GUC_SG_INTR_ENABLE);
> -		error->gtier[3] = I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE);
> -		error->gtier[4] = I915_READ(GEN11_CRYPTO_RSVD_INTR_ENABLE);
> -		error->gtier[5] = I915_READ(GEN11_GUNIT_CSME_INTR_ENABLE);
> +	if (INTEL_GEN(i915) >= 11) {
> +		error->ier = intel_uncore_read(uncore, GEN8_DE_MISC_IER);
> +		error->gtier[0] =
> +			intel_uncore_read(uncore,
> +					  GEN11_RENDER_COPY_INTR_ENABLE);
> +		error->gtier[1] =
> +			intel_uncore_read(uncore, GEN11_VCS_VECS_INTR_ENABLE);
> +		error->gtier[2] =
> +			intel_uncore_read(uncore, GEN11_GUC_SG_INTR_ENABLE);
> +		error->gtier[3] =
> +			intel_uncore_read(uncore,
> +					  GEN11_GPM_WGBOXPERF_INTR_ENABLE);
> +		error->gtier[4] =
> +			intel_uncore_read(uncore,
> +					  GEN11_CRYPTO_RSVD_INTR_ENABLE);
> +		error->gtier[5] =
> +			intel_uncore_read(uncore,
> +					  GEN11_GUNIT_CSME_INTR_ENABLE);
>  		error->ngtier = 6;
> -	} else if (INTEL_GEN(dev_priv) >= 8) {
> -		error->ier = I915_READ(GEN8_DE_MISC_IER);
> +	} else if (INTEL_GEN(i915) >= 8) {
> +		error->ier = intel_uncore_read(uncore, GEN8_DE_MISC_IER);
>  		for (i = 0; i < 4; i++)
> -			error->gtier[i] = I915_READ(GEN8_GT_IER(i));
> +			error->gtier[i] = intel_uncore_read(uncore,
> +							    GEN8_GT_IER(i));
>  		error->ngtier = 4;
> -	} else if (HAS_PCH_SPLIT(dev_priv)) {
> -		error->ier = I915_READ(DEIER);
> -		error->gtier[0] = I915_READ(GTIER);
> +	} else if (HAS_PCH_SPLIT(i915)) {
> +		error->ier = intel_uncore_read(uncore, DEIER);
> +		error->gtier[0] = intel_uncore_read(uncore, GTIER);
>  		error->ngtier = 1;
> -	} else if (IS_GEN(dev_priv, 2)) {
> -		error->ier = I915_READ16(GEN2_IER);
> -	} else if (!IS_VALLEYVIEW(dev_priv)) {
> -		error->ier = I915_READ(GEN2_IER);
> +	} else if (IS_GEN(i915, 2)) {
> +		error->ier = intel_uncore_read16(uncore, GEN2_IER);
> +	} else if (!IS_VALLEYVIEW(i915)) {
> +		error->ier = intel_uncore_read(uncore, GEN2_IER);
>  	}
> -	error->eir = I915_READ(EIR);
> -	error->pgtbl_er = I915_READ(PGTBL_ER);
> +	error->eir = intel_uncore_read(uncore, EIR);
> +	error->pgtbl_er = intel_uncore_read(uncore, PGTBL_ER);
>  }
>  
>  static const char *
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 0763ffffea53..9680f1376ac5 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1231,20 +1231,23 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
>  
>  static void ironlake_rps_change_irq_handler(struct drm_i915_private *dev_priv)
>  {
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	u32 busy_up, busy_down, max_avg, min_avg;
>  	u8 new_delay;
>  
>  	spin_lock(&mchdev_lock);
>  
> -	I915_WRITE16(MEMINTRSTS, I915_READ(MEMINTRSTS));
> +	intel_uncore_write16(uncore,
> +			     MEMINTRSTS,
> +			     intel_uncore_read(uncore, MEMINTRSTS));
>  
>  	new_delay = dev_priv->ips.cur_delay;
>  
> -	I915_WRITE16(MEMINTRSTS, MEMINT_EVAL_CHG);
> -	busy_up = I915_READ(RCPREVBSYTUPAVG);
> -	busy_down = I915_READ(RCPREVBSYTDNAVG);
> -	max_avg = I915_READ(RCBMAXAVG);
> -	min_avg = I915_READ(RCBMINAVG);
> +	intel_uncore_write16(uncore, MEMINTRSTS, MEMINT_EVAL_CHG);
> +	busy_up = intel_uncore_read(uncore, RCPREVBSYTUPAVG);
> +	busy_down = intel_uncore_read(uncore, RCPREVBSYTDNAVG);
> +	max_avg = intel_uncore_read(uncore, RCBMAXAVG);
> +	min_avg = intel_uncore_read(uncore, RCBMINAVG);
>  
>  	/* Handle RCS change request from hw */
>  	if (busy_up > max_avg) {
> @@ -4323,8 +4326,10 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
>  	struct intel_uncore *uncore = &dev_priv->uncore;
>  	u16 enable_mask;
>  
> -	I915_WRITE16(EMR, ~(I915_ERROR_PAGE_TABLE |
> -			    I915_ERROR_MEMORY_REFRESH));
> +	intel_uncore_write16(uncore,
> +			     EMR,
> +			     ~(I915_ERROR_PAGE_TABLE |
> +			       I915_ERROR_MEMORY_REFRESH));
>  
>  	/* Unmask the interrupts that we always want on. */
>  	dev_priv->irq_mask =
> @@ -4350,17 +4355,18 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static void i8xx_error_irq_ack(struct drm_i915_private *dev_priv,
> +static void i8xx_error_irq_ack(struct drm_i915_private *i915,
>  			       u16 *eir, u16 *eir_stuck)
>  {
> +	struct intel_uncore *uncore = &i915->uncore;
>  	u16 emr;
>  
> -	*eir = I915_READ16(EIR);
> +	*eir = intel_uncore_read16(uncore, EIR);
>  
>  	if (*eir)
> -		I915_WRITE16(EIR, *eir);
> +		intel_uncore_write16(uncore, EIR, *eir);
>  
> -	*eir_stuck = I915_READ16(EIR);
> +	*eir_stuck = intel_uncore_read16(uncore, EIR);
>  	if (*eir_stuck == 0)
>  		return;
>  
> @@ -4374,9 +4380,9 @@ static void i8xx_error_irq_ack(struct drm_i915_private *dev_priv,
>  	 * (or by a GPU reset) so we mask any bit that
>  	 * remains set.
>  	 */
> -	emr = I915_READ16(EMR);
> -	I915_WRITE16(EMR, 0xffff);
> -	I915_WRITE16(EMR, emr | *eir_stuck);
> +	emr = intel_uncore_read16(uncore, EMR);
> +	intel_uncore_write16(uncore, EMR, 0xffff);
> +	intel_uncore_write16(uncore, EMR, emr | *eir_stuck);
>  }
>  
>  static void i8xx_error_irq_handler(struct drm_i915_private *dev_priv,
> @@ -4442,7 +4448,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
>  		u16 eir = 0, eir_stuck = 0;
>  		u16 iir;
>  
> -		iir = I915_READ16(GEN2_IIR);
> +		iir = intel_uncore_read16(&dev_priv->uncore, GEN2_IIR);
>  		if (iir == 0)
>  			break;
>  
> @@ -4455,7 +4461,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
>  		if (iir & I915_MASTER_ERROR_INTERRUPT)
>  			i8xx_error_irq_ack(dev_priv, &eir, &eir_stuck);
>  
> -		I915_WRITE16(GEN2_IIR, iir);
> +		intel_uncore_write16(&dev_priv->uncore, GEN2_IIR, iir);
>  
>  		if (iir & I915_USER_INTERRUPT)
>  			intel_engine_breadcrumbs_irq(dev_priv->engine[RCS0]);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5a6679e2b6ee..2c7f3ebc0117 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -191,8 +191,8 @@ static void i915_ironlake_get_mem_freq(struct drm_i915_private *dev_priv)
>  {
>  	u16 ddrpll, csipll;
>  
> -	ddrpll = I915_READ16(DDRMPLL1);
> -	csipll = I915_READ16(CSIPLL0);
> +	ddrpll = intel_uncore_read16(&dev_priv->uncore, DDRMPLL1);
> +	csipll = intel_uncore_read16(&dev_priv->uncore, CSIPLL0);
>  
>  	switch (ddrpll & 0xff) {
>  	case 0xc:
> @@ -6432,26 +6432,27 @@ bool ironlake_set_drps(struct drm_i915_private *i915, u8 val)
>  
>  static void ironlake_enable_drps(struct drm_i915_private *dev_priv)
>  {
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	u32 rgvmodectl;
>  	u8 fmax, fmin, fstart, vstart;
>  
>  	spin_lock_irq(&mchdev_lock);
>  
> -	rgvmodectl = I915_READ(MEMMODECTL);
> +	rgvmodectl = intel_uncore_read(uncore, MEMMODECTL);
>  
>  	/* Enable temp reporting */
> -	I915_WRITE16(PMMISC, I915_READ(PMMISC) | MCPPCE_EN);
> -	I915_WRITE16(TSC1, I915_READ(TSC1) | TSE);
> +	intel_uncore_write16(uncore, PMMISC, I915_READ(PMMISC) | MCPPCE_EN);
> +	intel_uncore_write16(uncore, TSC1, I915_READ(TSC1) | TSE);
>  
>  	/* 100ms RC evaluation intervals */
> -	I915_WRITE(RCUPEI, 100000);
> -	I915_WRITE(RCDNEI, 100000);
> +	intel_uncore_write(uncore, RCUPEI, 100000);
> +	intel_uncore_write(uncore, RCDNEI, 100000);
>  
>  	/* Set max/min thresholds to 90ms and 80ms respectively */
> -	I915_WRITE(RCBMAXAVG, 90000);
> -	I915_WRITE(RCBMINAVG, 80000);
> +	intel_uncore_write(uncore, RCBMAXAVG, 90000);
> +	intel_uncore_write(uncore, RCBMINAVG, 80000);
>  
> -	I915_WRITE(MEMIHYST, 1);
> +	intel_uncore_write(uncore, MEMIHYST, 1);
>  
>  	/* Set up min, max, and cur for interrupt handling */
>  	fmax = (rgvmodectl & MEMMODE_FMAX_MASK) >> MEMMODE_FMAX_SHIFT;
> @@ -6459,8 +6460,8 @@ static void ironlake_enable_drps(struct drm_i915_private *dev_priv)
>  	fstart = (rgvmodectl & MEMMODE_FSTART_MASK) >>
>  		MEMMODE_FSTART_SHIFT;
>  
> -	vstart = (I915_READ(PXVFREQ(fstart)) & PXVFREQ_PX_MASK) >>
> -		PXVFREQ_PX_SHIFT;
> +	vstart = (intel_uncore_read(uncore, PXVFREQ(fstart)) &
> +		  PXVFREQ_PX_MASK) >> PXVFREQ_PX_SHIFT;
>  
>  	dev_priv->ips.fmax = fmax; /* IPS callback will increase this */
>  	dev_priv->ips.fstart = fstart;
> @@ -6472,53 +6473,66 @@ static void ironlake_enable_drps(struct drm_i915_private *dev_priv)
>  	DRM_DEBUG_DRIVER("fmax: %d, fmin: %d, fstart: %d\n",
>  			 fmax, fmin, fstart);
>  
> -	I915_WRITE(MEMINTREN, MEMINT_CX_SUPR_EN | MEMINT_EVAL_CHG_EN);
> +	intel_uncore_write(uncore,
> +			   MEMINTREN,
> +			   MEMINT_CX_SUPR_EN | MEMINT_EVAL_CHG_EN);
>  
>  	/*
>  	 * Interrupts will be enabled in ironlake_irq_postinstall
>  	 */
>  
> -	I915_WRITE(VIDSTART, vstart);
> -	POSTING_READ(VIDSTART);
> +	intel_uncore_write(uncore, VIDSTART, vstart);
> +	intel_uncore_posting_read(uncore, VIDSTART);
>  
>  	rgvmodectl |= MEMMODE_SWMODE_EN;
> -	I915_WRITE(MEMMODECTL, rgvmodectl);
> +	intel_uncore_write(uncore, MEMMODECTL, rgvmodectl);
>  
> -	if (wait_for_atomic((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10))
> +	if (wait_for_atomic((intel_uncore_read(uncore, MEMSWCTL) &
> +			     MEMCTL_CMD_STS) == 0, 10))
>  		DRM_ERROR("stuck trying to change perf mode\n");
>  	mdelay(1);
>  
>  	ironlake_set_drps(dev_priv, fstart);
>  
> -	dev_priv->ips.last_count1 = I915_READ(DMIEC) +
> -		I915_READ(DDREC) + I915_READ(CSIEC);
> +	dev_priv->ips.last_count1 =
> +		intel_uncore_read(uncore, DMIEC) +
> +		intel_uncore_read(uncore, DDREC) +
> +		intel_uncore_read(uncore, CSIEC);
>  	dev_priv->ips.last_time1 = jiffies_to_msecs(jiffies);
> -	dev_priv->ips.last_count2 = I915_READ(GFXEC);
> +	dev_priv->ips.last_count2 = intel_uncore_read(uncore, GFXEC);
>  	dev_priv->ips.last_time2 = ktime_get_raw_ns();
>  
>  	spin_unlock_irq(&mchdev_lock);
>  }
>  
> -static void ironlake_disable_drps(struct drm_i915_private *dev_priv)
> +static void ironlake_disable_drps(struct drm_i915_private *i915)
>  {
> +	struct intel_uncore *uncore = &i915->uncore;
>  	u16 rgvswctl;
>  
>  	spin_lock_irq(&mchdev_lock);
>  
> -	rgvswctl = I915_READ16(MEMSWCTL);
> +	rgvswctl = intel_uncore_read16(uncore, MEMSWCTL);
>  
>  	/* Ack interrupts, disable EFC interrupt */
> -	I915_WRITE(MEMINTREN, I915_READ(MEMINTREN) & ~MEMINT_EVAL_CHG_EN);
> -	I915_WRITE(MEMINTRSTS, MEMINT_EVAL_CHG);
> -	I915_WRITE(DEIER, I915_READ(DEIER) & ~DE_PCU_EVENT);
> -	I915_WRITE(DEIIR, DE_PCU_EVENT);
> -	I915_WRITE(DEIMR, I915_READ(DEIMR) | DE_PCU_EVENT);
> +	intel_uncore_write(uncore,
> +			   MEMINTREN,
> +			   intel_uncore_read(uncore, MEMINTREN) &
> +			   ~MEMINT_EVAL_CHG_EN);
> +	intel_uncore_write(uncore, MEMINTRSTS, MEMINT_EVAL_CHG);
> +	intel_uncore_write(uncore,
> +			   DEIER,
> +			   intel_uncore_read(uncore, DEIER) & ~DE_PCU_EVENT);
> +	intel_uncore_write(uncore, DEIIR, DE_PCU_EVENT);
> +	intel_uncore_write(uncore,
> +			   DEIMR,
> +			   intel_uncore_read(uncore, DEIMR) | DE_PCU_EVENT);
>  
>  	/* Go back to the starting frequency */
> -	ironlake_set_drps(dev_priv, dev_priv->ips.fstart);
> +	ironlake_set_drps(i915, i915->ips.fstart);
>  	mdelay(1);
>  	rgvswctl |= MEMCTL_CMD_STS;
> -	I915_WRITE(MEMSWCTL, rgvswctl);
> +	intel_uncore_write(uncore, MEMSWCTL, rgvswctl);
>  	mdelay(1);
>  
>  	spin_unlock_irq(&mchdev_lock);
> @@ -9504,16 +9518,21 @@ static void g4x_init_clock_gating(struct drm_i915_private *dev_priv)
>  
>  static void i965gm_init_clock_gating(struct drm_i915_private *dev_priv)
>  {
> -	I915_WRITE(RENCLK_GATE_D1, I965_RCC_CLOCK_GATE_DISABLE);
> -	I915_WRITE(RENCLK_GATE_D2, 0);
> -	I915_WRITE(DSPCLK_GATE_D, 0);
> -	I915_WRITE(RAMCLK_GATE_D, 0);
> -	I915_WRITE16(DEUC, 0);
> -	I915_WRITE(MI_ARB_STATE,
> -		   _MASKED_BIT_ENABLE(MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE));
> +	struct intel_uncore *uncore = &dev_priv->uncore;
> +
> +	intel_uncore_write(uncore, RENCLK_GATE_D1, I965_RCC_CLOCK_GATE_DISABLE);
> +	intel_uncore_write(uncore, RENCLK_GATE_D2, 0);
> +	intel_uncore_write(uncore, DSPCLK_GATE_D, 0);
> +	intel_uncore_write(uncore, RAMCLK_GATE_D, 0);
> +	intel_uncore_write16(uncore, DEUC, 0);
> +	intel_uncore_write(uncore,
> +			   MI_ARB_STATE,
> +			   _MASKED_BIT_ENABLE(MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE));
>  
>  	/* WaDisable_RenderCache_OperationalFlush:gen4 */
> -	I915_WRITE(CACHE_MODE_0, _MASKED_BIT_DISABLE(RC_OP_FLUSH_ENABLE));
> +	intel_uncore_write(uncore,
> +			   CACHE_MODE_0,
> +			   _MASKED_BIT_DISABLE(RC_OP_FLUSH_ENABLE));
>  }
>  
>  static void i965g_init_clock_gating(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 07/12] drm/i915: Remove I915_READ8
  2019-06-07 12:08 ` [RFC 07/12] drm/i915: Remove I915_READ8 Tvrtko Ursulin
@ 2019-06-07 13:11   ` Jani Nikula
  2019-06-07 13:19     ` Tvrtko Ursulin
  2019-06-11  8:58   ` Jani Nikula
  1 sibling, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2019-06-07 13:11 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Only a few call sites remain which have been converted to uncore mmio
> accessors and so the macro can be removed.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I have some reservations about this one and patch 11. Again, I'm fine
with nuking I915_READ8 and I915_READ_NOTRACE (in patch 11). I think
they're special cases and it's okay if they grow into a bit longer lines
or multiple lines.

The problem is converting regular I915_READ and I915_WRITE in display
code while at it.

I don't think en masse conversion of them to intel_uncore_read and
intel_uncore_write directly is going to happen in display code, because
there's too much code that gets turned too ugly with the increase in
function name length and the extra passed parameter. I think many of
those places are pretty ugly as it is already. That's my feeling anyway.

I understand your reasoning is to avoid the mixed use of intel_uncore_*
and I915_*. But I fear using intel_uncore_read and intel_uncore_write
now is going to promote their use all over the place, and *that* will
lead to mixed use. Which is not optimal if the overall feeling is that
we need to come up with a shorter function/macro for display code reads
and writes.

I presume Ville has something to say about the gt vs. display stuff as
well; does the whole series make that harder? I hope not, because I do
like the rest of what's being done here.


BR,
Jani.



> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  2 --
>  drivers/gpu/drm/i915/intel_crt.c | 41 ++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_pm.c  |  6 ++---
>  3 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b2763721b76d..13815795e197 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2852,8 +2852,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>  #define __I915_REG_OP(op__, dev_priv__, ...) \
>  	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
>  
> -#define I915_READ8(reg__)	  __I915_REG_OP(read8, dev_priv, (reg__))
> -
>  #define I915_READ16(reg__)	   __I915_REG_OP(read16, dev_priv, (reg__))
>  #define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
>  
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index bb56518576a1..3fcf2f84bcce 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -643,6 +643,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  {
>  	struct drm_device *dev = crt->base.base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	u32 save_bclrpat;
>  	u32 save_vtotal;
>  	u32 vtotal, vactive;
> @@ -663,9 +664,9 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  	pipeconf_reg = PIPECONF(pipe);
>  	pipe_dsl_reg = PIPEDSL(pipe);
>  
> -	save_bclrpat = I915_READ(bclrpat_reg);
> -	save_vtotal = I915_READ(vtotal_reg);
> -	vblank = I915_READ(vblank_reg);
> +	save_bclrpat = intel_uncore_read(uncore, bclrpat_reg);
> +	save_vtotal = intel_uncore_read(uncore, vtotal_reg);
> +	vblank = intel_uncore_read(uncore, vblank_reg);
>  
>  	vtotal = ((save_vtotal >> 16) & 0xfff) + 1;
>  	vactive = (save_vtotal & 0x7ff) + 1;
> @@ -674,21 +675,23 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  	vblank_end = ((vblank >> 16) & 0xfff) + 1;
>  
>  	/* Set the border color to purple. */
> -	I915_WRITE(bclrpat_reg, 0x500050);
> +	intel_uncore_write(uncore, bclrpat_reg, 0x500050);
>  
>  	if (!IS_GEN(dev_priv, 2)) {
> -		u32 pipeconf = I915_READ(pipeconf_reg);
> -		I915_WRITE(pipeconf_reg, pipeconf | PIPECONF_FORCE_BORDER);
> -		POSTING_READ(pipeconf_reg);
> +		u32 pipeconf = intel_uncore_read(uncore, pipeconf_reg);
> +		intel_uncore_write(uncore,
> +				   pipeconf_reg,
> +				   pipeconf | PIPECONF_FORCE_BORDER);
> +		intel_uncore_posting_read(uncore, pipeconf_reg);
>  		/* Wait for next Vblank to substitue
>  		 * border color for Color info */
>  		intel_wait_for_vblank(dev_priv, pipe);
> -		st00 = I915_READ8(_VGA_MSR_WRITE);
> +		st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>  		status = ((st00 & (1 << 4)) != 0) ?
>  			connector_status_connected :
>  			connector_status_disconnected;
>  
> -		I915_WRITE(pipeconf_reg, pipeconf);
> +		intel_uncore_write(uncore, pipeconf_reg, pipeconf);
>  	} else {
>  		bool restore_vblank = false;
>  		int count, detect;
> @@ -702,9 +705,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  			u32 vsync_start = (vsync & 0xffff) + 1;
>  
>  			vblank_start = vsync_start;
> -			I915_WRITE(vblank_reg,
> -				   (vblank_start - 1) |
> -				   ((vblank_end - 1) << 16));
> +			intel_uncore_write(uncore,
> +					   vblank_reg,
> +					   (vblank_start - 1) |
> +					   ((vblank_end - 1) << 16));
>  			restore_vblank = true;
>  		}
>  		/* sample in the vertical border, selecting the larger one */
> @@ -716,9 +720,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  		/*
>  		 * Wait for the border to be displayed
>  		 */
> -		while (I915_READ(pipe_dsl_reg) >= vactive)
> +		while (intel_uncore_read(uncore, pipe_dsl_reg) >= vactive)
>  			;
> -		while ((dsl = I915_READ(pipe_dsl_reg)) <= vsample)
> +		while ((dsl = intel_uncore_read(uncore, pipe_dsl_reg)) <=
> +		       vsample)
>  			;
>  		/*
>  		 * Watch ST00 for an entire scanline
> @@ -728,14 +733,14 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  		do {
>  			count++;
>  			/* Read the ST00 VGA status register */
> -			st00 = I915_READ8(_VGA_MSR_WRITE);
> +			st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>  			if (st00 & (1 << 4))
>  				detect++;
> -		} while ((I915_READ(pipe_dsl_reg) == dsl));
> +		} while ((intel_uncore_read(uncore, pipe_dsl_reg) == dsl));
>  
>  		/* restore vblank if necessary */
>  		if (restore_vblank)
> -			I915_WRITE(vblank_reg, vblank);
> +			intel_uncore_write(uncore, vblank_reg, vblank);
>  		/*
>  		 * If more than 3/4 of the scanline detected a monitor,
>  		 * then it is assumed to be present. This works even on i830,
> @@ -748,7 +753,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  	}
>  
>  	/* Restore previous settings */
> -	I915_WRITE(bclrpat_reg, save_bclrpat);
> +	intel_uncore_write(uncore, bclrpat_reg, save_bclrpat);
>  
>  	return status;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d7272d4ff258..93e411e6ad19 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -8160,15 +8160,15 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>  	return val;
>  }
>  
> -unsigned long i915_mch_val(struct drm_i915_private *dev_priv)
> +unsigned long i915_mch_val(struct drm_i915_private *i915)
>  {
>  	unsigned long m, x, b;
>  	u32 tsfs;
>  
> -	tsfs = I915_READ(TSFS);
> +	tsfs = intel_uncore_read(&i915->uncore, TSFS);
>  
>  	m = ((tsfs & TSFS_SLOPE_MASK) >> TSFS_SLOPE_SHIFT);
> -	x = I915_READ8(TR1);
> +	x = intel_uncore_read8(&i915->uncore, TR1);
>  
>  	b = tsfs & TSFS_INTR_MASK;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 07/12] drm/i915: Remove I915_READ8
  2019-06-07 13:11   ` Jani Nikula
@ 2019-06-07 13:19     ` Tvrtko Ursulin
  2019-06-07 13:44       ` Jani Nikula
  0 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-07 13:19 UTC (permalink / raw)
  To: Jani Nikula, Intel-gfx



On 07/06/2019 14:11, Jani Nikula wrote:
> On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Only a few call sites remain which have been converted to uncore mmio
>> accessors and so the macro can be removed.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> I have some reservations about this one and patch 11. Again, I'm fine
> with nuking I915_READ8 and I915_READ_NOTRACE (in patch 11). I think
> they're special cases and it's okay if they grow into a bit longer lines
> or multiple lines.
> 
> The problem is converting regular I915_READ and I915_WRITE in display
> code while at it.
> 
> I don't think en masse conversion of them to intel_uncore_read and
> intel_uncore_write directly is going to happen in display code, because
> there's too much code that gets turned too ugly with the increase in
> function name length and the extra passed parameter. I think many of
> those places are pretty ugly as it is already. That's my feeling anyway.
> 
> I understand your reasoning is to avoid the mixed use of intel_uncore_*

Exactly.

> and I915_*. But I fear using intel_uncore_read and intel_uncore_write
> now is going to promote their use all over the place, and *that* will
> lead to mixed use. Which is not optimal if the overall feeling is that
> we need to come up with a shorter function/macro for display code reads
> and writes.

I am fine with the argument that you may desire something shorter for 
display. But I don't think converting whole functions would create any 
more future mixed use than converting functions partially.

Btw have you seen the latest RFC from Daniele? That would allow that you 
only replace the assignemnts at the top of functions perhaps like from:

	struct intel_uncore *uncore = &dev_priv->uncore;

to:

	struct intel_uncore *uncore = &dev_priv->display_uncore;

But sure, if you desire shorter accessors then lets first have a 
definitive decision of that.

> I presume Ville has something to say about the gt vs. display stuff as
> well; does the whole series make that harder? I hope not, because I do
> like the rest of what's being done here.

It doesn't make it harder. I can easily drop the bits you don't like if 
that will be the final decision. In fact, I should probably do that 
straight away if that would unblock the remaining two patches because 
then I can proceed with other refactorings on top.

Regards,

Tvrtko

> 
> 
> BR,
> Jani.
> 
> 
> 
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h  |  2 --
>>   drivers/gpu/drm/i915/intel_crt.c | 41 ++++++++++++++++++--------------
>>   drivers/gpu/drm/i915/intel_pm.c  |  6 ++---
>>   3 files changed, 26 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index b2763721b76d..13815795e197 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2852,8 +2852,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>>   #define __I915_REG_OP(op__, dev_priv__, ...) \
>>   	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
>>   
>> -#define I915_READ8(reg__)	  __I915_REG_OP(read8, dev_priv, (reg__))
>> -
>>   #define I915_READ16(reg__)	   __I915_REG_OP(read16, dev_priv, (reg__))
>>   #define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>> index bb56518576a1..3fcf2f84bcce 100644
>> --- a/drivers/gpu/drm/i915/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> @@ -643,6 +643,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>   {
>>   	struct drm_device *dev = crt->base.base.dev;
>>   	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	struct intel_uncore *uncore = &dev_priv->uncore;
>>   	u32 save_bclrpat;
>>   	u32 save_vtotal;
>>   	u32 vtotal, vactive;
>> @@ -663,9 +664,9 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>   	pipeconf_reg = PIPECONF(pipe);
>>   	pipe_dsl_reg = PIPEDSL(pipe);
>>   
>> -	save_bclrpat = I915_READ(bclrpat_reg);
>> -	save_vtotal = I915_READ(vtotal_reg);
>> -	vblank = I915_READ(vblank_reg);
>> +	save_bclrpat = intel_uncore_read(uncore, bclrpat_reg);
>> +	save_vtotal = intel_uncore_read(uncore, vtotal_reg);
>> +	vblank = intel_uncore_read(uncore, vblank_reg);
>>   
>>   	vtotal = ((save_vtotal >> 16) & 0xfff) + 1;
>>   	vactive = (save_vtotal & 0x7ff) + 1;
>> @@ -674,21 +675,23 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>   	vblank_end = ((vblank >> 16) & 0xfff) + 1;
>>   
>>   	/* Set the border color to purple. */
>> -	I915_WRITE(bclrpat_reg, 0x500050);
>> +	intel_uncore_write(uncore, bclrpat_reg, 0x500050);
>>   
>>   	if (!IS_GEN(dev_priv, 2)) {
>> -		u32 pipeconf = I915_READ(pipeconf_reg);
>> -		I915_WRITE(pipeconf_reg, pipeconf | PIPECONF_FORCE_BORDER);
>> -		POSTING_READ(pipeconf_reg);
>> +		u32 pipeconf = intel_uncore_read(uncore, pipeconf_reg);
>> +		intel_uncore_write(uncore,
>> +				   pipeconf_reg,
>> +				   pipeconf | PIPECONF_FORCE_BORDER);
>> +		intel_uncore_posting_read(uncore, pipeconf_reg);
>>   		/* Wait for next Vblank to substitue
>>   		 * border color for Color info */
>>   		intel_wait_for_vblank(dev_priv, pipe);
>> -		st00 = I915_READ8(_VGA_MSR_WRITE);
>> +		st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>>   		status = ((st00 & (1 << 4)) != 0) ?
>>   			connector_status_connected :
>>   			connector_status_disconnected;
>>   
>> -		I915_WRITE(pipeconf_reg, pipeconf);
>> +		intel_uncore_write(uncore, pipeconf_reg, pipeconf);
>>   	} else {
>>   		bool restore_vblank = false;
>>   		int count, detect;
>> @@ -702,9 +705,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>   			u32 vsync_start = (vsync & 0xffff) + 1;
>>   
>>   			vblank_start = vsync_start;
>> -			I915_WRITE(vblank_reg,
>> -				   (vblank_start - 1) |
>> -				   ((vblank_end - 1) << 16));
>> +			intel_uncore_write(uncore,
>> +					   vblank_reg,
>> +					   (vblank_start - 1) |
>> +					   ((vblank_end - 1) << 16));
>>   			restore_vblank = true;
>>   		}
>>   		/* sample in the vertical border, selecting the larger one */
>> @@ -716,9 +720,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>   		/*
>>   		 * Wait for the border to be displayed
>>   		 */
>> -		while (I915_READ(pipe_dsl_reg) >= vactive)
>> +		while (intel_uncore_read(uncore, pipe_dsl_reg) >= vactive)
>>   			;
>> -		while ((dsl = I915_READ(pipe_dsl_reg)) <= vsample)
>> +		while ((dsl = intel_uncore_read(uncore, pipe_dsl_reg)) <=
>> +		       vsample)
>>   			;
>>   		/*
>>   		 * Watch ST00 for an entire scanline
>> @@ -728,14 +733,14 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>   		do {
>>   			count++;
>>   			/* Read the ST00 VGA status register */
>> -			st00 = I915_READ8(_VGA_MSR_WRITE);
>> +			st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>>   			if (st00 & (1 << 4))
>>   				detect++;
>> -		} while ((I915_READ(pipe_dsl_reg) == dsl));
>> +		} while ((intel_uncore_read(uncore, pipe_dsl_reg) == dsl));
>>   
>>   		/* restore vblank if necessary */
>>   		if (restore_vblank)
>> -			I915_WRITE(vblank_reg, vblank);
>> +			intel_uncore_write(uncore, vblank_reg, vblank);
>>   		/*
>>   		 * If more than 3/4 of the scanline detected a monitor,
>>   		 * then it is assumed to be present. This works even on i830,
>> @@ -748,7 +753,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>   	}
>>   
>>   	/* Restore previous settings */
>> -	I915_WRITE(bclrpat_reg, save_bclrpat);
>> +	intel_uncore_write(uncore, bclrpat_reg, save_bclrpat);
>>   
>>   	return status;
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index d7272d4ff258..93e411e6ad19 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -8160,15 +8160,15 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>>   	return val;
>>   }
>>   
>> -unsigned long i915_mch_val(struct drm_i915_private *dev_priv)
>> +unsigned long i915_mch_val(struct drm_i915_private *i915)
>>   {
>>   	unsigned long m, x, b;
>>   	u32 tsfs;
>>   
>> -	tsfs = I915_READ(TSFS);
>> +	tsfs = intel_uncore_read(&i915->uncore, TSFS);
>>   
>>   	m = ((tsfs & TSFS_SLOPE_MASK) >> TSFS_SLOPE_SHIFT);
>> -	x = I915_READ8(TR1);
>> +	x = intel_uncore_read8(&i915->uncore, TR1);
>>   
>>   	b = tsfs & TSFS_INTR_MASK;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 07/12] drm/i915: Remove I915_READ8
  2019-06-07 13:19     ` Tvrtko Ursulin
@ 2019-06-07 13:44       ` Jani Nikula
  2019-06-11  8:47         ` Tvrtko Ursulin
  0 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2019-06-07 13:44 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 07/06/2019 14:11, Jani Nikula wrote:
>> On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> Only a few call sites remain which have been converted to uncore mmio
>>> accessors and so the macro can be removed.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> 
>> I have some reservations about this one and patch 11. Again, I'm fine
>> with nuking I915_READ8 and I915_READ_NOTRACE (in patch 11). I think
>> they're special cases and it's okay if they grow into a bit longer lines
>> or multiple lines.
>> 
>> The problem is converting regular I915_READ and I915_WRITE in display
>> code while at it.
>> 
>> I don't think en masse conversion of them to intel_uncore_read and
>> intel_uncore_write directly is going to happen in display code, because
>> there's too much code that gets turned too ugly with the increase in
>> function name length and the extra passed parameter. I think many of
>> those places are pretty ugly as it is already. That's my feeling anyway.
>> 
>> I understand your reasoning is to avoid the mixed use of intel_uncore_*
>
> Exactly.
>
>> and I915_*. But I fear using intel_uncore_read and intel_uncore_write
>> now is going to promote their use all over the place, and *that* will
>> lead to mixed use. Which is not optimal if the overall feeling is that
>> we need to come up with a shorter function/macro for display code reads
>> and writes.
>
> I am fine with the argument that you may desire something shorter for 
> display. But I don't think converting whole functions would create any 
> more future mixed use than converting functions partially.
>
> Btw have you seen the latest RFC from Daniele?

Yes, but haven't had the time to digest it.

> That would allow that you 
> only replace the assignemnts at the top of functions perhaps like from:
>
> 	struct intel_uncore *uncore = &dev_priv->uncore;
>
> to:
>
> 	struct intel_uncore *uncore = &dev_priv->display_uncore;
>
> But sure, if you desire shorter accessors then lets first have a 
> definitive decision of that.

If there were display accessors they might just take i915 as pointer:

#define FOO_READ(i915, reg) intel_uncore_read(&i915->display_uncore, reg)

Dunno.

>
>> I presume Ville has something to say about the gt vs. display stuff as
>> well; does the whole series make that harder? I hope not, because I do
>> like the rest of what's being done here.
>
> It doesn't make it harder. I can easily drop the bits you don't like if 
> that will be the final decision. In fact, I should probably do that 
> straight away if that would unblock the remaining two patches because 
> then I can proceed with other refactorings on top.

Hum, you know what, it's not *that* big of a deal. Ack on the whole
series, we can tidy up later on if needed.

I guess I'd like to get Ville's ack on the series too. Ville?


BR,
Jani.


>
> Regards,
>
> Tvrtko
>
>> 
>> 
>> BR,
>> Jani.
>> 
>> 
>> 
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h  |  2 --
>>>   drivers/gpu/drm/i915/intel_crt.c | 41 ++++++++++++++++++--------------
>>>   drivers/gpu/drm/i915/intel_pm.c  |  6 ++---
>>>   3 files changed, 26 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index b2763721b76d..13815795e197 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -2852,8 +2852,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>>>   #define __I915_REG_OP(op__, dev_priv__, ...) \
>>>   	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
>>>   
>>> -#define I915_READ8(reg__)	  __I915_REG_OP(read8, dev_priv, (reg__))
>>> -
>>>   #define I915_READ16(reg__)	   __I915_REG_OP(read16, dev_priv, (reg__))
>>>   #define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
>>>   
>>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>>> index bb56518576a1..3fcf2f84bcce 100644
>>> --- a/drivers/gpu/drm/i915/intel_crt.c
>>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>>> @@ -643,6 +643,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>>   {
>>>   	struct drm_device *dev = crt->base.base.dev;
>>>   	struct drm_i915_private *dev_priv = to_i915(dev);
>>> +	struct intel_uncore *uncore = &dev_priv->uncore;
>>>   	u32 save_bclrpat;
>>>   	u32 save_vtotal;
>>>   	u32 vtotal, vactive;
>>> @@ -663,9 +664,9 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>>   	pipeconf_reg = PIPECONF(pipe);
>>>   	pipe_dsl_reg = PIPEDSL(pipe);
>>>   
>>> -	save_bclrpat = I915_READ(bclrpat_reg);
>>> -	save_vtotal = I915_READ(vtotal_reg);
>>> -	vblank = I915_READ(vblank_reg);
>>> +	save_bclrpat = intel_uncore_read(uncore, bclrpat_reg);
>>> +	save_vtotal = intel_uncore_read(uncore, vtotal_reg);
>>> +	vblank = intel_uncore_read(uncore, vblank_reg);
>>>   
>>>   	vtotal = ((save_vtotal >> 16) & 0xfff) + 1;
>>>   	vactive = (save_vtotal & 0x7ff) + 1;
>>> @@ -674,21 +675,23 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>>   	vblank_end = ((vblank >> 16) & 0xfff) + 1;
>>>   
>>>   	/* Set the border color to purple. */
>>> -	I915_WRITE(bclrpat_reg, 0x500050);
>>> +	intel_uncore_write(uncore, bclrpat_reg, 0x500050);
>>>   
>>>   	if (!IS_GEN(dev_priv, 2)) {
>>> -		u32 pipeconf = I915_READ(pipeconf_reg);
>>> -		I915_WRITE(pipeconf_reg, pipeconf | PIPECONF_FORCE_BORDER);
>>> -		POSTING_READ(pipeconf_reg);
>>> +		u32 pipeconf = intel_uncore_read(uncore, pipeconf_reg);
>>> +		intel_uncore_write(uncore,
>>> +				   pipeconf_reg,
>>> +				   pipeconf | PIPECONF_FORCE_BORDER);
>>> +		intel_uncore_posting_read(uncore, pipeconf_reg);
>>>   		/* Wait for next Vblank to substitue
>>>   		 * border color for Color info */
>>>   		intel_wait_for_vblank(dev_priv, pipe);
>>> -		st00 = I915_READ8(_VGA_MSR_WRITE);
>>> +		st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>>>   		status = ((st00 & (1 << 4)) != 0) ?
>>>   			connector_status_connected :
>>>   			connector_status_disconnected;
>>>   
>>> -		I915_WRITE(pipeconf_reg, pipeconf);
>>> +		intel_uncore_write(uncore, pipeconf_reg, pipeconf);
>>>   	} else {
>>>   		bool restore_vblank = false;
>>>   		int count, detect;
>>> @@ -702,9 +705,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>>   			u32 vsync_start = (vsync & 0xffff) + 1;
>>>   
>>>   			vblank_start = vsync_start;
>>> -			I915_WRITE(vblank_reg,
>>> -				   (vblank_start - 1) |
>>> -				   ((vblank_end - 1) << 16));
>>> +			intel_uncore_write(uncore,
>>> +					   vblank_reg,
>>> +					   (vblank_start - 1) |
>>> +					   ((vblank_end - 1) << 16));
>>>   			restore_vblank = true;
>>>   		}
>>>   		/* sample in the vertical border, selecting the larger one */
>>> @@ -716,9 +720,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>>   		/*
>>>   		 * Wait for the border to be displayed
>>>   		 */
>>> -		while (I915_READ(pipe_dsl_reg) >= vactive)
>>> +		while (intel_uncore_read(uncore, pipe_dsl_reg) >= vactive)
>>>   			;
>>> -		while ((dsl = I915_READ(pipe_dsl_reg)) <= vsample)
>>> +		while ((dsl = intel_uncore_read(uncore, pipe_dsl_reg)) <=
>>> +		       vsample)
>>>   			;
>>>   		/*
>>>   		 * Watch ST00 for an entire scanline
>>> @@ -728,14 +733,14 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>>   		do {
>>>   			count++;
>>>   			/* Read the ST00 VGA status register */
>>> -			st00 = I915_READ8(_VGA_MSR_WRITE);
>>> +			st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>>>   			if (st00 & (1 << 4))
>>>   				detect++;
>>> -		} while ((I915_READ(pipe_dsl_reg) == dsl));
>>> +		} while ((intel_uncore_read(uncore, pipe_dsl_reg) == dsl));
>>>   
>>>   		/* restore vblank if necessary */
>>>   		if (restore_vblank)
>>> -			I915_WRITE(vblank_reg, vblank);
>>> +			intel_uncore_write(uncore, vblank_reg, vblank);
>>>   		/*
>>>   		 * If more than 3/4 of the scanline detected a monitor,
>>>   		 * then it is assumed to be present. This works even on i830,
>>> @@ -748,7 +753,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>>>   	}
>>>   
>>>   	/* Restore previous settings */
>>> -	I915_WRITE(bclrpat_reg, save_bclrpat);
>>> +	intel_uncore_write(uncore, bclrpat_reg, save_bclrpat);
>>>   
>>>   	return status;
>>>   }
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index d7272d4ff258..93e411e6ad19 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -8160,15 +8160,15 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>>>   	return val;
>>>   }
>>>   
>>> -unsigned long i915_mch_val(struct drm_i915_private *dev_priv)
>>> +unsigned long i915_mch_val(struct drm_i915_private *i915)
>>>   {
>>>   	unsigned long m, x, b;
>>>   	u32 tsfs;
>>>   
>>> -	tsfs = I915_READ(TSFS);
>>> +	tsfs = intel_uncore_read(&i915->uncore, TSFS);
>>>   
>>>   	m = ((tsfs & TSFS_SLOPE_MASK) >> TSFS_SLOPE_SHIFT);
>>> -	x = I915_READ8(TR1);
>>> +	x = intel_uncore_read8(&i915->uncore, TR1);
>>>   
>>>   	b = tsfs & TSFS_INTR_MASK;
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for Legacy mmio accessor macro pruning
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (11 preceding siblings ...)
  2019-06-07 12:08 ` [RFC 12/12] drm/i915: Remove I915_READ16 and I915_WRITE16 Tvrtko Ursulin
@ 2019-06-07 14:36 ` Patchwork
  2019-06-07 15:04 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-06-10  9:57 ` ✓ Fi.CI.IGT: " Patchwork
  14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-06-07 14:36 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Legacy mmio accessor macro pruning
URL   : https://patchwork.freedesktop.org/series/61772/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
043c61153db4 drm/i915: Eliminate unused mmio accessors
103768ba0c26 drm/i915: Convert i915_reg_read_ioctl to use explicit mmio accessors
bb96d1829725 drm/i915: Convert icl_get_stolen_reserved to uncore mmio accessors
f9d38ebb6813 drm/i915: Convert gem_record_fences to uncore mmio accessors
ee7f850e7f59 drm/i915: Convert intel_read_wm_latency to uncore mmio accessors
d861df71b049 drm/i915: Remove I915_READ64 and I915_READ64_32x2
4603abb220e4 drm/i915: Remove I915_READ8
-:61: WARNING:LINE_SPACING: Missing a blank line after declarations
#61: FILE: drivers/gpu/drm/i915/intel_crt.c:682:
+		u32 pipeconf = intel_uncore_read(uncore, pipeconf_reg);
+		intel_uncore_write(uncore,

total: 0 errors, 1 warnings, 0 checks, 124 lines checked
da93f7fd6be9 drm/i915: Remove I915_POSTING_READ_FW
6fb2e990f7bd drm/i915: Remove POSTING_READ16
eced9b6e9753 drm/i915: Remove I915_WRITE_NOTRACE
2538e7755fad drm/i915: Remove I915_READ_NOTRACE
-:118: WARNING:LONG_LINE: line over 100 characters
#118: FILE: drivers/gpu/drm/i915/intel_dp.c:1090:
+#define C (((status = intel_uncore_read_notrace(&i915->uncore, ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)

total: 0 errors, 1 warnings, 0 checks, 195 lines checked
00d7ab1abb0b drm/i915: Remove I915_READ16 and I915_WRITE16

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* ✓ Fi.CI.BAT: success for Legacy mmio accessor macro pruning
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (12 preceding siblings ...)
  2019-06-07 14:36 ` ✗ Fi.CI.CHECKPATCH: warning for Legacy mmio accessor macro pruning Patchwork
@ 2019-06-07 15:04 ` Patchwork
  2019-06-10  9:57 ` ✓ Fi.CI.IGT: " Patchwork
  14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-06-07 15:04 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Legacy mmio accessor macro pruning
URL   : https://patchwork.freedesktop.org/series/61772/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6220 -> Patchwork_13206
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/

Known issues
------------

  Here are the changes found in Patchwork_13206 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_flip@basic-plain-flip:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/fi-icl-u3/igt@kms_flip@basic-plain-flip.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/fi-icl-u3/igt@kms_flip@basic-plain-flip.html

  
#### Possible fixes ####

  * igt@gem_ctx_exec@basic:
    - {fi-icl-guc}:       [INCOMPLETE][3] ([fdo#107713]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/fi-icl-guc/igt@gem_ctx_exec@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/fi-icl-guc/igt@gem_ctx_exec@basic.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [FAIL][5] ([fdo#110387]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
    - fi-icl-u3:          [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8] +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108044]: https://bugs.freedesktop.org/show_bug.cgi?id=108044
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#110387]: https://bugs.freedesktop.org/show_bug.cgi?id=110387
  [fdo#110829]: https://bugs.freedesktop.org/show_bug.cgi?id=110829


Participating hosts (54 -> 48)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6220 -> Patchwork_13206

  CI_DRM_6220: c3e52a30973adec1e27ebb2b61f0cf969ae9b168 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5048: 1a34b94f1ce07ac5978fe7893a17e8732d467868 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13206: 00d7ab1abb0ba3121859a0d331ef6e7c473d0ce7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

00d7ab1abb0b drm/i915: Remove I915_READ16 and I915_WRITE16
2538e7755fad drm/i915: Remove I915_READ_NOTRACE
eced9b6e9753 drm/i915: Remove I915_WRITE_NOTRACE
6fb2e990f7bd drm/i915: Remove POSTING_READ16
da93f7fd6be9 drm/i915: Remove I915_POSTING_READ_FW
4603abb220e4 drm/i915: Remove I915_READ8
d861df71b049 drm/i915: Remove I915_READ64 and I915_READ64_32x2
ee7f850e7f59 drm/i915: Convert intel_read_wm_latency to uncore mmio accessors
f9d38ebb6813 drm/i915: Convert gem_record_fences to uncore mmio accessors
bb96d1829725 drm/i915: Convert icl_get_stolen_reserved to uncore mmio accessors
103768ba0c26 drm/i915: Convert i915_reg_read_ioctl to use explicit mmio accessors
043c61153db4 drm/i915: Eliminate unused mmio accessors

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* ✓ Fi.CI.IGT: success for Legacy mmio accessor macro pruning
  2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
                   ` (13 preceding siblings ...)
  2019-06-07 15:04 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-06-10  9:57 ` Patchwork
  14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-06-10  9:57 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Legacy mmio accessor macro pruning
URL   : https://patchwork.freedesktop.org/series/61772/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6220_full -> Patchwork_13206_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_13206_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_gtt@forked-medium-copy:
    - shard-iclb:         [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-iclb3/igt@gem_mmap_gtt@forked-medium-copy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-iclb3/igt@gem_mmap_gtt@forked-medium-copy.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108566]) +5 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-apl7/igt@gem_workarounds@suspend-resume-context.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-apl2/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#104108])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-skl5/igt@kms_fbcon_fbt@psr-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-skl1/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [PASS][7] -> [INCOMPLETE][8] ([fdo#109507])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-skl9/igt@kms_flip@flip-vs-suspend-interruptible.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-skl7/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip_tiling@flip-x-tiled:
    - shard-iclb:         [PASS][9] -> [FAIL][10] ([fdo#108303])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-iclb3/igt@kms_flip_tiling@flip-x-tiled.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-iclb7/igt@kms_flip_tiling@flip-x-tiled.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103167]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - shard-kbl:          [PASS][13] -> [DMESG-WARN][14] ([fdo#108566])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-kbl6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-kbl6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103166])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [PASS][17] -> [FAIL][18] ([fdo#99912])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-kbl4/igt@kms_setmode@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-kbl2/igt@kms_setmode@basic.html

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         [PASS][19] -> [FAIL][20] ([fdo#100047])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-iclb7/igt@kms_sysfs_edid_timing.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-iclb3/igt@kms_sysfs_edid_timing.html

  * igt@perf@polling:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#110728])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-skl2/igt@perf@polling.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-skl2/igt@perf@polling.html

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          [FAIL][23] ([fdo#109661]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-snb5/igt@gem_eio@reset-stress.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-snb4/igt@gem_eio@reset-stress.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][25] ([fdo#104108]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-skl8/igt@gem_softpin@noreloc-s3.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-skl10/igt@gem_softpin@noreloc-s3.html

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          [DMESG-WARN][27] ([fdo#108566]) -> [PASS][28] +3 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-apl7/igt@i915_suspend@debugfs-reader.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-apl2/igt@i915_suspend@debugfs-reader.html

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-kbl:          [DMESG-WARN][29] ([fdo#103558] / [fdo#105602] / [fdo#110222]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-kbl2/igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-kbl1/igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [INCOMPLETE][31] ([fdo#109507]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-skl7/igt@kms_flip@flip-vs-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-skl4/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         [FAIL][33] ([fdo#103167]) -> [PASS][34] +5 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][35] ([fdo#103166]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_vblank@pipe-a-query-forked:
    - shard-kbl:          [DMESG-WARN][37] ([fdo#103558] / [fdo#105602]) -> [PASS][38] +25 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-kbl2/igt@kms_vblank@pipe-a-query-forked.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-kbl1/igt@kms_vblank@pipe-a-query-forked.html

  
#### Warnings ####

  * igt@gem_mmap_gtt@forked-big-copy-odd:
    - shard-iclb:         [TIMEOUT][39] ([fdo#109673]) -> [INCOMPLETE][40] ([fdo#107713] / [fdo#109100])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-iclb5/igt@gem_mmap_gtt@forked-big-copy-odd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-iclb7/igt@gem_mmap_gtt@forked-big-copy-odd.html

  * igt@kms_busy@basic-flip-f:
    - shard-kbl:          [SKIP][41] ([fdo#105602] / [fdo#109271] / [fdo#109278]) -> [SKIP][42] ([fdo#109271] / [fdo#109278])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-kbl2/igt@kms_busy@basic-flip-f.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-kbl1/igt@kms_busy@basic-flip-f.html

  * igt@kms_frontbuffer_tracking@fbcpsr-tilingchange:
    - shard-skl:          [FAIL][43] ([fdo#103167]) -> [FAIL][44] ([fdo#108040])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-skl5/igt@kms_frontbuffer_tracking@fbcpsr-tilingchange.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-skl9/igt@kms_frontbuffer_tracking@fbcpsr-tilingchange.html

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-mmap-wc:
    - shard-kbl:          [SKIP][45] ([fdo#105602] / [fdo#109271]) -> [SKIP][46] ([fdo#109271]) +15 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6220/shard-kbl2/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-mmap-wc.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/shard-kbl1/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-mmap-wc.html

  
  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (9 -> 9)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_6220 -> Patchwork_13206

  CI_DRM_6220: c3e52a30973adec1e27ebb2b61f0cf969ae9b168 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5048: 1a34b94f1ce07ac5978fe7893a17e8732d467868 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13206: 00d7ab1abb0ba3121859a0d331ef6e7c473d0ce7 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13206/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 07/12] drm/i915: Remove I915_READ8
  2019-06-07 13:44       ` Jani Nikula
@ 2019-06-11  8:47         ` Tvrtko Ursulin
  2019-06-11  8:59           ` Jani Nikula
  0 siblings, 1 reply; 28+ messages in thread
From: Tvrtko Ursulin @ 2019-06-11  8:47 UTC (permalink / raw)
  To: Jani Nikula, Intel-gfx


On 07/06/2019 14:44, Jani Nikula wrote:
> On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> On 07/06/2019 14:11, Jani Nikula wrote:
>>> On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Only a few call sites remain which have been converted to uncore mmio
>>>> accessors and so the macro can be removed.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> I have some reservations about this one and patch 11. Again, I'm fine
>>> with nuking I915_READ8 and I915_READ_NOTRACE (in patch 11). I think
>>> they're special cases and it's okay if they grow into a bit longer lines
>>> or multiple lines.
>>>
>>> The problem is converting regular I915_READ and I915_WRITE in display
>>> code while at it.
>>>
>>> I don't think en masse conversion of them to intel_uncore_read and
>>> intel_uncore_write directly is going to happen in display code, because
>>> there's too much code that gets turned too ugly with the increase in
>>> function name length and the extra passed parameter. I think many of
>>> those places are pretty ugly as it is already. That's my feeling anyway.
>>>
>>> I understand your reasoning is to avoid the mixed use of intel_uncore_*
>>
>> Exactly.
>>
>>> and I915_*. But I fear using intel_uncore_read and intel_uncore_write
>>> now is going to promote their use all over the place, and *that* will
>>> lead to mixed use. Which is not optimal if the overall feeling is that
>>> we need to come up with a shorter function/macro for display code reads
>>> and writes.
>>
>> I am fine with the argument that you may desire something shorter for
>> display. But I don't think converting whole functions would create any
>> more future mixed use than converting functions partially.
>>
>> Btw have you seen the latest RFC from Daniele?
> 
> Yes, but haven't had the time to digest it.
> 
>> That would allow that you
>> only replace the assignemnts at the top of functions perhaps like from:
>>
>> 	struct intel_uncore *uncore = &dev_priv->uncore;
>>
>> to:
>>
>> 	struct intel_uncore *uncore = &dev_priv->display_uncore;
>>
>> But sure, if you desire shorter accessors then lets first have a
>> definitive decision of that.
> 
> If there were display accessors they might just take i915 as pointer:
> 
> #define FOO_READ(i915, reg) intel_uncore_read(&i915->display_uncore, reg)
> 
> Dunno.
> 
>>
>>> I presume Ville has something to say about the gt vs. display stuff as
>>> well; does the whole series make that harder? I hope not, because I do
>>> like the rest of what's being done here.
>>
>> It doesn't make it harder. I can easily drop the bits you don't like if
>> that will be the final decision. In fact, I should probably do that
>> straight away if that would unblock the remaining two patches because
>> then I can proceed with other refactorings on top.
> 
> Hum, you know what, it's not *that* big of a deal. Ack on the whole
> series, we can tidy up later on if needed.
> 
> I guess I'd like to get Ville's ack on the series too. Ville?

So I have patches 7 & 11 with acks from Jani and no r-b here.

Depending on the opinion from Ville I can either ask for upgrade to r-b, 
or refactor them to minimize intel_uncore_read/write in favour of 
leaving the affected functions mixed (I915_READ/WRITE + 
intel_uncore_"more-exotic-accessors".

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 11/12] drm/i915: Remove I915_READ_NOTRACE
  2019-06-07 12:08 ` [RFC 11/12] drm/i915: Remove I915_READ_NOTRACE Tvrtko Ursulin
@ 2019-06-11  8:57   ` Jani Nikula
  0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2019-06-11  8:57 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Only a few call sites remain which have been converted to uncore mmio
> accessors and so the macro can be removed.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/gvt/debugfs.c  |  4 +--
>  drivers/gpu/drm/i915/gvt/firmware.c |  5 ++--
>  drivers/gpu/drm/i915/i915_drv.c     |  6 ++--
>  drivers/gpu/drm/i915/i915_drv.h     |  1 -
>  drivers/gpu/drm/i915/i915_pmu.c     |  8 ++++--
>  drivers/gpu/drm/i915/intel_dp.c     | 43 +++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_gmbus.c  | 11 ++++----
>  7 files changed, 43 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/debugfs.c b/drivers/gpu/drm/i915/gvt/debugfs.c
> index 8a9606f91e68..2fb7b73b260d 100644
> --- a/drivers/gpu/drm/i915/gvt/debugfs.c
> +++ b/drivers/gpu/drm/i915/gvt/debugfs.c
> @@ -58,12 +58,12 @@ static int mmio_offset_compare(void *priv,
>  static inline int mmio_diff_handler(struct intel_gvt *gvt,
>  				    u32 offset, void *data)
>  {
> -	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	struct drm_i915_private *i915 = gvt->dev_priv;
>  	struct mmio_diff_param *param = data;
>  	struct diff_mmio *node;
>  	u32 preg, vreg;
>  
> -	preg = I915_READ_NOTRACE(_MMIO(offset));
> +	preg = intel_uncore_read_notrace(&i915->uncore, _MMIO(offset));
>  	vreg = vgpu_vreg(param->vgpu, offset);
>  
>  	if (preg != vreg) {
> diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c
> index 4ac18b447247..049775e8e350 100644
> --- a/drivers/gpu/drm/i915/gvt/firmware.c
> +++ b/drivers/gpu/drm/i915/gvt/firmware.c
> @@ -68,9 +68,10 @@ static struct bin_attribute firmware_attr = {
>  
>  static int mmio_snapshot_handler(struct intel_gvt *gvt, u32 offset, void *data)
>  {
> -	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	struct drm_i915_private *i915 = gvt->dev_priv;
>  
> -	*(u32 *)(data + offset) = I915_READ_NOTRACE(_MMIO(offset));
> +	*(u32 *)(data + offset) = intel_uncore_read_notrace(&i915->uncore,
> +							    _MMIO(offset));
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1af6751e1b36..81ff2c78fd55 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2708,7 +2708,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  	I915_WRITE(VLV_GUNIT_CLOCK_GATE2,	s->clock_gate_dis2);
>  }
>  
> -static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
> +static int vlv_wait_for_pw_status(struct drm_i915_private *i915,
>  				  u32 mask, u32 val)
>  {
>  	i915_reg_t reg = VLV_GTLC_PW_STATUS;
> @@ -2722,7 +2722,9 @@ static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
>  	 * Transitioning between RC6 states should be at most 2ms (see
>  	 * valleyview_enable_rps) so use a 3ms timeout.
>  	 */
> -	ret = wait_for(((reg_value = I915_READ_NOTRACE(reg)) & mask) == val, 3);
> +	ret = wait_for(((reg_value =
> +			 intel_uncore_read_notrace(&i915->uncore, reg)) & mask)
> +		       == val, 3);
>  
>  	/* just trace the final value */
>  	trace_i915_reg_rw(false, reg, reg_value, sizeof(reg_value), true);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1ea2d5f42e52..9f0208e410ed 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2857,7 +2857,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>  
>  #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
>  #define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
> -#define I915_READ_NOTRACE(reg__)	 __I915_REG_OP(read_notrace, dev_priv, (reg__))
>  
>  #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
>  
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 1ccda0ee4ff5..eb9c0e0e545c 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -227,9 +227,11 @@ frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns)
>  		if (dev_priv->gt.awake) {
>  			intel_wakeref_t wakeref;
>  
> -			with_intel_runtime_pm_if_in_use(dev_priv, wakeref)
> -				val = intel_get_cagf(dev_priv,
> -						     I915_READ_NOTRACE(GEN6_RPSTAT1));
> +			with_intel_runtime_pm_if_in_use(dev_priv, wakeref) {
> +				val = intel_uncore_read_notrace(&dev_priv->uncore,
> +								GEN6_RPSTAT1);
> +				val = intel_get_cagf(dev_priv, val);
> +			}
>  		}
>  
>  		add_sample_mult(&dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT],
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b099a9dc28fd..0862678e68b9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1082,13 +1082,13 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
>  static u32
>  intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>  {
> -	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
>  	u32 status;
>  	bool done;
>  
> -#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> -	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> +#define C (((status = intel_uncore_read_notrace(&i915->uncore, ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> +	done = wait_event_timeout(i915->gmbus_wait_queue, C,
>  				  msecs_to_jiffies_timeout(10));
>  
>  	/* just trace the final value */
> @@ -1221,8 +1221,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  		  u32 aux_send_ctl_flags)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv =
> +	struct drm_i915_private *i915 =
>  			to_i915(intel_dig_port->base.base.dev);
> +	struct intel_uncore *uncore = &i915->uncore;
>  	i915_reg_t ch_ctl, ch_data[5];
>  	u32 aux_clock_divider;
>  	enum intel_display_power_domain aux_domain =
> @@ -1238,7 +1239,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  	for (i = 0; i < ARRAY_SIZE(ch_data); i++)
>  		ch_data[i] = intel_dp->aux_ch_data_reg(intel_dp, i);
>  
> -	aux_wakeref = intel_display_power_get(dev_priv, aux_domain);
> +	aux_wakeref = intel_display_power_get(i915, aux_domain);
>  	pps_wakeref = pps_lock(intel_dp);
>  
>  	/*
> @@ -1253,13 +1254,13 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  	 * lowest possible wakeup latency and so prevent the cpu from going into
>  	 * deep sleep states.
>  	 */
> -	pm_qos_update_request(&dev_priv->pm_qos, 0);
> +	pm_qos_update_request(&i915->pm_qos, 0);
>  
>  	intel_dp_check_edp(intel_dp);
>  
>  	/* Try to wait for any previous AUX channel activity */
>  	for (try = 0; try < 3; try++) {
> -		status = I915_READ_NOTRACE(ch_ctl);
> +		status = intel_uncore_read_notrace(uncore, ch_ctl);
>  		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
>  			break;
>  		msleep(1);
> @@ -1269,7 +1270,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  
>  	if (try == 3) {
>  		static u32 last_status = -1;
> -		const u32 status = I915_READ(ch_ctl);
> +		const u32 status = intel_uncore_read(uncore, ch_ctl);
>  
>  		if (status != last_status) {
>  			WARN(1, "dp_aux_ch not started status 0x%08x\n",
> @@ -1298,21 +1299,23 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  		for (try = 0; try < 5; try++) {
>  			/* Load the send data into the aux channel data registers */
>  			for (i = 0; i < send_bytes; i += 4)
> -				I915_WRITE(ch_data[i >> 2],
> -					   intel_dp_pack_aux(send + i,
> -							     send_bytes - i));
> +				intel_uncore_write(uncore,
> +						   ch_data[i >> 2],
> +						   intel_dp_pack_aux(send + i,
> +								     send_bytes - i));
>  
>  			/* Send the command and wait for it to complete */
> -			I915_WRITE(ch_ctl, send_ctl);
> +			intel_uncore_write(uncore, ch_ctl, send_ctl);
>  
>  			status = intel_dp_aux_wait_done(intel_dp);
>  
>  			/* Clear done status and any errors */
> -			I915_WRITE(ch_ctl,
> -				   status |
> -				   DP_AUX_CH_CTL_DONE |
> -				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -				   DP_AUX_CH_CTL_RECEIVE_ERROR);
> +			intel_uncore_write(uncore,
> +					   ch_ctl,
> +					   status |
> +					   DP_AUX_CH_CTL_DONE |
> +					   DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +					   DP_AUX_CH_CTL_RECEIVE_ERROR);
>  
>  			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
>  			 *   400us delay required for errors and timeouts
> @@ -1375,18 +1378,18 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  		recv_bytes = recv_size;
>  
>  	for (i = 0; i < recv_bytes; i += 4)
> -		intel_dp_unpack_aux(I915_READ(ch_data[i >> 2]),
> +		intel_dp_unpack_aux(intel_uncore_read(uncore, ch_data[i >> 2]),
>  				    recv + i, recv_bytes - i);
>  
>  	ret = recv_bytes;
>  out:
> -	pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
> +	pm_qos_update_request(&i915->pm_qos, PM_QOS_DEFAULT_VALUE);
>  
>  	if (vdd)
>  		edp_panel_vdd_off(intel_dp, false);
>  
>  	pps_unlock(intel_dp, pps_wakeref);
> -	intel_display_power_put_async(dev_priv, aux_domain, aux_wakeref);
> +	intel_display_power_put_async(i915, aux_domain, aux_wakeref);
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_gmbus.c b/drivers/gpu/drm/i915/intel_gmbus.c
> index 5e4c543e82e8..aa88e6e7cc65 100644
> --- a/drivers/gpu/drm/i915/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/intel_gmbus.c
> @@ -186,14 +186,15 @@ static void bxt_gmbus_clock_gating(struct drm_i915_private *dev_priv,
>  
>  static u32 get_reserved(struct intel_gmbus *bus)
>  {
> -	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	struct drm_i915_private *i915 = bus->dev_priv;
> +	struct intel_uncore *uncore = &i915->uncore;
>  	u32 reserved = 0;
>  
>  	/* On most chips, these bits must be preserved in software. */
> -	if (!IS_I830(dev_priv) && !IS_I845G(dev_priv))
> -		reserved = I915_READ_NOTRACE(bus->gpio_reg) &
> -					     (GPIO_DATA_PULLUP_DISABLE |
> -					      GPIO_CLOCK_PULLUP_DISABLE);
> +	if (!IS_I830(i915) && !IS_I845G(i915))
> +		reserved = intel_uncore_read_notrace(uncore, bus->gpio_reg) &
> +			   (GPIO_DATA_PULLUP_DISABLE |
> +			    GPIO_CLOCK_PULLUP_DISABLE);
>  
>  	return reserved;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 07/12] drm/i915: Remove I915_READ8
  2019-06-07 12:08 ` [RFC 07/12] drm/i915: Remove I915_READ8 Tvrtko Ursulin
  2019-06-07 13:11   ` Jani Nikula
@ 2019-06-11  8:58   ` Jani Nikula
  1 sibling, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2019-06-11  8:58 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Only a few call sites remain which have been converted to uncore mmio
> accessors and so the macro can be removed.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  2 --
>  drivers/gpu/drm/i915/intel_crt.c | 41 ++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_pm.c  |  6 ++---
>  3 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b2763721b76d..13815795e197 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2852,8 +2852,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>  #define __I915_REG_OP(op__, dev_priv__, ...) \
>  	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
>  
> -#define I915_READ8(reg__)	  __I915_REG_OP(read8, dev_priv, (reg__))
> -
>  #define I915_READ16(reg__)	   __I915_REG_OP(read16, dev_priv, (reg__))
>  #define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
>  
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index bb56518576a1..3fcf2f84bcce 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -643,6 +643,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  {
>  	struct drm_device *dev = crt->base.base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	u32 save_bclrpat;
>  	u32 save_vtotal;
>  	u32 vtotal, vactive;
> @@ -663,9 +664,9 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  	pipeconf_reg = PIPECONF(pipe);
>  	pipe_dsl_reg = PIPEDSL(pipe);
>  
> -	save_bclrpat = I915_READ(bclrpat_reg);
> -	save_vtotal = I915_READ(vtotal_reg);
> -	vblank = I915_READ(vblank_reg);
> +	save_bclrpat = intel_uncore_read(uncore, bclrpat_reg);
> +	save_vtotal = intel_uncore_read(uncore, vtotal_reg);
> +	vblank = intel_uncore_read(uncore, vblank_reg);
>  
>  	vtotal = ((save_vtotal >> 16) & 0xfff) + 1;
>  	vactive = (save_vtotal & 0x7ff) + 1;
> @@ -674,21 +675,23 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  	vblank_end = ((vblank >> 16) & 0xfff) + 1;
>  
>  	/* Set the border color to purple. */
> -	I915_WRITE(bclrpat_reg, 0x500050);
> +	intel_uncore_write(uncore, bclrpat_reg, 0x500050);
>  
>  	if (!IS_GEN(dev_priv, 2)) {
> -		u32 pipeconf = I915_READ(pipeconf_reg);
> -		I915_WRITE(pipeconf_reg, pipeconf | PIPECONF_FORCE_BORDER);
> -		POSTING_READ(pipeconf_reg);
> +		u32 pipeconf = intel_uncore_read(uncore, pipeconf_reg);
> +		intel_uncore_write(uncore,
> +				   pipeconf_reg,
> +				   pipeconf | PIPECONF_FORCE_BORDER);
> +		intel_uncore_posting_read(uncore, pipeconf_reg);

Just musing, a new intel_uncore_write_post() to combine the two?

Reviewed-by: Jani Nikula <jani.nikula@intel.com>



>  		/* Wait for next Vblank to substitue
>  		 * border color for Color info */
>  		intel_wait_for_vblank(dev_priv, pipe);
> -		st00 = I915_READ8(_VGA_MSR_WRITE);
> +		st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>  		status = ((st00 & (1 << 4)) != 0) ?
>  			connector_status_connected :
>  			connector_status_disconnected;
>  
> -		I915_WRITE(pipeconf_reg, pipeconf);
> +		intel_uncore_write(uncore, pipeconf_reg, pipeconf);
>  	} else {
>  		bool restore_vblank = false;
>  		int count, detect;
> @@ -702,9 +705,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  			u32 vsync_start = (vsync & 0xffff) + 1;
>  
>  			vblank_start = vsync_start;
> -			I915_WRITE(vblank_reg,
> -				   (vblank_start - 1) |
> -				   ((vblank_end - 1) << 16));
> +			intel_uncore_write(uncore,
> +					   vblank_reg,
> +					   (vblank_start - 1) |
> +					   ((vblank_end - 1) << 16));
>  			restore_vblank = true;
>  		}
>  		/* sample in the vertical border, selecting the larger one */
> @@ -716,9 +720,10 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  		/*
>  		 * Wait for the border to be displayed
>  		 */
> -		while (I915_READ(pipe_dsl_reg) >= vactive)
> +		while (intel_uncore_read(uncore, pipe_dsl_reg) >= vactive)
>  			;
> -		while ((dsl = I915_READ(pipe_dsl_reg)) <= vsample)
> +		while ((dsl = intel_uncore_read(uncore, pipe_dsl_reg)) <=
> +		       vsample)
>  			;
>  		/*
>  		 * Watch ST00 for an entire scanline
> @@ -728,14 +733,14 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  		do {
>  			count++;
>  			/* Read the ST00 VGA status register */
> -			st00 = I915_READ8(_VGA_MSR_WRITE);
> +			st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>  			if (st00 & (1 << 4))
>  				detect++;
> -		} while ((I915_READ(pipe_dsl_reg) == dsl));
> +		} while ((intel_uncore_read(uncore, pipe_dsl_reg) == dsl));
>  
>  		/* restore vblank if necessary */
>  		if (restore_vblank)
> -			I915_WRITE(vblank_reg, vblank);
> +			intel_uncore_write(uncore, vblank_reg, vblank);
>  		/*
>  		 * If more than 3/4 of the scanline detected a monitor,
>  		 * then it is assumed to be present. This works even on i830,
> @@ -748,7 +753,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>  	}
>  
>  	/* Restore previous settings */
> -	I915_WRITE(bclrpat_reg, save_bclrpat);
> +	intel_uncore_write(uncore, bclrpat_reg, save_bclrpat);
>  
>  	return status;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d7272d4ff258..93e411e6ad19 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -8160,15 +8160,15 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>  	return val;
>  }
>  
> -unsigned long i915_mch_val(struct drm_i915_private *dev_priv)
> +unsigned long i915_mch_val(struct drm_i915_private *i915)
>  {
>  	unsigned long m, x, b;
>  	u32 tsfs;
>  
> -	tsfs = I915_READ(TSFS);
> +	tsfs = intel_uncore_read(&i915->uncore, TSFS);
>  
>  	m = ((tsfs & TSFS_SLOPE_MASK) >> TSFS_SLOPE_SHIFT);
> -	x = I915_READ8(TR1);
> +	x = intel_uncore_read8(&i915->uncore, TR1);
>  
>  	b = tsfs & TSFS_INTR_MASK;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC 07/12] drm/i915: Remove I915_READ8
  2019-06-11  8:47         ` Tvrtko Ursulin
@ 2019-06-11  8:59           ` Jani Nikula
  0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2019-06-11  8:59 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

On Tue, 11 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> Depending on the opinion from Ville I can either ask for upgrade to r-b, 

r-b,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2019-06-11  8:56 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-07 12:08 [RFC 00/12] Legacy mmio accessor macro pruning Tvrtko Ursulin
2019-06-07 12:08 ` [RFC 01/12] drm/i915: Eliminate unused mmio accessors Tvrtko Ursulin
2019-06-07 12:08 ` [RFC 02/12] drm/i915: Convert i915_reg_read_ioctl to use explicit " Tvrtko Ursulin
2019-06-07 12:08 ` [RFC 03/12] drm/i915: Convert icl_get_stolen_reserved to uncore " Tvrtko Ursulin
2019-06-07 12:08 ` [RFC 04/12] drm/i915: Convert gem_record_fences " Tvrtko Ursulin
2019-06-07 12:08 ` [RFC 05/12] drm/i915: Convert intel_read_wm_latency " Tvrtko Ursulin
2019-06-07 12:08 ` [RFC 06/12] drm/i915: Remove I915_READ64 and I915_READ64_32x2 Tvrtko Ursulin
2019-06-07 12:44   ` Jani Nikula
2019-06-07 12:08 ` [RFC 07/12] drm/i915: Remove I915_READ8 Tvrtko Ursulin
2019-06-07 13:11   ` Jani Nikula
2019-06-07 13:19     ` Tvrtko Ursulin
2019-06-07 13:44       ` Jani Nikula
2019-06-11  8:47         ` Tvrtko Ursulin
2019-06-11  8:59           ` Jani Nikula
2019-06-11  8:58   ` Jani Nikula
2019-06-07 12:08 ` [RFC 08/12] drm/i915: Remove I915_POSTING_READ_FW Tvrtko Ursulin
2019-06-07 12:48   ` Jani Nikula
2019-06-07 12:08 ` [RFC 09/12] drm/i915: Remove POSTING_READ16 Tvrtko Ursulin
2019-06-07 12:49   ` Jani Nikula
2019-06-07 12:08 ` [RFC 10/12] drm/i915: Remove I915_WRITE_NOTRACE Tvrtko Ursulin
2019-06-07 12:50   ` Jani Nikula
2019-06-07 12:08 ` [RFC 11/12] drm/i915: Remove I915_READ_NOTRACE Tvrtko Ursulin
2019-06-11  8:57   ` Jani Nikula
2019-06-07 12:08 ` [RFC 12/12] drm/i915: Remove I915_READ16 and I915_WRITE16 Tvrtko Ursulin
2019-06-07 12:59   ` Jani Nikula
2019-06-07 14:36 ` ✗ Fi.CI.CHECKPATCH: warning for Legacy mmio accessor macro pruning Patchwork
2019-06-07 15:04 ` ✓ Fi.CI.BAT: success " Patchwork
2019-06-10  9:57 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.