On Fri, Sep 01, 2017 at 11:02:12AM +0530, Sagar Arun Kamble wrote:
> 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.
>
> Cc: Chheda Harsh J <harsh.j.chheda@xxxxxxxxx>
> Cc: Fry Gregory P <gregory.p.fry@xxxxxxxxx>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
> Cc: Spotswood John A <john.a.spotswood@xxxxxxxxx>
> Cc: Anusha Srivatsa <anusha.srivatsa@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 | 14 +++++++++++++-
> drivers/gpu/drm/i915/intel_guc_loader.c | 10 ++++++++++
> drivers/gpu/drm/i915/intel_guc_log.c | 7 ++++++-
> 3 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 5fa2860..353b081 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -132,6 +132,7 @@
> #define GUC_WQ_TRACK_ENABLED (1 << 8)
> #define GUC_ADS_ENABLED (1 << 9)
> #define GUC_DEBUG_RESERVED (1 << 10)
> +#define GUC_V9_CRITICAL_LOGGING_DISABLED (1 << 10)
It looks that GUC_DEBUG_RESERVED was never used, so maybe just remove old
name and add new one without V9 prefix ?
> #define GUC_ADS_ADDR_SHIFT 11
> #define GUC_ADS_ADDR_MASK 0xfffff800
>
> @@ -139,6 +140,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 NEEDS_GUC_CRITICAL_LOGGING(dev_priv, guc_fw) \
> + (((IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv)) && \
> + guc_fw->major_ver_found >= 9) || \
> + (IS_KABYLAKE(dev_priv) && guc_fw->major_ver_found >= 9 && \
> + guc_fw->minor_ver_found >= 39))
> +
As this is about Guc caps, then maybe simpler:
#define GUC_NEEDS_CRITICAL_LOGGING(guc) \
(((IS_SKYLAKE(guc_to_i915(guc)) || IS_BROXTON(guc_to_i915(guc)) && \
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 +550,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_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 81e03a6..d2b027c 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -106,6 +106,7 @@ static u32 get_core_family(struct drm_i915_private *dev_priv)
> static void guc_params_init(struct drm_i915_private *dev_priv)
> {
> struct intel_guc *guc = &dev_priv->guc;
> + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
> u32 params[GUC_CTL_MAX_DWORDS];
> int i;
>
> @@ -136,6 +137,15 @@ static void guc_params_init(struct drm_i915_private *dev_priv)
> } else
> params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
>
> + /*
> + * GuC has critical/default logging level which is to be enabled
> + * always from GuC v9 onwards.
> + */
> + if (NEEDS_GUC_CRITICAL_LOGGING(dev_priv, guc_fw)) {
> + params[GUC_CTL_DEBUG] &=
> + ~GUC_V9_CRITICAL_LOGGING_DISABLED;
Maybe I'm missing something, but when this bit was set that you have to clear it now?
> + }
> +
> /* If GuC submission is enabled, set up additional parameters here */
> if (i915.enable_guc_submission) {
> u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 16d3b87..6877e34 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -589,7 +589,7 @@ 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;
> -
> + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
> union guc_log_control log_param;
> int ret;
>
> @@ -603,6 +603,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 (NEEDS_GUC_CRITICAL_LOGGING(dev_priv, guc_fw))
> + 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);
> @@ -655,8 +658,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
> return;
>
> mutex_lock(&dev_priv->drm.struct_mutex);
> + intel_runtime_pm_get(dev_priv);
> /* GuC logging is currently the only user of Guc2Host interrupts */
> gen9_disable_guc_interrupts(dev_priv);
> + intel_runtime_pm_put(dev_priv);
Can this RPM be moved to separate patch ?
> guc_log_runtime_destroy(&dev_priv->guc);
> mutex_unlock(&dev_priv->drm.struct_mutex);
> }
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
