[PATCH v4 6/8] drm/xe: Cache data about user-visible engines
Umesh Nerlige Ramappa
umesh.nerlige.ramappa at intel.com
Thu May 16 18:33:54 UTC 2024
On Wed, May 15, 2024 at 02:42:56PM -0700, Lucas De Marchi wrote:
>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 at intel.com>
Just a few questions below, otherwise this looks good as is:
Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>---
> 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)
>+{
>+ 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);
I am not seeing a place where user_engines.mask is not a subset of
info.engine_mask in the driver, so the above check will always be true.
Did you mean to do and & instead of | above? That might make sense since
then you are making sure that the user_engines are a subset of
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
After the driver loads, do we expect ccs_mode to change dynamically
based on some criteria OR is it a one time configuration at driver load?
If former, can you provide an example where ccs_mode would change
dynamically, just curious.
Regards,
Umesh
>+ * 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
>
More information about the dri-devel
mailing list