diff mbox

[v2,4/4] Revert "drm/i915/guc: Keep GuC log disabled by default"

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

Commit Message

sagar.a.kamble@intel.com Jan. 23, 2018, 3:42 p.m. UTC
Comment DRM_ERROR_RATELIMIT outputs as log capturer won't be
running.

This reverts commit bd724318b682587ad2f989ab8e0f7b3d4486ced5.
---
 drivers/gpu/drm/i915/i915_params.h   | 2 +-
 drivers/gpu/drm/i915/intel_guc_log.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Michal Wajdeczko Jan. 23, 2018, 4:06 p.m. UTC | #1
On Tue, 23 Jan 2018 16:42:20 +0100, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> Comment DRM_ERROR_RATELIMIT outputs as log capturer won't be
> running.
>
> This reverts commit bd724318b682587ad2f989ab8e0f7b3d4486ced5.
> ---
>  drivers/gpu/drm/i915/i915_params.h   | 2 +-
>  drivers/gpu/drm/i915/intel_guc_log.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.h  
> b/drivers/gpu/drm/i915/i915_params.h
> index 430f5f9..c963603 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -48,7 +48,7 @@
>  	param(int, enable_ips, 1) \
>  	param(int, invert_brightness, 0) \
>  	param(int, enable_guc, 0) \
> -	param(int, guc_log_level, 0) \
> +	param(int, guc_log_level, -1) \

If this is HAX for CI then OK, but otherwise maybe we
should leave log disabled(0) to match disabled(0) GuC ;)

Then in the future we can change both to auto(-1)

>  	param(char *, guc_firmware_path, NULL) \
>  	param(char *, huc_firmware_path, NULL) \
>  	param(int, mmio_debug, 0) \
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
> b/drivers/gpu/drm/i915/intel_guc_log.c
> index be9f2ca..7604451 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -238,7 +238,7 @@ static bool guc_check_log_buf_overflow(struct  
> intel_guc *guc,
>  			/* buffer_full_cnt is a 4 bit counter */
>  			guc->log.total_overflow_count[type] += 16;
>  		}
> -		DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
> +		//DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");

I'm not sure that we should leave dead code
And using C++ style comments is not good too
(unless this patch is just a HAX)

>  	}
> 	return overflow;
> @@ -354,7 +354,7 @@ static void guc_read_update_log_buffer(struct  
> intel_guc *guc)
>  		/* Used rate limited to avoid deluge of messages, logs might be
>  		 * getting consumed by User at a slow rate.
>  		 */
> -		DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
> +		//DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
>  		guc->log.capture_miss_count++;
>  	}
>  	mutex_unlock(&guc->log.runtime.relay_lock);
sagar.a.kamble@intel.com Jan. 23, 2018, 4:32 p.m. UTC | #2
On 1/23/2018 9:36 PM, Michal Wajdeczko wrote:
> On Tue, 23 Jan 2018 16:42:20 +0100, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> Comment DRM_ERROR_RATELIMIT outputs as log capturer won't be
>> running.
>>
>> This reverts commit bd724318b682587ad2f989ab8e0f7b3d4486ced5.
>> ---
>>  drivers/gpu/drm/i915/i915_params.h   | 2 +-
>>  drivers/gpu/drm/i915/intel_guc_log.c | 4 ++--
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.h 
>> b/drivers/gpu/drm/i915/i915_params.h
>> index 430f5f9..c963603 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -48,7 +48,7 @@
>>      param(int, enable_ips, 1) \
>>      param(int, invert_brightness, 0) \
>>      param(int, enable_guc, 0) \
>> -    param(int, guc_log_level, 0) \
>> +    param(int, guc_log_level, -1) \
>
> If this is HAX for CI then OK, but otherwise maybe we
> should leave log disabled(0) to match disabled(0) GuC ;)
>
> Then in the future we can change both to auto(-1)
>
Yes. This is HAX. Missed tag in this rev.
>>      param(char *, guc_firmware_path, NULL) \
>>      param(char *, huc_firmware_path, NULL) \
>>      param(int, mmio_debug, 0) \
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
>> b/drivers/gpu/drm/i915/intel_guc_log.c
>> index be9f2ca..7604451 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -238,7 +238,7 @@ static bool guc_check_log_buf_overflow(struct 
>> intel_guc *guc,
>>              /* buffer_full_cnt is a 4 bit counter */
>>              guc->log.total_overflow_count[type] += 16;
>>          }
>> -        DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
>> +        //DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
>
> I'm not sure that we should leave dead code
> And using C++ style comments is not good too
> (unless this patch is just a HAX)
>
>>      }
>>     return overflow;
>> @@ -354,7 +354,7 @@ static void guc_read_update_log_buffer(struct 
>> intel_guc *guc)
>>          /* Used rate limited to avoid deluge of messages, logs might be
>>           * getting consumed by User at a slow rate.
>>           */
>> -        DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
>> +        //DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
>>          guc->log.capture_miss_count++;
>>      }
>>      mutex_unlock(&guc->log.runtime.relay_lock);
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 430f5f9..c963603 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -48,7 +48,7 @@ 
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
 	param(int, enable_guc, 0) \
-	param(int, guc_log_level, 0) \
+	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
 	param(int, mmio_debug, 0) \
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index be9f2ca..7604451 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -238,7 +238,7 @@  static bool guc_check_log_buf_overflow(struct intel_guc *guc,
 			/* buffer_full_cnt is a 4 bit counter */
 			guc->log.total_overflow_count[type] += 16;
 		}
-		DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
+		//DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
 	}
 
 	return overflow;
@@ -354,7 +354,7 @@  static void guc_read_update_log_buffer(struct intel_guc *guc)
 		/* Used rate limited to avoid deluge of messages, logs might be
 		 * getting consumed by User at a slow rate.
 		 */
-		DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
+		//DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
 		guc->log.capture_miss_count++;
 	}
 	mutex_unlock(&guc->log.runtime.relay_lock);