[PATCH v3 1/1] drm/xe: Expose number of dss per group and helpers
Summers, Stuart
stuart.summers at intel.com
Tue Jan 30 16:10:38 UTC 2024
On Mon, 2024-01-29 at 14:34 -0800, Zhanjun Dong wrote:
> Expose helper for dss per group, features like GuC register
Honestly, I realize the intent, but the steering mechanism - and even
the DSS construct in general - has nothing to do with GuC. This is
purely defined by register access (in general) and compute allocation.
I'd really rather leave this language out. Maybe something like:
Expose helper for dss per group. This is a precursor patch to allow
for easier iteration through MCR registers and other per-DSS uses.
> capture will need this info to prepare buffer required.
>
> Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
> ---
> drivers/gpu/drm/xe/xe_gt_mcr.c | 40
> ++++++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_gt_mcr.h | 17 +++++++++++
> drivers/gpu/drm/xe/xe_gt_topology.c | 3 --
> drivers/gpu/drm/xe/xe_hw_engine_types.h | 3 ++
> 4 files changed, 59 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..706f9fd43f6e 100644
> --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
Ok, hm.. really sorry for the back and forth here.
So the naming convention here was obviously based on the file name -
which I had missed for some reason. I wasn't in the original reviews
for this and I'm honestly not super happy to tie the MCR to DSS pieces
like this. IMO DSS are more about topology and MCR is just a user of
the DSS (there can be and are other users).
That said, I also don't really like the idea of mixing the naming
convention within this file. To me it would be better to do a clean
refactor, pull these functions out of here and add to a topology file
(or even just within xe_gt.c for example).
That said, what you're adding here is really not enough to warrant
that. The refactor really should be done separately.
So I'm really sorry to ask this, but can you add xe_gt_mcr_* to the
various function names I had talked about in the last review?
I'll keep an eye for the update, with that (and checkpatch happy), I'm
happy to add my R-B here.
Thanks,
Stuart
> @@ -291,11 +291,16 @@ static void init_steering_mslice(struct xe_gt
> *gt)
> gt->steering[LNCF].instance_target = 0; /* unused */
> }
>
> +int xe_gt_get_dss_per_group(struct xe_gt *gt)
> +{
> + return gt_to_xe(gt)->info.platform == XE_PVC ? 8 : 4;
> +}
> +
> 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_get_dss_per_group(gt);
>
> gt->steering[DSS].group_target = dss / dss_per_grp;
> gt->steering[DSS].instance_target = dss % dss_per_grp;
> @@ -683,3 +688,36 @@ void xe_gt_mcr_steering_dump(struct xe_gt *gt,
> struct drm_printer *p)
> }
> }
> }
> +
> +/**
> + * xe_gt_mcr_get_ss_steering - returns the group/instance steering
> for a SS
> + * @gt: GT structure
> + * @dss: DSS ID to obtain steering for
> + * @group: pointer to storage for steering group ID
> + * @instance: pointer to storage for steering instance ID
> + *
> + * Returns the steering IDs (via the @group and @instance
> parameters) that
> + * correspond to a specific subslice/DSS ID.
> + */
> +void xe_gt_get_ss_steering(struct xe_gt *gt, unsigned int dss,
> unsigned int *group,
> + unsigned int *instance)
> +{
> + int dss_per_grp = xe_gt_get_dss_per_group(gt);
> +
> + *group = dss / dss_per_grp;
> + *instance = dss % dss_per_grp;
> +}
> +
> +bool xe_gt_dss_has_subslice(struct xe_gt *gt, int slice, int
> subslice)
> +{
> + int dss_per_grp = xe_gt_get_dss_per_group(gt);
> + int index = slice * dss_per_grp + subslice;
> +
> + if (index >= XE_MAX_DSS_FUSE_BITS) {
> + xe_gt_dbg(gt, "DSS id is out of range. slice: %d
> subslice: %d\n", slice, subslice);
> + return false;
> + } else {
> + return test_bit(index, gt->fuse_topo.g_dss_mask) ||
> + test_bit(index, gt->fuse_topo.c_dss_mask);
> + }
> +}
> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.h
> b/drivers/gpu/drm/xe/xe_gt_mcr.h
> index 27ca1bc880a0..81b2efcb04c3 100644
> --- a/drivers/gpu/drm/xe/xe_gt_mcr.h
> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.h
> @@ -7,6 +7,7 @@
> #define _XE_GT_MCR_H_
>
> #include "regs/xe_reg_defs.h"
> +#include "xe_gt_types.h"
>
> struct drm_printer;
> struct xe_gt;
> @@ -25,5 +26,21 @@ void xe_gt_mcr_multicast_write(struct xe_gt *gt,
> struct xe_reg_mcr mcr_reg,
> u32 value);
>
> void xe_gt_mcr_steering_dump(struct xe_gt *gt, struct drm_printer
> *p);
> +int xe_gt_get_dss_per_group(struct xe_gt *gt);
> +void xe_gt_get_ss_steering(struct xe_gt *gt, unsigned int dss,
> unsigned int *group,
> + unsigned int *instance);
> +bool xe_gt_dss_has_subslice(struct xe_gt *gt, int slice, int
> subslice);
> +
> +#define _HAS_SS(gt_, group_, instance_) xe_gt_dss_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_get_ss_steering(gt_, 0, &group_,
> &instance_); \
> + ss_ < XE_MAX_DSS_FUSE_BITS; \
> + ss_++, xe_gt_get_ss_steering(gt_, ss_, &group_,
> &instance_)) \
> + for_each_if(_HAS_SS(gt_, group_, instance_))
>
> #endif /* _XE_GT_MCR_H_ */
> 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_hw_engine_types.h
> b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> index d7f828c76cc5..3e978a190041 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
> *
More information about the Intel-xe
mailing list