Re: [PATCH] drm/i915/guc: Sanitory checks for platform that dont have GuC

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Qua, 2016-10-12 às 16:13 -0700, Anusha Srivatsa escreveu:
> i915.enable_guc_loading/submission=2 forces the usage of GuC.
> For platforms that do not have a GuC, asking the kernel to
> use a GuC should not result in an error state. Do extra checks
> to see if the platform even has a GuC or not, regardless of the
> kernel parameter.
> 
> v2: Based on Rodrigo's patch and Paulo's suggestion(Paulo, Rodrigo)
> v3: Correct the Indentation(Jani, Paulo)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97573
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Zanoni Paulo <paulo.r.zanoni@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> 
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 7ace96b..811080f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -718,12 +718,16 @@ void intel_guc_init(struct drm_device *dev)
>  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>  	const char *fw_path;
>  
> -	/* A negative value means "use platform default" */
> -	if (i915.enable_guc_loading < 0)
> -		i915.enable_guc_loading = HAS_GUC_UCODE(dev);
> -	if (i915.enable_guc_submission < 0)
> -		i915.enable_guc_submission = HAS_GUC_SCHED(dev);
> -
> +	if (!HAS_GUC(dev)) {
> +		i915.enable_guc_loading = 0;
> +		i915.enable_guc_submission = 0;
> +	} else {
> +		/* A negative value means "use platform default" */
> +		if (i915.enable_guc_loading < 0)
> +			i915.enable_guc_loading =
> HAS_GUC_UCODE(dev);
> +		if (i915.enable_guc_submission < 0)
> +			i915.enable_guc_submission =
> HAS_GUC_SCHED(dev);
> +	}

Well, the indentation is correct now, but this patch is still removing
the blank line that was supposed to be at this spot, despite the fact
that I pointed this problem in my two previous reviews... Please always
pay attention to the reviews and try to make sure you're actually
following them when submit new versions. It's a good idea if you can
learn the patch format and look at the patch files directly in order to
spot problems like this before submitting.

I know this sounds super annoying since I'm just complaining about
blank lines, but since I expect you to start contributing much more, I
think it's worth trying to teach the coding style that's used by
everybody so the next patches all come in the expected format.

Anyway, I suppose we could amend this fix while applying the patch?

If someone fixes that while applying, we can add:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>  	if (!HAS_GUC_UCODE(dev)) {
>  		fw_path = NULL;
>  	} else if (IS_SKYLAKE(dev)) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx