[Intel-gfx] [PATCH 04/11] drm/i915/guc: Sanitize module parameter guc_log_level
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu Oct 19 07:22:17 UTC 2017
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 at intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>>> ---
>>> 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
More information about the Intel-gfx
mailing list