* [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files
@ 2018-01-19 1:29 Jackie Li
2018-01-19 1:29 ` [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset Jackie Li
` (7 more replies)
0 siblings, 8 replies; 23+ messages in thread
From: Jackie Li @ 2018-01-19 1:29 UTC (permalink / raw)
To: intel-gfx
intel_guc_reg.h should only include definition for GuC registers
and related register bits. GuC WOPCM related values should not
be defined in intel_guc_reg.h
This patch creates a better file structure by moving GuC WOPCM
related definitions int to a new header intel_guc_wopcm.h
and moving GuC WOPCM related functions to a new source file
intel_guc_wopcm.c
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/intel_guc.c | 11 --------
drivers/gpu/drm/i915/intel_guc.h | 2 +-
drivers/gpu/drm/i915/intel_guc_reg.h | 4 ---
drivers/gpu/drm/i915/intel_guc_wopcm.c | 47 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_guc_wopcm.h | 39 ++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_uc.c | 2 +-
drivers/gpu/drm/i915/intel_uc_fw.c | 2 +-
8 files changed, 90 insertions(+), 18 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_guc_wopcm.c
create mode 100644 drivers/gpu/drm/i915/intel_guc_wopcm.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 3bddd8a..1dc9988 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -88,6 +88,7 @@ i915-y += intel_uc.o \
intel_guc_fw.o \
intel_guc_log.o \
intel_guc_submission.o \
+ intel_guc_wopcm.o \
intel_huc.o
# autogenerated null render state
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index ea30e7c..b8b6d4a 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -504,14 +504,3 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
i915_gem_object_put(obj);
return vma;
}
-
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
-{
- u32 wopcm_size = GUC_WOPCM_TOP;
-
- /* On BXT, the top of WOPCM is reserved for RC6 context */
- if (IS_GEN9_LP(dev_priv))
- wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
-
- return wopcm_size;
-}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 52856a9..9e0a97e 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -31,6 +31,7 @@
#include "intel_guc_ct.h"
#include "intel_guc_log.h"
#include "intel_guc_reg.h"
+#include "intel_guc_wopcm.h"
#include "intel_uc_fw.h"
#include "i915_vma.h"
@@ -130,6 +131,5 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
int intel_guc_suspend(struct drm_i915_private *dev_priv);
int intel_guc_resume(struct drm_i915_private *dev_priv);
struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
#endif
diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index 19a9247..1f52fb8 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -68,7 +68,6 @@
#define DMA_GUC_WOPCM_OFFSET _MMIO(0xc340)
#define HUC_LOADING_AGENT_VCR (0<<1)
#define HUC_LOADING_AGENT_GUC (1<<1)
-#define GUC_WOPCM_OFFSET_VALUE 0x80000 /* 512KB */
#define GUC_MAX_IDLE_COUNT _MMIO(0xC3E4)
#define HUC_STATUS2 _MMIO(0xD3B0)
@@ -76,9 +75,6 @@
/* Defines WOPCM space available to GuC firmware */
#define GUC_WOPCM_SIZE _MMIO(0xc050)
-/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
-#define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */
-#define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */
/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
#define GUC_GGTT_TOP 0xFEE00000
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
new file mode 100644
index 0000000..87643a0
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
@@ -0,0 +1,47 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "intel_guc_wopcm.h"
+#include "i915_drv.h"
+
+/*
+ * intel_guc_wopcm_size() - Get the size of GuC WOPCM.
+ * @guc: intel guc.
+ *
+ * Get the platform specific GuC WOPCM size.
+ *
+ * Return: size of the GuC WOPCM.
+ */
+u32 intel_guc_wopcm_size(struct intel_guc *guc)
+{
+ struct drm_i915_private *i915 = guc_to_i915(guc);
+
+ u32 wopcm_size = GUC_WOPCM_TOP;
+
+ /* On BXT, the top of WOPCM is reserved for RC6 context */
+ if (IS_GEN9_LP(i915))
+ wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+
+ return wopcm_size;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
new file mode 100644
index 0000000..595fb1c
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _INTEL_GUC_WOPCM_H_
+#define _INTEL_GUC_WOPCM_H_
+
+#include <linux/types.h>
+
+struct intel_guc;
+
+#define GUC_WOPCM_OFFSET_VALUE 0x80000 /* 512KB */
+/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
+#define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */
+#define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */
+
+u32 intel_guc_wopcm_size(struct intel_guc *guc);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f78a17b..e9d0568 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -320,7 +320,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
gen9_reset_guc_interrupts(dev_priv);
/* init WOPCM */
- I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
+ I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));
I915_WRITE(DMA_GUC_WOPCM_OFFSET,
GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 784eff9..24945cf 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -97,7 +97,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
/* Header and uCode will be loaded to WOPCM */
size = uc_fw->header_size + uc_fw->ucode_size;
- if (size > intel_guc_wopcm_size(dev_priv)) {
+ if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
DRM_WARN("%s: Firmware is too large to fit in WOPCM\n",
intel_uc_fw_type_repr(uc_fw->type));
err = -E2BIG;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
@ 2018-01-19 1:29 ` Jackie Li
2018-02-01 7:27 ` Sagar Arun Kamble
2018-02-01 7:38 ` Chris Wilson
2018-01-19 1:29 ` [PATCH v7 3/6] drm/i915/guc: Implement dynamic GuC WOPCM offset and size Jackie Li
` (6 subsequent siblings)
7 siblings, 2 replies; 23+ messages in thread
From: Jackie Li @ 2018-01-19 1:29 UTC (permalink / raw)
To: intel-gfx
GuC related exported functions should start with "intel_guc_"
prefix and pass intel_guc as the first parameter since its guc
related. Current guc_ggtt_offset() failed to follow this code
convention.
This patch renames the guc_ggtt_offset to intel_guc_ggtt_offset
and updates the related code to pass intel_guc pointer to
this function call. so that we have a unified coding style for
GuC code.
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
drivers/gpu/drm/i915/intel_guc.c | 12 +++++++-----
drivers/gpu/drm/i915/intel_guc.h | 10 ++++++++--
drivers/gpu/drm/i915/intel_guc_ads.c | 25 +++++++++++++------------
drivers/gpu/drm/i915/intel_guc_ct.c | 5 +++--
drivers/gpu/drm/i915/intel_guc_fw.c | 2 +-
drivers/gpu/drm/i915/intel_guc_log.c | 2 +-
drivers/gpu/drm/i915/intel_guc_submission.c | 10 +++++-----
drivers/gpu/drm/i915/intel_huc.c | 6 ++++--
8 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index b8b6d4a..e70885b 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -264,8 +264,10 @@ void intel_guc_init_params(struct intel_guc *guc)
/* If GuC submission is enabled, set up additional parameters here */
if (USES_GUC_SUBMISSION(dev_priv)) {
- u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
- u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
+ u32 ads = intel_guc_ggtt_offset(guc,
+ guc->ads_vma) >> PAGE_SHIFT;
+ u32 pgs = intel_guc_ggtt_offset(guc,
+ dev_priv->guc.stage_desc_pool);
u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
@@ -413,7 +415,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
/* any value greater than GUC_POWER_D0 */
data[1] = GUC_POWER_D1;
- data[2] = guc_ggtt_offset(guc->shared_data);
+ data[2] = intel_guc_ggtt_offset(guc, guc->shared_data);
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
@@ -436,7 +438,7 @@ int intel_guc_reset_engine(struct intel_guc *guc,
data[3] = 0;
data[4] = 0;
data[5] = guc->execbuf_client->stage_id;
- data[6] = guc_ggtt_offset(guc->shared_data);
+ data[6] = intel_guc_ggtt_offset(guc, guc->shared_data);
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
@@ -458,7 +460,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
data[1] = GUC_POWER_D0;
- data[2] = guc_ggtt_offset(guc->shared_data);
+ data[2] = intel_guc_ggtt_offset(guc, guc->shared_data);
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 9e0a97e..b7e2a18 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -101,13 +101,19 @@ static inline void intel_guc_notify(struct intel_guc *guc)
guc->notify(guc);
}
-/*
+/* intel_guc_ggtt_offset() - Get the GGTT offset of @vma.
+ * @guc: intel guc.
+ * @vma: i915 graphics virtual memory area.
+ *
* GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
* which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
* 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
* used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
+ *
+ * Return: GGTT offset that meets the GuC gfx address requirement.
*/
-static inline u32 guc_ggtt_offset(struct i915_vma *vma)
+static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
+ struct i915_vma *vma)
{
u32 offset = i915_ggtt_offset(vma);
diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c b/drivers/gpu/drm/i915/intel_guc_ads.c
index ac62753..7215594 100644
--- a/drivers/gpu/drm/i915/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/intel_guc_ads.c
@@ -113,17 +113,6 @@ int intel_guc_ads_create(struct intel_guc *guc)
blob->reg_state.white_list[engine->guc_id].count = 0;
}
- /*
- * The GuC requires a "Golden Context" when it reinitialises
- * engines after a reset. Here we use the Render ring default
- * context, which must already exist and be pinned in the GGTT,
- * so its address won't change after we've told the GuC where
- * to find it. Note that we have to skip our header (1 page),
- * because our GuC shared data is there.
- */
- blob->ads.golden_context_lrca =
- guc_ggtt_offset(dev_priv->kernel_context->engine[RCS].state) +
- skipped_offset;
/*
* The GuC expects us to exclude the portion of the context image that
@@ -135,11 +124,23 @@ int intel_guc_ads_create(struct intel_guc *guc)
blob->ads.eng_state_size[engine->guc_id] =
engine->context_size - skipped_size;
- base = guc_ggtt_offset(vma);
+ base = intel_guc_ggtt_offset(guc, vma);
blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
blob->ads.reg_state_buffer = base + ptr_offset(blob, reg_state_buffer);
blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
+ /*
+ * The GuC requires a "Golden Context" when it reinitialises
+ * engines after a reset. Here we use the Render ring default
+ * context, which must already exist and be pinned in the GGTT,
+ * so its address won't change after we've told the GuC where
+ * to find it. Note that we have to skip our header (1 page),
+ * because our GuC shared data is there.
+ */
+ vma = dev_priv->kernel_context->engine[RCS].state;
+ blob->ads.golden_context_lrca = intel_guc_ggtt_offset(guc, vma) +
+ skipped_offset;
+
kunmap(page);
return 0;
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 24ad557..0a0d3d5 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -156,7 +156,8 @@ static int ctch_init(struct intel_guc *guc,
err = PTR_ERR(blob);
goto err_vma;
}
- DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
+ DRM_DEBUG_DRIVER("CT: vma base=%#x\n",
+ intel_guc_ggtt_offset(guc, ctch->vma));
/* store pointers to desc and cmds */
for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
@@ -202,7 +203,7 @@ static int ctch_open(struct intel_guc *guc,
}
/* vma should be already allocated and map'ed */
- base = guc_ggtt_offset(ctch->vma);
+ base = intel_guc_ggtt_offset(guc, ctch->vma);
/* (re)initialize descriptors
* cmds buffers are in the second half of the blob page
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 3b09329..178d339 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -165,7 +165,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
/* Set the source address for the new blob */
- offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
+ offset = intel_guc_ggtt_offset(guc, vma) + guc_fw->header_offset;
I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 2ffc966..4efe564 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -549,7 +549,7 @@ int intel_guc_log_create(struct intel_guc *guc)
(GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) |
(GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT);
- offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */
+ offset = intel_guc_ggtt_offset(guc, vma) >> PAGE_SHIFT; /* in pages */
guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
return 0;
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 1f3a878..c56e4f4 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -375,8 +375,8 @@ static void guc_stage_desc_init(struct intel_guc *guc,
lrc->context_desc = lower_32_bits(ce->lrc_desc);
/* The state page is after PPHWSP */
- lrc->ring_lrca =
- guc_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE;
+ lrc->ring_lrca = intel_guc_ggtt_offset(guc, ce->state) +
+ LRC_STATE_PN * PAGE_SIZE;
/* XXX: In direct submission, the GuC wants the HW context id
* here. In proxy submission, it wants the stage id
@@ -384,7 +384,7 @@ static void guc_stage_desc_init(struct intel_guc *guc,
lrc->context_id = (client->stage_id << GUC_ELC_CTXID_OFFSET) |
(guc_engine_id << GUC_ELC_ENGINE_OFFSET);
- lrc->ring_begin = guc_ggtt_offset(ce->ring->vma);
+ lrc->ring_begin = intel_guc_ggtt_offset(guc, ce->ring->vma);
lrc->ring_end = lrc->ring_begin + ce->ring->size - 1;
lrc->ring_next_free_location = lrc->ring_begin;
lrc->ring_current_tail_pointer_value = 0;
@@ -400,7 +400,7 @@ static void guc_stage_desc_init(struct intel_guc *guc,
* The doorbell, process descriptor, and workqueue are all parts
* of the client object, which the GuC will reference via the GGTT
*/
- gfx_addr = guc_ggtt_offset(client->vma);
+ gfx_addr = intel_guc_ggtt_offset(guc, client->vma);
desc->db_trigger_phy = sg_dma_address(client->vma->pages->sgl) +
client->doorbell_offset;
desc->db_trigger_cpu = ptr_to_u64(__get_doorbell(client));
@@ -596,7 +596,7 @@ static void inject_preempt_context(struct work_struct *work)
data[3] = engine->guc_id;
data[4] = guc->execbuf_client->priority;
data[5] = guc->execbuf_client->stage_id;
- data[6] = guc_ggtt_offset(guc->shared_data);
+ data[6] = intel_guc_ggtt_offset(guc, guc->shared_data);
if (WARN_ON(intel_guc_send(guc, data, ARRAY_SIZE(data)))) {
execlists_clear_active(&engine->execlists,
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 8ed0518..aed9c1c 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -137,7 +137,8 @@ static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
/* Set the source address for the uCode */
- offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
+ offset = intel_guc_ggtt_offset(&dev_priv->guc, vma) +
+ huc_fw->header_offset;
I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
@@ -213,7 +214,8 @@ int intel_huc_auth(struct intel_huc *huc)
}
ret = intel_guc_auth_huc(guc,
- guc_ggtt_offset(vma) + huc->fw.rsa_offset);
+ intel_guc_ggtt_offset(guc, vma) +
+ huc->fw.rsa_offset);
if (ret) {
DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
goto out;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v7 3/6] drm/i915/guc: Implement dynamic GuC WOPCM offset and size
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
2018-01-19 1:29 ` [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset Jackie Li
@ 2018-01-19 1:29 ` Jackie Li
2018-02-01 8:38 ` Sagar Arun Kamble
2018-01-19 1:29 ` [PATCH v7 4/6] drm/i915/guc: Add WOPCM partitioning support for CNL Jackie Li
` (5 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jackie Li @ 2018-01-19 1:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Sujaritha Sundaresan
Hardware may have specific restrictions on GuC WOPCM size
versus HuC firmware size. With static GuC WOPCM size,
there's no way to adjust the GuC WOPCM partition size based on
the actual HuC firmware size, so that GuC/HuC loading failure
would occur even if there was enough WOPCM space for both
GuC and HuC firmware.
This patch enables the dynamic calculation of the GuC WOPCM
aperture size used by GuC and HuC firmware. GuC WOPCM offset is
set to HuC size + reserved WOPCM size. GuC WOPCM size is set to
total WOPCM size - GuC WOPCM offset - RC6CTX size. In this case,
GuC WOPCM offset will be updated based on the size of HuC firmware
while GuC WOPCM size will be set to use all the remaining WOPCM space.
v2:
- Removed intel_wopcm_init (Ville/Sagar/Joonas)
- Renamed and Moved the intel_wopcm_partition into intel_guc (Sagar)
- Removed unnecessary function calls (Joonas)
- Init GuC WOPCM partition as soon as firmware fetching is completed
v3:
- Fixed indentation issues (Chris)
- Removed layering violation code (Chris/Michal)
- Created separat files for GuC wopcm code (Michal)
- Used inline function to avoid code duplication (Michal)
v4:
- Preset the GuC WOPCM top during early GuC init (Chris)
- Fail intel_uc_init_hw() as soon as GuC WOPCM partitioning failed
v5:
- Moved GuC DMA WOPCM register updating code into intel_guc_wopcm.c
- Took care of the locking status before writing to GuC DMA
Write-Once registers. (Joonas)
v6:
- Made sure the GuC WOPCM size to be multiple of 4K (4K aligned)
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: John Spotswood <john.a.spotswood@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 9 +--
drivers/gpu/drm/i915/intel_guc.c | 5 +-
drivers/gpu/drm/i915/intel_guc.h | 12 ++--
drivers/gpu/drm/i915/intel_guc_reg.h | 1 +
drivers/gpu/drm/i915/intel_guc_wopcm.c | 108 +++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/intel_guc_wopcm.h | 40 ++++++++++--
drivers/gpu/drm/i915/intel_huc.c | 2 +-
drivers/gpu/drm/i915/intel_uc.c | 11 +++-
drivers/gpu/drm/i915/intel_uc_fw.c | 11 +++-
drivers/gpu/drm/i915/intel_uc_fw.h | 16 +++++
10 files changed, 183 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 648e753..b485794 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -312,12 +312,13 @@ __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 above GUC_WOPCM_TOP. 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.
+ /*
+ * GuC requires the ring to be placed above GuC WOPCM top. 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 = GUC_WOPCM_TOP;
+ ctx->ggtt_offset_bias = dev_priv->guc.wopcm.top;
else
ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index e70885b..3521beb 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -64,6 +64,7 @@ void intel_guc_init_early(struct intel_guc *guc)
{
intel_guc_fw_init_early(guc);
intel_guc_ct_init_early(&guc->ct);
+ intel_guc_wopcm_init_early(&guc->wopcm);
mutex_init(&guc->send_mutex);
guc->send = intel_guc_send_nop;
@@ -473,7 +474,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
* This is a wrapper to create an object for use with the GuC. In order to
* use it inside the GuC, an object needs to be pinned lifetime, so we allocate
* both some backing storage and a range inside the Global GTT. We must pin
- * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
+ * it in the GGTT somewhere other than than [0, GuC WOPCM top) because that
* range is reserved inside GuC.
*
* Return: A i915_vma if successful, otherwise an ERR_PTR.
@@ -494,7 +495,7 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
goto err;
ret = i915_vma_pin(vma, 0, PAGE_SIZE,
- PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+ PIN_GLOBAL | PIN_OFFSET_BIAS | guc->wopcm.top);
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 b7e2a18..ea35911 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -49,6 +49,7 @@ struct intel_guc {
struct intel_uc_fw fw;
struct intel_guc_log log;
struct intel_guc_ct ct;
+ struct intel_guc_wopcm wopcm;
/* Log snapshot if GuC errors during load */
struct drm_i915_gem_object *load_err_log;
@@ -105,10 +106,10 @@ static inline void intel_guc_notify(struct intel_guc *guc)
* @guc: intel guc.
* @vma: i915 graphics virtual memory area.
*
- * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
- * which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
- * 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
- * used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
+ * GuC does not allow any gfx GGTT address that falls into range
+ * [0, GuC WOPCM top), which is reserved for Boot ROM, SRAM and WOPCM.
+ * All gfx objects used by GuC is pinned with PIN_OFFSET_BIAS along with
+ * top of WOPCM.
*
* Return: GGTT offset that meets the GuC gfx address requirement.
*/
@@ -117,7 +118,8 @@ static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
{
u32 offset = i915_ggtt_offset(vma);
- GEM_BUG_ON(offset < GUC_WOPCM_TOP);
+ GEM_BUG_ON(!guc->wopcm.valid);
+ GEM_BUG_ON(offset < guc->wopcm.top);
GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
return offset;
diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index 1f52fb8..9109be7 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -75,6 +75,7 @@
/* Defines WOPCM space available to GuC firmware */
#define GUC_WOPCM_SIZE _MMIO(0xc050)
+#define GUC_WOPCM_SIZE_MASK (0xfffff000)
/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
#define GUC_GGTT_TOP 0xFEE00000
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
index 87643a0..0532714 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
@@ -25,23 +25,111 @@
#include "intel_guc_wopcm.h"
#include "i915_drv.h"
+static inline u32 guc_reserved_wopcm_size(struct intel_guc *guc)
+{
+ struct drm_i915_private *i915 = guc_to_i915(guc);
+
+ /* On BXT, the top of WOPCM is reserved for RC6 context */
+ if (IS_GEN9_LP(i915))
+ return BXT_WOPCM_RC6_RESERVED;
+
+ return 0;
+}
+
+static inline int gen9_wocpm_size_check(struct drm_i915_private *i915)
+{
+ struct intel_guc_wopcm *wopcm = &i915->guc.wopcm;
+ u32 wopcm_base;
+ u32 delta;
+
+ /*
+ * Check hardware restriction on Gen9
+ * GuC WOPCM size is at least 4 bytes larger than GuC WOPCM base due
+ * to hardware limitation on Gen9.
+ */
+ wopcm_base = wopcm->offset + GEN9_GUC_WOPCM_OFFSET;
+ if (unlikely(wopcm_base > wopcm->size))
+ return -E2BIG;
+
+ delta = wopcm->size - wopcm_base;
+ if (unlikely(delta < GEN9_GUC_WOPCM_DELTA))
+ return -E2BIG;
+
+ return 0;
+}
+
+static inline int guc_wopcm_size_check(struct intel_guc *guc)
+{
+ struct drm_i915_private *i915 = guc_to_i915(guc);
+
+ if (IS_GEN9(i915))
+ return gen9_wocpm_size_check(i915);
+
+ return 0;
+}
+
/*
- * intel_guc_wopcm_size() - Get the size of GuC WOPCM.
+ * intel_guc_wopcm_init() - Initialize the GuC WOPCM partition.
* @guc: intel guc.
+ * @guc_fw_size: size of GuC firmware.
+ * @huc_fw_size: size of HuC firmware.
*
- * Get the platform specific GuC WOPCM size.
+ * This function tries to initialize the WOPCM partition based on HuC firmware
+ * size and the reserved WOPCM memory size.
*
- * Return: size of the GuC WOPCM.
+ * Return: 0 on success, non-zero error code on failure.
*/
-u32 intel_guc_wopcm_size(struct intel_guc *guc)
+int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
+ u32 huc_fw_size)
{
- struct drm_i915_private *i915 = guc_to_i915(guc);
+ u32 reserved = guc_reserved_wopcm_size(guc);
+ u32 offset, size, top;
+ int err;
- u32 wopcm_size = GUC_WOPCM_TOP;
+ if (guc->wopcm.valid)
+ return 0;
- /* On BXT, the top of WOPCM is reserved for RC6 context */
- if (IS_GEN9_LP(i915))
- wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+ if (!guc_fw_size)
+ return -EINVAL;
+
+ if (reserved >= WOPCM_DEFAULT_SIZE)
+ return -E2BIG;
+
+ offset = huc_fw_size + WOPCM_RESERVED_SIZE;
+ if (offset >= WOPCM_DEFAULT_SIZE)
+ return -E2BIG;
+
+ /* Hardware requires GuC WOPCM offset needs to be 16K aligned. */
+ offset = ALIGN(offset, WOPCM_OFFSET_ALIGNMENT);
+ if ((offset + reserved) >= WOPCM_DEFAULT_SIZE)
+ return -E2BIG;
+
+ top = WOPCM_DEFAULT_SIZE - offset;
+ size = top - reserved;
+
+ /* GuC WOPCM size must be 4K aligned. */
+ size &= GUC_WOPCM_SIZE_MASK;
+
+ /*
+ * GuC size needs to be less than or equal to GuC WOPCM size.
+ * Need extra 8K stack for GuC.
+ */
+ if ((guc_fw_size + GUC_WOPCM_STACK_RESERVED) > size)
+ return -E2BIG;
+
+ guc->wopcm.offset = offset;
+ guc->wopcm.size = size;
+ guc->wopcm.top = top;
+
+ /* Check platform specific restrictions */
+ err = guc_wopcm_size_check(guc);
+ if (err)
+ return err;
+
+ guc->wopcm.valid = true;
+
+ DRM_DEBUG_DRIVER("GuC WOPCM offset %dKB, size %dKB, top %dKB\n",
+ offset >> 10, size >> 10, top >> 10);
- return wopcm_size;
+ return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
index 595fb1c..352cf3d 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.h
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
@@ -29,11 +29,41 @@
struct intel_guc;
-#define GUC_WOPCM_OFFSET_VALUE 0x80000 /* 512KB */
-/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
-#define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */
-#define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */
+/* Default WOPCM size 1MB */
+#define WOPCM_DEFAULT_SIZE (0x1 << 20)
+/* Reserved WOPCM size 16KB */
+#define WOPCM_RESERVED_SIZE (0x4000)
+/* GUC WOPCM Offset need to be 16KB aligned */
+#define WOPCM_OFFSET_ALIGNMENT (0x4000)
+/* 8KB stack reserved for GuC FW*/
+#define GUC_WOPCM_STACK_RESERVED (0x2000)
+/* 24KB WOPCM reserved for RC6 CTX on BXT */
+#define BXT_WOPCM_RC6_RESERVED (0x6000)
-u32 intel_guc_wopcm_size(struct intel_guc *guc);
+#define GEN9_GUC_WOPCM_DELTA 4
+#define GEN9_GUC_WOPCM_OFFSET (0x24000)
+
+struct intel_guc_wopcm {
+ u32 offset;
+ u32 size;
+ u32 top;
+ bool valid;
+};
+
+/*
+ * intel_guc_wopcm_init_early() - Early initialization of the GuC WOPCM.
+ * @wopcm: GuC WOPCM.
+ *
+ * Setup the GuC WOPCM top to the top of the overall WOPCM. This will guarantee
+ * that the allocation of the GuC accessible objects won't fall into WOPCM when
+ * GuC partition isn't present.
+ *
+ */
+static inline void intel_guc_wopcm_init_early(struct intel_guc_wopcm *wopcm)
+{
+ wopcm->top = WOPCM_DEFAULT_SIZE;
+}
+
+int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_size, u32 huc_size);
#endif
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index aed9c1c..dc6a6c6 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -206,7 +206,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_WOPCM_TOP);
+ PIN_OFFSET_BIAS | guc->wopcm.top);
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.c b/drivers/gpu/drm/i915/intel_uc.c
index e9d0568..b5a08d1 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -263,6 +263,9 @@ void intel_uc_fini_wq(struct drm_i915_private *dev_priv)
int intel_uc_init(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
+ struct intel_huc *huc = &dev_priv->huc;
+ u32 guc_fw_size = intel_uc_fw_get_size(&guc->fw);
+ u32 huc_fw_size = intel_uc_fw_get_size(&huc->fw);
int ret;
if (!USES_GUC(dev_priv))
@@ -271,6 +274,10 @@ int intel_uc_init(struct drm_i915_private *dev_priv)
if (!HAS_GUC(dev_priv))
return -ENODEV;
+ ret = intel_guc_wopcm_init(guc, guc_fw_size, huc_fw_size);
+ if (ret)
+ return ret;
+
ret = intel_guc_init(guc);
if (ret)
return ret;
@@ -320,9 +327,9 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
gen9_reset_guc_interrupts(dev_priv);
/* init WOPCM */
- I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));
+ I915_WRITE(GUC_WOPCM_SIZE, guc->wopcm.size);
I915_WRITE(DMA_GUC_WOPCM_OFFSET,
- GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
+ guc->wopcm.offset | HUC_LOADING_AGENT_GUC);
/* WaEnableuKernelHeaderValidFix:skl */
/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 24945cf..791263a 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -95,9 +95,13 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
- /* Header and uCode will be loaded to WOPCM */
+ /*
+ * Header and uCode will be loaded to WOPCM
+ * Only check the size against the overall available WOPCM here. Will
+ * continue to check the size during WOPCM partition calculation.
+ */
size = uc_fw->header_size + uc_fw->ucode_size;
- if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
+ if (size > WOPCM_DEFAULT_SIZE) {
DRM_WARN("%s: Firmware is too large to fit in WOPCM\n",
intel_uc_fw_type_repr(uc_fw->type));
err = -E2BIG;
@@ -207,6 +211,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
int (*xfer)(struct intel_uc_fw *uc_fw,
struct i915_vma *vma))
{
+ struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
struct i915_vma *vma;
int err;
@@ -230,7 +235,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
}
vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
- PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+ PIN_OFFSET_BIAS | i915->guc.wopcm.top);
if (IS_ERR(vma)) {
err = PTR_ERR(vma);
DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index d5fd460..d00d888 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -115,6 +115,22 @@ static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw)
return uc_fw->path != NULL;
}
+/*
+ * intel_uc_fw_get_size() - Get the size of the firmware.
+ * @uc_fw: intel_uc_fw structure.
+ *
+ * Get the size of the firmware that will be placed in WOPCM.
+ *
+ * Return: Zero on invalid firmware status. actual size on success.
+ */
+static inline u32 intel_uc_fw_get_size(struct intel_uc_fw *uc_fw)
+{
+ if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+ return 0;
+
+ return uc_fw->header_size + uc_fw->ucode_size;
+}
+
void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct intel_uc_fw *uc_fw);
int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v7 4/6] drm/i915/guc: Add WOPCM partitioning support for CNL
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
2018-01-19 1:29 ` [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset Jackie Li
2018-01-19 1:29 ` [PATCH v7 3/6] drm/i915/guc: Implement dynamic GuC WOPCM offset and size Jackie Li
@ 2018-01-19 1:29 ` Jackie Li
2018-02-01 8:44 ` Sagar Arun Kamble
2018-01-19 1:29 ` [PATCH v7 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers Jackie Li
` (4 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jackie Li @ 2018-01-19 1:29 UTC (permalink / raw)
To: intel-gfx
CNL has different WOPCM size and hardware restriction on GuC
WOPCM size.
This patch returns the correct WOPCM reserved size on CNL and
adds the GuC WOPCM size check for CNL.
v6:
- Extended HuC FW size check against GuC WOPCM size to all
Gen9 and CNL A0 platforms
v7:
- Fixed patch format issues
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: John Spotswood <john.a.spotswood@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
drivers/gpu/drm/i915/intel_guc_wopcm.c | 23 ++++++++++++++++++++++-
drivers/gpu/drm/i915/intel_guc_wopcm.h | 4 ++++
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
index 0532714..ed3096c 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
@@ -33,6 +33,24 @@ static inline u32 guc_reserved_wopcm_size(struct intel_guc *guc)
if (IS_GEN9_LP(i915))
return BXT_WOPCM_RC6_RESERVED;
+ if (IS_GEN10(i915))
+ return CNL_WOPCM_RESERVED;
+
+ return 0;
+}
+
+/*
+ * On Gen9 & CNL A0, hardware requires GuC size to be larger than or equal to
+ * HuC kernel size.
+ */
+static inline int wopcm_huc_size_check(struct drm_i915_private *i915)
+{
+ struct intel_guc_wopcm *wopcm = &i915->guc.wopcm;
+ u32 huc_size = intel_uc_fw_get_size(&i915->huc.fw);
+
+ if (unlikely(wopcm->size - GUC_WOPCM_RESERVED < huc_size))
+ return -E2BIG;
+
return 0;
}
@@ -55,7 +73,7 @@ static inline int gen9_wocpm_size_check(struct drm_i915_private *i915)
if (unlikely(delta < GEN9_GUC_WOPCM_DELTA))
return -E2BIG;
- return 0;
+ return wopcm_huc_size_check(i915);
}
static inline int guc_wopcm_size_check(struct intel_guc *guc)
@@ -65,6 +83,9 @@ static inline int guc_wopcm_size_check(struct intel_guc *guc)
if (IS_GEN9(i915))
return gen9_wocpm_size_check(i915);
+ if (IS_CNL_REVID(i915, CNL_REVID_A0, CNL_REVID_A0))
+ return wopcm_huc_size_check(i915);
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
index 352cf3d..5306175 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.h
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
@@ -37,8 +37,12 @@ struct intel_guc;
#define WOPCM_OFFSET_ALIGNMENT (0x4000)
/* 8KB stack reserved for GuC FW*/
#define GUC_WOPCM_STACK_RESERVED (0x2000)
+/* 16KB reserved at the beginning of GuC WOPCM */
+#define GUC_WOPCM_RESERVED (0x4000)
/* 24KB WOPCM reserved for RC6 CTX on BXT */
#define BXT_WOPCM_RC6_RESERVED (0x6000)
+/* 36KB WOPCM reserved on CNL */
+#define CNL_WOPCM_RESERVED (0x9000)
#define GEN9_GUC_WOPCM_DELTA 4
#define GEN9_GUC_WOPCM_OFFSET (0x24000)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v7 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
` (2 preceding siblings ...)
2018-01-19 1:29 ` [PATCH v7 4/6] drm/i915/guc: Add WOPCM partitioning support for CNL Jackie Li
@ 2018-01-19 1:29 ` Jackie Li
2018-02-01 7:41 ` Chris Wilson
2018-01-19 1:29 ` [PATCH v7 6/6] HAX Enable GuC Submission for CI Jackie Li
` (3 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jackie Li @ 2018-01-19 1:29 UTC (permalink / raw)
To: intel-gfx
GuC WOPCM registers are write-once registers. Current driver code
accesses these registers without checking the accessibility to these
registers, this will lead unpredictable driver behaviors if these
registers were touch by other components (such as faulty BIOS code).
This patch moves the GuC WOPCM register updating operations into
intel_guc_wopcm.c and adds checks before and after the write to GuC
WOPCM registers to make sure the driver is in a known state before
and after writing to these write-once registers.
v6:
- Made sure module reloading won't bug the kernel while doing
locking status checking
v7:
- Fixed patch format issues
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
drivers/gpu/drm/i915/intel_guc.h | 2 +-
drivers/gpu/drm/i915/intel_guc_reg.h | 4 +-
drivers/gpu/drm/i915/intel_guc_wopcm.c | 91 ++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_guc_wopcm.h | 14 +++++-
drivers/gpu/drm/i915/intel_uc.c | 5 +-
5 files changed, 106 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index ea35911..7ed0c17 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -118,7 +118,7 @@ static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
{
u32 offset = i915_ggtt_offset(vma);
- GEM_BUG_ON(!guc->wopcm.valid);
+ GEM_BUG_ON(!(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID));
GEM_BUG_ON(offset < guc->wopcm.top);
GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index 9109be7..ee2b33ba 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -66,6 +66,7 @@
#define UOS_MOVE (1<<4)
#define START_DMA (1<<0)
#define DMA_GUC_WOPCM_OFFSET _MMIO(0xc340)
+#define DMA_GUC_WOPCM_OFFSET_MASK (0xffffc000)
#define HUC_LOADING_AGENT_VCR (0<<1)
#define HUC_LOADING_AGENT_GUC (1<<1)
#define GUC_MAX_IDLE_COUNT _MMIO(0xC3E4)
@@ -75,7 +76,8 @@
/* Defines WOPCM space available to GuC firmware */
#define GUC_WOPCM_SIZE _MMIO(0xc050)
-#define GUC_WOPCM_SIZE_MASK (0xfffff000)
+#define GUC_WOPCM_SIZE_MASK (0xfffff000)
+#define GUC_WOPCM_REG_LOCKED BIT(0)
/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
#define GUC_GGTT_TOP 0xFEE00000
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
index ed3096c..236fc32 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
@@ -89,6 +89,53 @@ static inline int guc_wopcm_size_check(struct intel_guc *guc)
return 0;
}
+static inline bool __reg_locked(struct drm_i915_private *dev_priv,
+ i915_reg_t reg)
+{
+ return !!(I915_READ(reg) & GUC_WOPCM_REG_LOCKED);
+}
+
+static inline bool guc_wopcm_locked(struct intel_guc *guc)
+{
+ struct drm_i915_private *i915 = guc_to_i915(guc);
+ bool size_reg_locked = __reg_locked(i915, GUC_WOPCM_SIZE);
+ bool offset_reg_locked = __reg_locked(i915, DMA_GUC_WOPCM_OFFSET);
+
+ return size_reg_locked && offset_reg_locked;
+}
+
+static inline void guc_wopcm_hw_update(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+ /* GuC WOPCM registers should be unlocked at this point. */
+ GEM_BUG_ON(__reg_locked(dev_priv, GUC_WOPCM_SIZE));
+ GEM_BUG_ON(__reg_locked(dev_priv, DMA_GUC_WOPCM_OFFSET));
+
+ I915_WRITE(GUC_WOPCM_SIZE, guc->wopcm.size);
+ I915_WRITE(DMA_GUC_WOPCM_OFFSET,
+ guc->wopcm.offset | HUC_LOADING_AGENT_GUC);
+
+ GEM_BUG_ON(!__reg_locked(dev_priv, GUC_WOPCM_SIZE));
+ GEM_BUG_ON(!__reg_locked(dev_priv, DMA_GUC_WOPCM_OFFSET));
+}
+
+static inline bool guc_wopcm_regs_valid(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ u32 size, offset;
+ bool guc_loads_huc;
+
+ size = I915_READ(GUC_WOPCM_SIZE) & GUC_WOPCM_SIZE_MASK;
+ offset = I915_READ(DMA_GUC_WOPCM_OFFSET);
+
+ guc_loads_huc = !!(offset & HUC_LOADING_AGENT_GUC);
+ offset &= DMA_GUC_WOPCM_OFFSET_MASK;
+
+ return guc_loads_huc && (size == guc->wopcm.size) &&
+ (offset == guc->wopcm.offset);
+}
+
/*
* intel_guc_wopcm_init() - Initialize the GuC WOPCM partition.
* @guc: intel guc.
@@ -107,8 +154,7 @@ int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
u32 offset, size, top;
int err;
- if (guc->wopcm.valid)
- return 0;
+ GEM_BUG_ON(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID);
if (!guc_fw_size)
return -EINVAL;
@@ -147,10 +193,49 @@ int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
if (err)
return err;
- guc->wopcm.valid = true;
+ guc->wopcm.flags |= INTEL_GUC_WOPCM_VALID;
DRM_DEBUG_DRIVER("GuC WOPCM offset %dKB, size %dKB, top %dKB\n",
offset >> 10, size >> 10, top >> 10);
return 0;
}
+
+/*
+ * intel_guc_wopcm_init_hw() - Setup GuC WOPCM registers.
+ * @guc: intel guc.
+ *
+ * Setup the GuC WOPCM size and offset registers with the stored values. It will
+ * also check the registers locking status to determine whether these registers
+ * are unlocked and can be updated.
+ */
+void intel_guc_wopcm_init_hw(struct intel_guc *guc)
+{
+ u32 locked = guc_wopcm_locked(guc);
+
+ GEM_BUG_ON(!(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID));
+
+ /*
+ * Bug if driver hasn't updated the HW Registers and GuC WOPCM has been
+ * locked. Return directly if WOPCM was locked and we have updated
+ * the registers.
+ */
+ if (locked) {
+ /*
+ * Mark as updated if registers contained correct values.
+ * This will happen while reloading the driver module without
+ * rebooting the system.
+ */
+ if (guc_wopcm_regs_valid(guc))
+ goto out;
+
+ GEM_BUG_ON(!(guc->wopcm.flags & INTEL_GUC_WOPCM_HW_UPDATED));
+ return;
+ }
+
+ /* Always update registers when GuC WOPCM is not locked. */
+ guc_wopcm_hw_update(guc);
+
+out:
+ guc->wopcm.flags |= INTEL_GUC_WOPCM_HW_UPDATED;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
index 5306175..f075e37 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.h
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
@@ -47,11 +47,22 @@ struct intel_guc;
#define GEN9_GUC_WOPCM_DELTA 4
#define GEN9_GUC_WOPCM_OFFSET (0x24000)
+/* GuC WOPCM flags*/
+#define INTEL_GUC_WOPCM_VALID BIT(0)
+#define INTEL_GUC_WOPCM_HW_UPDATED BIT(1)
+
+/*
+ * intel_guc_wopcm - GuC WOPCM related settings.
+ * @offset: GuC WOPCM offset.
+ * @size: size of GuC WOPCM for GuC firmware.
+ * @top: start of Non GuC WOPCM memory.
+ * @flags: bitmap of INTEL_GUC_WOPCM_<foo>.
+ */
struct intel_guc_wopcm {
u32 offset;
u32 size;
u32 top;
- bool valid;
+ u32 flags;
};
/*
@@ -69,5 +80,6 @@ static inline void intel_guc_wopcm_init_early(struct intel_guc_wopcm *wopcm)
}
int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_size, u32 huc_size);
+void intel_guc_wopcm_init_hw(struct intel_guc *guc);
#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index b5a08d1..0ee49b5 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -326,10 +326,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
guc_disable_communication(guc);
gen9_reset_guc_interrupts(dev_priv);
- /* init WOPCM */
- I915_WRITE(GUC_WOPCM_SIZE, guc->wopcm.size);
- I915_WRITE(DMA_GUC_WOPCM_OFFSET,
- guc->wopcm.offset | HUC_LOADING_AGENT_GUC);
+ intel_guc_wopcm_init_hw(guc);
/* WaEnableuKernelHeaderValidFix:skl */
/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v7 6/6] HAX Enable GuC Submission for CI
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
` (3 preceding siblings ...)
2018-01-19 1:29 ` [PATCH v7 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers Jackie Li
@ 2018-01-19 1:29 ` Jackie Li
2018-02-01 8:55 ` Sagar Arun Kamble
2018-01-19 7:03 ` ✗ Fi.CI.BAT: failure for series starting with [v7,1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Patchwork
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jackie Li @ 2018-01-19 1:29 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
drivers/gpu/drm/i915/i915_params.c | 2 +-
drivers/gpu/drm/i915/i915_params.h | 2 +-
drivers/gpu/drm/i915/intel_guc_wopcm.c | 6 ++++++
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 26dee5e..fed472a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3561,7 +3561,7 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
void i915_ggtt_enable_guc(struct drm_i915_private *i915)
{
- GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
+// GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
i915->ggtt.invalidate = guc_ggtt_invalidate;
@@ -3571,7 +3571,7 @@ void i915_ggtt_enable_guc(struct drm_i915_private *i915)
void i915_ggtt_disable_guc(struct drm_i915_private *i915)
{
/* We should only be called after i915_ggtt_enable_guc() */
- GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
+// GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
i915->ggtt.invalidate = gen6_ggtt_invalidate;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 0b553a8..b33d364 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -152,7 +152,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400,
i915_param_named_unsafe(enable_guc, int, 0400,
"Enable GuC load for GuC submission and/or HuC load. "
"Required functionality can be selected using bitmask values. "
- "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)");
+ "(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)");
i915_param_named(guc_log_level, int, 0400,
"GuC firmware logging level. Requires GuC to be loaded. "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c963603..53037b5 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -47,7 +47,7 @@ struct drm_printer;
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
- param(int, enable_guc, 0) \
+ param(int, enable_guc, -1) \
param(int, guc_log_level, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
index 236fc32..e972b7b 100644
--- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
@@ -154,6 +154,9 @@ int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
u32 offset, size, top;
int err;
+ DRM_DEBUG_DRIVER("guc_fw size %u, huc_fw_size %u\n", guc_fw_size,
+ huc_fw_size);
+
GEM_BUG_ON(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID);
if (!guc_fw_size)
@@ -213,6 +216,9 @@ void intel_guc_wopcm_init_hw(struct intel_guc *guc)
{
u32 locked = guc_wopcm_locked(guc);
+ DRM_DEBUG_DRIVER("locked = %s, flags = %#x\n", yesno(locked),
+ guc->wopcm.flags);
+
GEM_BUG_ON(!(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID));
/*
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [v7,1/6] drm/i915/guc: Move GuC WOPCM related code into separate files
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
` (4 preceding siblings ...)
2018-01-19 1:29 ` [PATCH v7 6/6] HAX Enable GuC Submission for CI Jackie Li
@ 2018-01-19 7:03 ` Patchwork
2018-02-01 7:04 ` [PATCH v7 1/6] " Sagar Arun Kamble
2018-02-01 7:37 ` Chris Wilson
7 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-01-19 7:03 UTC (permalink / raw)
To: Jackie Li; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v7,1/6] drm/i915/guc: Move GuC WOPCM related code into separate files
URL : https://patchwork.freedesktop.org/series/36748/
State : failure
== Summary ==
Series 36748v1 series starting with [v7,1/6] drm/i915/guc: Move GuC WOPCM related code into separate files
https://patchwork.freedesktop.org/api/1.0/series/36748/revisions/1/mbox/
Test core_auth:
Subgroup basic-auth:
pass -> SKIP (fi-skl-gvtdvm)
Test core_prop_blob:
Subgroup basic:
pass -> SKIP (fi-skl-gvtdvm)
Test debugfs_test:
Subgroup read_all_entries:
dmesg-fail -> DMESG-WARN (fi-elk-e7500) fdo#103989
incomplete -> PASS (fi-snb-2520m) fdo#103713
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k2)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-dsi)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-kbl-7560u)
pass -> DMESG-WARN (fi-kbl-7567u)
pass -> DMESG-WARN (fi-kbl-r)
Test drv_getparams_basic:
Subgroup basic-eu-total:
pass -> SKIP (fi-skl-gvtdvm)
Subgroup basic-subslice-total:
pass -> SKIP (fi-skl-gvtdvm)
Test drv_hangman:
Subgroup error-state-basic:
pass -> SKIP (fi-skl-gvtdvm)
Test gem_basic:
Subgroup bad-close:
pass -> SKIP (fi-skl-gvtdvm)
Subgroup create-close:
pass -> SKIP (fi-skl-gvtdvm)
Subgroup create-fd-close:
pass -> SKIP (fi-skl-gvtdvm)
Test gem_busy:
Subgroup basic-busy-default:
pass -> SKIP (fi-skl-gvtdvm)
Subgroup basic-hang-default:
pass -> SKIP (fi-skl-gvtdvm) fdo#104108 +6
Test gem_close_race:
Subgroup basic-process:
pass -> SKIP (fi-skl-gvtdvm)
Subgroup basic-threads:
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k2)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-dsi)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-kbl-7560u)
pass -> DMESG-WARN (fi-kbl-7567u)
pass -> DMESG-WARN (fi-kbl-r)
Test gem_cpu_reloc:
Subgroup basic:
pass -> SKIP (fi-skl-gvtdvm)
Test gem_cs_tlb:
Subgroup basic-default:
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-dsi)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7567u)
Test gem_ctx_create:
Subgroup basic:
pass -> SKIP (fi-skl-gvtdvm)
Subgroup basic-files:
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k2)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-dsi)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-kbl-7560u)
pass -> DMESG-WARN (fi-kbl-7567u)
pass -> DMESG-WARN (fi-kbl-r)
Test gem_ctx_exec:
Subgroup basic:
pass -> SKIP (fi-skl-gvtdvm)
Test gem_ctx_param:
Subgroup basic:
pass -> SKIP (fi-skl-gvtdvm)
Subgroup basic-default:
pass -> SKIP (fi-skl-gvtdvm)
Test gem_ctx_switch:
Subgroup basic-default:
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
WARNING: Long output truncated
d68aaa919e474b82f41514e5c3b87cb6dfe86f57 drm-tip: 2018y-01m-19d-05h-41m-44s UTC integration manifest
26a52553d057 HAX Enable GuC Submission for CI
64b9d77507d4 drm/i915/guc: Check the locking status of GuC WOPCM registers
ebf1ea269e48 drm/i915/guc: Add WOPCM partitioning support for CNL
1a82d2052596 drm/i915/guc: Implement dynamic GuC WOPCM offset and size
086cad3683c5 drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset
d895a809d357 drm/i915/guc: Move GuC WOPCM related code into separate files
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7716/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
` (5 preceding siblings ...)
2018-01-19 7:03 ` ✗ Fi.CI.BAT: failure for series starting with [v7,1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Patchwork
@ 2018-02-01 7:04 ` Sagar Arun Kamble
2018-02-01 7:37 ` Chris Wilson
7 siblings, 0 replies; 23+ messages in thread
From: Sagar Arun Kamble @ 2018-02-01 7:04 UTC (permalink / raw)
To: Jackie Li, intel-gfx
Patch looks good to me. Minor updates suggested below with r-b.
On 1/19/2018 6:59 AM, Jackie Li wrote:
> intel_guc_reg.h should only include definition for GuC registers
> and related register bits. GuC WOPCM related values should not
> be defined in intel_guc_reg.h
>
> This patch creates a better file structure by moving GuC WOPCM
> related definitions int to a new header intel_guc_wopcm.h
typo "into"
> and moving GuC WOPCM related functions to a new source file
> intel_guc_wopcm.c
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Jackie Li <yaodong.li@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/intel_guc.c | 11 --------
> drivers/gpu/drm/i915/intel_guc.h | 2 +-
> drivers/gpu/drm/i915/intel_guc_reg.h | 4 ---
> drivers/gpu/drm/i915/intel_guc_wopcm.c | 47 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_guc_wopcm.h | 39 ++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_uc.c | 2 +-
> drivers/gpu/drm/i915/intel_uc_fw.c | 2 +-
> 8 files changed, 90 insertions(+), 18 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_guc_wopcm.c
> create mode 100644 drivers/gpu/drm/i915/intel_guc_wopcm.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 3bddd8a..1dc9988 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -88,6 +88,7 @@ i915-y += intel_uc.o \
> intel_guc_fw.o \
> intel_guc_log.o \
> intel_guc_submission.o \
> + intel_guc_wopcm.o \
> intel_huc.o
>
> # autogenerated null render state
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index ea30e7c..b8b6d4a 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -504,14 +504,3 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
> i915_gem_object_put(obj);
> return vma;
> }
> -
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
> -{
> - u32 wopcm_size = GUC_WOPCM_TOP;
> -
> - /* On BXT, the top of WOPCM is reserved for RC6 context */
> - if (IS_GEN9_LP(dev_priv))
> - wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
> -
> - return wopcm_size;
> -}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 52856a9..9e0a97e 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -31,6 +31,7 @@
> #include "intel_guc_ct.h"
> #include "intel_guc_log.h"
> #include "intel_guc_reg.h"
> +#include "intel_guc_wopcm.h"
> #include "intel_uc_fw.h"
> #include "i915_vma.h"
>
> @@ -130,6 +131,5 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
> int intel_guc_suspend(struct drm_i915_private *dev_priv);
> int intel_guc_resume(struct drm_i915_private *dev_priv);
> struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
> index 19a9247..1f52fb8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -68,7 +68,6 @@
> #define DMA_GUC_WOPCM_OFFSET _MMIO(0xc340)
> #define HUC_LOADING_AGENT_VCR (0<<1)
> #define HUC_LOADING_AGENT_GUC (1<<1)
> -#define GUC_WOPCM_OFFSET_VALUE 0x80000 /* 512KB */
> #define GUC_MAX_IDLE_COUNT _MMIO(0xC3E4)
>
> #define HUC_STATUS2 _MMIO(0xD3B0)
> @@ -76,9 +75,6 @@
>
> /* Defines WOPCM space available to GuC firmware */
> #define GUC_WOPCM_SIZE _MMIO(0xc050)
> -/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
> -#define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */
> -#define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */
>
> /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
> #define GUC_GGTT_TOP 0xFEE00000
> diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> new file mode 100644
> index 0000000..87643a0
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "intel_guc_wopcm.h"
> +#include "i915_drv.h"
> +
> +/*
> + * intel_guc_wopcm_size() - Get the size of GuC WOPCM.
> + * @guc: intel guc.
> + *
> + * Get the platform specific GuC WOPCM size.
> + *
> + * Return: size of the GuC WOPCM.
> + */
> +u32 intel_guc_wopcm_size(struct intel_guc *guc)
> +{
> + struct drm_i915_private *i915 = guc_to_i915(guc);
> +
This line spacing is not needed.
> + u32 wopcm_size = GUC_WOPCM_TOP;
change the variable name to guc_wopcm_size or just size.
> +
> + /* On BXT, the top of WOPCM is reserved for RC6 context */
> + if (IS_GEN9_LP(i915))
> + wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
> +
> + return wopcm_size;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
> new file mode 100644
> index 0000000..595fb1c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef _INTEL_GUC_WOPCM_H_
> +#define _INTEL_GUC_WOPCM_H_
> +
> +#include <linux/types.h>
> +
> +struct intel_guc;
> +
> +#define GUC_WOPCM_OFFSET_VALUE 0x80000 /* 512KB */
> +/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
> +#define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */
> +#define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */
> +
> +u32 intel_guc_wopcm_size(struct intel_guc *guc);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index f78a17b..e9d0568 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -320,7 +320,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> gen9_reset_guc_interrupts(dev_priv);
>
> /* init WOPCM */
> - I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
> + I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));
> I915_WRITE(DMA_GUC_WOPCM_OFFSET,
> GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
>
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
> index 784eff9..24945cf 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -97,7 +97,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>
> /* Header and uCode will be loaded to WOPCM */
> size = uc_fw->header_size + uc_fw->ucode_size;
> - if (size > intel_guc_wopcm_size(dev_priv)) {
> + if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
> DRM_WARN("%s: Firmware is too large to fit in WOPCM\n",
> intel_uc_fw_type_repr(uc_fw->type));
> err = -E2BIG;
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset
2018-01-19 1:29 ` [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset Jackie Li
@ 2018-02-01 7:27 ` Sagar Arun Kamble
2018-02-01 19:29 ` Yaodong Li
2018-02-01 7:38 ` Chris Wilson
1 sibling, 1 reply; 23+ messages in thread
From: Sagar Arun Kamble @ 2018-02-01 7:27 UTC (permalink / raw)
To: Jackie Li, intel-gfx
On 1/19/2018 6:59 AM, Jackie Li wrote:
> GuC related exported functions should start with "intel_guc_"
> prefix and pass intel_guc as the first parameter since its guc
> related. Current guc_ggtt_offset() failed to follow this code
> convention.
>
> This patch renames the guc_ggtt_offset to intel_guc_ggtt_offset
> and updates the related code to pass intel_guc pointer to
> this function call. so that we have a unified coding style for
> GuC code.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Jackie Li <yaodong.li@intel.com>
> ---
> drivers/gpu/drm/i915/intel_guc.c | 12 +++++++-----
> drivers/gpu/drm/i915/intel_guc.h | 10 ++++++++--
> drivers/gpu/drm/i915/intel_guc_ads.c | 25 +++++++++++++------------
> drivers/gpu/drm/i915/intel_guc_ct.c | 5 +++--
> drivers/gpu/drm/i915/intel_guc_fw.c | 2 +-
> drivers/gpu/drm/i915/intel_guc_log.c | 2 +-
> drivers/gpu/drm/i915/intel_guc_submission.c | 10 +++++-----
> drivers/gpu/drm/i915/intel_huc.c | 6 ++++--
> 8 files changed, 42 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index b8b6d4a..e70885b 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -264,8 +264,10 @@ void intel_guc_init_params(struct intel_guc *guc)
>
> /* If GuC submission is enabled, set up additional parameters here */
> if (USES_GUC_SUBMISSION(dev_priv)) {
> - u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
> - u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
> + u32 ads = intel_guc_ggtt_offset(guc,
> + guc->ads_vma) >> PAGE_SHIFT;
> + u32 pgs = intel_guc_ggtt_offset(guc,
> + dev_priv->guc.stage_desc_pool);
> u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
>
> params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
> @@ -413,7 +415,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
> data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
> /* any value greater than GUC_POWER_D0 */
> data[1] = GUC_POWER_D1;
> - data[2] = guc_ggtt_offset(guc->shared_data);
> + data[2] = intel_guc_ggtt_offset(guc, guc->shared_data);
>
> return intel_guc_send(guc, data, ARRAY_SIZE(data));
> }
> @@ -436,7 +438,7 @@ int intel_guc_reset_engine(struct intel_guc *guc,
> data[3] = 0;
> data[4] = 0;
> data[5] = guc->execbuf_client->stage_id;
> - data[6] = guc_ggtt_offset(guc->shared_data);
> + data[6] = intel_guc_ggtt_offset(guc, guc->shared_data);
>
> return intel_guc_send(guc, data, ARRAY_SIZE(data));
> }
> @@ -458,7 +460,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
>
> data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
> data[1] = GUC_POWER_D0;
> - data[2] = guc_ggtt_offset(guc->shared_data);
> + data[2] = intel_guc_ggtt_offset(guc, guc->shared_data);
>
> return intel_guc_send(guc, data, ARRAY_SIZE(data));
> }
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 9e0a97e..b7e2a18 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -101,13 +101,19 @@ static inline void intel_guc_notify(struct intel_guc *guc)
> guc->notify(guc);
> }
>
> -/*
Should add "/**" instead of removing "/*"
> +/* intel_guc_ggtt_offset() - Get the GGTT offset of @vma.
I feel this function is more validating the offset so "Validate and get
the ggtt offset of @vma" ?
> + * @guc: intel guc.
> + * @vma: i915 graphics virtual memory area.
> + *
> * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
> * which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
> * 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
> * used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
> + *
> + * Return: GGTT offset that meets the GuC gfx address requirement.
> */
> -static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> +static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
> + struct i915_vma *vma)
> {
> u32 offset = i915_ggtt_offset(vma);
>
This function uses GUC_GGTT_TOP that is defined in guc_reg.h and I think
we can move it to guc.h similar
to other WOPCM related moves in earlier patch.
> diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c b/drivers/gpu/drm/i915/intel_guc_ads.c
> index ac62753..7215594 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ads.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ads.c
> @@ -113,17 +113,6 @@ int intel_guc_ads_create(struct intel_guc *guc)
> blob->reg_state.white_list[engine->guc_id].count = 0;
> }
>
> - /*
> - * The GuC requires a "Golden Context" when it reinitialises
> - * engines after a reset. Here we use the Render ring default
> - * context, which must already exist and be pinned in the GGTT,
> - * so its address won't change after we've told the GuC where
> - * to find it. Note that we have to skip our header (1 page),
> - * because our GuC shared data is there.
> - */
> - blob->ads.golden_context_lrca =
> - guc_ggtt_offset(dev_priv->kernel_context->engine[RCS].state) +
> - skipped_offset;
>
This move seems unnecessary
> /*
> * The GuC expects us to exclude the portion of the context image that
> @@ -135,11 +124,23 @@ int intel_guc_ads_create(struct intel_guc *guc)
> blob->ads.eng_state_size[engine->guc_id] =
> engine->context_size - skipped_size;
>
> - base = guc_ggtt_offset(vma);
> + base = intel_guc_ggtt_offset(guc, vma);
> blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
> blob->ads.reg_state_buffer = base + ptr_offset(blob, reg_state_buffer);
> blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
>
> + /*
> + * The GuC requires a "Golden Context" when it reinitialises
> + * engines after a reset. Here we use the Render ring default
> + * context, which must already exist and be pinned in the GGTT,
> + * so its address won't change after we've told the GuC where
> + * to find it. Note that we have to skip our header (1 page),
> + * because our GuC shared data is there.
> + */
> + vma = dev_priv->kernel_context->engine[RCS].state;
> + blob->ads.golden_context_lrca = intel_guc_ggtt_offset(guc, vma) +
> + skipped_offset;
> +
> kunmap(page);
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index 24ad557..0a0d3d5 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -156,7 +156,8 @@ static int ctch_init(struct intel_guc *guc,
> err = PTR_ERR(blob);
> goto err_vma;
> }
> - DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
> + DRM_DEBUG_DRIVER("CT: vma base=%#x\n",
> + intel_guc_ggtt_offset(guc, ctch->vma));
>
> /* store pointers to desc and cmds */
> for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
> @@ -202,7 +203,7 @@ static int ctch_open(struct intel_guc *guc,
> }
>
> /* vma should be already allocated and map'ed */
> - base = guc_ggtt_offset(ctch->vma);
> + base = intel_guc_ggtt_offset(guc, ctch->vma);
>
> /* (re)initialize descriptors
> * cmds buffers are in the second half of the blob page
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
> index 3b09329..178d339 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -165,7 +165,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
> I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
>
> /* Set the source address for the new blob */
> - offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
> + offset = intel_guc_ggtt_offset(guc, vma) + guc_fw->header_offset;
> I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 2ffc966..4efe564 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -549,7 +549,7 @@ int intel_guc_log_create(struct intel_guc *guc)
> (GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) |
> (GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT);
>
> - offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */
> + offset = intel_guc_ggtt_offset(guc, vma) >> PAGE_SHIFT; /* in pages */
> guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 1f3a878..c56e4f4 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -375,8 +375,8 @@ static void guc_stage_desc_init(struct intel_guc *guc,
> lrc->context_desc = lower_32_bits(ce->lrc_desc);
>
> /* The state page is after PPHWSP */
> - lrc->ring_lrca =
> - guc_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE;
> + lrc->ring_lrca = intel_guc_ggtt_offset(guc, ce->state) +
> + LRC_STATE_PN * PAGE_SIZE;
>
> /* XXX: In direct submission, the GuC wants the HW context id
> * here. In proxy submission, it wants the stage id
> @@ -384,7 +384,7 @@ static void guc_stage_desc_init(struct intel_guc *guc,
> lrc->context_id = (client->stage_id << GUC_ELC_CTXID_OFFSET) |
> (guc_engine_id << GUC_ELC_ENGINE_OFFSET);
>
> - lrc->ring_begin = guc_ggtt_offset(ce->ring->vma);
> + lrc->ring_begin = intel_guc_ggtt_offset(guc, ce->ring->vma);
> lrc->ring_end = lrc->ring_begin + ce->ring->size - 1;
> lrc->ring_next_free_location = lrc->ring_begin;
> lrc->ring_current_tail_pointer_value = 0;
> @@ -400,7 +400,7 @@ static void guc_stage_desc_init(struct intel_guc *guc,
> * The doorbell, process descriptor, and workqueue are all parts
> * of the client object, which the GuC will reference via the GGTT
> */
> - gfx_addr = guc_ggtt_offset(client->vma);
> + gfx_addr = intel_guc_ggtt_offset(guc, client->vma);
> desc->db_trigger_phy = sg_dma_address(client->vma->pages->sgl) +
> client->doorbell_offset;
> desc->db_trigger_cpu = ptr_to_u64(__get_doorbell(client));
> @@ -596,7 +596,7 @@ static void inject_preempt_context(struct work_struct *work)
> data[3] = engine->guc_id;
> data[4] = guc->execbuf_client->priority;
> data[5] = guc->execbuf_client->stage_id;
> - data[6] = guc_ggtt_offset(guc->shared_data);
> + data[6] = intel_guc_ggtt_offset(guc, guc->shared_data);
>
> if (WARN_ON(intel_guc_send(guc, data, ARRAY_SIZE(data)))) {
> execlists_clear_active(&engine->execlists,
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 8ed0518..aed9c1c 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -137,7 +137,8 @@ static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
> intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>
> /* Set the source address for the uCode */
> - offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
> + offset = intel_guc_ggtt_offset(&dev_priv->guc, vma) +
> + huc_fw->header_offset;
> I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
> I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
>
> @@ -213,7 +214,8 @@ int intel_huc_auth(struct intel_huc *huc)
> }
>
> ret = intel_guc_auth_huc(guc,
> - guc_ggtt_offset(vma) + huc->fw.rsa_offset);
> + intel_guc_ggtt_offset(guc, vma) +
> + huc->fw.rsa_offset);
> if (ret) {
> DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
> goto out;
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
` (6 preceding siblings ...)
2018-02-01 7:04 ` [PATCH v7 1/6] " Sagar Arun Kamble
@ 2018-02-01 7:37 ` Chris Wilson
2018-02-01 19:40 ` Yaodong Li
7 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2018-02-01 7:37 UTC (permalink / raw)
To: Jackie Li, intel-gfx
Quoting Jackie Li (2018-01-19 01:29:27)
> intel_guc_reg.h should only include definition for GuC registers
> and related register bits. GuC WOPCM related values should not
> be defined in intel_guc_reg.h
GuC registers does not include GuC WOPCM? The code does seem to suggest
they are related ;)
> This patch creates a better file structure by moving GuC WOPCM
> related definitions int to a new header intel_guc_wopcm.h
> and moving GuC WOPCM related functions to a new source file
> intel_guc_wopcm.c
Just needs one more sentence to sell this, perhaps "as future patches
will increase the complexity of determining the WOPCM layout".
One function per file is just crying out for LTO ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset
2018-01-19 1:29 ` [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset Jackie Li
2018-02-01 7:27 ` Sagar Arun Kamble
@ 2018-02-01 7:38 ` Chris Wilson
2018-02-01 19:47 ` Yaodong Li
1 sibling, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2018-02-01 7:38 UTC (permalink / raw)
To: Jackie Li, intel-gfx
Quoting Jackie Li (2018-01-19 01:29:28)
> GuC related exported functions should start with "intel_guc_"
> prefix and pass intel_guc as the first parameter since its guc
> related. Current guc_ggtt_offset() failed to follow this code
> convention.
But it was not, and still does not operate on the guc. Is that changing?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers
2018-01-19 1:29 ` [PATCH v7 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers Jackie Li
@ 2018-02-01 7:41 ` Chris Wilson
2018-02-01 22:27 ` Yaodong Li
0 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2018-02-01 7:41 UTC (permalink / raw)
To: Jackie Li, intel-gfx
Quoting Jackie Li (2018-01-19 01:29:31)
> GuC WOPCM registers are write-once registers. Current driver code
> accesses these registers without checking the accessibility to these
> registers, this will lead unpredictable driver behaviors if these
> registers were touch by other components (such as faulty BIOS code).
>
> This patch moves the GuC WOPCM register updating operations into
> intel_guc_wopcm.c and adds checks before and after the write to GuC
> WOPCM registers to make sure the driver is in a known state before
> and after writing to these write-once registers.
>
> v6:
> - Made sure module reloading won't bug the kernel while doing
> locking status checking
>
> v7:
> - Fixed patch format issues
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Jackie Li <yaodong.li@intel.com>
> ---
> +static inline bool __reg_locked(struct drm_i915_private *dev_priv,
> + i915_reg_t reg)
> +{
> + return !!(I915_READ(reg) & GUC_WOPCM_REG_LOCKED);
Why the double cast to bool?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 3/6] drm/i915/guc: Implement dynamic GuC WOPCM offset and size
2018-01-19 1:29 ` [PATCH v7 3/6] drm/i915/guc: Implement dynamic GuC WOPCM offset and size Jackie Li
@ 2018-02-01 8:38 ` Sagar Arun Kamble
2018-02-01 21:51 ` Yaodong Li
0 siblings, 1 reply; 23+ messages in thread
From: Sagar Arun Kamble @ 2018-02-01 8:38 UTC (permalink / raw)
To: Jackie Li, intel-gfx; +Cc: Sujaritha Sundaresan
On 1/19/2018 6:59 AM, Jackie Li wrote:
> Hardware may have specific restrictions on GuC WOPCM size
It would be good if you can tell about Gen9/CNL restriction briefly here.
> versus HuC firmware size. With static GuC WOPCM size,
> there's no way to adjust the GuC WOPCM partition size based on
> the actual HuC firmware size, so that GuC/HuC loading failure
> would occur even if there was enough WOPCM space for both
> GuC and HuC firmware.
>
> This patch enables the dynamic calculation of the GuC WOPCM
> aperture size used by GuC and HuC firmware.
we are also calculating for HuC?
> GuC WOPCM offset is
> set to HuC size + reserved WOPCM size. GuC WOPCM size is set to
> total WOPCM size - GuC WOPCM offset - RC6CTX size. In this case,
> GuC WOPCM offset will be updated based on the size of HuC firmware
> while GuC WOPCM size will be set to use all the remaining WOPCM space.
>
> v2:
> - Removed intel_wopcm_init (Ville/Sagar/Joonas)
> - Renamed and Moved the intel_wopcm_partition into intel_guc (Sagar)
> - Removed unnecessary function calls (Joonas)
> - Init GuC WOPCM partition as soon as firmware fetching is completed
>
> v3:
> - Fixed indentation issues (Chris)
> - Removed layering violation code (Chris/Michal)
> - Created separat files for GuC wopcm code (Michal)
> - Used inline function to avoid code duplication (Michal)
>
> v4:
> - Preset the GuC WOPCM top during early GuC init (Chris)
> - Fail intel_uc_init_hw() as soon as GuC WOPCM partitioning failed
>
> v5:
> - Moved GuC DMA WOPCM register updating code into intel_guc_wopcm.c
> - Took care of the locking status before writing to GuC DMA
> Write-Once registers. (Joonas)
>
> v6:
> - Made sure the GuC WOPCM size to be multiple of 4K (4K aligned)
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: John Spotswood <john.a.spotswood@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Jackie Li <yaodong.li@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 9 +--
> drivers/gpu/drm/i915/intel_guc.c | 5 +-
> drivers/gpu/drm/i915/intel_guc.h | 12 ++--
> drivers/gpu/drm/i915/intel_guc_reg.h | 1 +
> drivers/gpu/drm/i915/intel_guc_wopcm.c | 108 +++++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/intel_guc_wopcm.h | 40 ++++++++++--
> drivers/gpu/drm/i915/intel_huc.c | 2 +-
> drivers/gpu/drm/i915/intel_uc.c | 11 +++-
> drivers/gpu/drm/i915/intel_uc_fw.c | 11 +++-
> drivers/gpu/drm/i915/intel_uc_fw.h | 16 +++++
> 10 files changed, 183 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 648e753..b485794 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -312,12 +312,13 @@ __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 above GUC_WOPCM_TOP. 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.
> + /*
> + * GuC requires the ring to be placed above GuC WOPCM top. 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 = GUC_WOPCM_TOP;
> + ctx->ggtt_offset_bias = dev_priv->guc.wopcm.top;
> else
> ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index e70885b..3521beb 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -64,6 +64,7 @@ void intel_guc_init_early(struct intel_guc *guc)
> {
> intel_guc_fw_init_early(guc);
> intel_guc_ct_init_early(&guc->ct);
> + intel_guc_wopcm_init_early(&guc->wopcm);
>
> mutex_init(&guc->send_mutex);
> guc->send = intel_guc_send_nop;
> @@ -473,7 +474,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
> * This is a wrapper to create an object for use with the GuC. In order to
> * use it inside the GuC, an object needs to be pinned lifetime, so we allocate
> * both some backing storage and a range inside the Global GTT. We must pin
> - * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
> + * it in the GGTT somewhere other than than [0, GuC WOPCM top) because that
> * range is reserved inside GuC.
> *
> * Return: A i915_vma if successful, otherwise an ERR_PTR.
> @@ -494,7 +495,7 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
> goto err;
>
> ret = i915_vma_pin(vma, 0, PAGE_SIZE,
> - PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> + PIN_GLOBAL | PIN_OFFSET_BIAS | guc->wopcm.top);
> 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 b7e2a18..ea35911 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -49,6 +49,7 @@ struct intel_guc {
> struct intel_uc_fw fw;
> struct intel_guc_log log;
> struct intel_guc_ct ct;
> + struct intel_guc_wopcm wopcm;
>
> /* Log snapshot if GuC errors during load */
> struct drm_i915_gem_object *load_err_log;
> @@ -105,10 +106,10 @@ static inline void intel_guc_notify(struct intel_guc *guc)
> * @guc: intel guc.
> * @vma: i915 graphics virtual memory area.
> *
> - * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
> - * which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
> - * 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
> - * used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
> + * GuC does not allow any gfx GGTT address that falls into range
> + * [0, GuC WOPCM top), which is reserved for Boot ROM, SRAM and WOPCM.
> + * All gfx objects used by GuC is pinned with PIN_OFFSET_BIAS along with
> + * top of WOPCM.
> *
> * Return: GGTT offset that meets the GuC gfx address requirement.
> */
> @@ -117,7 +118,8 @@ static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
> {
> u32 offset = i915_ggtt_offset(vma);
>
> - GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> + GEM_BUG_ON(!guc->wopcm.valid);
> + GEM_BUG_ON(offset < guc->wopcm.top);
> GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
>
> return offset;
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
> index 1f52fb8..9109be7 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -75,6 +75,7 @@
>
> /* Defines WOPCM space available to GuC firmware */
> #define GUC_WOPCM_SIZE _MMIO(0xc050)
> +#define GUC_WOPCM_SIZE_MASK (0xfffff000)
>
> /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
> #define GUC_GGTT_TOP 0xFEE00000
> diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> index 87643a0..0532714 100644
> --- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> @@ -25,23 +25,111 @@
> #include "intel_guc_wopcm.h"
> #include "i915_drv.h"
>
> +static inline u32 guc_reserved_wopcm_size(struct intel_guc *guc)
> +{
> + struct drm_i915_private *i915 = guc_to_i915(guc);
> +
> + /* On BXT, the top of WOPCM is reserved for RC6 context */
> + if (IS_GEN9_LP(i915))
> + return BXT_WOPCM_RC6_RESERVED;
Keep the name as BXT_GUC_WOPCM_RC6_RESERVED as earlier. And we seem to
be changing
the size of this from 64KB to 24KB. Please update commit message also.
> +
> + return 0;
> +}
> +
> +static inline int gen9_wocpm_size_check(struct drm_i915_private *i915)
> +{
> + struct intel_guc_wopcm *wopcm = &i915->guc.wopcm;
> + u32 wopcm_base;
> + u32 delta;
> +
> + /*
> + * Check hardware restriction on Gen9
> + * GuC WOPCM size is at least 4 bytes larger than GuC WOPCM base due
> + * to hardware limitation on Gen9.
> + */
> + wopcm_base = wopcm->offset + GEN9_GUC_WOPCM_OFFSET;
> + if (unlikely(wopcm_base > wopcm->size))
Comment for this restriction?
> + return -E2BIG;
> +
> + delta = wopcm->size - wopcm_base;
> + if (unlikely(delta < GEN9_GUC_WOPCM_DELTA))
> + return -E2BIG;
> +
> + return 0;
> +}
> +
> +static inline int guc_wopcm_size_check(struct intel_guc *guc)
> +{
> + struct drm_i915_private *i915 = guc_to_i915(guc);
> +
> + if (IS_GEN9(i915))
> + return gen9_wocpm_size_check(i915);
> +
> + return 0;
> +}
> +
> /*
> - * intel_guc_wopcm_size() - Get the size of GuC WOPCM.
> + * intel_guc_wopcm_init() - Initialize the GuC WOPCM partition.
> * @guc: intel guc.
> + * @guc_fw_size: size of GuC firmware.
> + * @huc_fw_size: size of HuC firmware.
> *
> - * Get the platform specific GuC WOPCM size.
> + * This function tries to initialize the WOPCM partition based on HuC firmware
> + * size and the reserved WOPCM memory size.
> *
> - * Return: size of the GuC WOPCM.
> + * Return: 0 on success, non-zero error code on failure.
> */
> -u32 intel_guc_wopcm_size(struct intel_guc *guc)
> +int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
> + u32 huc_fw_size)
> {
> - struct drm_i915_private *i915 = guc_to_i915(guc);
> + u32 reserved = guc_reserved_wopcm_size(guc);
> + u32 offset, size, top;
> + int err;
>
> - u32 wopcm_size = GUC_WOPCM_TOP;
> + if (guc->wopcm.valid)
> + return 0;
>
> - /* On BXT, the top of WOPCM is reserved for RC6 context */
> - if (IS_GEN9_LP(i915))
> - wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
> + if (!guc_fw_size)
> + return -EINVAL;
> +
> + if (reserved >= WOPCM_DEFAULT_SIZE)
> + return -E2BIG;
> +
> + offset = huc_fw_size + WOPCM_RESERVED_SIZE;
> + if (offset >= WOPCM_DEFAULT_SIZE)
> + return -E2BIG;
> +
> + /* Hardware requires GuC WOPCM offset needs to be 16K aligned. */
> + offset = ALIGN(offset, WOPCM_OFFSET_ALIGNMENT);
> + if ((offset + reserved) >= WOPCM_DEFAULT_SIZE)
> + return -E2BIG;
> +
> + top = WOPCM_DEFAULT_SIZE - offset;
> + size = top - reserved;
> +
> + /* GuC WOPCM size must be 4K aligned. */
> + size &= GUC_WOPCM_SIZE_MASK;
> +
ALIGN(size, PAGE_SIZE)? Can avoid this new macro.
If you want to stay with GUC_WOPCM_SIZE_MASK, then that macro needs to
be in guc_wopcm.h
> + /*
> + * GuC size needs to be less than or equal to GuC WOPCM size.
> + * Need extra 8K stack for GuC.
> + */
> + if ((guc_fw_size + GUC_WOPCM_STACK_RESERVED) > size)
> + return -E2BIG;
> +
> + guc->wopcm.offset = offset;
> + guc->wopcm.size = size;
> + guc->wopcm.top = top;
> +
> + /* Check platform specific restrictions */
> + err = guc_wopcm_size_check(guc);
> + if (err)
> + return err;
> +
> + guc->wopcm.valid = true;
> +
> + DRM_DEBUG_DRIVER("GuC WOPCM offset %dKB, size %dKB, top %dKB\n",
> + offset >> 10, size >> 10, top >> 10);
>
> - return wopcm_size;
> + return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
> index 595fb1c..352cf3d 100644
> --- a/drivers/gpu/drm/i915/intel_guc_wopcm.h
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
> @@ -29,11 +29,41 @@
>
> struct intel_guc;
>
> -#define GUC_WOPCM_OFFSET_VALUE 0x80000 /* 512KB */
> -/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
> -#define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */
> -#define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */
> +/* Default WOPCM size 1MB */
> +#define WOPCM_DEFAULT_SIZE (0x1 << 20)
> +/* Reserved WOPCM size 16KB */
> +#define WOPCM_RESERVED_SIZE (0x4000)
WOPCM_BEGIN_RESERVED_SIZE?
> +/* GUC WOPCM Offset need to be 16KB aligned */
> +#define WOPCM_OFFSET_ALIGNMENT (0x4000)
> +/* 8KB stack reserved for GuC FW*/
> +#define GUC_WOPCM_STACK_RESERVED (0x2000)
GUC_WOPCM_BEGIN_RESERVED_STACK_SIZE?
> +/* 24KB WOPCM reserved for RC6 CTX on BXT */
> +#define BXT_WOPCM_RC6_RESERVED (0x6000)
BXT_GUC_WOPCM_END_RC6_RESERVED?
Layout in comments would simplify understanding. :)
Or just comments as initial 16kb is reserved in wopcm then huc can be
loaded,
post that 8kb is needed for GuC stack and then Guc is to be loaded till
point
from where rc6 context resides.
Distinguishing whether reserved from begin or end helps understand the
code logic.
>
> -u32 intel_guc_wopcm_size(struct intel_guc *guc);
> +#define GEN9_GUC_WOPCM_DELTA 4
> +#define GEN9_GUC_WOPCM_OFFSET (0x24000)
> +
> +struct intel_guc_wopcm {
> + u32 offset;
> + u32 size;
> + u32 top;
> + bool valid;
> +};
> +
> +/*
kernel doc comment format "/**"
> + * intel_guc_wopcm_init_early() - Early initialization of the GuC WOPCM.
> + * @wopcm: GuC WOPCM.
> + *
> + * Setup the GuC WOPCM top to the top of the overall WOPCM. This will guarantee
> + * that the allocation of the GuC accessible objects won't fall into WOPCM when
> + * GuC partition isn't present.
> + *
> + */
> +static inline void intel_guc_wopcm_init_early(struct intel_guc_wopcm *wopcm)
> +{
> + wopcm->top = WOPCM_DEFAULT_SIZE;
I think instead of hardcoding we can have it derived from
"reserved_total" in i915_gem_init_stolen.
Will need to init possibly during intel_uc_init_misc or we can derive
reserved_total here itself.
Also it seems we need to reduce usage of WOPCM_DEFAULT_SIZE and rely on
wopcm->top in
places needing the check.
> +}
> +
> +int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_size, u32 huc_size);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index aed9c1c..dc6a6c6 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -206,7 +206,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_WOPCM_TOP);
> + PIN_OFFSET_BIAS | guc->wopcm.top);
> 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.c b/drivers/gpu/drm/i915/intel_uc.c
> index e9d0568..b5a08d1 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -263,6 +263,9 @@ void intel_uc_fini_wq(struct drm_i915_private *dev_priv)
> int intel_uc_init(struct drm_i915_private *dev_priv)
> {
> struct intel_guc *guc = &dev_priv->guc;
> + struct intel_huc *huc = &dev_priv->huc;
> + u32 guc_fw_size = intel_uc_fw_get_size(&guc->fw);
> + u32 huc_fw_size = intel_uc_fw_get_size(&huc->fw);
> int ret;
>
> if (!USES_GUC(dev_priv))
> @@ -271,6 +274,10 @@ int intel_uc_init(struct drm_i915_private *dev_priv)
> if (!HAS_GUC(dev_priv))
> return -ENODEV;
>
> + ret = intel_guc_wopcm_init(guc, guc_fw_size, huc_fw_size);
> + if (ret)
> + return ret;
> +
> ret = intel_guc_init(guc);
> if (ret)
> return ret;
> @@ -320,9 +327,9 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> gen9_reset_guc_interrupts(dev_priv);
>
> /* init WOPCM */
> - I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));
> + I915_WRITE(GUC_WOPCM_SIZE, guc->wopcm.size);
> I915_WRITE(DMA_GUC_WOPCM_OFFSET,
> - GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
> + guc->wopcm.offset | HUC_LOADING_AGENT_GUC);
>
> /* WaEnableuKernelHeaderValidFix:skl */
> /* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
> index 24945cf..791263a 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -95,9 +95,13 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
> uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
>
> - /* Header and uCode will be loaded to WOPCM */
> + /*
> + * Header and uCode will be loaded to WOPCM
> + * Only check the size against the overall available WOPCM here. Will
> + * continue to check the size during WOPCM partition calculation.
> + */
> size = uc_fw->header_size + uc_fw->ucode_size;
> - if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
> + if (size > WOPCM_DEFAULT_SIZE) {
> DRM_WARN("%s: Firmware is too large to fit in WOPCM\n",
> intel_uc_fw_type_repr(uc_fw->type));
> err = -E2BIG;
> @@ -207,6 +211,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
> int (*xfer)(struct intel_uc_fw *uc_fw,
> struct i915_vma *vma))
> {
> + struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
> struct i915_vma *vma;
> int err;
>
> @@ -230,7 +235,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
> }
>
> vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
> - PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> + PIN_OFFSET_BIAS | i915->guc.wopcm.top);
> if (IS_ERR(vma)) {
> err = PTR_ERR(vma);
> DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
> index d5fd460..d00d888 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.h
> @@ -115,6 +115,22 @@ static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw)
> return uc_fw->path != NULL;
> }
>
> +/*
> + * intel_uc_fw_get_size() - Get the size of the firmware.
> + * @uc_fw: intel_uc_fw structure.
> + *
> + * Get the size of the firmware that will be placed in WOPCM.
> + *
> + * Return: Zero on invalid firmware status. actual size on success.
> + */
> +static inline u32 intel_uc_fw_get_size(struct intel_uc_fw *uc_fw)
> +{
> + if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
> + return 0;
> +
> + return uc_fw->header_size + uc_fw->ucode_size;
> +}
> +
> void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> struct intel_uc_fw *uc_fw);
> int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 4/6] drm/i915/guc: Add WOPCM partitioning support for CNL
2018-01-19 1:29 ` [PATCH v7 4/6] drm/i915/guc: Add WOPCM partitioning support for CNL Jackie Li
@ 2018-02-01 8:44 ` Sagar Arun Kamble
0 siblings, 0 replies; 23+ messages in thread
From: Sagar Arun Kamble @ 2018-02-01 8:44 UTC (permalink / raw)
To: Jackie Li, intel-gfx
On 1/19/2018 6:59 AM, Jackie Li wrote:
> CNL has different WOPCM size and hardware restriction on GuC
> WOPCM size.
You are also updating gen9 path so update subject and commit message.
> This patch returns the correct WOPCM reserved size on CNL and
> adds the GuC WOPCM size check for CNL.
>
> v6:
> - Extended HuC FW size check against GuC WOPCM size to all
> Gen9 and CNL A0 platforms
>
> v7:
> - Fixed patch format issues
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: John Spotswood <john.a.spotswood@intel.com>
> Cc: Jeff McGee <jeff.mcgee@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Jackie Li <yaodong.li@intel.com>
> ---
> drivers/gpu/drm/i915/intel_guc_wopcm.c | 23 ++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_guc_wopcm.h | 4 ++++
> 2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> index 0532714..ed3096c 100644
> --- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> @@ -33,6 +33,24 @@ static inline u32 guc_reserved_wopcm_size(struct intel_guc *guc)
> if (IS_GEN9_LP(i915))
> return BXT_WOPCM_RC6_RESERVED;
>
> + if (IS_GEN10(i915))
> + return CNL_WOPCM_RESERVED;
> +
> + return 0;
> +}
> +
> +/*
> + * On Gen9 & CNL A0, hardware requires GuC size to be larger than or equal to
Is there workaround name for this?
> + * HuC kernel size.
> + */
> +static inline int wopcm_huc_size_check(struct drm_i915_private *i915)
> +{
> + struct intel_guc_wopcm *wopcm = &i915->guc.wopcm;
> + u32 huc_size = intel_uc_fw_get_size(&i915->huc.fw);
> +
> + if (unlikely(wopcm->size - GUC_WOPCM_RESERVED < huc_size))
> + return -E2BIG;
> +
> return 0;
> }
>
> @@ -55,7 +73,7 @@ static inline int gen9_wocpm_size_check(struct drm_i915_private *i915)
> if (unlikely(delta < GEN9_GUC_WOPCM_DELTA))
> return -E2BIG;
>
> - return 0;
> + return wopcm_huc_size_check(i915);
> }
>
> static inline int guc_wopcm_size_check(struct intel_guc *guc)
> @@ -65,6 +83,9 @@ static inline int guc_wopcm_size_check(struct intel_guc *guc)
> if (IS_GEN9(i915))
> return gen9_wocpm_size_check(i915);
>
> + if (IS_CNL_REVID(i915, CNL_REVID_A0, CNL_REVID_A0))
> + return wopcm_huc_size_check(i915);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.h b/drivers/gpu/drm/i915/intel_guc_wopcm.h
> index 352cf3d..5306175 100644
> --- a/drivers/gpu/drm/i915/intel_guc_wopcm.h
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.h
> @@ -37,8 +37,12 @@ struct intel_guc;
> #define WOPCM_OFFSET_ALIGNMENT (0x4000)
> /* 8KB stack reserved for GuC FW*/
> #define GUC_WOPCM_STACK_RESERVED (0x2000)
> +/* 16KB reserved at the beginning of GuC WOPCM */
> +#define GUC_WOPCM_RESERVED (0x4000)
But then this is not considered in guc_wopcm_init like STACK_RESERVED
> /* 24KB WOPCM reserved for RC6 CTX on BXT */
> #define BXT_WOPCM_RC6_RESERVED (0x6000)
> +/* 36KB WOPCM reserved on CNL */
> +#define CNL_WOPCM_RESERVED (0x9000)
CNL_GUC_WOPCM_END_RC6_RESERVED?
>
> #define GEN9_GUC_WOPCM_DELTA 4
> #define GEN9_GUC_WOPCM_OFFSET (0x24000)
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 6/6] HAX Enable GuC Submission for CI
2018-01-19 1:29 ` [PATCH v7 6/6] HAX Enable GuC Submission for CI Jackie Li
@ 2018-02-01 8:55 ` Sagar Arun Kamble
0 siblings, 0 replies; 23+ messages in thread
From: Sagar Arun Kamble @ 2018-02-01 8:55 UTC (permalink / raw)
To: Jackie Li, intel-gfx
On 1/19/2018 6:59 AM, Jackie Li wrote:
> Signed-off-by: Jackie Li <yaodong.li@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
> drivers/gpu/drm/i915/i915_params.c | 2 +-
> drivers/gpu/drm/i915/i915_params.h | 2 +-
> drivers/gpu/drm/i915/intel_guc_wopcm.c | 6 ++++++
> 4 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 26dee5e..fed472a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3561,7 +3561,7 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
>
> void i915_ggtt_enable_guc(struct drm_i915_private *i915)
> {
> - GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
> +// GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
This is no more needed.
>
> i915->ggtt.invalidate = guc_ggtt_invalidate;
>
> @@ -3571,7 +3571,7 @@ void i915_ggtt_enable_guc(struct drm_i915_private *i915)
> void i915_ggtt_disable_guc(struct drm_i915_private *i915)
> {
> /* We should only be called after i915_ggtt_enable_guc() */
> - GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
> +// GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
>
> i915->ggtt.invalidate = gen6_ggtt_invalidate;
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 0b553a8..b33d364 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -152,7 +152,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400,
> i915_param_named_unsafe(enable_guc, int, 0400,
> "Enable GuC load for GuC submission and/or HuC load. "
> "Required functionality can be selected using bitmask values. "
> - "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)");
> + "(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)");
>
> i915_param_named(guc_log_level, int, 0400,
> "GuC firmware logging level. Requires GuC to be loaded. "
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index c963603..53037b5 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -47,7 +47,7 @@ struct drm_printer;
> param(int, disable_power_well, -1) \
> param(int, enable_ips, 1) \
> param(int, invert_brightness, 0) \
> - param(int, enable_guc, 0) \
> + param(int, enable_guc, -1) \
> param(int, guc_log_level, -1) \
> param(char *, guc_firmware_path, NULL) \
> param(char *, huc_firmware_path, NULL) \
> diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> index 236fc32..e972b7b 100644
> --- a/drivers/gpu/drm/i915/intel_guc_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c
> @@ -154,6 +154,9 @@ int intel_guc_wopcm_init(struct intel_guc *guc, u32 guc_fw_size,
> u32 offset, size, top;
> int err;
>
> + DRM_DEBUG_DRIVER("guc_fw size %u, huc_fw_size %u\n", guc_fw_size,
> + huc_fw_size);
> +
> GEM_BUG_ON(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID);
>
> if (!guc_fw_size)
> @@ -213,6 +216,9 @@ void intel_guc_wopcm_init_hw(struct intel_guc *guc)
> {
> u32 locked = guc_wopcm_locked(guc);
>
> + DRM_DEBUG_DRIVER("locked = %s, flags = %#x\n", yesno(locked),
> + guc->wopcm.flags);
> +
> GEM_BUG_ON(!(guc->wopcm.flags & INTEL_GUC_WOPCM_VALID));
>
> /*
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset
2018-02-01 7:27 ` Sagar Arun Kamble
@ 2018-02-01 19:29 ` Yaodong Li
0 siblings, 0 replies; 23+ messages in thread
From: Yaodong Li @ 2018-02-01 19:29 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx
>> diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c
>> b/drivers/gpu/drm/i915/intel_guc_ads.c
>> index ac62753..7215594 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_ads.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_ads.c
>> @@ -113,17 +113,6 @@ int intel_guc_ads_create(struct intel_guc *guc)
>> blob->reg_state.white_list[engine->guc_id].count = 0;
>> }
>> - /*
>> - * The GuC requires a "Golden Context" when it reinitialises
>> - * engines after a reset. Here we use the Render ring default
>> - * context, which must already exist and be pinned in the GGTT,
>> - * so its address won't change after we've told the GuC where
>> - * to find it. Note that we have to skip our header (1 page),
>> - * because our GuC shared data is there.
>> - */
>> - blob->ads.golden_context_lrca =
>> - guc_ggtt_offset(dev_priv->kernel_context->engine[RCS].state) +
>> - skipped_offset;
> This move seems unnecessary
This move is mainly for reusing "vma" variable so that the line won't
get too
long. Besides, this move can also make sure the assignment will get
closer to
the code that actually uses the value from
"blob->ads.golden_context_lrca":-)
>> /*
>> * The GuC expects us to exclude the portion of the context
>> image that
>> @@ -135,11 +124,23 @@ int intel_guc_ads_create(struct intel_guc *guc)
>> blob->ads.eng_state_size[engine->guc_id] =
>> engine->context_size - skipped_size;
>> - base = guc_ggtt_offset(vma);
>> + base = intel_guc_ggtt_offset(guc, vma);
>> blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
>> blob->ads.reg_state_buffer = base + ptr_offset(blob,
>> reg_state_buffer);
>> blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
>> + /*
>> + * The GuC requires a "Golden Context" when it reinitialises
>> + * engines after a reset. Here we use the Render ring default
>> + * context, which must already exist and be pinned in the GGTT,
>> + * so its address won't change after we've told the GuC where
>> + * to find it. Note that we have to skip our header (1 page),
>> + * because our GuC shared data is there.
>> + */
>> + vma = dev_priv->kernel_context->engine[RCS].state;
>> + blob->ads.golden_context_lrca = intel_guc_ggtt_offset(guc, vma) +
>> + skipped_offset;
>> +
>> kunmap(page);
>> return 0;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files
2018-02-01 7:37 ` Chris Wilson
@ 2018-02-01 19:40 ` Yaodong Li
0 siblings, 0 replies; 23+ messages in thread
From: Yaodong Li @ 2018-02-01 19:40 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/31/2018 11:37 PM, Chris Wilson wrote:
> Quoting Jackie Li (2018-01-19 01:29:27)
>> intel_guc_reg.h should only include definition for GuC registers
>> and related register bits. GuC WOPCM related values should not
>> be defined in intel_guc_reg.h
> GuC registers does not include GuC WOPCM? The code does seem to suggest
> they are related ;)
>
I should say "Non-register related GuC WOPCM macros should not be
defined in intel_guc_reg.h"?
>> This patch creates a better file structure by moving GuC WOPCM
>> related definitions int to a new header intel_guc_wopcm.h
>> and moving GuC WOPCM related functions to a new source file
>> intel_guc_wopcm.c
> Just needs one more sentence to sell this, perhaps "as future patches
> will increase the complexity of determining the WOPCM layout".
>
> One function per file is just crying out for LTO ;)
Thanks Chris! I will add it.
This is why I need your help and learn how to sell this:-)
Really appreciate the comments!
Regards,
-Jackie
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset
2018-02-01 7:38 ` Chris Wilson
@ 2018-02-01 19:47 ` Yaodong Li
2018-02-01 22:35 ` Chris Wilson
0 siblings, 1 reply; 23+ messages in thread
From: Yaodong Li @ 2018-02-01 19:47 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/31/2018 11:38 PM, Chris Wilson wrote:
> Quoting Jackie Li (2018-01-19 01:29:28)
>> GuC related exported functions should start with "intel_guc_"
>> prefix and pass intel_guc as the first parameter since its guc
>> related. Current guc_ggtt_offset() failed to follow this code
>> convention.
> But it was not, and still does not operate on the guc. Is that changing?
this problem is that it's guc related and the following patches do need
to access the data from intel_guc. Do you think it's getting better
if I add a sentence like "the future patches will need to access
the intel_guc to verify the offset"?
Regards,
-Jackie
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 3/6] drm/i915/guc: Implement dynamic GuC WOPCM offset and size
2018-02-01 8:38 ` Sagar Arun Kamble
@ 2018-02-01 21:51 ` Yaodong Li
2018-02-02 5:22 ` Sagar Arun Kamble
0 siblings, 1 reply; 23+ messages in thread
From: Yaodong Li @ 2018-02-01 21:51 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx; +Cc: Sujaritha Sundaresan
On 02/01/2018 12:38 AM, Sagar Arun Kamble wrote:
>
>
> On 1/19/2018 6:59 AM, Jackie Li wrote:
>> Hardware may have specific restrictions on GuC WOPCM size
> It would be good if you can tell about Gen9/CNL restriction briefly here.
>> versus HuC firmware size. With static GuC WOPCM size,
>> there's no way to adjust the GuC WOPCM partition size based on
>> the actual HuC firmware size, so that GuC/HuC loading failure
>> would occur even if there was enough WOPCM space for both
>> GuC and HuC firmware.
>>
>> This patch enables the dynamic calculation of the GuC WOPCM
>> aperture size used by GuC and HuC firmware.
> we are also calculating for HuC?
Strictly speaking, we are calculating the GuC WOPCM offset based on
GuC & HuC firmware sizes. Will make it clearer. Thanks.
>> + offset = huc_fw_size + WOPCM_RESERVED_SIZE;
>> + if (offset >= WOPCM_DEFAULT_SIZE)
>> + return -E2BIG;
>> +
>> + /* Hardware requires GuC WOPCM offset needs to be 16K aligned. */
>> + offset = ALIGN(offset, WOPCM_OFFSET_ALIGNMENT);
>> + if ((offset + reserved) >= WOPCM_DEFAULT_SIZE)
>> + return -E2BIG;
>> +
>> + top = WOPCM_DEFAULT_SIZE - offset;
>> + size = top - reserved;
>> +
>> + /* GuC WOPCM size must be 4K aligned. */
>> + size &= GUC_WOPCM_SIZE_MASK;
>> +
> ALIGN(size, PAGE_SIZE)? Can avoid this new macro.
> If you want to stay with GUC_WOPCM_SIZE_MASK, then that macro needs to
> be in guc_wopcm.h
I'd like to go with GUC_WOPCM_SIZE_MASK here since there's no sign that
it should be related to
page size. *Align* seems not accurate here since I actually wanted to
trim the size to 4k boundary,
will update the comments. Thanks!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers
2018-02-01 7:41 ` Chris Wilson
@ 2018-02-01 22:27 ` Yaodong Li
0 siblings, 0 replies; 23+ messages in thread
From: Yaodong Li @ 2018-02-01 22:27 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/31/2018 11:41 PM, Chris Wilson wrote:
>
>> - Fixed patch format issues
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Jackie Li <yaodong.li@intel.com>
>> ---
>> +static inline bool __reg_locked(struct drm_i915_private *dev_priv,
>> + i915_reg_t reg)
>> +{
>> + return !!(I915_READ(reg) & GUC_WOPCM_REG_LOCKED);
> Why the double cast to bool?
My thought was the code would be clearer to use bool as the return type and
have a explicit cast to bool. Are you suggesting I should use a return
type such as *int*
and remove the double cast?
Regards,
-Jackie
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset
2018-02-01 19:47 ` Yaodong Li
@ 2018-02-01 22:35 ` Chris Wilson
2018-02-01 22:46 ` Yaodong Li
0 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2018-02-01 22:35 UTC (permalink / raw)
To: Yaodong Li, intel-gfx
Quoting Yaodong Li (2018-02-01 19:47:53)
>
> On 01/31/2018 11:38 PM, Chris Wilson wrote:
> > Quoting Jackie Li (2018-01-19 01:29:28)
> >> GuC related exported functions should start with "intel_guc_"
> >> prefix and pass intel_guc as the first parameter since its guc
> >> related. Current guc_ggtt_offset() failed to follow this code
> >> convention.
> > But it was not, and still does not operate on the guc. Is that changing?
> this problem is that it's guc related and the following patches do need
> to access the data from intel_guc. Do you think it's getting better
> if I add a sentence like "the future patches will need to access
> the intel_guc to verify the offset"?
That's the idea. You need to explain _why_ you need a particular change,
in some cases like this where it's not clear from the context of the
patch, you need to fill in the missing details for the reader. In patch
series like this where there is upfront refactoring required, remember
the reader is starting at the beginning with no idea of what's coming
next, so a bit^Wlot of foreshadowing is required in the story you tell.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset
2018-02-01 22:35 ` Chris Wilson
@ 2018-02-01 22:46 ` Yaodong Li
0 siblings, 0 replies; 23+ messages in thread
From: Yaodong Li @ 2018-02-01 22:46 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 02/01/2018 02:35 PM, Chris Wilson wrote:
> Quoting Yaodong Li (2018-02-01 19:47:53)
>> On 01/31/2018 11:38 PM, Chris Wilson wrote:
>>> Quoting Jackie Li (2018-01-19 01:29:28)
>>>> GuC related exported functions should start with "intel_guc_"
>>>> prefix and pass intel_guc as the first parameter since its guc
>>>> related. Current guc_ggtt_offset() failed to follow this code
>>>> convention.
>>> But it was not, and still does not operate on the guc. Is that changing?
>> this problem is that it's guc related and the following patches do need
>> to access the data from intel_guc. Do you think it's getting better
>> if I add a sentence like "the future patches will need to access
>> the intel_guc to verify the offset"?
> That's the idea. You need to explain _why_ you need a particular change,
> in some cases like this where it's not clear from the context of the
> patch, you need to fill in the missing details for the reader. In patch
> series like this where there is upfront refactoring required, remember
> the reader is starting at the beginning with no idea of what's coming
> next, so a bit^Wlot of foreshadowing is required in the story you tell.
Got it. Will keep that in mind. Thank you Chris:-)
Regards,
-Jackie
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v7 3/6] drm/i915/guc: Implement dynamic GuC WOPCM offset and size
2018-02-01 21:51 ` Yaodong Li
@ 2018-02-02 5:22 ` Sagar Arun Kamble
0 siblings, 0 replies; 23+ messages in thread
From: Sagar Arun Kamble @ 2018-02-02 5:22 UTC (permalink / raw)
To: Yaodong Li, intel-gfx; +Cc: Sujaritha Sundaresan
On 2/2/2018 3:21 AM, Yaodong Li wrote:
>
> On 02/01/2018 12:38 AM, Sagar Arun Kamble wrote:
>>
>>
>> On 1/19/2018 6:59 AM, Jackie Li wrote:
>>> Hardware may have specific restrictions on GuC WOPCM size
>> It would be good if you can tell about Gen9/CNL restriction briefly
>> here.
>>> versus HuC firmware size. With static GuC WOPCM size,
>>> there's no way to adjust the GuC WOPCM partition size based on
>>> the actual HuC firmware size, so that GuC/HuC loading failure
>>> would occur even if there was enough WOPCM space for both
>>> GuC and HuC firmware.
>>>
>>> This patch enables the dynamic calculation of the GuC WOPCM
>>> aperture size used by GuC and HuC firmware.
>> we are also calculating for HuC?
> Strictly speaking, we are calculating the GuC WOPCM offset based on
> GuC & HuC firmware sizes. Will make it clearer. Thanks.
>
>>> + offset = huc_fw_size + WOPCM_RESERVED_SIZE;
>>> + if (offset >= WOPCM_DEFAULT_SIZE)
>>> + return -E2BIG;
>>> +
>>> + /* Hardware requires GuC WOPCM offset needs to be 16K aligned. */
>>> + offset = ALIGN(offset, WOPCM_OFFSET_ALIGNMENT);
>>> + if ((offset + reserved) >= WOPCM_DEFAULT_SIZE)
>>> + return -E2BIG;
>>> +
>>> + top = WOPCM_DEFAULT_SIZE - offset;
>>> + size = top - reserved;
>>> +
>>> + /* GuC WOPCM size must be 4K aligned. */
>>> + size &= GUC_WOPCM_SIZE_MASK;
>>> +
>> ALIGN(size, PAGE_SIZE)? Can avoid this new macro.
>> If you want to stay with GUC_WOPCM_SIZE_MASK, then that macro needs
>> to be in guc_wopcm.h
> I'd like to go with GUC_WOPCM_SIZE_MASK here since there's no sign
> that it should be related to
> page size. *Align* seems not accurate here since I actually wanted to
> trim the size to 4k boundary,
> will update the comments. Thanks!
Need to update comment though. /* GuC WOPCM size must be multiple of 4K
pages */
Align does not suit here. We could have used "& ~PAGE_MASK" but okay
with G_W_S_M too.
size being absolute here helps. had it been offset starting from 0 we
would have needed extra logic.
>
>
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2018-02-02 5:22 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-19 1:29 [PATCH v7 1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Jackie Li
2018-01-19 1:29 ` [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset Jackie Li
2018-02-01 7:27 ` Sagar Arun Kamble
2018-02-01 19:29 ` Yaodong Li
2018-02-01 7:38 ` Chris Wilson
2018-02-01 19:47 ` Yaodong Li
2018-02-01 22:35 ` Chris Wilson
2018-02-01 22:46 ` Yaodong Li
2018-01-19 1:29 ` [PATCH v7 3/6] drm/i915/guc: Implement dynamic GuC WOPCM offset and size Jackie Li
2018-02-01 8:38 ` Sagar Arun Kamble
2018-02-01 21:51 ` Yaodong Li
2018-02-02 5:22 ` Sagar Arun Kamble
2018-01-19 1:29 ` [PATCH v7 4/6] drm/i915/guc: Add WOPCM partitioning support for CNL Jackie Li
2018-02-01 8:44 ` Sagar Arun Kamble
2018-01-19 1:29 ` [PATCH v7 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers Jackie Li
2018-02-01 7:41 ` Chris Wilson
2018-02-01 22:27 ` Yaodong Li
2018-01-19 1:29 ` [PATCH v7 6/6] HAX Enable GuC Submission for CI Jackie Li
2018-02-01 8:55 ` Sagar Arun Kamble
2018-01-19 7:03 ` ✗ Fi.CI.BAT: failure for series starting with [v7,1/6] drm/i915/guc: Move GuC WOPCM related code into separate files Patchwork
2018-02-01 7:04 ` [PATCH v7 1/6] " Sagar Arun Kamble
2018-02-01 7:37 ` Chris Wilson
2018-02-01 19:40 ` Yaodong Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.