[PATCH] RFC drm/xe/xe_gt_idle: Expose Coarse power gating sysfs entries
Matt Roper
matthew.d.roper at intel.com
Fri Jul 19 17:32:05 UTC 2024
On Fri, Jul 19, 2024 at 11:16:05AM +0530, Riana Tauro wrote:
> Hi Matt
>
> Thank you for the review comments.
>
> On 7/18/2024 12:53 AM, Matt Roper wrote:
> > On Mon, Jul 15, 2024 at 03:24:55PM +0530, Riana Tauro wrote:
> > > Add coarse power gating sysfs interface under gtidle directory to
> > > display the powergate properties.
> >
> > You might also want to explain what "powergate" really means here.
> > I.e., how does this powergating differ from what happens with RC6/MC6?
> > When can powergating kick in?
> Will add these details in the commit message.
> >
> > > powergate sysfs directory will be enumerated for render and media
> > > if present.
> > >
> > > The new entries will have the below layout
> > >
> > > tile<n>/gt<n>/gtidle/
> > > ├── powergate0
> > > │ ├── enabled
> > > │ ├── status
> > > │ └── type
> > > └── powergate<n>
> > > ├── enabled
> > > ├── status
> > > └── type
> > >
> > > type: type of powergate [Render or Media]
> >
> > What userspace software is expected to be reading this interface and
> > what will it be doing with the data?
>
> IGT test is planned for this. Also this is required for debug purposes.
IGT doesn't count as a valid userspace for new uapi. If this winds up
being just for debugging with no real-world usage, then we'll need to
put this in debugfs instead of sysfs.
>
> > I'm surprised that they want just
> > a single "media" powergate rather than providing the actual powergates
> > that the hardware supports (e.g., one entry per individual media slice,
> > which would allow userspace to either consolidate them or split them out
> > as they see fit).
>
> i915 actually displays this info in debugfs and does it only for a single
> media slice (ie slice0).
>
> Since there would be multiple directories, combined it as one in this RFC.
> Will split it per media slice
>
>
> > If we just expose whatever the underlying hardware
> > has (which may vary by platform), then userspace is free to consolidate
> > some of the values itself if it doesn't need the lower level of detail.
> >
> > > enabled: true if powergate is enabled for the type
> > > status: Indicates status of powergate [Up/Down]
> > > For type media, status is "Up" if any of the media
> > > slices are powered on.
> > >
> > > Signed-off-by: Riana Tauro <riana.tauro at intel.com>
> > > ---
> > > drivers/gpu/drm/xe/regs/xe_gt_regs.h | 7 +
> > > drivers/gpu/drm/xe/xe_gt_idle.c | 214 +++++++++++++++++++++++++-
> > > drivers/gpu/drm/xe/xe_gt_idle_types.h | 17 ++
> > > 3 files changed, 237 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > index 8a94a94d2267..8586c830bea6 100644
> > > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > @@ -335,6 +335,13 @@
> > > #define CTC_SHIFT_PARAMETER_MASK REG_GENMASK(2, 1)
> > > #define CTC_SOURCE_DIVIDE_LOGIC REG_BIT(0)
> > > +#define POWERGATE_DOMAIN_STATUS XE_REG(0xa2a0)
> > > +#define MEDIA_SLICE0_AWAKE_STATUS REG_BIT(0)
> > > +#define RENDER_AWAKE_STATUS REG_BIT(1)
> > > +#define MEDIA_SLICE1_AWAKE_STATUS REG_BIT(2)
> > > +#define MEDIA_SLICE2_AWAKE_STATUS REG_BIT(3)
> > > +#define MEDIA_SLICE3_AWAKE_STATUS REG_BIT(4)
> >
> > Nitpick: bits are usually defined in descending order instead of
> > ascending.
> >
> will fix this
> > > +
> > > #define FORCEWAKE_RENDER XE_REG(0xa278)
> > > #define FORCEWAKE_MEDIA_VDBOX(n) XE_REG(0xa540 + (n) * 4)
> > > #define FORCEWAKE_MEDIA_VEBOX(n) XE_REG(0xa560 + (n) * 4)
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_idle.c b/drivers/gpu/drm/xe/xe_gt_idle.c
> > > index 67aba4140510..fca13e645d24 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_idle.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt_idle.c
> > > @@ -25,13 +25,26 @@
> > > * device/gt#/gtidle/name - name of the state
> > > * device/gt#/gtidle/idle_residency_ms - Provides residency of the idle state in ms
> > > * device/gt#/gtidle/idle_status - Provides current idle state
> > > + * device/gt#/gtidle/powergate<n>/
> > > + * ├── enabled - indicates whether powergate is enabled
> > > + * ├── status - status of powergate [Up / Down]
> > > + * └── type - type of powergate
> > > */
> > > +static struct xe_gt_idle *kobj_to_gtidle(struct kobject *kobj)
> > > +{
> > > + return &kobj_to_gt(kobj->parent)->gtidle;
> > > +}
> > > static struct xe_gt_idle *dev_to_gtidle(struct device *dev)
> > > {
> > > struct kobject *kobj = &dev->kobj;
> > > - return &kobj_to_gt(kobj->parent)->gtidle;
> > > + return kobj_to_gtidle(kobj);
> > > +}
> > > +
> > > +static struct xe_gt_powergate *kobj_to_powergate(struct kobject *kobj)
> > > +{
> > > + return container_of(kobj, struct xe_gt_powergate, kobj);
> > > }
> > > static struct xe_gt *gtidle_to_gt(struct xe_gt_idle *gtidle)
> > > @@ -129,6 +142,7 @@ void xe_gt_idle_enable_pg(struct xe_gt *gt)
> > > }
> > > xe_mmio_write32(gt, POWERGATE_ENABLE, pg_enable);
> > > +
> > > XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
> >
> > Unexpected whitespace change?
> yeah missed this. will fix this
> >
> > > }
> > > @@ -145,6 +159,70 @@ void xe_gt_idle_disable_pg(struct xe_gt *gt)
> > > XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
> > > }
> > > +static void gt_idle_powergate_enabled(struct xe_gt_idle *gtidle,
> > > + struct xe_gt_powergate *pg)
> > > +{
> > > + struct xe_gt *gt = gtidle_to_gt(gtidle);
> > > + u32 enabled;
> > > +
> > > + XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT));
> > > + enabled = xe_mmio_read32(gt, POWERGATE_ENABLE);
> > > + XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
> > > +
> > > + if (pg->is_media)
> > > + enabled &= MEDIA_POWERGATE_ENABLE;
> > > + else
> > > + enabled &= RENDER_POWERGATE_ENABLE;
> > > +
> > > + pg->enabled = !!enabled;
> > > +}
> > > +
> > > +static void gt_idle_powergate_status(struct xe_gt_idle *gtidle,
> > > + struct xe_gt_powergate *pg)
> > > +{
> > > + struct xe_gt *gt = gtidle_to_gt(gtidle);
> > > + u32 status, reg;
> > > +
> > > + XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT));
> > > + reg = xe_mmio_read32(gt, POWERGATE_DOMAIN_STATUS);
> > > + XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
> > > +
> > > + if (pg->is_media) {
> > > + /*
> > > + * Media Slices
> > > + *
> > > + * Slice 0: VCS0, VCS1, VECS0
> > > + * Slice 1: VCS2, VCS3, VECS1
> > > + * Slice 2: VCS4, VCS5, VECS2
> > > + * Slice 3: VCS6, VCS7, VECS3
> > > + */
> > > + if (gt->info.engine_mask & (BIT(XE_HW_ENGINE_VCS0)
> > > + | BIT(XE_HW_ENGINE_VCS1)
> > > + | BIT(XE_HW_ENGINE_VECS0)))
> >
> > If this is false...
>
> Won't media slice 0 be always present if media is available?
Not necessarily. Platforms that can support multiple media slices can
have various subsets of those slices fused off on the lower SKUs.
Sometimes that means slice0 isn't present, but slice1 is.
For example, https://gfxspecs.intel.com/Predator/Home/Index/48077 has
some diagrams that show configurations with a fused-off slice0 on
various platforms.
Matt
>
> >
> > > + status = (reg & MEDIA_SLICE0_AWAKE_STATUS);
> > > +
> > > + if (gt->info.engine_mask & (BIT(XE_HW_ENGINE_VCS2)
> > > + | BIT(XE_HW_ENGINE_VCS3)
> > > + | BIT(XE_HW_ENGINE_VECS1)))
> >
> > ...and this is true...
> >
> > > + status |= (reg & MEDIA_SLICE1_AWAKE_STATUS);
> >
> > Then status still has an undefined value here. Ditto for the other two
> > conditions below.
> >
> > > +
> > > + if (gt->info.engine_mask & (BIT(XE_HW_ENGINE_VCS4)
> > > + | BIT(XE_HW_ENGINE_VCS5)
> > > + | BIT(XE_HW_ENGINE_VECS2)))
> > > + status |= (reg & MEDIA_SLICE2_AWAKE_STATUS);
> > > +
> > > + if (gt->info.engine_mask & (BIT(XE_HW_ENGINE_VCS6)
> > > + | BIT(XE_HW_ENGINE_VCS7)
> > > + | BIT(XE_HW_ENGINE_VECS3)))
> > > + status |= (reg & MEDIA_SLICE3_AWAKE_STATUS);
> > > +
> > > + } else {
> > > + status = reg & RENDER_AWAKE_STATUS;
> > > + }
> > > +
> > > + pg->status = !!status;
> > > +}
> > > +
> > > static ssize_t name_show(struct device *dev,
> > > struct device_attribute *attr, char *buff)
> > > {
> > > @@ -197,10 +275,130 @@ static const struct attribute *gt_idle_attrs[] = {
> > > NULL,
> > > };
> > > +static ssize_t status_show(struct device *dev,
> > > + struct device_attribute *attr, char *buff)
> > > +{
> > > + struct kobject *kobj = &dev->kobj;
> > > + struct xe_gt_powergate *pg = kobj_to_powergate(kobj);
> > > + struct xe_gt_idle *gtidle = kobj_to_gtidle(kobj->parent);
> > > + struct xe_gt *gt = gtidle_to_gt(gtidle);
> > > +
> > > + xe_pm_runtime_get(gt_to_xe(gt));
> > > + gt_idle_powergate_status(gtidle, pg);
> > > + xe_pm_runtime_put(gt_to_xe(gt));
> > > +
> > > + return sysfs_emit(buff, "%s\n", pg->status ? "Up" : "Down");
> > > +}
> > > +static DEVICE_ATTR_RO(status);
> > > +
> > > +static ssize_t enabled_show(struct device *dev,
> > > + struct device_attribute *attr, char *buff)
> > > +{
> > > + struct kobject *kobj = &dev->kobj;
> > > + struct xe_gt_powergate *pg = kobj_to_powergate(kobj);
> > > + struct xe_gt_idle *gtidle = kobj_to_gtidle(kobj->parent);
> > > + struct xe_gt *gt = gtidle_to_gt(gtidle);
> > > +
> > > + xe_pm_runtime_get(gt_to_xe(gt));
> > > + gt_idle_powergate_enabled(gtidle, pg);
> > > + xe_pm_runtime_put(gt_to_xe(gt));
> > > +
> > > + return sysfs_emit(buff, "%u\n", pg->enabled);
> > > +}
> > > +static DEVICE_ATTR_RO(enabled);
> > > +
> > > +static ssize_t type_show(struct device *dev,
> > > + struct device_attribute *attr, char *buff)
> > > +{
> > > + struct xe_gt_powergate *pg = kobj_to_powergate(&dev->kobj);
> > > +
> > > + return sysfs_emit(buff, "%s\n", pg->is_media ? "Media" : "Render");
> > > +}
> > > +
> > > +static DEVICE_ATTR_RO(type);
> > > +
> > > +static const struct attribute *powergate_attrs[] = {
> > > + &dev_attr_type.attr,
> > > + &dev_attr_enabled.attr,
> > > + &dev_attr_status.attr,
> > > + NULL,
> > > +};
> > > +
> > > +static void powergate_sysfs_release(struct kobject *kobj) {};
> > > +
> > > +static const struct kobj_type powergate_sysfs_kobj_type = {
> > > + .release = powergate_sysfs_release,
> > > + .sysfs_ops = &kobj_sysfs_ops,
> > > +};
> > > +
> > > +static int register_powergate_sysfs(struct kobject *gtidle_kobj, int idx,
> > > + bool is_media)
> > > +{
> > > + struct xe_gt *gt = kobj_to_gt(gtidle_kobj->parent);
> > > + struct xe_device *xe = gt_to_xe(gt);
> > > + struct xe_gt_idle *gtidle = >->gtidle;
> > > + struct xe_gt_powergate *pg = >idle->powergate[idx];
> > > + int ret;
> > > +
> > > + kobject_init(&pg->kobj, &powergate_sysfs_kobj_type);
> > > +
> > > + ret = kobject_add(&pg->kobj, gtidle_kobj, "powergate%d", idx);
> > > +
> > > + if (ret) {
> > > + kobject_put(&pg->kobj);
> > > + drm_warn(&xe->drm, "Failed to create powergate%d sysfs directory\n", idx);
> > > + return ret;
> > > + }
> > > +
> > > + ret = sysfs_create_files(&pg->kobj, powergate_attrs);
> > > + if (ret) {
> > > + drm_warn(&xe->drm, "Failed to create powergate%d attrs\n", idx);
> > > + kobject_put(&pg->kobj);
> > > + return ret;
> > > + }
> > > +
> > > + pg->is_media = is_media;
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int gt_idle_create_pg_attrs(struct kobject *gtidle_kobj)
> > > +{
> > > + struct xe_gt *gt = kobj_to_gt(gtidle_kobj->parent);
> > > + struct xe_device *xe = gt_to_xe(gt);
> > > + int ret, idx = 0;
> > > +
> > > + /* CPG is disabled for PVC */
> > > + if (xe->info.platform == XE_PVC)
> > > + return 0;
> > > +
> > > + if (!xe_gt_is_media_type(gt)) {
> > > + ret = register_powergate_sysfs(gtidle_kobj, idx, false);
> > > + if (!ret)
> > > + idx++;
> > > + }
> > > +
> > > + /*
> > > + * Do not create media pg status if the entire first media slice
> > > + * is absent (i.e., no VCS0 or VECS0).
> >
> > Is this an attempt to work around the bug above? Otherwise I don't see
> > why we'd want to tie this to media slice 0 specifically and not report
> > powergating when only the other slice(s) are available.Won't media slice
> > 0 be always present if media is available? I saw some
> code in xe checking the presence of slice0 to check if media is available
>
> Thanks,
> Riana
> >
> >
> > Matt
> >
> > > + */
> > > + if (gt->info.engine_mask & (BIT(XE_HW_ENGINE_VCS0) | BIT(XE_HW_ENGINE_VECS0))) {
> > > + ret = register_powergate_sysfs(gtidle_kobj, idx, true);
> > > + if (!ret)
> > > + idx++;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static void gt_idle_fini(void *arg)
> > > {
> > > struct kobject *kobj = arg;
> > > struct xe_gt *gt = kobj_to_gt(kobj->parent);
> > > + struct xe_gt_idle *gtidle = >->gtidle;
> > > + struct xe_device *xe = gt_to_xe(gt);
> > > + struct xe_gt_powergate *pg;
> > > + int n;
> > > xe_gt_idle_disable_pg(gt);
> > > @@ -210,6 +408,18 @@ static void gt_idle_fini(void *arg)
> > > xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > > }
> > > + for (n = 0 ; n < 2; n++) {
> > > + if (xe->info.platform == XE_PVC)
> > > + break;
> > > +
> > > + pg = >idle->powergate[n];
> > > +
> > > + if (pg->kobj.state_initialized) {
> > > + sysfs_remove_files(&pg->kobj, powergate_attrs);
> > > + kobject_put(&pg->kobj);
> > > + }
> > > + }
> > > +
> > > sysfs_remove_files(kobj, gt_idle_attrs);
> > > kobject_put(kobj);
> > > }
> > > @@ -248,6 +458,8 @@ int xe_gt_idle_init(struct xe_gt_idle *gtidle)
> > > xe_gt_idle_enable_pg(gt);
> > > + gt_idle_create_pg_attrs(kobj);
> > > +
> > > return devm_add_action_or_reset(xe->drm.dev, gt_idle_fini, kobj);
> > > }
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_idle_types.h b/drivers/gpu/drm/xe/xe_gt_idle_types.h
> > > index f99b447534f3..603f7f6b509d 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_idle_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_gt_idle_types.h
> > > @@ -7,6 +7,7 @@
> > > #define _XE_GT_IDLE_SYSFS_TYPES_H_
> > > #include <linux/types.h>
> > > +#include <linux/kobject.h>
> > > struct xe_guc_pc;
> > > @@ -17,6 +18,20 @@ enum xe_gt_idle_state {
> > > GT_IDLE_UNKNOWN,
> > > };
> > > +/**
> > > + * struct xe_gt_powergate - Powergate properties
> > > + */
> > > +struct xe_gt_powergate {
> > > + /** @enabled: powergate enable */
> > > + bool enabled;
> > > + /** @status: status of powerwell*/
> > > + bool status;
> > > + /** @is_media: indicates media powergate */
> > > + bool is_media;
> > > + /** @kobj: kobject for powergate sysfs */
> > > + struct kobject kobj;
> > > +};
> > > +
> > > /**
> > > * struct xe_gt_idle - A struct that contains idle properties based of gt
> > > */
> > > @@ -29,6 +44,8 @@ struct xe_gt_idle {
> > > u64 cur_residency;
> > > /** @prev_residency: previous residency counter */
> > > u64 prev_residency;
> > > + /** @powergate: powergate properties */
> > > + struct xe_gt_powergate powergate[2];
> > > /** @idle_status: get the current idle state */
> > > enum xe_gt_idle_state (*idle_status)(struct xe_guc_pc *pc);
> > > /** @idle_residency: get idle residency counter */
> > > --
> > > 2.40.0
> > >
> >
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list