[Intel-xe] [PATCH V9 1/6] drm/xe: Add sysfs entries for engines under its GT
Upadhyay, Tejas
tejas.upadhyay at intel.com
Fri Jul 28 13:22:07 UTC 2023
> -----Original Message-----
> From: Brost, Matthew <matthew.brost at intel.com>
> Sent: Thursday, July 27, 2023 8:12 PM
> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> Cc: intel-xe at lists.freedesktop.org; Vishwanathapura, Niranjana
> <niranjana.vishwanathapura at intel.com>
> Subject: Re: [PATCH V9 1/6] drm/xe: Add sysfs entries for engines under its
> GT
>
> On Thu, Jul 27, 2023 at 01:52:30PM +0530, Tejas Upadhyay wrote:
> > Add engines sysfs directory under its GT and create sub directory for
> > all engine class (note its not per instance) present on GT.
> >
> > For example,
> > DUT# cat /sys/class/drm/cardX/device/tileN/gtN/engines/
> > bcs/ ccs/
> >
> > V5 :
> > - replace xe_engine with xe_hw_engine - Matt
> > V4 :
> > - Rebase to resolve conflicts - CI
> > V3 :
> > - Move code in its own file
> > - Rename API name
> > V2 :
> > - Correct class mask logic - Himal
> > - Remove extra parenthesis
> >
>
> A couple nits, otherwise LGTM.
>
> > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > ---
> > drivers/gpu/drm/xe/Makefile | 1 +
> > drivers/gpu/drm/xe/xe_gt.c | 7 ++
> > drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c | 111
> > ++++++++++++++++++ drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.h |
> > 13 ++
> > 4 files changed, 132 insertions(+)
> > create mode 100644 drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> > create mode 100644 drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.h
> >
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index 4ea9e3150c20..a3099a4fd56b 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -52,6 +52,7 @@ xe-y += xe_bb.o \
> > xe_device_sysfs.o \
> > xe_dma_buf.o \
> > xe_engine.o \
> > + xe_hw_engine_class_sysfs.o \
>
> This is out of place alphabetically.
>
> > xe_exec.o \
> > xe_execlist.o \
> > xe_force_wake.o \
> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > index 3e32d38aeeea..d52eb45896a7 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.c
> > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > @@ -27,6 +27,7 @@
> > #include "xe_gt_topology.h"
> > #include "xe_guc_engine_types.h"
> > #include "xe_hw_fence.h"
> > +#include "xe_hw_engine_class_sysfs.h"
> > #include "xe_irq.h"
> > #include "xe_lrc.h"
> > #include "xe_map.h"
> > @@ -322,6 +323,12 @@ static int gt_fw_domain_init(struct xe_gt *gt)
> > if (err)
> > goto err_force_wake;
> >
> > + err = xe_hw_engine_class_sysfs_init(gt);
> > + if (err)
> > + drm_warn(>_to_xe(gt)->drm,
> > + "failed to register engines sysfs directory, err: %d\n",
> > + err);
> > +
> > err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > XE_WARN_ON(err);
> > xe_device_mem_access_put(gt_to_xe(gt));
> > diff --git a/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> > b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> > new file mode 100644
> > index 000000000000..c2f092b54af1
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> > @@ -0,0 +1,111 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright (c) 2023 Intel Corporation */
> > +
> > +#include <drm/drm_managed.h>
> > +#include <linux/kobject.h>
> > +#include <linux/sysfs.h>
> > +
> > +#include "xe_hw_engine_class_sysfs.h"
> > +
> > +static void kobj_xe_hw_engine_release(struct kobject *kobj) {
> > + kfree(kobj);
> > +}
> > +
> > +static const struct kobj_type kobj_xe_hw_engine_type = {
> > + .release = kobj_xe_hw_engine_release,
> > + .sysfs_ops = &kobj_sysfs_ops
> > +};
> > +
> > +static struct kobject *
> > +kobj_xe_hw_engine(struct kobject *parent, char *name) {
> > + struct kobject *kobj;
> > +
> > + kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
> > + if (!kobj)
> > + return NULL;
> > +
> > + kobject_init(kobj, &kobj_xe_hw_engine_type);
> > + if (kobject_add(kobj, parent, "%s", name)) {
> > + kobject_put(kobj);
> > + return NULL;
> > + }
> > +
> > + return kobj;
> > +}
> > +
> > +static void xe_hw_engine_sysfs_kobj_release(struct kobject *kobj) {
> > + kfree(kobj);
> > +}
> > +
> > +static const struct kobj_type xe_hw_engine_sysfs_kobj_type = {
> > + .release = xe_hw_engine_sysfs_kobj_release,
> > + .sysfs_ops = &kobj_sysfs_ops,
> > +};
> > +
>
> We should have kernel doc for all non-static functions, this a WIP across the
> driver but since this is new code I think we should include kernel doc for this
> function.
>
> > +int xe_hw_engine_class_sysfs_init(struct xe_gt *gt) {
> > + struct xe_hw_engine *hwe;
> > + enum xe_hw_engine_id id;
> > + struct kobject *kobj;
> > + u16 class_mask = 0;
> > + int err = 0;
> > +
> > + kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
> > + if (!kobj)
> > + return -ENOMEM;
> > +
> > + kobject_init(kobj, &xe_hw_engine_sysfs_kobj_type);
> > +
> > + err = kobject_add(kobj, gt->sysfs, "engines");
> > + if (err) {
> > + kobject_put(kobj);
> > + return err;
> > + }
> > +
> > + for_each_hw_engine(hwe, gt, id) {
> > + char name[MAX_ENGINE_CLASS_NAME_LEN];
> > + struct kobject *khwe;
> > +
> > + if (hwe->class == XE_ENGINE_CLASS_OTHER ||
> > + hwe->class == XE_ENGINE_CLASS_MAX)
> > + continue;
> > +
> > + if ((class_mask >> hwe->class) & 1)
> > + continue;
> > +
> > + class_mask |= 1 << hwe->class;
> > +
> > + switch (hwe->class) {
> > + case XE_ENGINE_CLASS_RENDER:
> > + strcpy(name, "rcs");
> > + break;
> > + case XE_ENGINE_CLASS_VIDEO_DECODE:
> > + strcpy(name, "vcs");
> > + break;
> > + case XE_ENGINE_CLASS_VIDEO_ENHANCE:
> > + strcpy(name, "vecs");
> > + break;
> > + case XE_ENGINE_CLASS_COPY:
> > + strcpy(name, "bcs");
> > + break;
> > + case XE_ENGINE_CLASS_COMPUTE:
> > + strcpy(name, "ccs");
> > + break;
> > + default:
> > + kobject_put(kobj);
> > + return -EINVAL;
> > + }
> > +
> > + khwe = kobj_xe_hw_engine(kobj, name);
> > + if (!khwe) {
> > + kobject_put(kobj);
> > + return -EINVAL;
> > + }
> > + }
> > + return err;
> > +}
> > diff --git a/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.h
> > b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.h
> > new file mode 100644
> > index 000000000000..c461fac717c2
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright (c) 2023 Intel Corporation */
> > +
> > +#ifndef _XE_ENGINE_CLASS_SYSFS_H_
> > +#define _XE_ENGINE_CLASS_SYSFS_H__
> > +
> > +#include "xe_gt.h"
>
> No need to include this, just forward declare struct xe_gt.
Not only xe_gt, for_each_hw_engine and few others are also coming from same header.
Thanks,
Tejas
>
> > +
> > +#define MAX_ENGINE_CLASS_NAME_LEN 16
>
> This doesn't need to be in the header, just define in the .c file.
>
> > +int xe_hw_engine_class_sysfs_init(struct xe_gt *gt);
>
> Add a newline here.
>
> Matt
>
> > +#endif
> > --
> > 2.25.1
> >
More information about the Intel-xe
mailing list