Removing the pin bias from GuC allows us to not check for GuC every time
we pin a context, which fixes the assertion error on unresolved GuC
platform default in mock contexts selftest.
It also seems that we were using uninitialized WOPCM variables when
setting the GuC pin bias. The pin bias has to be set after the WOPCM,
but before the call to i915_gem_contexts_init where the first contexts
are pinned.
v2:
This also makes it so that there's no need to set GuC variables from
within the WOPCM init function or to move the WOPCM init, while keeping
the correct initialization order. Also for mock tests the pin bias is
left at 0 and we make sure that the pin bias with GuC will not be
smaller than without GuC.
v3:
Avoid unused i915 in intel_guc_ggtt_offset if debug is disabled.
v4:
Squash with WOPCM init reordering.
Moved the i915_ggtt_pin_bias helper to this patch, and made some
functions use it instead of directly dereferencing i915->ggtt.
Fixes: f7dc0157e4b5 ("drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init")
Testcase: igt/drv_selftest/mock_contexts #GuC
Signed-off-by: Jakub Bartmiński <jakub.bartminski@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem_context.c | 22 ++++++-----------
drivers/gpu/drm/i915/i915_gem_gtt.c | 18 ++++++++++----
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
drivers/gpu/drm/i915/i915_vma.h | 5 ++++
drivers/gpu/drm/i915/intel_guc.c | 33 +++++--------------------
drivers/gpu/drm/i915/intel_guc.h | 11 +++------
drivers/gpu/drm/i915/intel_huc.c | 2 +-
drivers/gpu/drm/i915/intel_uc_fw.c | 2 +-
drivers/gpu/drm/i915/intel_wopcm.h | 18 ++++++++++++++
9 files changed, 57 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b10770cfccd2..ae27caad1766 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -265,7 +265,7 @@ static u32 default_desc_template(const struct drm_i915_private *i915,
}
static struct i915_gem_context *
-__create_hw_context(struct drm_i915_private *dev_priv,
+__create_hw_context(struct drm_i915_private *i915,
struct drm_i915_file_private *file_priv)
{
struct i915_gem_context *ctx;
@@ -276,15 +276,15 @@ __create_hw_context(struct drm_i915_private *dev_priv,
if (ctx == NULL)
return ERR_PTR(-ENOMEM);
- ret = assign_hw_id(dev_priv, &ctx->hw_id);
+ ret = assign_hw_id(i915, &ctx->hw_id);
if (ret) {
kfree(ctx);
return ERR_PTR(ret);
}
kref_init(&ctx->ref);
- list_add_tail(&ctx->link, &dev_priv->contexts.list);
- ctx->i915 = dev_priv;
+ list_add_tail(&ctx->link, &i915->contexts.list);
+ ctx->i915 = i915;
ctx->sched.priority = I915_PRIORITY_NORMAL;
for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++) {
@@ -322,22 +322,14 @@ __create_hw_context(struct drm_i915_private *dev_priv,
/* NB: Mark all slices as needing a remap so that when the context first
* loads it will restore whatever remap state already exists. If there
* is no remap info, it will be a NOP. */
- ctx->remap_slice = ALL_L3_SLICES(dev_priv);
+ ctx->remap_slice = ALL_L3_SLICES(i915);
i915_gem_context_set_bannable(ctx);
ctx->ring_size = 4 * PAGE_SIZE;
ctx->desc_template =
- default_desc_template(dev_priv, dev_priv->mm.aliasing_ppgtt);
+ default_desc_template(i915, i915->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 = i915->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..1f5d0334f0f5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2901,7 +2901,7 @@ void i915_gem_fini_aliasing_ppgtt(struct drm_i915_private *i915)
ggtt->vm.vma_ops.unbind_vma = ggtt_unbind_vma;
}
-int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
+int i915_gem_init_ggtt(struct drm_i915_private *i915)
{
/* Let GEM Manage all of the aperture.
*
@@ -2912,12 +2912,20 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
* aperture. One page should be enough to keep any prefetching inside
* of the aperture.
*/
- struct i915_ggtt *ggtt = &dev_priv->ggtt;
+ struct i915_ggtt *ggtt = &i915->ggtt;
unsigned long hole_start, hole_end;
struct drm_mm_node *entry;
int ret;
- ret = intel_vgt_balloon(dev_priv);
+ /*
+ * 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.
+ */
+ ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE,
+ intel_wopcm_guc_pin_bias(&i915->wopcm));
+
+ ret = intel_vgt_balloon(i915);
if (ret)
return ret;
@@ -2940,8 +2948,8 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
/* And finally clear the reserved guard page */
ggtt->vm.clear_range(&ggtt->vm, ggtt->vm.total - PAGE_SIZE, PAGE_SIZE);
- if (USES_PPGTT(dev_priv) && !USES_FULL_PPGTT(dev_priv)) {
- ret = i915_gem_init_aliasing_ppgtt(dev_priv);
+ if (USES_PPGTT(i915) && !USES_FULL_PPGTT(i915)) {
+ ret = i915_gem_init_aliasing_ppgtt(i915);
if (ret)
goto err;
}
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/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index f06d66377107..abf6144e3296 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -207,6 +207,11 @@ static inline u32 i915_ggtt_offset(const struct i915_vma *vma)
return lower_32_bits(vma->node.start);
}
+static inline u32 i915_ggtt_pin_bias(struct i915_vma *vma)
+{
+ return i915_vm_to_ggtt(vma->vm)->pin_bias;
+}
+
static inline struct i915_vma *i915_vma_get(struct i915_vma *vma)
{
i915_gem_object_get(vma->obj);
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 17753952933e..3a60dd4ffdce 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -27,8 +27,6 @@
#include "intel_guc_submission.h"
#include "i915_drv.h"
-static void guc_init_ggtt_pin_bias(struct intel_guc *guc);
-
static void gen8_guc_raise_irq(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -142,8 +140,6 @@ int intel_guc_init_misc(struct intel_guc *guc)
struct drm_i915_private *i915 = guc_to_i915(guc);
int ret;
- guc_init_ggtt_pin_bias(guc);
-
ret = guc_init_wq(guc);
if (ret)
return ret;
@@ -604,24 +600,6 @@ int intel_guc_resume(struct intel_guc *guc)
* to DRAM. The value of the GuC ggtt_pin_bias is the GuC WOPCM size.
*/
-/**
- * guc_init_ggtt_pin_bias() - Initialize the GuC ggtt_pin_bias value.
- * @guc: intel_guc structure.
- *
- * This function will calculate and initialize the ggtt_pin_bias value
- * based on the GuC WOPCM size.
- */
-static void guc_init_ggtt_pin_bias(struct intel_guc *guc)
-{
- struct drm_i915_private *i915 = guc_to_i915(guc);
-
- GEM_BUG_ON(!i915->wopcm.size);
- GEM_BUG_ON(range_overflows(i915->wopcm.guc.base, i915->wopcm.guc.size,
- i915->wopcm.size));
-
- guc->ggtt_pin_bias = i915->wopcm.guc.size;
-}
-
/**
* intel_guc_allocate_vma() - Allocate a GGTT VMA for GuC usage
* @guc: the guc
@@ -637,21 +615,22 @@ static void guc_init_ggtt_pin_bias(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;
+ u64 flags;
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);
+ flags = PIN_GLOBAL | PIN_OFFSET_BIAS | i915_ggtt_pin_bias(vma);
+ ret = i915_vma_pin(vma, 0, PAGE_SIZE, flags);
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..751f31c3c6c4 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;
@@ -130,10 +127,10 @@ static inline void intel_guc_to_host_event_handler(struct intel_guc *guc)
* @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
+ * [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 GuC ggtt_pin_bias.
+ * and pinned with PIN_OFFSET_BIAS along with the value of ggtt.pin_bias.
*
* Return: GGTT offset of the @vma.
*/
@@ -142,7 +139,7 @@ static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
{
u32 offset = i915_ggtt_offset(vma);
- GEM_BUG_ON(offset < guc->ggtt_pin_bias);
+ GEM_BUG_ON(offset < i915_ggtt_pin_bias(vma));
GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
return offset;
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.h b/drivers/gpu/drm/i915/intel_wopcm.h
index 6298910a384c..2dbbb9b971a4 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.h
+++ b/drivers/gpu/drm/i915/intel_wopcm.h
@@ -7,6 +7,8 @@
#ifndef _INTEL_WOPCM_H_
#define _INTEL_WOPCM_H_
+#include "i915_gem.h"
+#include "i915_utils.h"
#include <linux/types.h>
/**
@@ -24,6 +26,22 @@ struct intel_wopcm {
} guc;
};
+/**
+ * intel_wopcm_guc_pin_bias() - Get the GuC pin bias value.
+ * @wopcm: pointer to intel_wopcm.
+ *
+ * Returns:
+ * 0 if GuC is not present or not in use.
+ * Otherwise, the GuC pin bias value based on the GuC WOPCM size.
+ */
+static inline u32 intel_wopcm_guc_pin_bias(struct intel_wopcm *wopcm)
+{
+ GEM_BUG_ON(!wopcm->size);
+ GEM_BUG_ON(range_overflows(wopcm->guc.base, wopcm->guc.size,
+ wopcm->size));
+ 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.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
