Message ID | 20170427231300.32841-14-michel.thierry@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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.
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;
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(-)