[PATCH v4 1/1] drm/xe: Expose number of dss per group and helpers
Dong, Zhanjun
zhanjun.dong at intel.com
Tue Jan 30 20:47:35 UTC 2024
On 2024-01-30 2:28 p.m., Summers, Stuart wrote:
> On Tue, 2024-01-30 at 10:57 -0800, Zhanjun Dong wrote:
>> Expose helper for dss per group. This is a precursor patch to allow
>> for easier iteration through MCR registers and other per-DSS uses.
>>
>> 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 | 1 -
>> drivers/gpu/drm/xe/xe_gt_types.h | 1 +
>> 4 files changed, 57 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c
>> b/drivers/gpu/drm/xe/xe_gt_mcr.c
>> index 77925b35cf8d..7cd93bcb811f 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
>> @@ -291,11 +291,16 @@ static void init_steering_mslice(struct xe_gt
>> *gt)
>> gt->steering[LNCF].instance_target = 0; /* unused */
>> }
>>
>> +int xe_gt_mcr_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_mcr_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_dss_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_mcr_get_dss_steering(struct xe_gt *gt, unsigned int dss,
>> unsigned int *group,
>> + unsigned int *instance)
>> +{
>> + int dss_per_grp = xe_gt_mcr_get_dss_per_group(gt);
>> +
>> + *group = dss / dss_per_grp;
>> + *instance = dss % dss_per_grp;
>> +}
>> +
>> +bool xe_gt_mcr_dss_has_subslice(struct xe_gt *gt, int slice, int
>> subslice)
>> +{
>> + int dss_per_grp = xe_gt_mcr_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 out of range: slice:%d
>> subslice:%d\n", slice, subslice);
>> + return false;
>> + }
>> +
>> + 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..7d1eb180befd 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_mcr_get_dss_per_group(struct xe_gt *gt);
>> +void xe_gt_mcr_get_dss_steering(struct xe_gt *gt, unsigned int dss,
>> unsigned int *group,
>> + unsigned int *instance);
>> +bool xe_gt_mcr_dss_has_subslice(struct xe_gt *gt, int slice, int
>> subslice);
>> +
>> +#define _HAS_SS(gt_, group_, instance_)
>
> Maybe gt__, group__, instance__
Thanks, that looks better
>
>> xe_gt_mcr_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_dss_steering(dss_, gt_, group_, instance_) \
>> + for (dss_ = 0, xe_gt_mcr_get_dss_steering(gt_, 0, &group_,
>> &instance_); \
>> + dss_ < XE_MAX_DSS_FUSE_BITS; \
>> + dss_++, xe_gt_mcr_get_dss_steering(gt_, dss_, &group_,
>> &instance_)) \
>
> I think you're still going to get those checkpatch warnings here.
>
> Are you planning on addressing that?
The checkpatch complains 6 warnings in about 2 kind of things:
#1. "CHECK:MACRO_ARG_REUSE: Macro argument reuse 'gt_' - possible
side-effects?"
The warning applies to all 4 aruguments, as they all being reused in
loop init and loop iteration, this is by purpose and I will accept it.
#2. CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'group_' may be better as
'(group_)' to avoid precedence issues
Applied to group_ and instance_
Macro body has unary operator & to get address, it could be fixed by:
#define for_each_dss_steering(dss_, gt_, group_, instance_) \
for (dss_ = 0, xe_gt_mcr_get_dss_steering(gt_, 0, &(group_),
&(instance_)); \
dss_ < XE_MAX_DSS_FUSE_BITS; \
dss_++, xe_gt_mcr_get_dss_steering(gt_, dss_, &(group_),
&(instance_))) \
for_each_if(_HAS_SS(gt_, (group_), (instance_)))
I will leaves type #1 but fix type #2, total checkpatch warnings will
reduced from 6 to 4.
>
>> + 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..c4942f2b3775 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_topology.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_topology.c
>> @@ -11,7 +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)
>
> Honestly I do think these should be in the same file at least. Can you
> move the EU FUSE BITS to xe_gt_types.h too?
>
> Thanks,
> Stuart
Sure I will move it together.
Regards,
Zhanjun
>
>>
>> static void
>> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h
>> b/drivers/gpu/drm/xe/xe_gt_types.h
>> index 70c615dd1498..b4df7d35dec7 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_types.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
>> @@ -25,6 +25,7 @@ enum xe_gt_type {
>> };
>>
>> #define XE_MAX_DSS_FUSE_REGS 3
>> +#define XE_MAX_DSS_FUSE_BITS (32 * XE_MAX_DSS_FUSE_REGS)
>> #define XE_MAX_EU_FUSE_REGS 1
>>
>> typedef unsigned long xe_dss_mask_t[BITS_TO_LONGS(32 *
>> XE_MAX_DSS_FUSE_REGS)];
>
More information about the Intel-xe
mailing list