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

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

 





On 6/27/2016 8:30 PM, Tvrtko Ursulin wrote:

On 27/06/16 13:16, akash.goel@xxxxxxxxx wrote:
From: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>

GuC Log buffer allocation was tied up with verbosity level kernel
parameter
i915.guc_log_level. User could be given a provision to enable logging at
runtime and not necessarily during load time only. This patch will
perform
allocation of shared log buffer always but will initially enable
logging on
GuC side through init params based on i915.guc_log_level.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_guc_submission.c | 3 ---
  drivers/gpu/drm/i915/intel_guc_loader.c    | 8 +++++---
  2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 355b647..28a810f 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -826,9 +826,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 8fe96a2..db3c897 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -173,11 +173,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;
+
+    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;

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


I did not manage to understand what is the benefit of always allocating
the log buffer? If the user never enables logging it just wasted 11
pages of memory, correct?

Yes if User never enables the logging at runtime, 11 RAM pages will be wasted.

Currently the pages are permanently pinned in GGTT also.
The GGTT address of log buffer is passed in the GuC firmware init params, at firmware loading time.

Probably this can be circumvented, if pages can be pinned right before
enabling logging (but using the same GGTT address).

Looking at the later patches in the series, could you instead create the
log buffer when logging is enabled via debugfs or implicitly via the
relayfs access?

Or is the problem then that you would then have to reset the GuC to
activate it?

Yes GuC would have to be reset & firmware needs to be reloaded to pass the log buffer address.

Best regards
Akash


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