[PATCH v2] drm/xe/xe_gt_idle: add debugfs entry for powergating info

Gupta, Anshuman anshuman.gupta at intel.com
Tue Jul 30 14:23:38 UTC 2024



> -----Original Message-----
> From: Tauro, Riana <riana.tauro at intel.com>
> Sent: Tuesday, July 30, 2024 7:33 PM
> To: Nilawar, Badal <badal.nilawar at intel.com>; intel-xe at lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta at intel.com>; Belgaumkar, Vinay
> <vinay.belgaumkar at intel.com>; Roper, Matthew D
> <matthew.d.roper at intel.com>; Wajdeczko, Michal
> <Michal.Wajdeczko at intel.com>
> Subject: Re: [PATCH v2] drm/xe/xe_gt_idle: add debugfs entry for powergating
> info
> 
> Hi Badal
> 
> Thanks for the review comments
> 
> On 7/30/2024 4:38 PM, Nilawar, Badal wrote:
> > Hi Riana,
> >
> > On 29-07-2024 15:59, Riana Tauro wrote:
> >> Coarse Powergating is a power saving technique where Render and Media
> >> can be power-gated independently irrespective of the rest of the GT.
> >>
> >> For debug purposes, it is useful to expose the powergating information.
> >>
> >> v2: move to debugfs
> >>      add details to commit message
> >>      add per-slice status for media
> >>      define reg bits in descending order (Matt Roper)
> >>
> >> v3: fix return statement
> >>      fix kernel-doc
> >>      use loop for media slices
> >>      use helper function for status (Michal)
> >>
> >> Signed-off-by: Riana Tauro <riana.tauro at intel.com>
> >> ---
> >>   drivers/gpu/drm/xe/regs/xe_gt_regs.h |  8 +++
> >>   drivers/gpu/drm/xe/xe_gt_debugfs.c   | 13 +++++
> >>   drivers/gpu/drm/xe/xe_gt_idle.c      | 83
> >> ++++++++++++++++++++++++++++
> >>   drivers/gpu/drm/xe/xe_gt_idle.h      |  2 +
> >>   4 files changed, 106 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> >> b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> >> index 8a94a94d2267..fe774d0894fa 100644
> >> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> >> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> >> @@ -336,6 +336,14 @@
> >>   #define   CTC_SOURCE_DIVIDE_LOGIC        REG_BIT(0)
> >>   #define FORCEWAKE_RENDER            XE_REG(0xa278)
> >> +
> >> +#define POWERGATE_DOMAIN_STATUS            XE_REG(0xa2a0) #define
> >> +MEDIA_SLICE3_AWAKE_STATUS        REG_BIT(4) #define
> >> +MEDIA_SLICE2_AWAKE_STATUS        REG_BIT(3) #define
> >> +MEDIA_SLICE1_AWAKE_STATUS        REG_BIT(2) #define
> >> +RENDER_AWAKE_STATUS            REG_BIT(1) #define
> >> +MEDIA_SLICE0_AWAKE_STATUS        REG_BIT(0)
> >> +
> >>   #define FORCEWAKE_MEDIA_VDBOX(n)        XE_REG(0xa540 + (n) * 4)
> >>   #define FORCEWAKE_MEDIA_VEBOX(n)        XE_REG(0xa560 + (n) * 4)
> >>   #define FORCEWAKE_GSC                XE_REG(0xa618) diff --git
> >> a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> >> b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> >> index 5e7fd937917a..c623295235d9 100644
> >> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> >> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> >> @@ -15,6 +15,7 @@
> >>   #include "xe_ggtt.h"
> >>   #include "xe_gt.h"
> >>   #include "xe_gt_mcr.h"
> >> +#include "xe_gt_idle.h"
> > Header inclusion is out of order.
> >>   #include "xe_gt_sriov_pf_debugfs.h"
> >>   #include "xe_gt_sriov_vf_debugfs.h"
> >>   #include "xe_gt_topology.h"
> >> @@ -107,6 +108,17 @@ static int hw_engines(struct xe_gt *gt, struct
> >> drm_printer *p)
> >>       return 0;
> >>   }
> >> +static int powergate_info(struct xe_gt *gt, struct drm_printer *p) {
> >> +    int ret;
> >> +
> >> +    xe_pm_runtime_get(gt_to_xe(gt));
> >> +    ret = xe_gt_idle_print(gt, p);
> >> +    xe_pm_runtime_put(gt_to_xe(gt));
> >> +
> >> +    return ret;
> >> +}
> >> +
> >>   static int force_reset(struct xe_gt *gt, struct drm_printer *p)
> >>   {
> >>       xe_pm_runtime_get(gt_to_xe(gt)); @@ -277,6 +289,7 @@ static
> >> const struct drm_info_list debugfs_list[] = {
> >>       {"topology", .show = xe_gt_debugfs_simple_show, .data =
> >> topology},
> >>       {"steering", .show = xe_gt_debugfs_simple_show, .data =
> >> steering},
> >>       {"ggtt", .show = xe_gt_debugfs_simple_show, .data = ggtt},
> >> +    {"powergate_info", .show = xe_gt_debugfs_simple_show, .data =
> >> powergate_info},
> >>       {"register-save-restore", .show = xe_gt_debugfs_simple_show,
> >> .data = register_save_restore},
> >>       {"workarounds", .show = xe_gt_debugfs_simple_show, .data =
> >> workarounds},
> >>       {"pat", .show = xe_gt_debugfs_simple_show, .data = pat}, diff
> >> --git a/drivers/gpu/drm/xe/xe_gt_idle.c
> >> b/drivers/gpu/drm/xe/xe_gt_idle.c index 67aba4140510..70fe76735621
> >> 100644
> >> --- a/drivers/gpu/drm/xe/xe_gt_idle.c
> >> +++ b/drivers/gpu/drm/xe/xe_gt_idle.c
> >> @@ -53,6 +53,11 @@ pc_to_xe(struct xe_guc_pc *pc)
> >>       return gt_to_xe(gt);
> >>   }
> >> +static inline const char *str_up_down(bool v) {
> >> +    return v ? "up" : "down";
> >> +}
> >> +
> >>   static const char *gt_idle_state_to_string(enum xe_gt_idle_state
> >> state)
> >>   {
> >>       switch (state) {
> >> @@ -145,6 +150,84 @@ void xe_gt_idle_disable_pg(struct xe_gt *gt)
> >>       XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
> >>   }
> >> +/**
> >> + * xe_gt_idle_print - Xe gt idle print
> >> + * @gt: GT object
> >> + * @p: drm_printer.
> >> + *
> >> + * This function prints the powergating information
> >> + *
> >> + * Return: 0 on success, negative error code otherwise  */ int
> >> +xe_gt_idle_print(struct xe_gt *gt, struct drm_printer *p)
> > Prefix xe_gt_idle points towards RC6, better to incorporate _pg_
> > sufix/prefix in function name.
> Kept the name general if anything else needs to be added.
> will add pg prefix
> > +{
> >> +    struct xe_device *xe = gt_to_xe(gt);
> >> +    u32 pg_enabled, pg_status;
> >> +    u32 vcs_mask, vecs_mask;
> >> +    int err, n;
> >> +    /*
> >> +     * Media Slices
> >> +     *
> >> +     * Slice 0: VCS0, VCS1, VECS0
> >> +     * Slice 1: VCS2, VCS3, VECS1
> >> +     * Slice 2: VCS4, VCS5, VECS2
> >> +     * Slice 3: VCS6, VCS7, VECS3
> >> +     */
> >> +    static const struct {
> >> +        u64 engines;
> >> +        u32 status_bit;
> >> +    } media_slices[] = {
> >> +        {(BIT(XE_HW_ENGINE_VCS0) | BIT(XE_HW_ENGINE_VCS1) |
> >> +          BIT(XE_HW_ENGINE_VECS0)), MEDIA_SLICE0_AWAKE_STATUS},
> >> +
> >> +        {(BIT(XE_HW_ENGINE_VCS2) | BIT(XE_HW_ENGINE_VCS3) |
> >> +           BIT(XE_HW_ENGINE_VECS1)), MEDIA_SLICE1_AWAKE_STATUS},
> >> +
> >> +        {(BIT(XE_HW_ENGINE_VCS4) | BIT(XE_HW_ENGINE_VCS5) |
> >> +           BIT(XE_HW_ENGINE_VECS2)), MEDIA_SLICE2_AWAKE_STATUS},
> >> +
> >> +        {(BIT(XE_HW_ENGINE_VCS6) | BIT(XE_HW_ENGINE_VCS7) |
> >> +           BIT(XE_HW_ENGINE_VECS3)), MEDIA_SLICE3_AWAKE_STATUS},
> >> +    };
> >> +
> >> +    if (xe->info.platform == XE_PVC) {
> >> +        drm_printf(p, "Power Gating Enabled: no\n");
> >> +        return 0;
> >> +    }
> >> +
> >> +    err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
> > When device is in RC6 is there any significance in waking it up and
> > checking pg status. I think we should check PG status only when device
> > not in RC6, may be we should add RC6 check at the top.
> 
> This is required only for debug purposes, is the coupling wih rc6 necessary? We
> cannot save the previous status as there is no park/unpark.
> 
> Any suggestions on what should be the output when in Rc6?
> Should it dump Rc6 status instead of powergating.
If it is in C6 then it is equivalent to power gated.
Thanks,
Anshuman Gupta.
> >> +    if (err)
> > Decrease the refcount / xe_force_wake_put
> If there is an err there is no need of forcewake put
> 
> Thanks,
> Riana
> >
> > Regards,
> > Badal
> >> +        return err;
> >> +
> >> +    pg_enabled = xe_mmio_read32(gt, POWERGATE_ENABLE);
> >> +    pg_status = xe_mmio_read32(gt, POWERGATE_DOMAIN_STATUS);
> >> +
> >> +    XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
> >> +
> >> +    if (gt->info.engine_mask & XE_HW_ENGINE_RCS_MASK) {
> >> +        drm_printf(p, "Render Power Gating Enabled: %s\n",
> >> +               str_yes_no(pg_enabled & RENDER_POWERGATE_ENABLE));
> >> +
> >> +        drm_printf(p, "Render Power Gate Status: %s\n",
> >> +               str_up_down(pg_status & RENDER_AWAKE_STATUS));
> >> +    }
> >> +
> >> +    vcs_mask = xe_hw_engine_mask_per_class(gt,
> >> XE_ENGINE_CLASS_VIDEO_DECODE);
> >> +    vecs_mask = xe_hw_engine_mask_per_class(gt,
> >> XE_ENGINE_CLASS_VIDEO_ENHANCE);
> >> +
> >> +    /* Print media CPG status only if media is present */
> >> +    if (vcs_mask || vecs_mask) {
> >> +        drm_printf(p, "Media Power Gating Enabled: %s\n",
> >> +               str_yes_no(pg_enabled & MEDIA_POWERGATE_ENABLE));
> >> +
> >> +        for (n = 0; n < ARRAY_SIZE(media_slices); n++)
> >> +            if (gt->info.engine_mask & media_slices[n].engines)
> >> +                drm_printf(p, "Media Slice%d Power Gate Status:
> >> %s\n", n,
> >> +                       str_up_down(pg_status &
> >> media_slices[n].status_bit));
> >> +    }
> >> +    return 0;
> >> +}
> >> +
> >>   static ssize_t name_show(struct device *dev,
> >>                struct device_attribute *attr, char *buff)
> >>   {
> >> diff --git a/drivers/gpu/drm/xe/xe_gt_idle.h
> >> b/drivers/gpu/drm/xe/xe_gt_idle.h index 554447b5d46d..4b8192d3aee0
> >> 100644
> >> --- a/drivers/gpu/drm/xe/xe_gt_idle.h
> >> +++ b/drivers/gpu/drm/xe/xe_gt_idle.h
> >> @@ -8,6 +8,7 @@
> >>   #include "xe_gt_idle_types.h"
> >> +struct drm_printer;
> >>   struct xe_gt;
> >>   int xe_gt_idle_init(struct xe_gt_idle *gtidle); @@ -15,5 +16,6 @@
> >> void xe_gt_idle_enable_c6(struct xe_gt *gt);
> >>   void xe_gt_idle_disable_c6(struct xe_gt *gt);
> >>   void xe_gt_idle_enable_pg(struct xe_gt *gt);
> >>   void xe_gt_idle_disable_pg(struct xe_gt *gt);
> >> +int xe_gt_idle_print(struct xe_gt *gt, struct drm_printer *p);
> >>   #endif /* _XE_GT_IDLE_H_ */


More information about the Intel-xe mailing list