diff mbox

[2/3] drm/i915/guc: Move the pin bias value from GuC to GGTT

Message ID 20180717115600.5938-2-jakub.bartminski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bartminski, Jakub July 17, 2018, 11:55 a.m. UTC
Removing the pin bias from GuC allows us to not check for GuC when
pinning contexts, which fixes the assertion error on unresolved GuC
platform default in mock context selftest.

WOPCM (and GuC-specific pin bias) initialization has to be postponed until
the default value is set for ggtt.pin_bias inside i915_gem_init_ggtt, and
still has to occur before context pinning inside i915_gem_contexts_init.

With this change the intel_guc_ggtt_offset function has to know the full
declaration of the drm_i915_private structure so it can no longer be
located in the intel_guc.h header file due to include order.

Signed-off-by: Jakub Bartmiński <jakub.bartminski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c           |  8 +++---
 drivers/gpu/drm/i915/i915_gem_context.c   | 10 +------
 drivers/gpu/drm/i915/i915_gem_gtt.c       |  6 +++++
 drivers/gpu/drm/i915/i915_gem_gtt.h       |  2 ++
 drivers/gpu/drm/i915/intel_guc.c          | 33 ++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_guc.h          | 28 +------------------
 drivers/gpu/drm/i915/intel_huc.c          |  2 +-
 drivers/gpu/drm/i915/intel_uc_fw.c        |  2 +-
 drivers/gpu/drm/i915/intel_wopcm.c        | 13 ++++-----
 drivers/gpu/drm/i915/selftests/mock_gtt.c |  2 ++
 10 files changed, 54 insertions(+), 52 deletions(-)

Comments

Chris Wilson July 17, 2018, 12:06 p.m. UTC | #1
Quoting Jakub Bartmiński (2018-07-17 12:55:59)
> @@ -5490,6 +5486,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>                 goto err_unlock;
>         }
>  
> +       ret = intel_wopcm_init(&dev_priv->wopcm);
> +       if (ret)
> +               goto err_uc_misc;

Hmm. are you sure about the unwind? Looks like it should be err_ggtt.

For science, I would suggest putting a couple of i915_inject_fault()
around the wopcm_init() paths.

> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index b10770cfccd2..32f96b8cd9c4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -329,15 +329,7 @@ __create_hw_context(struct drm_i915_private *dev_priv,
>         ctx->desc_template =
>                 default_desc_template(dev_priv, dev_priv->mm.aliasing_ppgtt);
>  
> -       /*
> -        * GuC requires the ring to be placed in Non-WOPCM memory. If GuC is not
> -        * present or not in use we still need a small bias as ring wraparound
> -        * at offset 0 sometimes hangs. No idea why.
> -        */
> -       if (USES_GUC(dev_priv))
> -               ctx->ggtt_offset_bias = dev_priv->guc.ggtt_pin_bias;
> -       else
> -               ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
> +       ctx->ggtt_offset_bias = dev_priv->ggtt.pin_bias;

We could probably also remove it from ctx now (in a later patch).

> -static void wopcm_init_guc_ggtt_pin_bias(struct intel_wopcm *wopcm)
> +static void wopcm_init_ggtt_pin_bias(struct intel_wopcm *wopcm)
>  {
>         struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
>  
>         GEM_BUG_ON(!wopcm->size);
>         GEM_BUG_ON(wopcm->size < wopcm->guc.base);
>  
> -       i915->guc.ggtt_pin_bias = wopcm->size - wopcm->guc.base;
> +       i915->ggtt.pin_bias = wopcm->size - wopcm->guc.base;

For safety,
	i915->ggtt.pin_bias = max(i915->gtt.pin_bias,
				  wopcm->size - wopcm->guc.base);

> @@ -224,7 +225,7 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
>         wopcm->guc.base = guc_wopcm_base;
>         wopcm->guc.size = guc_wopcm_size;
>  
> -       wopcm_init_guc_ggtt_pin_bias(wopcm);
> +       wopcm_init_ggtt_pin_bias(wopcm);
>  
>         return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c
> index a140ea5c3a7c..3afff10e94d9 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c
> @@ -117,6 +117,8 @@ void mock_init_ggtt(struct drm_i915_private *i915)
>         ggtt->vm.vma_ops.set_pages   = ggtt_set_pages;
>         ggtt->vm.vma_ops.clear_pages = clear_pages;
>  
> +       ggtt->pin_bias = I915_GTT_PAGE_SIZE;

For mocking, we can leave it at zero (I think we don't assert that
PIN_OFFSET_BIAS can't be zero).
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ed2be33ec58a..45f69f588e6d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5471,10 +5471,6 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
-	ret = intel_wopcm_init(&dev_priv->wopcm);
-	if (ret)
-		goto err_uc_misc;
-
 	/* This is just a security blanket to placate dragons.
 	 * On some systems, we very sporadically observe that the first TLBs
 	 * used by the CS may be stale, despite us poking the TLB reset. If
@@ -5490,6 +5486,10 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 		goto err_unlock;
 	}
 
+	ret = intel_wopcm_init(&dev_priv->wopcm);
+	if (ret)
+		goto err_uc_misc;
+
 	ret = i915_gem_contexts_init(dev_priv);
 	if (ret) {
 		GEM_BUG_ON(ret == -EIO);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b10770cfccd2..32f96b8cd9c4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -329,15 +329,7 @@  __create_hw_context(struct drm_i915_private *dev_priv,
 	ctx->desc_template =
 		default_desc_template(dev_priv, dev_priv->mm.aliasing_ppgtt);
 
-	/*
-	 * GuC requires the ring to be placed in Non-WOPCM memory. If GuC is not
-	 * present or not in use we still need a small bias as ring wraparound
-	 * at offset 0 sometimes hangs. No idea why.
-	 */
-	if (USES_GUC(dev_priv))
-		ctx->ggtt_offset_bias = dev_priv->guc.ggtt_pin_bias;
-	else
-		ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
+	ctx->ggtt_offset_bias = dev_priv->ggtt.pin_bias;
 
 	return ctx;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d0acef299b9c..62bfb06a3aae 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2917,6 +2917,12 @@  int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	struct drm_mm_node *entry;
 	int ret;
 
+	/*
+	 * We need a small bias as ring wraparound at offset 0
+	 * sometimes hangs. No idea why.
+	 */
+	ggtt->pin_bias = I915_GTT_PAGE_SIZE;
+
 	ret = intel_vgt_balloon(dev_priv);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 14e62651010b..71e8cf24c800 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -396,6 +396,8 @@  struct i915_ggtt {
 
 	int mtrr;
 
+	u32 pin_bias;
+
 	struct drm_mm_node error_capture;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 47cacd59ac32..eb9a2caa6560 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -607,6 +607,31 @@  int intel_guc_resume(struct intel_guc *guc)
  * actual GuC WOPCM size.
  */
 
+/**
+ * intel_guc_ggtt_offset() - Get and validate the GGTT offset of @vma
+ * @guc: intel_guc structure.
+ * @vma: i915 graphics virtual memory area.
+ *
+ * GuC does not allow any gfx GGTT address that falls into range
+ * [0, ggtt.pin_bias), which is reserved for Boot ROM, SRAM and WOPCM.
+ * Currently, in order to exclude [0, ggtt.pin_bias) address space from
+ * GGTT, all gfx objects used by GuC are allocated with intel_guc_allocate_vma()
+ * and pinned with PIN_OFFSET_BIAS along with the value of ggtt.pin_bias.
+ *
+ * Return: GGTT offset of the @vma.
+ */
+u32 intel_guc_ggtt_offset(struct intel_guc *guc, struct i915_vma *vma)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+
+	u32 offset = i915_ggtt_offset(vma);
+
+	GEM_BUG_ON(offset < i915->ggtt.pin_bias);
+	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
+
+	return offset;
+}
+
 /**
  * intel_guc_allocate_vma() - Allocate a GGTT VMA for GuC usage
  * @guc:	the guc
@@ -622,21 +647,21 @@  int intel_guc_resume(struct intel_guc *guc)
  */
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct drm_i915_private *i915 = guc_to_i915(guc);
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
 	int ret;
 
-	obj = i915_gem_object_create(dev_priv, size);
+	obj = i915_gem_object_create(i915, size);
 	if (IS_ERR(obj))
 		return ERR_CAST(obj);
 
-	vma = i915_vma_instance(obj, &dev_priv->ggtt.vm, NULL);
+	vma = i915_vma_instance(obj, &i915->ggtt.vm, NULL);
 	if (IS_ERR(vma))
 		goto err;
 
 	ret = i915_vma_pin(vma, 0, PAGE_SIZE,
-			   PIN_GLOBAL | PIN_OFFSET_BIAS | guc->ggtt_pin_bias);
+			   PIN_GLOBAL | PIN_OFFSET_BIAS | i915->ggtt.pin_bias);
 	if (ret) {
 		vma = ERR_PTR(ret);
 		goto err;
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 4121928a495e..b0e5c99c0338 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -49,9 +49,6 @@  struct intel_guc {
 	struct intel_guc_log log;
 	struct intel_guc_ct ct;
 
-	/* Offset where Non-WOPCM memory starts. */
-	u32 ggtt_pin_bias;
-
 	/* Log snapshot if GuC errors during load */
 	struct drm_i915_gem_object *load_err_log;
 
@@ -123,30 +120,7 @@  static inline void intel_guc_to_host_event_handler(struct intel_guc *guc)
 
 /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
 #define GUC_GGTT_TOP	0xFEE00000
-
-/**
- * intel_guc_ggtt_offset() - Get and validate the GGTT offset of @vma
- * @guc: intel_guc structure.
- * @vma: i915 graphics virtual memory area.
- *
- * GuC does not allow any gfx GGTT address that falls into range
- * [0, GuC ggtt_pin_bias), which is reserved for Boot ROM, SRAM and WOPCM.
- * Currently, in order to exclude [0, GuC ggtt_pin_bias) address space from
- * GGTT, all gfx objects used by GuC are allocated with intel_guc_allocate_vma()
- * and pinned with PIN_OFFSET_BIAS along with the value of GuC ggtt_pin_bias.
- *
- * Return: GGTT offset of the @vma.
- */
-static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
-					struct i915_vma *vma)
-{
-	u32 offset = i915_ggtt_offset(vma);
-
-	GEM_BUG_ON(offset < guc->ggtt_pin_bias);
-	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
-
-	return offset;
-}
+u32 intel_guc_ggtt_offset(struct intel_guc *guc, struct i915_vma *vma);
 
 void intel_guc_init_early(struct intel_guc *guc);
 void intel_guc_init_send_regs(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index ffcad5fad6a7..37ef540dd280 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -63,7 +63,7 @@  int intel_huc_auth(struct intel_huc *huc)
 		return -ENOEXEC;
 
 	vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
-				       PIN_OFFSET_BIAS | guc->ggtt_pin_bias);
+				       PIN_OFFSET_BIAS | i915->ggtt.pin_bias);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		DRM_ERROR("HuC: Failed to pin huc fw object %d\n", ret);
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 6e8e0b546743..fd496416087c 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -222,7 +222,7 @@  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 		goto fail;
 	}
 
-	ggtt_pin_bias = to_i915(uc_fw->obj->base.dev)->guc.ggtt_pin_bias;
+	ggtt_pin_bias = to_i915(uc_fw->obj->base.dev)->ggtt.pin_bias;
 	vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
 				       PIN_OFFSET_BIAS | ggtt_pin_bias);
 	if (IS_ERR(vma)) {
diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index 02f602db9548..a869fad27db3 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -141,20 +141,21 @@  static inline int check_hw_restriction(struct drm_i915_private *i915,
 }
 
 /**
- * wopcm_init_guc_ggtt_pin_bias() - Initialize the GuC ggtt_pin_bias value.
+ * wopcm_init_ggtt_pin_bias() - Initialize the ggtt pin bias value.
  * @wopcm: pointer to intel_wopcm.
  *
- * This function will calculate and initialize the GuC ggtt_pin_bias value based
- * on overall WOPCM size and GuC WOPCM size.
+ * Any objects shared with GuC can't overlap with WOPCM.
+ * This function will calculate and initialize the ggtt pin bias value
+ * based on overall WOPCM size and GuC WOPCM size.
  */
-static void wopcm_init_guc_ggtt_pin_bias(struct intel_wopcm *wopcm)
+static void wopcm_init_ggtt_pin_bias(struct intel_wopcm *wopcm)
 {
 	struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
 
 	GEM_BUG_ON(!wopcm->size);
 	GEM_BUG_ON(wopcm->size < wopcm->guc.base);
 
-	i915->guc.ggtt_pin_bias = wopcm->size - wopcm->guc.base;
+	i915->ggtt.pin_bias = wopcm->size - wopcm->guc.base;
 }
 
 /**
@@ -224,7 +225,7 @@  int intel_wopcm_init(struct intel_wopcm *wopcm)
 	wopcm->guc.base = guc_wopcm_base;
 	wopcm->guc.size = guc_wopcm_size;
 
-	wopcm_init_guc_ggtt_pin_bias(wopcm);
+	wopcm_init_ggtt_pin_bias(wopcm);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c
index a140ea5c3a7c..3afff10e94d9 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c
@@ -117,6 +117,8 @@  void mock_init_ggtt(struct drm_i915_private *i915)
 	ggtt->vm.vma_ops.set_pages   = ggtt_set_pages;
 	ggtt->vm.vma_ops.clear_pages = clear_pages;
 
+	ggtt->pin_bias = I915_GTT_PAGE_SIZE;
+
 	i915_address_space_init(&ggtt->vm, i915);
 }