[Intel-gfx] [PATCH] drm/i915/guc: Removed unused GuC parameters.

[Michel Thierry]

On 28/02/18 10:42, Piotr Piórkowski wrote:
> In the i915 driver, there is a function, intel_guc_init_params(),
> which initializes the GuC parameter block which is passed into
> the GuC. There is parameter GUC_CTL_DEVICE_INFO with values
> GfxGtType and GfxCoreFamily unused by GuC.
> 
> This patch remove GUC_CTL_DEVICE_INFO with GfxGtType and
> GfxCoreFamily parameters and also unnecessary functions
> get_gt_type() and get_core_family().
> 

Hi,

Looking at the fw code, you're partially right, GfxGtType is ignored... 
but GfxCoreFamily isn't.

If you don't pass a known GfxCoreFamily, SLPC will be disabled (enabling 
SLPC is in some manager's wish list). Also it seems nobody remembered to 
add GUC_CORE_FAMILY_GEN10 for CNL.

> Signed-off-by: Piotr Piórkowski <piotr.piorkowski at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_params.h    |  2 +-
>   drivers/gpu/drm/i915/intel_guc.c      | 24 ------------------------
>   drivers/gpu/drm/i915/intel_guc_fwif.h |  4 ----
>   drivers/gpu/drm/i915/intel_uc.c       |  2 ++
>   4 files changed, 3 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 430f5f9d0ff4..3deae1e22974 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -47,7 +47,7 @@ struct drm_printer;
>   	param(int, disable_power_well, -1) \
>   	param(int, enable_ips, 1) \
>   	param(int, invert_brightness, 0) \
> -	param(int, enable_guc, 0) \
> +	param(int, enable_guc, -1) \

This shouldn't be part of your patch, enable guc submission in a 2nd 
patch, for example: [PATCH 2/2] HAX: Enable GuC submission for CI

>   	param(int, guc_log_level, 0) \
>   	param(char *, guc_firmware_path, NULL) \
>   	param(char *, huc_firmware_path, NULL) \
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 21140ccd7a97..5f6d84251830 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -200,26 +200,6 @@ void intel_guc_fini(struct intel_guc *guc)
>   	guc_shared_data_destroy(guc);
>   }
>   
> -static u32 get_gt_type(struct drm_i915_private *dev_priv)
> -{
> -	/* XXX: GT type based on PCI device ID? field seems unused by fw */
> -	return 0;
> -}
> -
> -static u32 get_core_family(struct drm_i915_private *dev_priv)
> -{
> -	u32 gen = INTEL_GEN(dev_priv);
> -
> -	switch (gen) {
> -	case 9:
> -		return GUC_CORE_FAMILY_GEN9;
> -
> -	default:
> -		MISSING_CASE(gen);
> -		return GUC_CORE_FAMILY_UNKNOWN;
> -	}
> -}
> -
>   static u32 get_log_verbosity_flags(void)
>   {
>   	if (i915_modparams.guc_log_level > 0) {
> @@ -246,10 +226,6 @@ void intel_guc_init_params(struct intel_guc *guc)
>   
>   	memset(params, 0, sizeof(params));
>   
> -	params[GUC_CTL_DEVICE_INFO] |=
> -		(get_gt_type(dev_priv) << GUC_CTL_GT_TYPE_SHIFT) |
> -		(get_core_family(dev_priv) << GUC_CTL_CORE_FAMILY_SHIFT);
> -
>   	/*
>   	 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
>   	 * second. This ARAR is calculated by:
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 6a10aa6f04d3..0f381de44722 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -81,10 +81,6 @@
>   #define GUC_CTL_ARAT_HIGH		1
>   #define GUC_CTL_ARAT_LOW		2
>   
> -#define GUC_CTL_DEVICE_INFO		3
> -#define   GUC_CTL_GT_TYPE_SHIFT		0
> -#define   GUC_CTL_CORE_FAMILY_SHIFT	7
> -
>   #define GUC_CTL_LOG_PARAMS		4
>   #define   GUC_LOG_VALID			(1 << 0)
>   #define   GUC_LOG_NOTIFY_ON_HALF_FULL	(1 << 1)
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 9f1bac6398fb..b48056fb769d 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
>   		enable_guc |= ENABLE_GUC_LOAD_HUC;
>   
>   	/* Any platform specific fine-tuning can be done here */
> +	if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
> +		enable_guc = 0;

This is also part of [PATCH 2/2] HAX: Enable GuC submission for CI

>   
>   	return enable_guc;
>   }
> 

-Michel