[RFC v2 00/14] Implicit dev_priv removal

[RFC v2 00/14] Implicit dev_priv removal

[Tvrtko Ursulin]

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

Mostly patches reworking the code and GEM init paths to remove some implicit
dev_priv dependencies (I915_READ/I915_WRITE), plus some small tweaks to tidy
GEM init paths to use more logical input parameters (enabled by the conversion
to uncore mmio accessors).

v2:
 * Introduce struct intel_gt by popular demand.
 * Compile tested only this time.

Tvrtko Ursulin (14):
  drm/i915: Make i915_check_and_clear_faults take uncore
  drm/i915: Convert intel_vgt_(de)balloon to uncore
  drm/i915: Introduce struct intel_gt as replacement for anonymous
    i915->gt
  drm/i915: Add a couple intel_gt helpers
  drm/i915: Convert i915_gem_init_swizzling to intel_gt
  drm/i915: Convert init_unused_rings to intel_gt
  drm/i915: Convert gt workarounds to intel_gt
  drm/i915: Store backpointer to intel_gt in the engine
  drm/i915: Convert intel_mocs_init_l3cc_table to intel_gt
  drm/i915: Convert i915_ppgtt_init_hw to intel_gt
  drm/i915: Consolidate some open coded mmio rmw
  drm/i915: Convert i915_gem_init_hw to intel_gt
  drm/i915/guc: Move intel_guc_reserved_gtt_size to intel_wopcm_guc_size
  drm/i915: Make GuC GGTT reservation work on ggtt

 drivers/gpu/drm/i915/Makefile                |   1 +
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    |   3 +-
 drivers/gpu/drm/i915/gt/intel_engine_types.h |   2 +
 drivers/gpu/drm/i915/gt/intel_gt.c           |  44 ++++++
 drivers/gpu/drm/i915/gt/intel_gt.h           |  28 ++++
 drivers/gpu/drm/i915/gt/intel_gt_types.h     |  53 +++++++
 drivers/gpu/drm/i915/gt/intel_mocs.c         |  52 ++++---
 drivers/gpu/drm/i915/gt/intel_mocs.h         |   3 +-
 drivers/gpu/drm/i915/gt/intel_reset.c        |  28 ++--
 drivers/gpu/drm/i915/gt/intel_reset.h        |   2 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c  |  11 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.h  |   6 +-
 drivers/gpu/drm/i915/i915_drv.c              |   7 +-
 drivers/gpu/drm/i915/i915_drv.h              |  35 +----
 drivers/gpu/drm/i915/i915_gem.c              | 130 ++++++++--------
 drivers/gpu/drm/i915/i915_gem_gtt.c          | 148 ++++++++++++-------
 drivers/gpu/drm/i915/i915_gem_gtt.h          |   3 +-
 drivers/gpu/drm/i915/i915_vgpu.c             |  24 +--
 drivers/gpu/drm/i915/i915_vgpu.h             |   4 +-
 drivers/gpu/drm/i915/intel_guc.c             |  44 ------
 drivers/gpu/drm/i915/intel_guc.h             |   3 -
 drivers/gpu/drm/i915/intel_wopcm.h           |  17 +++
 22 files changed, 390 insertions(+), 258 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_gt.c
 create mode 100644 drivers/gpu/drm/i915/gt/intel_gt.h
 create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_types.h

-- 
2.20.1

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

[RFC 01/14] drm/i915: Make i915_check_and_clear_faults take uncore

[Tvrtko Ursulin]

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

Continuing the conversion and elimination of implicit dev_priv.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_reset.c     | 28 ++++++++++++-----------
 drivers/gpu/drm/i915/gt/intel_reset.h     |  2 +-
 drivers/gpu/drm/i915/i915_drv.c           |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c       |  4 ++--
 5 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index c0d986db5a75..a046e8dccc96 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -453,7 +453,7 @@ int intel_engines_init_mmio(struct drm_i915_private *i915)
 
 	RUNTIME_INFO(i915)->num_engines = hweight32(mask);
 
-	i915_check_and_clear_faults(i915);
+	i915_check_and_clear_faults(&i915->uncore);
 
 	intel_setup_engine_capabilities(i915);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 60d24110af80..13471916559b 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1166,10 +1166,10 @@ static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
 	GEN6_RING_FAULT_REG_POSTING_READ(engine);
 }
 
-static void clear_error_registers(struct drm_i915_private *i915,
+static void clear_error_registers(struct intel_uncore *uncore,
 				  intel_engine_mask_t engine_mask)
 {
-	struct intel_uncore *uncore = &i915->uncore;
+	struct drm_i915_private *i915 = uncore_to_i915(uncore);
 	u32 eir;
 
 	if (!IS_GEN(i915, 2))
@@ -1205,13 +1205,13 @@ static void clear_error_registers(struct drm_i915_private *i915,
 	}
 }
 
-static void gen6_check_faults(struct drm_i915_private *dev_priv)
+static void gen6_check_faults(struct intel_uncore *uncore)
 {
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	u32 fault;
 
-	for_each_engine(engine, dev_priv, id) {
+	for_each_engine(engine, uncore_to_i915(uncore), id) {
 		fault = GEN6_RING_FAULT_REG_READ(engine);
 		if (fault & RING_FAULT_VALID) {
 			DRM_DEBUG_DRIVER("Unexpected fault\n"
@@ -1227,16 +1227,16 @@ static void gen6_check_faults(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void gen8_check_faults(struct drm_i915_private *dev_priv)
+static void gen8_check_faults(struct intel_uncore *uncore)
 {
-	u32 fault = I915_READ(GEN8_RING_FAULT_REG);
+	u32 fault = intel_uncore_read(uncore, GEN8_RING_FAULT_REG);
 
 	if (fault & RING_FAULT_VALID) {
 		u32 fault_data0, fault_data1;
 		u64 fault_addr;
 
-		fault_data0 = I915_READ(GEN8_FAULT_TLB_DATA0);
-		fault_data1 = I915_READ(GEN8_FAULT_TLB_DATA1);
+		fault_data0 = intel_uncore_read(uncore, GEN8_FAULT_TLB_DATA0);
+		fault_data1 = intel_uncore_read(uncore, GEN8_FAULT_TLB_DATA1);
 		fault_addr = ((u64)(fault_data1 & FAULT_VA_HIGH_BITS) << 44) |
 			     ((u64)fault_data0 << 12);
 
@@ -1255,17 +1255,19 @@ static void gen8_check_faults(struct drm_i915_private *dev_priv)
 	}
 }
 
-void i915_check_and_clear_faults(struct drm_i915_private *i915)
+void i915_check_and_clear_faults(struct intel_uncore *uncore)
 {
+	struct drm_i915_private *i915 = uncore_to_i915(uncore);
+
 	/* From GEN8 onwards we only have one 'All Engine Fault Register' */
 	if (INTEL_GEN(i915) >= 8)
-		gen8_check_faults(i915);
+		gen8_check_faults(uncore);
 	else if (INTEL_GEN(i915) >= 6)
-		gen6_check_faults(i915);
+		gen6_check_faults(uncore);
 	else
 		return;
 
-	clear_error_registers(i915, ALL_ENGINES);
+	clear_error_registers(uncore, ALL_ENGINES);
 }
 
 /**
@@ -1316,7 +1318,7 @@ void i915_handle_error(struct drm_i915_private *i915,
 
 	if (flags & I915_ERROR_CAPTURE) {
 		i915_capture_error_state(i915, engine_mask, msg);
-		clear_error_registers(i915, engine_mask);
+		clear_error_registers(&i915->uncore, engine_mask);
 	}
 
 	/*
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h
index 580ebdb59eca..d5cf217ba719 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.h
+++ b/drivers/gpu/drm/i915/gt/intel_reset.h
@@ -25,7 +25,7 @@ void i915_handle_error(struct drm_i915_private *i915,
 		       const char *fmt, ...);
 #define I915_ERROR_CAPTURE BIT(0)
 
-void i915_check_and_clear_faults(struct drm_i915_private *i915);
+void i915_check_and_clear_faults(struct intel_uncore *uncore);
 
 void i915_reset(struct drm_i915_private *i915,
 		intel_engine_mask_t stalled_mask,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 81ff2c78fd55..ee9af4293133 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2340,7 +2340,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
 	intel_uncore_resume_early(&dev_priv->uncore);
 
-	i915_check_and_clear_faults(dev_priv);
+	i915_check_and_clear_faults(&dev_priv->uncore);
 
 	if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
 		gen9_sanitize_dc_state(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2e15850bd987..904cdabc5b64 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2310,7 +2310,7 @@ void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv)
 	if (INTEL_GEN(dev_priv) < 6)
 		return;
 
-	i915_check_and_clear_faults(dev_priv);
+	i915_check_and_clear_faults(&dev_priv->uncore);
 
 	ggtt->vm.clear_range(&ggtt->vm, 0, ggtt->vm.total);
 
@@ -3588,7 +3588,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct i915_vma *vma, *vn;
 
-	i915_check_and_clear_faults(dev_priv);
+	i915_check_and_clear_faults(&dev_priv->uncore);
 
 	mutex_lock(&ggtt->vm.mutex);
 
-- 
2.20.1

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

[RFC 02/14] drm/i915: Convert intel_vgt_(de)balloon to uncore

[Tvrtko Ursulin]

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

More removal of implicit dev_priv from using old mmio accessors.

Furthermore these calls really operate on ggtt so it logically makes sense
if they take it as parameter.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c |  4 ++--
 drivers/gpu/drm/i915/i915_vgpu.c    | 24 ++++++++++++++----------
 drivers/gpu/drm/i915/i915_vgpu.h    |  4 ++--
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 904cdabc5b64..50a74674a2a5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2832,7 +2832,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE,
 			       intel_guc_reserved_gtt_size(&dev_priv->guc));
 
-	ret = intel_vgt_balloon(dev_priv);
+	ret = intel_vgt_balloon(ggtt);
 	if (ret)
 		return ret;
 
@@ -2900,7 +2900,7 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
 	intel_guc_release_ggtt_top(&dev_priv->guc);
 
 	if (drm_mm_initialized(&ggtt->vm.mm)) {
-		intel_vgt_deballoon(dev_priv);
+		intel_vgt_deballoon(ggtt);
 		i915_address_space_fini(&ggtt->vm);
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 94d3992b599d..41ed9a3f52b4 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -117,17 +117,17 @@ static void vgt_deballoon_space(struct i915_ggtt *ggtt,
  * This function is called to deallocate the ballooned-out graphic memory, when
  * driver is unloaded or when ballooning fails.
  */
-void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
+void intel_vgt_deballoon(struct i915_ggtt *ggtt)
 {
 	int i;
 
-	if (!intel_vgpu_active(dev_priv))
+	if (!intel_vgpu_active(ggtt->vm.i915))
 		return;
 
 	DRM_DEBUG("VGT deballoon.\n");
 
 	for (i = 0; i < 4; i++)
-		vgt_deballoon_space(&dev_priv->ggtt, &bl_info.space[i]);
+		vgt_deballoon_space(ggtt, &bl_info.space[i]);
 }
 
 static int vgt_balloon_space(struct i915_ggtt *ggtt,
@@ -195,22 +195,26 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
  * Returns:
  * zero on success, non-zero if configuration invalid or ballooning failed
  */
-int intel_vgt_balloon(struct drm_i915_private *dev_priv)
+int intel_vgt_balloon(struct i915_ggtt *ggtt)
 {
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
+	struct intel_uncore *uncore = &ggtt->vm.i915->uncore;
 	unsigned long ggtt_end = ggtt->vm.total;
 
 	unsigned long mappable_base, mappable_size, mappable_end;
 	unsigned long unmappable_base, unmappable_size, unmappable_end;
 	int ret;
 
-	if (!intel_vgpu_active(dev_priv))
+	if (!intel_vgpu_active(ggtt->vm.i915))
 		return 0;
 
-	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
-	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
-	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
-	unmappable_size = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.size));
+	mappable_base =
+	  intel_uncore_read(uncore, vgtif_reg(avail_rs.mappable_gmadr.base));
+	mappable_size =
+	  intel_uncore_read(uncore, vgtif_reg(avail_rs.mappable_gmadr.size));
+	unmappable_base =
+	  intel_uncore_read(uncore, vgtif_reg(avail_rs.nonmappable_gmadr.base));
+	unmappable_size =
+	  intel_uncore_read(uncore, vgtif_reg(avail_rs.nonmappable_gmadr.size));
 
 	mappable_end = mappable_base + mappable_size;
 	unmappable_end = unmappable_base + unmappable_size;
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index ebe1b7bced98..e918f418503f 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -42,7 +42,7 @@ intel_vgpu_has_huge_gtt(struct drm_i915_private *dev_priv)
 	return dev_priv->vgpu.caps & VGT_CAPS_HUGE_GTT;
 }
 
-int intel_vgt_balloon(struct drm_i915_private *dev_priv);
-void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
+int intel_vgt_balloon(struct i915_ggtt *ggtt);
+void intel_vgt_deballoon(struct i915_ggtt *ggtt);
 
 #endif /* _I915_VGPU_H_ */
-- 
2.20.1

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

[RFC 03/14] drm/i915: Introduce struct intel_gt as replacement for anonymous i915->gt

[Tvrtko Ursulin]

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

We have long been slighlty annoyed by the anonymous i915->gt.

Promote it to a separate structure and give it its own header.

This is a first step towards cleaning up the separation between i915 and gt.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_types.h | 53 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h          | 34 +--------------
 2 files changed, 55 insertions(+), 32 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_types.h

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
new file mode 100644
index 000000000000..dcdb18e0dd84
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -0,0 +1,53 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __INTEL_GT_TYPES__
+#define __INTEL_GT_TYPES__
+
+#include <linux/ktime.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include "i915_vma.h"
+#include "intel_wakeref.h"
+
+struct intel_gt {
+	struct i915_gt_timelines {
+		struct mutex mutex; /* protects list, tainted by GPU */
+		struct list_head active_list;
+
+		/* Pack multiple timelines' seqnos into the same page */
+		spinlock_t hwsp_lock;
+		struct list_head hwsp_free_list;
+	} timelines;
+
+	struct list_head active_rings;
+
+	struct intel_wakeref wakeref;
+
+	struct list_head closed_vma;
+	spinlock_t closed_lock; /* guards the list of closed_vma */
+
+	/**
+	 * Is the GPU currently considered idle, or busy executing
+	 * userspace requests? Whilst idle, we allow runtime power
+	 * management to power down the hardware and display clocks.
+	 * In order to reduce the effect on performance, there
+	 * is a slight delay before we do so.
+	 */
+	intel_wakeref_t awake;
+
+	struct blocking_notifier_head pm_notifications;
+
+	ktime_t last_init_time;
+
+	struct i915_vma *scratch;
+};
+
+#endif /* __INTEL_GT_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b39b6e526189..2cca65633bcc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -65,6 +65,7 @@
 
 #include "gt/intel_lrc.h"
 #include "gt/intel_engine.h"
+#include "gt/intel_gt_types.h"
 #include "gt/intel_workarounds.h"
 
 #include "intel_bios.h"
@@ -1888,38 +1889,7 @@ struct drm_i915_private {
 	} perf;
 
 	/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
-	struct {
-		struct i915_gt_timelines {
-			struct mutex mutex; /* protects list, tainted by GPU */
-			struct list_head active_list;
-
-			/* Pack multiple timelines' seqnos into the same page */
-			spinlock_t hwsp_lock;
-			struct list_head hwsp_free_list;
-		} timelines;
-
-		struct list_head active_rings;
-
-		struct intel_wakeref wakeref;
-
-		struct list_head closed_vma;
-		spinlock_t closed_lock; /* guards the list of closed_vma */
-
-		/**
-		 * Is the GPU currently considered idle, or busy executing
-		 * userspace requests? Whilst idle, we allow runtime power
-		 * management to power down the hardware and display clocks.
-		 * In order to reduce the effect on performance, there
-		 * is a slight delay before we do so.
-		 */
-		intel_wakeref_t awake;
-
-		struct blocking_notifier_head pm_notifications;
-
-		ktime_t last_init_time;
-
-		struct i915_vma *scratch;
-	} gt;
+	struct intel_gt gt;
 
 	struct {
 		struct notifier_block pm_notifier;
-- 
2.20.1

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

[RFC 04/14] drm/i915: Add a couple intel_gt helpers

[Tvrtko Ursulin]

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

Two trivial helpers to convert from intel_gt to i915 and uncore which will
be needed by the following patches.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_gt.h

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
new file mode 100644
index 000000000000..b672f8b03bfd
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -0,0 +1,26 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __INTEL_GT__
+#define __INTEL_GT__
+
+#include "gt/intel_gt_types.h"
+
+#include "intel_uncore.h"
+
+#include "i915_drv.h"
+
+static inline struct drm_i915_private *gt_to_i915(struct intel_gt *gt)
+{
+	return container_of(gt, struct drm_i915_private, gt);
+}
+
+static inline struct intel_uncore *gt_to_uncore(struct intel_gt *gt)
+{
+	return &gt_to_i915(gt)->uncore;
+}
+
+#endif /* __INTEL_GT_H__ */
\ No newline at end of file
-- 
2.20.1

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

[RFC 05/14] drm/i915: Convert i915_gem_init_swizzling to intel_gt

[Tvrtko Ursulin]

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

Start using the newly introduced struct intel_gt to fuse together correct
logical init flow with uncore for more removal of implicit dev_priv in
mmio access.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/Makefile      |  1 +
 drivers/gpu/drm/i915/gt/intel_gt.c | 44 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt.h |  2 ++
 drivers/gpu/drm/i915/i915_drv.c    |  5 ++--
 drivers/gpu/drm/i915/i915_drv.h    |  1 -
 drivers/gpu/drm/i915/i915_gem.c    | 26 ++----------------
 6 files changed, 52 insertions(+), 27 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_gt.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c0a7b2994077..8df1bf2855d0 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -73,6 +73,7 @@ gt-y += \
 	gt/intel_context.o \
 	gt/intel_engine_cs.o \
 	gt/intel_engine_pm.o \
+	gt/intel_gt.o \
 	gt/intel_gt_pm.o \
 	gt/intel_hangcheck.o \
 	gt/intel_lrc.o \
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
new file mode 100644
index 000000000000..4bc0bb4d343e
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -0,0 +1,44 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2008-2018 Intel Corporation
+ */
+
+#include "intel_gt.h"
+
+void intel_gt_init_swizzling(struct intel_gt *gt)
+{
+	struct drm_i915_private *i915 = gt_to_i915(gt);
+	struct intel_uncore *uncore = gt_to_uncore(gt);
+
+	if (INTEL_GEN(i915) < 5 ||
+	    i915->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
+		return;
+
+	intel_uncore_write(uncore,
+			   DISP_ARB_CTL,
+			   intel_uncore_read(uncore, DISP_ARB_CTL) |
+			   DISP_TILE_SURFACE_SWIZZLING);
+
+	if (IS_GEN(i915, 5))
+		return;
+
+	intel_uncore_write(uncore,
+			   TILECTL,
+			   intel_uncore_read(uncore, TILECTL) | TILECTL_SWZCTL);
+
+	if (IS_GEN(i915, 6))
+		intel_uncore_write(uncore,
+				   ARB_MODE,
+				   _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_SNB));
+	else if (IS_GEN(i915, 7))
+		intel_uncore_write(uncore,
+				   ARB_MODE,
+				   _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_IVB));
+	else if (IS_GEN(i915, 8))
+		intel_uncore_write(uncore,
+				   GAMTARBMODE,
+				   _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_BDW));
+	else
+		MISSING_CASE(INTEL_GEN(i915));
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
index b672f8b03bfd..20594e710356 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -23,4 +23,6 @@ static inline struct intel_uncore *gt_to_uncore(struct intel_gt *gt)
 	return &gt_to_i915(gt)->uncore;
 }
 
+void intel_gt_init_swizzling(struct intel_gt *gt);
+
 #endif /* __INTEL_GT_H__ */
\ No newline at end of file
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ee9af4293133..56423f431613 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,6 +49,7 @@
 
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_ioctls.h"
+#include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_reset.h"
 #include "gt/intel_workarounds.h"
@@ -2935,7 +2936,7 @@ static int intel_runtime_suspend(struct device *kdev)
 
 		intel_uc_resume(dev_priv);
 
-		i915_gem_init_swizzling(dev_priv);
+		intel_gt_init_swizzling(&dev_priv->gt);
 		i915_gem_restore_fences(dev_priv);
 
 		enable_rpm_wakeref_asserts(dev_priv);
@@ -3036,7 +3037,7 @@ static int intel_runtime_resume(struct device *kdev)
 	 * No point of rolling back things in case of an error, as the best
 	 * we can do is to hope that things will still work (and disable RPM).
 	 */
-	i915_gem_init_swizzling(dev_priv);
+	intel_gt_init_swizzling(&dev_priv->gt);
 	i915_gem_restore_fences(dev_priv);
 
 	/*
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2cca65633bcc..19084549f44f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2606,7 +2606,6 @@ bool i915_gem_unset_wedged(struct drm_i915_private *dev_priv);
 void i915_gem_init_mmio(struct drm_i915_private *i915);
 int __must_check i915_gem_init(struct drm_i915_private *dev_priv);
 int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv);
-void i915_gem_init_swizzling(struct drm_i915_private *dev_priv);
 void i915_gem_fini_hw(struct drm_i915_private *dev_priv);
 void i915_gem_fini(struct drm_i915_private *dev_priv);
 int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 94d85b0fb860..0e0e6bb08e23 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -44,6 +44,7 @@
 #include "gem/i915_gem_pm.h"
 #include "gem/i915_gemfs.h"
 #include "gt/intel_engine_pm.h"
+#include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_mocs.h"
 #include "gt/intel_reset.h"
@@ -1200,29 +1201,6 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
 	mutex_unlock(&i915->drm.struct_mutex);
 }
 
-void i915_gem_init_swizzling(struct drm_i915_private *dev_priv)
-{
-	if (INTEL_GEN(dev_priv) < 5 ||
-	    dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
-		return;
-
-	I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
-				 DISP_TILE_SURFACE_SWIZZLING);
-
-	if (IS_GEN(dev_priv, 5))
-		return;
-
-	I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_SWZCTL);
-	if (IS_GEN(dev_priv, 6))
-		I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_SNB));
-	else if (IS_GEN(dev_priv, 7))
-		I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_IVB));
-	else if (IS_GEN(dev_priv, 8))
-		I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_BDW));
-	else
-		BUG();
-}
-
 static void init_unused_ring(struct drm_i915_private *dev_priv, u32 base)
 {
 	I915_WRITE(RING_CTL(base), 0);
@@ -1269,7 +1247,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	/* ...and determine whether they are sticking. */
 	intel_gt_verify_workarounds(dev_priv, "init");
 
-	i915_gem_init_swizzling(dev_priv);
+	intel_gt_init_swizzling(&dev_priv->gt);
 
 	/*
 	 * At least 830 can leave some of the unused rings
-- 
2.20.1

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

[RFC 06/14] drm/i915: Convert init_unused_rings to intel_gt

[Tvrtko Ursulin]

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

More removal of implicit dev_priv from using old mmio accessors.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 42 ++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0e0e6bb08e23..cc8d289814cd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1201,28 +1201,32 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
 	mutex_unlock(&i915->drm.struct_mutex);
 }
 
-static void init_unused_ring(struct drm_i915_private *dev_priv, u32 base)
+static void init_unused_ring(struct intel_gt *gt, u32 base)
 {
-	I915_WRITE(RING_CTL(base), 0);
-	I915_WRITE(RING_HEAD(base), 0);
-	I915_WRITE(RING_TAIL(base), 0);
-	I915_WRITE(RING_START(base), 0);
+	struct intel_uncore *uncore = gt_to_uncore(gt);
+
+	intel_uncore_write(uncore, RING_CTL(base), 0);
+	intel_uncore_write(uncore, RING_HEAD(base), 0);
+	intel_uncore_write(uncore, RING_TAIL(base), 0);
+	intel_uncore_write(uncore, RING_START(base), 0);
 }
 
-static void init_unused_rings(struct drm_i915_private *dev_priv)
+static void init_unused_rings(struct intel_gt *gt)
 {
-	if (IS_I830(dev_priv)) {
-		init_unused_ring(dev_priv, PRB1_BASE);
-		init_unused_ring(dev_priv, SRB0_BASE);
-		init_unused_ring(dev_priv, SRB1_BASE);
-		init_unused_ring(dev_priv, SRB2_BASE);
-		init_unused_ring(dev_priv, SRB3_BASE);
-	} else if (IS_GEN(dev_priv, 2)) {
-		init_unused_ring(dev_priv, SRB0_BASE);
-		init_unused_ring(dev_priv, SRB1_BASE);
-	} else if (IS_GEN(dev_priv, 3)) {
-		init_unused_ring(dev_priv, PRB1_BASE);
-		init_unused_ring(dev_priv, PRB2_BASE);
+	struct drm_i915_private *i915 = gt_to_i915(gt);
+
+	if (IS_I830(i915)) {
+		init_unused_ring(gt, PRB1_BASE);
+		init_unused_ring(gt, SRB0_BASE);
+		init_unused_ring(gt, SRB1_BASE);
+		init_unused_ring(gt, SRB2_BASE);
+		init_unused_ring(gt, SRB3_BASE);
+	} else if (IS_GEN(i915, 2)) {
+		init_unused_ring(gt, SRB0_BASE);
+		init_unused_ring(gt, SRB1_BASE);
+	} else if (IS_GEN(i915, 3)) {
+		init_unused_ring(gt, PRB1_BASE);
+		init_unused_ring(gt, PRB2_BASE);
 	}
 }
 
@@ -1255,7 +1259,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	 * will prevent c3 entry. Makes sure all unused rings
 	 * are totally idle.
 	 */
-	init_unused_rings(dev_priv);
+	init_unused_rings(&dev_priv->gt);
 
 	BUG_ON(!dev_priv->kernel_context);
 	ret = i915_terminally_wedged(dev_priv);
-- 
2.20.1

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

[RFC 07/14] drm/i915: Convert gt workarounds to intel_gt

[Tvrtko Ursulin]

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

More conversion of i915_gem_init_hw to uncore.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 11 ++++++-----
 drivers/gpu/drm/i915/gt/intel_workarounds.h |  6 +++---
 drivers/gpu/drm/i915/i915_gem.c             |  4 ++--
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 165b0a45e009..99f43158b7d6 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -6,6 +6,7 @@
 
 #include "i915_drv.h"
 #include "intel_context.h"
+#include "intel_gt.h"
 #include "intel_workarounds.h"
 
 /**
@@ -984,9 +985,9 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal)
 	spin_unlock_irqrestore(&uncore->lock, flags);
 }
 
-void intel_gt_apply_workarounds(struct drm_i915_private *i915)
+void intel_gt_apply_workarounds(struct intel_gt *gt)
 {
-	wa_list_apply(&i915->uncore, &i915->gt_wa_list);
+	wa_list_apply(gt_to_uncore(gt), &gt_to_i915(gt)->gt_wa_list);
 }
 
 static bool wa_list_verify(struct intel_uncore *uncore,
@@ -1005,10 +1006,10 @@ static bool wa_list_verify(struct intel_uncore *uncore,
 	return ok;
 }
 
-bool intel_gt_verify_workarounds(struct drm_i915_private *i915,
-				 const char *from)
+bool intel_gt_verify_workarounds(struct intel_gt *gt, const char *from)
 {
-	return wa_list_verify(&i915->uncore, &i915->gt_wa_list, from);
+	return wa_list_verify(gt_to_uncore(gt),
+			      &gt_to_i915(gt)->gt_wa_list, from);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h
index 3761a6ee58bb..8c9c769c2204 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h
@@ -14,6 +14,7 @@
 struct drm_i915_private;
 struct i915_request;
 struct intel_engine_cs;
+struct intel_gt;
 
 static inline void intel_wa_list_free(struct i915_wa_list *wal)
 {
@@ -25,9 +26,8 @@ void intel_engine_init_ctx_wa(struct intel_engine_cs *engine);
 int intel_engine_emit_ctx_wa(struct i915_request *rq);
 
 void intel_gt_init_workarounds(struct drm_i915_private *i915);
-void intel_gt_apply_workarounds(struct drm_i915_private *i915);
-bool intel_gt_verify_workarounds(struct drm_i915_private *i915,
-				 const char *from);
+void intel_gt_apply_workarounds(struct intel_gt *gt);
+bool intel_gt_verify_workarounds(struct intel_gt *gt, const char *from);
 
 void intel_engine_init_whitelist(struct intel_engine_cs *engine);
 void intel_engine_apply_whitelist(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cc8d289814cd..9dd014770e06 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1247,9 +1247,9 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 			   LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
 
 	/* Apply the GT workarounds... */
-	intel_gt_apply_workarounds(dev_priv);
+	intel_gt_apply_workarounds(&dev_priv->gt);
 	/* ...and determine whether they are sticking. */
-	intel_gt_verify_workarounds(dev_priv, "init");
+	intel_gt_verify_workarounds(&dev_priv->gt, "init");
 
 	intel_gt_init_swizzling(&dev_priv->gt);
 
-- 
2.20.1

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

[RFC 08/14] drm/i915: Store backpointer to intel_gt in the engine

[Tvrtko Ursulin]

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

It will come useful in the next patch.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 1 +
 drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index a046e8dccc96..d4385422e2b3 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -314,6 +314,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 	engine->id = id;
 	engine->mask = BIT(id);
 	engine->i915 = dev_priv;
+	engine->gt = &dev_priv->gt;
 	engine->uncore = &dev_priv->uncore;
 	__sprint_engine_name(engine->name, info);
 	engine->hw_id = engine->guc_id = info->hw_id;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 01223864237a..343c4459e8a3 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -34,6 +34,7 @@ struct drm_i915_reg_table;
 struct i915_gem_context;
 struct i915_request;
 struct i915_sched_attr;
+struct intel_gt;
 struct intel_uncore;
 
 typedef u8 intel_engine_mask_t;
@@ -266,6 +267,7 @@ struct intel_engine_execlists {
 
 struct intel_engine_cs {
 	struct drm_i915_private *i915;
+	struct intel_gt *gt;
 	struct intel_uncore *uncore;
 	char name[INTEL_ENGINE_CS_MAX_NAME];
 
-- 
2.20.1

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

[RFC 09/14] drm/i915: Convert intel_mocs_init_l3cc_table to intel_gt

[Tvrtko Ursulin]

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

More removal of implicit dev_priv from using old mmio accessors.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_mocs.c | 52 +++++++++++++++++-----------
 drivers/gpu/drm/i915/gt/intel_mocs.h |  3 +-
 drivers/gpu/drm/i915/i915_gem.c      |  2 +-
 3 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 79df66022d3a..1bb98f277407 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -23,6 +23,7 @@
 #include "i915_drv.h"
 
 #include "intel_engine.h"
+#include "intel_gt.h"
 #include "intel_mocs.h"
 #include "intel_lrc.h"
 
@@ -239,7 +240,7 @@ static const struct drm_i915_mocs_entry icelake_mocs_table[] = {
 
 /**
  * get_mocs_settings()
- * @dev_priv:	i915 device.
+ * @gt:		gt device
  * @table:      Output table that will be made to point at appropriate
  *	      MOCS values for the device.
  *
@@ -249,33 +250,34 @@ static const struct drm_i915_mocs_entry icelake_mocs_table[] = {
  *
  * Return: true if there are applicable MOCS settings for the device.
  */
-static bool get_mocs_settings(struct drm_i915_private *dev_priv,
+static bool get_mocs_settings(struct intel_gt *gt,
 			      struct drm_i915_mocs_table *table)
 {
+	struct drm_i915_private *i915 = gt_to_i915(gt);
 	bool result = false;
 
-	if (INTEL_GEN(dev_priv) >= 11) {
+	if (INTEL_GEN(i915) >= 11) {
 		table->size  = ARRAY_SIZE(icelake_mocs_table);
 		table->table = icelake_mocs_table;
 		table->n_entries = GEN11_NUM_MOCS_ENTRIES;
 		result = true;
-	} else if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN9_BC(i915) || IS_CANNONLAKE(i915)) {
 		table->size  = ARRAY_SIZE(skylake_mocs_table);
 		table->n_entries = GEN9_NUM_MOCS_ENTRIES;
 		table->table = skylake_mocs_table;
 		result = true;
-	} else if (IS_GEN9_LP(dev_priv)) {
+	} else if (IS_GEN9_LP(i915)) {
 		table->size  = ARRAY_SIZE(broxton_mocs_table);
 		table->n_entries = GEN9_NUM_MOCS_ENTRIES;
 		table->table = broxton_mocs_table;
 		result = true;
 	} else {
-		WARN_ONCE(INTEL_GEN(dev_priv) >= 9,
+		WARN_ONCE(INTEL_GEN(i915) >= 9,
 			  "Platform that should have a MOCS table does not.\n");
 	}
 
 	/* WaDisableSkipCaching:skl,bxt,kbl,glk */
-	if (IS_GEN(dev_priv, 9)) {
+	if (IS_GEN(i915, 9)) {
 		int i;
 
 		for (i = 0; i < table->size; i++)
@@ -330,12 +332,13 @@ static u32 get_entry_control(const struct drm_i915_mocs_table *table,
  */
 void intel_mocs_init_engine(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
+	struct intel_gt *gt = engine->gt;
+	struct intel_uncore *uncore = gt_to_uncore(gt);
 	struct drm_i915_mocs_table table;
 	unsigned int index;
 	u32 unused_value;
 
-	if (!get_mocs_settings(dev_priv, &table))
+	if (!get_mocs_settings(gt, &table))
 		return;
 
 	/* Set unused values to PTE */
@@ -344,12 +347,16 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 	for (index = 0; index < table.size; index++) {
 		u32 value = get_entry_control(&table, index);
 
-		I915_WRITE(mocs_register(engine->id, index), value);
+		intel_uncore_write(uncore,
+				   mocs_register(engine->id, index),
+				   value);
 	}
 
 	/* All remaining entries are also unused */
 	for (; index < table.n_entries; index++)
-		I915_WRITE(mocs_register(engine->id, index), unused_value);
+		intel_uncore_write(uncore,
+				   mocs_register(engine->id, index),
+				   unused_value);
 }
 
 /**
@@ -494,13 +501,14 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
  *
  * Return: Nothing.
  */
-void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv)
+void intel_mocs_init_l3cc_table(struct intel_gt *gt)
 {
+	struct intel_uncore *uncore = gt_to_uncore(gt);
 	struct drm_i915_mocs_table table;
 	unsigned int i;
 	u16 unused_value;
 
-	if (!get_mocs_settings(dev_priv, &table))
+	if (!get_mocs_settings(gt, &table))
 		return;
 
 	/* Set unused values to PTE */
@@ -510,23 +518,27 @@ void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv)
 		u16 low = get_entry_l3cc(&table, 2 * i);
 		u16 high = get_entry_l3cc(&table, 2 * i + 1);
 
-		I915_WRITE(GEN9_LNCFCMOCS(i),
-			   l3cc_combine(&table, low, high));
+		intel_uncore_write(uncore,
+				   GEN9_LNCFCMOCS(i),
+				   l3cc_combine(&table, low, high));
 	}
 
 	/* Odd table size - 1 left over */
 	if (table.size & 0x01) {
 		u16 low = get_entry_l3cc(&table, 2 * i);
 
-		I915_WRITE(GEN9_LNCFCMOCS(i),
-			   l3cc_combine(&table, low, unused_value));
+		intel_uncore_write(uncore,
+				   GEN9_LNCFCMOCS(i),
+				   l3cc_combine(&table, low, unused_value));
 		i++;
 	}
 
 	/* All remaining entries are also unused */
 	for (; i < table.n_entries / 2; i++)
-		I915_WRITE(GEN9_LNCFCMOCS(i),
-			   l3cc_combine(&table, unused_value, unused_value));
+		intel_uncore_write(uncore,
+				   GEN9_LNCFCMOCS(i),
+				   l3cc_combine(&table, unused_value,
+						unused_value));
 }
 
 /**
@@ -550,7 +562,7 @@ int intel_rcs_context_init_mocs(struct i915_request *rq)
 	struct drm_i915_mocs_table t;
 	int ret;
 
-	if (get_mocs_settings(rq->i915, &t)) {
+	if (get_mocs_settings(rq->engine->gt, &t)) {
 		/* Program the RCS control registers */
 		ret = emit_mocs_control_table(rq, &t);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.h b/drivers/gpu/drm/i915/gt/intel_mocs.h
index 0913704a1af2..8b9813e6f9ac 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.h
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.h
@@ -52,9 +52,10 @@
 struct drm_i915_private;
 struct i915_request;
 struct intel_engine_cs;
+struct intel_gt;
 
 int intel_rcs_context_init_mocs(struct i915_request *rq);
-void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv);
+void intel_mocs_init_l3cc_table(struct intel_gt *gt);
 void intel_mocs_init_engine(struct intel_engine_cs *engine);
 
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9dd014770e06..fea6fc3659ba 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1285,7 +1285,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 		goto out;
 	}
 
-	intel_mocs_init_l3cc_table(dev_priv);
+	intel_mocs_init_l3cc_table(&dev_priv->gt);
 
 	/* Only when the HW is re-initialised, can we replay the requests */
 	ret = intel_engines_resume(dev_priv);
-- 
2.20.1

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

[RFC 10/14] drm/i915: Convert i915_ppgtt_init_hw to intel_gt

[Tvrtko Ursulin]

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

More removal of implicit dev_priv from using old mmio accessors.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c     |   2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 102 ++++++++++++++++++----------
 drivers/gpu/drm/i915/i915_gem_gtt.h |   3 +-
 3 files changed, 68 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fea6fc3659ba..72e584136909 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1266,7 +1266,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	if (ret)
 		goto out;
 
-	ret = i915_ppgtt_init_hw(dev_priv);
+	ret = i915_ppgtt_init_hw(&dev_priv->gt);
 	if (ret) {
 		DRM_ERROR("Enabling PPGTT failed (%d)\n", ret);
 		goto out;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 50a74674a2a5..7d0b7a47f761 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -41,6 +41,7 @@
 #include "i915_vgpu.h"
 #include "intel_drv.h"
 #include "intel_frontbuffer.h"
+#include "gt/intel_gt.h"
 
 #define I915_GFP_ALLOW_FAIL (GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
 
@@ -1693,25 +1694,29 @@ static inline void gen6_write_pde(const struct gen6_hw_ppgtt *ppgtt,
 		  ppgtt->pd_addr + pde);
 }
 
-static void gen7_ppgtt_enable(struct drm_i915_private *dev_priv)
+static void gen7_ppgtt_enable(struct intel_gt *gt)
 {
+	struct drm_i915_private *i915 = gt_to_i915(gt);
+	struct intel_uncore *uncore = gt_to_uncore(gt);
 	struct intel_engine_cs *engine;
 	u32 ecochk, ecobits;
 	enum intel_engine_id id;
 
-	ecobits = I915_READ(GAC_ECO_BITS);
-	I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_PPGTT_CACHE64B);
+	ecobits = intel_uncore_read(uncore, GAC_ECO_BITS);
+	intel_uncore_write(uncore,
+			   GAC_ECO_BITS,
+			   ecobits | ECOBITS_PPGTT_CACHE64B);
 
-	ecochk = I915_READ(GAM_ECOCHK);
-	if (IS_HASWELL(dev_priv)) {
+	ecochk = intel_uncore_read(uncore, GAM_ECOCHK);
+	if (IS_HASWELL(i915)) {
 		ecochk |= ECOCHK_PPGTT_WB_HSW;
 	} else {
 		ecochk |= ECOCHK_PPGTT_LLC_IVB;
 		ecochk &= ~ECOCHK_PPGTT_GFDT_IVB;
 	}
-	I915_WRITE(GAM_ECOCHK, ecochk);
+	intel_uncore_write(uncore, GAM_ECOCHK, ecochk);
 
-	for_each_engine(engine, dev_priv, id) {
+	for_each_engine(engine, i915, id) {
 		/* GFX_MODE is per-ring on gen7+ */
 		ENGINE_WRITE(engine,
 			     RING_MODE_GEN7,
@@ -1719,22 +1724,30 @@ static void gen7_ppgtt_enable(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void gen6_ppgtt_enable(struct drm_i915_private *dev_priv)
+static void gen6_ppgtt_enable(struct intel_gt *gt)
 {
+	struct intel_uncore *uncore = gt_to_uncore(gt);
 	u32 ecochk, gab_ctl, ecobits;
 
-	ecobits = I915_READ(GAC_ECO_BITS);
-	I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_SNB_BIT |
-		   ECOBITS_PPGTT_CACHE64B);
+	ecobits = intel_uncore_read(uncore, GAC_ECO_BITS);
+	intel_uncore_write(uncore,
+			   GAC_ECO_BITS,
+			   ecobits | ECOBITS_SNB_BIT | ECOBITS_PPGTT_CACHE64B);
 
-	gab_ctl = I915_READ(GAB_CTL);
-	I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
+	gab_ctl = intel_uncore_read(uncore, GAB_CTL);
+	intel_uncore_write(uncore,
+			   GAB_CTL,
+			   gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
 
-	ecochk = I915_READ(GAM_ECOCHK);
-	I915_WRITE(GAM_ECOCHK, ecochk | ECOCHK_SNB_BIT | ECOCHK_PPGTT_CACHE64B);
+	ecochk = intel_uncore_read(uncore, GAM_ECOCHK);
+	intel_uncore_write(uncore,
+			   GAM_ECOCHK,
+			   ecochk | ECOCHK_SNB_BIT | ECOCHK_PPGTT_CACHE64B);
 
-	if (HAS_PPGTT(dev_priv)) /* may be disabled for VT-d */
-		I915_WRITE(GFX_MODE, _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
+	if (HAS_PPGTT(uncore_to_i915(uncore))) /* may be disabled for VT-d */
+		intel_uncore_write(uncore,
+				   GFX_MODE,
+				   _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
 }
 
 /* PPGTT support for Sandybdrige/Gen6 and later */
@@ -2186,21 +2199,32 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	return ERR_PTR(err);
 }
 
-static void gtt_write_workarounds(struct drm_i915_private *dev_priv)
+static void gtt_write_workarounds(struct intel_gt *gt)
 {
+	struct drm_i915_private *i915 = gt_to_i915(gt);
+	struct intel_uncore *uncore = gt_to_uncore(gt);
+
 	/* This function is for gtt related workarounds. This function is
 	 * called on driver load and after a GPU reset, so you can place
 	 * workarounds here even if they get overwritten by GPU reset.
 	 */
 	/* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt,kbl,glk,cfl,cnl,icl */
-	if (IS_BROADWELL(dev_priv))
-		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW);
-	else if (IS_CHERRYVIEW(dev_priv))
-		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV);
-	else if (IS_GEN9_LP(dev_priv))
-		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT);
-	else if (INTEL_GEN(dev_priv) >= 9)
-		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
+	if (IS_BROADWELL(i915))
+		intel_uncore_write(uncore,
+				   GEN8_L3_LRA_1_GPGPU,
+				   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW);
+	else if (IS_CHERRYVIEW(i915))
+		intel_uncore_write(uncore,
+				   GEN8_L3_LRA_1_GPGPU,
+				   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV);
+	else if (IS_GEN9_LP(i915))
+		intel_uncore_write(uncore,
+				   GEN8_L3_LRA_1_GPGPU,
+				   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT);
+	else if (INTEL_GEN(i915) >= 9)
+		intel_uncore_write(uncore,
+				   GEN8_L3_LRA_1_GPGPU,
+				   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
 
 	/*
 	 * To support 64K PTEs we need to first enable the use of the
@@ -2213,21 +2237,25 @@ static void gtt_write_workarounds(struct drm_i915_private *dev_priv)
 	 * 32K pages, but we don't currently have any support for it in our
 	 * driver.
 	 */
-	if (HAS_PAGE_SIZES(dev_priv, I915_GTT_PAGE_SIZE_64K) &&
-	    INTEL_GEN(dev_priv) <= 10)
-		I915_WRITE(GEN8_GAMW_ECO_DEV_RW_IA,
-			   I915_READ(GEN8_GAMW_ECO_DEV_RW_IA) |
-			   GAMW_ECO_ENABLE_64K_IPS_FIELD);
+	if (HAS_PAGE_SIZES(i915, I915_GTT_PAGE_SIZE_64K) &&
+	    INTEL_GEN(i915) <= 10)
+		intel_uncore_write(uncore,
+				   GEN8_GAMW_ECO_DEV_RW_IA,
+				   intel_uncore_read(uncore,
+						     GEN8_GAMW_ECO_DEV_RW_IA) |
+				   GAMW_ECO_ENABLE_64K_IPS_FIELD);
 }
 
-int i915_ppgtt_init_hw(struct drm_i915_private *dev_priv)
+int i915_ppgtt_init_hw(struct intel_gt *gt)
 {
-	gtt_write_workarounds(dev_priv);
+	struct drm_i915_private *i915 = gt_to_i915(gt);
+
+	gtt_write_workarounds(gt);
 
-	if (IS_GEN(dev_priv, 6))
-		gen6_ppgtt_enable(dev_priv);
-	else if (IS_GEN(dev_priv, 7))
-		gen7_ppgtt_enable(dev_priv);
+	if (IS_GEN(i915, 6))
+		gen6_ppgtt_enable(gt);
+	else if (IS_GEN(i915, 7))
+		gen7_ppgtt_enable(gt);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 97700a37c12b..82a5359a11fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -64,6 +64,7 @@ struct drm_i915_file_private;
 struct drm_i915_fence_reg;
 struct drm_i915_gem_object;
 struct i915_vma;
+struct intel_gt;
 
 typedef u32 gen6_pte_t;
 typedef u64 gen8_pte_t;
@@ -625,7 +626,7 @@ void i915_ggtt_disable_guc(struct drm_i915_private *i915);
 int i915_gem_init_ggtt(struct drm_i915_private *dev_priv);
 void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv);
 
-int i915_ppgtt_init_hw(struct drm_i915_private *dev_priv);
+int i915_ppgtt_init_hw(struct intel_gt *gt);
 
 struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_i915_private *dev_priv);
 void i915_ppgtt_release(struct kref *kref);
-- 
2.20.1

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

[RFC 11/14] drm/i915: Consolidate some open coded mmio rmw

[Tvrtko Ursulin]

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

Replace some gen6/7 open coded rmw with intel_uncore_rmw.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 41 +++++++++++++----------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7d0b7a47f761..979305343ac3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1699,13 +1699,10 @@ static void gen7_ppgtt_enable(struct intel_gt *gt)
 	struct drm_i915_private *i915 = gt_to_i915(gt);
 	struct intel_uncore *uncore = gt_to_uncore(gt);
 	struct intel_engine_cs *engine;
-	u32 ecochk, ecobits;
 	enum intel_engine_id id;
+	u32 ecochk;
 
-	ecobits = intel_uncore_read(uncore, GAC_ECO_BITS);
-	intel_uncore_write(uncore,
-			   GAC_ECO_BITS,
-			   ecobits | ECOBITS_PPGTT_CACHE64B);
+	intel_uncore_rmw(uncore, GAC_ECO_BITS, 0, ECOBITS_PPGTT_CACHE64B);
 
 	ecochk = intel_uncore_read(uncore, GAM_ECOCHK);
 	if (IS_HASWELL(i915)) {
@@ -1727,22 +1724,21 @@ static void gen7_ppgtt_enable(struct intel_gt *gt)
 static void gen6_ppgtt_enable(struct intel_gt *gt)
 {
 	struct intel_uncore *uncore = gt_to_uncore(gt);
-	u32 ecochk, gab_ctl, ecobits;
 
-	ecobits = intel_uncore_read(uncore, GAC_ECO_BITS);
-	intel_uncore_write(uncore,
-			   GAC_ECO_BITS,
-			   ecobits | ECOBITS_SNB_BIT | ECOBITS_PPGTT_CACHE64B);
+	intel_uncore_rmw(uncore,
+			 GAC_ECO_BITS,
+			 0,
+			 ECOBITS_SNB_BIT | ECOBITS_PPGTT_CACHE64B);
 
-	gab_ctl = intel_uncore_read(uncore, GAB_CTL);
-	intel_uncore_write(uncore,
-			   GAB_CTL,
-			   gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
+	intel_uncore_rmw(uncore,
+			 GAB_CTL,
+			 0,
+			 GAB_CTL_CONT_AFTER_PAGEFAULT);
 
-	ecochk = intel_uncore_read(uncore, GAM_ECOCHK);
-	intel_uncore_write(uncore,
-			   GAM_ECOCHK,
-			   ecochk | ECOCHK_SNB_BIT | ECOCHK_PPGTT_CACHE64B);
+	intel_uncore_rmw(uncore,
+			 GAM_ECOCHK,
+			 0,
+			 ECOCHK_SNB_BIT | ECOCHK_PPGTT_CACHE64B);
 
 	if (HAS_PPGTT(uncore_to_i915(uncore))) /* may be disabled for VT-d */
 		intel_uncore_write(uncore,
@@ -2239,11 +2235,10 @@ static void gtt_write_workarounds(struct intel_gt *gt)
 	 */
 	if (HAS_PAGE_SIZES(i915, I915_GTT_PAGE_SIZE_64K) &&
 	    INTEL_GEN(i915) <= 10)
-		intel_uncore_write(uncore,
-				   GEN8_GAMW_ECO_DEV_RW_IA,
-				   intel_uncore_read(uncore,
-						     GEN8_GAMW_ECO_DEV_RW_IA) |
-				   GAMW_ECO_ENABLE_64K_IPS_FIELD);
+		intel_uncore_rmw(uncore,
+				 GEN8_GAMW_ECO_DEV_RW_IA,
+				 0,
+				 GAMW_ECO_ENABLE_64K_IPS_FIELD);
 }
 
 int i915_ppgtt_init_hw(struct intel_gt *gt)
-- 
2.20.1

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

[RFC 12/14] drm/i915: Convert i915_gem_init_hw to intel_gt

[Tvrtko Ursulin]

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

More removal of implicit dev_priv from using old mmio accessors.

Actually the top level function remains but is split into a part which
writes to i915 and part which operates on uncore to initialize the
hardware.

GuC and engines are the only odd ones out remaining.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 68 ++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 72e584136909..cacc4ff1a160 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1230,28 +1230,30 @@ static void init_unused_rings(struct intel_gt *gt)
 	}
 }
 
-int i915_gem_init_hw(struct drm_i915_private *dev_priv)
+static int init_hw(struct intel_gt *gt)
 {
+	struct drm_i915_private *i915 = gt_to_i915(gt);
+	struct intel_uncore *uncore = gt_to_uncore(gt);
 	int ret;
 
-	dev_priv->gt.last_init_time = ktime_get();
-
 	/* Double layer security blanket, see i915_gem_init() */
-	intel_uncore_forcewake_get(&dev_priv->uncore, FORCEWAKE_ALL);
+	intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
 
-	if (HAS_EDRAM(dev_priv) && INTEL_GEN(dev_priv) < 9)
-		I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
+	if (HAS_EDRAM(i915) && INTEL_GEN(i915) < 9)
+		intel_uncore_rmw(uncore, HSW_IDICR, 0, IDIHASHMSK(0xf));
 
-	if (IS_HASWELL(dev_priv))
-		I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) ?
-			   LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
+	if (IS_HASWELL(i915))
+		intel_uncore_write(uncore,
+				   MI_PREDICATE_RESULT_2,
+				   IS_HSW_GT3(i915) ?
+				   LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
 
 	/* Apply the GT workarounds... */
-	intel_gt_apply_workarounds(&dev_priv->gt);
+	intel_gt_apply_workarounds(gt);
 	/* ...and determine whether they are sticking. */
-	intel_gt_verify_workarounds(&dev_priv->gt, "init");
+	intel_gt_verify_workarounds(gt, "init");
 
-	intel_gt_init_swizzling(&dev_priv->gt);
+	intel_gt_init_swizzling(gt);
 
 	/*
 	 * At least 830 can leave some of the unused rings
@@ -1259,48 +1261,60 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	 * will prevent c3 entry. Makes sure all unused rings
 	 * are totally idle.
 	 */
-	init_unused_rings(&dev_priv->gt);
+	init_unused_rings(gt);
 
-	BUG_ON(!dev_priv->kernel_context);
-	ret = i915_terminally_wedged(dev_priv);
-	if (ret)
-		goto out;
-
-	ret = i915_ppgtt_init_hw(&dev_priv->gt);
+	ret = i915_ppgtt_init_hw(gt);
 	if (ret) {
 		DRM_ERROR("Enabling PPGTT failed (%d)\n", ret);
 		goto out;
 	}
 
-	ret = intel_wopcm_init_hw(&dev_priv->wopcm);
+	ret = intel_wopcm_init_hw(&i915->wopcm);
 	if (ret) {
 		DRM_ERROR("Enabling WOPCM failed (%d)\n", ret);
 		goto out;
 	}
 
 	/* We can't enable contexts until all firmware is loaded */
-	ret = intel_uc_init_hw(dev_priv);
+	ret = intel_uc_init_hw(i915);
 	if (ret) {
 		DRM_ERROR("Enabling uc failed (%d)\n", ret);
 		goto out;
 	}
 
-	intel_mocs_init_l3cc_table(&dev_priv->gt);
+	intel_mocs_init_l3cc_table(gt);
 
 	/* Only when the HW is re-initialised, can we replay the requests */
-	ret = intel_engines_resume(dev_priv);
+	ret = intel_engines_resume(i915);
 	if (ret)
 		goto cleanup_uc;
 
-	intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL);
+	intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
 
-	intel_engines_set_scheduler_caps(dev_priv);
 	return 0;
 
 cleanup_uc:
-	intel_uc_fini_hw(dev_priv);
+	intel_uc_fini_hw(i915);
 out:
-	intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL);
+	intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
+
+	return ret;
+}
+
+int i915_gem_init_hw(struct drm_i915_private *i915)
+{
+	int ret;
+
+	i915->gt.last_init_time = ktime_get();
+
+	BUG_ON(!i915->kernel_context);
+	ret = i915_terminally_wedged(i915);
+	if (ret)
+		return ret;
+
+	ret = init_hw(&i915->gt);
+
+	intel_engines_set_scheduler_caps(i915);
 
 	return ret;
 }
-- 
2.20.1

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

[RFC 13/14] drm/i915/guc: Move intel_guc_reserved_gtt_size to intel_wopcm_guc_size

[Tvrtko Ursulin]

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

Reduces pointer chasing and gets more to the point.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c |  2 +-
 drivers/gpu/drm/i915/intel_guc.c    | 17 -----------------
 drivers/gpu/drm/i915/intel_guc.h    |  1 -
 drivers/gpu/drm/i915/intel_wopcm.h  | 17 +++++++++++++++++
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 979305343ac3..e62041eb10b8 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2853,7 +2853,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	 * why.
 	 */
 	ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE,
-			       intel_guc_reserved_gtt_size(&dev_priv->guc));
+			       intel_wopcm_guc_size(&dev_priv->wopcm));
 
 	ret = intel_vgt_balloon(ggtt);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 43232242d167..d45d97624402 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -686,23 +686,6 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 	return vma;
 }
 
-/**
- * intel_guc_reserved_gtt_size()
- * @guc:	intel_guc structure
- *
- * The GuC WOPCM mapping shadows the lower part of the GGTT, so if we are using
- * GuC we can't have any objects pinned in that region. This function returns
- * the size of the shadowed region.
- *
- * Returns:
- * 0 if GuC is not present or not in use.
- * Otherwise, the GuC WOPCM size.
- */
-u32 intel_guc_reserved_gtt_size(struct intel_guc *guc)
-{
-	return guc_to_i915(guc)->wopcm.guc.size;
-}
-
 int intel_guc_reserve_ggtt_top(struct intel_guc *guc)
 {
 	struct drm_i915_private *i915 = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index e07e4c69cf08..85c3b02a0c08 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -172,7 +172,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 int intel_guc_suspend(struct intel_guc *guc);
 int intel_guc_resume(struct intel_guc *guc);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-u32 intel_guc_reserved_gtt_size(struct intel_guc *guc);
 int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
 void intel_guc_release_ggtt_top(struct intel_guc *guc);
 
diff --git a/drivers/gpu/drm/i915/intel_wopcm.h b/drivers/gpu/drm/i915/intel_wopcm.h
index 6298910a384c..1c32d449fc10 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.h
+++ b/drivers/gpu/drm/i915/intel_wopcm.h
@@ -24,6 +24,23 @@ struct intel_wopcm {
 	} guc;
 };
 
+/**
+ * intel_wopcm_guc_size()
+ * @wopcm:	intel_wopcm structure
+ *
+ * The GuC WOPCM mapping shadows the lower part of the GGTT, so if we are using
+ * GuC we can't have any objects pinned in that region. This function returns
+ * the size of the shadowed region.
+ *
+ * Returns:
+ * 0 if GuC is not present or not in use.
+ * Otherwise, the GuC WOPCM size.
+ */
+static inline u32 intel_wopcm_guc_size(struct intel_wopcm *wopcm)
+{
+    return wopcm->guc.size;
+}
+
 void intel_wopcm_init_early(struct intel_wopcm *wopcm);
 int intel_wopcm_init(struct intel_wopcm *wopcm);
 int intel_wopcm_init_hw(struct intel_wopcm *wopcm);
-- 
2.20.1

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

[RFC 14/14] drm/i915: Make GuC GGTT reservation work on ggtt

[Tvrtko Ursulin]

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

These functions operate on ggtt so make them take that directly as
parameter.

At the same time move the USES_GUC conditional down to
intel_guc_reserve_ggtt_top for symmetry with
intel_guc_reserved_gtt_size.

v2:
 * Rename and move functions to be static in i915_gem_gtt.c (Michal)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 37 +++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_guc.c    | 27 ---------------------
 drivers/gpu/drm/i915/intel_guc.h    |  2 --
 3 files changed, 30 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e62041eb10b8..394f347a90ee 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2830,6 +2830,31 @@ static void fini_aliasing_ppgtt(struct drm_i915_private *i915)
 	ggtt->vm.vma_ops.unbind_vma = ggtt_unbind_vma;
 }
 
+static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
+{
+	u64 size;
+	int ret;
+
+	if (!USES_GUC(ggtt->vm.i915))
+		return 0;
+
+	size = ggtt->vm.total - GUC_GGTT_TOP;
+
+	ret = i915_gem_gtt_reserve(&ggtt->vm, &ggtt->uc_fw, size,
+				   GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
+				   PIN_NOEVICT);
+	if (ret)
+		DRM_DEBUG_DRIVER("GuC: failed to reserve top of ggtt\n");
+
+	return ret;
+}
+
+static void ggtt_release_guc_top(struct i915_ggtt *ggtt)
+{
+	if (drm_mm_node_allocated(&ggtt->uc_fw))
+		drm_mm_remove_node(&ggtt->uc_fw);
+}
+
 int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 {
 	/* Let GEM Manage all of the aperture.
@@ -2867,11 +2892,9 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
-	if (USES_GUC(dev_priv)) {
-		ret = intel_guc_reserve_ggtt_top(&dev_priv->guc);
-		if (ret)
-			goto err_reserve;
-	}
+	ret = ggtt_reserve_guc_top(ggtt);
+	if (ret)
+		goto err_reserve;
 
 	/* Clear any non-preallocated blocks */
 	drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
@@ -2893,7 +2916,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	return 0;
 
 err_appgtt:
-	intel_guc_release_ggtt_top(&dev_priv->guc);
+	ggtt_release_guc_top(ggtt);
 err_reserve:
 	drm_mm_remove_node(&ggtt->error_capture);
 	return ret;
@@ -2920,7 +2943,7 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
 	if (drm_mm_node_allocated(&ggtt->error_capture))
 		drm_mm_remove_node(&ggtt->error_capture);
 
-	intel_guc_release_ggtt_top(&dev_priv->guc);
+	ggtt_release_guc_top(ggtt);
 
 	if (drm_mm_initialized(&ggtt->vm.mm)) {
 		intel_vgt_deballoon(ggtt);
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index d45d97624402..c40a6efdd33a 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -685,30 +685,3 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 	i915_gem_object_put(obj);
 	return vma;
 }
-
-int intel_guc_reserve_ggtt_top(struct intel_guc *guc)
-{
-	struct drm_i915_private *i915 = guc_to_i915(guc);
-	struct i915_ggtt *ggtt = &i915->ggtt;
-	u64 size;
-	int ret;
-
-	size = ggtt->vm.total - GUC_GGTT_TOP;
-
-	ret = i915_gem_gtt_reserve(&ggtt->vm, &ggtt->uc_fw, size,
-				   GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
-				   PIN_NOEVICT);
-	if (ret)
-		DRM_DEBUG_DRIVER("GuC: failed to reserve top of ggtt\n");
-
-	return ret;
-}
-
-void intel_guc_release_ggtt_top(struct intel_guc *guc)
-{
-	struct drm_i915_private *i915 = guc_to_i915(guc);
-	struct i915_ggtt *ggtt = &i915->ggtt;
-
-	if (drm_mm_node_allocated(&ggtt->uc_fw))
-		drm_mm_remove_node(&ggtt->uc_fw);
-}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 85c3b02a0c08..08c906abdfa2 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -172,8 +172,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 int intel_guc_suspend(struct intel_guc *guc);
 int intel_guc_resume(struct intel_guc *guc);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
-void intel_guc_release_ggtt_top(struct intel_guc *guc);
 
 static inline bool intel_guc_is_loaded(struct intel_guc *guc)
 {
-- 
2.20.1

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

Re: [RFC 08/14] drm/i915: Store backpointer to intel_gt in the engine

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-06-10 16:54:13)
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 01223864237a..343c4459e8a3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -34,6 +34,7 @@ struct drm_i915_reg_table;
>  struct i915_gem_context;
>  struct i915_request;
>  struct i915_sched_attr;
> +struct intel_gt;
>  struct intel_uncore;
>  
>  typedef u8 intel_engine_mask_t;
> @@ -266,6 +267,7 @@ struct intel_engine_execlists {
>  
>  struct intel_engine_cs {
>         struct drm_i915_private *i915;
> +       struct intel_gt *gt;

I'd push for gt as being the backpointer, and i915 its distant grand
parent. Not sure how much pain that would bring just for the elimination
of one more drm_i915_private, but that's how I picture the
encapsulation.

I'm sure I'm missing a link or two :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 04/14] drm/i915: Add a couple intel_gt helpers

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-06-10 16:54:09)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Two trivial helpers to convert from intel_gt to i915 and uncore which will
> be needed by the following patches.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/gt/intel_gt.h
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
> new file mode 100644
> index 000000000000..b672f8b03bfd
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
> @@ -0,0 +1,26 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2019 Intel Corporation
> + */
> +
> +#ifndef __INTEL_GT__
> +#define __INTEL_GT__
> +
> +#include "gt/intel_gt_types.h"
> +
> +#include "intel_uncore.h"
> +
> +#include "i915_drv.h"
> +
> +static inline struct drm_i915_private *gt_to_i915(struct intel_gt *gt)
> +{
> +       return container_of(gt, struct drm_i915_private, gt);

Ok, I can buy that a single i915 device will only have a single GT slot.

> +}
> +
> +static inline struct intel_uncore *gt_to_uncore(struct intel_gt *gt)
> +{
> +       return &gt_to_i915(gt)->uncore;

But I suspect it will be cleaner just to have a gt->uncore pointer. With
any luck, Daniele's splitting of display and GT uncore bears fruit and
it will be distinct without its own set of locks, powerwells and
cacheline rules.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] drm/i915: Make i915_check_and_clear_faults take uncore

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-06-10 16:54:06)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Continuing the conversion and elimination of implicit dev_priv.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c |  2 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c     | 28 ++++++++++++-----------
>  drivers/gpu/drm/i915/gt/intel_reset.h     |  2 +-
>  drivers/gpu/drm/i915/i915_drv.c           |  2 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c       |  4 ++--
>  5 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index c0d986db5a75..a046e8dccc96 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -453,7 +453,7 @@ int intel_engines_init_mmio(struct drm_i915_private *i915)
>  
>         RUNTIME_INFO(i915)->num_engines = hweight32(mask);
>  
> -       i915_check_and_clear_faults(i915);
> +       i915_check_and_clear_faults(&i915->uncore);

This name is still setting off red flags for me, but I have to confess
that staring at it, passing uncore does make sense.

I just wish we have per-engines faults everywhere and this could be
reduced to passing engine.

Hmm, this I guess we will just have to revisit in the near future as we
may get the opportunity to put these regs under more scrutiny.

>  
>         intel_setup_engine_capabilities(i915);
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 60d24110af80..13471916559b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -1166,10 +1166,10 @@ static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
>         GEN6_RING_FAULT_REG_POSTING_READ(engine);
>  }
>  
> -static void clear_error_registers(struct drm_i915_private *i915,
> +static void clear_error_registers(struct intel_uncore *uncore,
>                                   intel_engine_mask_t engine_mask)
>  {
> -       struct intel_uncore *uncore = &i915->uncore;
> +       struct drm_i915_private *i915 = uncore_to_i915(uncore);

Grr, I should have objected to uncore_to_i915() loudly from the
beginning

What's done is done,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for Implicit dev_priv removal (rev2)

[Patchwork]

== Series Details ==

Series: Implicit dev_priv removal (rev2)
URL   : https://patchwork.freedesktop.org/series/61705/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
885d95410d8d drm/i915: Make i915_check_and_clear_faults take uncore
4594ba392e2c drm/i915: Convert intel_vgt_(de)balloon to uncore
b81a04a33e41 drm/i915: Introduce struct intel_gt as replacement for anonymous i915->gt
-:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#11: 
This is a first step towards cleaning up the separation between i915 and gt.

-:16: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#16: 
new file mode 100644

-:21: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#21: FILE: drivers/gpu/drm/i915/gt/intel_gt_types.h:1:
+/*

-:22: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#22: FILE: drivers/gpu/drm/i915/gt/intel_gt_types.h:2:
+ * SPDX-License-Identifier: MIT

total: 0 errors, 4 warnings, 0 checks, 99 lines checked
eb4225fc81d8 drm/i915: Add a couple intel_gt helpers
-:12: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#12: 
new file mode 100644

-:17: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#17: FILE: drivers/gpu/drm/i915/gt/intel_gt.h:1:
+/*

-:18: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#18: FILE: drivers/gpu/drm/i915/gt/intel_gt.h:2:
+ * SPDX-License-Identifier: MIT

-:42: WARNING:MISSING_EOF_NEWLINE: adding a line without newline at end of file
#42: FILE: drivers/gpu/drm/i915/gt/intel_gt.h:26:
+#endif /* __INTEL_GT_H__ */

total: 0 errors, 4 warnings, 0 checks, 26 lines checked
cfef12d54404 drm/i915: Convert i915_gem_init_swizzling to intel_gt
-:25: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#25: 
new file mode 100644

-:30: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#30: FILE: drivers/gpu/drm/i915/gt/intel_gt.c:1:
+/*

-:31: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#31: FILE: drivers/gpu/drm/i915/gt/intel_gt.c:2:
+ * SPDX-License-Identifier: MIT

total: 0 errors, 3 warnings, 0 checks, 131 lines checked
9de87c85d59e drm/i915: Convert init_unused_rings to intel_gt
a3efc681b7d4 drm/i915: Convert gt workarounds to intel_gt
9815d56874c7 drm/i915: Store backpointer to intel_gt in the engine
0ea90265d50d drm/i915: Convert intel_mocs_init_l3cc_table to intel_gt
1e3e07823505 drm/i915: Convert i915_ppgtt_init_hw to intel_gt
8789e9c71eaa drm/i915: Consolidate some open coded mmio rmw
36083aedd680 drm/i915: Convert i915_gem_init_hw to intel_gt
-:128: WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()
#128: FILE: drivers/gpu/drm/i915/i915_gem.c:1309:
+	BUG_ON(!i915->kernel_context);

total: 0 errors, 1 warnings, 0 checks, 117 lines checked
7f7e63f7489c drm/i915/guc: Move intel_guc_reserved_gtt_size to intel_wopcm_guc_size
-:88: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#88: FILE: drivers/gpu/drm/i915/intel_wopcm.h:41:
+    return wopcm->guc.size;$

total: 0 errors, 1 warnings, 0 checks, 61 lines checked
f862592a446f drm/i915: Make GuC GGTT reservation work on ggtt

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

✗ Fi.CI.SPARSE: warning for Implicit dev_priv removal (rev2)

[Patchwork]

== Series Details ==

Series: Implicit dev_priv removal (rev2)
URL   : https://patchwork.freedesktop.org/series/61705/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Make i915_check_and_clear_faults take uncore
Okay!

Commit: drm/i915: Convert intel_vgt_(de)balloon to uncore
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2832:26: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:2832:26: warning: expression using sizeof(void)

Commit: drm/i915: Introduce struct intel_gt as replacement for anonymous i915->gt
+./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)

Commit: drm/i915: Add a couple intel_gt helpers
-
+drivers/gpu/drm/i915/gt/intel_gt.h:26:27: warning: no newline at end of file
+./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)

Commit: drm/i915: Convert i915_gem_init_swizzling to intel_gt
-O:drivers/gpu/drm/i915/gt/intel_gt.h:26:27: warning: no newline at end of file
+drivers/gpu/drm/i915/gt/intel_gt.h:28:27: warning: no newline at end of file
+drivers/gpu/drm/i915/gt/intel_gt.h:28:27: warning: no newline at end of file
+drivers/gpu/drm/i915/gt/intel_gt.h:28:27: warning: no newline at end of file
+drivers/gpu/drm/i915/gt/intel_gt.h:28:27: warning: no newline at end of file
+./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)

Commit: drm/i915: Convert init_unused_rings to intel_gt
Okay!

Commit: drm/i915: Convert gt workarounds to intel_gt
+drivers/gpu/drm/i915/gt/intel_gt.h:28:27: warning: no newline at end of file

Commit: drm/i915: Store backpointer to intel_gt in the engine
Okay!

Commit: drm/i915: Convert intel_mocs_init_l3cc_table to intel_gt
+drivers/gpu/drm/i915/gt/intel_gt.h:28:27: warning: no newline at end of file

Commit: drm/i915: Convert i915_ppgtt_init_hw to intel_gt
+drivers/gpu/drm/i915/gt/intel_gt.h:28:27: warning: no newline at end of file

Commit: drm/i915: Consolidate some open coded mmio rmw
Okay!

Commit: drm/i915: Convert i915_gem_init_hw to intel_gt
Okay!

Commit: drm/i915/guc: Move intel_guc_reserved_gtt_size to intel_wopcm_guc_size
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2855:26: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:2855:26: warning: expression using sizeof(void)

Commit: drm/i915: Make GuC GGTT reservation work on ggtt
Okay!

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

✓ Fi.CI.BAT: success for Implicit dev_priv removal (rev2)

[Patchwork]

== Series Details ==

Series: Implicit dev_priv removal (rev2)
URL   : https://patchwork.freedesktop.org/series/61705/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6227 -> Patchwork_13227
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_basic@bad-close:
    - fi-icl-u2:          [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/fi-icl-u2/igt@gem_basic@bad-close.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/fi-icl-u2/igt@gem_basic@bad-close.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][5] -> [FAIL][6] ([fdo#108511])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  
#### Possible fixes ####

  * igt@gem_basic@bad-close:
    - fi-icl-u3:          [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/fi-icl-u3/igt@gem_basic@bad-close.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/fi-icl-u3/igt@gem_basic@bad-close.html
    - fi-icl-dsi:         [INCOMPLETE][9] ([fdo#107713]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/fi-icl-dsi/igt@gem_basic@bad-close.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/fi-icl-dsi/igt@gem_basic@bad-close.html

  * igt@gem_ctx_switch@basic-default:
    - {fi-icl-guc}:       [INCOMPLETE][11] ([fdo#107713] / [fdo#108569]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/fi-icl-guc/igt@gem_ctx_switch@basic-default.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/fi-icl-guc/igt@gem_ctx_switch@basic-default.html

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

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569


Participating hosts (52 -> 46)
------------------------------

  Additional (1): fi-snb-2600 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6227 -> Patchwork_13227

  CI_DRM_6227: fe62c0390420632afe2193a40097c9f03a0bf725 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5050: 4c072238c784e6acb00634a80c3c55fb8358058b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13227: f862592a446ffe102ba9e650a22fe22e4ec9ff95 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f862592a446f drm/i915: Make GuC GGTT reservation work on ggtt
7f7e63f7489c drm/i915/guc: Move intel_guc_reserved_gtt_size to intel_wopcm_guc_size
36083aedd680 drm/i915: Convert i915_gem_init_hw to intel_gt
8789e9c71eaa drm/i915: Consolidate some open coded mmio rmw
1e3e07823505 drm/i915: Convert i915_ppgtt_init_hw to intel_gt
0ea90265d50d drm/i915: Convert intel_mocs_init_l3cc_table to intel_gt
9815d56874c7 drm/i915: Store backpointer to intel_gt in the engine
a3efc681b7d4 drm/i915: Convert gt workarounds to intel_gt
9de87c85d59e drm/i915: Convert init_unused_rings to intel_gt
cfef12d54404 drm/i915: Convert i915_gem_init_swizzling to intel_gt
eb4225fc81d8 drm/i915: Add a couple intel_gt helpers
b81a04a33e41 drm/i915: Introduce struct intel_gt as replacement for anonymous i915->gt
4594ba392e2c drm/i915: Convert intel_vgt_(de)balloon to uncore
885d95410d8d drm/i915: Make i915_check_and_clear_faults take uncore

== Logs ==

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

Re: [RFC 08/14] drm/i915: Store backpointer to intel_gt in the engine

[Daniele Ceraolo Spurio]

On 6/10/19 9:16 AM, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-06-10 16:54:13)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> index 01223864237a..343c4459e8a3 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> @@ -34,6 +34,7 @@ struct drm_i915_reg_table;
>>   struct i915_gem_context;
>>   struct i915_request;
>>   struct i915_sched_attr;
>> +struct intel_gt;
>>   struct intel_uncore;
>>   
>>   typedef u8 intel_engine_mask_t;
>> @@ -266,6 +267,7 @@ struct intel_engine_execlists {
>>   
>>   struct intel_engine_cs {
>>          struct drm_i915_private *i915;
>> +       struct intel_gt *gt;
> 
> I'd push for gt as being the backpointer, and i915 its distant grand
> parent. Not sure how much pain that would bring just for the elimination
> of one more drm_i915_private, but that's how I picture the
> encapsulation.
> 

Would it be worth moving some of the flags in the device_info structure 
in a gt substructure, like we did for display, and get a pointer to that 
in intel_gt? We could save some jumps back that way and be more coherent 
in where we store the info.

Daniele

> I'm sure I'm missing a link or two :)
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 13/14] drm/i915/guc: Move intel_guc_reserved_gtt_size to intel_wopcm_guc_size

[Michal Wajdeczko]

On Mon, 10 Jun 2019 17:54:18 +0200, Tvrtko Ursulin  
<tvrtko.ursulin@linux.intel.com> wrote:

> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Reduces pointer chasing and gets more to the point.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

with small nit below

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c |  2 +-
>  drivers/gpu/drm/i915/intel_guc.c    | 17 -----------------
>  drivers/gpu/drm/i915/intel_guc.h    |  1 -
>  drivers/gpu/drm/i915/intel_wopcm.h  | 17 +++++++++++++++++
>  4 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c  
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 979305343ac3..e62041eb10b8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2853,7 +2853,7 @@ int i915_gem_init_ggtt(struct drm_i915_private  
> *dev_priv)
>  	 * why.
>  	 */
>  	ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE,
> -			       intel_guc_reserved_gtt_size(&dev_priv->guc));
> +			       intel_wopcm_guc_size(&dev_priv->wopcm));
> 	ret = intel_vgt_balloon(ggtt);
>  	if (ret)
> diff --git a/drivers/gpu/drm/i915/intel_guc.c  
> b/drivers/gpu/drm/i915/intel_guc.c
> index 43232242d167..d45d97624402 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -686,23 +686,6 @@ struct i915_vma *intel_guc_allocate_vma(struct  
> intel_guc *guc, u32 size)
>  	return vma;
>  }
> -/**
> - * intel_guc_reserved_gtt_size()
> - * @guc:	intel_guc structure
> - *
> - * The GuC WOPCM mapping shadows the lower part of the GGTT, so if we  
> are using
> - * GuC we can't have any objects pinned in that region. This function  
> returns
> - * the size of the shadowed region.
> - *
> - * Returns:
> - * 0 if GuC is not present or not in use.
> - * Otherwise, the GuC WOPCM size.
> - */
> -u32 intel_guc_reserved_gtt_size(struct intel_guc *guc)
> -{
> -	return guc_to_i915(guc)->wopcm.guc.size;
> -}
> -
>  int intel_guc_reserve_ggtt_top(struct intel_guc *guc)
>  {
>  	struct drm_i915_private *i915 = guc_to_i915(guc);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h  
> b/drivers/gpu/drm/i915/intel_guc.h
> index e07e4c69cf08..85c3b02a0c08 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -172,7 +172,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32  
> rsa_offset);
>  int intel_guc_suspend(struct intel_guc *guc);
>  int intel_guc_resume(struct intel_guc *guc);
>  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32  
> size);
> -u32 intel_guc_reserved_gtt_size(struct intel_guc *guc);
>  int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
>  void intel_guc_release_ggtt_top(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.h  
> b/drivers/gpu/drm/i915/intel_wopcm.h
> index 6298910a384c..1c32d449fc10 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.h
> +++ b/drivers/gpu/drm/i915/intel_wopcm.h
> @@ -24,6 +24,23 @@ struct intel_wopcm {
>  	} guc;
>  };
> +/**
> + * intel_wopcm_guc_size()
> + * @wopcm:	intel_wopcm structure
> + *
> + * The GuC WOPCM mapping shadows the lower part of the GGTT, so if we  
> are using
> + * GuC we can't have any objects pinned in that region.

maybe we can drop above sentence is we already have something similar
as comment in i915_gem_init_ggtt ? pin/GGTT doesn't fit here.

> This function returns
> + * the size of the shadowed region.
> + *
> + * Returns:
> + * 0 if GuC is not present or not in use.
> + * Otherwise, the GuC WOPCM size.
> + */
> +static inline u32 intel_wopcm_guc_size(struct intel_wopcm *wopcm)
> +{
> +    return wopcm->guc.size;
> +}
> +
>  void intel_wopcm_init_early(struct intel_wopcm *wopcm);
>  int intel_wopcm_init(struct intel_wopcm *wopcm);
>  int intel_wopcm_init_hw(struct intel_wopcm *wopcm);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 14/14] drm/i915: Make GuC GGTT reservation work on ggtt

[Michal Wajdeczko]

On Mon, 10 Jun 2019 17:54:19 +0200, Tvrtko Ursulin  
<tvrtko.ursulin@linux.intel.com> wrote:

> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> These functions operate on ggtt so make them take that directly as
> parameter.
>
> At the same time move the USES_GUC conditional down to
> intel_guc_reserve_ggtt_top for symmetry with
> intel_guc_reserved_gtt_size.
>
> v2:
>  * Rename and move functions to be static in i915_gem_gtt.c (Michal)
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>

with some nits below,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 37 +++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_guc.c    | 27 ---------------------
>  drivers/gpu/drm/i915/intel_guc.h    |  2 --
>  3 files changed, 30 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c  
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index e62041eb10b8..394f347a90ee 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2830,6 +2830,31 @@ static void fini_aliasing_ppgtt(struct  
> drm_i915_private *i915)
>  	ggtt->vm.vma_ops.unbind_vma = ggtt_unbind_vma;
>  }
> +static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)

maybe we should copy some comment from commit 911800765ef6
directly to the code here, to explain why we do that ?

> +{
> +	u64 size;
> +	int ret;
> +
> +	if (!USES_GUC(ggtt->vm.i915))
> +		return 0;
> +
> +	size = ggtt->vm.total - GUC_GGTT_TOP;

what about making sure that ggtt size is large enough:

	GEM_BUG_ON(ggtt->vm.total > GUC_GGTT_TOP)

> +
> +	ret = i915_gem_gtt_reserve(&ggtt->vm, &ggtt->uc_fw, size,
> +				   GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
> +				   PIN_NOEVICT);
> +	if (ret)
> +		DRM_DEBUG_DRIVER("GuC: failed to reserve top of ggtt\n");

this is now ggtt code, so:

	DRM_DEBUG_DRIVER("Failed to reserve top of GGTT for GuC\n");

> +
> +	return ret;
> +}
> +
> +static void ggtt_release_guc_top(struct i915_ggtt *ggtt)
> +{
> +	if (drm_mm_node_allocated(&ggtt->uc_fw))
> +		drm_mm_remove_node(&ggtt->uc_fw);
> +}
> +
>  int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
>  {
>  	/* Let GEM Manage all of the aperture.
> @@ -2867,11 +2892,9 @@ int i915_gem_init_ggtt(struct drm_i915_private  
> *dev_priv)
>  	if (ret)
>  		return ret;
> -	if (USES_GUC(dev_priv)) {
> -		ret = intel_guc_reserve_ggtt_top(&dev_priv->guc);
> -		if (ret)
> -			goto err_reserve;
> -	}
> +	ret = ggtt_reserve_guc_top(ggtt);
> +	if (ret)
> +		goto err_reserve;
> 	/* Clear any non-preallocated blocks */
>  	drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
> @@ -2893,7 +2916,7 @@ int i915_gem_init_ggtt(struct drm_i915_private  
> *dev_priv)
>  	return 0;
> err_appgtt:
> -	intel_guc_release_ggtt_top(&dev_priv->guc);
> +	ggtt_release_guc_top(ggtt);
>  err_reserve:
>  	drm_mm_remove_node(&ggtt->error_capture);
>  	return ret;
> @@ -2920,7 +2943,7 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private  
> *dev_priv)
>  	if (drm_mm_node_allocated(&ggtt->error_capture))
>  		drm_mm_remove_node(&ggtt->error_capture);
> -	intel_guc_release_ggtt_top(&dev_priv->guc);
> +	ggtt_release_guc_top(ggtt);
> 	if (drm_mm_initialized(&ggtt->vm.mm)) {
>  		intel_vgt_deballoon(ggtt);
> diff --git a/drivers/gpu/drm/i915/intel_guc.c  
> b/drivers/gpu/drm/i915/intel_guc.c
> index d45d97624402..c40a6efdd33a 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -685,30 +685,3 @@ struct i915_vma *intel_guc_allocate_vma(struct  
> intel_guc *guc, u32 size)
>  	i915_gem_object_put(obj);
>  	return vma;
>  }
> -
> -int intel_guc_reserve_ggtt_top(struct intel_guc *guc)
> -{
> -	struct drm_i915_private *i915 = guc_to_i915(guc);
> -	struct i915_ggtt *ggtt = &i915->ggtt;
> -	u64 size;
> -	int ret;
> -
> -	size = ggtt->vm.total - GUC_GGTT_TOP;
> -
> -	ret = i915_gem_gtt_reserve(&ggtt->vm, &ggtt->uc_fw, size,
> -				   GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
> -				   PIN_NOEVICT);
> -	if (ret)
> -		DRM_DEBUG_DRIVER("GuC: failed to reserve top of ggtt\n");
> -
> -	return ret;
> -}
> -
> -void intel_guc_release_ggtt_top(struct intel_guc *guc)
> -{
> -	struct drm_i915_private *i915 = guc_to_i915(guc);
> -	struct i915_ggtt *ggtt = &i915->ggtt;
> -
> -	if (drm_mm_node_allocated(&ggtt->uc_fw))
> -		drm_mm_remove_node(&ggtt->uc_fw);
> -}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h  
> b/drivers/gpu/drm/i915/intel_guc.h
> index 85c3b02a0c08..08c906abdfa2 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -172,8 +172,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32  
> rsa_offset);
>  int intel_guc_suspend(struct intel_guc *guc);
>  int intel_guc_resume(struct intel_guc *guc);
>  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32  
> size);
> -int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
> -void intel_guc_release_ggtt_top(struct intel_guc *guc);
> static inline bool intel_guc_is_loaded(struct intel_guc *guc)
>  {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] drm/i915: Make i915_check_and_clear_faults take uncore

[Tvrtko Ursulin]

On 10/06/2019 17:26, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-06-10 16:54:06)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Continuing the conversion and elimination of implicit dev_priv.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_engine_cs.c |  2 +-
>>   drivers/gpu/drm/i915/gt/intel_reset.c     | 28 ++++++++++++-----------
>>   drivers/gpu/drm/i915/gt/intel_reset.h     |  2 +-
>>   drivers/gpu/drm/i915/i915_drv.c           |  2 +-
>>   drivers/gpu/drm/i915/i915_gem_gtt.c       |  4 ++--
>>   5 files changed, 20 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> index c0d986db5a75..a046e8dccc96 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> @@ -453,7 +453,7 @@ int intel_engines_init_mmio(struct drm_i915_private *i915)
>>   
>>          RUNTIME_INFO(i915)->num_engines = hweight32(mask);
>>   
>> -       i915_check_and_clear_faults(i915);
>> +       i915_check_and_clear_faults(&i915->uncore);
> 
> This name is still setting off red flags for me, but I have to confess
> that staring at it, passing uncore does make sense.

Rename to intel_uncore_check_and_clear_faults?

Or move later in the series as intel_gt_check_and_clear_faults?

> 
> I just wish we have per-engines faults everywhere and this could be
> reduced to passing engine.
> 
> Hmm, this I guess we will just have to revisit in the near future as we
> may get the opportunity to put these regs under more scrutiny.
> 
>>   
>>          intel_setup_engine_capabilities(i915);
>>   
>> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
>> index 60d24110af80..13471916559b 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
>> @@ -1166,10 +1166,10 @@ static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
>>          GEN6_RING_FAULT_REG_POSTING_READ(engine);
>>   }
>>   
>> -static void clear_error_registers(struct drm_i915_private *i915,
>> +static void clear_error_registers(struct intel_uncore *uncore,
>>                                    intel_engine_mask_t engine_mask)
>>   {
>> -       struct intel_uncore *uncore = &i915->uncore;
>> +       struct drm_i915_private *i915 = uncore_to_i915(uncore);
> 
> Grr, I should have objected to uncore_to_i915() loudly from the
> beginning
> 
> What's done is done,

Is it too late already? Shouldn't be. My thinking was the implementation 
can easily be changed if/when backpointer is added (instead of 
container_of). But if you would prefer we start without a helper, but 
with a direct access to backpointer straight away that is fine by me.

Regards,

Tvrtko

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 08/14] drm/i915: Store backpointer to intel_gt in the engine

[Tvrtko Ursulin]

On 10/06/2019 19:17, Daniele Ceraolo Spurio wrote:
> On 6/10/19 9:16 AM, Chris Wilson wrote:
>> Quoting Tvrtko Ursulin (2019-06-10 16:54:13)
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
>>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> index 01223864237a..343c4459e8a3 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> @@ -34,6 +34,7 @@ struct drm_i915_reg_table;
>>>   struct i915_gem_context;
>>>   struct i915_request;
>>>   struct i915_sched_attr;
>>> +struct intel_gt;
>>>   struct intel_uncore;
>>>   typedef u8 intel_engine_mask_t;
>>> @@ -266,6 +267,7 @@ struct intel_engine_execlists {
>>>   struct intel_engine_cs {
>>>          struct drm_i915_private *i915;
>>> +       struct intel_gt *gt;
>>
>> I'd push for gt as being the backpointer, and i915 its distant grand
>> parent. Not sure how much pain that would bring just for the elimination
>> of one more drm_i915_private, but that's how I picture the
>> encapsulation.

It depends on overall direction. Are we going to go with helpers 
(XXX_to_i915) or not. Well for removing engine->i915 there would be 
churn already. But same churn regardless of whether we pick 
engine_to_i915 or engine->gt->i915.

But I don't see a problem with having both i915 and gt pointers in the 
engine. It's a short cut to avoid pointer chasing and verbosity. Our 
code is fundamentally still very dependent on runtime checks against 
INTEL_GEN and INTEL_INFO, so i915 is pretty much in need all over the place.

> Would it be worth moving some of the flags in the device_info structure 
> in a gt substructure, like we did for display, and get a pointer to that 
> in intel_gt? We could save some jumps back that way and be more coherent 
> in where we store the info.

So even with this we maybe reduce the need to chase all the way to i915 
a bit, but not fully. Unless we decide to duplicate gen in intel_gt as 
well. Well.. now I am scared we will just decide to do that. :D

Regards,

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

Re: [RFC 01/14] drm/i915: Make i915_check_and_clear_faults take uncore

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-06-11 09:35:07)
> 
> On 10/06/2019 17:26, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-06-10 16:54:06)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Continuing the conversion and elimination of implicit dev_priv.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/gt/intel_engine_cs.c |  2 +-
> >>   drivers/gpu/drm/i915/gt/intel_reset.c     | 28 ++++++++++++-----------
> >>   drivers/gpu/drm/i915/gt/intel_reset.h     |  2 +-
> >>   drivers/gpu/drm/i915/i915_drv.c           |  2 +-
> >>   drivers/gpu/drm/i915/i915_gem_gtt.c       |  4 ++--
> >>   5 files changed, 20 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >> index c0d986db5a75..a046e8dccc96 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >> @@ -453,7 +453,7 @@ int intel_engines_init_mmio(struct drm_i915_private *i915)
> >>   
> >>          RUNTIME_INFO(i915)->num_engines = hweight32(mask);
> >>   
> >> -       i915_check_and_clear_faults(i915);
> >> +       i915_check_and_clear_faults(&i915->uncore);
> > 
> > This name is still setting off red flags for me, but I have to confess
> > that staring at it, passing uncore does make sense.
> 
> Rename to intel_uncore_check_and_clear_faults?
> 
> Or move later in the series as intel_gt_check_and_clear_faults?

I think I prefer the latter option, intel_gt_check_and_clear_faults.

> > I just wish we have per-engines faults everywhere and this could be
> > reduced to passing engine.
> > 
> > Hmm, this I guess we will just have to revisit in the near future as we
> > may get the opportunity to put these regs under more scrutiny.
> > 
> >>   
> >>          intel_setup_engine_capabilities(i915);
> >>   
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> >> index 60d24110af80..13471916559b 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> >> @@ -1166,10 +1166,10 @@ static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> >>          GEN6_RING_FAULT_REG_POSTING_READ(engine);
> >>   }
> >>   
> >> -static void clear_error_registers(struct drm_i915_private *i915,
> >> +static void clear_error_registers(struct intel_uncore *uncore,
> >>                                    intel_engine_mask_t engine_mask)
> >>   {
> >> -       struct intel_uncore *uncore = &i915->uncore;
> >> +       struct drm_i915_private *i915 = uncore_to_i915(uncore);
> > 
> > Grr, I should have objected to uncore_to_i915() loudly from the
> > beginning
> > 
> > What's done is done,
> 
> Is it too late already? Shouldn't be. My thinking was the implementation 
> can easily be changed if/when backpointer is added (instead of 
> container_of). But if you would prefer we start without a helper, but 
> with a direct access to backpointer straight away that is fine by me.

I'm optimistic that we can land a split display/gt intel_uncore early
and so the churn is in the not too distant future.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 08/14] drm/i915: Store backpointer to intel_gt in the engine

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-06-11 09:41:02)
> 
> On 10/06/2019 19:17, Daniele Ceraolo Spurio wrote:
> > On 6/10/19 9:16 AM, Chris Wilson wrote:
> >> Quoting Tvrtko Ursulin (2019-06-10 16:54:13)
> >>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
> >>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> >>> index 01223864237a..343c4459e8a3 100644
> >>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> >>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> >>> @@ -34,6 +34,7 @@ struct drm_i915_reg_table;
> >>>   struct i915_gem_context;
> >>>   struct i915_request;
> >>>   struct i915_sched_attr;
> >>> +struct intel_gt;
> >>>   struct intel_uncore;
> >>>   typedef u8 intel_engine_mask_t;
> >>> @@ -266,6 +267,7 @@ struct intel_engine_execlists {
> >>>   struct intel_engine_cs {
> >>>          struct drm_i915_private *i915;
> >>> +       struct intel_gt *gt;
> >>
> >> I'd push for gt as being the backpointer, and i915 its distant grand
> >> parent. Not sure how much pain that would bring just for the elimination
> >> of one more drm_i915_private, but that's how I picture the
> >> encapsulation.
> 
> It depends on overall direction. Are we going to go with helpers 
> (XXX_to_i915) or not. Well for removing engine->i915 there would be 
> churn already. But same churn regardless of whether we pick 
> engine_to_i915 or engine->gt->i915.
> 
> But I don't see a problem with having both i915 and gt pointers in the 
> engine. It's a short cut to avoid pointer chasing and verbosity. Our 
> code is fundamentally still very dependent on runtime checks against 
> INTEL_GEN and INTEL_INFO, so i915 is pretty much in need all over the place.
> 
> > Would it be worth moving some of the flags in the device_info structure 
> > in a gt substructure, like we did for display, and get a pointer to that 
> > in intel_gt? We could save some jumps back that way and be more coherent 
> > in where we store the info.
> 
> So even with this we maybe reduce the need to chase all the way to i915 
> a bit, but not fully. Unless we decide to duplicate gen in intel_gt as 
> well. Well.. now I am scared we will just decide to do that. :D

Kind off, we are already reducing the runtime checks into feature flags
or vfuncs for hot paths. I do hope the only time we need to go back to
i915 is during init. This should be reasonably true for engine; looking
at intel_lrc.c the common access is for i915->scratch, which we need to
move under intel_gt. And I expect that we will see similar natural
transitions for engine->i915.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] drm/i915: Make i915_check_and_clear_faults take uncore

[Tvrtko Ursulin]

On 11/06/2019 09:52, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-06-11 09:35:07)
>>
>> On 10/06/2019 17:26, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-06-10 16:54:06)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Continuing the conversion and elimination of implicit dev_priv.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/gt/intel_engine_cs.c |  2 +-
>>>>    drivers/gpu/drm/i915/gt/intel_reset.c     | 28 ++++++++++++-----------
>>>>    drivers/gpu/drm/i915/gt/intel_reset.h     |  2 +-
>>>>    drivers/gpu/drm/i915/i915_drv.c           |  2 +-
>>>>    drivers/gpu/drm/i915/i915_gem_gtt.c       |  4 ++--
>>>>    5 files changed, 20 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>> index c0d986db5a75..a046e8dccc96 100644
>>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>> @@ -453,7 +453,7 @@ int intel_engines_init_mmio(struct drm_i915_private *i915)
>>>>    
>>>>           RUNTIME_INFO(i915)->num_engines = hweight32(mask);
>>>>    
>>>> -       i915_check_and_clear_faults(i915);
>>>> +       i915_check_and_clear_faults(&i915->uncore);
>>>
>>> This name is still setting off red flags for me, but I have to confess
>>> that staring at it, passing uncore does make sense.
>>
>> Rename to intel_uncore_check_and_clear_faults?
>>
>> Or move later in the series as intel_gt_check_and_clear_faults?
> 
> I think I prefer the latter option, intel_gt_check_and_clear_faults.

Yep agreed.

Any comments on the intel_gt.c the series added?

And the end result in i915_gem_init(_hw)?

>>> I just wish we have per-engines faults everywhere and this could be
>>> reduced to passing engine.
>>>
>>> Hmm, this I guess we will just have to revisit in the near future as we
>>> may get the opportunity to put these regs under more scrutiny.
>>>
>>>>    
>>>>           intel_setup_engine_capabilities(i915);
>>>>    
>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
>>>> index 60d24110af80..13471916559b 100644
>>>> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
>>>> @@ -1166,10 +1166,10 @@ static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
>>>>           GEN6_RING_FAULT_REG_POSTING_READ(engine);
>>>>    }
>>>>    
>>>> -static void clear_error_registers(struct drm_i915_private *i915,
>>>> +static void clear_error_registers(struct intel_uncore *uncore,
>>>>                                     intel_engine_mask_t engine_mask)
>>>>    {
>>>> -       struct intel_uncore *uncore = &i915->uncore;
>>>> +       struct drm_i915_private *i915 = uncore_to_i915(uncore);
>>>
>>> Grr, I should have objected to uncore_to_i915() loudly from the
>>> beginning
>>>
>>> What's done is done,
>>
>> Is it too late already? Shouldn't be. My thinking was the implementation
>> can easily be changed if/when backpointer is added (instead of
>> container_of). But if you would prefer we start without a helper, but
>> with a direct access to backpointer straight away that is fine by me.
> 
> I'm optimistic that we can land a split display/gt intel_uncore early
> and so the churn is in the not too distant future.

Okay but that doesn't explicitly answer whether you prefer I just drop 
all the XXX_to_YYY wrappers in favour of using direct pointer dereferences.

You are also in favour of replacing engine->i915 with engine->gt 
straight away?

Regards,

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

Re: [RFC 01/14] drm/i915: Make i915_check_and_clear_faults take uncore

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-06-11 13:05:58)
> 
> On 11/06/2019 09:52, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-06-11 09:35:07)
> >>
> >> On 10/06/2019 17:26, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-06-10 16:54:06)
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Continuing the conversion and elimination of implicit dev_priv.
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >>>> ---
> >>>>    drivers/gpu/drm/i915/gt/intel_engine_cs.c |  2 +-
> >>>>    drivers/gpu/drm/i915/gt/intel_reset.c     | 28 ++++++++++++-----------
> >>>>    drivers/gpu/drm/i915/gt/intel_reset.h     |  2 +-
> >>>>    drivers/gpu/drm/i915/i915_drv.c           |  2 +-
> >>>>    drivers/gpu/drm/i915/i915_gem_gtt.c       |  4 ++--
> >>>>    5 files changed, 20 insertions(+), 18 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >>>> index c0d986db5a75..a046e8dccc96 100644
> >>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >>>> @@ -453,7 +453,7 @@ int intel_engines_init_mmio(struct drm_i915_private *i915)
> >>>>    
> >>>>           RUNTIME_INFO(i915)->num_engines = hweight32(mask);
> >>>>    
> >>>> -       i915_check_and_clear_faults(i915);
> >>>> +       i915_check_and_clear_faults(&i915->uncore);
> >>>
> >>> This name is still setting off red flags for me, but I have to confess
> >>> that staring at it, passing uncore does make sense.
> >>
> >> Rename to intel_uncore_check_and_clear_faults?
> >>
> >> Or move later in the series as intel_gt_check_and_clear_faults?
> > 
> > I think I prefer the latter option, intel_gt_check_and_clear_faults.
> 
> Yep agreed.
> 
> Any comments on the intel_gt.c the series added?

Good. It's the direction I think we need.
 
> And the end result in i915_gem_init(_hw)?

Definitely not the end yet, but passable for now :)

> >>> I just wish we have per-engines faults everywhere and this could be
> >>> reduced to passing engine.
> >>>
> >>> Hmm, this I guess we will just have to revisit in the near future as we
> >>> may get the opportunity to put these regs under more scrutiny.
> >>>
> >>>>    
> >>>>           intel_setup_engine_capabilities(i915);
> >>>>    
> >>>> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> >>>> index 60d24110af80..13471916559b 100644
> >>>> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> >>>> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> >>>> @@ -1166,10 +1166,10 @@ static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> >>>>           GEN6_RING_FAULT_REG_POSTING_READ(engine);
> >>>>    }
> >>>>    
> >>>> -static void clear_error_registers(struct drm_i915_private *i915,
> >>>> +static void clear_error_registers(struct intel_uncore *uncore,
> >>>>                                     intel_engine_mask_t engine_mask)
> >>>>    {
> >>>> -       struct intel_uncore *uncore = &i915->uncore;
> >>>> +       struct drm_i915_private *i915 = uncore_to_i915(uncore);
> >>>
> >>> Grr, I should have objected to uncore_to_i915() loudly from the
> >>> beginning
> >>>
> >>> What's done is done,
> >>
> >> Is it too late already? Shouldn't be. My thinking was the implementation
> >> can easily be changed if/when backpointer is added (instead of
> >> container_of). But if you would prefer we start without a helper, but
> >> with a direct access to backpointer straight away that is fine by me.
> > 
> > I'm optimistic that we can land a split display/gt intel_uncore early
> > and so the churn is in the not too distant future.
> 
> Okay but that doesn't explicitly answer whether you prefer I just drop 
> all the XXX_to_YYY wrappers in favour of using direct pointer dereferences.

I'm in favour of uncore->i915 over uncore_to_i915(uncore) and drop the
embedding knowledge.

> You are also in favour of replacing engine->i915 with engine->gt 
> straight away?

I can live with an extra back pointer. I think it will ultimately be
redundant, but expect a incremental evolution will be easier.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 08/14] drm/i915: Store backpointer to intel_gt in the engine

[Daniele Ceraolo Spurio]

On 6/11/2019 2:36 AM, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-06-11 09:41:02)
>> On 10/06/2019 19:17, Daniele Ceraolo Spurio wrote:
>>> On 6/10/19 9:16 AM, Chris Wilson wrote:
>>>> Quoting Tvrtko Ursulin (2019-06-10 16:54:13)
>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>>>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>>>> index 01223864237a..343c4459e8a3 100644
>>>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>>>> @@ -34,6 +34,7 @@ struct drm_i915_reg_table;
>>>>>    struct i915_gem_context;
>>>>>    struct i915_request;
>>>>>    struct i915_sched_attr;
>>>>> +struct intel_gt;
>>>>>    struct intel_uncore;
>>>>>    typedef u8 intel_engine_mask_t;
>>>>> @@ -266,6 +267,7 @@ struct intel_engine_execlists {
>>>>>    struct intel_engine_cs {
>>>>>           struct drm_i915_private *i915;
>>>>> +       struct intel_gt *gt;
>>>> I'd push for gt as being the backpointer, and i915 its distant grand
>>>> parent. Not sure how much pain that would bring just for the elimination
>>>> of one more drm_i915_private, but that's how I picture the
>>>> encapsulation.
>> It depends on overall direction. Are we going to go with helpers
>> (XXX_to_i915) or not. Well for removing engine->i915 there would be
>> churn already. But same churn regardless of whether we pick
>> engine_to_i915 or engine->gt->i915.
>>
>> But I don't see a problem with having both i915 and gt pointers in the
>> engine. It's a short cut to avoid pointer chasing and verbosity. Our
>> code is fundamentally still very dependent on runtime checks against
>> INTEL_GEN and INTEL_INFO, so i915 is pretty much in need all over the place.
>>
>>> Would it be worth moving some of the flags in the device_info structure
>>> in a gt substructure, like we did for display, and get a pointer to that
>>> in intel_gt? We could save some jumps back that way and be more coherent
>>> in where we store the info.
>> So even with this we maybe reduce the need to chase all the way to i915
>> a bit, but not fully. Unless we decide to duplicate gen in intel_gt as
>> well. Well.. now I am scared we will just decide to do that. :D
> Kind off, we are already reducing the runtime checks into feature flags
> or vfuncs for hot paths. I do hope the only time we need to go back to
> i915 is during init. This should be reasonably true for engine; looking
> at intel_lrc.c the common access is for i915->scratch, which we need to
> move under intel_gt. And I expect that we will see similar natural
> transitions for engine->i915.
> -Chris

There was also a mention a while back of splitting gt and display gens 
(https://patchwork.freedesktop.org/series/51860/), if we ever decide 
that that makes sense the gt gen will just naturally move and we'll save 
most of the jumps to i915.

Daniele

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

✓ Fi.CI.IGT: success for Implicit dev_priv removal (rev2)

[Patchwork]

== Series Details ==

Series: Implicit dev_priv removal (rev2)
URL   : https://patchwork.freedesktop.org/series/61705/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6227_full -> Patchwork_13227_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_13227_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13227_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13227_full:

### IGT changes ###

#### Warnings ####

  * igt@kms_hdmi_inject@inject-audio:
    - shard-iclb:         [DMESG-FAIL][1] ([fdo#109593]) -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-iclb1/igt@kms_hdmi_inject@inject-audio.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-iclb7/igt@kms_hdmi_inject@inject-audio.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108686])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-apl5/igt@gem_tiled_swapping@non-threaded.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-apl5/igt@gem_tiled_swapping@non-threaded.html

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#104108])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-skl4/igt@i915_pm_backlight@fade_with_suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-skl7/igt@i915_pm_backlight@fade_with_suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-256x256-random:
    - shard-snb:          [PASS][7] -> [SKIP][8] ([fdo#109271]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-snb4/igt@kms_cursor_crc@pipe-b-cursor-256x256-random.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-snb2/igt@kms_cursor_crc@pipe-b-cursor-256x256-random.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#105363])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-skl1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-pwrite:
    - shard-hsw:          [PASS][11] -> [SKIP][12] ([fdo#109271]) +17 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-hsw4/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-pwrite.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#103167])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([fdo#108566]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-apl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-apl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145] / [fdo#110403])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108145])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#103166])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [PASS][23] -> [FAIL][24] ([fdo#99912])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-kbl3/igt@kms_setmode@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-kbl2/igt@kms_setmode@basic.html

  * igt@perf_pmu@rc6:
    - shard-kbl:          [PASS][25] -> [SKIP][26] ([fdo#109271])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-kbl4/igt@perf_pmu@rc6.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-kbl2/igt@perf_pmu@rc6.html

  
#### Possible fixes ####

  * igt@gem_ctx_engines@execute-one:
    - shard-skl:          [DMESG-WARN][27] ([fdo#110869]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-skl4/igt@gem_ctx_engines@execute-one.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-skl2/igt@gem_ctx_engines@execute-one.html
    - shard-hsw:          [DMESG-WARN][29] ([fdo#110869]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-hsw5/igt@gem_ctx_engines@execute-one.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-hsw2/igt@gem_ctx_engines@execute-one.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-apl:          [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-apl1/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-apl6/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_cursor_legacy@cursora-vs-flipa-legacy:
    - shard-snb:          [SKIP][33] ([fdo#109271]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-snb6/igt@kms_cursor_legacy@cursora-vs-flipa-legacy.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-snb5/igt@kms_cursor_legacy@cursora-vs-flipa-legacy.html

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-ytiled:
    - shard-skl:          [FAIL][35] ([fdo#103184] / [fdo#103232] / [fdo#108222]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-skl9/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-ytiled.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-skl4/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-ytiled.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][37] ([fdo#105363]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][39] ([fdo#105363]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-skl5/igt@kms_flip@flip-vs-expired-vblank.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite:
    - shard-hsw:          [SKIP][41] ([fdo#109271]) -> [PASS][42] +32 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-hsw6/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         [FAIL][43] ([fdo#103167]) -> [PASS][44] +3 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [INCOMPLETE][45] ([fdo#103665]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-kbl2/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [FAIL][47] ([fdo#108145]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6227/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13227/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

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

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108222]: https://bugs.freedesktop.org/show_bug.cgi?id=108222
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110789]: https://bugs.freedesktop.org/show_bug.cgi?id=110789
  [fdo#110869]: https://bugs.freedesktop.org/show_bug.cgi?id=110869
  [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_6227 -> Patchwork_13227

  CI_DRM_6227: fe62c0390420632afe2193a40097c9f03a0bf725 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5050: 4c072238c784e6acb00634a80c3c55fb8358058b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13227: f862592a446ffe102ba9e650a22fe22e4ec9ff95 @ 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_13227/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx