On 11/07/16 13:11, Goel, Akash wrote:
On 7/11/2016 5:20 PM, Tvrtko Ursulin wrote:
On 11/07/16 12:41, Goel, Akash wrote:
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
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.
Ah yes, I got confused by jumping between one file with your patch
applied and one without it.
+
+ 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.
Yes, this clarifies it. Although I do have one more question then - what
happens if at boot i915.guc_log_level == -1 and then with later patches
logging gets enabled via debugfs - who and how sets
params[GUC_CTL_DEBUG]? Host2GuC overrides this parameter?
Yes through Host2GuC action type, UK_LOG_ENABLE_LOGGING, Host will
request GuC firmware to enable/disable logging and alter the verbosity
level.
The params[GUC_CTL_DEBUG] is just part of the firmware initialization
parameters and is not used after that.
Got it now, in that case:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
