All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/4] Display uncore
@ 2019-06-24 20:31 Daniele Ceraolo Spurio
  2019-06-24 20:31 ` [RFC 1/4] drm/i915: split out uncore_mmio_debug Daniele Ceraolo Spurio
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-06-24 20:31 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

More focused proposal for display/GT uncore split.
A few notes/considerations:

- I've copied the MMIO ranges check in patch 3 from a patch from Ville
  that did something similar in regards to splitting GT/DE register
  access. I'm seeing some check failures on gen 3 and 4 [1], so I'd
  appreciate it if someone could double check those ranges since I'm
  not familiar with those gens. For the same reason I don't expect BAT
  to pass.

- Instead of doing a range check to find erroneous accesses, I
  considered using a new structure for display registers (wrapping
  i915_reg_t) and therefore force a compile-time check, but the sheer
  amount of register definitions that would need updating makes this
  hard to merge together with the switch to the new functions without
  hitting conflicts. If there is interest in this option, I'll put
  something out after all the display register accesses have
  transitioned to the new accessors.

- When MMIO debug is enabled, the accesses from GT and DE are still
  serialized because the HW debug infrastructure is shared. Since our
  testing defaults the MMIO debug on, we won't really test parallel
  accesses. After the series settles down I'm going to submit a CI run
  with MMIO debug off, but maybe we should update some tests to run
  with MMIO debug disabled. Thoughts? 

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>

[1] https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_4491/?

Daniele Ceraolo Spurio (4):
  drm/i915: split out uncore_mmio_debug
  drm/i915: rework mmio debug stop/start
  drm/i915: introduce display_uncore
  drm/i915: convert intel_hdmi to display reg accessors

 drivers/gpu/drm/i915/display/intel_hdmi.c | 278 ++++++++++------------
 drivers/gpu/drm/i915/i915_debugfs.c       |   2 +-
 drivers/gpu/drm/i915/i915_drv.c           |  20 +-
 drivers/gpu/drm/i915/i915_drv.h           |  59 +++++
 drivers/gpu/drm/i915/intel_uncore.c       | 100 +++++---
 drivers/gpu/drm/i915/intel_uncore.h       |  18 +-
 6 files changed, 292 insertions(+), 185 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] 15+ messages in thread

* [RFC 1/4] drm/i915: split out uncore_mmio_debug
  2019-06-24 20:31 [RFC 0/4] Display uncore Daniele Ceraolo Spurio
@ 2019-06-24 20:31 ` Daniele Ceraolo Spurio
  2019-06-26 10:02   ` Chris Wilson
  2019-06-24 20:31 ` [RFC 2/4] drm/i915: rework mmio debug stop/start Daniele Ceraolo Spurio
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-06-24 20:31 UTC (permalink / raw)
  To: intel-gfx

Multiple uncore structures will share the debug infrastructure, so
move it to a common place and add extra locking around it.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c     |  1 +
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_uncore.c | 54 +++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_uncore.h |  9 ++++-
 4 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e1817f89f5d5..1c4cfdb04867 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -901,6 +901,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
 
 	intel_device_info_subplatform_init(dev_priv);
 
+	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
 	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
 
 	spin_lock_init(&dev_priv->irq_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 62e7c5e8aee5..a53b65d78159 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1341,6 +1341,7 @@ struct drm_i915_private {
 	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
 
 	struct intel_uncore uncore;
+	struct intel_uncore_mmio_debug mmio_debug;
 
 	struct i915_virtual_gpu vgpu;
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 68d54e126d79..f2dfaccd6614 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -34,6 +34,13 @@
 
 #define __raw_posting_read(...) ((void)__raw_uncore_read32(__VA_ARGS__))
 
+void
+intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug)
+{
+	spin_lock_init(&mmio_debug->lock);
+	mmio_debug->unclaimed_mmio_check = 1;
+}
+
 static const char * const forcewake_domain_names[] = {
 	"render",
 	"blitter",
@@ -473,6 +480,8 @@ check_for_unclaimed_mmio(struct intel_uncore *uncore)
 {
 	bool ret = false;
 
+	lockdep_assert_held(&uncore->debug->lock);
+
 	if (intel_uncore_has_fpga_dbg_unclaimed(uncore))
 		ret |= fpga_check_for_unclaimed_mmio(uncore);
 
@@ -605,18 +614,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
 void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
 {
 	spin_lock_irq(&uncore->lock);
+	spin_lock(&uncore->debug->lock);
 	if (!uncore->user_forcewake.count++) {
 		intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
 
 		/* Save and disable mmio debugging for the user bypass */
 		uncore->user_forcewake.saved_mmio_check =
-			uncore->unclaimed_mmio_check;
+			uncore->debug->unclaimed_mmio_check;
 		uncore->user_forcewake.saved_mmio_debug =
 			i915_modparams.mmio_debug;
 
-		uncore->unclaimed_mmio_check = 0;
+		uncore->debug->unclaimed_mmio_check = 0;
 		i915_modparams.mmio_debug = 0;
 	}
+	spin_unlock(&uncore->debug->lock);
 	spin_unlock_irq(&uncore->lock);
 }
 
@@ -630,18 +641,20 @@ void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
 void intel_uncore_forcewake_user_put(struct intel_uncore *uncore)
 {
 	spin_lock_irq(&uncore->lock);
+	spin_lock(&uncore->debug->lock);
 	if (!--uncore->user_forcewake.count) {
-		if (intel_uncore_unclaimed_mmio(uncore))
+		if (check_for_unclaimed_mmio(uncore))
 			dev_info(uncore->i915->drm.dev,
 				 "Invalid mmio detected during user access\n");
 
-		uncore->unclaimed_mmio_check =
+		uncore->debug->unclaimed_mmio_check =
 			uncore->user_forcewake.saved_mmio_check;
 		i915_modparams.mmio_debug =
 			uncore->user_forcewake.saved_mmio_debug;
 
 		intel_uncore_forcewake_put__locked(uncore, FORCEWAKE_ALL);
 	}
+	spin_unlock(&uncore->debug->lock);
 	spin_unlock_irq(&uncore->lock);
 }
 
@@ -1059,7 +1072,16 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
 	if (likely(!i915_modparams.mmio_debug))
 		return;
 
+	/* interrupts are disabled and re-enabled around uncore->lock usage */
+	lockdep_assert_held(&uncore->lock);
+
+	if (before)
+		spin_lock(&uncore->debug->lock);
+
 	__unclaimed_reg_debug(uncore, reg, read, before);
+
+	if (!before)
+		spin_unlock(&uncore->debug->lock);
 }
 
 #define GEN2_READ_HEADER(x) \
@@ -1579,6 +1601,7 @@ void intel_uncore_init_early(struct intel_uncore *uncore,
 	spin_lock_init(&uncore->lock);
 	uncore->i915 = i915;
 	uncore->rpm = &i915->runtime_pm;
+	uncore->debug = &i915->mmio_debug;
 }
 
 static void uncore_raw_init(struct intel_uncore *uncore)
@@ -1604,7 +1627,6 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
 	ret = intel_uncore_fw_domains_init(uncore);
 	if (ret)
 		return ret;
-
 	forcewake_early_sanitize(uncore, 0);
 
 	if (IS_GEN_RANGE(i915, 6, 7)) {
@@ -1653,8 +1675,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
 	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
 		uncore->flags |= UNCORE_HAS_FORCEWAKE;
 
-	uncore->unclaimed_mmio_check = 1;
-
 	if (!intel_uncore_has_forcewake(uncore)) {
 		uncore_raw_init(uncore);
 	} else {
@@ -1679,7 +1699,7 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
 		uncore->flags |= UNCORE_HAS_FIFO;
 
 	/* clear out unclaimed reg detection bit */
-	if (check_for_unclaimed_mmio(uncore))
+	if (intel_uncore_unclaimed_mmio(uncore))
 		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
 
 	return 0;
@@ -1924,7 +1944,13 @@ int __intel_wait_for_register(struct intel_uncore *uncore,
 
 bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore)
 {
-	return check_for_unclaimed_mmio(uncore);
+	bool ret;
+
+	spin_lock_irq(&uncore->debug->lock);
+	ret = check_for_unclaimed_mmio(uncore);
+	spin_unlock_irq(&uncore->debug->lock);
+
+	return ret;
 }
 
 bool
@@ -1932,24 +1958,24 @@ intel_uncore_arm_unclaimed_mmio_detection(struct intel_uncore *uncore)
 {
 	bool ret = false;
 
-	spin_lock_irq(&uncore->lock);
+	spin_lock_irq(&uncore->debug->lock);
 
-	if (unlikely(uncore->unclaimed_mmio_check <= 0))
+	if (unlikely(uncore->debug->unclaimed_mmio_check <= 0))
 		goto out;
 
-	if (unlikely(intel_uncore_unclaimed_mmio(uncore))) {
+	if (unlikely(check_for_unclaimed_mmio(uncore))) {
 		if (!i915_modparams.mmio_debug) {
 			DRM_DEBUG("Unclaimed register detected, "
 				  "enabling oneshot unclaimed register reporting. "
 				  "Please use i915.mmio_debug=N for more information.\n");
 			i915_modparams.mmio_debug++;
 		}
-		uncore->unclaimed_mmio_check--;
+		uncore->debug->unclaimed_mmio_check--;
 		ret = true;
 	}
 
 out:
-	spin_unlock_irq(&uncore->lock);
+	spin_unlock_irq(&uncore->debug->lock);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 7108475d9b24..d1b12b3b8353 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -36,6 +36,11 @@ struct drm_i915_private;
 struct intel_runtime_pm;
 struct intel_uncore;
 
+struct intel_uncore_mmio_debug {
+	spinlock_t lock; /** lock is also taken in irq contexts. */
+	int unclaimed_mmio_check;
+};
+
 enum forcewake_domain_id {
 	FW_DOMAIN_ID_RENDER = 0,
 	FW_DOMAIN_ID_BLITTER,
@@ -143,7 +148,7 @@ struct intel_uncore {
 		int saved_mmio_debug;
 	} user_forcewake;
 
-	int unclaimed_mmio_check;
+	struct intel_uncore_mmio_debug *debug;
 };
 
 /* Iterate over initialised fw domains */
@@ -178,6 +183,8 @@ intel_uncore_has_fifo(const struct intel_uncore *uncore)
 	return uncore->flags & UNCORE_HAS_FIFO;
 }
 
+void
+intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug);
 void intel_uncore_init_early(struct intel_uncore *uncore,
 			     struct drm_i915_private *i915);
 int intel_uncore_init_mmio(struct intel_uncore *uncore);
-- 
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] 15+ messages in thread

* [RFC 2/4] drm/i915: rework mmio debug stop/start
  2019-06-24 20:31 [RFC 0/4] Display uncore Daniele Ceraolo Spurio
  2019-06-24 20:31 ` [RFC 1/4] drm/i915: split out uncore_mmio_debug Daniele Ceraolo Spurio
@ 2019-06-24 20:31 ` Daniele Ceraolo Spurio
  2019-06-24 20:31 ` [RFC 3/4] drm/i915: introduce display_uncore Daniele Ceraolo Spurio
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-06-24 20:31 UTC (permalink / raw)
  To: intel-gfx

Since we now have a separate object, it is cleaner to have dedicated
functions working on the object to stop and restart the mmio debug.
Apart from the cosmetic changes, this patch introduces 2 functional
updates:

- All calls to check_for_unclaimed_mmio will now return false when
  the debug is suspended, not just the ones that are active only when
  i915_modparams.mmio_debug is set. If we don't trust the result of the
  check while a user is doing mmio access then we shouldn't attempt the
  check anywhere.

- i915_modparams.mmio_debug is not save/restored anymore around user
  access. The value is now never touched by the kernel while debug is
  disabled so no need for save/restore.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
 drivers/gpu/drm/i915/i915_drv.c     |  2 +-
 drivers/gpu/drm/i915/intel_uncore.c | 43 ++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_uncore.h |  9 ++----
 4 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index eeecdad0e3ca..1eb72bd74ab9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1218,7 +1218,7 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
 	unsigned int tmp;
 
 	seq_printf(m, "user.bypass_count = %u\n",
-		   uncore->user_forcewake.count);
+		   uncore->user_forcewake_count);
 
 	for_each_fw_domain(fw_domain, uncore, tmp)
 		seq_printf(m, "%s.wake_count = %u\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1c4cfdb04867..2d6c643dde51 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2204,7 +2204,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
 
 out:
 	enable_rpm_wakeref_asserts(rpm);
-	if (!dev_priv->uncore.user_forcewake.count)
+	if (!dev_priv->uncore.user_forcewake_count)
 		intel_runtime_pm_cleanup(rpm);
 
 	return ret;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f2dfaccd6614..1905fd94dd3c 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -41,6 +41,25 @@ intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug)
 	mmio_debug->unclaimed_mmio_check = 1;
 }
 
+void uncore_mmio_debug_suspend(struct intel_uncore_mmio_debug *mmio_debug)
+{
+	lockdep_assert_held(&mmio_debug->lock);
+
+	/* Save and disable mmio debugging for the user bypass */
+	if (!mmio_debug->suspend_count++) {
+		mmio_debug->saved_mmio_check = mmio_debug->unclaimed_mmio_check;
+		mmio_debug->unclaimed_mmio_check = 0;
+	}
+}
+
+void uncore_mmio_debug_resume(struct intel_uncore_mmio_debug *mmio_debug)
+{
+	lockdep_assert_held(&mmio_debug->lock);
+
+	if (!--mmio_debug->suspend_count)
+		mmio_debug->unclaimed_mmio_check = mmio_debug->saved_mmio_check;
+}
+
 static const char * const forcewake_domain_names[] = {
 	"render",
 	"blitter",
@@ -482,6 +501,9 @@ check_for_unclaimed_mmio(struct intel_uncore *uncore)
 
 	lockdep_assert_held(&uncore->debug->lock);
 
+	if (uncore->debug->suspend_count)
+		return false;
+
 	if (intel_uncore_has_fpga_dbg_unclaimed(uncore))
 		ret |= fpga_check_for_unclaimed_mmio(uncore);
 
@@ -615,17 +637,9 @@ void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
 {
 	spin_lock_irq(&uncore->lock);
 	spin_lock(&uncore->debug->lock);
-	if (!uncore->user_forcewake.count++) {
+	if (!uncore->user_forcewake_count++) {
 		intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
-
-		/* Save and disable mmio debugging for the user bypass */
-		uncore->user_forcewake.saved_mmio_check =
-			uncore->debug->unclaimed_mmio_check;
-		uncore->user_forcewake.saved_mmio_debug =
-			i915_modparams.mmio_debug;
-
-		uncore->debug->unclaimed_mmio_check = 0;
-		i915_modparams.mmio_debug = 0;
+		uncore_mmio_debug_suspend(uncore->debug);
 	}
 	spin_unlock(&uncore->debug->lock);
 	spin_unlock_irq(&uncore->lock);
@@ -642,16 +656,13 @@ void intel_uncore_forcewake_user_put(struct intel_uncore *uncore)
 {
 	spin_lock_irq(&uncore->lock);
 	spin_lock(&uncore->debug->lock);
-	if (!--uncore->user_forcewake.count) {
+	if (!--uncore->user_forcewake_count) {
+		uncore_mmio_debug_resume(uncore->debug);
+
 		if (check_for_unclaimed_mmio(uncore))
 			dev_info(uncore->i915->drm.dev,
 				 "Invalid mmio detected during user access\n");
 
-		uncore->debug->unclaimed_mmio_check =
-			uncore->user_forcewake.saved_mmio_check;
-		i915_modparams.mmio_debug =
-			uncore->user_forcewake.saved_mmio_debug;
-
 		intel_uncore_forcewake_put__locked(uncore, FORCEWAKE_ALL);
 	}
 	spin_unlock(&uncore->debug->lock);
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index d1b12b3b8353..3969dd12208b 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -39,6 +39,8 @@ struct intel_uncore;
 struct intel_uncore_mmio_debug {
 	spinlock_t lock; /** lock is also taken in irq contexts. */
 	int unclaimed_mmio_check;
+	int saved_mmio_check;
+	u32 suspend_count;
 };
 
 enum forcewake_domain_id {
@@ -141,12 +143,7 @@ struct intel_uncore {
 		u32 __iomem *reg_ack;
 	} *fw_domain[FW_DOMAIN_ID_COUNT];
 
-	struct {
-		unsigned int count;
-
-		int saved_mmio_check;
-		int saved_mmio_debug;
-	} user_forcewake;
+	unsigned int user_forcewake_count;
 
 	struct intel_uncore_mmio_debug *debug;
 };
-- 
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] 15+ messages in thread

* [RFC 3/4] drm/i915: introduce display_uncore
  2019-06-24 20:31 [RFC 0/4] Display uncore Daniele Ceraolo Spurio
  2019-06-24 20:31 ` [RFC 1/4] drm/i915: split out uncore_mmio_debug Daniele Ceraolo Spurio
  2019-06-24 20:31 ` [RFC 2/4] drm/i915: rework mmio debug stop/start Daniele Ceraolo Spurio
@ 2019-06-24 20:31 ` Daniele Ceraolo Spurio
  2019-06-26 18:42   ` Ville Syrjälä
  2019-06-24 20:31 ` [RFC 4/4] drm/i915: convert intel_hdmi to display reg accessors Daniele Ceraolo Spurio
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-06-24 20:31 UTC (permalink / raw)
  To: intel-gfx

A forcewake-less uncore to be used to decouple GT accesses from display
ones to avoid serializing them when there is no need.

New accessors that implicitly use the new uncore have also been added.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c     | 17 ++++++++-
 drivers/gpu/drm/i915/i915_drv.h     | 58 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uncore.c |  9 ++++-
 3 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2d6c643dde51..e22c0a6b3992 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -903,6 +903,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
 
 	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
 	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
+	intel_uncore_init_early(&dev_priv->de_uncore, dev_priv);
 
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->gpu_error.lock);
@@ -1001,6 +1002,10 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
 	if (ret < 0)
 		goto err_bridge;
 
+	ret = intel_uncore_init_mmio(&dev_priv->de_uncore);
+	if (ret < 0)
+		goto err_uncore;
+
 	/* Try to make sure MCHBAR is enabled before poking at it */
 	intel_setup_mchbar(dev_priv);
 
@@ -1012,14 +1017,16 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
 
 	ret = intel_engines_init_mmio(dev_priv);
 	if (ret)
-		goto err_uncore;
+		goto err_mchbar;
 
 	i915_gem_init_mmio(dev_priv);
 
 	return 0;
 
-err_uncore:
+err_mchbar:
 	intel_teardown_mchbar(dev_priv);
+	intel_uncore_fini_mmio(&dev_priv->de_uncore);
+err_uncore:
 	intel_uncore_fini_mmio(&dev_priv->uncore);
 err_bridge:
 	pci_dev_put(dev_priv->bridge_dev);
@@ -1034,6 +1041,7 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
 static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
 {
 	intel_teardown_mchbar(dev_priv);
+	intel_uncore_fini_mmio(&dev_priv->de_uncore);
 	intel_uncore_fini_mmio(&dev_priv->uncore);
 	pci_dev_put(dev_priv->bridge_dev);
 }
@@ -2166,6 +2174,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
 
 	i915_gem_suspend_late(dev_priv);
 
+	intel_uncore_suspend(&dev_priv->de_uncore);
 	intel_uncore_suspend(&dev_priv->uncore);
 
 	intel_power_domains_suspend(dev_priv,
@@ -2363,6 +2372,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
 			  ret);
 
 	intel_uncore_resume_early(&dev_priv->uncore);
+	intel_uncore_resume_early(&dev_priv->de_uncore);
 
 	intel_gt_check_and_clear_faults(&dev_priv->gt);
 
@@ -2937,6 +2947,7 @@ static int intel_runtime_suspend(struct device *kdev)
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
 
+	intel_uncore_suspend(&dev_priv->de_uncore);
 	intel_uncore_suspend(&dev_priv->uncore);
 
 	ret = 0;
@@ -2955,6 +2966,7 @@ static int intel_runtime_suspend(struct device *kdev)
 	if (ret) {
 		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
 		intel_uncore_runtime_resume(&dev_priv->uncore);
+		intel_uncore_runtime_resume(&dev_priv->de_uncore);
 
 		intel_runtime_pm_enable_interrupts(dev_priv);
 
@@ -3053,6 +3065,7 @@ static int intel_runtime_resume(struct device *kdev)
 	}
 
 	intel_uncore_runtime_resume(&dev_priv->uncore);
+	intel_uncore_runtime_resume(&dev_priv->de_uncore);
 
 	intel_runtime_pm_enable_interrupts(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a53b65d78159..c554b7697bdc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1341,6 +1341,7 @@ struct drm_i915_private {
 	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
 
 	struct intel_uncore uncore;
+	struct intel_uncore de_uncore;
 	struct intel_uncore_mmio_debug mmio_debug;
 
 	struct i915_virtual_gpu vgpu;
@@ -2746,6 +2747,63 @@ 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__))
 
+/*
+ * The following are mmio-accessors that use an independent lock and skip all
+ * the forcewake logic, to be used to access display registers, which are
+ * outside the GT forcewake wells.
+ */
+static inline void
+intel_assert_register_is_display(struct drm_i915_private *i915, i915_reg_t reg)
+{
+#ifdef CONFIG_DRM_I915_DEBUG_MMIO
+	u32 offset = i915_mmio_reg_offset(reg);
+	bool is_de_register;
+
+	if (INTEL_GEN(i915) >= 5) {
+		is_de_register = offset >= 0x40000 &&
+			(INTEL_GEN(i915) < 11 || offset < 0x1c0000);
+	} else {
+		is_de_register =
+			(offset >= 0x5000 && offset <= 0x5fff) ||
+			(offset >= 0x6000 && offset <= 0x6fff) ||
+			(offset >= 0xa000 && offset <= 0xafff) ||
+			offset >= 0x30000;
+	}
+
+	WARN_ONCE(!is_de_register,
+		  "display reg access function used for non-display reg 0x%08x\n",
+		  offset);
+#endif
+}
+
+static inline u32
+intel_de_read(struct drm_i915_private *i915, i915_reg_t reg)
+{
+	intel_assert_register_is_display(i915, reg);
+	return intel_uncore_read(&i915->de_uncore, reg);
+}
+
+static inline void
+intel_de_posting_read(struct drm_i915_private *i915, i915_reg_t reg)
+{
+	intel_assert_register_is_display(i915, reg);
+	intel_uncore_posting_read(&i915->de_uncore, reg);
+}
+
+static inline void
+intel_de_write(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
+{
+	intel_assert_register_is_display(i915, reg);
+	intel_uncore_write(&i915->de_uncore, reg, val);
+}
+
+static inline void
+intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 set)
+{
+	intel_assert_register_is_display(i915, reg);
+	intel_uncore_rmw(&i915->de_uncore, reg, clear, set);
+}
+
 /* "Broadcast RGB" property */
 #define INTEL_BROADCAST_RGB_AUTO 0
 #define INTEL_BROADCAST_RGB_FULL 1
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 1905fd94dd3c..94e153acb0af 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1674,6 +1674,12 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
 	return 0;
 }
 
+static bool
+intel_uncore_is_display(const struct intel_uncore *uncore)
+{
+	return uncore == &uncore->i915->de_uncore;
+}
+
 int intel_uncore_init_mmio(struct intel_uncore *uncore)
 {
 	struct drm_i915_private *i915 = uncore->i915;
@@ -1683,7 +1689,8 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
 	if (ret)
 		return ret;
 
-	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
+	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915) &&
+	    !intel_uncore_is_display(uncore))
 		uncore->flags |= UNCORE_HAS_FORCEWAKE;
 
 	if (!intel_uncore_has_forcewake(uncore)) {
-- 
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] 15+ messages in thread

* [RFC 4/4] drm/i915: convert intel_hdmi to display reg accessors
  2019-06-24 20:31 [RFC 0/4] Display uncore Daniele Ceraolo Spurio
                   ` (2 preceding siblings ...)
  2019-06-24 20:31 ` [RFC 3/4] drm/i915: introduce display_uncore Daniele Ceraolo Spurio
@ 2019-06-24 20:31 ` Daniele Ceraolo Spurio
  2019-06-24 20:43 ` ✗ Fi.CI.SPARSE: warning for Display uncore (rev2) Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-06-24 20:31 UTC (permalink / raw)
  To: intel-gfx

As an example of usage of the new accessors.

Changes done mechanically with some manual post-processing to use
rmw where appropriate and fix line length and formatting.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 278 ++++++++++------------
 1 file changed, 132 insertions(+), 146 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 0ebec69bbbfc..33ad59361d54 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -72,7 +72,7 @@ assert_hdmi_port_disabled(struct intel_hdmi *intel_hdmi)
 
 	enabled_bits = HAS_DDI(dev_priv) ? DDI_BUF_CTL_ENABLE : SDVO_ENABLE;
 
-	WARN(I915_READ(intel_hdmi->hdmi_reg) & enabled_bits,
+	WARN(intel_de_read(dev_priv, intel_hdmi->hdmi_reg) & enabled_bits,
 	     "HDMI port enabled, expecting disabled\n");
 }
 
@@ -80,7 +80,7 @@ static void
 assert_hdmi_transcoder_func_disabled(struct drm_i915_private *dev_priv,
 				     enum transcoder cpu_transcoder)
 {
-	WARN(I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder)) &
+	WARN(intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder)) &
 	     TRANS_DDI_FUNC_ENABLE,
 	     "HDMI transcoder function enabled, expecting disabled\n");
 }
@@ -208,7 +208,7 @@ static void g4x_write_infoframe(struct intel_encoder *encoder,
 {
 	const u32 *data = frame;
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	u32 val = I915_READ(VIDEO_DIP_CTL);
+	u32 val = intel_de_read(dev_priv, VIDEO_DIP_CTL);
 	int i;
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
@@ -218,22 +218,22 @@ static void g4x_write_infoframe(struct intel_encoder *encoder,
 
 	val &= ~g4x_infoframe_enable(type);
 
-	I915_WRITE(VIDEO_DIP_CTL, val);
+	intel_de_write(dev_priv, VIDEO_DIP_CTL, val);
 
 	for (i = 0; i < len; i += 4) {
-		I915_WRITE(VIDEO_DIP_DATA, *data);
+		intel_de_write(dev_priv, VIDEO_DIP_DATA, *data);
 		data++;
 	}
 	/* Write every possible data byte to force correct ECC calculation. */
 	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
-		I915_WRITE(VIDEO_DIP_DATA, 0);
+		intel_de_write(dev_priv, VIDEO_DIP_DATA, 0);
 
 	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
-	I915_WRITE(VIDEO_DIP_CTL, val);
-	POSTING_READ(VIDEO_DIP_CTL);
+	intel_de_write(dev_priv, VIDEO_DIP_CTL, val);
+	intel_de_posting_read(dev_priv, VIDEO_DIP_CTL);
 }
 
 static void g4x_read_infoframe(struct intel_encoder *encoder,
@@ -242,25 +242,22 @@ static void g4x_read_infoframe(struct intel_encoder *encoder,
 			       void *frame, ssize_t len)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	u32 val, *data = frame;
+	u32 *data = frame;
 	int i;
 
-	val = I915_READ(VIDEO_DIP_CTL);
-
-	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
-	val |= g4x_infoframe_index(type);
-
-	I915_WRITE(VIDEO_DIP_CTL, val);
+	intel_de_rmw(dev_priv, VIDEO_DIP_CTL,
+		     VIDEO_DIP_SELECT_MASK | 0xf, /* clear DIP data offset */
+		     g4x_infoframe_index(type));
 
 	for (i = 0; i < len; i += 4)
-		*data++ = I915_READ(VIDEO_DIP_DATA);
+		*data++ = intel_de_read(dev_priv, VIDEO_DIP_DATA);
 }
 
 static u32 g4x_infoframes_enabled(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	u32 val = I915_READ(VIDEO_DIP_CTL);
+	u32 val = intel_de_read(dev_priv, VIDEO_DIP_CTL);
 
 	if ((val & VIDEO_DIP_ENABLE) == 0)
 		return 0;
@@ -281,7 +278,7 @@ static void ibx_write_infoframe(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	i915_reg_t reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 	int i;
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
@@ -291,22 +288,22 @@ static void ibx_write_infoframe(struct intel_encoder *encoder,
 
 	val &= ~g4x_infoframe_enable(type);
 
-	I915_WRITE(reg, val);
+	intel_de_write(dev_priv, reg, val);
 
 	for (i = 0; i < len; i += 4) {
-		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
+		intel_de_write(dev_priv, TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
 	/* Write every possible data byte to force correct ECC calculation. */
 	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
-		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
+		intel_de_write(dev_priv, TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 
 	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 }
 
 static void ibx_read_infoframe(struct intel_encoder *encoder,
@@ -316,18 +313,15 @@ static void ibx_read_infoframe(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	u32 val, *data = frame;
+	u32 *data = frame;
 	int i;
 
-	val = I915_READ(TVIDEO_DIP_CTL(crtc->pipe));
-
-	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
-	val |= g4x_infoframe_index(type);
-
-	I915_WRITE(TVIDEO_DIP_CTL(crtc->pipe), val);
+	intel_de_rmw(dev_priv, TVIDEO_DIP_CTL(crtc->pipe),
+		     VIDEO_DIP_SELECT_MASK | 0xf, /* clear DIP data offset */
+		     g4x_infoframe_index(type));
 
 	for (i = 0; i < len; i += 4)
-		*data++ = I915_READ(TVIDEO_DIP_DATA(crtc->pipe));
+		*data++ = intel_de_read(dev_priv, TVIDEO_DIP_DATA(crtc->pipe));
 }
 
 static u32 ibx_infoframes_enabled(struct intel_encoder *encoder,
@@ -336,7 +330,7 @@ static u32 ibx_infoframes_enabled(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe;
 	i915_reg_t reg = TVIDEO_DIP_CTL(pipe);
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 
 	if ((val & VIDEO_DIP_ENABLE) == 0)
 		return 0;
@@ -358,7 +352,7 @@ static void cpt_write_infoframe(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	i915_reg_t reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 	int i;
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
@@ -371,22 +365,22 @@ static void cpt_write_infoframe(struct intel_encoder *encoder,
 	if (type != HDMI_INFOFRAME_TYPE_AVI)
 		val &= ~g4x_infoframe_enable(type);
 
-	I915_WRITE(reg, val);
+	intel_de_write(dev_priv, reg, val);
 
 	for (i = 0; i < len; i += 4) {
-		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
+		intel_de_write(dev_priv, TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
 	/* Write every possible data byte to force correct ECC calculation. */
 	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
-		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
+		intel_de_write(dev_priv, TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 
 	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 }
 
 static void cpt_read_infoframe(struct intel_encoder *encoder,
@@ -396,18 +390,15 @@ static void cpt_read_infoframe(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	u32 val, *data = frame;
+	u32 *data = frame;
 	int i;
 
-	val = I915_READ(TVIDEO_DIP_CTL(crtc->pipe));
-
-	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
-	val |= g4x_infoframe_index(type);
-
-	I915_WRITE(TVIDEO_DIP_CTL(crtc->pipe), val);
+	intel_de_rmw(dev_priv, TVIDEO_DIP_CTL(crtc->pipe),
+		     VIDEO_DIP_SELECT_MASK | 0xf, /* clear DIP data offset */
+		     g4x_infoframe_index(type));
 
 	for (i = 0; i < len; i += 4)
-		*data++ = I915_READ(TVIDEO_DIP_DATA(crtc->pipe));
+		*data++ = intel_de_read(dev_priv, TVIDEO_DIP_DATA(crtc->pipe));
 }
 
 static u32 cpt_infoframes_enabled(struct intel_encoder *encoder,
@@ -415,7 +406,7 @@ static u32 cpt_infoframes_enabled(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe;
-	u32 val = I915_READ(TVIDEO_DIP_CTL(pipe));
+	u32 val = intel_de_read(dev_priv, TVIDEO_DIP_CTL(pipe));
 
 	if ((val & VIDEO_DIP_ENABLE) == 0)
 		return 0;
@@ -434,7 +425,7 @@ static void vlv_write_infoframe(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	i915_reg_t reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 	int i;
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
@@ -444,22 +435,22 @@ static void vlv_write_infoframe(struct intel_encoder *encoder,
 
 	val &= ~g4x_infoframe_enable(type);
 
-	I915_WRITE(reg, val);
+	intel_de_write(dev_priv, reg, val);
 
 	for (i = 0; i < len; i += 4) {
-		I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
+		intel_de_write(dev_priv, VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
 	/* Write every possible data byte to force correct ECC calculation. */
 	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
-		I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
+		intel_de_write(dev_priv, VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 
 	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 }
 
 static void vlv_read_infoframe(struct intel_encoder *encoder,
@@ -469,18 +460,15 @@ static void vlv_read_infoframe(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	u32 val, *data = frame;
+	u32 *data = frame;
 	int i;
 
-	val = I915_READ(VLV_TVIDEO_DIP_CTL(crtc->pipe));
-
-	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
-	val |= g4x_infoframe_index(type);
-
-	I915_WRITE(VLV_TVIDEO_DIP_CTL(crtc->pipe), val);
+	intel_de_rmw(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe),
+		     VIDEO_DIP_SELECT_MASK | 0xf, /* clear DIP data offset */
+		     g4x_infoframe_index(type));
 
 	for (i = 0; i < len; i += 4)
-		*data++ = I915_READ(VLV_TVIDEO_DIP_DATA(crtc->pipe));
+		*data++ = intel_de_read(dev_priv, VLV_TVIDEO_DIP_DATA(crtc->pipe));
 }
 
 static u32 vlv_infoframes_enabled(struct intel_encoder *encoder,
@@ -488,7 +476,7 @@ static u32 vlv_infoframes_enabled(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe;
-	u32 val = I915_READ(VLV_TVIDEO_DIP_CTL(pipe));
+	u32 val = intel_de_read(dev_priv, VLV_TVIDEO_DIP_CTL(pipe));
 
 	if ((val & VIDEO_DIP_ENABLE) == 0)
 		return 0;
@@ -512,26 +500,26 @@ static void hsw_write_infoframe(struct intel_encoder *encoder,
 	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
 	int data_size;
 	int i;
-	u32 val = I915_READ(ctl_reg);
+	u32 val = intel_de_read(dev_priv, ctl_reg);
 
 	data_size = hsw_dip_data_size(type);
 
 	val &= ~hsw_infoframe_enable(type);
-	I915_WRITE(ctl_reg, val);
+	intel_de_write(dev_priv, ctl_reg, val);
 
 	for (i = 0; i < len; i += 4) {
-		I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
-					    type, i >> 2), *data);
+		intel_de_write(dev_priv, hsw_dip_data_reg(dev_priv, cpu_transcoder,
+							  type, i >> 2), *data);
 		data++;
 	}
 	/* Write every possible data byte to force correct ECC calculation. */
 	for (; i < data_size; i += 4)
-		I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
-					    type, i >> 2), 0);
+		intel_de_write(dev_priv, hsw_dip_data_reg(dev_priv, cpu_transcoder,
+							  type, i >> 2), 0);
 
 	val |= hsw_infoframe_enable(type);
-	I915_WRITE(ctl_reg, val);
-	POSTING_READ(ctl_reg);
+	intel_de_write(dev_priv, ctl_reg, val);
+	intel_de_posting_read(dev_priv, ctl_reg);
 }
 
 static void hsw_read_infoframe(struct intel_encoder *encoder,
@@ -544,18 +532,20 @@ static void hsw_read_infoframe(struct intel_encoder *encoder,
 	u32 val, *data = frame;
 	int i;
 
-	val = I915_READ(HSW_TVIDEO_DIP_CTL(cpu_transcoder));
+	val = intel_de_read(dev_priv, HSW_TVIDEO_DIP_CTL(cpu_transcoder));
 
 	for (i = 0; i < len; i += 4)
-		*data++ = I915_READ(hsw_dip_data_reg(dev_priv, cpu_transcoder,
-						     type, i >> 2));
+		*data++ = intel_de_read(dev_priv,
+					hsw_dip_data_reg(dev_priv, cpu_transcoder,
+							 type, i >> 2));
 }
 
 static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	u32 val = I915_READ(HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder));
+	u32 val = intel_de_read(dev_priv,
+				HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder));
 	u32 mask;
 
 	mask = (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
@@ -841,7 +831,7 @@ static void g4x_set_infoframes(struct intel_encoder *encoder,
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_hdmi *intel_hdmi = &intel_dig_port->hdmi;
 	i915_reg_t reg = VIDEO_DIP_CTL;
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 	u32 port = VIDEO_DIP_PORT(encoder->port);
 
 	assert_hdmi_port_disabled(intel_hdmi);
@@ -867,8 +857,8 @@ static void g4x_set_infoframes(struct intel_encoder *encoder,
 		}
 		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
 			 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_SPD);
-		I915_WRITE(reg, val);
-		POSTING_READ(reg);
+		intel_de_write(dev_priv, reg, val);
+		intel_de_posting_read(dev_priv, reg);
 		return;
 	}
 
@@ -886,8 +876,8 @@ static void g4x_set_infoframes(struct intel_encoder *encoder,
 	val &= ~(VIDEO_DIP_ENABLE_AVI |
 		 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_SPD);
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_AVI,
@@ -964,7 +954,7 @@ static bool intel_hdmi_set_gcp_infoframe(struct intel_encoder *encoder,
 	else
 		return false;
 
-	I915_WRITE(reg, crtc_state->infoframes.gcp);
+	intel_de_write(dev_priv, reg, crtc_state->infoframes.gcp);
 
 	return true;
 }
@@ -989,7 +979,7 @@ void intel_hdmi_read_gcp_infoframe(struct intel_encoder *encoder,
 	else
 		return;
 
-	crtc_state->infoframes.gcp = I915_READ(reg);
+	crtc_state->infoframes.gcp = intel_de_read(dev_priv, reg);
 }
 
 static void intel_hdmi_compute_gcp_infoframe(struct intel_encoder *encoder,
@@ -1024,7 +1014,7 @@ static void ibx_set_infoframes(struct intel_encoder *encoder,
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_hdmi *intel_hdmi = &intel_dig_port->hdmi;
 	i915_reg_t reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 	u32 port = VIDEO_DIP_PORT(encoder->port);
 
 	assert_hdmi_port_disabled(intel_hdmi);
@@ -1038,8 +1028,8 @@ static void ibx_set_infoframes(struct intel_encoder *encoder,
 		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
 			 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
 			 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
-		I915_WRITE(reg, val);
-		POSTING_READ(reg);
+		intel_de_write(dev_priv, reg, val);
+		intel_de_posting_read(dev_priv, reg);
 		return;
 	}
 
@@ -1059,8 +1049,8 @@ static void ibx_set_infoframes(struct intel_encoder *encoder,
 	if (intel_hdmi_set_gcp_infoframe(encoder, crtc_state, conn_state))
 		val |= VIDEO_DIP_ENABLE_GCP;
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_AVI,
@@ -1082,7 +1072,7 @@ static void cpt_set_infoframes(struct intel_encoder *encoder,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	i915_reg_t reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 
 	assert_hdmi_port_disabled(intel_hdmi);
 
@@ -1095,8 +1085,8 @@ static void cpt_set_infoframes(struct intel_encoder *encoder,
 		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
 			 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
 			 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
-		I915_WRITE(reg, val);
-		POSTING_READ(reg);
+		intel_de_write(dev_priv, reg, val);
+		intel_de_posting_read(dev_priv, reg);
 		return;
 	}
 
@@ -1108,8 +1098,8 @@ static void cpt_set_infoframes(struct intel_encoder *encoder,
 	if (intel_hdmi_set_gcp_infoframe(encoder, crtc_state, conn_state))
 		val |= VIDEO_DIP_ENABLE_GCP;
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_AVI,
@@ -1131,7 +1121,7 @@ static void vlv_set_infoframes(struct intel_encoder *encoder,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	i915_reg_t reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 	u32 port = VIDEO_DIP_PORT(encoder->port);
 
 	assert_hdmi_port_disabled(intel_hdmi);
@@ -1145,8 +1135,8 @@ static void vlv_set_infoframes(struct intel_encoder *encoder,
 		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI |
 			 VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
 			 VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP);
-		I915_WRITE(reg, val);
-		POSTING_READ(reg);
+		intel_de_write(dev_priv, reg, val);
+		intel_de_posting_read(dev_priv, reg);
 		return;
 	}
 
@@ -1166,8 +1156,8 @@ static void vlv_set_infoframes(struct intel_encoder *encoder,
 	if (intel_hdmi_set_gcp_infoframe(encoder, crtc_state, conn_state))
 		val |= VIDEO_DIP_ENABLE_GCP;
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_AVI,
@@ -1187,7 +1177,7 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	i915_reg_t reg = HSW_TVIDEO_DIP_CTL(crtc_state->cpu_transcoder);
-	u32 val = I915_READ(reg);
+	u32 val = intel_de_read(dev_priv, reg);
 
 	assert_hdmi_transcoder_func_disabled(dev_priv,
 					     crtc_state->cpu_transcoder);
@@ -1198,16 +1188,16 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 		 VIDEO_DIP_ENABLE_DRM_GLK);
 
 	if (!enable) {
-		I915_WRITE(reg, val);
-		POSTING_READ(reg);
+		intel_de_write(dev_priv, reg, val);
+		intel_de_posting_read(dev_priv, reg);
 		return;
 	}
 
 	if (intel_hdmi_set_gcp_infoframe(encoder, crtc_state, conn_state))
 		val |= VIDEO_DIP_ENABLE_GCP_HSW;
 
-	I915_WRITE(reg, val);
-	POSTING_READ(reg);
+	intel_de_write(dev_priv, reg, val);
+	intel_de_posting_read(dev_priv, reg);
 
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_AVI,
@@ -1437,7 +1427,7 @@ static int kbl_repositioning_enc_en_signal(struct intel_connector *connector)
 	int ret;
 
 	for (;;) {
-		scanline = I915_READ(PIPEDSL(intel_crtc->pipe));
+		scanline = intel_de_read(dev_priv, PIPEDSL(intel_crtc->pipe));
 		if (scanline > 100 && scanline < 200)
 			break;
 		usleep_range(25, 50);
@@ -1502,13 +1492,13 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
 	if (ret)
 		return false;
 
-	I915_WRITE(PORT_HDCP_RPRIME(port), ri.reg);
+	intel_de_write(dev_priv, PORT_HDCP_RPRIME(port), ri.reg);
 
 	/* Wait for Ri prime match */
-	if (wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
+	if (wait_for(intel_de_read(dev_priv, PORT_HDCP_STATUS(port)) &
 		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
 		DRM_ERROR("Ri' mismatch detected, link check failed (%x)\n",
-			  I915_READ(PORT_HDCP_STATUS(port)));
+			  intel_de_read(dev_priv, PORT_HDCP_STATUS(port)));
 		return false;
 	}
 	return true;
@@ -1751,8 +1741,8 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder,
 	else
 		hdmi_val |= SDVO_PIPE_SEL(crtc->pipe);
 
-	I915_WRITE(intel_hdmi->hdmi_reg, hdmi_val);
-	POSTING_READ(intel_hdmi->hdmi_reg);
+	intel_de_write(dev_priv, intel_hdmi->hdmi_reg, hdmi_val);
+	intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 }
 
 static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
@@ -1786,7 +1776,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 
 	pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
 
-	tmp = I915_READ(intel_hdmi->hdmi_reg);
+	tmp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
 		flags |= DRM_MODE_FLAG_PHSYNC;
@@ -1862,14 +1852,14 @@ static void g4x_enable_hdmi(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	u32 temp;
 
-	temp = I915_READ(intel_hdmi->hdmi_reg);
+	temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	temp |= SDVO_ENABLE;
 	if (pipe_config->has_audio)
 		temp |= HDMI_AUDIO_ENABLE;
 
-	I915_WRITE(intel_hdmi->hdmi_reg, temp);
-	POSTING_READ(intel_hdmi->hdmi_reg);
+	intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+	intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	if (pipe_config->has_audio)
 		intel_enable_hdmi_audio(encoder, pipe_config, conn_state);
@@ -1884,7 +1874,7 @@ static void ibx_enable_hdmi(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	u32 temp;
 
-	temp = I915_READ(intel_hdmi->hdmi_reg);
+	temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	temp |= SDVO_ENABLE;
 	if (pipe_config->has_audio)
@@ -1894,10 +1884,10 @@ static void ibx_enable_hdmi(struct intel_encoder *encoder,
 	 * HW workaround, need to write this twice for issue
 	 * that may result in first write getting masked.
 	 */
-	I915_WRITE(intel_hdmi->hdmi_reg, temp);
-	POSTING_READ(intel_hdmi->hdmi_reg);
-	I915_WRITE(intel_hdmi->hdmi_reg, temp);
-	POSTING_READ(intel_hdmi->hdmi_reg);
+	intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+	intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
+	intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+	intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	/*
 	 * HW workaround, need to toggle enable bit off and on
@@ -1908,17 +1898,17 @@ static void ibx_enable_hdmi(struct intel_encoder *encoder,
 	 */
 	if (pipe_config->pipe_bpp > 24 &&
 	    pipe_config->pixel_multiplier > 1) {
-		I915_WRITE(intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE);
-		POSTING_READ(intel_hdmi->hdmi_reg);
+		intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE);
+		intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 
 		/*
 		 * HW workaround, need to write this twice for issue
 		 * that may result in first write getting masked.
 		 */
-		I915_WRITE(intel_hdmi->hdmi_reg, temp);
-		POSTING_READ(intel_hdmi->hdmi_reg);
-		I915_WRITE(intel_hdmi->hdmi_reg, temp);
-		POSTING_READ(intel_hdmi->hdmi_reg);
+		intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+		intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
+		intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+		intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 	}
 
 	if (pipe_config->has_audio)
@@ -1936,7 +1926,7 @@ static void cpt_enable_hdmi(struct intel_encoder *encoder,
 	enum pipe pipe = crtc->pipe;
 	u32 temp;
 
-	temp = I915_READ(intel_hdmi->hdmi_reg);
+	temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	temp |= SDVO_ENABLE;
 	if (pipe_config->has_audio)
@@ -1953,27 +1943,25 @@ static void cpt_enable_hdmi(struct intel_encoder *encoder,
 	 */
 
 	if (pipe_config->pipe_bpp > 24) {
-		I915_WRITE(TRANS_CHICKEN1(pipe),
-			   I915_READ(TRANS_CHICKEN1(pipe)) |
-			   TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE);
+		intel_de_rmw(dev_priv, TRANS_CHICKEN1(pipe),
+			     0, TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE);
 
 		temp &= ~SDVO_COLOR_FORMAT_MASK;
 		temp |= SDVO_COLOR_FORMAT_8bpc;
 	}
 
-	I915_WRITE(intel_hdmi->hdmi_reg, temp);
-	POSTING_READ(intel_hdmi->hdmi_reg);
+	intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+	intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	if (pipe_config->pipe_bpp > 24) {
 		temp &= ~SDVO_COLOR_FORMAT_MASK;
 		temp |= HDMI_COLOR_FORMAT_12bpc;
 
-		I915_WRITE(intel_hdmi->hdmi_reg, temp);
-		POSTING_READ(intel_hdmi->hdmi_reg);
+		intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+		intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 
-		I915_WRITE(TRANS_CHICKEN1(pipe),
-			   I915_READ(TRANS_CHICKEN1(pipe)) &
-			   ~TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE);
+		intel_de_rmw(dev_priv, TRANS_CHICKEN1(pipe),
+			     TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE, 0);
 	}
 
 	if (pipe_config->has_audio)
@@ -1998,11 +1986,11 @@ static void intel_disable_hdmi(struct intel_encoder *encoder,
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
 	u32 temp;
 
-	temp = I915_READ(intel_hdmi->hdmi_reg);
+	temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	temp &= ~(SDVO_ENABLE | HDMI_AUDIO_ENABLE);
-	I915_WRITE(intel_hdmi->hdmi_reg, temp);
-	POSTING_READ(intel_hdmi->hdmi_reg);
+	intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+	intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 
 	/*
 	 * HW workaround for IBX, we need to move the port
@@ -2023,14 +2011,14 @@ static void intel_disable_hdmi(struct intel_encoder *encoder,
 		 * HW workaround, need to write this twice for issue
 		 * that may result in first write getting masked.
 		 */
-		I915_WRITE(intel_hdmi->hdmi_reg, temp);
-		POSTING_READ(intel_hdmi->hdmi_reg);
-		I915_WRITE(intel_hdmi->hdmi_reg, temp);
-		POSTING_READ(intel_hdmi->hdmi_reg);
+		intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+		intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
+		intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+		intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 
 		temp &= ~SDVO_ENABLE;
-		I915_WRITE(intel_hdmi->hdmi_reg, temp);
-		POSTING_READ(intel_hdmi->hdmi_reg);
+		intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
+		intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
 
 		intel_wait_for_vblank_if_active(dev_priv, PIPE_A);
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
@@ -3132,10 +3120,8 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	 * 0xd.  Failure to do so will result in spurious interrupts being
 	 * generated on the port when a cable is not attached.
 	 */
-	if (IS_G45(dev_priv)) {
-		u32 temp = I915_READ(PEG_BAND_GAP_DATA);
-		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
-	}
+	if (IS_G45(dev_priv))
+		intel_de_rmw(dev_priv, PEG_BAND_GAP_DATA, 0xf, 0xd);
 
 	intel_hdmi->cec_notifier = cec_notifier_get_conn(dev->dev,
 							 port_identifier(port));
-- 
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] 15+ messages in thread

* ✗ Fi.CI.SPARSE: warning for Display uncore (rev2)
  2019-06-24 20:31 [RFC 0/4] Display uncore Daniele Ceraolo Spurio
                   ` (3 preceding siblings ...)
  2019-06-24 20:31 ` [RFC 4/4] drm/i915: convert intel_hdmi to display reg accessors Daniele Ceraolo Spurio
@ 2019-06-24 20:43 ` Patchwork
  2019-06-24 21:37 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-06-25  1:04 ` ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-06-24 20:43 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: Display uncore (rev2)
URL   : https://patchwork.freedesktop.org/series/61735/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: split out uncore_mmio_debug
+drivers/gpu/drm/i915/intel_uncore.c:1192:1: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1192:1: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1192:1: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1192:1: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1193:1: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1193:1: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1193:1: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1193:1: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1194:1: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1194:1: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1194:1: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1194:1: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1257:1: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1258:1: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1259:1: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1283:1: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1283:1: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1283:1: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1284:1: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1284:1: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1284:1: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1285:1: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1285:1: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+drivers/gpu/drm/i915/intel_uncore.c:1285:1: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block

Commit: drm/i915: rework mmio debug stop/start
+drivers/gpu/drm/i915/intel_uncore.c:44:6: warning: symbol 'uncore_mmio_debug_suspend' was not declared. Should it be static?
+drivers/gpu/drm/i915/intel_uncore.c:55:6: warning: symbol 'uncore_mmio_debug_resume' was not declared. Should it be static?

Commit: drm/i915: introduce display_uncore
Okay!

Commit: drm/i915: convert intel_hdmi to display reg accessors
Okay!

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

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

* ✓ Fi.CI.BAT: success for Display uncore (rev2)
  2019-06-24 20:31 [RFC 0/4] Display uncore Daniele Ceraolo Spurio
                   ` (4 preceding siblings ...)
  2019-06-24 20:43 ` ✗ Fi.CI.SPARSE: warning for Display uncore (rev2) Patchwork
@ 2019-06-24 21:37 ` Patchwork
  2019-06-25  1:04 ` ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-06-24 21:37 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: Display uncore (rev2)
URL   : https://patchwork.freedesktop.org/series/61735/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6337 -> Patchwork_13408
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [PASS][1] -> [FAIL][2] ([fdo#103167])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-icl-dsi:         [INCOMPLETE][3] ([fdo#107713] / [fdo#109100]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@i915_selftest@live_blt:
    - fi-skl-iommu:       [INCOMPLETE][5] ([fdo#108602]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/fi-skl-iommu/igt@i915_selftest@live_blt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/fi-skl-iommu/igt@i915_selftest@live_blt.html

  * igt@kms_busy@basic-flip-c:
    - fi-kbl-7500u:       [SKIP][7] ([fdo#109271] / [fdo#109278]) -> [PASS][8] +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-kbl-7500u:       [DMESG-WARN][9] ([fdo#103558] / [fdo#105602]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/fi-kbl-7500u/igt@kms_chamelium@dp-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/fi-kbl-7500u/igt@kms_chamelium@dp-hpd-fast.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [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


Participating hosts (50 -> 42)
------------------------------

  Additional (1): fi-gdg-551 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-skl-lmem fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6337 -> Patchwork_13408

  CI_DRM_6337: 294d5056c1f3fc839b6c3bbd2aa6f451ac316991 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5066: a6f5cc854efb4b7dfed7f0a2c1039a9ddd1a35a5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13408: 6280c29422ffbbcb09f51d05aa3ad149ab90a480 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6280c29422ff drm/i915: convert intel_hdmi to display reg accessors
3a32a4f1b42c drm/i915: introduce display_uncore
be0532e5172b drm/i915: rework mmio debug stop/start
e7d02b0ed57e drm/i915: split out uncore_mmio_debug

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for Display uncore (rev2)
  2019-06-24 20:31 [RFC 0/4] Display uncore Daniele Ceraolo Spurio
                   ` (5 preceding siblings ...)
  2019-06-24 21:37 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-06-25  1:04 ` Patchwork
  6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-06-25  1:04 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: Display uncore (rev2)
URL   : https://patchwork.freedesktop.org/series/61735/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6337_full -> Patchwork_13408_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          [PASS][1] -> [FAIL][2] ([fdo#110667])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-kbl6/igt@gem_eio@in-flight-suspend.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-kbl6/igt@gem_eio@in-flight-suspend.html

  * igt@gem_eio@reset-stress:
    - shard-snb:          [PASS][3] -> [FAIL][4] ([fdo#109661])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-snb4/igt@gem_eio@reset-stress.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-snb7/igt@gem_eio@reset-stress.html

  * igt@gem_exec_reloc@basic-wc-cpu-active:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#110913 ]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-apl2/igt@gem_exec_reloc@basic-wc-cpu-active.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-apl3/igt@gem_exec_reloc@basic-wc-cpu-active.html

  * igt@gem_partial_pwrite_pread@write:
    - shard-kbl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#110913 ]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-kbl6/igt@gem_partial_pwrite_pread@write.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-kbl7/igt@gem_partial_pwrite_pread@write.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-hsw:          [PASS][9] -> [DMESG-WARN][10] ([fdo#110789] / [fdo#110913 ]) +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-hsw8/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-hsw1/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_softpin@softpin:
    - shard-snb:          [PASS][11] -> [DMESG-WARN][12] ([fdo#110789] / [fdo#110913 ]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-snb4/igt@gem_softpin@softpin.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-snb6/igt@gem_softpin@softpin.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         [PASS][13] -> [DMESG-WARN][14] ([fdo#108686])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb6/igt@gem_tiled_swapping@non-threaded.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb5/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
    - shard-glk:          [PASS][15] -> [DMESG-WARN][16] ([fdo#110913 ]) +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-glk3/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-glk1/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html

  * igt@i915_pm_rpm@i2c:
    - shard-hsw:          [PASS][17] -> [FAIL][18] ([fdo#104097])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-hsw5/igt@i915_pm_rpm@i2c.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-hsw1/igt@i915_pm_rpm@i2c.html

  * igt@i915_pm_rpm@universal-planes-dpms:
    - shard-iclb:         [PASS][19] -> [INCOMPLETE][20] ([fdo#107713] / [fdo#108840])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb3/igt@i915_pm_rpm@universal-planes-dpms.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb8/igt@i915_pm_rpm@universal-planes-dpms.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [PASS][21] -> [INCOMPLETE][22] ([fdo#109507])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-skl2/igt@kms_flip@flip-vs-suspend-interruptible.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-skl8/igt@kms_flip@flip-vs-suspend-interruptible.html
    - shard-kbl:          [PASS][23] -> [INCOMPLETE][24] ([fdo#103665])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-kbl6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-kbl1/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt:
    - shard-hsw:          [PASS][25] -> [SKIP][26] ([fdo#109271]) +11 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-hsw5/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         [PASS][27] -> [FAIL][28] ([fdo#103167]) +4 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][29] -> [DMESG-WARN][30] ([fdo#108566]) +3 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-apl5/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][31] -> [FAIL][32] ([fdo#108145])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][33] -> [FAIL][34] ([fdo#103166])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [PASS][35] -> [SKIP][36] ([fdo#109441]) +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb7/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         [PASS][37] -> [FAIL][38] ([fdo#100047])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb8/igt@kms_sysfs_edid_timing.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb3/igt@kms_sysfs_edid_timing.html

  * igt@perf_pmu@rc6:
    - shard-kbl:          [PASS][39] -> [SKIP][40] ([fdo#109271])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-kbl6/igt@perf_pmu@rc6.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-kbl7/igt@perf_pmu@rc6.html

  
#### Possible fixes ####

  * igt@gem_eio@wait-10ms:
    - shard-apl:          [DMESG-WARN][41] ([fdo#110913 ]) -> [PASS][42] +4 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-apl3/igt@gem_eio@wait-10ms.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-apl8/igt@gem_eio@wait-10ms.html

  * igt@gem_exec_reloc@basic-wc-cpu-active:
    - shard-glk:          [DMESG-WARN][43] ([fdo#110913 ]) -> [PASS][44] +4 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-glk7/igt@gem_exec_reloc@basic-wc-cpu-active.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-glk9/igt@gem_exec_reloc@basic-wc-cpu-active.html

  * igt@gem_partial_pwrite_pread@reads-uncached:
    - shard-kbl:          [DMESG-WARN][45] ([fdo#110913 ]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-kbl6/igt@gem_partial_pwrite_pread@reads-uncached.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-kbl6/igt@gem_partial_pwrite_pread@reads-uncached.html

  * igt@gem_partial_pwrite_pread@writes-after-reads-display:
    - shard-hsw:          [DMESG-WARN][47] ([fdo#110789] / [fdo#110913 ]) -> [PASS][48] +3 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-hsw2/igt@gem_partial_pwrite_pread@writes-after-reads-display.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-hsw2/igt@gem_partial_pwrite_pread@writes-after-reads-display.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-skl:          [DMESG-WARN][49] ([fdo#110913 ]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-skl4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-skl7/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-snb:          [DMESG-WARN][51] ([fdo#110789] / [fdo#110913 ]) -> [PASS][52] +2 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-snb7/igt@gem_userptr_blits@sync-unmap.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-snb4/igt@gem_userptr_blits@sync-unmap.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          [SKIP][53] ([fdo#109271]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-kbl3/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-kbl6/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][55] ([fdo#105363]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-skl5/igt@kms_flip@flip-vs-expired-vblank.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-skl3/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-glk:          [FAIL][57] ([fdo#103060]) -> [PASS][58] +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-glk1/igt@kms_flip@modeset-vs-vblank-race.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-glk8/igt@kms_flip@modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt:
    - shard-iclb:         [FAIL][59] ([fdo#103167]) -> [PASS][60] +2 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-msflip-blt:
    - shard-hsw:          [SKIP][61] ([fdo#109271]) -> [PASS][62] +21 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-msflip-blt.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-hsw5/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-msflip-blt.html

  * igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
    - shard-skl:          [FAIL][63] ([fdo#103166]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-skl9/igt@kms_plane@plane-panning-bottom-right-pipe-a-planes.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-skl5/igt@kms_plane@plane-panning-bottom-right-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][65] ([fdo#108145]) -> [PASS][66] +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][67] ([fdo#108145] / [fdo#110403]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][69] ([fdo#109441]) -> [PASS][70] +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb5/igt@kms_psr@psr2_cursor_render.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][71] ([fdo#99912]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-kbl4/igt@kms_setmode@basic.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-kbl6/igt@kms_setmode@basic.html

  * igt@perf@blocking:
    - shard-iclb:         [FAIL][73] ([fdo#110728]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-iclb5/igt@perf@blocking.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-iclb3/igt@perf@blocking.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-skl:          [FAIL][75] ([fdo#103167]) -> [FAIL][76] ([fdo#108040])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6337/shard-skl6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13408/shard-skl8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html

  
  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110667]: https://bugs.freedesktop.org/show_bug.cgi?id=110667
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110789]: https://bugs.freedesktop.org/show_bug.cgi?id=110789
  [fdo#110913 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110913 
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * Linux: CI_DRM_6337 -> Patchwork_13408

  CI_DRM_6337: 294d5056c1f3fc839b6c3bbd2aa6f451ac316991 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5066: a6f5cc854efb4b7dfed7f0a2c1039a9ddd1a35a5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13408: 6280c29422ffbbcb09f51d05aa3ad149ab90a480 @ 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_13408/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC 1/4] drm/i915: split out uncore_mmio_debug
  2019-06-24 20:31 ` [RFC 1/4] drm/i915: split out uncore_mmio_debug Daniele Ceraolo Spurio
@ 2019-06-26 10:02   ` Chris Wilson
  2019-06-26 17:38     ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-06-26 10:02 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, intel-gfx

Quoting Daniele Ceraolo Spurio (2019-06-24 21:31:49)
> @@ -605,18 +614,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
>  void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>  {
>         spin_lock_irq(&uncore->lock);
> +       spin_lock(&uncore->debug->lock);
>         if (!uncore->user_forcewake.count++) {

Afaict, uncore->user_forcewake.count is only guarded by uncore->lock
and we only need to take debug->lock for the debug->unclaimed_mmio_check
manipulation. But there needs to be a shared usage counter around the
debug as it is shared state.

>                 intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
>  
>                 /* Save and disable mmio debugging for the user bypass */
>                 uncore->user_forcewake.saved_mmio_check =
> -                       uncore->unclaimed_mmio_check;
> +                       uncore->debug->unclaimed_mmio_check;
>                 uncore->user_forcewake.saved_mmio_debug =
>                         i915_modparams.mmio_debug;

Something more like

spin_lock_irq(&uncore->lock);
if (!uncore->user_forcewake_count++) {
	spin_lock(&uncore->debug->lock);
	if (!uncore->debug->usage_count++) {
		...
	}
	spin_unlock(&uncore->debug->lock);
}
...
spin_unlock_irq(&uncore->lock);
?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC 1/4] drm/i915: split out uncore_mmio_debug
  2019-06-26 10:02   ` Chris Wilson
@ 2019-06-26 17:38     ` Daniele Ceraolo Spurio
  2019-06-26 17:58       ` Chris Wilson
  0 siblings, 1 reply; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-06-26 17:38 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx



On 6/26/19 3:02 AM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2019-06-24 21:31:49)
>> @@ -605,18 +614,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
>>   void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>>   {
>>          spin_lock_irq(&uncore->lock);
>> +       spin_lock(&uncore->debug->lock);
>>          if (!uncore->user_forcewake.count++) {
> 
> Afaict, uncore->user_forcewake.count is only guarded by uncore->lock
> and we only need to take debug->lock for the debug->unclaimed_mmio_check
> manipulation. But there needs to be a shared usage counter around the
> debug as it is shared state.
> 
>>                  intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
>>   
>>                  /* Save and disable mmio debugging for the user bypass */
>>                  uncore->user_forcewake.saved_mmio_check =
>> -                       uncore->unclaimed_mmio_check;
>> +                       uncore->debug->unclaimed_mmio_check;
>>                  uncore->user_forcewake.saved_mmio_debug =
>>                          i915_modparams.mmio_debug;
> 
> Something more like
> 
> spin_lock_irq(&uncore->lock);
> if (!uncore->user_forcewake_count++) {
> 	spin_lock(&uncore->debug->lock);
> 	if (!uncore->debug->usage_count++) {
> 		...
> 	}
> 	spin_unlock(&uncore->debug->lock);
> }
> ...
> spin_unlock_irq(&uncore->lock);
> ?
> -Chris
> 

I do something similar in the next patch in the series (with the lock 
still on the outside of the if). I've split that out because I've added 
some functional changes to the flow. I can squash the 2 patches if you 
thing it is better that way.

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

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

* Re: [RFC 1/4] drm/i915: split out uncore_mmio_debug
  2019-06-26 17:38     ` Daniele Ceraolo Spurio
@ 2019-06-26 17:58       ` Chris Wilson
  2019-06-26 18:20         ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-06-26 17:58 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, intel-gfx

Quoting Daniele Ceraolo Spurio (2019-06-26 18:38:45)
> 
> 
> On 6/26/19 3:02 AM, Chris Wilson wrote:
> > Quoting Daniele Ceraolo Spurio (2019-06-24 21:31:49)
> >> @@ -605,18 +614,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
> >>   void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
> >>   {
> >>          spin_lock_irq(&uncore->lock);
> >> +       spin_lock(&uncore->debug->lock);
> >>          if (!uncore->user_forcewake.count++) {
> > 
> > Afaict, uncore->user_forcewake.count is only guarded by uncore->lock
> > and we only need to take debug->lock for the debug->unclaimed_mmio_check
> > manipulation. But there needs to be a shared usage counter around the
> > debug as it is shared state.
> > 
> >>                  intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
> >>   
> >>                  /* Save and disable mmio debugging for the user bypass */
> >>                  uncore->user_forcewake.saved_mmio_check =
> >> -                       uncore->unclaimed_mmio_check;
> >> +                       uncore->debug->unclaimed_mmio_check;
> >>                  uncore->user_forcewake.saved_mmio_debug =
> >>                          i915_modparams.mmio_debug;
> > 
> > Something more like
> > 
> > spin_lock_irq(&uncore->lock);
> > if (!uncore->user_forcewake_count++) {
> >       spin_lock(&uncore->debug->lock);
> >       if (!uncore->debug->usage_count++) {
> >               ...
> >       }
> >       spin_unlock(&uncore->debug->lock);
> > }
> > ...
> > spin_unlock_irq(&uncore->lock);
> > ?
> > -Chris
> > 
> 
> I do something similar in the next patch in the series (with the lock 
> still on the outside of the if). I've split that out because I've added 
> some functional changes to the flow. I can squash the 2 patches if you 
> thing it is better that way.

Yes. Looking at the second patch, that is clearer wrt what data we are
guarding with the locks.

Don't drop the intel_ prefix from intel_uncore_debug as it looks to
still be visible outside of its enclave (it's getting long, but it should
be more or less internal to the various intel_uncore implementations) and
squash these 2 as I feel more comfortable with intel_uncore_debug taking
control of the locking as required for sharing and reviewing the locking
wrt to the shared data.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC 1/4] drm/i915: split out uncore_mmio_debug
  2019-06-26 17:58       ` Chris Wilson
@ 2019-06-26 18:20         ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-06-26 18:20 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx



On 6/26/19 10:58 AM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2019-06-26 18:38:45)
>>
>>
>> On 6/26/19 3:02 AM, Chris Wilson wrote:
>>> Quoting Daniele Ceraolo Spurio (2019-06-24 21:31:49)
>>>> @@ -605,18 +614,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
>>>>    void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>>>>    {
>>>>           spin_lock_irq(&uncore->lock);
>>>> +       spin_lock(&uncore->debug->lock);
>>>>           if (!uncore->user_forcewake.count++) {
>>>
>>> Afaict, uncore->user_forcewake.count is only guarded by uncore->lock
>>> and we only need to take debug->lock for the debug->unclaimed_mmio_check
>>> manipulation. But there needs to be a shared usage counter around the
>>> debug as it is shared state.
>>>
>>>>                   intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
>>>>    
>>>>                   /* Save and disable mmio debugging for the user bypass */
>>>>                   uncore->user_forcewake.saved_mmio_check =
>>>> -                       uncore->unclaimed_mmio_check;
>>>> +                       uncore->debug->unclaimed_mmio_check;
>>>>                   uncore->user_forcewake.saved_mmio_debug =
>>>>                           i915_modparams.mmio_debug;
>>>
>>> Something more like
>>>
>>> spin_lock_irq(&uncore->lock);
>>> if (!uncore->user_forcewake_count++) {
>>>        spin_lock(&uncore->debug->lock);
>>>        if (!uncore->debug->usage_count++) {
>>>                ...
>>>        }
>>>        spin_unlock(&uncore->debug->lock);
>>> }
>>> ...
>>> spin_unlock_irq(&uncore->lock);
>>> ?
>>> -Chris
>>>
>>
>> I do something similar in the next patch in the series (with the lock
>> still on the outside of the if). I've split that out because I've added
>> some functional changes to the flow. I can squash the 2 patches if you
>> thing it is better that way.
> 
> Yes. Looking at the second patch, that is clearer wrt what data we are
> guarding with the locks.
> 
> Don't drop the intel_ prefix from intel_uncore_debug as it looks to
> still be visible outside of its enclave (it's getting long, but it should

I'm assuming you're referring to the 2 new functions. These are actually 
meant to only used within the file and not be externally visible, I just 
forgot the static tag (and sparse complained for that). Everything else 
should still have the intel_ prefix.

Daniele

> be more or less internal to the various intel_uncore implementations) and
> squash these 2 as I feel more comfortable with intel_uncore_debug taking
> control of the locking as required for sharing and reviewing the locking
> wrt to the shared data.
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC 3/4] drm/i915: introduce display_uncore
  2019-06-24 20:31 ` [RFC 3/4] drm/i915: introduce display_uncore Daniele Ceraolo Spurio
@ 2019-06-26 18:42   ` Ville Syrjälä
  2019-06-26 20:27     ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2019-06-26 18:42 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

On Mon, Jun 24, 2019 at 01:31:51PM -0700, Daniele Ceraolo Spurio wrote:
> A forcewake-less uncore to be used to decouple GT accesses from display
> ones to avoid serializing them when there is no need.
> 
> New accessors that implicitly use the new uncore have also been added.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c     | 17 ++++++++-
>  drivers/gpu/drm/i915/i915_drv.h     | 58 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uncore.c |  9 ++++-
>  3 files changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2d6c643dde51..e22c0a6b3992 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
>  
>  	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
>  	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
> +	intel_uncore_init_early(&dev_priv->de_uncore, dev_priv);
>  
>  	spin_lock_init(&dev_priv->irq_lock);
>  	spin_lock_init(&dev_priv->gpu_error.lock);
> @@ -1001,6 +1002,10 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
>  	if (ret < 0)
>  		goto err_bridge;
>  
> +	ret = intel_uncore_init_mmio(&dev_priv->de_uncore);
> +	if (ret < 0)
> +		goto err_uncore;
> +
>  	/* Try to make sure MCHBAR is enabled before poking at it */
>  	intel_setup_mchbar(dev_priv);
>  
> @@ -1012,14 +1017,16 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
>  
>  	ret = intel_engines_init_mmio(dev_priv);
>  	if (ret)
> -		goto err_uncore;
> +		goto err_mchbar;
>  
>  	i915_gem_init_mmio(dev_priv);
>  
>  	return 0;
>  
> -err_uncore:
> +err_mchbar:
>  	intel_teardown_mchbar(dev_priv);
> +	intel_uncore_fini_mmio(&dev_priv->de_uncore);
> +err_uncore:
>  	intel_uncore_fini_mmio(&dev_priv->uncore);
>  err_bridge:
>  	pci_dev_put(dev_priv->bridge_dev);
> @@ -1034,6 +1041,7 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
>  static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
>  {
>  	intel_teardown_mchbar(dev_priv);
> +	intel_uncore_fini_mmio(&dev_priv->de_uncore);
>  	intel_uncore_fini_mmio(&dev_priv->uncore);
>  	pci_dev_put(dev_priv->bridge_dev);
>  }
> @@ -2166,6 +2174,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>  
>  	i915_gem_suspend_late(dev_priv);
>  
> +	intel_uncore_suspend(&dev_priv->de_uncore);
>  	intel_uncore_suspend(&dev_priv->uncore);
>  
>  	intel_power_domains_suspend(dev_priv,
> @@ -2363,6 +2372,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>  			  ret);
>  
>  	intel_uncore_resume_early(&dev_priv->uncore);
> +	intel_uncore_resume_early(&dev_priv->de_uncore);
>  
>  	intel_gt_check_and_clear_faults(&dev_priv->gt);
>  
> @@ -2937,6 +2947,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  
>  	intel_runtime_pm_disable_interrupts(dev_priv);
>  
> +	intel_uncore_suspend(&dev_priv->de_uncore);
>  	intel_uncore_suspend(&dev_priv->uncore);
>  
>  	ret = 0;
> @@ -2955,6 +2966,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  	if (ret) {
>  		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
>  		intel_uncore_runtime_resume(&dev_priv->uncore);
> +		intel_uncore_runtime_resume(&dev_priv->de_uncore);
>  
>  		intel_runtime_pm_enable_interrupts(dev_priv);
>  
> @@ -3053,6 +3065,7 @@ static int intel_runtime_resume(struct device *kdev)
>  	}
>  
>  	intel_uncore_runtime_resume(&dev_priv->uncore);
> +	intel_uncore_runtime_resume(&dev_priv->de_uncore);
>  
>  	intel_runtime_pm_enable_interrupts(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a53b65d78159..c554b7697bdc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1341,6 +1341,7 @@ struct drm_i915_private {
>  	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
>  
>  	struct intel_uncore uncore;
> +	struct intel_uncore de_uncore;
>  	struct intel_uncore_mmio_debug mmio_debug;
>  
>  	struct i915_virtual_gpu vgpu;
> @@ -2746,6 +2747,63 @@ 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__))
>  
> +/*
> + * The following are mmio-accessors that use an independent lock and skip all
> + * the forcewake logic, to be used to access display registers, which are
> + * outside the GT forcewake wells.
> + */
> +static inline void
> +intel_assert_register_is_display(struct drm_i915_private *i915, i915_reg_t reg)
> +{
> +#ifdef CONFIG_DRM_I915_DEBUG_MMIO
> +	u32 offset = i915_mmio_reg_offset(reg);
> +	bool is_de_register;
> +
> +	if (INTEL_GEN(i915) >= 5) {
> +		is_de_register = offset >= 0x40000 &&
> +			(INTEL_GEN(i915) < 11 || offset < 0x1c0000);

I think the gen11+ thing is wrong. There are eg. PHY registers
above 0x162000.

> +	} else {
> +		is_de_register =
> +			(offset >= 0x5000 && offset <= 0x5fff) ||
> +			(offset >= 0x6000 && offset <= 0x6fff) ||
> +			(offset >= 0xa000 && offset <= 0xafff) ||
> +			offset >= 0x30000;

I think the WARNs are coming from the mchbar range (0x10000+ on
these platforms). Not really sure if we should designate that
gt or de.

Unfortunately the register ranges don't seem to follow much
of a sensible pattern. Eg. we seem to have both gt and de
related stuff around the 0x130000 mark :(

Maybe we should try separate only the pipe/plane registers
which are part of your typical atomic commit under their
own lock. Could even do per-pipe locks maybe...

> +	}
> +
> +	WARN_ONCE(!is_de_register,
> +		  "display reg access function used for non-display reg 0x%08x\n",
> +		  offset);
> +#endif
> +}
> +
> +static inline u32
> +intel_de_read(struct drm_i915_private *i915, i915_reg_t reg)
> +{
> +	intel_assert_register_is_display(i915, reg);
> +	return intel_uncore_read(&i915->de_uncore, reg);
> +}
> +
> +static inline void
> +intel_de_posting_read(struct drm_i915_private *i915, i915_reg_t reg)
> +{
> +	intel_assert_register_is_display(i915, reg);
> +	intel_uncore_posting_read(&i915->de_uncore, reg);
> +}
> +
> +static inline void
> +intel_de_write(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
> +{
> +	intel_assert_register_is_display(i915, reg);
> +	intel_uncore_write(&i915->de_uncore, reg, val);
> +}
> +
> +static inline void
> +intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 set)
> +{
> +	intel_assert_register_is_display(i915, reg);
> +	intel_uncore_rmw(&i915->de_uncore, reg, clear, set);
> +}
> +
>  /* "Broadcast RGB" property */
>  #define INTEL_BROADCAST_RGB_AUTO 0
>  #define INTEL_BROADCAST_RGB_FULL 1
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 1905fd94dd3c..94e153acb0af 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1674,6 +1674,12 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
>  	return 0;
>  }
>  
> +static bool
> +intel_uncore_is_display(const struct intel_uncore *uncore)
> +{
> +	return uncore == &uncore->i915->de_uncore;
> +}
> +
>  int intel_uncore_init_mmio(struct intel_uncore *uncore)
>  {
>  	struct drm_i915_private *i915 = uncore->i915;
> @@ -1683,7 +1689,8 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>  	if (ret)
>  		return ret;
>  
> -	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
> +	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915) &&
> +	    !intel_uncore_is_display(uncore))
>  		uncore->flags |= UNCORE_HAS_FORCEWAKE;
>  
>  	if (!intel_uncore_has_forcewake(uncore)) {
> -- 
> 2.20.1

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

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

* Re: [RFC 3/4] drm/i915: introduce display_uncore
  2019-06-26 18:42   ` Ville Syrjälä
@ 2019-06-26 20:27     ` Daniele Ceraolo Spurio
  2019-06-27 12:41       ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-06-26 20:27 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx



On 6/26/19 11:42 AM, Ville Syrjälä wrote:
> On Mon, Jun 24, 2019 at 01:31:51PM -0700, Daniele Ceraolo Spurio wrote:
>> A forcewake-less uncore to be used to decouple GT accesses from display
>> ones to avoid serializing them when there is no need.
>>
>> New accessors that implicitly use the new uncore have also been added.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c     | 17 ++++++++-
>>   drivers/gpu/drm/i915/i915_drv.h     | 58 +++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_uncore.c |  9 ++++-
>>   3 files changed, 81 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 2d6c643dde51..e22c0a6b3992 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
>>   
>>   	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
>>   	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
>> +	intel_uncore_init_early(&dev_priv->de_uncore, dev_priv);
>>   
>>   	spin_lock_init(&dev_priv->irq_lock);
>>   	spin_lock_init(&dev_priv->gpu_error.lock);
>> @@ -1001,6 +1002,10 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
>>   	if (ret < 0)
>>   		goto err_bridge;
>>   
>> +	ret = intel_uncore_init_mmio(&dev_priv->de_uncore);
>> +	if (ret < 0)
>> +		goto err_uncore;
>> +
>>   	/* Try to make sure MCHBAR is enabled before poking at it */
>>   	intel_setup_mchbar(dev_priv);
>>   
>> @@ -1012,14 +1017,16 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
>>   
>>   	ret = intel_engines_init_mmio(dev_priv);
>>   	if (ret)
>> -		goto err_uncore;
>> +		goto err_mchbar;
>>   
>>   	i915_gem_init_mmio(dev_priv);
>>   
>>   	return 0;
>>   
>> -err_uncore:
>> +err_mchbar:
>>   	intel_teardown_mchbar(dev_priv);
>> +	intel_uncore_fini_mmio(&dev_priv->de_uncore);
>> +err_uncore:
>>   	intel_uncore_fini_mmio(&dev_priv->uncore);
>>   err_bridge:
>>   	pci_dev_put(dev_priv->bridge_dev);
>> @@ -1034,6 +1041,7 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
>>   static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
>>   {
>>   	intel_teardown_mchbar(dev_priv);
>> +	intel_uncore_fini_mmio(&dev_priv->de_uncore);
>>   	intel_uncore_fini_mmio(&dev_priv->uncore);
>>   	pci_dev_put(dev_priv->bridge_dev);
>>   }
>> @@ -2166,6 +2174,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>>   
>>   	i915_gem_suspend_late(dev_priv);
>>   
>> +	intel_uncore_suspend(&dev_priv->de_uncore);
>>   	intel_uncore_suspend(&dev_priv->uncore);
>>   
>>   	intel_power_domains_suspend(dev_priv,
>> @@ -2363,6 +2372,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>>   			  ret);
>>   
>>   	intel_uncore_resume_early(&dev_priv->uncore);
>> +	intel_uncore_resume_early(&dev_priv->de_uncore);
>>   
>>   	intel_gt_check_and_clear_faults(&dev_priv->gt);
>>   
>> @@ -2937,6 +2947,7 @@ static int intel_runtime_suspend(struct device *kdev)
>>   
>>   	intel_runtime_pm_disable_interrupts(dev_priv);
>>   
>> +	intel_uncore_suspend(&dev_priv->de_uncore);
>>   	intel_uncore_suspend(&dev_priv->uncore);
>>   
>>   	ret = 0;
>> @@ -2955,6 +2966,7 @@ static int intel_runtime_suspend(struct device *kdev)
>>   	if (ret) {
>>   		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
>>   		intel_uncore_runtime_resume(&dev_priv->uncore);
>> +		intel_uncore_runtime_resume(&dev_priv->de_uncore);
>>   
>>   		intel_runtime_pm_enable_interrupts(dev_priv);
>>   
>> @@ -3053,6 +3065,7 @@ static int intel_runtime_resume(struct device *kdev)
>>   	}
>>   
>>   	intel_uncore_runtime_resume(&dev_priv->uncore);
>> +	intel_uncore_runtime_resume(&dev_priv->de_uncore);
>>   
>>   	intel_runtime_pm_enable_interrupts(dev_priv);
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index a53b65d78159..c554b7697bdc 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1341,6 +1341,7 @@ struct drm_i915_private {
>>   	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
>>   
>>   	struct intel_uncore uncore;
>> +	struct intel_uncore de_uncore;
>>   	struct intel_uncore_mmio_debug mmio_debug;
>>   
>>   	struct i915_virtual_gpu vgpu;
>> @@ -2746,6 +2747,63 @@ 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__))
>>   
>> +/*
>> + * The following are mmio-accessors that use an independent lock and skip all
>> + * the forcewake logic, to be used to access display registers, which are
>> + * outside the GT forcewake wells.
>> + */
>> +static inline void
>> +intel_assert_register_is_display(struct drm_i915_private *i915, i915_reg_t reg)
>> +{
>> +#ifdef CONFIG_DRM_I915_DEBUG_MMIO
>> +	u32 offset = i915_mmio_reg_offset(reg);
>> +	bool is_de_register;
>> +
>> +	if (INTEL_GEN(i915) >= 5) {
>> +		is_de_register = offset >= 0x40000 &&
>> +			(INTEL_GEN(i915) < 11 || offset < 0x1c0000);
> 
> I think the gen11+ thing is wrong. There are eg. PHY registers
> above 0x162000.

But that's still below 0x1c0000. Media engines start at 0x1c0000, so 
we're back in GT-land at least until 0x1dffff.

> 
>> +	} else {
>> +		is_de_register =
>> +			(offset >= 0x5000 && offset <= 0x5fff) ||
>> +			(offset >= 0x6000 && offset <= 0x6fff) ||
>> +			(offset >= 0xa000 && offset <= 0xafff) ||
>> +			offset >= 0x30000;
> 
> I think the WARNs are coming from the mchbar range (0x10000+ on
> these platforms). Not really sure if we should designate that
> gt or de.
> 
> Unfortunately the register ranges don't seem to follow much
> of a sensible pattern. Eg. we seem to have both gt and de
> related stuff around the 0x130000 mark :(
> 

Should I just drop the check for now then? Alternatively, I could change 
to WARN if we try to access a register that is inside one of the 
forcewake ranges if CONFIG_DRM_I915_DEBUG_MMIO is set.

Daniele

> Maybe we should try separate only the pipe/plane registers
> which are part of your typical atomic commit under their
> own lock. Could even do per-pipe locks maybe...
> 
>> +	}
>> +
>> +	WARN_ONCE(!is_de_register,
>> +		  "display reg access function used for non-display reg 0x%08x\n",
>> +		  offset);
>> +#endif
>> +}
>> +
>> +static inline u32
>> +intel_de_read(struct drm_i915_private *i915, i915_reg_t reg)
>> +{
>> +	intel_assert_register_is_display(i915, reg);
>> +	return intel_uncore_read(&i915->de_uncore, reg);
>> +}
>> +
>> +static inline void
>> +intel_de_posting_read(struct drm_i915_private *i915, i915_reg_t reg)
>> +{
>> +	intel_assert_register_is_display(i915, reg);
>> +	intel_uncore_posting_read(&i915->de_uncore, reg);
>> +}
>> +
>> +static inline void
>> +intel_de_write(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
>> +{
>> +	intel_assert_register_is_display(i915, reg);
>> +	intel_uncore_write(&i915->de_uncore, reg, val);
>> +}
>> +
>> +static inline void
>> +intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 set)
>> +{
>> +	intel_assert_register_is_display(i915, reg);
>> +	intel_uncore_rmw(&i915->de_uncore, reg, clear, set);
>> +}
>> +
>>   /* "Broadcast RGB" property */
>>   #define INTEL_BROADCAST_RGB_AUTO 0
>>   #define INTEL_BROADCAST_RGB_FULL 1
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index 1905fd94dd3c..94e153acb0af 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1674,6 +1674,12 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
>>   	return 0;
>>   }
>>   
>> +static bool
>> +intel_uncore_is_display(const struct intel_uncore *uncore)
>> +{
>> +	return uncore == &uncore->i915->de_uncore;
>> +}
>> +
>>   int intel_uncore_init_mmio(struct intel_uncore *uncore)
>>   {
>>   	struct drm_i915_private *i915 = uncore->i915;
>> @@ -1683,7 +1689,8 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>>   	if (ret)
>>   		return ret;
>>   
>> -	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
>> +	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915) &&
>> +	    !intel_uncore_is_display(uncore))
>>   		uncore->flags |= UNCORE_HAS_FORCEWAKE;
>>   
>>   	if (!intel_uncore_has_forcewake(uncore)) {
>> -- 
>> 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] 15+ messages in thread

* Re: [RFC 3/4] drm/i915: introduce display_uncore
  2019-06-26 20:27     ` Daniele Ceraolo Spurio
@ 2019-06-27 12:41       ` Ville Syrjälä
  0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2019-06-27 12:41 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

On Wed, Jun 26, 2019 at 01:27:57PM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 6/26/19 11:42 AM, Ville Syrjälä wrote:
> > On Mon, Jun 24, 2019 at 01:31:51PM -0700, Daniele Ceraolo Spurio wrote:
> >> A forcewake-less uncore to be used to decouple GT accesses from display
> >> ones to avoid serializing them when there is no need.
> >>
> >> New accessors that implicitly use the new uncore have also been added.
> >>
> >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/i915_drv.c     | 17 ++++++++-
> >>   drivers/gpu/drm/i915/i915_drv.h     | 58 +++++++++++++++++++++++++++++
> >>   drivers/gpu/drm/i915/intel_uncore.c |  9 ++++-
> >>   3 files changed, 81 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index 2d6c643dde51..e22c0a6b3992 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
> >>   
> >>   	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
> >>   	intel_uncore_init_early(&dev_priv->uncore, dev_priv);
> >> +	intel_uncore_init_early(&dev_priv->de_uncore, dev_priv);
> >>   
> >>   	spin_lock_init(&dev_priv->irq_lock);
> >>   	spin_lock_init(&dev_priv->gpu_error.lock);
> >> @@ -1001,6 +1002,10 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
> >>   	if (ret < 0)
> >>   		goto err_bridge;
> >>   
> >> +	ret = intel_uncore_init_mmio(&dev_priv->de_uncore);
> >> +	if (ret < 0)
> >> +		goto err_uncore;
> >> +
> >>   	/* Try to make sure MCHBAR is enabled before poking at it */
> >>   	intel_setup_mchbar(dev_priv);
> >>   
> >> @@ -1012,14 +1017,16 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
> >>   
> >>   	ret = intel_engines_init_mmio(dev_priv);
> >>   	if (ret)
> >> -		goto err_uncore;
> >> +		goto err_mchbar;
> >>   
> >>   	i915_gem_init_mmio(dev_priv);
> >>   
> >>   	return 0;
> >>   
> >> -err_uncore:
> >> +err_mchbar:
> >>   	intel_teardown_mchbar(dev_priv);
> >> +	intel_uncore_fini_mmio(&dev_priv->de_uncore);
> >> +err_uncore:
> >>   	intel_uncore_fini_mmio(&dev_priv->uncore);
> >>   err_bridge:
> >>   	pci_dev_put(dev_priv->bridge_dev);
> >> @@ -1034,6 +1041,7 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
> >>   static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
> >>   {
> >>   	intel_teardown_mchbar(dev_priv);
> >> +	intel_uncore_fini_mmio(&dev_priv->de_uncore);
> >>   	intel_uncore_fini_mmio(&dev_priv->uncore);
> >>   	pci_dev_put(dev_priv->bridge_dev);
> >>   }
> >> @@ -2166,6 +2174,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
> >>   
> >>   	i915_gem_suspend_late(dev_priv);
> >>   
> >> +	intel_uncore_suspend(&dev_priv->de_uncore);
> >>   	intel_uncore_suspend(&dev_priv->uncore);
> >>   
> >>   	intel_power_domains_suspend(dev_priv,
> >> @@ -2363,6 +2372,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> >>   			  ret);
> >>   
> >>   	intel_uncore_resume_early(&dev_priv->uncore);
> >> +	intel_uncore_resume_early(&dev_priv->de_uncore);
> >>   
> >>   	intel_gt_check_and_clear_faults(&dev_priv->gt);
> >>   
> >> @@ -2937,6 +2947,7 @@ static int intel_runtime_suspend(struct device *kdev)
> >>   
> >>   	intel_runtime_pm_disable_interrupts(dev_priv);
> >>   
> >> +	intel_uncore_suspend(&dev_priv->de_uncore);
> >>   	intel_uncore_suspend(&dev_priv->uncore);
> >>   
> >>   	ret = 0;
> >> @@ -2955,6 +2966,7 @@ static int intel_runtime_suspend(struct device *kdev)
> >>   	if (ret) {
> >>   		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
> >>   		intel_uncore_runtime_resume(&dev_priv->uncore);
> >> +		intel_uncore_runtime_resume(&dev_priv->de_uncore);
> >>   
> >>   		intel_runtime_pm_enable_interrupts(dev_priv);
> >>   
> >> @@ -3053,6 +3065,7 @@ static int intel_runtime_resume(struct device *kdev)
> >>   	}
> >>   
> >>   	intel_uncore_runtime_resume(&dev_priv->uncore);
> >> +	intel_uncore_runtime_resume(&dev_priv->de_uncore);
> >>   
> >>   	intel_runtime_pm_enable_interrupts(dev_priv);
> >>   
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index a53b65d78159..c554b7697bdc 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -1341,6 +1341,7 @@ struct drm_i915_private {
> >>   	resource_size_t stolen_usable_size;	/* Total size minus reserved ranges */
> >>   
> >>   	struct intel_uncore uncore;
> >> +	struct intel_uncore de_uncore;
> >>   	struct intel_uncore_mmio_debug mmio_debug;
> >>   
> >>   	struct i915_virtual_gpu vgpu;
> >> @@ -2746,6 +2747,63 @@ 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__))
> >>   
> >> +/*
> >> + * The following are mmio-accessors that use an independent lock and skip all
> >> + * the forcewake logic, to be used to access display registers, which are
> >> + * outside the GT forcewake wells.
> >> + */
> >> +static inline void
> >> +intel_assert_register_is_display(struct drm_i915_private *i915, i915_reg_t reg)
> >> +{
> >> +#ifdef CONFIG_DRM_I915_DEBUG_MMIO
> >> +	u32 offset = i915_mmio_reg_offset(reg);
> >> +	bool is_de_register;
> >> +
> >> +	if (INTEL_GEN(i915) >= 5) {
> >> +		is_de_register = offset >= 0x40000 &&
> >> +			(INTEL_GEN(i915) < 11 || offset < 0x1c0000);
> > 
> > I think the gen11+ thing is wrong. There are eg. PHY registers
> > above 0x162000.
> 
> But that's still below 0x1c0000. Media engines start at 0x1c0000, so 
> we're back in GT-land at least until 0x1dffff.

Doh. Misread the offset.

> 
> > 
> >> +	} else {
> >> +		is_de_register =
> >> +			(offset >= 0x5000 && offset <= 0x5fff) ||
> >> +			(offset >= 0x6000 && offset <= 0x6fff) ||
> >> +			(offset >= 0xa000 && offset <= 0xafff) ||
> >> +			offset >= 0x30000;
> > 
> > I think the WARNs are coming from the mchbar range (0x10000+ on
> > these platforms). Not really sure if we should designate that
> > gt or de.
> > 
> > Unfortunately the register ranges don't seem to follow much
> > of a sensible pattern. Eg. we seem to have both gt and de
> > related stuff around the 0x130000 mark :(
> > 
> 
> Should I just drop the check for now then? Alternatively, I could change 
> to WARN if we try to access a register that is inside one of the 
> forcewake ranges if CONFIG_DRM_I915_DEBUG_MMIO is set.

Would be nice to have a warning like this so we don't accidentally end
up touching the same register (or cacheline) under two different locks
because that could trigger a system hang.

I guess from correcness POV it doesn't really matter which ranges are
under which lock. So we could try to include the mchbar range under
the de lock on old platforms as well. Maybe just s/0x30000/0x10000/ on
old platforms? I don't immediately see anything gt related above
0x10000. Hmm. But that still leaves out ilk which still has mchbar
at 0x10000. So we'd need to account for that as well.

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

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

end of thread, other threads:[~2019-06-27 12:41 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 20:31 [RFC 0/4] Display uncore Daniele Ceraolo Spurio
2019-06-24 20:31 ` [RFC 1/4] drm/i915: split out uncore_mmio_debug Daniele Ceraolo Spurio
2019-06-26 10:02   ` Chris Wilson
2019-06-26 17:38     ` Daniele Ceraolo Spurio
2019-06-26 17:58       ` Chris Wilson
2019-06-26 18:20         ` Daniele Ceraolo Spurio
2019-06-24 20:31 ` [RFC 2/4] drm/i915: rework mmio debug stop/start Daniele Ceraolo Spurio
2019-06-24 20:31 ` [RFC 3/4] drm/i915: introduce display_uncore Daniele Ceraolo Spurio
2019-06-26 18:42   ` Ville Syrjälä
2019-06-26 20:27     ` Daniele Ceraolo Spurio
2019-06-27 12:41       ` Ville Syrjälä
2019-06-24 20:31 ` [RFC 4/4] drm/i915: convert intel_hdmi to display reg accessors Daniele Ceraolo Spurio
2019-06-24 20:43 ` ✗ Fi.CI.SPARSE: warning for Display uncore (rev2) Patchwork
2019-06-24 21:37 ` ✓ Fi.CI.BAT: success " Patchwork
2019-06-25  1:04 ` ✓ 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.