On 21/06/16 19:11, Peter Antoine wrote:
This patch adds the HuC Loading for the BXT.
Version 1.x of the HuC firmware.
Signed-off-by: Peter Antoine <peter.antoine@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem.c | 13 +++----------
drivers/gpu/drm/i915/intel_guc_loader.c | 29 +++++++++++++++++------------
drivers/gpu/drm/i915/intel_huc_loader.c | 7 +++++++
3 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 549dd3f..6abd5e5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5143,16 +5143,9 @@ i915_gem_init_hw(struct drm_device *dev)
intel_mocs_init_l3cc_table(dev);
/* We can't enable contexts until all firmware is loaded */
- if (HAS_GUC(dev)) {
- /* init WOPCM */
- I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));
- I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE);
-
- intel_huc_load(dev);
- ret = intel_guc_setup(dev);
- if (ret)
- goto out;
- }
+ ret = intel_guc_setup(dev);
+ if (ret)
+ goto out;
This reverts one of the earlier changes that I objected to!
Please squash them together so it isn't changed and then changed back.
/*
* Increment the next seqno by 0x100 so we have a visible break
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index e876a23f..289b5b6 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -82,6 +82,17 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
}
};
+u32 guc_wopcm_size(struct drm_device *dev)
+{
+ u32 wopcm_size = GUC_WOPCM_TOP;
+
+ /* On BXT, the top of WOPCM is reserved for RC6 context */
+ if (IS_BROXTON(dev))
+ wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+
+ return wopcm_size;
+}
+
static void direct_interrupts_to_host(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
@@ -296,17 +307,6 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
return ret;
}
-u32 guc_wopcm_size(struct drm_device *dev)
-{
- u32 wopcm_size = GUC_WOPCM_TOP;
-
- /* On BXT, the top of WOPCM is reserved for RC6 context */
- if (IS_BROXTON(dev))
- wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
-
- return wopcm_size;
-}
-
This is just being moved around? Does it need to be?
/*
* Load the GuC firmware blob into the MinuteIA.
*/
@@ -333,6 +333,10 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+ /* init WOPCM */
+ I915_WRITE(GUC_WOPCM_SIZE, guc_wopcm_size(dev));
+ I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE);
+
Is this being moved around (again?)
I think we should end up with GUC_WOPCM_SIZE being set just once.
/* Enable MIA caching. GuC clock gating is disabled. */
I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
@@ -483,6 +487,7 @@ int intel_guc_setup(struct drm_device *dev)
goto fail;
}
+ intel_huc_load(dev);
err = guc_ucode_xfer(dev_priv);
if (!err)
break;
@@ -638,7 +643,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct intel_uc_fw *uc_fw)
size = uc_fw->header_size + uc_fw->ucode_size;
/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
- if (size > guc_wopcm_size(dev->dev_private)) {
+ if (size > guc_wopcm_size(dev)) {
DRM_ERROR("Firmware is too large to fit in WOPCM\n");
goto fail;
}
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 7205e9e..7a43d4e 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -49,6 +49,9 @@
#define I915_SKL_HUC_UCODE "i915/skl_huc_ver1.bin"
MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
+#define I915_BXT_HUC_UCODE "i915/bxt_huc_ver1.bin"
+MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
+
/**
* intel_huc_load_ucode() - DMA's the firmware
* @dev: the drm device
@@ -157,6 +160,10 @@ void intel_huc_init(struct drm_device *dev)
fw_path = I915_SKL_HUC_UCODE;
huc_fw->major_ver_wanted = 1;
huc_fw->minor_ver_wanted = 5;
+ } else if (IS_BROXTON(dev)) {
+ fw_path = I915_BXT_HUC_UCODE;
+ huc_fw->major_ver_wanted = 1;
+ huc_fw->minor_ver_wanted = 0;
}
if (fw_path == NULL)
Overall: I think the general functionality is OK, but there are a few
specific points that need to be fixed (unless they already are, by a
later patch). In addition, the series need to be reorganised so that we
don't have the same lines of code repeatedly shuffled from one place to
another, or a change made and then reverted.
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
