[Intel-gfx] [PATCH v2 2/2] drm/i915/guc: Change values for i915_guc_log_control

[Sagar Arun Kamble]

On 1/11/2018 8:54 PM, Michal Wajdeczko wrote:
> Today we have format mismatch between read/write operations
> of i915_guc_log_control entry. For read we return (0, 1..4)
> that represents disable/verbosity levels, but for write we
> force user to follow internal structure format (0,1,9,11,13).
0x0, 0x1, 0x11, 0x21, 0x31
> Let's hide internals from the user and accept same values
> as we support for read and related guc_log_level modparam.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_log.c | 27 +++++++++++++++------------
>   1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index e6d59bf..1719f1e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -58,11 +58,15 @@ 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, u32 control_val)
> +static int guc_log_control(struct intel_guc *guc, bool enable, u32 verbosity)
>   {
> +	union guc_log_control control_val = { .logging_enabled = enable,
> +					      .reserved1 = 0,
> +					      .verbosity = verbosity,
> +					      .reserved2 = 0 };
>   	u32 action[] = {
>   		INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING,
> -		control_val
> +		control_val.value
>   	};
>   
>   	return intel_guc_send(guc, action, ARRAY_SIZE(action));
> @@ -567,28 +571,27 @@ void intel_guc_log_destroy(struct intel_guc *guc)
>   int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
>   {
>   	struct intel_guc *guc = &dev_priv->guc;
> -
> -	union guc_log_control log_param;
> +	bool enable_logging = control_val > 0;
> +	u32 verbosity;
>   	int ret;
>   
> -	log_param.value = control_val;
> -
> -	if (log_param.verbosity < GUC_LOG_VERBOSITY_MIN ||
> -	    log_param.verbosity > GUC_LOG_VERBOSITY_MAX)
> +	BUILD_BUG_ON(GUC_LOG_VERBOSITY_MIN);
> +	if (control_val > 1 + GUC_LOG_VERBOSITY_MAX)
>   		return -EINVAL;
>   
>   	/* This combination doesn't make sense & won't have any effect */
> -	if (!log_param.logging_enabled && !i915_modparams.guc_log_level)
> +	if (!enable_logging && !i915_modparams.guc_log_level)
>   		return 0;
>   
> -	ret = guc_log_control(guc, log_param.value);
> +	verbosity = enable_logging ? control_val - 1 : 0;
> +	ret = guc_log_control(guc, enable_logging, verbosity);
>   	if (ret < 0) {
>   		DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
>   		return ret;
>   	}
>   
> -	if (log_param.logging_enabled) {
> -		i915_modparams.guc_log_level = 1 + log_param.verbosity;
> +	if (enable_logging) {
> +		i915_modparams.guc_log_level = 1 + verbosity;
>   
>   		/*
>   		 * If log was disabled at boot time, then the relay channel file