Re: [Intel-gfx] [PATCH 4/8] drm/i915/guc: Disable critical logging in GuC by default from GuC v9

2017-08-16 Thread Srivatsa, Anusha


>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Kamble, Sagar A
>Sent: Monday, August 14, 2017 3:45 AM
>To: Chris Wilson ; intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915/guc: Disable critical logging in 
>GuC
>by default from GuC v9
>
>
>
>-Original Message-
>From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
>Sent: Wednesday, August 9, 2017 4:20 PM
>To: Kamble, Sagar A ; intel-
>g...@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915/guc: Disable critical logging in 
>GuC
>by default from GuC v9
>
>Quoting Sagar Arun Kamble (2017-08-09 11:23:48)
>> From GuC v9 firmware (for KBL v9.39+), separate control is added to
>> enable critical logging in GuC to enable capturing minimal important
>> logs in production systems.
>> i915.guc_log_level controls the verbosity and logging in GuC for logs
>> other than critical logs. By default, logging in GuC is disabled
>> through i915.guc_log_level.
>> This patch introduces new kernel param i915.enable_guc_critical_logging.
>> For Linux release builds, if needed critical GuC logs can be enabled
>> separately through this parameter. GuC log snapshot captured in error
>> state will have these minimal critical events logged.
>> Default value for this parameter is currently set to false.
>> This patch updates the initialization parameter sent during GuC load
>> to disable critical logging unless i915.guc_log_level is set to enable
>> logging and ensures it is enabled/disabling while enabling/disabling
>> through debugfs based on i915.enable_guc_critical_logging.
>>
>> Cc: Arkadiusz Hiler 
>> Cc: Spotswood John A 
>> Cc: Anusha Srivatsa 
>> Signed-off-by: Jeff McGee 
>> Signed-off-by: Sagar Arun Kamble 
>> ---
>>  drivers/gpu/drm/i915/i915_params.c  |  5 +
>>  drivers/gpu/drm/i915/i915_params.h  |  3 ++-
>>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 14 +-
>>  drivers/gpu/drm/i915/intel_guc_loader.c | 21 -
>>  drivers/gpu/drm/i915/intel_guc_log.c|  9 -
>>  5 files changed, 48 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.c
>> b/drivers/gpu/drm/i915/i915_params.c
>> index 14e2c2e..902bf2c 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -59,6 +59,7 @@ struct i915_params i915 __read_mostly = {
>> .enable_guc_loading = 0,
>> .enable_guc_submission = 0,
>> .guc_log_level = -1,
>> +   .enable_guc_critical_logging = false,
>
>What's the point in having a log level if LOG_LEVEL_CRITICAL is not part of it?
>Please do explain why the current parameter does not cover this.
>-Chris

I agree with Chris. Apart from introducing a new parameter, this is also 
introducing the dependency on an existing one - guc_log_level. 

Anusha 
> Agree that this could have been enumerated as another log level. Having
>it as log level simplifies other capture code too which I am seeing needs fix 
>with
>current change. Will check if this can be changed in upcoming releases.
>
>___
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 4/8] drm/i915/guc: Disable critical logging in GuC by default from GuC v9

2017-08-09 Thread Chris Wilson
Quoting Sagar Arun Kamble (2017-08-09 11:23:48)
> From GuC v9 firmware (for KBL v9.39+), separate control is added to enable
> critical logging in GuC to enable capturing minimal important logs in
> production systems.
> i915.guc_log_level controls the verbosity and logging in GuC for logs other
> than critical logs. By default, logging in GuC is disabled through
> i915.guc_log_level.
> This patch introduces new kernel param i915.enable_guc_critical_logging.
> For Linux release builds, if needed critical GuC logs can be enabled
> separately through this parameter. GuC log snapshot captured in error state
> will have these minimal critical events logged.
> Default value for this parameter is currently set to false.
> This patch updates the initialization parameter sent during GuC load to
> disable critical logging unless i915.guc_log_level is set to enable logging
> and ensures it is enabled/disabling while enabling/disabling through
> debugfs based on i915.enable_guc_critical_logging.
> 
> Cc: Arkadiusz Hiler 
> Cc: Spotswood John A 
> Cc: Anusha Srivatsa 
> Signed-off-by: Jeff McGee 
> Signed-off-by: Sagar Arun Kamble 
> ---
>  drivers/gpu/drm/i915/i915_params.c  |  5 +
>  drivers/gpu/drm/i915/i915_params.h  |  3 ++-
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 14 +-
>  drivers/gpu/drm/i915/intel_guc_loader.c | 21 -
>  drivers/gpu/drm/i915/intel_guc_log.c|  9 -
>  5 files changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c 
> b/drivers/gpu/drm/i915/i915_params.c
> index 14e2c2e..902bf2c 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -59,6 +59,7 @@ struct i915_params i915 __read_mostly = {
> .enable_guc_loading = 0,
> .enable_guc_submission = 0,
> .guc_log_level = -1,
> +   .enable_guc_critical_logging = false,

What's the point in having a log level if LOG_LEVEL_CRITICAL is not part
of it? Please do explain why the current parameter does not cover this.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx