Re: [PATCH 01/17] drm/i915: Decouple GuC log setup from verbosity parameter

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

 





On 7/11/2016 3:07 PM, Tvrtko Ursulin wrote:

On 10/07/16 14:41, akash.goel@xxxxxxxxx wrote:
From: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>

b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2112e02..8a9a0cb 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -832,9 +832,6 @@ static void guc_create_log(struct intel_guc *guc)
      unsigned long offset;
      uint32_t size, flags;

-    if (i915.guc_log_level < GUC_LOG_VERBOSITY_MIN)
-        return;
-
      if (i915.guc_log_level > GUC_LOG_VERBOSITY_MAX)
          i915.guc_log_level = GUC_LOG_VERBOSITY_MAX;

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 605c696..b211bd0 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -175,11 +175,13 @@ static void set_guc_init_params(struct
drm_i915_private *dev_priv)
      params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
              GUC_CTL_VCS2_ENABLED;

-    if (i915.guc_log_level >= 0) {
-        params[GUC_CTL_LOG_PARAMS] = guc->log_flags;
+    params[GUC_CTL_LOG_PARAMS] = guc->log_flags;

guc->log_flags will be zero when logging is not configured because guc
is a part of dev_priv. So it looks safe - although I reckon it would be
clearer to set this (GUC_CTL_LOG_PARAMS) explicitly inside the if-else
below?

If logging is not enabled at (due to guc_log_level < 0), then also log_flags needs to be setup & passed to GuC firmware. log_flags shall not be zero even when logging is not be enabled (at boot time).
Actually log_flags will also contain the address of the log buffer.


+
+    if (i915.guc_log_level >= 0)
          params[GUC_CTL_DEBUG] =
              i915.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
-    }
+    else
+        params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;

I also wonder how come GUC_LOG_DISABLED isn't set today when
i915.guc_log_level == -1, given that:

#define   GUC_LOG_DISABLED             (1 << 6)

Is that bit set by default somehow if i915 does not program it?


Yes currently GUC_LOG_DISABLED won't get set for guc_log_level = -1.
But then log buffer address will go as NULL and GUC_LOG_VALID flag will
go as 0, for guc_log_level = -1. So this way logging on GuC side will not get enabled.
I hope I understood your concern correctly.

Best regards
Akash


      if (guc->ads_obj) {
          u32 ads = (u32)i915_gem_obj_ggtt_offset(guc->ads_obj)


Regards,

Tvrtko

_______________________________________________
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