* [PATCH v2 0/4] Don't sanitize enable_guc
@ 2019-07-31 14:38 Michal Wajdeczko
2019-07-31 14:38 ` [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports* Michal Wajdeczko
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Michal Wajdeczko @ 2019-07-31 14:38 UTC (permalink / raw)
To: intel-gfx
v2: rename + don't care about undocumented values <0
changes in IGT to stop using enable_guc modparam to detect
guc_submission mode will follow shortly
Michal Wajdeczko (4):
drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports*
drm/i915/uc: Consider enable_guc modparam during fw selection
drm/i915/guc: Use dedicated flag to track submission mode
drm/i915/uc: Stop sanitizing enable_guc modparam
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 9 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 12 ++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 16 +++
.../gpu/drm/i915/gt/uc/intel_guc_submission.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 +
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 130 +++++++-----------
drivers/gpu/drm/i915/gt/uc/intel_uc.h | 15 +-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 23 +++-
drivers/gpu/drm/i915/i915_drv.h | 4 +-
10 files changed, 116 insertions(+), 101 deletions(-)
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports*
2019-07-31 14:38 [PATCH v2 0/4] Don't sanitize enable_guc Michal Wajdeczko
@ 2019-07-31 14:38 ` Michal Wajdeczko
2019-07-31 20:47 ` Chris Wilson
2019-07-31 20:49 ` Chris Wilson
2019-07-31 14:38 ` [PATCH v2 2/4] drm/i915/uc: Consider enable_guc modparam during fw selection Michal Wajdeczko
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Michal Wajdeczko @ 2019-07-31 14:38 UTC (permalink / raw)
To: intel-gfx
Rename intel_uc_is_using* into intel_uc_supports* to make clear
distinction from actual state (compare intel_uc_fw_is_running)
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 8 ++---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 44 +++++++++++++-------------
drivers/gpu/drm/i915/gt/uc/intel_uc.h | 6 ++--
drivers/gpu/drm/i915/i915_drv.h | 4 +--
5 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 13fbbffd05c7..f8fc34816e2c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -144,7 +144,7 @@ static u32 guc_ctl_feature_flags(struct intel_guc *guc)
{
u32 flags = 0;
- if (!intel_uc_is_using_guc_submission(&guc_to_gt(guc)->uc))
+ if (!intel_uc_supports_guc_submission(&guc_to_gt(guc)->uc))
flags |= GUC_CTL_DISABLE_SCHEDULER;
return flags;
@@ -154,7 +154,7 @@ static u32 guc_ctl_ctxinfo_flags(struct intel_guc *guc)
{
u32 flags = 0;
- if (intel_uc_is_using_guc_submission(&guc_to_gt(guc)->uc)) {
+ if (intel_uc_supports_guc_submission(&guc_to_gt(guc)->uc)) {
u32 ctxnum, base;
base = intel_guc_ggtt_offset(guc, guc->stage_desc_pool);
@@ -290,7 +290,7 @@ int intel_guc_init(struct intel_guc *guc)
if (ret)
goto err_ads;
- if (intel_uc_is_using_guc_submission(>->uc)) {
+ if (intel_uc_supports_guc_submission(>->uc)) {
/*
* This is stuff we need to have available at fw load time
* if we are planning to enable submission later
@@ -329,7 +329,7 @@ void intel_guc_fini(struct intel_guc *guc)
i915_ggtt_disable_guc(gt->ggtt);
- if (intel_uc_is_using_guc_submission(>->uc))
+ if (intel_uc_supports_guc_submission(>->uc))
intel_guc_submission_fini(guc);
intel_guc_ct_fini(&guc->ct);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index c9535caba844..d642b167a389 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -185,7 +185,7 @@ int intel_huc_check_status(struct intel_huc *huc)
intel_wakeref_t wakeref;
u32 status = 0;
- if (!intel_uc_is_using_huc(>->uc))
+ if (!intel_uc_supports_huc(>->uc))
return -ENODEV;
with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 66b226be6759..5d674e418e5e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -95,11 +95,11 @@ static void sanitize_options_early(struct intel_uc *uc)
DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
i915_modparams.enable_guc,
- yesno(intel_uc_is_using_guc_submission(uc)),
- yesno(intel_uc_is_using_huc(uc)));
+ yesno(intel_uc_supports_guc_submission(uc)),
+ yesno(intel_uc_supports_huc(uc)));
/* Verify GuC firmware availability */
- if (intel_uc_is_using_guc(uc) && !intel_uc_fw_supported(guc_fw)) {
+ if (intel_uc_supports_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);
@@ -108,7 +108,7 @@ static void sanitize_options_early(struct intel_uc *uc)
}
/* Verify HuC firmware availability */
- if (intel_uc_is_using_huc(uc) && !intel_uc_fw_supported(huc_fw)) {
+ if (intel_uc_supports_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);
@@ -117,7 +117,7 @@ static void sanitize_options_early(struct intel_uc *uc)
}
/* XXX: GuC submission is unavailable for now */
- if (intel_uc_is_using_guc_submission(uc)) {
+ if (intel_uc_supports_guc_submission(uc)) {
DRM_INFO("Incompatible option detected: enable_guc=%d, "
"but GuC submission is not supported!\n",
i915_modparams.enable_guc);
@@ -309,21 +309,21 @@ void intel_uc_fetch_firmwares(struct intel_uc *uc)
{
struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
- if (!intel_uc_is_using_guc(uc))
+ if (!intel_uc_supports_guc(uc))
return;
intel_uc_fw_fetch(&uc->guc.fw, i915);
- if (intel_uc_is_using_huc(uc))
+ if (intel_uc_supports_huc(uc))
intel_uc_fw_fetch(&uc->huc.fw, i915);
}
void intel_uc_cleanup_firmwares(struct intel_uc *uc)
{
- if (!intel_uc_is_using_guc(uc))
+ if (!intel_uc_supports_guc(uc))
return;
- if (intel_uc_is_using_huc(uc))
+ if (intel_uc_supports_huc(uc))
intel_uc_fw_cleanup_fetch(&uc->huc.fw);
intel_uc_fw_cleanup_fetch(&uc->guc.fw);
@@ -335,20 +335,20 @@ int intel_uc_init(struct intel_uc *uc)
struct intel_huc *huc = &uc->huc;
int ret;
- if (!intel_uc_is_using_guc(uc))
+ if (!intel_uc_supports_guc(uc))
return 0;
if (!intel_uc_fw_supported(&guc->fw))
return -ENODEV;
/* XXX: GuC submission is unavailable for now */
- GEM_BUG_ON(intel_uc_is_using_guc_submission(uc));
+ GEM_BUG_ON(intel_uc_supports_guc_submission(uc));
ret = intel_guc_init(guc);
if (ret)
return ret;
- if (intel_uc_is_using_huc(uc)) {
+ if (intel_uc_supports_huc(uc)) {
ret = intel_huc_init(huc);
if (ret)
goto err_guc;
@@ -365,12 +365,12 @@ void intel_uc_fini(struct intel_uc *uc)
{
struct intel_guc *guc = &uc->guc;
- if (!intel_uc_is_using_guc(uc))
+ if (!intel_uc_supports_guc(uc))
return;
GEM_BUG_ON(!intel_uc_fw_supported(&guc->fw));
- if (intel_uc_is_using_huc(uc))
+ if (intel_uc_supports_huc(uc))
intel_huc_fini(&uc->huc);
intel_guc_fini(guc);
@@ -391,7 +391,7 @@ static void __uc_sanitize(struct intel_uc *uc)
void intel_uc_sanitize(struct intel_uc *uc)
{
- if (!intel_uc_is_using_guc(uc))
+ if (!intel_uc_supports_guc(uc))
return;
__uc_sanitize(uc);
@@ -404,11 +404,11 @@ static int uc_init_wopcm(struct intel_uc *uc)
struct intel_uncore *uncore = gt->uncore;
u32 base = intel_wopcm_guc_base(>->i915->wopcm);
u32 size = intel_wopcm_guc_size(>->i915->wopcm);
- u32 huc_agent = intel_uc_is_using_huc(uc) ? HUC_LOADING_AGENT_GUC : 0;
+ u32 huc_agent = intel_uc_supports_huc(uc) ? HUC_LOADING_AGENT_GUC : 0;
u32 mask;
int err;
- GEM_BUG_ON(!intel_uc_is_using_guc(uc));
+ GEM_BUG_ON(!intel_uc_supports_guc(uc));
GEM_BUG_ON(!(base & GUC_WOPCM_OFFSET_MASK));
GEM_BUG_ON(base & ~GUC_WOPCM_OFFSET_MASK);
GEM_BUG_ON(!(size & GUC_WOPCM_SIZE_MASK));
@@ -447,7 +447,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
struct intel_huc *huc = &uc->huc;
int ret, attempts;
- if (!intel_uc_is_using_guc(uc))
+ if (!intel_uc_supports_guc(uc))
return 0;
GEM_BUG_ON(!intel_uc_fw_supported(&guc->fw));
@@ -474,7 +474,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
if (ret)
goto err_out;
- if (intel_uc_is_using_huc(uc)) {
+ if (intel_uc_supports_huc(uc)) {
ret = intel_huc_fw_upload(huc);
if (ret && intel_uc_fw_is_overridden(&huc->fw))
goto err_out;
@@ -508,7 +508,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
if (ret)
goto err_communication;
- if (intel_uc_is_using_guc_submission(uc)) {
+ if (intel_uc_supports_guc_submission(uc)) {
ret = intel_guc_submission_enable(guc);
if (ret)
goto err_communication;
@@ -517,7 +517,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
dev_info(i915->drm.dev, "GuC firmware version %u.%u\n",
guc->fw.major_ver_found, guc->fw.minor_ver_found);
dev_info(i915->drm.dev, "GuC submission %s\n",
- enableddisabled(intel_uc_is_using_guc_submission(uc)));
+ enableddisabled(intel_uc_supports_guc_submission(uc)));
dev_info(i915->drm.dev, "HuC %s\n",
enableddisabled(intel_huc_is_authenticated(huc)));
@@ -553,7 +553,7 @@ void intel_uc_fini_hw(struct intel_uc *uc)
GEM_BUG_ON(!intel_uc_fw_supported(&guc->fw));
- if (intel_uc_is_using_guc_submission(uc))
+ if (intel_uc_supports_guc_submission(uc))
intel_guc_submission_disable(guc);
guc_disable_communication(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 25da51e95417..66d8b1ee6f1d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -49,19 +49,19 @@ void intel_uc_runtime_suspend(struct intel_uc *uc);
int intel_uc_resume(struct intel_uc *uc);
int intel_uc_runtime_resume(struct intel_uc *uc);
-static inline bool intel_uc_is_using_guc(struct intel_uc *uc)
+static inline bool intel_uc_supports_guc(struct intel_uc *uc)
{
GEM_BUG_ON(i915_modparams.enable_guc < 0);
return i915_modparams.enable_guc > 0;
}
-static inline bool intel_uc_is_using_guc_submission(struct intel_uc *uc)
+static inline bool intel_uc_supports_guc_submission(struct intel_uc *uc)
{
GEM_BUG_ON(i915_modparams.enable_guc < 0);
return i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION;
}
-static inline bool intel_uc_is_using_huc(struct intel_uc *uc)
+static inline bool intel_uc_supports_huc(struct intel_uc *uc)
{
GEM_BUG_ON(i915_modparams.enable_guc < 0);
return i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdede6325d87..572a84da3926 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2275,8 +2275,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_GT_UC(dev_priv) (INTEL_INFO(dev_priv)->has_gt_uc)
/* Having GuC is not the same as using GuC */
-#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_GUC(dev_priv) intel_uc_supports_guc(&(dev_priv)->gt.uc)
+#define USES_GUC_SUBMISSION(dev_priv) intel_uc_supports_guc_submission(&(dev_priv)->gt.uc)
#define HAS_POOLED_EU(dev_priv) (INTEL_INFO(dev_priv)->has_pooled_eu)
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] drm/i915/uc: Consider enable_guc modparam during fw selection
2019-07-31 14:38 [PATCH v2 0/4] Don't sanitize enable_guc Michal Wajdeczko
2019-07-31 14:38 ` [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports* Michal Wajdeczko
@ 2019-07-31 14:38 ` Michal Wajdeczko
2019-07-31 17:47 ` Chris Wilson
2019-07-31 14:38 ` [PATCH v2 3/4] drm/i915/guc: Use dedicated flag to track submission mode Michal Wajdeczko
2019-07-31 14:38 ` [PATCH v2 4/4] drm/i915/uc: Stop sanitizing enable_guc modparam Michal Wajdeczko
3 siblings, 1 reply; 13+ messages in thread
From: Michal Wajdeczko @ 2019-07-31 14:38 UTC (permalink / raw)
To: intel-gfx
We can use value of enable_guc modparam during firmware path selection
and start using firmware status to see if GuC/HuC is being used.
This is first step to make enable_guc modparam read-only.
v2: rebased, don't care about <0 (Chris)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v1
---
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 5 +++++
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 +++++
drivers/gpu/drm/i915/gt/uc/intel_uc.h | 6 ++----
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 23 +++++++++++++++++++++--
4 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 714e9892aaff..5901506672cd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -172,6 +172,11 @@ 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_supported(struct intel_guc *guc)
+{
+ return intel_uc_fw_supported(&guc->fw);
+}
+
static inline bool intel_guc_is_running(struct intel_guc *guc)
{
return intel_uc_fw_is_running(&guc->fw);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 4465209ce233..a6ae59b8cb77 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -55,6 +55,11 @@ static inline int intel_huc_sanitize(struct intel_huc *huc)
return 0;
}
+static inline bool intel_huc_is_supported(struct intel_huc *huc)
+{
+ return intel_uc_fw_supported(&huc->fw);
+}
+
static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
{
return intel_uc_fw_is_running(&huc->fw);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 66d8b1ee6f1d..cf6c60cffdfb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -51,8 +51,7 @@ int intel_uc_runtime_resume(struct intel_uc *uc);
static inline bool intel_uc_supports_guc(struct intel_uc *uc)
{
- GEM_BUG_ON(i915_modparams.enable_guc < 0);
- return i915_modparams.enable_guc > 0;
+ return intel_guc_is_supported(&uc->guc);
}
static inline bool intel_uc_supports_guc_submission(struct intel_uc *uc)
@@ -63,8 +62,7 @@ static inline bool intel_uc_supports_guc_submission(struct intel_uc *uc)
static inline bool intel_uc_supports_huc(struct intel_uc *uc)
{
- GEM_BUG_ON(i915_modparams.enable_guc < 0);
- return i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC;
+ return intel_huc_is_supported(&uc->huc);
}
#endif
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 ac91e3efd02b..3bf5ca35a005 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -132,6 +132,25 @@ __uc_fw_auto_select(struct intel_uc_fw *uc_fw, enum intel_platform p, u8 rev)
uc_fw->path = NULL;
}
}
+
+ /* We don't want to enable GuC/HuC on pre-Gen11 by default */
+ if (i915_modparams.enable_guc == -1 && p < INTEL_ICELAKE)
+ uc_fw->path = NULL;
+}
+
+static const char *__override_guc_firmware_path(void)
+{
+ if (i915_modparams.enable_guc & (ENABLE_GUC_SUBMISSION ||
+ ENABLE_GUC_LOAD_HUC))
+ return i915_modparams.guc_firmware_path;
+ return "";
+}
+
+static const char *__override_huc_firmware_path(void)
+{
+ if (i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC)
+ return i915_modparams.huc_firmware_path;
+ return "";
}
static bool
@@ -139,10 +158,10 @@ __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;
+ uc_fw->path = __override_guc_firmware_path();
break;
case INTEL_UC_FW_TYPE_HUC:
- uc_fw->path = i915_modparams.huc_firmware_path;
+ uc_fw->path = __override_huc_firmware_path();
break;
}
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] drm/i915/guc: Use dedicated flag to track submission mode
2019-07-31 14:38 [PATCH v2 0/4] Don't sanitize enable_guc Michal Wajdeczko
2019-07-31 14:38 ` [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports* Michal Wajdeczko
2019-07-31 14:38 ` [PATCH v2 2/4] drm/i915/uc: Consider enable_guc modparam during fw selection Michal Wajdeczko
@ 2019-07-31 14:38 ` Michal Wajdeczko
2019-07-31 20:52 ` Chris Wilson
2019-07-31 14:38 ` [PATCH v2 4/4] drm/i915/uc: Stop sanitizing enable_guc modparam Michal Wajdeczko
3 siblings, 1 reply; 13+ messages in thread
From: Michal Wajdeczko @ 2019-07-31 14:38 UTC (permalink / raw)
To: intel-gfx
Instead of relying on enable_guc modparam to represent actual
GuC submission mode, use dedicated flag and look at modparam
only to check if submission was explicitly disabled by the user.
v2: rebased, simplified condition (Chris)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 1 +
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 7 +++++++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 16 ++++++++++++++++
.../gpu/drm/i915/gt/uc/intel_guc_submission.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_uc.h | 3 +--
5 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index f8fc34816e2c..da14f8067497 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -82,6 +82,7 @@ void intel_guc_init_early(struct intel_guc *guc)
intel_guc_fw_init_early(guc);
intel_guc_ct_init_early(&guc->ct);
intel_guc_log_init_early(&guc->log);
+ intel_guc_submission_init_early(guc);
mutex_init(&guc->send_mutex);
spin_lock_init(&guc->irq_lock);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 5901506672cd..6edb29b9ceaa 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -61,6 +61,8 @@ struct intel_guc {
void (*disable)(struct intel_guc *guc);
} interrupts;
+ bool submission_supported;
+
struct i915_vma *ads_vma;
struct __guc_ads_blob *ads_blob;
@@ -190,6 +192,11 @@ static inline int intel_guc_sanitize(struct intel_guc *guc)
return 0;
}
+static inline bool intel_guc_is_submission_supported(struct intel_guc *guc)
+{
+ return guc->submission_supported;
+}
+
static inline void intel_guc_enable_msg(struct intel_guc *guc, u32 mask)
{
spin_lock_irq(&guc->irq_lock);
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 b4238fe16a03..b4b508f19a1c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1163,6 +1163,22 @@ void intel_guc_submission_disable(struct intel_guc *guc)
guc_clients_disable(guc);
}
+static bool __guc_submission_support(struct intel_guc *guc)
+{
+ /* XXX: GuC submission is unavailable for now */
+ return false;
+
+ if (!intel_guc_is_supported(guc))
+ return false;
+
+ return i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION;
+}
+
+void intel_guc_submission_init_early(struct intel_guc *guc)
+{
+ guc->submission_supported = __guc_submission_support(guc);
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftest_guc.c"
#endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index 87a38cb6faf3..c4ad2702ec8d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -77,6 +77,7 @@ struct intel_guc_client {
I915_SELFTEST_DECLARE(bool use_nop_wqi);
};
+void intel_guc_submission_init_early(struct intel_guc *guc);
int intel_guc_submission_init(struct intel_guc *guc);
int intel_guc_submission_enable(struct intel_guc *guc);
void intel_guc_submission_disable(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index cf6c60cffdfb..bd05d1a1d39f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -56,8 +56,7 @@ static inline bool intel_uc_supports_guc(struct intel_uc *uc)
static inline bool intel_uc_supports_guc_submission(struct intel_uc *uc)
{
- GEM_BUG_ON(i915_modparams.enable_guc < 0);
- return i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION;
+ return intel_guc_is_submission_supported(&uc->guc);
}
static inline bool intel_uc_supports_huc(struct intel_uc *uc)
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] drm/i915/uc: Stop sanitizing enable_guc modparam
2019-07-31 14:38 [PATCH v2 0/4] Don't sanitize enable_guc Michal Wajdeczko
` (2 preceding siblings ...)
2019-07-31 14:38 ` [PATCH v2 3/4] drm/i915/guc: Use dedicated flag to track submission mode Michal Wajdeczko
@ 2019-07-31 14:38 ` Michal Wajdeczko
2019-07-31 20:55 ` Chris Wilson
3 siblings, 1 reply; 13+ messages in thread
From: Michal Wajdeczko @ 2019-07-31 14:38 UTC (permalink / raw)
To: intel-gfx
As we already track GuC/HuC uses by other means than modparam
there is no point in sanitizing it. Just scan modparam for
major discrepancies between what was requested vs actual.
v2: rebased, reworded info messages
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 92 ++++++++-------------------
1 file changed, 28 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 5d674e418e5e..1ea9b4f23b24 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -55,78 +55,42 @@ static int __intel_uc_reset_hw(struct intel_uc *uc)
return ret;
}
-static int __get_platform_enable_guc(struct intel_uc *uc)
+static void __confirm_options(struct intel_uc *uc)
{
- struct intel_uc_fw *guc_fw = &uc->guc.fw;
- struct intel_uc_fw *huc_fw = &uc->huc.fw;
- int enable_guc = 0;
-
- if (!HAS_GT_UC(uc_to_gt(uc)->i915))
- return 0;
-
- /* We don't want to enable GuC/HuC on pre-Gen11 by default */
- if (INTEL_GEN(uc_to_gt(uc)->i915) < 11)
- return 0;
-
- if (intel_uc_fw_supported(guc_fw) && intel_uc_fw_supported(huc_fw))
- enable_guc |= ENABLE_GUC_LOAD_HUC;
-
- return enable_guc;
-}
-
-/**
- * sanitize_options_early - sanitize uC related modparam options
- * @uc: the intel_uc structure
- *
- * In case of "enable_guc" option this function will attempt to modify
- * it only if it was initially set to "auto(-1)". Default value for this
- * modparam varies between platforms and it is hardcoded in driver code.
- * Any other modparam value is only monitored against availability of the
- * related hardware or firmware definitions.
- */
-static void sanitize_options_early(struct intel_uc *uc)
-{
- struct intel_uc_fw *guc_fw = &uc->guc.fw;
- struct intel_uc_fw *huc_fw = &uc->huc.fw;
-
- /* A negative value means "use platform default" */
- if (i915_modparams.enable_guc < 0)
- i915_modparams.enable_guc = __get_platform_enable_guc(uc);
-
- DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
+ DRM_DEBUG_DRIVER("enable_guc=%d (guc:%s submission:%s huc:%s)\n",
i915_modparams.enable_guc,
+ yesno(intel_uc_supports_guc(uc)),
yesno(intel_uc_supports_guc_submission(uc)),
yesno(intel_uc_supports_huc(uc)));
- /* Verify GuC firmware availability */
- if (intel_uc_supports_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;
- }
+ if (i915_modparams.enable_guc == -1)
+ return;
- /* Verify HuC firmware availability */
- if (intel_uc_supports_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;
+ if (i915_modparams.enable_guc == 0) {
+ GEM_BUG_ON(intel_uc_supports_guc(uc));
+ GEM_BUG_ON(intel_uc_supports_guc_submission(uc));
+ GEM_BUG_ON(intel_uc_supports_huc(uc));
+ return;
}
- /* XXX: GuC submission is unavailable for now */
- if (intel_uc_supports_guc_submission(uc)) {
- 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;
- }
+ if (!intel_uc_supports_guc(uc))
+ DRM_INFO("Incompatible option enable_guc=%d - %s\n",
+ i915_modparams.enable_guc, "GuC is not supported!");
+
+ if (i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC &&
+ !intel_uc_supports_huc(uc))
+ DRM_INFO("Incompatible option enable_guc=%d - %s\n",
+ i915_modparams.enable_guc, "HuC is not supported!");
+
+ if (i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION &&
+ !intel_uc_supports_guc_submission(uc))
+ DRM_INFO("Incompatible option enable_guc=%d - %s\n",
+ i915_modparams.enable_guc, "GuC submission is N/A");
- /* Make sure that sanitization was done */
- GEM_BUG_ON(i915_modparams.enable_guc < 0);
+ if (i915_modparams.enable_guc & ~(ENABLE_GUC_SUBMISSION ||
+ ENABLE_GUC_LOAD_HUC))
+ DRM_INFO("Incompatible option enable_guc=%d - %s\n",
+ i915_modparams.enable_guc, "undocumented flag");
}
void intel_uc_init_early(struct intel_uc *uc)
@@ -134,7 +98,7 @@ void intel_uc_init_early(struct intel_uc *uc)
intel_guc_init_early(&uc->guc);
intel_huc_init_early(&uc->huc);
- sanitize_options_early(uc);
+ __confirm_options(uc);
}
void intel_uc_cleanup_early(struct intel_uc *uc)
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] drm/i915/uc: Consider enable_guc modparam during fw selection
2019-07-31 14:38 ` [PATCH v2 2/4] drm/i915/uc: Consider enable_guc modparam during fw selection Michal Wajdeczko
@ 2019-07-31 17:47 ` Chris Wilson
2019-07-31 17:50 ` Michal Wajdeczko
0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2019-07-31 17:47 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Quoting Michal Wajdeczko (2019-07-31 15:38:54)
> +static const char *__override_guc_firmware_path(void)
> +{
> + if (i915_modparams.enable_guc & (ENABLE_GUC_SUBMISSION ||
> + ENABLE_GUC_LOAD_HUC))
|| ?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] drm/i915/uc: Consider enable_guc modparam during fw selection
2019-07-31 17:47 ` Chris Wilson
@ 2019-07-31 17:50 ` Michal Wajdeczko
2019-07-31 20:50 ` Chris Wilson
0 siblings, 1 reply; 13+ messages in thread
From: Michal Wajdeczko @ 2019-07-31 17:50 UTC (permalink / raw)
To: intel-gfx, Chris Wilson
On Wed, 31 Jul 2019 19:47:27 +0200, Chris Wilson
<chris@chris-wilson.co.uk> wrote:
> Quoting Michal Wajdeczko (2019-07-31 15:38:54)
>> +static const char *__override_guc_firmware_path(void)
>> +{
>> + if (i915_modparams.enable_guc & (ENABLE_GUC_SUBMISSION ||
>> + ENABLE_GUC_LOAD_HUC))
>
> || ?
oops, thanks for catching this (last minute addition, hrrr)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports*
2019-07-31 14:38 ` [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports* Michal Wajdeczko
@ 2019-07-31 20:47 ` Chris Wilson
2019-07-31 20:49 ` Chris Wilson
1 sibling, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2019-07-31 20:47 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Quoting Michal Wajdeczko (2019-07-31 15:38:53)
> Rename intel_uc_is_using* into intel_uc_supports* to make clear
> distinction from actual state (compare intel_uc_fw_is_running)
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
I think that reads quite nicely, I am certainly less baffled :)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports*
2019-07-31 14:38 ` [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports* Michal Wajdeczko
2019-07-31 20:47 ` Chris Wilson
@ 2019-07-31 20:49 ` Chris Wilson
2019-07-31 22:23 ` Michal Wajdeczko
1 sibling, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2019-07-31 20:49 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Quoting Michal Wajdeczko (2019-07-31 15:38:53)
> @@ -365,12 +365,12 @@ void intel_uc_fini(struct intel_uc *uc)
> {
> struct intel_guc *guc = &uc->guc;
>
> - if (!intel_uc_is_using_guc(uc))
> + if (!intel_uc_supports_guc(uc))
> return;
Note by this point we should know whether or not we loaded the guc and
need to cleanup. So here is_using_guc() does read better. If we even
need to bother and just leave it to the huc_fini / guc_fini to be no-ops
when there is nothing to do.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] drm/i915/uc: Consider enable_guc modparam during fw selection
2019-07-31 17:50 ` Michal Wajdeczko
@ 2019-07-31 20:50 ` Chris Wilson
0 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2019-07-31 20:50 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Quoting Michal Wajdeczko (2019-07-31 18:50:23)
> On Wed, 31 Jul 2019 19:47:27 +0200, Chris Wilson
> <chris@chris-wilson.co.uk> wrote:
>
> > Quoting Michal Wajdeczko (2019-07-31 15:38:54)
> >> +static const char *__override_guc_firmware_path(void)
> >> +{
> >> + if (i915_modparams.enable_guc & (ENABLE_GUC_SUBMISSION ||
> >> + ENABLE_GUC_LOAD_HUC))
> >
> > || ?
>
> oops, thanks for catching this (last minute addition, hrrr)
With that,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/guc: Use dedicated flag to track submission mode
2019-07-31 14:38 ` [PATCH v2 3/4] drm/i915/guc: Use dedicated flag to track submission mode Michal Wajdeczko
@ 2019-07-31 20:52 ` Chris Wilson
0 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2019-07-31 20:52 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Quoting Michal Wajdeczko (2019-07-31 15:38:55)
> Instead of relying on enable_guc modparam to represent actual
> GuC submission mode, use dedicated flag and look at modparam
> only to check if submission was explicitly disabled by the user.
>
> v2: rebased, simplified condition (Chris)
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] drm/i915/uc: Stop sanitizing enable_guc modparam
2019-07-31 14:38 ` [PATCH v2 4/4] drm/i915/uc: Stop sanitizing enable_guc modparam Michal Wajdeczko
@ 2019-07-31 20:55 ` Chris Wilson
0 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2019-07-31 20:55 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Quoting Michal Wajdeczko (2019-07-31 15:38:56)
> - /* Make sure that sanitization was done */
> - GEM_BUG_ON(i915_modparams.enable_guc < 0);
> + if (i915_modparams.enable_guc & ~(ENABLE_GUC_SUBMISSION ||
> + ENABLE_GUC_LOAD_HUC))
> + DRM_INFO("Incompatible option enable_guc=%d - %s\n",
> + i915_modparams.enable_guc, "undocumented flag");
You know something is wrong when DRM_INFO is right! (That is modparams
being global and not tied to any device;
rmdir /sys/modules/i915/parameters!)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports*
2019-07-31 20:49 ` Chris Wilson
@ 2019-07-31 22:23 ` Michal Wajdeczko
0 siblings, 0 replies; 13+ messages in thread
From: Michal Wajdeczko @ 2019-07-31 22:23 UTC (permalink / raw)
To: intel-gfx, Chris Wilson
On Wed, 31 Jul 2019 22:49:57 +0200, Chris Wilson
<chris@chris-wilson.co.uk> wrote:
> Quoting Michal Wajdeczko (2019-07-31 15:38:53)
>> @@ -365,12 +365,12 @@ void intel_uc_fini(struct intel_uc *uc)
>> {
>> struct intel_guc *guc = &uc->guc;
>>
>> - if (!intel_uc_is_using_guc(uc))
>> + if (!intel_uc_supports_guc(uc))
>> return;
>
> Note by this point we should know whether or not we loaded the guc and
> need to cleanup. So here is_using_guc() does read better. If we even
> need to bother and just leave it to the huc_fini / guc_fini to be no-ops
> when there is nothing to do.
note that here we already have sanitized uc even if it was loaded
and note that we also have intel_uc_fini_hw()
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-07-31 22:23 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-31 14:38 [PATCH v2 0/4] Don't sanitize enable_guc Michal Wajdeczko
2019-07-31 14:38 ` [PATCH v2 1/4] drm/i915/uc: Rename intel_uc_is_using* into intel_uc_supports* Michal Wajdeczko
2019-07-31 20:47 ` Chris Wilson
2019-07-31 20:49 ` Chris Wilson
2019-07-31 22:23 ` Michal Wajdeczko
2019-07-31 14:38 ` [PATCH v2 2/4] drm/i915/uc: Consider enable_guc modparam during fw selection Michal Wajdeczko
2019-07-31 17:47 ` Chris Wilson
2019-07-31 17:50 ` Michal Wajdeczko
2019-07-31 20:50 ` Chris Wilson
2019-07-31 14:38 ` [PATCH v2 3/4] drm/i915/guc: Use dedicated flag to track submission mode Michal Wajdeczko
2019-07-31 20:52 ` Chris Wilson
2019-07-31 14:38 ` [PATCH v2 4/4] drm/i915/uc: Stop sanitizing enable_guc modparam Michal Wajdeczko
2019-07-31 20:55 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.