* [PATCH v6 0/3] drm/i915/guc : Removing enable_guc_loading module and Decoupling logs and ADS from submission
@ 2017-10-10 18:32 Sujaritha Sundaresan
2017-10-10 18:32 ` [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support Sujaritha Sundaresan
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Sujaritha Sundaresan @ 2017-10-10 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Sujaritha Sundaresan
The first patch simply unifies different seq_puts messages found in debugfs.
Patch 2 focuses on replacing the enable_guc_loading module. Patch 3 deals with
decoupling guc logs and ADS from submission.
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Sujaritha Sundaresan (3):
drm/i915/guc : Unifying seq_puts messages for feature support
drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter
drm/i915/guc : Decouple logs and ADS from submission
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_debugfs.c | 18 +++--
drivers/gpu/drm/i915/i915_drv.h | 9 ++-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 105 +------------------------
drivers/gpu/drm/i915/i915_irq.c | 2 +-
drivers/gpu/drm/i915/i915_params.c | 4 -
drivers/gpu/drm/i915/i915_params.h | 1 -
drivers/gpu/drm/i915/intel_guc.h | 3 +-
drivers/gpu/drm/i915/intel_guc_ads.c | 120 +++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_guc_ads.h | 31 ++++++++
drivers/gpu/drm/i915/intel_guc_loader.c | 9 +--
drivers/gpu/drm/i915/intel_guc_log.c | 6 +-
drivers/gpu/drm/i915/intel_huc.c | 4 +-
drivers/gpu/drm/i915/intel_uc.c | 112 ++++++++++++++++++---------
drivers/gpu/drm/i915/intel_uncore.c | 3 +-
17 files changed, 262 insertions(+), 170 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_guc_ads.c
create mode 100644 drivers/gpu/drm/i915/intel_guc_ads.h
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support
2017-10-10 18:32 [PATCH v6 0/3] drm/i915/guc : Removing enable_guc_loading module and Decoupling logs and ADS from submission Sujaritha Sundaresan
@ 2017-10-10 18:32 ` Sujaritha Sundaresan
2017-10-13 9:05 ` Sagar Arun Kamble
2017-10-10 18:32 ` [PATCH v6 2/3] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter Sujaritha Sundaresan
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Sujaritha Sundaresan @ 2017-10-10 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Sujaritha Sundaresan
Unifying the various seq_puts messages in debugfs to the simplest one for
feature support.
v2: Clarifying the commit message (Anusha)
v3: Re-factoring code as per review (Michal)
v4: Rebase
v5: Split from following patch
v6: Re-factoring code (Michal, Sagar)
Clarifying commit message (Sagar)
Suggested by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5b58d2b..9d0c27b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1670,7 +1670,7 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
struct drm_i915_private *dev_priv = node_to_i915(m->private);
if (!HAS_FBC(dev_priv)) {
- seq_puts(m, "FBC unsupported on this chipset\n");
+ seq_puts(m, "not supported\n");
return 0;
}
@@ -1837,7 +1837,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
unsigned int max_gpu_freq, min_gpu_freq;
if (!HAS_LLC(dev_priv)) {
- seq_puts(m, "unsupported on this chipset\n");
+ seq_puts(m, "not supported\n");
return 0;
}
@@ -2391,6 +2391,7 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data)
struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
if (!HAS_HUC_UCODE(dev_priv))
+ seq_puts(m, "not supported\n");
return 0;
seq_puts(m, "HuC firmware status:\n");
@@ -2424,6 +2425,7 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
u32 tmp, i;
if (!HAS_GUC_UCODE(dev_priv))
+ seq_puts(m, "not supported\n");
return 0;
seq_printf(m, "GuC firmware status:\n");
@@ -2708,7 +2710,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
bool enabled = false;
if (!HAS_PSR(dev_priv)) {
- seq_puts(m, "PSR not supported\n");
+ seq_puts(m, "not supported\n");
return 0;
}
@@ -3565,7 +3567,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
mutex_lock(&drrs->mutex);
/* DRRS Supported */
- seq_puts(m, "\tDRRS Supported: Yes\n");
+ seq_puts(m, "supported\n");
/* disable_drrs() will make drrs->dp NULL */
if (!drrs->dp) {
@@ -3597,7 +3599,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
mutex_unlock(&drrs->mutex);
} else {
/* DRRS not supported. Print the VBT parameter*/
- seq_puts(m, "\tDRRS Supported : No");
+ seq_puts(m, "not supported\n");
}
seq_puts(m, "\n");
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v6 2/3] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter
2017-10-10 18:32 [PATCH v6 0/3] drm/i915/guc : Removing enable_guc_loading module and Decoupling logs and ADS from submission Sujaritha Sundaresan
2017-10-10 18:32 ` [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support Sujaritha Sundaresan
@ 2017-10-10 18:32 ` Sujaritha Sundaresan
2017-10-13 9:42 ` Sagar Arun Kamble
2017-10-10 18:32 ` [PATCH v6 3/3] drm/i915/guc : Decouple logs and ADS from submission Sujaritha Sundaresan
2017-10-10 19:07 ` ✗ Fi.CI.BAT: failure for drm/i915/guc : Removing enable_guc_loading module and Decoupling " Patchwork
3 siblings, 1 reply; 10+ messages in thread
From: Sujaritha Sundaresan @ 2017-10-10 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Sujaritha Sundaresan
We currently have two module parameters that control GuC: "enable_guc_loading" and "enable_guc_submission".
Whenever we need i915_modparams.enable_guc_submission=1, we also need enable_guc_loading=1.
We also need enable_guc_loading=1 when we want to verify the HuC,
which is every time we have a HuC (but all platforms with HuC have a GuC and viceversa).
v2: Clarifying the commit message (Anusha)
v3: Unify seq_puts messages, Re-factoring code as per review (Michal)
v4: Rebase
v5: Separating message unification into a separate patch
v6: Re-factoring code (Sagar, Michal)
Rebase
Suggested by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 6 +--
drivers/gpu/drm/i915/i915_drv.h | 9 +++--
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 2 +-
drivers/gpu/drm/i915/i915_params.c | 4 --
drivers/gpu/drm/i915/i915_params.h | 1 -
drivers/gpu/drm/i915/intel_guc.h | 2 +-
drivers/gpu/drm/i915/intel_guc_loader.c | 9 ++---
drivers/gpu/drm/i915/intel_huc.c | 4 +-
drivers/gpu/drm/i915/intel_uc.c | 72 +++++++++++++++++----------------
drivers/gpu/drm/i915/intel_uncore.c | 3 +-
12 files changed, 59 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9d0c27b..8abc47c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2390,7 +2390,7 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data)
struct drm_i915_private *dev_priv = node_to_i915(m->private);
struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
- if (!HAS_HUC_UCODE(dev_priv))
+ if (!HAS_GUC(dev_priv))
seq_puts(m, "not supported\n");
return 0;
@@ -2424,7 +2424,7 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
u32 tmp, i;
- if (!HAS_GUC_UCODE(dev_priv))
+ if (!HAS_GUC(dev_priv))
seq_puts(m, "not supported\n");
return 0;
@@ -2521,7 +2521,7 @@ static bool check_guc_submission(struct seq_file *m)
if (!guc->execbuf_client) {
seq_printf(m, "GuC submission %s\n",
- HAS_GUC_SCHED(dev_priv) ?
+ HAS_GUC(dev_priv) ?
"disabled" :
"not supported");
return false;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 770305b..194cbc9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3182,9 +3182,12 @@ static inline unsigned int i915_sg_segment_size(void)
*/
#define HAS_GUC(dev_priv) ((dev_priv)->info.has_guc)
#define HAS_GUC_CT(dev_priv) ((dev_priv)->info.has_guc_ct)
-#define HAS_GUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
-#define HAS_GUC_SCHED(dev_priv) (HAS_GUC(dev_priv))
-#define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
+#define HAS_GUC_UCODE(dev_priv) ((dev_priv)->guc.fw.path != NULL)
+#define HAS_HUC_UCODE(dev_priv) ((dev_priv)->huc.fw.path != NULL)
+
+#define NEEDS_GUC_LOADING(dev_priv) \
+ (HAS_GUC(dev_priv) && \
+ (i915_modparams.enable_guc_submission || HAS_HUC_UCODE(dev_priv)))
#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 5bf96a2..692d609 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -314,7 +314,7 @@ static u32 default_desc_template(const struct drm_i915_private *i915,
* present or not in use we still need a small bias as ring wraparound
* at offset 0 sometimes hangs. No idea why.
*/
- if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading)
+ if (NEEDS_GUC_LOADING(dev_priv))
ctx->ggtt_offset_bias = GUC_WOPCM_TOP;
else
ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4c60578..b71fd24 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3483,7 +3483,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
* currently don't have any bits spare to pass in this upper
* restriction!
*/
- if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading) {
+ if (NEEDS_GUC_LOADING(dev_priv)) {
ggtt->base.total = min_t(u64, ggtt->base.total, GUC_GGTT_TOP);
ggtt->mappable_end = min(ggtt->mappable_end, ggtt->base.total);
}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index de77713..19ad4dc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4022,7 +4022,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
for (i = 0; i < MAX_L3_SLICES; ++i)
dev_priv->l3_parity.remap_info[i] = NULL;
- if (HAS_GUC_SCHED(dev_priv))
+ if (NEEDS_GUC_LOADING(dev_priv))
dev_priv->pm_guc_events = GEN9_GUC_TO_HOST_INT_EVENT;
/* Let's track the enabled rps events */
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b4faeb6..1c25f45 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -162,10 +162,6 @@ struct i915_params i915_modparams __read_mostly = {
"(0=use value from vbt [default], 1=low power swing(200mV),"
"2=default swing(400mV))");
-i915_param_named_unsafe(enable_guc_loading, int, 0400,
- "Enable GuC firmware loading "
- "(-1=auto, 0=never [default], 1=if available, 2=required)");
-
i915_param_named_unsafe(enable_guc_submission, int, 0400,
"Enable GuC submission "
"(-1=auto, 0=never [default], 1=if available, 2=required)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c729226..9e1e231 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,7 +44,6 @@
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
- param(int, enable_guc_loading, 0) \
param(int, enable_guc_submission, 0) \
param(int, guc_log_level, -1) \
param(char *, guc_firmware_path, NULL) \
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index aa9a7b5..fa09c2b 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -103,7 +103,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
int intel_guc_resume(struct drm_i915_private *dev_priv);
struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-int intel_guc_select_fw(struct intel_guc *guc);
+void intel_guc_select_fw(struct intel_guc *guc);
int intel_guc_init_hw(struct intel_guc *guc);
u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c7a800a..cae8333 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -380,9 +380,8 @@ int intel_guc_init_hw(struct intel_guc *guc)
* intel_guc_select_fw() - selects GuC firmware for loading
* @guc: intel_guc struct
*
- * Return: zero when we know firmware, non-zero in other case
*/
-int intel_guc_select_fw(struct intel_guc *guc)
+void intel_guc_select_fw(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -409,9 +408,9 @@ int intel_guc_select_fw(struct intel_guc *guc)
guc->fw.major_ver_wanted = GLK_FW_MAJOR;
guc->fw.minor_ver_wanted = GLK_FW_MINOR;
} else {
- DRM_ERROR("No GuC firmware known for platform with GuC!\n");
- return -ENOENT;
+ if(HAS_GUC(dev_priv))
+ DRM_ERROR("No GUC FW known for a platform with GuC!\n");
+ return;
}
- return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 4b4cf56..7f59c8e 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -175,7 +175,9 @@ void intel_huc_select_fw(struct intel_huc *huc)
huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR;
huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR;
} else {
- DRM_ERROR("No HuC firmware known for platform with HuC!\n");
+ /* For now, everything with a GuC also has a HuC */
+ if (HAS_GUC(dev_priv))
+ DRM_ERROR("No HuC FW known for a platform with HuC!\n");
return;
}
}
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 7b938e8..b687d97 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -49,45 +49,53 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
{
+ /* Verify hardware support */
if (!HAS_GUC(dev_priv)) {
- if (i915_modparams.enable_guc_loading > 0 ||
- i915_modparams.enable_guc_submission > 0)
- DRM_INFO("Ignoring GuC options, no hardware\n");
-
- i915_modparams.enable_guc_loading = 0;
- i915_modparams.enable_guc_submission = 0;
+ if (i915_modparams.enable_guc_submission > 0)
+ DRM_INFO("Ignoring GuC submission enable, no HW\n");
+ i915_modparams.enable_guc_submission = 0;
return;
}
- /* A negative value means "use platform default" */
- if (i915_modparams.enable_guc_loading < 0)
- i915_modparams.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
+ /* Verify firmware support */
+ if (!HAS_GUC_UCODE(dev_priv)) {
+ if (i915_modparams.enable_guc_submission == 1) {
+ DRM_INFO("Ignoring GuC submission enable, no FW\n");
+ i915_modparams.enable_guc_submission = 0;
+ return;
+ }
- /* Verify firmware version */
- if (i915_modparams.enable_guc_loading) {
- if (HAS_HUC_UCODE(dev_priv))
- intel_huc_select_fw(&dev_priv->huc);
+ if (i915_modparams.enable_guc_submission < 0) {
+ i915_modparams.enable_guc_submission = 0;
+ return;
+ }
- if (intel_guc_select_fw(&dev_priv->guc))
- i915_modparams.enable_guc_loading = 0;
+ /*
+ * If "required" (> 1), let it continue and we will fail later
+ * due to the lack of firmware
+ */
}
- /* Can't enable guc submission without guc loaded */
- if (!i915_modparams.enable_guc_loading)
- i915_modparams.enable_guc_submission = 0;
-
- /* A negative value means "use platform default" */
+ /*
+ * A negative value means "use platform default" (enabled if we have
+ * survived to get here)
+ */
if (i915_modparams.enable_guc_submission < 0)
- i915_modparams.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
+ i915_modparams.enable_guc_submission = HAS_GUC(dev_priv);
+
}
void intel_uc_init_early(struct drm_i915_private *dev_priv)
{
intel_guc_init_early(&dev_priv->guc);
+ intel_guc_select_fw(&dev_priv->guc);
+ intel_huc_select_fw(&dev_priv->huc);
}
void intel_uc_init_fw(struct drm_i915_private *dev_priv)
{
+ if (!HAS_GUC(dev_priv))
+ return;
intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw);
}
@@ -154,7 +162,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
struct intel_guc *guc = &dev_priv->guc;
int ret, attempts;
- if (!i915_modparams.enable_guc_loading)
+ if (!NEEDS_GUC_LOADING(dev_priv))
return 0;
guc_disable_communication(guc);
@@ -244,19 +252,15 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
i915_ggtt_disable_guc(dev_priv);
DRM_ERROR("GuC init failed\n");
- if (i915_modparams.enable_guc_loading > 1 ||
- i915_modparams.enable_guc_submission > 1)
+ if (i915_modparams.enable_guc_submission > 1) {
+ DRM_NOTE("GuC is required, so marking the GPU as wedged\n");
ret = -EIO;
- else
- ret = 0;
-
- if (i915_modparams.enable_guc_submission) {
- i915_modparams.enable_guc_submission = 0;
+ } else if (i915_modparams.enable_guc_submission == 1) {
DRM_NOTE("Falling back from GuC submission to execlist mode\n");
- }
-
- i915_modparams.enable_guc_loading = 0;
- DRM_NOTE("GuC firmware loading disabled\n");
+ i915_modparams.enable_guc_submission = 0;
+ ret = 0;
+ } else
+ ret = 0;
return ret;
}
@@ -265,7 +269,7 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
{
guc_free_load_err_log(&dev_priv->guc);
- if (!i915_modparams.enable_guc_loading)
+ if (!NEEDS_GUC_LOADING(dev_priv))
return;
if (i915_modparams.enable_guc_submission)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 983617b..696e11f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1802,8 +1802,7 @@ int intel_guc_reset(struct drm_i915_private *dev_priv)
{
int ret;
- if (!HAS_GUC(dev_priv))
- return -EINVAL;
+ GEM_BUG_ON(!HAS_GUC(dev_priv));
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v6 3/3] drm/i915/guc : Decouple logs and ADS from submission
2017-10-10 18:32 [PATCH v6 0/3] drm/i915/guc : Removing enable_guc_loading module and Decoupling logs and ADS from submission Sujaritha Sundaresan
2017-10-10 18:32 ` [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support Sujaritha Sundaresan
2017-10-10 18:32 ` [PATCH v6 2/3] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter Sujaritha Sundaresan
@ 2017-10-10 18:32 ` Sujaritha Sundaresan
2017-10-10 19:07 ` ✗ Fi.CI.BAT: failure for drm/i915/guc : Removing enable_guc_loading module and Decoupling " Patchwork
3 siblings, 0 replies; 10+ messages in thread
From: Sujaritha Sundaresan @ 2017-10-10 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Sujaritha Sundaresan
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 16896 bytes --]
The Additional Data Struct (ADS) contains objects that are required by
guc post FW load and are not necessarily submission-only (although that's
our current only use-case). If in the future we load GuC with submission
disabled to use some other GuC feature we might still end up requiring
something inside the ADS, so it makes more sense for them to be always
created if GuC is loaded.
Similarly, we still want to access GuC logs even if GuC submission is
disable to debug issues with GuC loading or with wathever we're using
GuC for.
To make a concrete example, the pages used by GuC to save state during
suspend are allocated as part of the ADS.
v3: Group initialization of GuC objects
v2: Decoupling ADS together with logs
v3: Re-factoring code as per review (Michal)
v4: Rebase
v5: Separating group object initialization into next patch
Clarifying commit message
v6: Reverting to goto err format (Michal)
Moved guc_ads functions to dedicated file
Rebase
Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_guc_submission.c | 105 +------------------------
drivers/gpu/drm/i915/intel_guc.h | 1 +
drivers/gpu/drm/i915/intel_guc_ads.c | 120 +++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_guc_ads.h | 31 ++++++++
drivers/gpu/drm/i915/intel_guc_log.c | 6 +-
drivers/gpu/drm/i915/intel_uc.c | 40 +++++++++-
7 files changed, 196 insertions(+), 108 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_guc_ads.c
create mode 100644 drivers/gpu/drm/i915/intel_guc_ads.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 66d23b6..3aed5bf 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -62,6 +62,7 @@ i915-y += i915_cmd_parser.o \
i915-y += intel_uc.o \
intel_uc_fw.o \
intel_guc.o \
+ intel_guc_ads.o \
intel_guc_ct.o \
intel_guc_log.o \
intel_guc_loader.o \
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 31381a3..1ad1060 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -72,13 +72,6 @@
* ELSP context descriptor dword into Work Item.
* See guc_wq_item_append()
*
- * ADS:
- * The Additional Data Struct (ADS) has pointers for different buffers used by
- * the GuC. One single gem object contains the ADS struct itself (guc_ads), the
- * scheduling policies (guc_policies), a structure describing a collection of
- * register sets (guc_mmio_reg_state) and some extra pages for the GuC to save
- * its internal state for sleep.
- *
*/
static inline bool is_high_priority(struct i915_guc_client* client)
@@ -863,7 +856,7 @@ static void guc_policy_init(struct guc_policy *policy)
policy->policy_flags = 0;
}
-static void guc_policies_init(struct guc_policies *policies)
+void i915_guc_policies_init(struct guc_policies *policies)
{
struct guc_policy *policy;
u32 p, i;
@@ -883,88 +876,6 @@ static void guc_policies_init(struct guc_policies *policies)
}
/*
- * The first 80 dwords of the register state context, containing the
- * execlists and ppgtt registers.
- */
-#define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
-
-static int guc_ads_create(struct intel_guc *guc)
-{
- struct drm_i915_private *dev_priv = guc_to_i915(guc);
- struct i915_vma *vma;
- struct page *page;
- /* The ads obj includes the struct itself and buffers passed to GuC */
- struct {
- struct guc_ads ads;
- struct guc_policies policies;
- struct guc_mmio_reg_state reg_state;
- u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
- } __packed *blob;
- struct intel_engine_cs *engine;
- enum intel_engine_id id;
- const u32 skipped_offset = LRC_HEADER_PAGES * PAGE_SIZE;
- const u32 skipped_size = LRC_PPHWSP_SZ * PAGE_SIZE + LR_HW_CONTEXT_SIZE;
- u32 base;
-
- GEM_BUG_ON(guc->ads_vma);
-
- vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(sizeof(*blob)));
- if (IS_ERR(vma))
- return PTR_ERR(vma);
-
- guc->ads_vma = vma;
-
- page = i915_vma_first_page(vma);
- blob = kmap(page);
-
- /* GuC scheduling policies */
- guc_policies_init(&blob->policies);
-
- /* MMIO reg state */
- for_each_engine(engine, dev_priv, id) {
- blob->reg_state.white_list[engine->guc_id].mmio_start =
- engine->mmio_base + GUC_MMIO_WHITE_LIST_START;
-
- /* Nothing to be saved or restored for now. */
- blob->reg_state.white_list[engine->guc_id].count = 0;
- }
-
- /*
- * The GuC requires a "Golden Context" when it reinitialises
- * engines after a reset. Here we use the Render ring default
- * context, which must already exist and be pinned in the GGTT,
- * so its address won't change after we've told the GuC where
- * to find it. Note that we have to skip our header (1 page),
- * because our GuC shared data is there.
- */
- blob->ads.golden_context_lrca =
- guc_ggtt_offset(dev_priv->kernel_context->engine[RCS].state) + skipped_offset;
-
- /*
- * The GuC expects us to exclude the portion of the context image that
- * it skips from the size it is to read. It starts reading from after
- * the execlist context (so skipping the first page [PPHWSP] and 80
- * dwords). Weird guc is weird.
- */
- for_each_engine(engine, dev_priv, id)
- blob->ads.eng_state_size[engine->guc_id] = engine->context_size - skipped_size;
-
- base = guc_ggtt_offset(vma);
- blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
- blob->ads.reg_state_buffer = base + ptr_offset(blob, reg_state_buffer);
- blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
-
- kunmap(page);
-
- return 0;
-}
-
-static void guc_ads_destroy(struct intel_guc *guc)
-{
- i915_vma_unpin_and_release(&guc->ads_vma);
-}
-
-/*
* Set up the memory resources to be shared with the GuC (via the GGTT)
* at firmware loading time.
*/
@@ -994,22 +905,10 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
guc->stage_desc_pool_vaddr = vaddr;
- ret = intel_guc_log_create(guc);
- if (ret < 0)
- goto err_vaddr;
-
- ret = guc_ads_create(guc);
- if (ret < 0)
- goto err_log;
-
ida_init(&guc->stage_ids);
return 0;
-err_log:
- intel_guc_log_destroy(guc);
-err_vaddr:
- i915_gem_object_unpin_map(guc->stage_desc_pool->obj);
err_vma:
i915_vma_unpin_and_release(&guc->stage_desc_pool);
return ret;
@@ -1020,8 +919,6 @@ void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
struct intel_guc *guc = &dev_priv->guc;
ida_destroy(&guc->stage_ids);
- guc_ads_destroy(guc);
- intel_guc_log_destroy(guc);
i915_gem_object_unpin_map(guc->stage_desc_pool->obj);
i915_vma_unpin_and_release(&guc->stage_desc_pool);
}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index fa09c2b..24ff14f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -102,6 +102,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
int intel_guc_suspend(struct drm_i915_private *dev_priv);
int intel_guc_resume(struct drm_i915_private *dev_priv);
struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
+void i915_guc_policies_init(struct guc_policies *policies);
void intel_guc_select_fw(struct intel_guc *guc);
int intel_guc_init_hw(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c b/drivers/gpu/drm/i915/intel_guc_ads.c
new file mode 100644
index 0000000..3acf319
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_ads.c
@@ -0,0 +1,120 @@
+/*
+ * Copyright © 2014-2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "intel_uc.h"
+#include "i915_drv.h"
+#include "intel_guc.h"
+#include "i915_guc_submission.h"
+
+/*
+ * ADS:
+ * The Additional Data Struct (ADS) has pointers for different buffers used by
+ * the GuC. One single gem object contains the ADS struct itself (guc_ads), the
+ * scheduling policies (guc_policies), a structure describing a collection of
+ * register sets (guc_mmio_reg_state) and some extra pages for the GuC to save
+ * its internal state for sleep.
+ *
+ */
+
+/*
+ * The first 80 dwords of the register state context, containing the
+ * execlists and ppgtt registers.
+ */
+#define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
+
+int guc_ads_create(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ struct i915_vma *vma;
+ struct page *page;
+ /* The ads obj includes the struct itself and buffers passed to GuC */
+ struct {
+ struct guc_ads ads;
+ struct guc_policies policies;
+ struct guc_mmio_reg_state reg_state;
+ u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
+ } __packed *blob;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ const u32 skipped_offset = LRC_HEADER_PAGES * PAGE_SIZE;
+ const u32 skipped_size = LRC_PPHWSP_SZ * PAGE_SIZE + LR_HW_CONTEXT_SIZE;
+ u32 base;
+
+ GEM_BUG_ON(guc->ads_vma);
+
+ vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(sizeof(*blob)));
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
+
+ guc->ads_vma = vma;
+
+ page = i915_vma_first_page(vma);
+ blob = kmap(page);
+
+ /* GuC scheduling policies */
+ i915_guc_policies_init(&blob->policies);
+
+ /* MMIO reg state */
+ for_each_engine(engine, dev_priv, id) {
+ blob->reg_state.white_list[engine->guc_id].mmio_start =
+ engine->mmio_base + GUC_MMIO_WHITE_LIST_START;
+
+ /* Nothing to be saved or restored for now. */
+ blob->reg_state.white_list[engine->guc_id].count = 0;
+ }
+
+ /*
+ * The GuC requires a "Golden Context" when it reinitialises
+ * engines after a reset. Here we use the Render ring default
+ * context, which must already exist and be pinned in the GGTT,
+ * so its address won't change after we've told the GuC where
+ * to find it. Note that we have to skip our header (1 page),
+ * because our GuC shared data is there.
+ */
+ blob->ads.golden_context_lrca =
+ guc_ggtt_offset(dev_priv->kernel_context->engine[RCS].state) + skipped_offset;
+
+ /*
+ * The GuC expects us to exclude the portion of the context image that
+ * it skips from the size it is to read. It starts reading from after
+ * the execlist context (so skipping the first page [PPHWSP] and 80
+ * dwords). Weird guc is weird.
+ */
+ for_each_engine(engine, dev_priv, id)
+ blob->ads.eng_state_size[engine->guc_id] = engine->context_size - skipped_size;
+
+ base = guc_ggtt_offset(vma);
+ blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
+ blob->ads.reg_state_buffer = base + ptr_offset(blob, reg_state_buffer);
+ blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
+
+ kunmap(page);
+
+ return 0;
+}
+
+void guc_ads_destroy(struct intel_guc *guc)
+{
+ i915_vma_unpin_and_release(&guc->ads_vma);
+}
diff --git a/drivers/gpu/drm/i915/intel_guc_ads.h b/drivers/gpu/drm/i915/intel_guc_ads.h
new file mode 100644
index 0000000..ad4d9a0
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_ads.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright © 2014-2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _INTEL_GUC_ADS_H_
+#define _INTEL_GUC_ADS_H_
+
+int guc_ads_create(struct intel_guc *guc);
+void guc_ads_destroy(struct intel_guc *guc);
+
+#endif
\ No newline at end of file
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 76d3eb1..1616fdb 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -505,7 +505,7 @@ static void guc_flush_logs(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
- if (!i915_modparams.enable_guc_submission ||
+ if (!NEEDS_GUC_LOADING(dev_priv) ||
(i915_modparams.guc_log_level < 0))
return;
@@ -646,7 +646,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
void i915_guc_log_register(struct drm_i915_private *dev_priv)
{
- if (!i915_modparams.enable_guc_submission ||
+ if (!NEEDS_GUC_LOADING(dev_priv) ||
(i915_modparams.guc_log_level < 0))
return;
@@ -657,7 +657,7 @@ void i915_guc_log_register(struct drm_i915_private *dev_priv)
void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
{
- if (!i915_modparams.enable_guc_submission)
+ if (!NEEDS_GUC_LOADING(dev_priv))
return;
mutex_lock(&dev_priv->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index b687d97..50c1434 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -24,6 +24,7 @@
#include "intel_uc.h"
#include "i915_drv.h"
+#include "intel_guc_ads.h"
#include "i915_guc_submission.h"
/* Reset GuC providing us with fresh state for both GuC and HuC.
@@ -157,6 +158,36 @@ static void guc_disable_communication(struct intel_guc *guc)
guc->send = intel_guc_send_nop;
}
+static int guc_shared_objects_init(struct intel_guc *guc)
+{
+ int ret;
+
+ if (guc->ads_vma)
+ return 0;
+
+ ret = intel_guc_log_create(guc);
+ if (ret < 0)
+ goto err_vaddr;
+
+ ret = guc_ads_create(guc);
+ if (ret < 0)
+ goto err_log;
+
+ return ret;
+
+err_log:
+ intel_guc_log_destroy(guc);
+err_vaddr:
+ i915_gem_object_unpin_map(guc->stage_desc_pool->obj);
+ return ret;
+}
+
+static void guc_shared_objects_fini(struct intel_guc *guc)
+{
+ guc_ads_destroy(guc);
+ intel_guc_log_destroy(guc);
+}
+
int intel_uc_init_hw(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
@@ -171,6 +202,10 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
/* We need to notify the guc whenever we change the GGTT */
i915_ggtt_enable_guc(dev_priv);
+ ret = guc_shared_objects_init(guc);
+ if (ret)
+ goto err_guc;
+
if (i915_modparams.enable_guc_submission) {
/*
* This is stuff we need to have available at fw load time
@@ -178,7 +213,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
*/
ret = i915_guc_submission_init(dev_priv);
if (ret)
- goto err_guc;
+ goto err_shared;
}
/* init WOPCM */
@@ -248,6 +283,8 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
err_submission:
if (i915_modparams.enable_guc_submission)
i915_guc_submission_fini(dev_priv);
+err_shared:
+ guc_shared_objects_fini(guc);
err_guc:
i915_ggtt_disable_guc(dev_priv);
@@ -282,5 +319,6 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
i915_guc_submission_fini(dev_priv);
}
+ guc_shared_objects_fini(&dev_priv->guc);
i915_ggtt_disable_guc(dev_priv);
}
--
1.9.1
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/guc : Removing enable_guc_loading module and Decoupling logs and ADS from submission
2017-10-10 18:32 [PATCH v6 0/3] drm/i915/guc : Removing enable_guc_loading module and Decoupling logs and ADS from submission Sujaritha Sundaresan
` (2 preceding siblings ...)
2017-10-10 18:32 ` [PATCH v6 3/3] drm/i915/guc : Decouple logs and ADS from submission Sujaritha Sundaresan
@ 2017-10-10 19:07 ` Patchwork
3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-10-10 19:07 UTC (permalink / raw)
To: Sujaritha Sundaresan; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/guc : Removing enable_guc_loading module and Decoupling logs and ADS from submission
URL : https://patchwork.freedesktop.org/series/31677/
State : failure
== Summary ==
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CHK include/generated/bounds.h
CHK include/generated/timeconst.h
CHK include/generated/asm-offsets.h
CALL scripts/checksyscalls.sh
CHK scripts/mod/devicetable-offsets.h
CHK include/generated/compile.h
CHK kernel/config_data.h
CC [M] drivers/gpu/drm/i915/i915_debugfs.o
drivers/gpu/drm/i915/i915_debugfs.c: In function ‘i915_huc_load_status_info’:
drivers/gpu/drm/i915/i915_debugfs.c:2393:2: error: this ‘if’ clause does not guard... [-Werror=misleading-indentation]
if (!HAS_GUC(dev_priv))
^~
drivers/gpu/drm/i915/i915_debugfs.c:2395:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘if’
return 0;
^~~~~~
drivers/gpu/drm/i915/i915_debugfs.c: In function ‘i915_guc_load_status_info’:
drivers/gpu/drm/i915/i915_debugfs.c:2427:2: error: this ‘if’ clause does not guard... [-Werror=misleading-indentation]
if (!HAS_GUC(dev_priv))
^~
drivers/gpu/drm/i915/i915_debugfs.c:2429:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘if’
return 0;
^~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:313: recipe for target 'drivers/gpu/drm/i915/i915_debugfs.o' failed
make[4]: *** [drivers/gpu/drm/i915/i915_debugfs.o] Error 1
scripts/Makefile.build:572: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:572: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:572: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1019: recipe for target 'drivers' failed
make: *** [drivers] Error 2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support
2017-10-10 18:32 ` [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support Sujaritha Sundaresan
@ 2017-10-13 9:05 ` Sagar Arun Kamble
2017-10-16 16:33 ` Sujaritha
2017-10-16 17:12 ` Sujaritha
0 siblings, 2 replies; 10+ messages in thread
From: Sagar Arun Kamble @ 2017-10-13 9:05 UTC (permalink / raw)
To: Sujaritha Sundaresan, intel-gfx
Keep subject as "drm/i915" as this is generic change. Also I saw
i915_runtime_pm_status debugfs output not updated.
Could you please check.
On 10/11/2017 12:02 AM, Sujaritha Sundaresan wrote:
> Unifying the various seq_puts messages in debugfs to the simplest one for
> feature support.
>
> v2: Clarifying the commit message (Anusha)
>
> v3: Re-factoring code as per review (Michal)
>
> v4: Rebase
>
> v5: Split from following patch
>
> v6: Re-factoring code (Michal, Sagar)
> Clarifying commit message (Sagar)
>
> Suggested by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5b58d2b..9d0c27b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1670,7 +1670,7 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
>
> if (!HAS_FBC(dev_priv)) {
> - seq_puts(m, "FBC unsupported on this chipset\n");
> + seq_puts(m, "not supported\n");
> return 0;
> }
>
> @@ -1837,7 +1837,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
> unsigned int max_gpu_freq, min_gpu_freq;
>
> if (!HAS_LLC(dev_priv)) {
> - seq_puts(m, "unsupported on this chipset\n");
> + seq_puts(m, "not supported\n");
> return 0;
> }
>
> @@ -2391,6 +2391,7 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data)
> struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
>
> if (!HAS_HUC_UCODE(dev_priv))
> + seq_puts(m, "not supported\n");
> return 0;
>
> seq_puts(m, "HuC firmware status:\n");
> @@ -2424,6 +2425,7 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
> u32 tmp, i;
>
> if (!HAS_GUC_UCODE(dev_priv))
> + seq_puts(m, "not supported\n");
> return 0;
>
> seq_printf(m, "GuC firmware status:\n");
> @@ -2708,7 +2710,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> bool enabled = false;
>
> if (!HAS_PSR(dev_priv)) {
> - seq_puts(m, "PSR not supported\n");
> + seq_puts(m, "not supported\n");
> return 0;
> }
>
> @@ -3565,7 +3567,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
>
> mutex_lock(&drrs->mutex);
> /* DRRS Supported */
> - seq_puts(m, "\tDRRS Supported: Yes\n");
> + seq_puts(m, "supported\n");
>
> /* disable_drrs() will make drrs->dp NULL */
> if (!drrs->dp) {
> @@ -3597,7 +3599,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
> mutex_unlock(&drrs->mutex);
> } else {
> /* DRRS not supported. Print the VBT parameter*/
> - seq_puts(m, "\tDRRS Supported : No");
> + seq_puts(m, "not supported\n");
> }
> seq_puts(m, "\n");
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 2/3] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter
2017-10-10 18:32 ` [PATCH v6 2/3] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter Sujaritha Sundaresan
@ 2017-10-13 9:42 ` Sagar Arun Kamble
2017-10-16 16:36 ` Sujaritha
0 siblings, 1 reply; 10+ messages in thread
From: Sagar Arun Kamble @ 2017-10-13 9:42 UTC (permalink / raw)
To: Sujaritha Sundaresan, intel-gfx
On 10/11/2017 12:02 AM, Sujaritha Sundaresan wrote:
> We currently have two module parameters that control GuC: "enable_guc_loading" and "enable_guc_submission".
> Whenever we need i915_modparams.enable_guc_submission=1, we also need enable_guc_loading=1.
> We also need enable_guc_loading=1 when we want to verify the HuC,
> which is every time we have a HuC (but all platforms with HuC have a GuC and viceversa).
>
> v2: Clarifying the commit message (Anusha)
>
> v3: Unify seq_puts messages, Re-factoring code as per review (Michal)
>
> v4: Rebase
>
> v5: Separating message unification into a separate patch
>
> v6: Re-factoring code (Sagar, Michal)
> Rebase
>
> Suggested by: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +--
> drivers/gpu/drm/i915/i915_drv.h | 9 +++--
> drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
> drivers/gpu/drm/i915/i915_irq.c | 2 +-
> drivers/gpu/drm/i915/i915_params.c | 4 --
> drivers/gpu/drm/i915/i915_params.h | 1 -
> drivers/gpu/drm/i915/intel_guc.h | 2 +-
> drivers/gpu/drm/i915/intel_guc_loader.c | 9 ++---
> drivers/gpu/drm/i915/intel_huc.c | 4 +-
> drivers/gpu/drm/i915/intel_uc.c | 72 +++++++++++++++++----------------
> drivers/gpu/drm/i915/intel_uncore.c | 3 +-
> 12 files changed, 59 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9d0c27b..8abc47c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2390,7 +2390,7 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data)
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
>
> - if (!HAS_HUC_UCODE(dev_priv))
> + if (!HAS_GUC(dev_priv))
> seq_puts(m, "not supported\n");
> return 0;
>
> @@ -2424,7 +2424,7 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
> struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
> u32 tmp, i;
>
> - if (!HAS_GUC_UCODE(dev_priv))
> + if (!HAS_GUC(dev_priv))
> seq_puts(m, "not supported\n");
> return 0;
>
> @@ -2521,7 +2521,7 @@ static bool check_guc_submission(struct seq_file *m)
>
> if (!guc->execbuf_client) {
> seq_printf(m, "GuC submission %s\n",
> - HAS_GUC_SCHED(dev_priv) ?
> + HAS_GUC(dev_priv) ?
> "disabled" :
> "not supported");
> return false;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 770305b..194cbc9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3182,9 +3182,12 @@ static inline unsigned int i915_sg_segment_size(void)
> */
> #define HAS_GUC(dev_priv) ((dev_priv)->info.has_guc)
> #define HAS_GUC_CT(dev_priv) ((dev_priv)->info.has_guc_ct)
> -#define HAS_GUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
> -#define HAS_GUC_SCHED(dev_priv) (HAS_GUC(dev_priv))
> -#define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
> +#define HAS_GUC_UCODE(dev_priv) ((dev_priv)->guc.fw.path != NULL)
> +#define HAS_HUC_UCODE(dev_priv) ((dev_priv)->huc.fw.path != NULL)
> +
> +#define NEEDS_GUC_LOADING(dev_priv) \
> + (HAS_GUC(dev_priv) && \
> + (i915_modparams.enable_guc_submission || HAS_HUC_UCODE(dev_priv)))
>
> #define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 5bf96a2..692d609 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -314,7 +314,7 @@ static u32 default_desc_template(const struct drm_i915_private *i915,
> * present or not in use we still need a small bias as ring wraparound
> * at offset 0 sometimes hangs. No idea why.
> */
> - if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading)
> + if (NEEDS_GUC_LOADING(dev_priv))
> ctx->ggtt_offset_bias = GUC_WOPCM_TOP;
> else
> ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4c60578..b71fd24 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3483,7 +3483,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
> * currently don't have any bits spare to pass in this upper
> * restriction!
> */
> - if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading) {
> + if (NEEDS_GUC_LOADING(dev_priv)) {
> ggtt->base.total = min_t(u64, ggtt->base.total, GUC_GGTT_TOP);
> ggtt->mappable_end = min(ggtt->mappable_end, ggtt->base.total);
> }
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index de77713..19ad4dc 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4022,7 +4022,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> for (i = 0; i < MAX_L3_SLICES; ++i)
> dev_priv->l3_parity.remap_info[i] = NULL;
>
> - if (HAS_GUC_SCHED(dev_priv))
> + if (NEEDS_GUC_LOADING(dev_priv))
> dev_priv->pm_guc_events = GEN9_GUC_TO_HOST_INT_EVENT;
At this point enable_guc_submission parameter is not sanitized. it is
sanitized during driver_init_hw that
happens post driver_init_early(intel_irq_init). So I think this should
be only HAS_GUC.
>
> /* Let's track the enabled rps events */
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index b4faeb6..1c25f45 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -162,10 +162,6 @@ struct i915_params i915_modparams __read_mostly = {
> "(0=use value from vbt [default], 1=low power swing(200mV),"
> "2=default swing(400mV))");
>
> -i915_param_named_unsafe(enable_guc_loading, int, 0400,
> - "Enable GuC firmware loading "
> - "(-1=auto, 0=never [default], 1=if available, 2=required)");
> -
> i915_param_named_unsafe(enable_guc_submission, int, 0400,
> "Enable GuC submission "
> "(-1=auto, 0=never [default], 1=if available, 2=required)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index c729226..9e1e231 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -44,7 +44,6 @@
> param(int, disable_power_well, -1) \
> param(int, enable_ips, 1) \
> param(int, invert_brightness, 0) \
> - param(int, enable_guc_loading, 0) \
> param(int, enable_guc_submission, 0) \
> param(int, guc_log_level, -1) \
> param(char *, guc_firmware_path, NULL) \
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index aa9a7b5..fa09c2b 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -103,7 +103,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> int intel_guc_resume(struct drm_i915_private *dev_priv);
> struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
>
> -int intel_guc_select_fw(struct intel_guc *guc);
> +void intel_guc_select_fw(struct intel_guc *guc);
> int intel_guc_init_hw(struct intel_guc *guc);
> u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index c7a800a..cae8333 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -380,9 +380,8 @@ int intel_guc_init_hw(struct intel_guc *guc)
> * intel_guc_select_fw() - selects GuC firmware for loading
> * @guc: intel_guc struct
> *
> - * Return: zero when we know firmware, non-zero in other case
This change can be a new patch and since this/most of the patch is
depending on Michal's series I would suggest to post the series once
that gets merged.
> */
> -int intel_guc_select_fw(struct intel_guc *guc)
> +void intel_guc_select_fw(struct intel_guc *guc)
> {
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
>
> @@ -409,9 +408,9 @@ int intel_guc_select_fw(struct intel_guc *guc)
> guc->fw.major_ver_wanted = GLK_FW_MAJOR;
> guc->fw.minor_ver_wanted = GLK_FW_MINOR;
> } else {
> - DRM_ERROR("No GuC firmware known for platform with GuC!\n");
> - return -ENOENT;
> + if(HAS_GUC(dev_priv))
> + DRM_ERROR("No GUC FW known for a platform with GuC!\n");
> + return;
> }
>
> - return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 4b4cf56..7f59c8e 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -175,7 +175,9 @@ void intel_huc_select_fw(struct intel_huc *huc)
> huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR;
> huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR;
> } else {
> - DRM_ERROR("No HuC firmware known for platform with HuC!\n");
> + /* For now, everything with a GuC also has a HuC */
> + if (HAS_GUC(dev_priv))
> + DRM_ERROR("No HuC FW known for a platform with HuC!\n");
> return;
> }
> }
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 7b938e8..b687d97 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -49,45 +49,53 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
>
> void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
> {
> + /* Verify hardware support */
> if (!HAS_GUC(dev_priv)) {
> - if (i915_modparams.enable_guc_loading > 0 ||
> - i915_modparams.enable_guc_submission > 0)
> - DRM_INFO("Ignoring GuC options, no hardware\n");
> -
> - i915_modparams.enable_guc_loading = 0;
> - i915_modparams.enable_guc_submission = 0;
> + if (i915_modparams.enable_guc_submission > 0)
> + DRM_INFO("Ignoring GuC submission enable, no HW\n");
Can we say option in the message - "Ignoring GuC submission enable
option, no hardware"
split the message over two lines.
> + i915_modparams.enable_guc_submission = 0;
> return;
> }
>
> - /* A negative value means "use platform default" */
> - if (i915_modparams.enable_guc_loading < 0)
> - i915_modparams.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
> + /* Verify firmware support */
> + if (!HAS_GUC_UCODE(dev_priv)) {
> + if (i915_modparams.enable_guc_submission == 1) {
> + DRM_INFO("Ignoring GuC submission enable, no FW\n");
> + i915_modparams.enable_guc_submission = 0;
> + return;
this return is not aligned properly. code indentation needs to updated.
please run "scripts/checkpatch.pl --strict" on all patches that helps
identify such issues faster.
> + }
>
> - /* Verify firmware version */
> - if (i915_modparams.enable_guc_loading) {
> - if (HAS_HUC_UCODE(dev_priv))
> - intel_huc_select_fw(&dev_priv->huc);
> + if (i915_modparams.enable_guc_submission < 0) {
> + i915_modparams.enable_guc_submission = 0;
> + return;
> + }
>
> - if (intel_guc_select_fw(&dev_priv->guc))
> - i915_modparams.enable_guc_loading = 0;
> + /*
> + * If "required" (> 1), let it continue and we will fail later
> + * due to the lack of firmware
> + */
> }
>
> - /* Can't enable guc submission without guc loaded */
> - if (!i915_modparams.enable_guc_loading)
> - i915_modparams.enable_guc_submission = 0;
> -
> - /* A negative value means "use platform default" */
> + /*
> + * A negative value means "use platform default" (enabled if we have
> + * survived to get here)
> + */
> if (i915_modparams.enable_guc_submission < 0)
> - i915_modparams.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
> + i915_modparams.enable_guc_submission = HAS_GUC(dev_priv);
> +
> }
>
> void intel_uc_init_early(struct drm_i915_private *dev_priv)
> {
> intel_guc_init_early(&dev_priv->guc);
> + intel_guc_select_fw(&dev_priv->guc);
> + intel_huc_select_fw(&dev_priv->huc);
> }
>
> void intel_uc_init_fw(struct drm_i915_private *dev_priv)
> {
> + if (!HAS_GUC(dev_priv))
> + return;
> intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
> intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw);
> }
> @@ -154,7 +162,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> struct intel_guc *guc = &dev_priv->guc;
> int ret, attempts;
>
> - if (!i915_modparams.enable_guc_loading)
> + if (!NEEDS_GUC_LOADING(dev_priv))
> return 0;
>
> guc_disable_communication(guc);
> @@ -244,19 +252,15 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> i915_ggtt_disable_guc(dev_priv);
>
> DRM_ERROR("GuC init failed\n");
> - if (i915_modparams.enable_guc_loading > 1 ||
> - i915_modparams.enable_guc_submission > 1)
> + if (i915_modparams.enable_guc_submission > 1) {
> + DRM_NOTE("GuC is required, so marking the GPU as wedged\n");
> ret = -EIO;
> - else
> - ret = 0;
> -
> - if (i915_modparams.enable_guc_submission) {
> - i915_modparams.enable_guc_submission = 0;
> + } else if (i915_modparams.enable_guc_submission == 1) {
> DRM_NOTE("Falling back from GuC submission to execlist mode\n");
> - }
> -
> - i915_modparams.enable_guc_loading = 0;
> - DRM_NOTE("GuC firmware loading disabled\n");
> + i915_modparams.enable_guc_submission = 0;
> + ret = 0;
> + } else
> + ret = 0;
>
> return ret;
> }
> @@ -265,7 +269,7 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> {
> guc_free_load_err_log(&dev_priv->guc);
>
> - if (!i915_modparams.enable_guc_loading)
> + if (!NEEDS_GUC_LOADING(dev_priv))
> return;
>
> if (i915_modparams.enable_guc_submission)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 983617b..696e11f 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1802,8 +1802,7 @@ int intel_guc_reset(struct drm_i915_private *dev_priv)
> {
> int ret;
>
> - if (!HAS_GUC(dev_priv))
> - return -EINVAL;
> + GEM_BUG_ON(!HAS_GUC(dev_priv));
>
> intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC);
Overall patch logic looks fine. Will need some more updates/rebase later
as suggested.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support
2017-10-13 9:05 ` Sagar Arun Kamble
@ 2017-10-16 16:33 ` Sujaritha
2017-10-16 17:12 ` Sujaritha
1 sibling, 0 replies; 10+ messages in thread
From: Sujaritha @ 2017-10-16 16:33 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx
On 10/13/2017 02:05 AM, Sagar Arun Kamble wrote:
> Keep subject as "drm/i915" as this is generic change. Also I saw
> i915_runtime_pm_status debugfs output not updated.
>
> Could you please check.
>
I will change the subject and check the if the output is being updated.
>
>
> On 10/11/2017 12:02 AM, Sujaritha Sundaresan wrote:
>> Unifying the various seq_puts messages in debugfs to the simplest one
>> for
>> feature support.
>>
>> v2: Clarifying the commit message (Anusha)
>>
>> v3: Re-factoring code as per review (Michal)
>>
>> v4: Rebase
>>
>> v5: Split from following patch
>>
>> v6: Re-factoring code (Michal, Sagar)
>> Clarifying commit message (Sagar)
>>
>> Suggested by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 5b58d2b..9d0c27b 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1670,7 +1670,7 @@ static int i915_fbc_status(struct seq_file *m,
>> void *unused)
>> struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> if (!HAS_FBC(dev_priv)) {
>> - seq_puts(m, "FBC unsupported on this chipset\n");
>> + seq_puts(m, "not supported\n");
>> return 0;
>> }
>> @@ -1837,7 +1837,7 @@ static int i915_ring_freq_table(struct
>> seq_file *m, void *unused)
>> unsigned int max_gpu_freq, min_gpu_freq;
>> if (!HAS_LLC(dev_priv)) {
>> - seq_puts(m, "unsupported on this chipset\n");
>> + seq_puts(m, "not supported\n");
>> return 0;
>> }
>> @@ -2391,6 +2391,7 @@ static int i915_huc_load_status_info(struct
>> seq_file *m, void *data)
>> struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
>> if (!HAS_HUC_UCODE(dev_priv))
>> + seq_puts(m, "not supported\n");
>> return 0;
>> seq_puts(m, "HuC firmware status:\n");
>> @@ -2424,6 +2425,7 @@ static int i915_guc_load_status_info(struct
>> seq_file *m, void *data)
>> u32 tmp, i;
>> if (!HAS_GUC_UCODE(dev_priv))
>> + seq_puts(m, "not supported\n");
>> return 0;
>> seq_printf(m, "GuC firmware status:\n");
>> @@ -2708,7 +2710,7 @@ static int i915_edp_psr_status(struct seq_file
>> *m, void *data)
>> bool enabled = false;
>> if (!HAS_PSR(dev_priv)) {
>> - seq_puts(m, "PSR not supported\n");
>> + seq_puts(m, "not supported\n");
>> return 0;
>> }
>> @@ -3565,7 +3567,7 @@ static void drrs_status_per_crtc(struct
>> seq_file *m,
>> mutex_lock(&drrs->mutex);
>> /* DRRS Supported */
>> - seq_puts(m, "\tDRRS Supported: Yes\n");
>> + seq_puts(m, "supported\n");
>> /* disable_drrs() will make drrs->dp NULL */
>> if (!drrs->dp) {
>> @@ -3597,7 +3599,7 @@ static void drrs_status_per_crtc(struct
>> seq_file *m,
>> mutex_unlock(&drrs->mutex);
>> } else {
>> /* DRRS not supported. Print the VBT parameter*/
>> - seq_puts(m, "\tDRRS Supported : No");
>> + seq_puts(m, "not supported\n");
>> }
>> seq_puts(m, "\n");
>> }
>
Thanks for the review,
Regards,
Sujaritha
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 2/3] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter
2017-10-13 9:42 ` Sagar Arun Kamble
@ 2017-10-16 16:36 ` Sujaritha
0 siblings, 0 replies; 10+ messages in thread
From: Sujaritha @ 2017-10-16 16:36 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx
On 10/13/2017 02:42 AM, Sagar Arun Kamble wrote:
>
>
> On 10/11/2017 12:02 AM, Sujaritha Sundaresan wrote:
>> We currently have two module parameters that control GuC:
>> "enable_guc_loading" and "enable_guc_submission".
>> Whenever we need i915_modparams.enable_guc_submission=1, we also need
>> enable_guc_loading=1.
>> We also need enable_guc_loading=1 when we want to verify the HuC,
>> which is every time we have a HuC (but all platforms with HuC have a
>> GuC and viceversa).
>>
>> v2: Clarifying the commit message (Anusha)
>>
>> v3: Unify seq_puts messages, Re-factoring code as per review (Michal)
>>
>> v4: Rebase
>>
>> v5: Separating message unification into a separate patch
>>
>> v6: Re-factoring code (Sagar, Michal)
>> Rebase
>>
>> Suggested by: Oscar Mateo <oscar.mateo@intel.com>
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 6 +--
>> drivers/gpu/drm/i915/i915_drv.h | 9 +++--
>> drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
>> drivers/gpu/drm/i915/i915_irq.c | 2 +-
>> drivers/gpu/drm/i915/i915_params.c | 4 --
>> drivers/gpu/drm/i915/i915_params.h | 1 -
>> drivers/gpu/drm/i915/intel_guc.h | 2 +-
>> drivers/gpu/drm/i915/intel_guc_loader.c | 9 ++---
>> drivers/gpu/drm/i915/intel_huc.c | 4 +-
>> drivers/gpu/drm/i915/intel_uc.c | 72
>> +++++++++++++++++----------------
>> drivers/gpu/drm/i915/intel_uncore.c | 3 +-
>> 12 files changed, 59 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 9d0c27b..8abc47c 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2390,7 +2390,7 @@ static int i915_huc_load_status_info(struct
>> seq_file *m, void *data)
>> struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
>> - if (!HAS_HUC_UCODE(dev_priv))
>> + if (!HAS_GUC(dev_priv))
>> seq_puts(m, "not supported\n");
>> return 0;
>> @@ -2424,7 +2424,7 @@ static int i915_guc_load_status_info(struct
>> seq_file *m, void *data)
>> struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
>> u32 tmp, i;
>> - if (!HAS_GUC_UCODE(dev_priv))
>> + if (!HAS_GUC(dev_priv))
>> seq_puts(m, "not supported\n");
>> return 0;
>> @@ -2521,7 +2521,7 @@ static bool check_guc_submission(struct
>> seq_file *m)
>> if (!guc->execbuf_client) {
>> seq_printf(m, "GuC submission %s\n",
>> - HAS_GUC_SCHED(dev_priv) ?
>> + HAS_GUC(dev_priv) ?
>> "disabled" :
>> "not supported");
>> return false;
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 770305b..194cbc9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3182,9 +3182,12 @@ static inline unsigned int
>> i915_sg_segment_size(void)
>> */
>> #define HAS_GUC(dev_priv) ((dev_priv)->info.has_guc)
>> #define HAS_GUC_CT(dev_priv) ((dev_priv)->info.has_guc_ct)
>> -#define HAS_GUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
>> -#define HAS_GUC_SCHED(dev_priv) (HAS_GUC(dev_priv))
>> -#define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv))
>> +#define HAS_GUC_UCODE(dev_priv) ((dev_priv)->guc.fw.path != NULL)
>> +#define HAS_HUC_UCODE(dev_priv) ((dev_priv)->huc.fw.path != NULL)
>> +
>> +#define NEEDS_GUC_LOADING(dev_priv) \
>> + (HAS_GUC(dev_priv) && \
>> + (i915_modparams.enable_guc_submission || HAS_HUC_UCODE(dev_priv)))
>> #define HAS_RESOURCE_STREAMER(dev_priv)
>> ((dev_priv)->info.has_resource_streamer)
>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
>> b/drivers/gpu/drm/i915/i915_gem_context.c
>> index 5bf96a2..692d609 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -314,7 +314,7 @@ static u32 default_desc_template(const struct
>> drm_i915_private *i915,
>> * present or not in use we still need a small bias as ring
>> wraparound
>> * at offset 0 sometimes hangs. No idea why.
>> */
>> - if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading)
>> + if (NEEDS_GUC_LOADING(dev_priv))
>> ctx->ggtt_offset_bias = GUC_WOPCM_TOP;
>> else
>> ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 4c60578..b71fd24 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -3483,7 +3483,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private
>> *dev_priv)
>> * currently don't have any bits spare to pass in this upper
>> * restriction!
>> */
>> - if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading) {
>> + if (NEEDS_GUC_LOADING(dev_priv)) {
>> ggtt->base.total = min_t(u64, ggtt->base.total, GUC_GGTT_TOP);
>> ggtt->mappable_end = min(ggtt->mappable_end,
>> ggtt->base.total);
>> }
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c
>> b/drivers/gpu/drm/i915/i915_irq.c
>> index de77713..19ad4dc 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -4022,7 +4022,7 @@ void intel_irq_init(struct drm_i915_private
>> *dev_priv)
>> for (i = 0; i < MAX_L3_SLICES; ++i)
>> dev_priv->l3_parity.remap_info[i] = NULL;
>> - if (HAS_GUC_SCHED(dev_priv))
>> + if (NEEDS_GUC_LOADING(dev_priv))
>> dev_priv->pm_guc_events = GEN9_GUC_TO_HOST_INT_EVENT;
> At this point enable_guc_submission parameter is not sanitized. it is
> sanitized during driver_init_hw that
> happens post driver_init_early(intel_irq_init). So I think this should
> be only HAS_GUC.
Thanks for pointing this out. I will make the change.
>> /* Let's track the enabled rps events */
>> diff --git a/drivers/gpu/drm/i915/i915_params.c
>> b/drivers/gpu/drm/i915/i915_params.c
>> index b4faeb6..1c25f45 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -162,10 +162,6 @@ struct i915_params i915_modparams __read_mostly = {
>> "(0=use value from vbt [default], 1=low power swing(200mV),"
>> "2=default swing(400mV))");
>> -i915_param_named_unsafe(enable_guc_loading, int, 0400,
>> - "Enable GuC firmware loading "
>> - "(-1=auto, 0=never [default], 1=if available, 2=required)");
>> -
>> i915_param_named_unsafe(enable_guc_submission, int, 0400,
>> "Enable GuC submission "
>> "(-1=auto, 0=never [default], 1=if available, 2=required)");
>> diff --git a/drivers/gpu/drm/i915/i915_params.h
>> b/drivers/gpu/drm/i915/i915_params.h
>> index c729226..9e1e231 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -44,7 +44,6 @@
>> param(int, disable_power_well, -1) \
>> param(int, enable_ips, 1) \
>> param(int, invert_brightness, 0) \
>> - param(int, enable_guc_loading, 0) \
>> param(int, enable_guc_submission, 0) \
>> param(int, guc_log_level, -1) \
>> param(char *, guc_firmware_path, NULL) \
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index aa9a7b5..fa09c2b 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -103,7 +103,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma
>> *vma)
>> int intel_guc_resume(struct drm_i915_private *dev_priv);
>> struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32
>> size);
>> -int intel_guc_select_fw(struct intel_guc *guc);
>> +void intel_guc_select_fw(struct intel_guc *guc);
>> int intel_guc_init_hw(struct intel_guc *guc);
>> u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index c7a800a..cae8333 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -380,9 +380,8 @@ int intel_guc_init_hw(struct intel_guc *guc)
>> * intel_guc_select_fw() - selects GuC firmware for loading
>> * @guc: intel_guc struct
>> *
>> - * Return: zero when we know firmware, non-zero in other case
> This change can be a new patch and since this/most of the patch is
> depending on Michal's series I would suggest to post the series once
> that gets merged.
Will do.
>> */
>> -int intel_guc_select_fw(struct intel_guc *guc)
>> +void intel_guc_select_fw(struct intel_guc *guc)
>> {
>> struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> @@ -409,9 +408,9 @@ int intel_guc_select_fw(struct intel_guc *guc)
>> guc->fw.major_ver_wanted = GLK_FW_MAJOR;
>> guc->fw.minor_ver_wanted = GLK_FW_MINOR;
>> } else {
>> - DRM_ERROR("No GuC firmware known for platform with GuC!\n");
>> - return -ENOENT;
>> + if(HAS_GUC(dev_priv))
>> + DRM_ERROR("No GUC FW known for a platform with GuC!\n");
>> + return;
>> }
>> - return 0;
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_huc.c
>> b/drivers/gpu/drm/i915/intel_huc.c
>> index 4b4cf56..7f59c8e 100644
>> --- a/drivers/gpu/drm/i915/intel_huc.c
>> +++ b/drivers/gpu/drm/i915/intel_huc.c
>> @@ -175,7 +175,9 @@ void intel_huc_select_fw(struct intel_huc *huc)
>> huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR;
>> huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR;
>> } else {
>> - DRM_ERROR("No HuC firmware known for platform with HuC!\n");
>> + /* For now, everything with a GuC also has a HuC */
>> + if (HAS_GUC(dev_priv))
>> + DRM_ERROR("No HuC FW known for a platform with HuC!\n");
>> return;
>> }
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 7b938e8..b687d97 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -49,45 +49,53 @@ static int __intel_uc_reset_hw(struct
>> drm_i915_private *dev_priv)
>> void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
>> {
>> + /* Verify hardware support */
>> if (!HAS_GUC(dev_priv)) {
>> - if (i915_modparams.enable_guc_loading > 0 ||
>> - i915_modparams.enable_guc_submission > 0)
>> - DRM_INFO("Ignoring GuC options, no hardware\n");
>> -
>> - i915_modparams.enable_guc_loading = 0;
>> - i915_modparams.enable_guc_submission = 0;
>> + if (i915_modparams.enable_guc_submission > 0)
>> + DRM_INFO("Ignoring GuC submission enable, no HW\n");
> Can we say option in the message - "Ignoring GuC submission enable
> option, no hardware"
> split the message over two lines.
Will do.
>> + i915_modparams.enable_guc_submission = 0;
>> return;
>> }
>> - /* A negative value means "use platform default" */
>> - if (i915_modparams.enable_guc_loading < 0)
>> - i915_modparams.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
>> + /* Verify firmware support */
>> + if (!HAS_GUC_UCODE(dev_priv)) {
>> + if (i915_modparams.enable_guc_submission == 1) {
>> + DRM_INFO("Ignoring GuC submission enable, no FW\n");
>> + i915_modparams.enable_guc_submission = 0;
>> + return;
> this return is not aligned properly. code indentation needs to updated.
> please run "scripts/checkpatch.pl --strict" on all patches that helps
> identify such issues faster.
I will check the alignment.
>> + }
>> - /* Verify firmware version */
>> - if (i915_modparams.enable_guc_loading) {
>> - if (HAS_HUC_UCODE(dev_priv))
>> - intel_huc_select_fw(&dev_priv->huc);
>> + if (i915_modparams.enable_guc_submission < 0) {
>> + i915_modparams.enable_guc_submission = 0;
>> + return;
>> + }
>> - if (intel_guc_select_fw(&dev_priv->guc))
>> - i915_modparams.enable_guc_loading = 0;
>> + /*
>> + * If "required" (> 1), let it continue and we will fail later
>> + * due to the lack of firmware
>> + */
>> }
>> - /* Can't enable guc submission without guc loaded */
>> - if (!i915_modparams.enable_guc_loading)
>> - i915_modparams.enable_guc_submission = 0;
>> -
>> - /* A negative value means "use platform default" */
>> + /*
>> + * A negative value means "use platform default" (enabled if we
>> have
>> + * survived to get here)
>> + */
>> if (i915_modparams.enable_guc_submission < 0)
>> - i915_modparams.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
>> + i915_modparams.enable_guc_submission = HAS_GUC(dev_priv);
>> +
>> }
>> void intel_uc_init_early(struct drm_i915_private *dev_priv)
>> {
>> intel_guc_init_early(&dev_priv->guc);
>> + intel_guc_select_fw(&dev_priv->guc);
>> + intel_huc_select_fw(&dev_priv->huc);
>> }
>> void intel_uc_init_fw(struct drm_i915_private *dev_priv)
>> {
>> + if (!HAS_GUC(dev_priv))
>> + return;
>> intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
>> intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw);
>> }
>> @@ -154,7 +162,7 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> struct intel_guc *guc = &dev_priv->guc;
>> int ret, attempts;
>> - if (!i915_modparams.enable_guc_loading)
>> + if (!NEEDS_GUC_LOADING(dev_priv))
>> return 0;
>> guc_disable_communication(guc);
>> @@ -244,19 +252,15 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> i915_ggtt_disable_guc(dev_priv);
>> DRM_ERROR("GuC init failed\n");
>> - if (i915_modparams.enable_guc_loading > 1 ||
>> - i915_modparams.enable_guc_submission > 1)
>> + if (i915_modparams.enable_guc_submission > 1) {
>> + DRM_NOTE("GuC is required, so marking the GPU as wedged\n");
>> ret = -EIO;
>> - else
>> - ret = 0;
>> -
>> - if (i915_modparams.enable_guc_submission) {
>> - i915_modparams.enable_guc_submission = 0;
>> + } else if (i915_modparams.enable_guc_submission == 1) {
>> DRM_NOTE("Falling back from GuC submission to execlist
>> mode\n");
>> - }
>> -
>> - i915_modparams.enable_guc_loading = 0;
>> - DRM_NOTE("GuC firmware loading disabled\n");
>> + i915_modparams.enable_guc_submission = 0;
>> + ret = 0;
>> + } else
>> + ret = 0;
>> return ret;
>> }
>> @@ -265,7 +269,7 @@ void intel_uc_fini_hw(struct drm_i915_private
>> *dev_priv)
>> {
>> guc_free_load_err_log(&dev_priv->guc);
>> - if (!i915_modparams.enable_guc_loading)
>> + if (!NEEDS_GUC_LOADING(dev_priv))
>> return;
>> if (i915_modparams.enable_guc_submission)
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> b/drivers/gpu/drm/i915/intel_uncore.c
>> index 983617b..696e11f 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1802,8 +1802,7 @@ int intel_guc_reset(struct drm_i915_private
>> *dev_priv)
>> {
>> int ret;
>> - if (!HAS_GUC(dev_priv))
>> - return -EINVAL;
>> + GEM_BUG_ON(!HAS_GUC(dev_priv));
>> intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC);
> Overall patch logic looks fine. Will need some more updates/rebase
> later as suggested.
I will wait for Michal's series before sending the next revision.
Thanks for the review.
Regards,
Sujaritha
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support
2017-10-13 9:05 ` Sagar Arun Kamble
2017-10-16 16:33 ` Sujaritha
@ 2017-10-16 17:12 ` Sujaritha
1 sibling, 0 replies; 10+ messages in thread
From: Sujaritha @ 2017-10-16 17:12 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx
On 10/13/2017 02:05 AM, Sagar Arun Kamble wrote:
> Keep subject as "drm/i915" as this is generic change. Also I saw
> i915_runtime_pm_status debugfs output not updated.
>
> Could you please check.
>
>
I did not change the seq_puts message of i915_runtime_pm_status debugfs
since it doesn't fit into the unified "early return" pattern.
Sujaritha
>
> On 10/11/2017 12:02 AM, Sujaritha Sundaresan wrote:
>> Unifying the various seq_puts messages in debugfs to the simplest one
>> for
>> feature support.
>>
>> v2: Clarifying the commit message (Anusha)
>>
>> v3: Re-factoring code as per review (Michal)
>>
>> v4: Rebase
>>
>> v5: Split from following patch
>>
>> v6: Re-factoring code (Michal, Sagar)
>> Clarifying commit message (Sagar)
>>
>> Suggested by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 5b58d2b..9d0c27b 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1670,7 +1670,7 @@ static int i915_fbc_status(struct seq_file *m,
>> void *unused)
>> struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> if (!HAS_FBC(dev_priv)) {
>> - seq_puts(m, "FBC unsupported on this chipset\n");
>> + seq_puts(m, "not supported\n");
>> return 0;
>> }
>> @@ -1837,7 +1837,7 @@ static int i915_ring_freq_table(struct
>> seq_file *m, void *unused)
>> unsigned int max_gpu_freq, min_gpu_freq;
>> if (!HAS_LLC(dev_priv)) {
>> - seq_puts(m, "unsupported on this chipset\n");
>> + seq_puts(m, "not supported\n");
>> return 0;
>> }
>> @@ -2391,6 +2391,7 @@ static int i915_huc_load_status_info(struct
>> seq_file *m, void *data)
>> struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
>> if (!HAS_HUC_UCODE(dev_priv))
>> + seq_puts(m, "not supported\n");
>> return 0;
>> seq_puts(m, "HuC firmware status:\n");
>> @@ -2424,6 +2425,7 @@ static int i915_guc_load_status_info(struct
>> seq_file *m, void *data)
>> u32 tmp, i;
>> if (!HAS_GUC_UCODE(dev_priv))
>> + seq_puts(m, "not supported\n");
>> return 0;
>> seq_printf(m, "GuC firmware status:\n");
>> @@ -2708,7 +2710,7 @@ static int i915_edp_psr_status(struct seq_file
>> *m, void *data)
>> bool enabled = false;
>> if (!HAS_PSR(dev_priv)) {
>> - seq_puts(m, "PSR not supported\n");
>> + seq_puts(m, "not supported\n");
>> return 0;
>> }
>> @@ -3565,7 +3567,7 @@ static void drrs_status_per_crtc(struct
>> seq_file *m,
>> mutex_lock(&drrs->mutex);
>> /* DRRS Supported */
>> - seq_puts(m, "\tDRRS Supported: Yes\n");
>> + seq_puts(m, "supported\n");
>> /* disable_drrs() will make drrs->dp NULL */
>> if (!drrs->dp) {
>> @@ -3597,7 +3599,7 @@ static void drrs_status_per_crtc(struct
>> seq_file *m,
>> mutex_unlock(&drrs->mutex);
>> } else {
>> /* DRRS not supported. Print the VBT parameter*/
>> - seq_puts(m, "\tDRRS Supported : No");
>> + seq_puts(m, "not supported\n");
>> }
>> seq_puts(m, "\n");
>> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-16 17:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-10 18:32 [PATCH v6 0/3] drm/i915/guc : Removing enable_guc_loading module and Decoupling logs and ADS from submission Sujaritha Sundaresan
2017-10-10 18:32 ` [PATCH v6 1/3] drm/i915/guc : Unifying seq_puts messages for feature support Sujaritha Sundaresan
2017-10-13 9:05 ` Sagar Arun Kamble
2017-10-16 16:33 ` Sujaritha
2017-10-16 17:12 ` Sujaritha
2017-10-10 18:32 ` [PATCH v6 2/3] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter Sujaritha Sundaresan
2017-10-13 9:42 ` Sagar Arun Kamble
2017-10-16 16:36 ` Sujaritha
2017-10-10 18:32 ` [PATCH v6 3/3] drm/i915/guc : Decouple logs and ADS from submission Sujaritha Sundaresan
2017-10-10 19:07 ` ✗ Fi.CI.BAT: failure for drm/i915/guc : Removing enable_guc_loading module and Decoupling " Patchwork
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.