On 10/07/16 14:41, akash.goel@xxxxxxxxx wrote:
From: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
GuC Log buffer allocation was tied up with verbosity level module param
i915.guc_log_level. User would be given a provision to enable firmware
logging at runtime, through a host2guc action, and not necessarily during
Driver load time. But the address of log buffer can be passed only in
init params, at firmware load time, so GuC has to be reset and firmware
needs to be reloaded to pass the log buffer address at runtime.
To avoid reset of GuC & reload of firmware, allocation of log buffer will
be done always but logging would be enabled initially on GuC side based on
the value of module paramter guc_log_level.
v2: Update commit message to describe the constraint with allocation of
log buffer at runtime. (Tvrtko)
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 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 (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?
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
