* [PATCH v3 1/8] drm/i915/uc: Unify uC platform check
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
@ 2019-07-25 0:18 ` Daniele Ceraolo Spurio
2019-07-25 0:18 ` [PATCH v3 2/8] drm/i915: Fix handling of non-supported uC Daniele Ceraolo Spurio
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-07-25 0:18 UTC (permalink / raw)
To: intel-gfx
We have several HAS_* checks for GuC and HuC but we mostly use HAS_GUC
and HAS_HUC, with only 1 exception. Since our HW always has either
both uC or neither of them, just replace all the checks with a unified
HAS_UC.
v2: use HAS_GT_UC (Michal)
v3: fix comment (Michal)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/gt/intel_reset.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 4 ++--
drivers/gpu/drm/i915/i915_debugfs.c | 6 +++---
drivers/gpu/drm/i915/i915_drv.h | 15 ++-------------
drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
drivers/gpu/drm/i915/i915_irq.c | 2 +-
drivers/gpu/drm/i915/i915_pci.c | 4 ++--
drivers/gpu/drm/i915/intel_device_info.h | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
drivers/gpu/drm/i915/intel_wopcm.c | 4 ++--
13 files changed, 21 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 55e2ddcbd215..98c071fe532b 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -595,7 +595,7 @@ int intel_reset_guc(struct intel_gt *gt)
INTEL_GEN(gt->i915) >= 11 ? GEN11_GRDOM_GUC : GEN9_GRDOM_GUC;
int ret;
- GEM_BUG_ON(!HAS_GUC(gt->i915));
+ GEM_BUG_ON(!HAS_GT_UC(gt->i915));
intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
ret = gen6_hw_domain_reset(gt, guc_domain);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 3dfa40fdbe99..87169e826747 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -80,7 +80,7 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw)
GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
- if (!HAS_GUC(i915)) {
+ if (!HAS_GT_UC(i915)) {
guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
return;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 543854c42d9d..ff6f7b157ecb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -74,7 +74,7 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw)
GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
- if (!HAS_HUC(dev_priv)) {
+ if (!HAS_GT_UC(dev_priv)) {
huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
return;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 25a8ab3bd22c..bdb171c3f36e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -61,7 +61,7 @@ static int __get_platform_enable_guc(struct intel_uc *uc)
struct intel_uc_fw *huc_fw = &uc->huc.fw;
int enable_guc = 0;
- if (!HAS_GUC(uc_to_gt(uc)->i915))
+ if (!HAS_GT_UC(uc_to_gt(uc)->i915))
return 0;
/* We don't want to enable GuC/HuC on pre-Gen11 by default */
diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
index 93f7c930ab18..371f7a60c987 100644
--- a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
@@ -134,7 +134,7 @@ static int igt_guc_clients(void *args)
struct intel_guc *guc;
int err = 0;
- GEM_BUG_ON(!HAS_GUC(dev_priv));
+ GEM_BUG_ON(!HAS_GT_UC(dev_priv));
mutex_lock(&dev_priv->drm.struct_mutex);
wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
@@ -226,7 +226,7 @@ static int igt_guc_doorbells(void *arg)
int i, err = 0;
u16 db_id;
- GEM_BUG_ON(!HAS_GUC(dev_priv));
+ GEM_BUG_ON(!HAS_GT_UC(dev_priv));
mutex_lock(&dev_priv->drm.struct_mutex);
wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6d3911469801..24787bb48c9f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1865,7 +1865,7 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data)
intel_wakeref_t wakeref;
struct drm_printer p;
- if (!HAS_HUC(dev_priv))
+ if (!HAS_GT_UC(dev_priv))
return -ENODEV;
p = drm_seq_file_printer(m);
@@ -1883,7 +1883,7 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
intel_wakeref_t wakeref;
struct drm_printer p;
- if (!HAS_GUC(dev_priv))
+ if (!HAS_GT_UC(dev_priv))
return -ENODEV;
p = drm_seq_file_printer(m);
@@ -2062,7 +2062,7 @@ static int i915_guc_log_dump(struct seq_file *m, void *data)
u32 *log;
int i = 0;
- if (!HAS_GUC(dev_priv))
+ if (!HAS_GT_UC(dev_priv))
return -ENODEV;
if (dump_load_err)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0e44cc4b2ca1..59d4a1146039 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2271,20 +2271,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_IPC(dev_priv) (INTEL_INFO(dev_priv)->display.has_ipc)
-/*
- * For now, anything with a GuC requires uCode loading, and then supports
- * command submission once loaded. But these are logically independent
- * properties, so we have separate macros to test them.
- */
-#define HAS_GUC(dev_priv) (INTEL_INFO(dev_priv)->has_guc)
-#define HAS_GUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
-#define HAS_GUC_SCHED(dev_priv) (HAS_GUC(dev_priv))
-
-/* For now, anything with a GuC has also HuC */
-#define HAS_HUC(dev_priv) (HAS_GUC(dev_priv))
-#define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
+#define HAS_GT_UC(dev_priv) (INTEL_INFO(dev_priv)->has_gt_uc)
-/* Having a GuC is not the same as using a GuC */
+/* Having GuC/HuC is not the same as using GuC/HuC */
#define USES_GUC(dev_priv) intel_uc_is_using_guc(&(dev_priv)->gt.uc)
#define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submission(&(dev_priv)->gt.uc)
#define USES_HUC(dev_priv) intel_uc_is_using_huc(&(dev_priv)->gt.uc)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 2193687eac72..56dfc2650836 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -651,7 +651,7 @@ static void err_print_uc(struct drm_i915_error_state_buf *m,
const struct i915_gpu_state *error =
container_of(error_uc, typeof(*error), uc);
- if (!error->device_info.has_guc)
+ if (!error->device_info.has_gt_uc)
return;
intel_uc_fw_dump(&error_uc->guc_fw, &p);
@@ -1455,7 +1455,7 @@ capture_uc_state(struct i915_gpu_state *error, struct compress *compress)
struct intel_uc *uc = &i915->gt.uc;
/* Capturing uC state won't be useful if there is no GuC */
- if (!error->device_info.has_guc)
+ if (!error->device_info.has_gt_uc)
return;
error_uc->guc_fw = uc->guc.fw;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 11c73af92597..a17d4fd17962 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4766,7 +4766,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
dev_priv->l3_parity.remap_info[i] = NULL;
/* pre-gen11 the guc irqs bits are in the upper 16 bits of the pm reg */
- if (HAS_GUC_SCHED(dev_priv) && INTEL_GEN(dev_priv) < 11)
+ if (HAS_GT_UC(dev_priv) && INTEL_GEN(dev_priv) < 11)
dev_priv->gt.pm_guc_events = GUC_INTR_GUC2HOST << 16;
/* Let's track the enabled rps events */
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 40076ba431d4..bd9211b3d76e 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -595,7 +595,7 @@ static const struct intel_device_info intel_cherryview_info = {
GEN9_DEFAULT_PAGE_SIZES, \
.has_logical_ring_preemption = 1, \
.display.has_csr = 1, \
- .has_guc = 1, \
+ .has_gt_uc = 1, \
.display.has_ipc = 1, \
.ddb_size = 896
@@ -647,7 +647,7 @@ static const struct intel_device_info intel_skylake_gt4_info = {
.display.has_dp_mst = 1, \
.has_logical_ring_contexts = 1, \
.has_logical_ring_preemption = 1, \
- .has_guc = 1, \
+ .has_gt_uc = 1, \
.ppgtt_type = INTEL_PPGTT_FULL, \
.ppgtt_size = 48, \
.has_reset_engine = 1, \
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 45a9badc9b8e..4f58e8d71b67 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -112,7 +112,7 @@ enum intel_ppgtt_type {
func(gpu_reset_clobbers_display); \
func(has_reset_engine); \
func(has_fpga_dbg); \
- func(has_guc); \
+ func(has_gt_uc); \
func(has_l3_dpf); \
func(has_llc); \
func(has_logical_ring_contexts); \
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 22472f2bd31b..30399b245f07 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7162,7 +7162,7 @@ static void gen11_enable_rc6(struct drm_i915_private *dev_priv)
for_each_engine(engine, dev_priv, id)
I915_WRITE(RING_MAX_IDLE(engine->mmio_base), 10);
- if (HAS_GUC(dev_priv))
+ if (HAS_GT_UC(dev_priv))
I915_WRITE(GUC_MAX_IDLE_COUNT, 0xA);
I915_WRITE(GEN6_RC_SLEEP, 0);
@@ -7243,7 +7243,7 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
for_each_engine(engine, dev_priv, id)
I915_WRITE(RING_MAX_IDLE(engine->mmio_base), 10);
- if (HAS_GUC(dev_priv))
+ if (HAS_GT_UC(dev_priv))
I915_WRITE(GUC_MAX_IDLE_COUNT, 0xA);
I915_WRITE(GEN6_RC_SLEEP, 0);
diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index fafd4e6a1147..0e86a9e85b49 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -74,7 +74,7 @@ void intel_wopcm_init_early(struct intel_wopcm *wopcm)
{
struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
- if (!HAS_GUC(i915))
+ if (!HAS_GT_UC(i915))
return;
if (INTEL_GEN(i915) >= 11)
@@ -263,7 +263,7 @@ int intel_wopcm_init_hw(struct intel_wopcm *wopcm, struct intel_gt *gt)
if (!USES_GUC(i915))
return 0;
- GEM_BUG_ON(!HAS_GUC(i915));
+ GEM_BUG_ON(!HAS_GT_UC(i915));
GEM_BUG_ON(!wopcm->guc.size);
GEM_BUG_ON(!wopcm->guc.base);
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 2/8] drm/i915: Fix handling of non-supported uC
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
2019-07-25 0:18 ` [PATCH v3 1/8] drm/i915/uc: Unify uC platform check Daniele Ceraolo Spurio
@ 2019-07-25 0:18 ` Daniele Ceraolo Spurio
2019-07-25 5:51 ` Michal Wajdeczko
2019-07-25 0:18 ` [PATCH v3 3/8] drm/i915/uc: Unify uC FW selection Daniele Ceraolo Spurio
` (8 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-07-25 0:18 UTC (permalink / raw)
To: intel-gfx
There are 2 issues around handling of missing uC support:
- We treat lack of uC HW and lack of uC FW definition as 2 different
cases, but both of them mean that we don't support the uC on the
platform we're running on.
- We rely on the modparam to decide if we can take uC paths or not, but
we don't sanitize it if it is set incorrectly on platform with no uC
support.
To fix both of them, unify the 2 cases in a single one and sanitize the
modparam on invalid configuration (after printing an error message).
The log has been adapted as well, since the user doesn't care why we
don't support GuC/HuC (no HW or no FW), just that we do not. Developers
can easily find the answer based on the platform, so we can simplify the
log.
Correcting the modparam has been preferred over failing the load since
this is what we usually do for non-supported feature (e.g. the now gone
enable_ppgtt would fall back to the highest supported PPGTT mode if the
selected one was not available).
Note that this patch purposely doesn't change the behavior for platforms
that do have uC support, in which case we will still fail if enable_guc
is set and the firmware is not available on the system.
Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 11 ++++---
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 11 ++++---
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 37 ++++++++++++-----------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 8 -----
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 5 ---
5 files changed, 31 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 87169e826747..17ce78240cf8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -80,12 +80,10 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw)
GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
- if (!HAS_GT_UC(i915)) {
- guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
- return;
- }
+ guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
- guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+ if (!HAS_GT_UC(i915))
+ return;
if (i915_modparams.guc_firmware_path) {
guc_fw->path = i915_modparams.guc_firmware_path;
@@ -112,6 +110,9 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw)
guc_fw->major_ver_wanted = SKL_GUC_FW_MAJOR;
guc_fw->minor_ver_wanted = SKL_GUC_FW_MINOR;
}
+
+ if (guc_fw->path)
+ guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
}
/**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index ff6f7b157ecb..c3a7bd57fb55 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -74,12 +74,10 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw)
GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
- if (!HAS_GT_UC(dev_priv)) {
- huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
- return;
- }
+ huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
- huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+ if (!HAS_GT_UC(dev_priv))
+ return;
if (i915_modparams.huc_firmware_path) {
huc_fw->path = i915_modparams.huc_firmware_path;
@@ -106,6 +104,9 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw)
huc_fw->major_ver_wanted = ICL_HUC_FW_MAJOR;
huc_fw->minor_ver_wanted = ICL_HUC_FW_MINOR;
}
+
+ if (huc_fw->path)
+ huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
}
/**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index bdb171c3f36e..3f672ea7456b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -68,7 +68,7 @@ static int __get_platform_enable_guc(struct intel_uc *uc)
if (INTEL_GEN(uc_to_gt(uc)->i915) < 11)
return 0;
- if (intel_uc_fw_is_selected(guc_fw) && intel_uc_fw_is_selected(huc_fw))
+ if (intel_uc_fw_supported(guc_fw) && intel_uc_fw_supported(huc_fw))
enable_guc |= ENABLE_GUC_LOAD_HUC;
return enable_guc;
@@ -123,26 +123,28 @@ static void sanitize_options_early(struct intel_uc *uc)
yesno(intel_uc_is_using_huc(uc)));
/* Verify GuC firmware availability */
- if (intel_uc_is_using_guc(uc) && !intel_uc_fw_is_selected(guc_fw)) {
- DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
- "enable_guc", i915_modparams.enable_guc,
- !intel_uc_fw_supported(guc_fw) ?
- "no GuC hardware" : "no GuC firmware");
+ if (intel_uc_is_using_guc(uc) && !intel_uc_fw_supported(guc_fw)) {
+ DRM_WARN("Incompatible option detected: enable_guc=%d, "
+ "but GuC is not supported!\n",
+ i915_modparams.enable_guc);
+ DRM_INFO("Disabling GuC/HuC loading!\n");
+ i915_modparams.enable_guc = 0;
}
/* Verify HuC firmware availability */
- if (intel_uc_is_using_huc(uc) && !intel_uc_fw_is_selected(huc_fw)) {
- DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
- "enable_guc", i915_modparams.enable_guc,
- !intel_uc_fw_supported(huc_fw) ?
- "no HuC hardware" : "no HuC firmware");
+ if (intel_uc_is_using_huc(uc) && !intel_uc_fw_supported(huc_fw)) {
+ DRM_WARN("Incompatible option detected: enable_guc=%d, "
+ "but HuC is not supported!\n",
+ i915_modparams.enable_guc);
+ DRM_INFO("Disabling HuC loading!\n");
+ i915_modparams.enable_guc &= ~ENABLE_GUC_LOAD_HUC;
}
/* XXX: GuC submission is unavailable for now */
if (intel_uc_is_using_guc_submission(uc)) {
- DRM_INFO("Incompatible option detected: %s=%d, %s!\n",
- "enable_guc", i915_modparams.enable_guc,
- "GuC submission not supported");
+ DRM_INFO("Incompatible option detected: enable_guc=%d, "
+ "but GuC submission is not supported!\n",
+ i915_modparams.enable_guc);
DRM_INFO("Switching to non-GuC submission mode!\n");
i915_modparams.enable_guc &= ~ENABLE_GUC_SUBMISSION;
}
@@ -153,10 +155,9 @@ static void sanitize_options_early(struct intel_uc *uc)
__get_default_guc_log_level(uc);
if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc(uc)) {
- DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
- "guc_log_level", i915_modparams.guc_log_level,
- !intel_uc_fw_supported(guc_fw) ?
- "no GuC hardware" : "GuC not enabled");
+ DRM_WARN("Incompatible option detected: guc_log_level=%d, "
+ "but GuC is not enabled!\n",
+ i915_modparams.guc_log_level);
i915_modparams.guc_log_level = 0;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 8ce7210907c0..432b632b04c0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -49,14 +49,6 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
- if (!uc_fw->path) {
- dev_info(dev_priv->drm.dev,
- "%s: No firmware was defined for %s!\n",
- intel_uc_fw_type_repr(uc_fw->type),
- intel_platform_name(INTEL_INFO(dev_priv)->platform));
- return;
- }
-
DRM_DEBUG_DRIVER("%s fw fetch %s\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 833d04d06576..55ac9eeab440 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -125,11 +125,6 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
uc_fw->type = type;
}
-static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw)
-{
- return uc_fw->path != NULL;
-}
-
static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
{
return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 2/8] drm/i915: Fix handling of non-supported uC
2019-07-25 0:18 ` [PATCH v3 2/8] drm/i915: Fix handling of non-supported uC Daniele Ceraolo Spurio
@ 2019-07-25 5:51 ` Michal Wajdeczko
0 siblings, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2019-07-25 5:51 UTC (permalink / raw)
To: intel-gfx, Daniele Ceraolo Spurio
On Thu, 25 Jul 2019 02:18:07 +0200, Daniele Ceraolo Spurio
<daniele.ceraolospurio@intel.com> wrote:
> There are 2 issues around handling of missing uC support:
>
> - We treat lack of uC HW and lack of uC FW definition as 2 different
> cases, but both of them mean that we don't support the uC on the
> platform we're running on.
>
> - We rely on the modparam to decide if we can take uC paths or not, but
> we don't sanitize it if it is set incorrectly on platform with no uC
> support.
>
> To fix both of them, unify the 2 cases in a single one and sanitize the
> modparam on invalid configuration (after printing an error message).
> The log has been adapted as well, since the user doesn't care why we
> don't support GuC/HuC (no HW or no FW), just that we do not. Developers
> can easily find the answer based on the platform, so we can simplify the
> log.
>
> Correcting the modparam has been preferred over failing the load since
> this is what we usually do for non-supported feature (e.g. the now gone
> enable_ppgtt would fall back to the highest supported PPGTT mode if the
> selected one was not available).
>
> Note that this patch purposely doesn't change the behavior for platforms
> that do have uC support, in which case we will still fail if enable_guc
> is set and the firmware is not available on the system.
>
> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Note that more changes are planned around this code,
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 11 ++++---
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 11 ++++---
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 37 ++++++++++++-----------
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 8 -----
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 5 ---
> 5 files changed, 31 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> index 87169e826747..17ce78240cf8 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -80,12 +80,10 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw)
> GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
> - if (!HAS_GT_UC(i915)) {
> - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> - return;
> - }
> + guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> + if (!HAS_GT_UC(i915))
> + return;
> if (i915_modparams.guc_firmware_path) {
> guc_fw->path = i915_modparams.guc_firmware_path;
> @@ -112,6 +110,9 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw)
> guc_fw->major_ver_wanted = SKL_GUC_FW_MAJOR;
> guc_fw->minor_ver_wanted = SKL_GUC_FW_MINOR;
> }
> +
> + if (guc_fw->path)
> + guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> }
> /**
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> index ff6f7b157ecb..c3a7bd57fb55 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -74,12 +74,10 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw)
> GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
> - if (!HAS_GT_UC(dev_priv)) {
> - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> - return;
> - }
> + huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> + if (!HAS_GT_UC(dev_priv))
> + return;
> if (i915_modparams.huc_firmware_path) {
> huc_fw->path = i915_modparams.huc_firmware_path;
> @@ -106,6 +104,9 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw)
> huc_fw->major_ver_wanted = ICL_HUC_FW_MAJOR;
> huc_fw->minor_ver_wanted = ICL_HUC_FW_MINOR;
> }
> +
> + if (huc_fw->path)
> + huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> }
> /**
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index bdb171c3f36e..3f672ea7456b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -68,7 +68,7 @@ static int __get_platform_enable_guc(struct intel_uc
> *uc)
> if (INTEL_GEN(uc_to_gt(uc)->i915) < 11)
> return 0;
> - if (intel_uc_fw_is_selected(guc_fw) && intel_uc_fw_is_selected(huc_fw))
> + if (intel_uc_fw_supported(guc_fw) && intel_uc_fw_supported(huc_fw))
> enable_guc |= ENABLE_GUC_LOAD_HUC;
> return enable_guc;
> @@ -123,26 +123,28 @@ static void sanitize_options_early(struct intel_uc
> *uc)
> yesno(intel_uc_is_using_huc(uc)));
> /* Verify GuC firmware availability */
> - if (intel_uc_is_using_guc(uc) && !intel_uc_fw_is_selected(guc_fw)) {
> - DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
> - "enable_guc", i915_modparams.enable_guc,
> - !intel_uc_fw_supported(guc_fw) ?
> - "no GuC hardware" : "no GuC firmware");
> + if (intel_uc_is_using_guc(uc) && !intel_uc_fw_supported(guc_fw)) {
> + DRM_WARN("Incompatible option detected: enable_guc=%d, "
> + "but GuC is not supported!\n",
> + i915_modparams.enable_guc);
> + DRM_INFO("Disabling GuC/HuC loading!\n");
> + i915_modparams.enable_guc = 0;
> }
> /* Verify HuC firmware availability */
> - if (intel_uc_is_using_huc(uc) && !intel_uc_fw_is_selected(huc_fw)) {
> - DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
> - "enable_guc", i915_modparams.enable_guc,
> - !intel_uc_fw_supported(huc_fw) ?
> - "no HuC hardware" : "no HuC firmware");
> + if (intel_uc_is_using_huc(uc) && !intel_uc_fw_supported(huc_fw)) {
> + DRM_WARN("Incompatible option detected: enable_guc=%d, "
> + "but HuC is not supported!\n",
> + i915_modparams.enable_guc);
> + DRM_INFO("Disabling HuC loading!\n");
> + i915_modparams.enable_guc &= ~ENABLE_GUC_LOAD_HUC;
> }
> /* XXX: GuC submission is unavailable for now */
> if (intel_uc_is_using_guc_submission(uc)) {
> - DRM_INFO("Incompatible option detected: %s=%d, %s!\n",
> - "enable_guc", i915_modparams.enable_guc,
> - "GuC submission not supported");
> + DRM_INFO("Incompatible option detected: enable_guc=%d, "
> + "but GuC submission is not supported!\n",
> + i915_modparams.enable_guc);
> DRM_INFO("Switching to non-GuC submission mode!\n");
> i915_modparams.enable_guc &= ~ENABLE_GUC_SUBMISSION;
> }
> @@ -153,10 +155,9 @@ static void sanitize_options_early(struct intel_uc
> *uc)
> __get_default_guc_log_level(uc);
> if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc(uc)) {
> - DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
> - "guc_log_level", i915_modparams.guc_log_level,
> - !intel_uc_fw_supported(guc_fw) ?
> - "no GuC hardware" : "GuC not enabled");
> + DRM_WARN("Incompatible option detected: guc_log_level=%d, "
> + "but GuC is not enabled!\n",
> + i915_modparams.guc_log_level);
> i915_modparams.guc_log_level = 0;
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index 8ce7210907c0..432b632b04c0 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -49,14 +49,6 @@ void intel_uc_fw_fetch(struct drm_i915_private
> *dev_priv,
> GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
> - if (!uc_fw->path) {
> - dev_info(dev_priv->drm.dev,
> - "%s: No firmware was defined for %s!\n",
> - intel_uc_fw_type_repr(uc_fw->type),
> - intel_platform_name(INTEL_INFO(dev_priv)->platform));
> - return;
> - }
> -
> DRM_DEBUG_DRIVER("%s fw fetch %s\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index 833d04d06576..55ac9eeab440 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -125,11 +125,6 @@ void intel_uc_fw_init_early(struct intel_uc_fw
> *uc_fw,
> uc_fw->type = type;
> }
> -static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw)
> -{
> - return uc_fw->path != NULL;
> -}
> -
> static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
> {
> return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/8] drm/i915/uc: Unify uC FW selection
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
2019-07-25 0:18 ` [PATCH v3 1/8] drm/i915/uc: Unify uC platform check Daniele Ceraolo Spurio
2019-07-25 0:18 ` [PATCH v3 2/8] drm/i915: Fix handling of non-supported uC Daniele Ceraolo Spurio
@ 2019-07-25 0:18 ` Daniele Ceraolo Spurio
2019-07-25 5:54 ` Michal Wajdeczko
2019-07-25 0:18 ` [PATCH v3 4/8] drm/i915/uc: Unify uc_fw status tracking Daniele Ceraolo Spurio
` (7 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-07-25 0:18 UTC (permalink / raw)
To: intel-gfx
Instead of having 2 identical functions for GuC and HuC firmware
selection, we can unify the selection logic and just use different lists
based on FW type.
Note that the revid is not relevant for current blobs, but the upcoming
CML will be identified as CFL rev 5, so by considering the revid we're
ready for that.
v2: rework blob list defs (Michal), add order check (Chris), fuse GuC
and HuC lists into one.
v3: remove difference between no uC HW and no uC FW, simplify related
selection code, check the whole fw list (Michal)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v2
---
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 89 +-----------
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 91 +------------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 156 ++++++++++++++++++++++
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 24 +---
4 files changed, 164 insertions(+), 196 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 17ce78240cf8..99f44d8ae026 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -31,90 +31,6 @@
#include "intel_guc_fw.h"
#include "i915_drv.h"
-#define __MAKE_GUC_FW_PATH(KEY) \
- "i915/" \
- __stringify(KEY##_GUC_FW_PREFIX) "_guc_" \
- __stringify(KEY##_GUC_FW_MAJOR) "." \
- __stringify(KEY##_GUC_FW_MINOR) "." \
- __stringify(KEY##_GUC_FW_PATCH) ".bin"
-
-#define SKL_GUC_FW_PREFIX skl
-#define SKL_GUC_FW_MAJOR 33
-#define SKL_GUC_FW_MINOR 0
-#define SKL_GUC_FW_PATCH 0
-#define SKL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(SKL)
-MODULE_FIRMWARE(SKL_GUC_FIRMWARE_PATH);
-
-#define BXT_GUC_FW_PREFIX bxt
-#define BXT_GUC_FW_MAJOR 33
-#define BXT_GUC_FW_MINOR 0
-#define BXT_GUC_FW_PATCH 0
-#define BXT_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(BXT)
-MODULE_FIRMWARE(BXT_GUC_FIRMWARE_PATH);
-
-#define KBL_GUC_FW_PREFIX kbl
-#define KBL_GUC_FW_MAJOR 33
-#define KBL_GUC_FW_MINOR 0
-#define KBL_GUC_FW_PATCH 0
-#define KBL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(KBL)
-MODULE_FIRMWARE(KBL_GUC_FIRMWARE_PATH);
-
-#define GLK_GUC_FW_PREFIX glk
-#define GLK_GUC_FW_MAJOR 33
-#define GLK_GUC_FW_MINOR 0
-#define GLK_GUC_FW_PATCH 0
-#define GLK_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(GLK)
-MODULE_FIRMWARE(GLK_GUC_FIRMWARE_PATH);
-
-#define ICL_GUC_FW_PREFIX icl
-#define ICL_GUC_FW_MAJOR 33
-#define ICL_GUC_FW_MINOR 0
-#define ICL_GUC_FW_PATCH 0
-#define ICL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(ICL)
-MODULE_FIRMWARE(ICL_GUC_FIRMWARE_PATH);
-
-static void guc_fw_select(struct intel_uc_fw *guc_fw)
-{
- struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
- struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
-
- GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
-
- guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
-
- if (!HAS_GT_UC(i915))
- return;
-
- if (i915_modparams.guc_firmware_path) {
- guc_fw->path = i915_modparams.guc_firmware_path;
- guc_fw->major_ver_wanted = 0;
- guc_fw->minor_ver_wanted = 0;
- } else if (IS_ICELAKE(i915)) {
- guc_fw->path = ICL_GUC_FIRMWARE_PATH;
- guc_fw->major_ver_wanted = ICL_GUC_FW_MAJOR;
- guc_fw->minor_ver_wanted = ICL_GUC_FW_MINOR;
- } else if (IS_GEMINILAKE(i915)) {
- guc_fw->path = GLK_GUC_FIRMWARE_PATH;
- guc_fw->major_ver_wanted = GLK_GUC_FW_MAJOR;
- guc_fw->minor_ver_wanted = GLK_GUC_FW_MINOR;
- } else if (IS_KABYLAKE(i915) || IS_COFFEELAKE(i915)) {
- guc_fw->path = KBL_GUC_FIRMWARE_PATH;
- guc_fw->major_ver_wanted = KBL_GUC_FW_MAJOR;
- guc_fw->minor_ver_wanted = KBL_GUC_FW_MINOR;
- } else if (IS_BROXTON(i915)) {
- guc_fw->path = BXT_GUC_FIRMWARE_PATH;
- guc_fw->major_ver_wanted = BXT_GUC_FW_MAJOR;
- guc_fw->minor_ver_wanted = BXT_GUC_FW_MINOR;
- } else if (IS_SKYLAKE(i915)) {
- guc_fw->path = SKL_GUC_FIRMWARE_PATH;
- guc_fw->major_ver_wanted = SKL_GUC_FW_MAJOR;
- guc_fw->minor_ver_wanted = SKL_GUC_FW_MINOR;
- }
-
- if (guc_fw->path)
- guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
-}
-
/**
* intel_guc_fw_init_early() - initializes GuC firmware struct
* @guc: intel_guc struct
@@ -123,10 +39,7 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw)
*/
void intel_guc_fw_init_early(struct intel_guc *guc)
{
- struct intel_uc_fw *guc_fw = &guc->fw;
-
- intel_uc_fw_init_early(guc_fw, INTEL_UC_FW_TYPE_GUC);
- guc_fw_select(guc_fw);
+ intel_uc_fw_init_early(&guc->fw, INTEL_UC_FW_TYPE_GUC, guc_to_gt(guc)->i915);
}
static void guc_prepare_xfer(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index c3a7bd57fb55..ba2e1a835830 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -23,92 +23,6 @@
* Note that HuC firmware loading must be done before GuC loading.
*/
-#define BXT_HUC_FW_MAJOR 01
-#define BXT_HUC_FW_MINOR 8
-#define BXT_BLD_NUM 2893
-
-#define SKL_HUC_FW_MAJOR 01
-#define SKL_HUC_FW_MINOR 07
-#define SKL_BLD_NUM 1398
-
-#define KBL_HUC_FW_MAJOR 02
-#define KBL_HUC_FW_MINOR 00
-#define KBL_BLD_NUM 1810
-
-#define GLK_HUC_FW_MAJOR 03
-#define GLK_HUC_FW_MINOR 01
-#define GLK_BLD_NUM 2893
-
-#define ICL_HUC_FW_MAJOR 8
-#define ICL_HUC_FW_MINOR 4
-#define ICL_BLD_NUM 3238
-
-#define HUC_FW_PATH(platform, major, minor, bld_num) \
- "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
- __stringify(minor) "_" __stringify(bld_num) ".bin"
-
-#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \
- SKL_HUC_FW_MINOR, SKL_BLD_NUM)
-MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
-
-#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \
- BXT_HUC_FW_MINOR, BXT_BLD_NUM)
-MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
-
-#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \
- KBL_HUC_FW_MINOR, KBL_BLD_NUM)
-MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
-
-#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \
- GLK_HUC_FW_MINOR, GLK_BLD_NUM)
-MODULE_FIRMWARE(I915_GLK_HUC_UCODE);
-
-#define I915_ICL_HUC_UCODE HUC_FW_PATH(icl, ICL_HUC_FW_MAJOR, \
- ICL_HUC_FW_MINOR, ICL_BLD_NUM)
-MODULE_FIRMWARE(I915_ICL_HUC_UCODE);
-
-static void huc_fw_select(struct intel_uc_fw *huc_fw)
-{
- struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
- struct drm_i915_private *dev_priv = huc_to_gt(huc)->i915;
-
- GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
-
- huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
-
- if (!HAS_GT_UC(dev_priv))
- return;
-
- if (i915_modparams.huc_firmware_path) {
- huc_fw->path = i915_modparams.huc_firmware_path;
- huc_fw->major_ver_wanted = 0;
- huc_fw->minor_ver_wanted = 0;
- } else if (IS_SKYLAKE(dev_priv)) {
- huc_fw->path = I915_SKL_HUC_UCODE;
- huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
- huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
- } else if (IS_BROXTON(dev_priv)) {
- huc_fw->path = I915_BXT_HUC_UCODE;
- huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
- huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
- } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
- huc_fw->path = I915_KBL_HUC_UCODE;
- huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
- huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
- } else if (IS_GEMINILAKE(dev_priv)) {
- huc_fw->path = I915_GLK_HUC_UCODE;
- huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR;
- huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR;
- } else if (IS_ICELAKE(dev_priv)) {
- huc_fw->path = I915_ICL_HUC_UCODE;
- huc_fw->major_ver_wanted = ICL_HUC_FW_MAJOR;
- huc_fw->minor_ver_wanted = ICL_HUC_FW_MINOR;
- }
-
- if (huc_fw->path)
- huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
-}
-
/**
* intel_huc_fw_init_early() - initializes HuC firmware struct
* @huc: intel_huc struct
@@ -117,10 +31,7 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw)
*/
void intel_huc_fw_init_early(struct intel_huc *huc)
{
- struct intel_uc_fw *huc_fw = &huc->fw;
-
- intel_uc_fw_init_early(huc_fw, INTEL_UC_FW_TYPE_HUC);
- huc_fw_select(huc_fw);
+ intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC, huc_to_gt(huc)->i915);
}
static void huc_xfer_rsa(struct intel_huc *huc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 432b632b04c0..9206d4221789 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -29,6 +29,162 @@
#include "intel_uc_fw.h"
#include "i915_drv.h"
+/*
+ * List of required GuC and HuC binaries per-platform.
+ * Must be ordered based on platform + revid, from newer to older.
+ */
+#define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
+ fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 8, 4, 3238)) \
+ fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01, 2893)) \
+ fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01, 8, 2893)) \
+ fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07, 1398))
+
+#define __MAKE_UC_FW_PATH(prefix_, name_, separator_, major_, minor_, patch_) \
+ "i915/" \
+ __stringify(prefix_) name_ \
+ __stringify(major_) separator_ \
+ __stringify(minor_) separator_ \
+ __stringify(patch_) ".bin"
+
+#define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \
+ __MAKE_UC_FW_PATH(prefix_, "_guc_", ".", major_, minor_, patch_)
+
+#define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \
+ __MAKE_UC_FW_PATH(prefix_, "_huc_ver", "_", major_, minor_, bld_num_)
+
+/* All blobs need to be declared via MODULE_FIRMWARE() */
+#define INTEL_UC_MODULE_FW(platform_, revid_, guc_, huc_) \
+ MODULE_FIRMWARE(guc_); \
+ MODULE_FIRMWARE(huc_);
+
+INTEL_UC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH, MAKE_HUC_FW_PATH)
+
+/* The below structs and macros are used to iterate across the list of blobs */
+struct __packed uc_fw_blob {
+ u8 major;
+ u8 minor;
+ const char *path;
+};
+
+#define UC_FW_BLOB(major_, minor_, path_) \
+ { .major = major_, .minor = minor_, .path = path_ }
+
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB(major_, minor_, \
+ MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
+
+#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \
+ UC_FW_BLOB(major_, minor_, \
+ MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_))
+
+struct __packed uc_fw_platform_requirement {
+ enum intel_platform p;
+ u8 rev; /* first platform rev using this FW */
+ const struct uc_fw_blob blobs[INTEL_UC_FW_NUM_TYPES];
+};
+
+#define MAKE_FW_LIST(platform_, revid_, guc_, huc_) \
+{ \
+ .p = INTEL_##platform_, \
+ .rev = revid_, \
+ .blobs[INTEL_UC_FW_TYPE_GUC] = guc_, \
+ .blobs[INTEL_UC_FW_TYPE_HUC] = huc_, \
+},
+
+static void
+__uc_fw_auto_select(struct intel_uc_fw *uc_fw, enum intel_platform p, u8 rev)
+{
+ static const struct uc_fw_platform_requirement fw_blobs[] = {
+ INTEL_UC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, HUC_FW_BLOB)
+ };
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(fw_blobs) && p <= fw_blobs[i].p; i++) {
+ if (p == fw_blobs[i].p && rev >= fw_blobs[i].rev) {
+ const struct uc_fw_blob *blob =
+ &fw_blobs[i].blobs[uc_fw->type];
+ uc_fw->path = blob->path;
+ uc_fw->major_ver_wanted = blob->major;
+ uc_fw->minor_ver_wanted = blob->minor;
+ break;
+ }
+ }
+
+ /* make sure the list is ordered as expected */
+ if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) {
+ for (i = 1; i < ARRAY_SIZE(fw_blobs); i++) {
+ if (fw_blobs[i].p < fw_blobs[i - 1].p)
+ continue;
+
+ if (fw_blobs[i].p == fw_blobs[i - 1].p &&
+ fw_blobs[i].rev < fw_blobs[i - 1].rev)
+ continue;
+
+ pr_err("invalid FW blob order: %s r%u comes before %s r%u\n",
+ intel_platform_name(fw_blobs[i - 1].p),
+ fw_blobs[i - 1].rev,
+ intel_platform_name(fw_blobs[i].p),
+ fw_blobs[i].rev);
+
+ uc_fw->path = NULL;
+ }
+ }
+}
+
+static bool
+__uc_fw_override(struct intel_uc_fw *uc_fw)
+{
+ switch (uc_fw->type) {
+ case INTEL_UC_FW_TYPE_GUC:
+ uc_fw->path = i915_modparams.guc_firmware_path;
+ break;
+ case INTEL_UC_FW_TYPE_HUC:
+ uc_fw->path = i915_modparams.huc_firmware_path;
+ break;
+ }
+
+ return uc_fw->path;
+}
+
+/**
+ * intel_uc_fw_init_early - initialize the uC object and select the firmware
+ * @i915: device private
+ * @uc_fw: uC firmware
+ * @type: type of uC
+ *
+ * Initialize the state of our uC object and relevant tracking and select the
+ * firmware to fetch and load.
+ */
+void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
+ enum intel_uc_fw_type type,
+ struct drm_i915_private *i915)
+{
+ /*
+ * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status
+ * before we're looked at the HW caps to see if we have uc support
+ */
+ BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED);
+ GEM_BUG_ON(uc_fw->fetch_status);
+ GEM_BUG_ON(uc_fw->load_status);
+ GEM_BUG_ON(uc_fw->path);
+
+ uc_fw->type = type;
+
+ if (HAS_GT_UC(i915) && likely(!__uc_fw_override(uc_fw)))
+ __uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform,
+ INTEL_REVID(i915));
+
+ if (uc_fw->path) {
+ uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+ uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+ } else {
+ uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
+ uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
+ }
+}
+
/**
* intel_uc_fw_fetch - fetch uC firmware
*
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 55ac9eeab440..c93e271917c9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -44,9 +44,10 @@ enum intel_uc_fw_status {
};
enum intel_uc_fw_type {
- INTEL_UC_FW_TYPE_GUC,
+ INTEL_UC_FW_TYPE_GUC = 0,
INTEL_UC_FW_TYPE_HUC
};
+#define INTEL_UC_FW_NUM_TYPES 2
/*
* This structure encapsulates all the data needed during the process
@@ -109,22 +110,6 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
return "uC";
}
-static inline
-void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
- enum intel_uc_fw_type type)
-{
- /*
- * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status
- * before we're looked at the HW caps to see if we have uc support
- */
- BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED);
-
- uc_fw->path = NULL;
- uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED;
- uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED;
- uc_fw->type = type;
-}
-
static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
{
return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
@@ -159,7 +144,10 @@ static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
return uc_fw->header_size + uc_fw->ucode_size;
}
-void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
+void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
+ enum intel_uc_fw_type type,
+ struct drm_i915_private *i915);
+void intel_uc_fw_fetch(struct drm_i915_private *i915,
struct intel_uc_fw *uc_fw);
void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/8] drm/i915/uc: Unify uC FW selection
2019-07-25 0:18 ` [PATCH v3 3/8] drm/i915/uc: Unify uC FW selection Daniele Ceraolo Spurio
@ 2019-07-25 5:54 ` Michal Wajdeczko
0 siblings, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2019-07-25 5:54 UTC (permalink / raw)
To: intel-gfx, Daniele Ceraolo Spurio
On Thu, 25 Jul 2019 02:18:08 +0200, Daniele Ceraolo Spurio
<daniele.ceraolospurio@intel.com> wrote:
> Instead of having 2 identical functions for GuC and HuC firmware
> selection, we can unify the selection logic and just use different lists
> based on FW type.
>
> Note that the revid is not relevant for current blobs, but the upcoming
> CML will be identified as CFL rev 5, so by considering the revid we're
> ready for that.
>
> v2: rework blob list defs (Michal), add order check (Chris), fuse GuC
> and HuC lists into one.
>
> v3: remove difference between no uC HW and no uC FW, simplify related
> selection code, check the whole fw list (Michal)
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v2
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 89 +-----------
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 91 +------------
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 156 ++++++++++++++++++++++
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 24 +---
> 4 files changed, 164 insertions(+), 196 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> index 17ce78240cf8..99f44d8ae026 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -31,90 +31,6 @@
> #include "intel_guc_fw.h"
> #include "i915_drv.h"
> -#define __MAKE_GUC_FW_PATH(KEY) \
> - "i915/" \
> - __stringify(KEY##_GUC_FW_PREFIX) "_guc_" \
> - __stringify(KEY##_GUC_FW_MAJOR) "." \
> - __stringify(KEY##_GUC_FW_MINOR) "." \
> - __stringify(KEY##_GUC_FW_PATCH) ".bin"
> -
> -#define SKL_GUC_FW_PREFIX skl
> -#define SKL_GUC_FW_MAJOR 33
> -#define SKL_GUC_FW_MINOR 0
> -#define SKL_GUC_FW_PATCH 0
> -#define SKL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(SKL)
> -MODULE_FIRMWARE(SKL_GUC_FIRMWARE_PATH);
> -
> -#define BXT_GUC_FW_PREFIX bxt
> -#define BXT_GUC_FW_MAJOR 33
> -#define BXT_GUC_FW_MINOR 0
> -#define BXT_GUC_FW_PATCH 0
> -#define BXT_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(BXT)
> -MODULE_FIRMWARE(BXT_GUC_FIRMWARE_PATH);
> -
> -#define KBL_GUC_FW_PREFIX kbl
> -#define KBL_GUC_FW_MAJOR 33
> -#define KBL_GUC_FW_MINOR 0
> -#define KBL_GUC_FW_PATCH 0
> -#define KBL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(KBL)
> -MODULE_FIRMWARE(KBL_GUC_FIRMWARE_PATH);
> -
> -#define GLK_GUC_FW_PREFIX glk
> -#define GLK_GUC_FW_MAJOR 33
> -#define GLK_GUC_FW_MINOR 0
> -#define GLK_GUC_FW_PATCH 0
> -#define GLK_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(GLK)
> -MODULE_FIRMWARE(GLK_GUC_FIRMWARE_PATH);
> -
> -#define ICL_GUC_FW_PREFIX icl
> -#define ICL_GUC_FW_MAJOR 33
> -#define ICL_GUC_FW_MINOR 0
> -#define ICL_GUC_FW_PATCH 0
> -#define ICL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(ICL)
> -MODULE_FIRMWARE(ICL_GUC_FIRMWARE_PATH);
> -
> -static void guc_fw_select(struct intel_uc_fw *guc_fw)
> -{
> - struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
> - struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> -
> - GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
> -
> - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> -
> - if (!HAS_GT_UC(i915))
> - return;
> -
> - if (i915_modparams.guc_firmware_path) {
> - guc_fw->path = i915_modparams.guc_firmware_path;
> - guc_fw->major_ver_wanted = 0;
> - guc_fw->minor_ver_wanted = 0;
> - } else if (IS_ICELAKE(i915)) {
> - guc_fw->path = ICL_GUC_FIRMWARE_PATH;
> - guc_fw->major_ver_wanted = ICL_GUC_FW_MAJOR;
> - guc_fw->minor_ver_wanted = ICL_GUC_FW_MINOR;
> - } else if (IS_GEMINILAKE(i915)) {
> - guc_fw->path = GLK_GUC_FIRMWARE_PATH;
> - guc_fw->major_ver_wanted = GLK_GUC_FW_MAJOR;
> - guc_fw->minor_ver_wanted = GLK_GUC_FW_MINOR;
> - } else if (IS_KABYLAKE(i915) || IS_COFFEELAKE(i915)) {
> - guc_fw->path = KBL_GUC_FIRMWARE_PATH;
> - guc_fw->major_ver_wanted = KBL_GUC_FW_MAJOR;
> - guc_fw->minor_ver_wanted = KBL_GUC_FW_MINOR;
> - } else if (IS_BROXTON(i915)) {
> - guc_fw->path = BXT_GUC_FIRMWARE_PATH;
> - guc_fw->major_ver_wanted = BXT_GUC_FW_MAJOR;
> - guc_fw->minor_ver_wanted = BXT_GUC_FW_MINOR;
> - } else if (IS_SKYLAKE(i915)) {
> - guc_fw->path = SKL_GUC_FIRMWARE_PATH;
> - guc_fw->major_ver_wanted = SKL_GUC_FW_MAJOR;
> - guc_fw->minor_ver_wanted = SKL_GUC_FW_MINOR;
> - }
> -
> - if (guc_fw->path)
> - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> -}
> -
> /**
> * intel_guc_fw_init_early() - initializes GuC firmware struct
> * @guc: intel_guc struct
> @@ -123,10 +39,7 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw)
> */
> void intel_guc_fw_init_early(struct intel_guc *guc)
> {
> - struct intel_uc_fw *guc_fw = &guc->fw;
> -
> - intel_uc_fw_init_early(guc_fw, INTEL_UC_FW_TYPE_GUC);
> - guc_fw_select(guc_fw);
> + intel_uc_fw_init_early(&guc->fw, INTEL_UC_FW_TYPE_GUC,
> guc_to_gt(guc)->i915);
> }
> static void guc_prepare_xfer(struct intel_guc *guc)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> index c3a7bd57fb55..ba2e1a835830 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -23,92 +23,6 @@
> * Note that HuC firmware loading must be done before GuC loading.
> */
> -#define BXT_HUC_FW_MAJOR 01
> -#define BXT_HUC_FW_MINOR 8
> -#define BXT_BLD_NUM 2893
> -
> -#define SKL_HUC_FW_MAJOR 01
> -#define SKL_HUC_FW_MINOR 07
> -#define SKL_BLD_NUM 1398
> -
> -#define KBL_HUC_FW_MAJOR 02
> -#define KBL_HUC_FW_MINOR 00
> -#define KBL_BLD_NUM 1810
> -
> -#define GLK_HUC_FW_MAJOR 03
> -#define GLK_HUC_FW_MINOR 01
> -#define GLK_BLD_NUM 2893
> -
> -#define ICL_HUC_FW_MAJOR 8
> -#define ICL_HUC_FW_MINOR 4
> -#define ICL_BLD_NUM 3238
> -
> -#define HUC_FW_PATH(platform, major, minor, bld_num) \
> - "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
> - __stringify(minor) "_" __stringify(bld_num) ".bin"
> -
> -#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \
> - SKL_HUC_FW_MINOR, SKL_BLD_NUM)
> -MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
> -
> -#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \
> - BXT_HUC_FW_MINOR, BXT_BLD_NUM)
> -MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
> -
> -#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \
> - KBL_HUC_FW_MINOR, KBL_BLD_NUM)
> -MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
> -
> -#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \
> - GLK_HUC_FW_MINOR, GLK_BLD_NUM)
> -MODULE_FIRMWARE(I915_GLK_HUC_UCODE);
> -
> -#define I915_ICL_HUC_UCODE HUC_FW_PATH(icl, ICL_HUC_FW_MAJOR, \
> - ICL_HUC_FW_MINOR, ICL_BLD_NUM)
> -MODULE_FIRMWARE(I915_ICL_HUC_UCODE);
> -
> -static void huc_fw_select(struct intel_uc_fw *huc_fw)
> -{
> - struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
> - struct drm_i915_private *dev_priv = huc_to_gt(huc)->i915;
> -
> - GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
> -
> - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> -
> - if (!HAS_GT_UC(dev_priv))
> - return;
> -
> - if (i915_modparams.huc_firmware_path) {
> - huc_fw->path = i915_modparams.huc_firmware_path;
> - huc_fw->major_ver_wanted = 0;
> - huc_fw->minor_ver_wanted = 0;
> - } else if (IS_SKYLAKE(dev_priv)) {
> - huc_fw->path = I915_SKL_HUC_UCODE;
> - huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
> - huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
> - } else if (IS_BROXTON(dev_priv)) {
> - huc_fw->path = I915_BXT_HUC_UCODE;
> - huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
> - huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
> - } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> - huc_fw->path = I915_KBL_HUC_UCODE;
> - huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
> - huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
> - } else if (IS_GEMINILAKE(dev_priv)) {
> - huc_fw->path = I915_GLK_HUC_UCODE;
> - huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR;
> - huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR;
> - } else if (IS_ICELAKE(dev_priv)) {
> - huc_fw->path = I915_ICL_HUC_UCODE;
> - huc_fw->major_ver_wanted = ICL_HUC_FW_MAJOR;
> - huc_fw->minor_ver_wanted = ICL_HUC_FW_MINOR;
> - }
> -
> - if (huc_fw->path)
> - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> -}
> -
> /**
> * intel_huc_fw_init_early() - initializes HuC firmware struct
> * @huc: intel_huc struct
> @@ -117,10 +31,7 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw)
> */
> void intel_huc_fw_init_early(struct intel_huc *huc)
> {
> - struct intel_uc_fw *huc_fw = &huc->fw;
> -
> - intel_uc_fw_init_early(huc_fw, INTEL_UC_FW_TYPE_HUC);
> - huc_fw_select(huc_fw);
> + intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC,
> huc_to_gt(huc)->i915);
> }
> static void huc_xfer_rsa(struct intel_huc *huc)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index 432b632b04c0..9206d4221789 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -29,6 +29,162 @@
> #include "intel_uc_fw.h"
> #include "i915_drv.h"
> +/*
> + * List of required GuC and HuC binaries per-platform.
> + * Must be ordered based on platform + revid, from newer to older.
> + */
> +#define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
> + fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 8, 4,
> 3238)) \
> + fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00,
> 1810)) \
> + fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01,
> 2893)) \
> + fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00,
> 1810)) \
> + fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01, 8,
> 2893)) \
> + fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07,
> 1398))
> +
> +#define __MAKE_UC_FW_PATH(prefix_, name_, separator_, major_, minor_,
> patch_) \
> + "i915/" \
> + __stringify(prefix_) name_ \
> + __stringify(major_) separator_ \
> + __stringify(minor_) separator_ \
> + __stringify(patch_) ".bin"
> +
> +#define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \
> + __MAKE_UC_FW_PATH(prefix_, "_guc_", ".", major_, minor_, patch_)
> +
> +#define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \
> + __MAKE_UC_FW_PATH(prefix_, "_huc_ver", "_", major_, minor_, bld_num_)
> +
> +/* All blobs need to be declared via MODULE_FIRMWARE() */
> +#define INTEL_UC_MODULE_FW(platform_, revid_, guc_, huc_) \
> + MODULE_FIRMWARE(guc_); \
> + MODULE_FIRMWARE(huc_);
> +
> +INTEL_UC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH,
> MAKE_HUC_FW_PATH)
> +
> +/* The below structs and macros are used to iterate across the list of
> blobs */
> +struct __packed uc_fw_blob {
> + u8 major;
> + u8 minor;
> + const char *path;
> +};
> +
> +#define UC_FW_BLOB(major_, minor_, path_) \
> + { .major = major_, .minor = minor_, .path = path_ }
> +
> +#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
> + UC_FW_BLOB(major_, minor_, \
> + MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
> +
> +#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \
> + UC_FW_BLOB(major_, minor_, \
> + MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_))
> +
> +struct __packed uc_fw_platform_requirement {
> + enum intel_platform p;
> + u8 rev; /* first platform rev using this FW */
> + const struct uc_fw_blob blobs[INTEL_UC_FW_NUM_TYPES];
> +};
> +
> +#define MAKE_FW_LIST(platform_, revid_, guc_, huc_) \
> +{ \
> + .p = INTEL_##platform_, \
> + .rev = revid_, \
> + .blobs[INTEL_UC_FW_TYPE_GUC] = guc_, \
> + .blobs[INTEL_UC_FW_TYPE_HUC] = huc_, \
> +},
> +
> +static void
> +__uc_fw_auto_select(struct intel_uc_fw *uc_fw, enum intel_platform p,
> u8 rev)
> +{
> + static const struct uc_fw_platform_requirement fw_blobs[] = {
> + INTEL_UC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, HUC_FW_BLOB)
> + };
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(fw_blobs) && p <= fw_blobs[i].p; i++) {
> + if (p == fw_blobs[i].p && rev >= fw_blobs[i].rev) {
> + const struct uc_fw_blob *blob =
> + &fw_blobs[i].blobs[uc_fw->type];
> + uc_fw->path = blob->path;
> + uc_fw->major_ver_wanted = blob->major;
> + uc_fw->minor_ver_wanted = blob->minor;
> + break;
> + }
> + }
> +
> + /* make sure the list is ordered as expected */
> + if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) {
> + for (i = 1; i < ARRAY_SIZE(fw_blobs); i++) {
> + if (fw_blobs[i].p < fw_blobs[i - 1].p)
> + continue;
> +
> + if (fw_blobs[i].p == fw_blobs[i - 1].p &&
> + fw_blobs[i].rev < fw_blobs[i - 1].rev)
> + continue;
> +
> + pr_err("invalid FW blob order: %s r%u comes before %s r%u\n",
> + intel_platform_name(fw_blobs[i - 1].p),
> + fw_blobs[i - 1].rev,
> + intel_platform_name(fw_blobs[i].p),
> + fw_blobs[i].rev);
> +
> + uc_fw->path = NULL;
> + }
> + }
I feel little uncomfortable that we don't check the fw blob list before
doing actual fw selection, but since it recovers correctly:
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> +}
> +
> +static bool
> +__uc_fw_override(struct intel_uc_fw *uc_fw)
> +{
> + switch (uc_fw->type) {
> + case INTEL_UC_FW_TYPE_GUC:
> + uc_fw->path = i915_modparams.guc_firmware_path;
> + break;
> + case INTEL_UC_FW_TYPE_HUC:
> + uc_fw->path = i915_modparams.huc_firmware_path;
> + break;
> + }
> +
> + return uc_fw->path;
> +}
> +
> +/**
> + * intel_uc_fw_init_early - initialize the uC object and select the
> firmware
> + * @i915: device private
> + * @uc_fw: uC firmware
> + * @type: type of uC
> + *
> + * Initialize the state of our uC object and relevant tracking and
> select the
> + * firmware to fetch and load.
> + */
> +void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
> + enum intel_uc_fw_type type,
> + struct drm_i915_private *i915)
> +{
> + /*
> + * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status
> + * before we're looked at the HW caps to see if we have uc support
> + */
> + BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED);
> + GEM_BUG_ON(uc_fw->fetch_status);
> + GEM_BUG_ON(uc_fw->load_status);
> + GEM_BUG_ON(uc_fw->path);
> +
> + uc_fw->type = type;
> +
> + if (HAS_GT_UC(i915) && likely(!__uc_fw_override(uc_fw)))
> + __uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform,
> + INTEL_REVID(i915));
> +
> + if (uc_fw->path) {
> + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> + uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> + } else {
> + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> + uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> + }
> +}
> +
> /**
> * intel_uc_fw_fetch - fetch uC firmware
> *
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index 55ac9eeab440..c93e271917c9 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -44,9 +44,10 @@ enum intel_uc_fw_status {
> };
> enum intel_uc_fw_type {
> - INTEL_UC_FW_TYPE_GUC,
> + INTEL_UC_FW_TYPE_GUC = 0,
> INTEL_UC_FW_TYPE_HUC
> };
> +#define INTEL_UC_FW_NUM_TYPES 2
> /*
> * This structure encapsulates all the data needed during the process
> @@ -109,22 +110,6 @@ static inline const char
> *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
> return "uC";
> }
> -static inline
> -void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
> - enum intel_uc_fw_type type)
> -{
> - /*
> - * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status
> - * before we're looked at the HW caps to see if we have uc support
> - */
> - BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED);
> -
> - uc_fw->path = NULL;
> - uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED;
> - uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> - uc_fw->type = type;
> -}
> -
> static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
> {
> return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
> @@ -159,7 +144,10 @@ static inline u32
> intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
> return uc_fw->header_size + uc_fw->ucode_size;
> }
> -void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> +void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
> + enum intel_uc_fw_type type,
> + struct drm_i915_private *i915);
> +void intel_uc_fw_fetch(struct drm_i915_private *i915,
> struct intel_uc_fw *uc_fw);
> void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
> int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 4/8] drm/i915/uc: Unify uc_fw status tracking
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
` (2 preceding siblings ...)
2019-07-25 0:18 ` [PATCH v3 3/8] drm/i915/uc: Unify uC FW selection Daniele Ceraolo Spurio
@ 2019-07-25 0:18 ` Daniele Ceraolo Spurio
2019-07-25 0:18 ` [PATCH v3 5/8] drm/i915/uc: Move xfer rsa logic to common function Daniele Ceraolo Spurio
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-07-25 0:18 UTC (permalink / raw)
To: intel-gfx
We currently track fetch and load status separately, but the 2 are
actually sequential in the uc lifetime (fetch must complete before we
can attempt the load!). Unifying the 2 variables we can better follow
the sequential states and improve our trackng of the uC state.
Also, sprinkle some GEM_BUG_ON to make sure we transition correctly
between states.
v2: rename states, add the running state (Michal), drop some logs in
the fetch path (Michal, Chris)
v3: re-rename states, extend early status check to all helpers (Michal)
Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 4 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 6 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 8 +-
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 ++
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 10 +--
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 78 +++++++------------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 63 ++++++++++-----
8 files changed, 92 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index ac6333ad7102..714e9892aaff 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -172,9 +172,9 @@ int intel_guc_suspend(struct intel_guc *guc);
int intel_guc_resume(struct intel_guc *guc);
struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-static inline bool intel_guc_is_loaded(struct intel_guc *guc)
+static inline bool intel_guc_is_running(struct intel_guc *guc)
{
- return intel_uc_fw_is_loaded(&guc->fw);
+ return intel_uc_fw_is_running(&guc->fw);
}
static inline int intel_guc_sanitize(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 99f44d8ae026..eec767383e92 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -230,5 +230,9 @@ static int guc_fw_xfer(struct intel_uc_fw *guc_fw)
*/
int intel_guc_fw_upload(struct intel_guc *guc)
{
- return intel_uc_fw_upload(&guc->fw, guc_fw_xfer);
+ int ret = intel_uc_fw_upload(&guc->fw, guc_fw_xfer);
+ if (!ret)
+ guc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
+
+ return ret;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0f2a01365bc..b4238fe16a03 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -941,7 +941,7 @@ static void __guc_client_disable(struct intel_guc_client *client)
* the case, instead of trying (in vain) to communicate with it, let's
* just cleanup the doorbell HW and our internal state.
*/
- if (intel_guc_is_loaded(client->guc))
+ if (intel_guc_is_running(client->guc))
destroy_doorbell(client);
else
__fini_doorbell(client);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index ab6c1564b6a7..a45976e56af7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -117,8 +117,8 @@ int intel_huc_auth(struct intel_huc *huc)
struct intel_guc *guc = >->uc.guc;
int ret;
- if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
- return -ENOEXEC;
+ GEM_BUG_ON(!intel_uc_fw_is_loaded(&huc->fw));
+ GEM_BUG_ON(intel_huc_is_authenticated(huc));
ret = intel_guc_auth_huc(guc,
intel_guc_ggtt_offset(guc, huc->rsa_data));
@@ -138,10 +138,12 @@ int intel_huc_auth(struct intel_huc *huc)
goto fail;
}
+ huc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
+
return 0;
fail:
- huc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
+ huc->fw.status = INTEL_UC_FIRMWARE_FAIL;
DRM_ERROR("HuC: Authentication failed %d\n", ret);
return ret;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 9fa3d4629f2e..ea340f85bc46 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -56,4 +56,9 @@ static inline int intel_huc_sanitize(struct intel_huc *huc)
return 0;
}
+static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
+{
+ return intel_uc_fw_is_running(&huc->fw);
+}
+
#endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 3f672ea7456b..b1815abecf30 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -560,7 +560,7 @@ void intel_uc_fini_hw(struct intel_uc *uc)
{
struct intel_guc *guc = &uc->guc;
- if (!intel_guc_is_loaded(guc))
+ if (!intel_guc_is_running(guc))
return;
GEM_BUG_ON(!intel_uc_fw_supported(&guc->fw));
@@ -582,7 +582,7 @@ void intel_uc_reset_prepare(struct intel_uc *uc)
{
struct intel_guc *guc = &uc->guc;
- if (!intel_guc_is_loaded(guc))
+ if (!intel_guc_is_running(guc))
return;
guc_stop_communication(guc);
@@ -594,7 +594,7 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
struct intel_guc *guc = &uc->guc;
int err;
- if (!intel_guc_is_loaded(guc))
+ if (!intel_guc_is_running(guc))
return;
err = intel_guc_suspend(guc);
@@ -609,7 +609,7 @@ void intel_uc_suspend(struct intel_uc *uc)
struct intel_guc *guc = &uc->guc;
intel_wakeref_t wakeref;
- if (!intel_guc_is_loaded(guc))
+ if (!intel_guc_is_running(guc))
return;
with_intel_runtime_pm(&uc_to_gt(uc)->i915->runtime_pm, wakeref)
@@ -621,7 +621,7 @@ int intel_uc_resume(struct intel_uc *uc)
struct intel_guc *guc = &uc->guc;
int err;
- if (!intel_guc_is_loaded(guc))
+ if (!intel_guc_is_running(guc))
return 0;
guc_enable_communication(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 9206d4221789..1e7df2c19265 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -162,12 +162,11 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
struct drm_i915_private *i915)
{
/*
- * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status
+ * we use FIRMWARE_UNINITIALIZED to detect checks against uc_fw->status
* before we're looked at the HW caps to see if we have uc support
*/
BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED);
- GEM_BUG_ON(uc_fw->fetch_status);
- GEM_BUG_ON(uc_fw->load_status);
+ GEM_BUG_ON(uc_fw->status);
GEM_BUG_ON(uc_fw->path);
uc_fw->type = type;
@@ -176,13 +175,10 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
__uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform,
INTEL_REVID(i915));
- if (uc_fw->path) {
- uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
- uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED;
- } else {
- uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
- uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
- }
+ if (uc_fw->path)
+ uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
+ else
+ uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
}
/**
@@ -205,20 +201,9 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
- DRM_DEBUG_DRIVER("%s fw fetch %s\n",
- intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
-
- uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
- DRM_DEBUG_DRIVER("%s fw fetch %s\n",
- intel_uc_fw_type_repr(uc_fw->type),
- intel_uc_fw_status_repr(uc_fw->fetch_status));
-
err = request_firmware(&fw, uc_fw->path, &pdev->dev);
- if (err) {
- DRM_DEBUG_DRIVER("%s fw request_firmware err=%d\n",
- intel_uc_fw_type_repr(uc_fw->type), err);
+ if (err)
goto fail;
- }
DRM_DEBUG_DRIVER("%s fw size %zu ptr %p\n",
intel_uc_fw_type_repr(uc_fw->type), fw->size, fw);
@@ -320,19 +305,13 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
uc_fw->obj = obj;
uc_fw->size = fw->size;
- uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
- DRM_DEBUG_DRIVER("%s fw fetch %s\n",
- intel_uc_fw_type_repr(uc_fw->type),
- intel_uc_fw_status_repr(uc_fw->fetch_status));
+ uc_fw->status = INTEL_UC_FIRMWARE_AVAILABLE;
release_firmware(fw);
return;
fail:
- uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
- DRM_DEBUG_DRIVER("%s fw fetch %s\n",
- intel_uc_fw_type_repr(uc_fw->type),
- intel_uc_fw_status_repr(uc_fw->fetch_status));
+ uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
@@ -388,14 +367,11 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
DRM_DEBUG_DRIVER("%s fw load %s\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
- if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
- return -ENOEXEC;
-
- uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
- DRM_DEBUG_DRIVER("%s fw load %s\n",
- intel_uc_fw_type_repr(uc_fw->type),
- intel_uc_fw_status_repr(uc_fw->load_status));
+ /* make sure the status was cleared the last time we reset the uc */
+ GEM_BUG_ON(intel_uc_fw_is_loaded(uc_fw));
+ if (!intel_uc_fw_is_available(uc_fw))
+ return -ENOEXEC;
/* Call custom loader */
intel_uc_fw_ggtt_bind(uc_fw);
err = xfer(uc_fw);
@@ -403,10 +379,9 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
if (err)
goto fail;
- uc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
- DRM_DEBUG_DRIVER("%s fw load %s\n",
- intel_uc_fw_type_repr(uc_fw->type),
- intel_uc_fw_status_repr(uc_fw->load_status));
+ uc_fw->status = INTEL_UC_FIRMWARE_TRANSFERRED;
+ DRM_DEBUG_DRIVER("%s fw xfer completed\n",
+ intel_uc_fw_type_repr(uc_fw->type));
DRM_INFO("%s: Loaded firmware %s (version %u.%u)\n",
intel_uc_fw_type_repr(uc_fw->type),
@@ -416,10 +391,9 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
return 0;
fail:
- uc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
- DRM_DEBUG_DRIVER("%s fw load %s\n",
- intel_uc_fw_type_repr(uc_fw->type),
- intel_uc_fw_status_repr(uc_fw->load_status));
+ uc_fw->status = INTEL_UC_FIRMWARE_FAIL;
+ DRM_DEBUG_DRIVER("%s fw load failed\n",
+ intel_uc_fw_type_repr(uc_fw->type));
DRM_WARN("%s: Failed to load firmware %s (error %d)\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
@@ -431,7 +405,10 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
{
int err;
- if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+ /* this should happen before the load! */
+ GEM_BUG_ON(intel_uc_fw_is_loaded(uc_fw));
+
+ if (!intel_uc_fw_is_available(uc_fw))
return -ENOEXEC;
err = i915_gem_object_pin_pages(uc_fw->obj);
@@ -444,7 +421,7 @@ int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
{
- if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+ if (!intel_uc_fw_is_available(uc_fw))
return;
i915_gem_object_unpin_pages(uc_fw->obj);
@@ -478,7 +455,7 @@ void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw)
if (obj)
i915_gem_object_put(obj);
- uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
+ uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
}
/**
@@ -492,9 +469,8 @@ void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p)
{
drm_printf(p, "%s firmware: %s\n",
intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
- drm_printf(p, "\tstatus: fetch %s, load %s\n",
- intel_uc_fw_status_repr(uc_fw->fetch_status),
- intel_uc_fw_status_repr(uc_fw->load_status));
+ drm_printf(p, "\tstatus: %s\n",
+ intel_uc_fw_status_repr(uc_fw->status));
drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n",
uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted,
uc_fw->major_ver_found, uc_fw->minor_ver_found);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index c93e271917c9..f6aa2e3e4d1f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -35,12 +35,14 @@ struct drm_i915_private;
#define INTEL_UC_FIRMWARE_URL "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915"
enum intel_uc_fw_status {
- INTEL_UC_FIRMWARE_NOT_SUPPORTED = -2, /* no uc HW */
- INTEL_UC_FIRMWARE_FAIL = -1,
+ INTEL_UC_FIRMWARE_FAIL = -3, /* failed to xfer or init/auth the fw */
+ INTEL_UC_FIRMWARE_MISSING = -2, /* blob not found on the system */
+ INTEL_UC_FIRMWARE_NOT_SUPPORTED = -1, /* no uc HW */
INTEL_UC_FIRMWARE_UNINITIALIZED = 0, /* used to catch checks done too early */
- INTEL_UC_FIRMWARE_NOT_STARTED = 1,
- INTEL_UC_FIRMWARE_PENDING,
- INTEL_UC_FIRMWARE_SUCCESS
+ INTEL_UC_FIRMWARE_SELECTED, /* selected the blob we want to load */
+ INTEL_UC_FIRMWARE_AVAILABLE, /* blob found and copied in mem */
+ INTEL_UC_FIRMWARE_TRANSFERRED, /* dma xfer done */
+ INTEL_UC_FIRMWARE_RUNNING /* init/auth done */
};
enum intel_uc_fw_type {
@@ -57,8 +59,7 @@ struct intel_uc_fw {
const char *path;
size_t size;
struct drm_i915_gem_object *obj;
- enum intel_uc_fw_status fetch_status;
- enum intel_uc_fw_status load_status;
+ enum intel_uc_fw_status status;
/*
* The firmware build process will generate a version header file with major and
@@ -83,18 +84,22 @@ static inline
const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
{
switch (status) {
- case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
- return "N/A - uc HW not available";
case INTEL_UC_FIRMWARE_FAIL:
return "FAIL";
+ case INTEL_UC_FIRMWARE_MISSING:
+ return "MISSING";
+ case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
+ return "N/A";
case INTEL_UC_FIRMWARE_UNINITIALIZED:
return "UNINITIALIZED";
- case INTEL_UC_FIRMWARE_NOT_STARTED:
- return "NOT_STARTED";
- case INTEL_UC_FIRMWARE_PENDING:
- return "PENDING";
- case INTEL_UC_FIRMWARE_SUCCESS:
- return "SUCCESS";
+ case INTEL_UC_FIRMWARE_SELECTED:
+ return "SELECTED";
+ case INTEL_UC_FIRMWARE_AVAILABLE:
+ return "AVAILABLE";
+ case INTEL_UC_FIRMWARE_TRANSFERRED:
+ return "TRANSFERRED";
+ case INTEL_UC_FIRMWARE_RUNNING:
+ return "RUNNING";
}
return "<invalid>";
}
@@ -110,22 +115,38 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
return "uC";
}
+static inline enum intel_uc_fw_status
+__intel_uc_fw_status(struct intel_uc_fw *uc_fw)
+{
+ /* shouldn't call this before checking hw/blob availability */
+ GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);
+ return uc_fw->status;
+}
+
+static inline bool intel_uc_fw_is_available(struct intel_uc_fw *uc_fw)
+{
+ return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_AVAILABLE;
+}
+
static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
{
- return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
+ return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_TRANSFERRED;
+}
+
+static inline bool intel_uc_fw_is_running(struct intel_uc_fw *uc_fw)
+{
+ return __intel_uc_fw_status(uc_fw) == INTEL_UC_FIRMWARE_RUNNING;
}
static inline bool intel_uc_fw_supported(struct intel_uc_fw *uc_fw)
{
- /* shouldn't call this before checking hw/blob availability */
- GEM_BUG_ON(uc_fw->fetch_status == INTEL_UC_FIRMWARE_UNINITIALIZED);
- return uc_fw->fetch_status != INTEL_UC_FIRMWARE_NOT_SUPPORTED;
+ return __intel_uc_fw_status(uc_fw) != INTEL_UC_FIRMWARE_NOT_SUPPORTED;
}
static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
{
if (intel_uc_fw_is_loaded(uc_fw))
- uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+ uc_fw->status = INTEL_UC_FIRMWARE_AVAILABLE;
}
/**
@@ -138,7 +159,7 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
*/
static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
{
- if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+ if (!intel_uc_fw_is_available(uc_fw))
return 0;
return uc_fw->header_size + uc_fw->ucode_size;
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 5/8] drm/i915/uc: Move xfer rsa logic to common function
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
` (3 preceding siblings ...)
2019-07-25 0:18 ` [PATCH v3 4/8] drm/i915/uc: Unify uc_fw status tracking Daniele Ceraolo Spurio
@ 2019-07-25 0:18 ` Daniele Ceraolo Spurio
2019-07-25 0:18 ` [PATCH v3 6/8] drm/i915/huc: Copy huc rsa only once Daniele Ceraolo Spurio
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-07-25 0:18 UTC (permalink / raw)
To: intel-gfx
The way we copy the RSA is the same for GuC and HuC, so we can move the
logic in a common function. this will also make any update needed for
local memory easier.
v2: return the number of copied bytes and check it (Chris)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v1
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 7 +++----
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 10 +++++-----
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 20 ++++++++++++++++++++
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 1 +
4 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index eec767383e92..385f6d38bf49 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -75,13 +75,12 @@ static void guc_prepare_xfer(struct intel_guc *guc)
static void guc_xfer_rsa(struct intel_guc *guc)
{
struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
- struct intel_uc_fw *fw = &guc->fw;
- struct sg_table *pages = fw->obj->mm.pages;
u32 rsa[UOS_RSA_SCRATCH_COUNT];
+ size_t copied;
int i;
- sg_pcopy_to_buffer(pages->sgl, pages->nents,
- rsa, sizeof(rsa), fw->rsa_offset);
+ copied = intel_uc_fw_copy_rsa(&guc->fw, rsa, sizeof(rsa));
+ GEM_BUG_ON(copied < sizeof(rsa));
for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
intel_uncore_write(uncore, UOS_RSA_SCRATCH(i), rsa[i]);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index ba2e1a835830..472568843ccf 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -36,17 +36,17 @@ void intel_huc_fw_init_early(struct intel_huc *huc)
static void huc_xfer_rsa(struct intel_huc *huc)
{
- struct intel_uc_fw *fw = &huc->fw;
- struct sg_table *pages = fw->obj->mm.pages;
+ size_t copied;
/*
* HuC firmware image is outside GuC accessible range.
* Copy the RSA signature out of the image into
* the perma-pinned region set aside for it
*/
- sg_pcopy_to_buffer(pages->sgl, pages->nents,
- huc->rsa_data_vaddr, fw->rsa_size,
- fw->rsa_offset);
+ GEM_BUG_ON(huc->fw.rsa_size > huc->rsa_data->size);
+ copied = intel_uc_fw_copy_rsa(&huc->fw, huc->rsa_data_vaddr,
+ huc->rsa_data->size);
+ GEM_BUG_ON(copied < huc->fw.rsa_size);
}
static int huc_xfer_ucode(struct intel_huc *huc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 1e7df2c19265..f60129c17e40 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -458,6 +458,26 @@ void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw)
uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
}
+/**
+ * intel_uc_fw_copy_rsa - copy fw RSA to buffer
+ *
+ * @uc_fw: uC firmware
+ * @dst: dst buffer
+ * @max_len: max number of bytes to copy
+ *
+ * Return: number of copied bytes.
+ */
+size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len)
+{
+ struct sg_table *pages = uc_fw->obj->mm.pages;
+ u32 size = min_t(u32, uc_fw->rsa_size, max_len);
+
+ GEM_BUG_ON(!intel_uc_fw_is_available(uc_fw));
+
+ return sg_pcopy_to_buffer(pages->sgl, pages->nents,
+ dst, size, uc_fw->rsa_offset);
+}
+
/**
* intel_uc_fw_dump - dump information about uC firmware
* @uc_fw: uC firmware
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index f6aa2e3e4d1f..c843d00b1b75 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -176,6 +176,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
int intel_uc_fw_init(struct intel_uc_fw *uc_fw);
void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw);
+size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len);
void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p);
#endif
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 6/8] drm/i915/huc: Copy huc rsa only once
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
` (4 preceding siblings ...)
2019-07-25 0:18 ` [PATCH v3 5/8] drm/i915/uc: Move xfer rsa logic to common function Daniele Ceraolo Spurio
@ 2019-07-25 0:18 ` Daniele Ceraolo Spurio
2019-07-25 6:00 ` Michal Wajdeczko
2019-07-25 0:18 ` [PATCH v3 7/8] drm/i915/uc: Plumb the gt through fw_upload Daniele Ceraolo Spurio
` (4 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-07-25 0:18 UTC (permalink / raw)
To: intel-gfx
The binary is perma-pinned and the rsa is not going to change, so copy
it only once and not on every load.
v2: onion unwind (Chris)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Fernando Pacheco <fernando.pacheco@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v1
---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 27 +++++++++++++++++++----
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 1 -
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 17 --------------
3 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index a45976e56af7..c9535caba844 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -50,6 +50,7 @@ static int intel_huc_rsa_data_create(struct intel_huc *huc)
struct intel_gt *gt = huc_to_gt(huc);
struct intel_guc *guc = >->uc.guc;
struct i915_vma *vma;
+ size_t copied;
void *vaddr;
/*
@@ -62,6 +63,7 @@ static int intel_huc_rsa_data_create(struct intel_huc *huc)
* the authentication since its GGTT offset will be GuC
* accessible.
*/
+ GEM_BUG_ON(huc->fw.rsa_size > PAGE_SIZE);
vma = intel_guc_allocate_vma(guc, PAGE_SIZE);
if (IS_ERR(vma))
return PTR_ERR(vma);
@@ -72,26 +74,43 @@ static int intel_huc_rsa_data_create(struct intel_huc *huc)
return PTR_ERR(vaddr);
}
+ copied = intel_uc_fw_copy_rsa(&huc->fw, vaddr, vma->size);
+ GEM_BUG_ON(copied < huc->fw.rsa_size);
+
+ i915_gem_object_unpin_map(vma->obj);
+
huc->rsa_data = vma;
- huc->rsa_data_vaddr = vaddr;
return 0;
}
static void intel_huc_rsa_data_destroy(struct intel_huc *huc)
{
- i915_vma_unpin_and_release(&huc->rsa_data, I915_VMA_RELEASE_MAP);
+ i915_vma_unpin_and_release(&huc->rsa_data, 0);
}
int intel_huc_init(struct intel_huc *huc)
{
int err;
- err = intel_huc_rsa_data_create(huc);
+ err = intel_uc_fw_init(&huc->fw);
if (err)
return err;
- return intel_uc_fw_init(&huc->fw);
+ /*
+ * HuC firmware image is outside GuC accessible range.
+ * Copy the RSA signature out of the image into
+ * a perma-pinned region set aside for it
+ */
+ err = intel_huc_rsa_data_create(huc);
+ if (err)
+ goto out_fini;
+
+ return 0;
+
+out_fini:
+ intel_uc_fw_fini(&huc->fw);
+ return err;
}
void intel_huc_fini(struct intel_huc *huc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index ea340f85bc46..4465209ce233 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -35,7 +35,6 @@ struct intel_huc {
/* HuC-specific additions */
struct i915_vma *rsa_data;
- void *rsa_data_vaddr;
struct {
i915_reg_t reg;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 472568843ccf..7d2d2eb94d22 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -34,21 +34,6 @@ void intel_huc_fw_init_early(struct intel_huc *huc)
intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC, huc_to_gt(huc)->i915);
}
-static void huc_xfer_rsa(struct intel_huc *huc)
-{
- size_t copied;
-
- /*
- * HuC firmware image is outside GuC accessible range.
- * Copy the RSA signature out of the image into
- * the perma-pinned region set aside for it
- */
- GEM_BUG_ON(huc->fw.rsa_size > huc->rsa_data->size);
- copied = intel_uc_fw_copy_rsa(&huc->fw, huc->rsa_data_vaddr,
- huc->rsa_data->size);
- GEM_BUG_ON(copied < huc->fw.rsa_size);
-}
-
static int huc_xfer_ucode(struct intel_huc *huc)
{
struct intel_uc_fw *huc_fw = &huc->fw;
@@ -108,8 +93,6 @@ static int huc_fw_xfer(struct intel_uc_fw *huc_fw)
{
struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
- huc_xfer_rsa(huc);
-
return huc_xfer_ucode(huc);
}
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 7/8] drm/i915/uc: Plumb the gt through fw_upload
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
` (5 preceding siblings ...)
2019-07-25 0:18 ` [PATCH v3 6/8] drm/i915/huc: Copy huc rsa only once Daniele Ceraolo Spurio
@ 2019-07-25 0:18 ` Daniele Ceraolo Spurio
2019-07-25 0:18 ` [PATCH v3 8/8] drm/i915/uc: Unify uC firmware upload Daniele Ceraolo Spurio
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-07-25 0:18 UTC (permalink / raw)
To: intel-gfx
The gt is our new central structure for uc-related code, so we can use
that instead of jumping back to i915 via the fw object. Since we have it
in the upload function it is easy to pass it through the lower levels of
the xfer process instead of continuosly jumping via uc_fw->uc->gt, which
will also make things a bit cleaner for the next patch.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 35 +++++++++++------------
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 32 ++++++++-------------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 29 ++++++++++---------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 8 ++++--
4 files changed, 48 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 385f6d38bf49..3ea0de6f4b73 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -42,10 +42,8 @@ void intel_guc_fw_init_early(struct intel_guc *guc)
intel_uc_fw_init_early(&guc->fw, INTEL_UC_FW_TYPE_GUC, guc_to_gt(guc)->i915);
}
-static void guc_prepare_xfer(struct intel_guc *guc)
+static void guc_prepare_xfer(struct intel_uncore *uncore)
{
- struct intel_gt *gt = guc_to_gt(guc);
- struct intel_uncore *uncore = gt->uncore;
u32 shim_flags = GUC_DISABLE_SRAM_INIT_TO_ZEROES |
GUC_ENABLE_READ_CACHE_LOGIC |
GUC_ENABLE_MIA_CACHING |
@@ -56,12 +54,12 @@ static void guc_prepare_xfer(struct intel_guc *guc)
/* Must program this register before loading the ucode with DMA */
intel_uncore_write(uncore, GUC_SHIM_CONTROL, shim_flags);
- if (IS_GEN9_LP(gt->i915))
+ if (IS_GEN9_LP(uncore->i915))
intel_uncore_write(uncore, GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
else
intel_uncore_write(uncore, GEN9_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
- if (IS_GEN(gt->i915, 9)) {
+ if (IS_GEN(uncore->i915, 9)) {
/* DOP Clock Gating Enable for GuC clocks */
intel_uncore_rmw(uncore, GEN7_MISCCPCTL,
0, GEN8_DOP_CLOCK_GATE_GUC_ENABLE);
@@ -72,14 +70,14 @@ static void guc_prepare_xfer(struct intel_guc *guc)
}
/* Copy RSA signature from the fw image to HW for verification */
-static void guc_xfer_rsa(struct intel_guc *guc)
+static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
+ struct intel_uncore *uncore)
{
- struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
u32 rsa[UOS_RSA_SCRATCH_COUNT];
size_t copied;
int i;
- copied = intel_uc_fw_copy_rsa(&guc->fw, rsa, sizeof(rsa));
+ copied = intel_uc_fw_copy_rsa(guc_fw, rsa, sizeof(rsa));
GEM_BUG_ON(copied < sizeof(rsa));
for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
@@ -155,10 +153,10 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
* transfer between GTT locations. This functionality is left out of the API
* for now as there is no need for it.
*/
-static int guc_xfer_ucode(struct intel_guc *guc)
+static int guc_xfer_ucode(struct intel_uc_fw *guc_fw,
+ struct intel_gt *gt)
{
- struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
- struct intel_uc_fw *guc_fw = &guc->fw;
+ struct intel_uncore *uncore = gt->uncore;
unsigned long offset;
/*
@@ -169,7 +167,7 @@ static int guc_xfer_ucode(struct intel_guc *guc)
guc_fw->header_size + guc_fw->ucode_size);
/* Set the source address for the new blob */
- offset = intel_uc_fw_ggtt_offset(guc_fw) + guc_fw->header_offset;
+ offset = intel_uc_fw_ggtt_offset(guc_fw, gt->ggtt) + guc_fw->header_offset;
intel_uncore_write(uncore, DMA_ADDR_0_LOW, lower_32_bits(offset));
intel_uncore_write(uncore, DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
@@ -189,26 +187,25 @@ static int guc_xfer_ucode(struct intel_guc *guc)
/*
* Load the GuC firmware blob into the MinuteIA.
*/
-static int guc_fw_xfer(struct intel_uc_fw *guc_fw)
+static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct intel_gt *gt)
{
- struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
- struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
+ struct intel_uncore *uncore = gt->uncore;
int ret;
GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
- guc_prepare_xfer(guc);
+ guc_prepare_xfer(uncore);
/*
* Note that GuC needs the CSS header plus uKernel code to be copied
* by the DMA engine in one operation, whereas the RSA signature is
* loaded via MMIO.
*/
- guc_xfer_rsa(guc);
+ guc_xfer_rsa(guc_fw, uncore);
- ret = guc_xfer_ucode(guc);
+ ret = guc_xfer_ucode(guc_fw, gt);
intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
@@ -229,7 +226,7 @@ static int guc_fw_xfer(struct intel_uc_fw *guc_fw)
*/
int intel_guc_fw_upload(struct intel_guc *guc)
{
- int ret = intel_uc_fw_upload(&guc->fw, guc_fw_xfer);
+ int ret = intel_uc_fw_upload(&guc->fw, guc_to_gt(guc), guc_fw_xfer);
if (!ret)
guc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 7d2d2eb94d22..2e7ac8863728 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -34,10 +34,17 @@ void intel_huc_fw_init_early(struct intel_huc *huc)
intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC, huc_to_gt(huc)->i915);
}
-static int huc_xfer_ucode(struct intel_huc *huc)
+/**
+ * huc_fw_xfer() - DMA's the firmware
+ * @huc_fw: the firmware descriptor
+ *
+ * Transfer the firmware image to RAM for execution by the microcontroller.
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct intel_gt *gt)
{
- struct intel_uc_fw *huc_fw = &huc->fw;
- struct intel_uncore *uncore = huc_to_gt(huc)->uncore;
+ struct intel_uncore *uncore = gt->uncore;
unsigned long offset = 0;
u32 size;
int ret;
@@ -47,7 +54,7 @@ static int huc_xfer_ucode(struct intel_huc *huc)
intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
/* Set the source address for the uCode */
- offset = intel_uc_fw_ggtt_offset(huc_fw) +
+ offset = intel_uc_fw_ggtt_offset(huc_fw, gt->ggtt) +
huc_fw->header_offset;
intel_uncore_write(uncore, DMA_ADDR_0_LOW,
lower_32_bits(offset));
@@ -81,21 +88,6 @@ static int huc_xfer_ucode(struct intel_huc *huc)
return ret;
}
-/**
- * huc_fw_xfer() - DMA's the firmware
- * @huc_fw: the firmware descriptor
- *
- * Transfer the firmware image to RAM for execution by the microcontroller.
- *
- * Return: 0 on success, non-zero on failure
- */
-static int huc_fw_xfer(struct intel_uc_fw *huc_fw)
-{
- struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
-
- return huc_xfer_ucode(huc);
-}
-
/**
* intel_huc_fw_upload() - load HuC uCode to device
* @huc: intel_huc structure
@@ -110,5 +102,5 @@ static int huc_fw_xfer(struct intel_uc_fw *huc_fw)
*/
int intel_huc_fw_upload(struct intel_huc *huc)
{
- return intel_uc_fw_upload(&huc->fw, huc_fw_xfer);
+ return intel_uc_fw_upload(&huc->fw, huc_to_gt(huc), huc_fw_xfer);
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index f60129c17e40..8d099dac0224 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -321,12 +321,13 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
release_firmware(fw); /* OK even if fw is NULL */
}
-static void intel_uc_fw_ggtt_bind(struct intel_uc_fw *uc_fw)
+static void intel_uc_fw_ggtt_bind(struct intel_uc_fw *uc_fw,
+ struct intel_gt *gt)
{
struct drm_i915_gem_object *obj = uc_fw->obj;
- struct i915_ggtt *ggtt = &to_i915(obj->base.dev)->ggtt;
+ struct i915_ggtt *ggtt = gt->ggtt;
struct i915_vma dummy = {
- .node.start = intel_uc_fw_ggtt_offset(uc_fw),
+ .node.start = intel_uc_fw_ggtt_offset(uc_fw, ggtt),
.node.size = obj->base.size,
.pages = obj->mm.pages,
.vm = &ggtt->vm,
@@ -341,11 +342,12 @@ static void intel_uc_fw_ggtt_bind(struct intel_uc_fw *uc_fw)
ggtt->vm.insert_entries(&ggtt->vm, &dummy, I915_CACHE_NONE, 0);
}
-static void intel_uc_fw_ggtt_unbind(struct intel_uc_fw *uc_fw)
+static void intel_uc_fw_ggtt_unbind(struct intel_uc_fw *uc_fw,
+ struct intel_gt *gt)
{
struct drm_i915_gem_object *obj = uc_fw->obj;
- struct i915_ggtt *ggtt = &to_i915(obj->base.dev)->ggtt;
- u64 start = intel_uc_fw_ggtt_offset(uc_fw);
+ struct i915_ggtt *ggtt = gt->ggtt;
+ u64 start = intel_uc_fw_ggtt_offset(uc_fw, ggtt);
ggtt->vm.clear_range(&ggtt->vm, start, obj->base.size);
}
@@ -353,14 +355,15 @@ static void intel_uc_fw_ggtt_unbind(struct intel_uc_fw *uc_fw)
/**
* intel_uc_fw_upload - load uC firmware using custom loader
* @uc_fw: uC firmware
+ * @gt: the intel_gt structure
* @xfer: custom uC firmware loader function
*
* Loads uC firmware using custom loader and updates internal flags.
*
* Return: 0 on success, non-zero on failure.
*/
-int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
- int (*xfer)(struct intel_uc_fw *uc_fw))
+int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
+ int (*xfer)(struct intel_uc_fw *uc_fw, struct intel_gt *gt))
{
int err;
@@ -373,9 +376,9 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
if (!intel_uc_fw_is_available(uc_fw))
return -ENOEXEC;
/* Call custom loader */
- intel_uc_fw_ggtt_bind(uc_fw);
- err = xfer(uc_fw);
- intel_uc_fw_ggtt_unbind(uc_fw);
+ intel_uc_fw_ggtt_bind(uc_fw, gt);
+ err = xfer(uc_fw, gt);
+ intel_uc_fw_ggtt_unbind(uc_fw, gt);
if (err)
goto fail;
@@ -427,10 +430,8 @@ void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
i915_gem_object_unpin_pages(uc_fw->obj);
}
-u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw)
+u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
{
- struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
- struct i915_ggtt *ggtt = &i915->ggtt;
struct drm_mm_node *node = &ggtt->uc_fw;
GEM_BUG_ON(!node->allocated);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index c843d00b1b75..a69b6f00fe16 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -30,6 +30,8 @@
struct drm_printer;
struct drm_i915_private;
+struct intel_gt;
+struct i915_ggtt;
/* Home of GuC, HuC and DMC firmwares */
#define INTEL_UC_FIRMWARE_URL "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915"
@@ -171,11 +173,11 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
void intel_uc_fw_fetch(struct drm_i915_private *i915,
struct intel_uc_fw *uc_fw);
void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
-int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
- int (*xfer)(struct intel_uc_fw *uc_fw));
+int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
+ int (*xfer)(struct intel_uc_fw *uc_fw, struct intel_gt *gt));
int intel_uc_fw_init(struct intel_uc_fw *uc_fw);
void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
-u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw);
+u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt);
size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len);
void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p);
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 8/8] drm/i915/uc: Unify uC firmware upload
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
` (6 preceding siblings ...)
2019-07-25 0:18 ` [PATCH v3 7/8] drm/i915/uc: Plumb the gt through fw_upload Daniele Ceraolo Spurio
@ 2019-07-25 0:18 ` Daniele Ceraolo Spurio
2019-07-25 0:38 ` ✗ Fi.CI.CHECKPATCH: warning for uC fw path unification + misc clean-up (rev3) Patchwork
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-07-25 0:18 UTC (permalink / raw)
To: intel-gfx
The way we load the firmwares is the same for both GuC and HuC, the only
difference is in the wopcm destination address and the dma flags, so we
easily can move the logic to a common function and pass in offset and
flags. The only other difference in the uplaod path are some the extra
steps that guc does before and after the xfer, but those don't require
the guc fw to be pinned in ggtt and can safely be performed before
calling the uc_upload function.
Note that this patch re-introduces the dma xfer wait for guc loading that
was removed with "drm/i915/guc: Propagate the fw xfer timeout". This is
not going to slow us down on a successful load (the dma has to complete
before fw init can start), but could slightly increase the timeout in case
of a fw init error.
v2: use _fw variants for uncore accesses (Chris), fix guc_fw status on
failed wait.
v3: use dev_err and print DMA_CTRL (Chris)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 107 ++++++----------------
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 57 +-----------
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 80 ++++++++++++----
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 4 +-
4 files changed, 93 insertions(+), 155 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 3ea0de6f4b73..28735c14b9a0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -84,13 +84,6 @@ static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
intel_uncore_write(uncore, UOS_RSA_SCRATCH(i), rsa[i]);
}
-static bool guc_xfer_completed(struct intel_uncore *uncore, u32 *status)
-{
- /* Did we complete the xfer? */
- *status = intel_uncore_read(uncore, DMA_CTRL);
- return !(*status & START_DMA);
-}
-
/*
* Read the GuC status register (GUC_STATUS) and store it in the
* specified location; then return a boolean indicating whether
@@ -137,65 +130,27 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
ret = -ENXIO;
}
- if (ret == 0 && !guc_xfer_completed(uncore, &status)) {
- DRM_ERROR("GuC is ready, but the xfer %08x is incomplete\n",
- status);
- ret = -ENXIO;
- }
-
return ret;
}
-/*
- * Transfer the firmware image to RAM for execution by the microcontroller.
+/**
+ * intel_guc_fw_upload() - load GuC uCode to device
+ * @guc: intel_guc structure
*
- * Architecturally, the DMA engine is bidirectional, and can potentially even
- * transfer between GTT locations. This functionality is left out of the API
- * for now as there is no need for it.
- */
-static int guc_xfer_ucode(struct intel_uc_fw *guc_fw,
- struct intel_gt *gt)
-{
- struct intel_uncore *uncore = gt->uncore;
- unsigned long offset;
-
- /*
- * The header plus uCode will be copied to WOPCM via DMA, excluding any
- * other components
- */
- intel_uncore_write(uncore, DMA_COPY_SIZE,
- guc_fw->header_size + guc_fw->ucode_size);
-
- /* Set the source address for the new blob */
- offset = intel_uc_fw_ggtt_offset(guc_fw, gt->ggtt) + guc_fw->header_offset;
- intel_uncore_write(uncore, DMA_ADDR_0_LOW, lower_32_bits(offset));
- intel_uncore_write(uncore, DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
-
- /*
- * Set the DMA destination. Current uCode expects the code to be
- * loaded at 8k; locations below this are used for the stack.
- */
- intel_uncore_write(uncore, DMA_ADDR_1_LOW, 0x2000);
- intel_uncore_write(uncore, DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
-
- /* Finally start the DMA */
- intel_uncore_write(uncore, DMA_CTRL,
- _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
-
- return guc_wait_ucode(uncore);
-}
-/*
- * Load the GuC firmware blob into the MinuteIA.
+ * Called from intel_uc_init_hw() during driver load, resume from sleep and
+ * after a GPU reset.
+ *
+ * The firmware image should have already been fetched into memory, so only
+ * check that fetch succeeded, and then transfer the image to the h/w.
+ *
+ * Return: non-zero code on error
*/
-static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct intel_gt *gt)
+int intel_guc_fw_upload(struct intel_guc *guc)
{
+ struct intel_gt *gt = guc_to_gt(guc);
struct intel_uncore *uncore = gt->uncore;
int ret;
- GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
-
- intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
-
guc_prepare_xfer(uncore);
/*
@@ -203,32 +158,24 @@ static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct intel_gt *gt)
* by the DMA engine in one operation, whereas the RSA signature is
* loaded via MMIO.
*/
- guc_xfer_rsa(guc_fw, uncore);
+ guc_xfer_rsa(&guc->fw, uncore);
- ret = guc_xfer_ucode(guc_fw, gt);
-
- intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
+ /*
+ * Current uCode expects the code to be loaded at 8k; locations below
+ * this are used for the stack.
+ */
+ ret = intel_uc_fw_upload(&guc->fw, gt, 0x2000, UOS_MOVE);
+ if (ret)
+ goto out;
- return ret;
-}
+ ret = guc_wait_ucode(uncore);
+ if (ret)
+ goto out;
-/**
- * intel_guc_fw_upload() - load GuC uCode to device
- * @guc: intel_guc structure
- *
- * Called from intel_uc_init_hw() during driver load, resume from sleep and
- * after a GPU reset.
- *
- * The firmware image should have already been fetched into memory, so only
- * check that fetch succeeded, and then transfer the image to the h/w.
- *
- * Return: non-zero code on error
- */
-int intel_guc_fw_upload(struct intel_guc *guc)
-{
- int ret = intel_uc_fw_upload(&guc->fw, guc_to_gt(guc), guc_fw_xfer);
- if (!ret)
- guc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
+ guc->fw.status = INTEL_UC_FIRMWARE_RUNNING;
+ return 0;
+out:
+ guc->fw.status = INTEL_UC_FIRMWARE_FAIL;
return ret;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 2e7ac8863728..0e885859c828 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -34,60 +34,6 @@ void intel_huc_fw_init_early(struct intel_huc *huc)
intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC, huc_to_gt(huc)->i915);
}
-/**
- * huc_fw_xfer() - DMA's the firmware
- * @huc_fw: the firmware descriptor
- *
- * Transfer the firmware image to RAM for execution by the microcontroller.
- *
- * Return: 0 on success, non-zero on failure
- */
-static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct intel_gt *gt)
-{
- struct intel_uncore *uncore = gt->uncore;
- unsigned long offset = 0;
- u32 size;
- int ret;
-
- GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
-
- intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
-
- /* Set the source address for the uCode */
- offset = intel_uc_fw_ggtt_offset(huc_fw, gt->ggtt) +
- huc_fw->header_offset;
- intel_uncore_write(uncore, DMA_ADDR_0_LOW,
- lower_32_bits(offset));
- intel_uncore_write(uncore, DMA_ADDR_0_HIGH,
- upper_32_bits(offset) & 0xFFFF);
-
- /*
- * Hardware doesn't look at destination address for HuC. Set it to 0,
- * but still program the correct address space.
- */
- intel_uncore_write(uncore, DMA_ADDR_1_LOW, 0);
- intel_uncore_write(uncore, DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
-
- size = huc_fw->header_size + huc_fw->ucode_size;
- intel_uncore_write(uncore, DMA_COPY_SIZE, size);
-
- /* Start the DMA */
- intel_uncore_write(uncore, DMA_CTRL,
- _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
-
- /* Wait for DMA to finish */
- ret = intel_wait_for_register_fw(uncore, DMA_CTRL, START_DMA, 0, 100);
-
- DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
-
- /* Disable the bits once DMA is over */
- intel_uncore_write(uncore, DMA_CTRL, _MASKED_BIT_DISABLE(HUC_UKERNEL));
-
- intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
-
- return ret;
-}
-
/**
* intel_huc_fw_upload() - load HuC uCode to device
* @huc: intel_huc structure
@@ -102,5 +48,6 @@ static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct intel_gt *gt)
*/
int intel_huc_fw_upload(struct intel_huc *huc)
{
- return intel_uc_fw_upload(&huc->fw, huc_to_gt(huc), huc_fw_xfer);
+ /* HW doesn't look at destination address for HuC, so set it to 0 */
+ return intel_uc_fw_upload(&huc->fw, huc_to_gt(huc), 0, HUC_UKERNEL);
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 8d099dac0224..789b3d7228a4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -321,13 +321,24 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
release_firmware(fw); /* OK even if fw is NULL */
}
+static u32 uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
+{
+ struct drm_mm_node *node = &ggtt->uc_fw;
+
+ GEM_BUG_ON(!node->allocated);
+ GEM_BUG_ON(upper_32_bits(node->start));
+ GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
+
+ return lower_32_bits(node->start);
+}
+
static void intel_uc_fw_ggtt_bind(struct intel_uc_fw *uc_fw,
struct intel_gt *gt)
{
struct drm_i915_gem_object *obj = uc_fw->obj;
struct i915_ggtt *ggtt = gt->ggtt;
struct i915_vma dummy = {
- .node.start = intel_uc_fw_ggtt_offset(uc_fw, ggtt),
+ .node.start = uc_fw_ggtt_offset(uc_fw, ggtt),
.node.size = obj->base.size,
.pages = obj->mm.pages,
.vm = &ggtt->vm,
@@ -347,23 +358,69 @@ static void intel_uc_fw_ggtt_unbind(struct intel_uc_fw *uc_fw,
{
struct drm_i915_gem_object *obj = uc_fw->obj;
struct i915_ggtt *ggtt = gt->ggtt;
- u64 start = intel_uc_fw_ggtt_offset(uc_fw, ggtt);
+ u64 start = uc_fw_ggtt_offset(uc_fw, ggtt);
ggtt->vm.clear_range(&ggtt->vm, start, obj->base.size);
}
+static int uc_fw_xfer(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
+ u32 wopcm_offset, u32 dma_flags)
+{
+ struct intel_uncore *uncore = gt->uncore;
+ u64 offset;
+ int ret;
+
+ intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
+
+ /* Set the source address for the uCode */
+ offset = uc_fw_ggtt_offset(uc_fw, gt->ggtt) + uc_fw->header_offset;
+ GEM_BUG_ON(upper_32_bits(offset) & 0xFFFF0000);
+ intel_uncore_write_fw(uncore, DMA_ADDR_0_LOW, lower_32_bits(offset));
+ intel_uncore_write_fw(uncore, DMA_ADDR_0_HIGH, upper_32_bits(offset));
+
+ /* Set the DMA destination */
+ intel_uncore_write_fw(uncore, DMA_ADDR_1_LOW, wopcm_offset);
+ intel_uncore_write_fw(uncore, DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
+
+ /*
+ * Set the transfer size. The header plus uCode will be copied to WOPCM
+ * via DMA, excluding any other components
+ */
+ intel_uncore_write_fw(uncore, DMA_COPY_SIZE,
+ uc_fw->header_size + uc_fw->ucode_size);
+
+ /* Start the DMA */
+ intel_uncore_write_fw(uncore, DMA_CTRL,
+ _MASKED_BIT_ENABLE(dma_flags | START_DMA));
+
+ /* Wait for DMA to finish */
+ ret = intel_wait_for_register_fw(uncore, DMA_CTRL, START_DMA, 0, 100);
+ if (ret)
+ dev_err(gt->i915->drm.dev, "DMA for %s fw failed, DMA_CTRL=%u\n",
+ intel_uc_fw_type_repr(uc_fw->type),
+ intel_uncore_read_fw(uncore, DMA_CTRL));
+
+ /* Disable the bits once DMA is over */
+ intel_uncore_write_fw(uncore, DMA_CTRL, _MASKED_BIT_DISABLE(dma_flags));
+
+ intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
+
+ return ret;
+}
+
/**
* intel_uc_fw_upload - load uC firmware using custom loader
* @uc_fw: uC firmware
* @gt: the intel_gt structure
- * @xfer: custom uC firmware loader function
+ * @wopcm_offset: destination offset in wopcm
+ * @dma_flags: flags for flags for dma ctrl
*
- * Loads uC firmware using custom loader and updates internal flags.
+ * Loads uC firmware and updates internal flags.
*
* Return: 0 on success, non-zero on failure.
*/
int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
- int (*xfer)(struct intel_uc_fw *uc_fw, struct intel_gt *gt))
+ u32 wopcm_offset, u32 dma_flags)
{
int err;
@@ -377,7 +434,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
return -ENOEXEC;
/* Call custom loader */
intel_uc_fw_ggtt_bind(uc_fw, gt);
- err = xfer(uc_fw, gt);
+ err = uc_fw_xfer(uc_fw, gt, wopcm_offset, dma_flags);
intel_uc_fw_ggtt_unbind(uc_fw, gt);
if (err)
goto fail;
@@ -430,17 +487,6 @@ void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
i915_gem_object_unpin_pages(uc_fw->obj);
}
-u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
-{
- struct drm_mm_node *node = &ggtt->uc_fw;
-
- GEM_BUG_ON(!node->allocated);
- GEM_BUG_ON(upper_32_bits(node->start));
- GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
-
- return lower_32_bits(node->start);
-}
-
/**
* intel_uc_fw_cleanup_fetch - cleanup uC firmware
*
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index a69b6f00fe16..ff684c0c808e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -31,7 +31,6 @@
struct drm_printer;
struct drm_i915_private;
struct intel_gt;
-struct i915_ggtt;
/* Home of GuC, HuC and DMC firmwares */
#define INTEL_UC_FIRMWARE_URL "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915"
@@ -174,10 +173,9 @@ void intel_uc_fw_fetch(struct drm_i915_private *i915,
struct intel_uc_fw *uc_fw);
void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
- int (*xfer)(struct intel_uc_fw *uc_fw, struct intel_gt *gt));
+ u32 wopcm_offset, u32 dma_flags);
int intel_uc_fw_init(struct intel_uc_fw *uc_fw);
void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
-u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt);
size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len);
void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p);
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for uC fw path unification + misc clean-up (rev3)
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
` (7 preceding siblings ...)
2019-07-25 0:18 ` [PATCH v3 8/8] drm/i915/uc: Unify uC firmware upload Daniele Ceraolo Spurio
@ 2019-07-25 0:38 ` Patchwork
2019-07-25 0:43 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-07-25 0:57 ` ✗ Fi.CI.BAT: failure " Patchwork
10 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-07-25 0:38 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: uC fw path unification + misc clean-up (rev3)
URL : https://patchwork.freedesktop.org/series/64039/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
cbaf15df0ad3 drm/i915/uc: Unify uC platform check
c38f4e39f2fe drm/i915: Fix handling of non-supported uC
245ade6cfe0c drm/i915/uc: Unify uC FW selection
-:254: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#254: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:36:
+#define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
+ fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 8, 4, 3238)) \
+ fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01, 2893)) \
+ fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01, 8, 2893)) \
+ fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07, 1398))
-:254: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'fw_def' - possible side-effects?
#254: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:36:
+#define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
+ fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 8, 4, 3238)) \
+ fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01, 2893)) \
+ fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01, 8, 2893)) \
+ fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07, 1398))
-:254: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'guc_def' - possible side-effects?
#254: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:36:
+#define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
+ fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 8, 4, 3238)) \
+ fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01, 2893)) \
+ fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01, 8, 2893)) \
+ fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07, 1398))
-:254: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'huc_def' - possible side-effects?
#254: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:36:
+#define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
+ fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 8, 4, 3238)) \
+ fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01, 2893)) \
+ fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
+ fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01, 8, 2893)) \
+ fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07, 1398))
-:262: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'separator_' - possible side-effects?
#262: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:44:
+#define __MAKE_UC_FW_PATH(prefix_, name_, separator_, major_, minor_, patch_) \
+ "i915/" \
+ __stringify(prefix_) name_ \
+ __stringify(major_) separator_ \
+ __stringify(minor_) separator_ \
+ __stringify(patch_) ".bin"
-:276: ERROR:MULTISTATEMENT_MACRO_USE_DO_WHILE: Macros with multiple statements should be enclosed in a do - while loop
#276: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:58:
+#define INTEL_UC_MODULE_FW(platform_, revid_, guc_, huc_) \
+ MODULE_FIRMWARE(guc_); \
+ MODULE_FIRMWARE(huc_);
-:276: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#276: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:58:
+#define INTEL_UC_MODULE_FW(platform_, revid_, guc_, huc_) \
+ MODULE_FIRMWARE(guc_); \
+ MODULE_FIRMWARE(huc_);
-:292: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'major_' - possible side-effects?
#292: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:74:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB(major_, minor_, \
+ MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
-:292: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'minor_' - possible side-effects?
#292: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:74:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB(major_, minor_, \
+ MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
-:296: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'major_' - possible side-effects?
#296: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:78:
+#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \
+ UC_FW_BLOB(major_, minor_, \
+ MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_))
-:296: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'minor_' - possible side-effects?
#296: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:78:
+#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \
+ UC_FW_BLOB(major_, minor_, \
+ MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_))
-:421: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#421: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h:50:
};
+#define INTEL_UC_FW_NUM_TYPES 2
total: 2 errors, 1 warnings, 9 checks, 410 lines checked
69bbd167ca68 drm/i915/uc: Unify uc_fw status tracking
-:50: WARNING:LINE_SPACING: Missing a blank line after declarations
#50: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c:234:
+ int ret = intel_uc_fw_upload(&guc->fw, guc_fw_xfer);
+ if (!ret)
total: 0 errors, 1 warnings, 0 checks, 365 lines checked
81ae123e1dc6 drm/i915/uc: Move xfer rsa logic to common function
a3eb70cdbcbf drm/i915/huc: Copy huc rsa only once
97094f111a12 drm/i915/uc: Plumb the gt through fw_upload
64323d6a78ce drm/i915/uc: Unify uC firmware upload
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* ✗ Fi.CI.SPARSE: warning for uC fw path unification + misc clean-up (rev3)
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
` (8 preceding siblings ...)
2019-07-25 0:38 ` ✗ Fi.CI.CHECKPATCH: warning for uC fw path unification + misc clean-up (rev3) Patchwork
@ 2019-07-25 0:43 ` Patchwork
2019-07-25 0:57 ` ✗ Fi.CI.BAT: failure " Patchwork
10 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-07-25 0:43 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: uC fw path unification + misc clean-up (rev3)
URL : https://patchwork.freedesktop.org/series/64039/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/uc: Unify uC platform check
Okay!
Commit: drm/i915: Fix handling of non-supported uC
Okay!
Commit: drm/i915/uc: Unify uC FW selection
Okay!
Commit: drm/i915/uc: Unify uc_fw status tracking
Okay!
Commit: drm/i915/uc: Move xfer rsa logic to common function
+drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:473:20: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:473:20: warning: expression using sizeof(void)
Commit: drm/i915/huc: Copy huc rsa only once
Okay!
Commit: drm/i915/uc: Plumb the gt through fw_upload
Okay!
Commit: drm/i915/uc: Unify uC firmware upload
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* ✗ Fi.CI.BAT: failure for uC fw path unification + misc clean-up (rev3)
2019-07-25 0:18 [PATCH v3 0/8] uC fw path unification + misc clean-up Daniele Ceraolo Spurio
` (9 preceding siblings ...)
2019-07-25 0:43 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-07-25 0:57 ` Patchwork
2019-07-25 6:38 ` Chris Wilson
10 siblings, 1 reply; 16+ messages in thread
From: Patchwork @ 2019-07-25 0:57 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: uC fw path unification + misc clean-up (rev3)
URL : https://patchwork.freedesktop.org/series/64039/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6545 -> Patchwork_13741
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_13741 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_13741, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_13741:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@reload-with-fault-injection:
- fi-snb-2520m: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6545/fi-snb-2520m/igt@i915_module_load@reload-with-fault-injection.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/fi-snb-2520m/igt@i915_module_load@reload-with-fault-injection.html
Known issues
------------
Here are the changes found in Patchwork_13741 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6545/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
* igt@prime_vgem@basic-fence-flip:
- fi-kbl-7500u: [PASS][5] -> [SKIP][6] ([fdo#109271]) +23 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6545/fi-kbl-7500u/igt@prime_vgem@basic-fence-flip.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/fi-kbl-7500u/igt@prime_vgem@basic-fence-flip.html
#### Possible fixes ####
* igt@gem_ctx_create@basic-files:
- fi-icl-u2: [INCOMPLETE][7] ([fdo#107713] / [fdo#109100]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6545/fi-icl-u2/igt@gem_ctx_create@basic-files.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/fi-icl-u2/igt@gem_ctx_create@basic-files.html
* igt@i915_selftest@live_hangcheck:
- fi-kbl-guc: [INCOMPLETE][9] ([fdo#108744]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6545/fi-kbl-guc/igt@i915_selftest@live_hangcheck.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/fi-kbl-guc/igt@i915_selftest@live_hangcheck.html
* igt@kms_frontbuffer_tracking@basic:
- fi-icl-u3: [FAIL][11] ([fdo#103167]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6545/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
- {fi-icl-u4}: [FAIL][13] ([fdo#103167]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6545/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
[fdo#111046 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111046
[fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
Participating hosts (52 -> 43)
------------------------------
Additional (1): fi-skl-gvtdvm
Missing (10): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-gdg-551 fi-icl-y fi-icl-guc fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6545 -> Patchwork_13741
CI-20190529: 20190529
CI_DRM_6545: a6efe73f1e086c7935d56b08342f9e1c5565fcf3 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5109: e5fd509e16ec649436be31f38eaa5b85cb7f72f1 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13741: 64323d6a78ce25f36754be89f68267ba3b96fd31 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
64323d6a78ce drm/i915/uc: Unify uC firmware upload
97094f111a12 drm/i915/uc: Plumb the gt through fw_upload
a3eb70cdbcbf drm/i915/huc: Copy huc rsa only once
81ae123e1dc6 drm/i915/uc: Move xfer rsa logic to common function
69bbd167ca68 drm/i915/uc: Unify uc_fw status tracking
245ade6cfe0c drm/i915/uc: Unify uC FW selection
c38f4e39f2fe drm/i915: Fix handling of non-supported uC
cbaf15df0ad3 drm/i915/uc: Unify uC platform check
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13741/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread