Re: [PATCH 5/6] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/11/2017 11:34 PM, Michal Wajdeczko wrote:
On Mon, Sep 11, 2017 at 11:32:23AM +0530, Sagar Arun Kamble wrote:
From: "Kamble, Sagar A" <sagar.a.kamble@xxxxxxxxx>

With GuC v9, new type of Default/critical logging in GuC to enable
capturing minimal important logs in production systems efficiently.
This patch enables this logging in GuC by default always. It should
be noted that streaming support with half-full interrupt mechanism
that is present for normal logging is not present for this type of
logging.

v2: Emulating GuC critical logging through i915.guc_log_level. Setting
this to 0 will make GuC critical logging ON and setting it to 1-4 will
communicate log level of 0-3 to GuC.

v3: Commit message update. Enable default/critical logging in GuC always.
Fixed RPM wake during guc_log_unregister in the unload path.

v4: Moved RPM wake change to separate patch. Removed GUC_DEBUG_RESERVED
and updated name of new bit to be version agnostic. Updated parameter to
struct intel_guc * and name of macro NEEDS_GUC_CRITICAL_LOGGING.
Removed explicit clearing of GUC_CRITICAL_LOGGING_DISABLED from
params[GUC_CTL_DEBUG] as it is unnecessary. (Michal Wajdeczko)

Cc: Chheda Harsh J <harsh.j.chheda@xxxxxxxxx>
Cc: Fry Gregory P <gregory.p.fry@xxxxxxxxx>
Cc: Spotswood John A <john.a.spotswood@xxxxxxxxx>
Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
Signed-off-by: Jeff McGee <jeff.mcgee@xxxxxxxxx>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_guc_fwif.h | 15 +++++++++++++--
  drivers/gpu/drm/i915/intel_guc_log.c  |  4 +++-
  2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..3ef228b 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -131,7 +131,7 @@
  #define   GUC_PROFILE_ENABLED		(1 << 7)
  #define   GUC_WQ_TRACK_ENABLED		(1 << 8)
  #define   GUC_ADS_ENABLED		(1 << 9)
-#define   GUC_DEBUG_RESERVED		(1 << 10)
+#define   GUC_CRITICAL_LOGGING_DISABLED	(1 << 10)
Looks like we don't need this at all
I had disabled this logging in the initial version of patch to reduce the impact. So earlier I had to explicitly declare this bit and set in the guc_params[CTL_DEBUG]. But recommendation from GuC team is to keep this always enabled hence we can do away with not touching guc_params[CTL_DEBUG] at all but then in order to make the users aware that bit 10 being unset has some meaning I was clearing it explicitly. Wont removing this bit definition now eliminate the meaning for bit 10 while loading GuC all together?



  #define   GUC_ADS_ADDR_SHIFT		11
  #define   GUC_ADS_ADDR_MASK		0xfffff800
@@ -139,6 +139,16 @@ #define GUC_CTL_MAX_DWORDS (SOFT_SCRATCH_COUNT - 2) /* [1..14] */ +/*
+ * Critical logging in GuC is to be enabled always from GuC v9+.
+ * (for KBL - v9.39+)
+ */
+#define GUC_NEEDS_CRITICAL_LOGGING(guc)	\
+	(((IS_SKYLAKE(guc_to_i915(guc)) || IS_BROXTON(guc_to_i915(guc))) && \
Can we use here HAS_GUC() ? Comment says that this is for all Guc

Michal
Yes. Will add this.


+				    guc->fw.major_ver_found >= 9) || \
+	  (IS_KABYLAKE(guc_to_i915(guc)) && guc->fw.major_ver_found >= 9 && \
+				    guc->fw.minor_ver_found >= 39))
+
  /**
   * DOC: GuC Firmware Layout
   *
@@ -539,7 +549,8 @@ struct guc_log_buffer_state {
  		u32 logging_enabled:1;
  		u32 reserved1:3;
  		u32 verbosity:4;
-		u32 reserved2:24;
+		u32 critical_logging_enabled:1;
+		u32 reserved2:23;
  	};
  	u32 value;
  } __packed;
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..ba36162 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -589,7 +589,6 @@ void intel_guc_log_destroy(struct intel_guc *guc)
  int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
  {
  	struct intel_guc *guc = &dev_priv->guc;
-
  	union guc_log_control log_param;
  	int ret;
@@ -603,6 +602,9 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
  	if (!log_param.logging_enabled && (i915.guc_log_level < 0))
  		return 0;
+ if (GUC_NEEDS_CRITICAL_LOGGING(guc))
+		log_param.critical_logging_enabled = 1;
+
  	ret = guc_log_control(guc, log_param.value);
  	if (ret < 0) {
  		DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
--
1.9.1


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux