Re: [PATCH 04/11] drm/i915/guc: Sanitize module parameter guc_log_level

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

 




On 18/10/2017 16:50, Sagar Arun Kamble wrote:
On 10/18/2017 6:29 PM, Tvrtko Ursulin wrote:

On 18/10/2017 07:46, Sagar Arun Kamble wrote:
Parameter guc_log_level needs to be sanitized based on GuC support and
enable_guc_loading parameter since it depends on them like
enable_guc_submission. This will make GuC logging paths independent of
enable_guc_submission parameter in further patches.

v2: Added documentation to intel_guc_log.c and param description about
GuC loading dependency. (Michal Wajdeczko)

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>
---
  drivers/gpu/drm/i915/i915_params.c   |  4 +++-
  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
  drivers/gpu/drm/i915/intel_uc.c      | 10 +++++++---
  3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b4faeb6..774c56e 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -171,7 +171,9 @@ struct i915_params i915_modparams __read_mostly = {
      "(-1=auto, 0=never [default], 1=if available, 2=required)");
    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_loading) (-1:disabled (default), "
+    "0-3:enabled)");
    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 f53c663..200f0a1 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_loading.    * 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.
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 62738ad..8feefcd 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -51,11 +51,13 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
  {
      if (!HAS_GUC(dev_priv)) {
          if (i915_modparams.enable_guc_loading > 0 ||
-            i915_modparams.enable_guc_submission > 0)
+            i915_modparams.enable_guc_submission > 0 ||
+            i915_modparams.guc_log_level > 0)
              DRM_INFO("Ignoring GuC options, no hardware\n");

Hm, this won't fire on <=gen8 once enable_guc_submission starts to default to one? Or we can worry about that when we get there...
This will fire for <=gen8 for +ve values which is expected.

i915_modparams.enable_guc_loading = 0;
          i915_modparams.enable_guc_submission = 0;
+        i915_modparams.guc_log_level = -1;
          return;
      }
  @@ -72,9 +74,11 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
              i915_modparams.enable_guc_loading = 0;
      }
  -    /* Can't enable guc submission without guc loaded */
-    if (!i915_modparams.enable_guc_loading)
+    /* Can't enable guc submission and logging without guc loaded */
+    if (!i915_modparams.enable_guc_loading) {
          i915_modparams.enable_guc_submission = 0;
+        i915_modparams.guc_log_level = -1;

Hm2, how does this interact with the changes which are removing enable_guc_loading? Or it is a race?
It interacts. Will race. I think I should pause this series till the other one gets merged.

Okay, it's your call (as in you guys who are refactoring GuC heavily at the moment).

Should I continue to the end of this one then? It is not that big so I just as well can.

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