[v7,13/20] drm/i915/guc: Rename the function that resets the GuC

Submitted by Michel Thierry on April 27, 2017, 11:12 p.m.

Details

Message ID 20170427231300.32841-14-michel.thierry@intel.com
State New
Headers show
Series "Gen8+ engine-reset" ( rev: 10 9 8 7 6 5 4 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Michel Thierry April 27, 2017, 11:12 p.m.
intel_guc_reset sounds more like the microcontroller is the one performing
a reset, while in this case is the opposite. intel_reset_guc not only
makes it clearer, it follows the other intel_reset functions available.

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     | 2 +-
 drivers/gpu/drm/i915/intel_uc.c     | 4 ++--
 drivers/gpu/drm/i915/intel_uncore.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9ff7f726d47..e9e04c92a376 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3031,7 +3031,7 @@  extern int i915_reset_engine(struct intel_engine_cs *engine);
 extern bool intel_has_reset_engine(struct drm_i915_private *dev_priv);
 extern int intel_reset_engine_start(struct intel_engine_cs *engine);
 extern void intel_reset_engine_cancel(struct intel_engine_cs *engine);
-extern int intel_guc_reset(struct drm_i915_private *dev_priv);
+extern int intel_reset_guc(struct drm_i915_private *dev_priv);
 extern void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
 extern void intel_hangcheck_init(struct drm_i915_private *dev_priv);
 extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 900e3767a899..bad282b6c886 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -46,9 +46,9 @@  static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
 	int ret;
 	u32 guc_status;
 
-	ret = intel_guc_reset(dev_priv);
+	ret = intel_reset_guc(dev_priv);
 	if (ret) {
-		DRM_ERROR("GuC reset failed, ret = %d\n", ret);
+		DRM_ERROR("Reset GuC failed, ret = %d\n", ret);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 120fb440bb8b..00251d83e7bd 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1792,7 +1792,7 @@  bool intel_has_reset_engine(struct drm_i915_private *dev_priv)
 		i915.reset == 2);
 }
 
-int intel_guc_reset(struct drm_i915_private *dev_priv)
+int intel_reset_guc(struct drm_i915_private *dev_priv)
 {
 	int ret;
 

Comments

On 28/04/2017 00:12, Michel Thierry wrote:
> intel_guc_reset sounds more like the microcontroller is the one performing
> a reset, while in this case is the opposite. intel_reset_guc not only
> makes it clearer, it follows the other intel_reset functions available.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 2 +-
>  drivers/gpu/drm/i915/intel_uc.c     | 4 ++--
>  drivers/gpu/drm/i915/intel_uncore.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c9ff7f726d47..e9e04c92a376 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3031,7 +3031,7 @@ extern int i915_reset_engine(struct intel_engine_cs *engine);
>  extern bool intel_has_reset_engine(struct drm_i915_private *dev_priv);
>  extern int intel_reset_engine_start(struct intel_engine_cs *engine);
>  extern void intel_reset_engine_cancel(struct intel_engine_cs *engine);
> -extern int intel_guc_reset(struct drm_i915_private *dev_priv);
> +extern int intel_reset_guc(struct drm_i915_private *dev_priv);
>  extern void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
>  extern void intel_hangcheck_init(struct drm_i915_private *dev_priv);
>  extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 900e3767a899..bad282b6c886 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -46,9 +46,9 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
>  	int ret;
>  	u32 guc_status;
>
> -	ret = intel_guc_reset(dev_priv);
> +	ret = intel_reset_guc(dev_priv);
>  	if (ret) {
> -		DRM_ERROR("GuC reset failed, ret = %d\n", ret);
> +		DRM_ERROR("Reset GuC failed, ret = %d\n", ret);

As a non-native speaker I might be wrong, but was thinking something 
like "Failed to reset GuC", "Resetting GuC failed", "Reset of GuC 
failed" would be clearer? I leave it for someone more competent to decide.

>  		return ret;
>  	}
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 120fb440bb8b..00251d83e7bd 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1792,7 +1792,7 @@ bool intel_has_reset_engine(struct drm_i915_private *dev_priv)
>  		i915.reset == 2);
>  }
>
> -int intel_guc_reset(struct drm_i915_private *dev_priv)
> +int intel_reset_guc(struct drm_i915_private *dev_priv)
>  {
>  	int ret;
>
>

Potential message bikeshed aside:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
On 28/04/17 00:40, Tvrtko Ursulin wrote:
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -46,9 +46,9 @@ static int __intel_uc_reset_hw(struct
>> drm_i915_private *dev_priv)
>>      int ret;
>>      u32 guc_status;
>>
>> -    ret = intel_guc_reset(dev_priv);
>> +    ret = intel_reset_guc(dev_priv);
>>      if (ret) {
>> -        DRM_ERROR("GuC reset failed, ret = %d\n", ret);
>> +        DRM_ERROR("Reset GuC failed, ret = %d\n", ret);
>
> As a non-native speaker I might be wrong, but was thinking something
> like "Failed to reset GuC", "Resetting GuC failed", "Reset of GuC
> failed" would be clearer? I leave it for someone more competent to decide.

"Failed to reset GuC" sounds good to me.