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

[Paulo Zanoni]

Em Qua, 2016-10-05 às 16:50 -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.

I'm not exactly sure who did what here, but I think Rodrigo deserves
some credit for his initial work (and possibly debug). Maybe add:
Based-on-patch-by: Rodrigo Vivi <rodrigo.vivi at intel.com>


> Cc: Zanoni Paulo <paulo.r.zanoni at intel.com>

I see the Cc tag here but no Cc mail header with my email on it, so
this email wasn't colored blue on my intel-gfx folder. Why didn't I
actually get Cc'd here? Did you use git-send-email? This may be worth
investigating since it may happen again in the future, and you want to
make sure people you Cc actually get Cc'd.


> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 15 +++++++++------
>  1 file changed, 9 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..15d2d53 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -717,13 +717,16 @@ void intel_guc_init(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>  	const char *fw_path;
> -

Please do not remove the blank line above.

> +	if (!HAS_GUC(dev)) {
> +		i915.enable_guc_loading = 0;
> +		i915.enable_guc_submission = 0;
> +	} else {
>  	/* A negative value means "use platform default" */

Bad indentation here.

> -	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);
> -

Please also do not remove the blank line above.

Otherwise, looks good.

> +		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_UCODE(dev)) {
>  		fw_path = NULL;
>  	} else if (IS_SKYLAKE(dev)) {