Commit dd18cedf authored by Jakub Bartmiński's avatar Jakub Bartmiński Committed by Chris Wilson
Browse files

drm/i915/guc: Move the pin bias value from GuC to GGTT 



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.

v5:
Since we now don't use wopcm.guc.base for the pin bias there's no need to
validate it. It also has already been verified in WOPCM init.

v6:
Deleted the now unnecessarily introduced includes from previous versions.
Dropped naming changes from dev_priv to i915 for better patch readability.

v7:
Changed some comments to make more sense in the context they're in.

v8:
Moved and renamed the function which now returns the wopcm.guc.size to
intel_guc.c:intel_guc_reserved_gtt_size to avoid any possible confusion
with the pin_bias in ggtt, which should be used for pinning.
Fixed patch not applying or the most recent upstream.

Fixes: f7dc0157 ("drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init")
Testcase: igt/drv_selftest/mock_contexts #GuC
Signed-off-by: default avatarJakub 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>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180727141148.30874-3-jakub.bartminski@intel.com
parent b6445e17

drivers/gpu/drm/i915/i915_gem_context.c

+1 −9
Original line number Diff line number Diff line
@@ -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;

drivers/gpu/drm/i915/i915_gem_gtt.c

+9 −0
Original line number Diff line number Diff line
@@ -2929,6 +2929,15 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) 
	struct drm_mm_node *entry;

	int ret;



	/*

	 * GuC requires all resources that we're sharing with it 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_guc_reserved_gtt_size(&dev_priv->guc));



	ret = intel_vgt_balloon(dev_priv);

	if (ret)

		return ret;

drivers/gpu/drm/i915/i915_gem_gtt.h

+2 −0
Original line number Diff line number Diff line
@@ -401,6 +401,8 @@ struct i915_ggtt { 


	int mtrr;



	u32 pin_bias;



	struct drm_mm_node error_capture;

};

drivers/gpu/drm/i915/i915_vma.h

+5 −0
Original line number Diff line number Diff line
@@ -208,6 +208,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);

drivers/gpu/drm/i915/intel_guc.c

+20 −22
Original line number Diff line number Diff line
@@ -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);
@@ -144,8 +142,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;
@@ -606,22 +602,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);



	guc->ggtt_pin_bias = i915->wopcm.guc.size;

}



/**

 * intel_guc_allocate_vma() - Allocate a GGTT VMA for GuC usage

 * @guc:	the guc
@@ -640,6 +620,7 @@ 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_gem_object *obj;

	struct i915_vma *vma;

	u64 flags;

	int ret;



	obj = i915_gem_object_create(dev_priv, size);
@@ -650,8 +631,8 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size) 
	if (IS_ERR(vma))

		goto err;



	ret = i915_vma_pin(vma, 0, 0,

			   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, 0, flags);

	if (ret) {

		vma = ERR_PTR(ret);

		goto err;
@@ -663,3 +644,20 @@ err: 
	i915_gem_object_put(obj);

	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;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE