[Intel-gfx] [PATCH v13 02/21] drm/i915/guc: Sanitize module parameter guc_log_level

[Michal Wajdeczko]

On Wed, 11 Oct 2017 10:53:57 +0200, Sagar Arun Kamble  
<sagar.a.kamble at intel.com> wrote:

> Parameter guc_log_level needs to be sanitized based on GuC support and
> enable_guc_loading parameter since it depends on them like
> enable_guc_submission. This will make GuC logging paths independent of
> enable_guc_submission parameter in further patches.
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index b33d469..3cf3cbd 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -56,6 +56,7 @@ void intel_uc_sanitize_options(struct drm_i915_private  
> *dev_priv)

Btw, I think the message "Ignoring GuC options, no hardware\n" is also
applicable when user specified guc_log_level>=0. Please update condition
that controls that message.

> 		i915_modparams.enable_guc_loading = 0;
>  		i915_modparams.enable_guc_submission = 0;
> +		i915_modparams.guc_log_level = -1;
>  		return;
>  	}
> @@ -72,9 +73,11 @@ void intel_uc_sanitize_options(struct  
> drm_i915_private *dev_priv)
>  			i915_modparams.enable_guc_loading = 0;
>  	}
> -	/* Can't enable guc submission without guc loaded */
> -	if (!i915_modparams.enable_guc_loading)
> +	/* Can't enable guc submission and logging without guc loaded */
> +	if (!i915_modparams.enable_guc_loading) {
>  		i915_modparams.enable_guc_submission = 0;
> +		i915_modparams.guc_log_level = -1;
> +	}
> 	/* A negative value means "use platform default" */
>  	if (i915_modparams.enable_guc_submission < 0)

Looks good, but please also update condition in i915_guc_log_register()
as we may now rely only on sanitized guc_log_level:

void i915_guc_log_register(struct drm_i915_private *dev_priv)
{
	if (!i915_modparams.enable_guc_submission ||
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	    (i915_modparams.guc_log_level < 0))
		return;

Also, maybe it is worth to update DOC in intel_guc_log.c and param
description in i915_params.c to indicate (obvious) dependency on guc.

Michal