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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx