On 1/5/2018 10:24 AM, Sagar Arun Kamble wrote:
On 1/4/2018 10:22 PM, Michal Wajdeczko wrote:
On Thu, 04 Jan 2018 17:21:46 +0100, Sagar Arun Kamble
<sagar.a.kamble@xxxxxxxxx> wrote:
guc_log_level parameter takes effect when GuC is loaded which is
controlled through enable_guc parameter. Add this relation info.
^^^
Extra "."
in parameter description and documentation.
Earlier, this patch was added to sanitize guc_log_level like old
GuC parameters enable_guc_loading/submission. With new parameter
enable_guc, sanitization of guc_log_level is no more needed.
Hmm, I think we still need to sanitize log_level if it was wrongly
enabled without enabling GuC first (in intel_uc_sanitize_options).
I think it would not be harmful as all decisions based on it will be
gated by USES_GUC.
I was wrong. i915_guc_log_register/unregister were changed by my series
to only rely on guc_log_level.
I have added HAS_GUC check in those function in v4 patch. Is that option
okay or we should sanitize this parameter?
v2: Added documentation to intel_guc_log.c and param description
about GuC loading dependency. (Michal Wajdeczko)
v3: Removed sanitization of module parameter guc_log_level.
Previous review comments not applicable now.
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> #v2
---
drivers/gpu/drm/i915/i915_params.c | 3 ++-
drivers/gpu/drm/i915/intel_guc_log.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index b5f3eb4..a93a6ca 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -155,7 +155,8 @@ struct i915_params i915_modparams __read_mostly = {
"(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)");
i915_param_named(guc_log_level, int, 0400,
- "GuC firmware logging level (-1:disabled (default),
0-3:enabled)");
+ "GuC firmware logging level. This takes effect only if GuC is
to be "
+ "loaded (depends on enable_guc) (-1:disabled (default),
0-3:enabled)");
Btw, I was planing to change above values to follow schema used in
other modparams:
-1: auto (then it can be controlled by USES_GUC and DRM_I915_DEBUG_GUC)
0: disabled
1: enabled (legacy level 0)
2: enabled (legacy level 1)
3: enabled (legacy level 2)
4: enabled (legacy level 3)
So now I'm not sure that I want your patch ;)
Makes sense. Will drop this patch.
Thanks
Sagar
i915_param_named_unsafe(guc_firmware_path, charp, 0400,
"GuC firmware path to use instead of the default one");
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c
b/drivers/gpu/drm/i915/intel_guc_log.c
index 59a9021..d0131bc 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -34,6 +34,7 @@
* DOC: GuC firmware log
*
* Firmware log is enabled by setting i915.guc_log_level to
non-negative level.
+ * This takes effect only if GuC is to be loaded based on enable_guc.
... based on i915.enable_guc modparam.
* Log data is printed out via reading debugfs i915_guc_log_dump.
Reading from
* i915_guc_load_status will print out firmware loading status and
scratch
* registers value.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
