Re: [PATCH v2] drm/i915/guc: Remove GUC_CTL_DEVICE_INFO parameter

[Michel Thierry <michel.thierry@intel.com>]

On 3/5/2018 5:13 AM, Piotr Piórkowski wrote:
> It looks that GuC does not actively use GUC_CTL_DEVICE_INFO parameter
> where we are passing GT type and Core family values.
> Lets stop setup this parameter and remove related definitions.
> 
> v2: (this time without squashed HAX)
>    - New title and description
>    - Remove also GUC_CORE_FAMILY_* definitions (Michel)
> 
> Signed-off-by: Piotr Piórkowski <piotr.piorkowski@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: John A Spotswood <john.a.spotswood@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc.c      | 24 ------------------------
>   drivers/gpu/drm/i915/intel_guc_fwif.h |  7 -------
>   2 files changed, 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index ff08ea0ebf49..efc413137a89 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..5131e67e663f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -23,9 +23,6 @@
>   #ifndef _INTEL_GUC_FWIF_H
>   #define _INTEL_GUC_FWIF_H
>   
> -#define GUC_CORE_FAMILY_GEN9		12
> -#define GUC_CORE_FAMILY_UNKNOWN		0x7fffffff
> -
>   #define GUC_CLIENT_PRIORITY_KMD_HIGH	0
>   #define GUC_CLIENT_PRIORITY_HIGH	1
>   #define GUC_CLIENT_PRIORITY_KMD_NORMAL	2
> @@ -81,10 +78,6 @@
>   #define GUC_CTL_ARAT_HIGH		1
>   #define GUC_CTL_ARAT_LOW		2
>   
> -#define GUC_CTL_DEVICE_INFO		3

I would keep GUC_CTL_DEVICE_INFO (still removing _GT_TYPE_SHIFT and 
_CORE_FAMILY_SHIFT). The reason is that these constants kindof also 
document what the guc_control is. Other people may even suggest to 
rename it GUC_CTL_RSRVD0.

That's why we have GUC_CTL_PAGE_FAULT_CONTROL and GUC_CTL_RSRVD defined 
but not used.

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

I think the consensus was that gt_type and core_family are not needed 
now, and if someone needs them at some point in the very very distant 
future, then the same person can re-add them, so

Acked-by: Michel Thierry <michel.thierry@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx