-----Original Message-----
From: Intel-xe <intel-xe-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Lucas De Marchi
Sent: Wednesday, May 15, 2024 2:43 PM
To: intel-xe@xxxxxxxxxxxxxxxxxxxxx
Cc: Tvrtko Ursulin <tursulin@xxxxxxxxxxx>; Nerlige Ramappa, Umesh <umesh.nerlige.ramappa@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; De Marchi, Lucas <lucas.demarchi@xxxxxxxxx>
Subject: [PATCH v4 6/8] drm/xe: Cache data about user-visible engines
>
> gt->info.engine_mask used to indicate the available engines, but that
> is not always true anymore: some engines are reserved to kernel and some
> may be exposed as a single engine (e.g. with ccs_mode).
>
> Runtime changes only happen when no clients exist, so it's safe to cache
> the list of engines in the gt and update that when it's needed. This
> will help implementing per client engine utilization so this (mostly
> constant) information doesn't need to be re-calculated on every query.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
I have a minor nit lower down, but nothing worth blocking over:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
> ---
> drivers/gpu/drm/xe/xe_gt.c | 23 +++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_gt.h | 13 +++++++++++++
> drivers/gpu/drm/xe/xe_gt_ccs_mode.c | 1 +
> drivers/gpu/drm/xe/xe_gt_types.h | 21 ++++++++++++++++++++-
> 4 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index e69a03ddd255..5194a3d38e76 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -560,9 +560,32 @@ int xe_gt_init(struct xe_gt *gt)
> if (err)
> return err;
>
> + xe_gt_record_user_engines(gt);
> +
> return drmm_add_action_or_reset(>_to_xe(gt)->drm, gt_fini, gt);
> }
>
> +void xe_gt_record_user_engines(struct xe_gt *gt)
On one hand, I think it might look better to put
xe_gt_record_user_engines above xe_gt_init. On the
other hand, I recognize that it's not strictly necessary to
make this change because xe_gt_record_user_engines
is declared in xe_gt.h, so this request would just be for
style reasons.
Feel free to disregard, this is just a suggestion.
-Jonathan Cavitt
> +{
> + struct xe_hw_engine *hwe;
> + enum xe_hw_engine_id id;
> +
> + gt->user_engines.mask = 0;
> + memset(gt->user_engines.instances_per_class, 0,
> + sizeof(gt->user_engines.instances_per_class));
> +
> + for_each_hw_engine(hwe, gt, id) {
> + if (xe_hw_engine_is_reserved(hwe))
> + continue;
> +
> + gt->user_engines.mask |= BIT_ULL(id);
> + gt->user_engines.instances_per_class[hwe->class]++;
> + }
> +
> + xe_gt_assert(gt, (gt->user_engines.mask | gt->info.engine_mask)
> + == gt->info.engine_mask);
> +}
> +
> static int do_gt_reset(struct xe_gt *gt)
> {
> int err;
> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> index 8474c50b1b30..ad3fd31e0a41 100644
> --- a/drivers/gpu/drm/xe/xe_gt.h
> +++ b/drivers/gpu/drm/xe/xe_gt.h
> @@ -38,6 +38,19 @@ int xe_gt_init_hwconfig(struct xe_gt *gt);
> int xe_gt_init_early(struct xe_gt *gt);
> int xe_gt_init(struct xe_gt *gt);
> int xe_gt_record_default_lrcs(struct xe_gt *gt);
> +
> +/**
> + * @xe_gt_record_user_engines - save data related to engines available to
> + * usersapce
> + * @gt: GT structure
> + *
> + * Walk the available HW engines from gt->info.engine_mask and calculate data
> + * related to those engines that may be used by userspace. To be used whenever
> + * available engines change in runtime (e.g. with ccs_mode) or during
> + * initialization
> + */
> +void xe_gt_record_user_engines(struct xe_gt *gt);
> +
> void xe_gt_suspend_prepare(struct xe_gt *gt);
> int xe_gt_suspend(struct xe_gt *gt);
> int xe_gt_resume(struct xe_gt *gt);
> diff --git a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> index a34c9a24dafc..c36218f4f6c8 100644
> --- a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> +++ b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> @@ -134,6 +134,7 @@ ccs_mode_store(struct device *kdev, struct device_attribute *attr,
> if (gt->ccs_mode != num_engines) {
> xe_gt_info(gt, "Setting compute mode to %d\n", num_engines);
> gt->ccs_mode = num_engines;
> + xe_gt_record_user_engines(gt);
> xe_gt_reset_async(gt);
> }
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> index 5a114fc9dde7..aaf2951749a6 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -112,7 +112,11 @@ struct xe_gt {
> enum xe_gt_type type;
> /** @info.reference_clock: clock frequency */
> u32 reference_clock;
> - /** @info.engine_mask: mask of engines present on GT */
> + /**
> + * @info.engine_mask: mask of engines present on GT. Some of
> + * them may be reserved in runtime and not available for user.
> + * See @user_engines.mask
> + */
> u64 engine_mask;
> /** @info.gmdid: raw GMD_ID value from hardware */
> u32 gmdid;
> @@ -365,6 +369,21 @@ struct xe_gt {
> /** @wa_active.oob: bitmap with active OOB workaroudns */
> unsigned long *oob;
> } wa_active;
> +
> + /** @user_engines: engines present in GT and available to userspace */
> + struct {
> + /**
> + * @mask: like @info->engine_mask, but take in consideration
> + * only engines available to userspace
> + */
> + u64 mask;
> +
> + /**
> + * @instances_per_class: aggregate per class the number of
> + * engines available to userspace
> + */
> + u8 instances_per_class[XE_ENGINE_CLASS_MAX];
> + } user_engines;
> };
>
> #endif
> --
> 2.43.0
>
>
