| Message ID | 20180305131311.359-1-piotr.piorkowski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
On 3/5/2018 6:43 PM, 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. Minor change to sentence above: Let's stop/remove setup of this parameter ... > > 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> With Michel's suggestion and then corresponding subject update patch looks good to me. Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@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 > -#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)
On Tue, 2018-03-06 at 10:07 +0530, Sagar Arun Kamble wrote: > > On 3/5/2018 6:43 PM, 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. > > Minor change to sentence above: Let's stop/remove setup of this > parameter ... > > > > 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> > > With Michel's suggestion and then corresponding subject update patch > looks good to me. > Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@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 > > -#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) > > What's next with this patch? Can we merge it ?
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 -#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)
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(-)