[PATCH v3 4/6] drm/xe: Add misc functions to support read of specific DSS registers
Dong, Zhanjun
zhanjun.dong at intel.com
Tue Jan 30 15:54:28 UTC 2024
On 2024-01-30 9:42 a.m., Souza, Jose wrote:
> On Mon, 2024-01-29 at 13:24 -0800, Matt Roper wrote:
>> On Mon, Jan 29, 2024 at 10:17:40AM -0800, José Roberto de Souza wrote:
>>> Next patch will read register in specific DSS registers and this
>>> are the functions missing to do so.
>>>
>>> xe_gt_mcr_get_dss_steering() calculate and return the group and
>>> instance that will be used by xe_gt_mcr_unicast_read().
>>>
>>> xe_gt_has_geometry_dss() and xe_gt_has_compute_dss() returns true
>>> if DSS is available for geometry of compute.
>>>
>>> for_each_geometry/compute_dss() to simply the iteration over each
>>> available DSS
>>>
>>> v3:
>>> - add for_each_geometry/compute_dss()
>>>
>>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>> Cc: Matt Roper <matthew.d.roper at intel.com>
>>> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
>>> ---
>>> drivers/gpu/drm/xe/xe_gt.c | 17 ++++++++++++++++
>>> drivers/gpu/drm/xe/xe_gt.h | 3 +++
>>> drivers/gpu/drm/xe/xe_gt_mcr.c | 17 +++++++++++++++-
>>> drivers/gpu/drm/xe/xe_gt_mcr.h | 31 +++++++++++++++++++++++++++++
>>> drivers/gpu/drm/xe/xe_gt_topology.c | 1 -
>>> drivers/gpu/drm/xe/xe_gt_types.h | 3 ++-
>>> 6 files changed, 69 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>>> index 675a2927a19ef..9a3dce45b92ba 100644
>>> --- a/drivers/gpu/drm/xe/xe_gt.c
>>> +++ b/drivers/gpu/drm/xe/xe_gt.c
>>> @@ -795,3 +795,20 @@ struct xe_hw_engine *xe_gt_any_hw_engine_by_reset_domain(struct xe_gt *gt,
>>>
>>> return NULL;
>>> }
>>> +
>>> +static bool has_dss(xe_dss_mask_t dss_mask, unsigned int dss)
>>> +{
>>> + unsigned long value = bitmap_get_value8(dss_mask, (dss / 8) * 8);
>>> +
>>> + return value & BIT(dss % 8);
>>> +}
>>> +
>>> +bool xe_gt_has_geometry_dss(struct xe_gt *gt, unsigned int dss)
>>> +{
>>> + return has_dss(gt->fuse_topo.g_dss_mask, dss);
>>> +}
>>> +
>>> +bool xe_gt_has_compute_dss(struct xe_gt *gt, unsigned int dss)
>>> +{
>>> + return has_dss(gt->fuse_topo.c_dss_mask, dss);
>>> +}
>>
>> It feels like these belong better in xe_gt_topology.[ch]. Ideally we'd
>> keep all the low-level representation of the masks abstracted away
>> inside of those files and just provide a sane interface for the rest of
>> the driver to query whatever it needs.
>
> sure, can move it.
>
>>
>>> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
>>> index c1675bd44cf6d..36815d8cbc107 100644
>>> --- a/drivers/gpu/drm/xe/xe_gt.h
>>> +++ b/drivers/gpu/drm/xe/xe_gt.h
>>> @@ -70,4 +70,7 @@ static inline bool xe_gt_is_usm_hwe(struct xe_gt *gt, struct xe_hw_engine *hwe)
>>> hwe->instance == gt->usm.reserved_bcs_instance;
>>> }
>>>
>>> +bool xe_gt_has_geometry_dss(struct xe_gt *gt, unsigned int dss);
>>> +bool xe_gt_has_compute_dss(struct xe_gt *gt, unsigned int dss);
>>> +
>>> #endif
>>> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
>>> index 77925b35cf8dc..e76cb0ae457aa 100644
>>> --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
>>> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
>>> @@ -291,11 +291,17 @@ static void init_steering_mslice(struct xe_gt *gt)
>>> gt->steering[LNCF].instance_target = 0; /* unused */
>>> }
>>>
>>> +static unsigned int
>>> +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 = 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 +689,12 @@ void xe_gt_mcr_steering_dump(struct xe_gt *gt, struct drm_printer *p)
>>> }
>>> }
>>> }
>>> +
>>> +void
>>> +xe_gt_mcr_get_dss_steering(struct xe_gt *gt, unsigned int dss, int *group,
>>> + int *instance)
>>
>> I think you're primarily adding this for eventual use in the
>> devcoredump, right? But I think there's also been some other series
>> trying to add some MCR registers of types other than DSS (e.g.,
>> per-mslice registers and such). Given that, maybe we should make this
>> function more general where you pass the MCR class as a parameter too?
>> E.g.,
>
> Yes, need that to get instdone registers for the Mesa error dump parser tool.
>
> Thanks for pointing that Zhanjun was also working on that.
> Looking at Zhanjun patches looks like we had almost the same functions but he is using 'ss' instead of 'dss'.
Yes, we are working on the same functions, sure I can use 'dss', no problem.
>
>>
>> void
>> xe_gt_mcr_get_steering(struct xe_gt *gt,
>> enum xe_steering_type type,
>> unsigned int id,
>> unsigned int *group,
>> unsigned int *instance)
>>
>>
>> BTW, I think Zhanjun is working along similar lines to what you have
>> here, so you guys might want to sync up?
>
> I don't think the function above is needed, what do you think Zhanjun?
This function is to get group/instance steering from dss id, the only
place used is by for_each_xxx, the macro call this function, loop with
dss id, check against mask and call a code block, that's the typical
usage. For Xe, as we are using the g_dss_mask and c_dss_mask to test
bit, the converted group and instance is not used, so looks like
xe_gt_mcr_get_steering could be optimized out and the for_each macro
could be optimized to:
#define for_each_geometry_dss(gt, dss) \
for (dss = 0; dss < XE_MAX_DSS_FUSE_BITS; dss++) \
if (_HAS_DSS(gt, dss))
*_HAS_DSS means xe_gt_has_geometry_dss/xe_gt_has_compute_dss
The above is based on only this macro calls xe_gt_mcr_get_steering, not
sure if Matthew has some thought about the steering_type?
Regards,
Zhanjun
>
>>
>>
>> Matt
>>
>>> +{
>>> + unsigned int dss_per_group = get_dss_per_group(gt);
>>> + *group = dss / dss_per_group;
>>> + *instance = dss % dss_per_group;
>>> +}
>>> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.h b/drivers/gpu/drm/xe/xe_gt_mcr.h
>>> index 27ca1bc880a00..9f5f7dbb6fca8 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;
>>> @@ -24,6 +25,36 @@ void xe_gt_mcr_unicast_write(struct xe_gt *gt, struct xe_reg_mcr mcr_reg,
>>> void xe_gt_mcr_multicast_write(struct xe_gt *gt, struct xe_reg_mcr mcr_reg,
>>> u32 value);
>>>
>>> +void
>>> +xe_gt_mcr_get_dss_steering(struct xe_gt *gt, unsigned int dss, int *group,
>>> + int *instance);
>>> +
>>> void xe_gt_mcr_steering_dump(struct xe_gt *gt, struct drm_printer *p);
>>>
>>> +/**
>>> + * for_each_geometry_dss - Iterate over each DSS available for geometry
>>> + * @gt: GT structure
>>> + * @dss: DSS id
>>> + * @grp: group id to be in xe_gt_mcr_unicast_read()
>>> + * @inst: instance id to be in xe_gt_mcr_unicast_read()
>>> + */
>>> +#define for_each_geometry_dss(gt, dss, grp, inst) \
>>> + for (dss = 0, xe_gt_mcr_get_dss_steering(gt, dss, &grp, &inst); \
>>> + dss < XE_MAX_DSS_FUSE_BITS; \
>>> + dss++, xe_gt_mcr_get_dss_steering(gt, dss, &grp, &inst)) \
>>> + if (xe_gt_has_geometry_dss(gt, dss))
>>> +
>>> +/**
>>> + * for_each_compute_dss - Iterate over each DSS available for compute
>>> + * @gt: GT structure
>>> + * @dss: DSS id
>>> + * @grp: group id to be in xe_gt_mcr_unicast_read()
>>> + * @inst: instance id to be in xe_gt_mcr_unicast_read()
>>> + */
>>> +#define for_each_compute_dss(gt, dss, grp, inst) \
>>> + for (dss = 0, xe_gt_mcr_get_dss_steering(gt, dss, &grp, &inst); \
>>> + dss < XE_MAX_DSS_FUSE_BITS; \
>>> + dss++, xe_gt_mcr_get_dss_steering(gt, dss, &grp, &inst)) \
>>> + if (xe_gt_has_compute_dss(gt, dss))
>>> +
>>> #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 a8d7f272c30a0..c4942f2b37751 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)
>>>
>>> static void
>>> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
>>> index 70c615dd14986..bb6dc1fcaa7dd 100644
>>> --- a/drivers/gpu/drm/xe/xe_gt_types.h
>>> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
>>> @@ -25,9 +25,10 @@ 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)];
>>> +typedef unsigned long xe_dss_mask_t[BITS_TO_LONGS(XE_MAX_DSS_FUSE_BITS)];
>>> typedef unsigned long xe_eu_mask_t[BITS_TO_LONGS(32 * XE_MAX_EU_FUSE_REGS)];
>>>
>>> struct xe_mmio_range {
>>> --
>>> 2.43.0
>>>
>>
>
More information about the Intel-xe
mailing list