[Intel-gfx] [PATCH] drm/i915/guc: Unify naming of private GuC action functions

Michał Winiarski michal.winiarski at intel.com
Thu Mar 15 15:57:26 UTC 2018


On Wed, Mar 14, 2018 at 06:37:15PM +0000, Michal Wajdeczko wrote:
> We should avoid using guc_log prefix for functions that don't
> operate on GuC log, but rather request action from the GuC.
> Better to use guc_action prefix.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Michal Winiarski <michal.winiarski at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_log.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index b9c7bd7..457168a 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -39,7 +39,7 @@
>   * registers value.
>   */
>  
> -static int guc_log_flush_complete(struct intel_guc *guc)
> +static int guc_action_flush_log_complete(struct intel_guc *guc)
>  {
>  	u32 action[] = {
>  		INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE
> @@ -48,7 +48,7 @@ static int guc_log_flush_complete(struct intel_guc *guc)
>  	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
>  
> -static int guc_log_flush(struct intel_guc *guc)
> +static int guc_action_flush_log(struct intel_guc *guc)
>  {
>  	u32 action[] = {
>  		INTEL_GUC_ACTION_FORCE_LOG_BUFFER_FLUSH,
> @@ -58,7 +58,8 @@ static int guc_log_flush(struct intel_guc *guc)
>  	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
>  
> -static int guc_log_control(struct intel_guc *guc, bool enable, u32 verbosity)
> +static int guc_action_enable_log(struct intel_guc *guc, bool enable,
> +				 u32 verbosity)

Let's hide the fact that the actual action is called "ENABLE_LOGGING", and stick
with guc_action_log_control, especially since we're using guc_log_control union,
and the action itself is also used for verbosity (and default log... more than
just enable/disable switch).

With that:

Reviewed-by: Michał Winiarski <michal.winiarski at intel.com>

-Michał

>  {
>  	union guc_log_control control_val = {
>  		{
> @@ -525,7 +526,7 @@ static void guc_log_capture_logs(struct intel_guc *guc)
>  	 * time, so get/put should be really quick.
>  	 */
>  	intel_runtime_pm_get(dev_priv);
> -	guc_log_flush_complete(guc);
> +	guc_action_flush_log_complete(guc);
>  	intel_runtime_pm_put(dev_priv);
>  }
>  
> @@ -541,7 +542,7 @@ static void guc_flush_logs(struct intel_guc *guc)
>  
>  	/* Ask GuC to update the log buffer state */
>  	intel_runtime_pm_get(dev_priv);
> -	guc_log_flush(guc);
> +	guc_action_flush_log(guc);
>  	intel_runtime_pm_put(dev_priv);
>  
>  	/* GuC would have updated log buffer by now, so capture it */
> @@ -639,10 +640,11 @@ int intel_guc_log_control_set(struct intel_guc *guc, u64 val)
>  	}
>  
>  	intel_runtime_pm_get(dev_priv);
> -	ret = guc_log_control(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val));
> +	ret = guc_action_enable_log(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val));
>  	intel_runtime_pm_put(dev_priv);
>  	if (ret) {
> -		DRM_DEBUG_DRIVER("guc_log_control action failed %d\n", ret);
> +		DRM_DEBUG_DRIVER("GuC action to %s log failed (%d)\n",
> +				 enabled ? "enable" : "disable", ret);
>  		goto out_unlock;
>  	}
>  
> -- 
> 1.9.1
> 


More information about the Intel-gfx mailing list