From: Riana Tauro <riana.tauro@intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: <intel-xe@lists.freedesktop.org>, <anshuman.gupta@intel.com>,
<vinay.belgaumkar@intel.com>
Subject: Re: [PATCH 2/2] RFC drm/xe: Enable Coarse Power Gating
Date: Wed, 15 May 2024 13:05:26 +0530 [thread overview]
Message-ID: <a93400bf-a328-49b0-b297-73e1cd56eabe@intel.com> (raw)
In-Reply-To: <ZkQn1MQiOKRR5c36@intel.com>
Hi Rodrigo
Thanks for the review
On 5/15/2024 8:41 AM, Rodrigo Vivi wrote:
> On Tue, May 14, 2024 at 12:13:09PM +0530, Riana Tauro wrote:
>> Coarse Power Gating (CPG), where Render and Media can enter C6
>> independent of the remaining GT. Enable render and media
>> power gating.
>
> Something strange with this phrase...
Will rephrase it
>
>>
>> Also enable VD HCP/MFX sub-pipe power gating.
>> HCP/MFX power gating is disabled by default, turn it on for
>> the vd units available
>
> Could be simplified to something like:
> "Enable every power gating that are disabled by default,
> for every unit and sub-pipe available"
> (or something like that).
will change this
>
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>> drivers/gpu/drm/xe/regs/xe_gt_regs.h | 2 ++
>> drivers/gpu/drm/xe/xe_gt.c | 10 +++++++
>> drivers/gpu/drm/xe/xe_gt_idle.c | 45 ++++++++++++++++++++++++++--
>> drivers/gpu/drm/xe/xe_gt_idle.h | 2 ++
>> 4 files changed, 56 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> index 7c173db7d585..1cb0343ab581 100644
>> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> @@ -317,6 +317,8 @@
>> #define FORCEWAKE_GT XE_REG(0xa188)
>>
>> #define POWERGATE_ENABLE XE_REG(0xa210)
>> +#define RENDER_POWERGATE_ENABLE REG_BIT(0)
>> +#define MEDIA_POWERGATE_ENABLE REG_BIT(1)
>> #define VDN_HCP_POWERGATE_ENABLE(n) REG_BIT(3 + 2 * (n))
>> #define VDN_MFXVDENC_POWERGATE_ENABLE(n) REG_BIT(4 + 2 * (n))
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>> index 36c7b1631fa6..8a09630f5f38 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.c
>> +++ b/drivers/gpu/drm/xe/xe_gt.c
>> @@ -567,6 +567,10 @@ int xe_gt_init(struct xe_gt *gt)
>> if (err)
>> return err;
>>
>> + err = xe_gt_idle_enable_pg(gt);
>> + if (err)
>> + return err;
>> +
>> return drmm_add_action_or_reset(>_to_xe(gt)->drm, gt_fini, gt);
>> }
>>
>> @@ -624,6 +628,10 @@ static int do_gt_restart(struct xe_gt *gt)
>> if (err)
>> return err;
>>
>> + err = xe_gt_idle_enable_pg(gt);
>> + if (err)
>> + return err;
>> +
>> for_each_hw_engine(hwe, gt, id) {
>> xe_reg_sr_apply_mmio(&hwe->reg_sr, gt);
>> xe_reg_sr_apply_whitelist(hwe);
>> @@ -720,6 +728,8 @@ void xe_gt_suspend_prepare(struct xe_gt *gt)
>> {
>> XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL));
>>
>> + xe_gt_idle_disable_pg(gt);
>> +
>
> they are not the symmetric equivalents... we should probably find
> a more suitable symmetric place for that.
To disable/enable pg during suspend resume, i can move it to
gt_suspend/gt_resume [But these are also called during runtime pm] /
pm_suspend/pm_resume
>
>> xe_uc_stop_prepare(>->uc);
>>
>> XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
>> diff --git a/drivers/gpu/drm/xe/xe_gt_idle.c b/drivers/gpu/drm/xe/xe_gt_idle.c
>> index 4384f7e80258..f549bbb0e110 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_idle.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_idle.c
>> @@ -12,6 +12,7 @@
>> #include "xe_gt_sysfs.h"
>> #include "xe_guc_pc.h"
>> #include "regs/xe_gt_regs.h"
>> +#include "xe_macros.h"
>> #include "xe_mmio.h"
>> #include "xe_pm.h"
>>
>> @@ -93,6 +94,42 @@ static u64 get_residency_ms(struct xe_gt_idle *gtidle, u64 cur_residency)
>> return cur_residency;
>> }
>>
>> +int xe_gt_idle_enable_pg(struct xe_gt *gt)
>> +{
>> + int ret, i;
>> + u32 pg_enable;
>> +
>> + xe_device_assert_mem_access(gt_to_xe(gt));
>> +
>> + pg_enable = RENDER_POWERGATE_ENABLE | MEDIA_POWERGATE_ENABLE;
>> +
>> + for (i = XE_HW_ENGINE_VCS0; i <= XE_HW_ENGINE_VCS7; i++) {
>> + if ((gt->info.engine_mask & BIT(i)))
>> + pg_enable |= (VDN_HCP_POWERGATE_ENABLE(i) |
>> + VDN_MFXVDENC_POWERGATE_ENABLE(i));
>> + }
>> +
>> + ret = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
>> + if (ret)
>> + return ret;
>> +
>> + xe_mmio_write32(gt, POWERGATE_ENABLE, pg_enable);
>> +
>> + XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
>> +
>> + return 0;
>> +}
>> +
>> +void xe_gt_idle_disable_pg(struct xe_gt *gt)
>> +{
>> + xe_device_assert_mem_access(gt_to_xe(gt));
>> + XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT));
>> +
>> + xe_mmio_write32(gt, POWERGATE_ENABLE, 0);
>> +
>> + XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
>> +}
>> +
>> static ssize_t name_show(struct device *dev,
>> struct device_attribute *attr, char *buff)
>> {
>> @@ -145,9 +182,12 @@ static const struct attribute *gt_idle_attrs[] = {
>> NULL,
>> };
>>
>> -static void gt_idle_sysfs_fini(struct drm_device *drm, void *arg)
>> +static void gt_idle_fini(struct drm_device *drm, void *arg)
>> {
>> struct kobject *kobj = arg;
>> + struct xe_gt *gt = kobj_to_gt(kobj->parent);
>> +
>> + xe_gt_idle_disable_pg(gt);
>>
>> sysfs_remove_files(kobj, gt_idle_attrs);
>> kobject_put(kobj);
>> @@ -182,7 +222,7 @@ int xe_gt_idle_sysfs_init(struct xe_gt_idle *gtidle)
>
> we should perhaps also change the name of the init function
> to ensure symmetry?
Will rename it. Will also move xe_gt_idle_enable_pg during probe into
this function.
Thanks
Riana
>
>> return err;
>> }
>>
>> - return drmm_add_action_or_reset(&xe->drm, gt_idle_sysfs_fini, kobj);
>> + return drmm_add_action_or_reset(&xe->drm, gt_idle_fini, kobj);
>> }
>>
>> void xe_gt_idle_enable_c6(struct xe_gt *gt)
>> @@ -202,7 +242,6 @@ void xe_gt_idle_disable_c6(struct xe_gt *gt)
>> xe_device_assert_mem_access(gt_to_xe(gt));
>> xe_force_wake_assert_held(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>>
>> - xe_mmio_write32(gt, POWERGATE_ENABLE, 0);
>> xe_mmio_write32(gt, RC_CONTROL, 0);
>> xe_mmio_write32(gt, RC_STATE, 0);
>> }
>> diff --git a/drivers/gpu/drm/xe/xe_gt_idle.h b/drivers/gpu/drm/xe/xe_gt_idle.h
>> index 75bd99659b1b..ba7aa36348fc 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_idle.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_idle.h
>> @@ -13,5 +13,7 @@ struct xe_gt;
>> int xe_gt_idle_sysfs_init(struct xe_gt_idle *gtidle);
>> void xe_gt_idle_enable_c6(struct xe_gt *gt);
>> void xe_gt_idle_disable_c6(struct xe_gt *gt);
>> +void xe_gt_idle_disable_pg(struct xe_gt *gt);
>> +int xe_gt_idle_enable_pg(struct xe_gt *gt);
>>
>> #endif /* _XE_GT_IDLE_H_ */
>> --
>> 2.40.0
>>
next prev parent reply other threads:[~2024-05-15 7:35 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 6:43 [PATCH 0/2] Enable Coarse Power Gating Riana Tauro
2024-05-14 6:36 ` ✓ CI.Patch_applied: success for " Patchwork
2024-05-14 6:36 ` ✓ CI.checkpatch: " Patchwork
2024-05-14 6:37 ` ✓ CI.KUnit: " Patchwork
2024-05-14 6:43 ` [PATCH 1/2] drm/xe: change the power gating register names Riana Tauro
2024-05-15 2:59 ` Rodrigo Vivi
2024-05-15 5:02 ` Riana Tauro
2024-05-14 6:43 ` [PATCH 2/2] RFC drm/xe: Enable Coarse Power Gating Riana Tauro
2024-05-15 3:11 ` Rodrigo Vivi
2024-05-15 7:35 ` Riana Tauro [this message]
2024-05-14 6:49 ` ✓ CI.Build: success for " Patchwork
2024-05-14 6:51 ` ✓ CI.Hooks: " Patchwork
2024-05-14 6:53 ` ✓ CI.checksparse: " Patchwork
2024-05-14 7:15 ` ✓ CI.BAT: " Patchwork
2024-05-14 8:25 ` ✗ CI.FULL: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a93400bf-a328-49b0-b297-73e1cd56eabe@intel.com \
--to=riana.tauro@intel.com \
--cc=anshuman.gupta@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=rodrigo.vivi@intel.com \
--cc=vinay.belgaumkar@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).