Re: [PATCH v2] drm/xe/guc: Expose dss per group for GuC error capture

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 16 Jan 2024, Zhanjun Dong <zhanjun.dong@intel.com> wrote:
> Expose helper for dss per group of mcr, GuC error capture feature
> need this info to prepare buffer required.
>
> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_mcr.c          |  2 +-
>  drivers/gpu/drm/xe/xe_gt_mcr.h          |  3 +++
>  drivers/gpu/drm/xe/xe_gt_topology.c     |  3 ---
>  drivers/gpu/drm/xe/xe_guc_capture.c     | 27 +++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_hw_engine_types.h |  3 +++
>  5 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
> index 77925b35cf8d..5874eb71cbc1 100644
> --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
> @@ -295,7 +295,7 @@ static void init_steering_dss(struct xe_gt *gt)
>  {
>  	unsigned int dss = min(xe_dss_mask_group_ffs(gt->fuse_topo.g_dss_mask, 0, 0),
>  			       xe_dss_mask_group_ffs(gt->fuse_topo.c_dss_mask, 0, 0));
> -	unsigned int dss_per_grp = gt_to_xe(gt)->info.platform == XE_PVC ? 8 : 4;
> +	unsigned int dss_per_grp = XE_GT_MCR_DSS_PER_GROUP(gt);
>  
>  	gt->steering[DSS].group_target = dss / dss_per_grp;
>  	gt->steering[DSS].instance_target = dss % dss_per_grp;
> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.h b/drivers/gpu/drm/xe/xe_gt_mcr.h
> index 27ca1bc880a0..fa879bf5c271 100644
> --- a/drivers/gpu/drm/xe/xe_gt_mcr.h
> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.h
> @@ -7,10 +7,13 @@
>  #define _XE_GT_MCR_H_
>  
>  #include "regs/xe_reg_defs.h"
> +#include "xe_gt_types.h"
>  
>  struct drm_printer;
>  struct xe_gt;
>  
> +#define XE_GT_MCR_DSS_PER_GROUP(gt) (gt_to_xe(gt)->info.platform == XE_PVC ? 8 : 4);
> +
>  void xe_gt_mcr_init(struct xe_gt *gt);
>  
>  void xe_gt_mcr_set_implicit_defaults(struct xe_gt *gt);
> diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
> index a8d7f272c30a..e973eeaac7f1 100644
> --- a/drivers/gpu/drm/xe/xe_gt_topology.c
> +++ b/drivers/gpu/drm/xe/xe_gt_topology.c
> @@ -11,9 +11,6 @@
>  #include "xe_gt.h"
>  #include "xe_mmio.h"
>  
> -#define XE_MAX_DSS_FUSE_BITS (32 * XE_MAX_DSS_FUSE_REGS)
> -#define XE_MAX_EU_FUSE_BITS (32 * XE_MAX_EU_FUSE_REGS)
> -
>  static void
>  load_dss_mask(struct xe_gt *gt, xe_dss_mask_t mask, int numregs, ...)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c
> index 0870bfd1b88d..09a61894cf6f 100644
> --- a/drivers/gpu/drm/xe/xe_guc_capture.c
> +++ b/drivers/gpu/drm/xe/xe_guc_capture.c
> @@ -93,6 +93,33 @@
>  	{ SFC_DONE(2),		    0,      0, "SFC_DONE[2]" }, \
>  	{ SFC_DONE(3),		    0,      0, "SFC_DONE[3]" }
>  
> +static inline void xe_gt_mcr_get_ss_steering(struct xe_gt *gt, unsigned int dss,
> +					     unsigned int *group, unsigned int *instance)
> +{
> +	int dss_per_grp = XE_GT_MCR_DSS_PER_GROUP(gt);
> +	*group = dss / dss_per_grp;
> +	*instance = dss % dss_per_grp;
> +}
> +
> +static inline bool xe_sseu_has_subslice(struct xe_gt *gt, int slice, int subslice)
> +{
> +	int dss_per_grp = XE_GT_MCR_DSS_PER_GROUP(gt);
> +	int index = slice * dss_per_grp + subslice;
> +	return index >= XE_MAX_DSS_FUSE_BITS ? false : test_bit(index, gt->fuse_topo.g_dss_mask);
> +}

Generally speaking, you're better off dropping the inline keyword in C
files, and just letting the compiler do its job. In fact, having the
inline prevents the compiler from telling you if the functions are
unused.

> +
> +#define _HAS_SS(ss_, gt_, group_, instance_) xe_sseu_has_subslice(gt_, group_, instance_)
> +
> +/*
> + * Loop over each subslice/DSS and determine the group and instance IDs that
> + * should be used to steer MCR accesses toward this DSS.
> + */
> +#define for_each_ss_steering(ss_, gt_, group_, instance_) \
> +	for (ss_ = 0, xe_gt_mcr_get_ss_steering(gt_, 0, &group_, &instance_); \
> +	     ss_ < XE_MAX_DSS_FUSE_BITS; \
> +	     ss_++, xe_gt_mcr_get_ss_steering(gt_, ss_, &group_, &instance_)) \
> +		for_each_if(_HAS_SS(ss_, gt_, group_, instance_))
> +
>  int xe_guc_capture_init(struct xe_guc *guc)
>  {
>  	return 0;
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_types.h b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> index dfeaaac08b7f..c258228b244f 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_types.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> @@ -65,6 +65,9 @@ struct xe_bo;
>  struct xe_execlist_port;
>  struct xe_gt;
>  
> +#define XE_MAX_DSS_FUSE_BITS (32 * XE_MAX_DSS_FUSE_REGS)
> +#define XE_MAX_EU_FUSE_BITS (32 * XE_MAX_EU_FUSE_REGS)
> +
>  /**
>   * struct xe_hw_engine_class_intf - per hw engine class struct interface
>   *

-- 
Jani Nikula, Intel