diff mbox

[10/11] drm/i915/guc: Skip interrupt enabling if logging is already enabled

Message ID 1508309222-26406-11-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com Oct. 18, 2017, 6:47 a.m. UTC
To balance the GuC interrupt references we don't allow enabling interrupts
If logging was enabled earlier with different verbosity.
We allow request to change log parameters to be sent to GuC though as
user may want to just update the verbosity level at runtime.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Tvrtko Ursulin Oct. 19, 2017, 10:31 a.m. UTC | #1
On 18/10/2017 07:47, Sagar Arun Kamble wrote:
> To balance the GuC interrupt references we don't allow enabling interrupts
> If logging was enabled earlier with different verbosity.

I've noticed in a couple of your previous commits words in the middle of 
sentences starting with upper case which is a tiny bit distracting when 
reading.

> We allow request to change log parameters to be sent to GuC though as
> user may want to just update the verbosity level at runtime.
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_log.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 8c41d9a..90b8caf 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -613,6 +613,11 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
>   	}
>   
>   	if (log_param.logging_enabled) {
> +		if (i915_modparams.guc_log_level >= 0) {
> +			i915_modparams.guc_log_level = log_param.verbosity;
> +			return 0;
> +		}

Ok this will change if you go for the refactoring of how modparam is 
used mentioned earlier in the series.

> +
>   		i915_modparams.guc_log_level = log_param.verbosity;
>   
>   		/* If log_level was set as -1 at boot time, then the relay channel file
> 

Regards,

Tvrtko
sagar.a.kamble@intel.com Oct. 21, 2017, 4:47 p.m. UTC | #2
On 10/19/2017 4:01 PM, Tvrtko Ursulin wrote:
>
> On 18/10/2017 07:47, Sagar Arun Kamble wrote:
>> To balance the GuC interrupt references we don't allow enabling 
>> interrupts
>> If logging was enabled earlier with different verbosity.
>
> I've noticed in a couple of your previous commits words in the middle 
> of sentences starting with upper case which is a tiny bit distracting 
> when reading.
Sorry.  Will fix those. I hope it is okay to start words like 
"GuC"/"IRQ" or acronyms with upper case.
>
>> We allow request to change log parameters to be sent to GuC though as
>> user may want to just update the verbosity level at runtime.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc_log.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
>> b/drivers/gpu/drm/i915/intel_guc_log.c
>> index 8c41d9a..90b8caf 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -613,6 +613,11 @@ int i915_guc_log_control(struct drm_i915_private 
>> *dev_priv, u64 control_val)
>>       }
>>         if (log_param.logging_enabled) {
>> +        if (i915_modparams.guc_log_level >= 0) {
>> +            i915_modparams.guc_log_level = log_param.verbosity;
>> +            return 0;
>> +        }
>
> Ok this will change if you go for the refactoring of how modparam is 
> used mentioned earlier in the series.
Yes. will update.
>
>> +
>>           i915_modparams.guc_log_level = log_param.verbosity;
>>             /* If log_level was set as -1 at boot time, then the 
>> relay channel file
>>
>
> Regards,
>
> Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 8c41d9a..90b8caf 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -613,6 +613,11 @@  int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
 	}
 
 	if (log_param.logging_enabled) {
+		if (i915_modparams.guc_log_level >= 0) {
+			i915_modparams.guc_log_level = log_param.verbosity;
+			return 0;
+		}
+
 		i915_modparams.guc_log_level = log_param.verbosity;
 
 		/* If log_level was set as -1 at boot time, then the relay channel file