* [Intel-gfx] [PATCH 01/11] drm/i915: Avoid rpm helpers in intel_guc_global_policies_update
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 02/11] drm/i915: Avoid rpm helpers in intel_guc_slpc_set_media_ratio_mode Tilak Tangudu
` (11 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
1. Removed rpm helpers in intel_guc_global_policies_update
and added rpm helpers in below higher level functions,
-intel_selftest_modify_policy
-intel_selftest_restore_policy
-notify_guc
-intel_uc_reset_finish calls intel_guc_global_policies_update
via intel_guc_submission_reset_finish
2. Removed rpm helpers in intel_guc_submission_reset_finish
and added rpm helpers in below higher level functions,
- at intel_uc_reset_finish in reset_finish
- intel_gt_resume (already rpm wakeref is available)
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/gt/intel_reset.c | 4 +++-
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 5 +----
drivers/gpu/drm/i915/i915_debugfs_params.c | 7 +++++--
.../drm/i915/selftests/intel_scheduler_helpers.c | 13 +++++++++++--
4 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index a5338c3fde7a..c8e05b48c14f 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -828,6 +828,7 @@ static void reset_finish(struct intel_gt *gt, intel_engine_mask_t awake)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
+ intel_wakeref_t wakeref;
for_each_engine(engine, gt, id) {
reset_finish_engine(engine);
@@ -835,7 +836,8 @@ static void reset_finish(struct intel_gt *gt, intel_engine_mask_t awake)
intel_engine_pm_put(engine);
}
- intel_uc_reset_finish(>->uc);
+ with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+ intel_uc_reset_finish(>->uc);
}
static void nop_submit_request(struct i915_request *request)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
index ba7541f3ca61..3f24ad4cb2e1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -182,9 +182,7 @@ static int guc_action_policies_update(struct intel_guc *guc, u32 policy_offset)
int intel_guc_global_policies_update(struct intel_guc *guc)
{
- struct intel_gt *gt = guc_to_gt(guc);
u32 scheduler_policies;
- intel_wakeref_t wakeref;
int ret;
if (iosys_map_is_null(&guc->ads_map))
@@ -198,8 +196,7 @@ int intel_guc_global_policies_update(struct intel_guc *guc)
if (!intel_guc_is_ready(guc))
return 0;
- with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
- ret = guc_action_policies_update(guc, scheduler_policies);
+ ret = guc_action_policies_update(guc, scheduler_policies);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_debugfs_params.c b/drivers/gpu/drm/i915/i915_debugfs_params.c
index 783c8676eee2..cf92c98fa81a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs_params.c
+++ b/drivers/gpu/drm/i915/i915_debugfs_params.c
@@ -39,9 +39,12 @@ static int i915_param_int_open(struct inode *inode, struct file *file)
static int notify_guc(struct drm_i915_private *i915)
{
int ret = 0;
+ intel_wakeref_t wakeref;
- if (intel_uc_uses_guc_submission(&to_gt(i915)->uc))
- ret = intel_guc_global_policies_update(&to_gt(i915)->uc.guc);
+ if (intel_uc_uses_guc_submission(&to_gt(i915)->uc)) {
+ with_intel_runtime_pm(to_gt(i915)->uncore->rpm, wakeref)
+ ret = intel_guc_global_policies_update(&to_gt(i915)->uc.guc);
+ }
return ret;
}
diff --git a/drivers/gpu/drm/i915/selftests/intel_scheduler_helpers.c b/drivers/gpu/drm/i915/selftests/intel_scheduler_helpers.c
index 310fb83c527e..463a378f0abb 100644
--- a/drivers/gpu/drm/i915/selftests/intel_scheduler_helpers.c
+++ b/drivers/gpu/drm/i915/selftests/intel_scheduler_helpers.c
@@ -32,6 +32,7 @@ int intel_selftest_modify_policy(struct intel_engine_cs *engine,
{
int err;
+ intel_wakeref_t wakeref;
saved->reset = engine->i915->params.reset;
saved->flags = engine->flags;
@@ -66,7 +67,9 @@ int intel_selftest_modify_policy(struct intel_engine_cs *engine,
if (!intel_engine_uses_guc(engine))
return 0;
- err = intel_guc_global_policies_update(&engine->gt->uc.guc);
+ with_intel_runtime_pm(engine->gt->uncore->rpm, wakeref)
+ err = intel_guc_global_policies_update(&engine->gt->uc.guc);
+
if (err)
intel_selftest_restore_policy(engine, saved);
@@ -76,6 +79,9 @@ int intel_selftest_modify_policy(struct intel_engine_cs *engine,
int intel_selftest_restore_policy(struct intel_engine_cs *engine,
struct intel_selftest_saved_policy *saved)
{
+ intel_wakeref_t wakeref;
+ int ret;
+
/* Restore the original policies */
engine->i915->params.reset = saved->reset;
engine->flags = saved->flags;
@@ -85,7 +91,10 @@ int intel_selftest_restore_policy(struct intel_engine_cs *engine,
if (!intel_engine_uses_guc(engine))
return 0;
- return intel_guc_global_policies_update(&engine->gt->uc.guc);
+ with_intel_runtime_pm(engine->gt->uncore->rpm, wakeref)
+ ret = intel_guc_global_policies_update(&engine->gt->uc.guc);
+
+ return ret;
}
int intel_selftest_wait_for_rq(struct i915_request *rq)
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Intel-gfx] [PATCH 02/11] drm/i915: Avoid rpm helpers in intel_guc_slpc_set_media_ratio_mode
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 01/11] drm/i915: Avoid rpm helpers in intel_guc_global_policies_update Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 03/11] drm/i915: Avoid rpm helpers in intel_gt_suspend_late Tilak Tangudu
` (10 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Removed rpm helpers from intel_guc_slpc_set_media_ratio_mode
and added rpm helpers at below high level functions.
-media_freq_factor_store
-intel_guc_slpc_enable via intel_gt_init_hw (already rpm wakeref is hold)
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 2 ++
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 8 +++-----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index ae8a8f725f01..b8f151044780 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -648,6 +648,7 @@ static ssize_t media_freq_factor_store(struct device *dev,
{
struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
struct intel_guc_slpc *slpc = >->uc.guc.slpc;
+ intel_wakeref_t wakeref;
u32 factor, mode;
int err;
@@ -663,6 +664,7 @@ static ssize_t media_freq_factor_store(struct device *dev,
if (mode > SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO)
return -EINVAL;
+ with_intel_runtime_pm(gt->uncore->rpm, wakeref)
err = intel_guc_slpc_set_media_ratio_mode(slpc, mode);
if (!err) {
slpc->media_ratio_mode = mode;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index 2df31af70d63..9a8440378dc2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -510,16 +510,14 @@ int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val)
int intel_guc_slpc_set_media_ratio_mode(struct intel_guc_slpc *slpc, u32 val)
{
struct drm_i915_private *i915 = slpc_to_i915(slpc);
- intel_wakeref_t wakeref;
int ret = 0;
if (!HAS_MEDIA_RATIO_MODE(i915))
return -ENODEV;
- with_intel_runtime_pm(&i915->runtime_pm, wakeref)
- ret = slpc_set_param(slpc,
- SLPC_PARAM_MEDIA_FF_RATIO_MODE,
- val);
+ ret = slpc_set_param(slpc,
+ SLPC_PARAM_MEDIA_FF_RATIO_MODE,
+ val);
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Intel-gfx] [PATCH 03/11] drm/i915: Avoid rpm helpers in intel_gt_suspend_late
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 01/11] drm/i915: Avoid rpm helpers in intel_guc_global_policies_update Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 02/11] drm/i915: Avoid rpm helpers in intel_guc_slpc_set_media_ratio_mode Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper Tilak Tangudu
` (9 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Removed rpm helpers from intel_gt_suspend_late
and added rpm helpers at below high level functions
-__intel_gt_disable
-live_gt_resume
intel_gt_suspend_late is used in i915_gem_suspend_late
and i915_gem_suspend_late need to avoid rpm helpers
so added rpm helpers at higher level functions
-i915_gem_driver_remove
-i915_drm_suspend_late(already holds rpm wakeref)
-do_suspend (already holds rpm wakeref)
Removed rpm helpers from intel_uc_suspend and
acquired rpm wakref from above high functions.
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/gt/intel_gt.c | 4 +++-
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 11 ++++-------
drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 4 +++-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 9 +++------
drivers/gpu/drm/i915/i915_gem.c | 5 ++++-
5 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index be9877c4b496..bb04ec32c54f 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -590,10 +590,12 @@ static int __engines_verify_workarounds(struct intel_gt *gt)
static void __intel_gt_disable(struct intel_gt *gt)
{
+ intel_wakeref_t wakeref = 0;
intel_gt_set_wedged_on_fini(gt);
intel_gt_suspend_prepare(gt);
- intel_gt_suspend_late(gt);
+ with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+ intel_gt_suspend_late(gt);
GEM_BUG_ON(intel_gt_pm_is_awake(gt));
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index f553e2173bda..be99b01a0984 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -326,8 +326,6 @@ static suspend_state_t pm_suspend_target(void)
void intel_gt_suspend_late(struct intel_gt *gt)
{
- intel_wakeref_t wakeref;
-
/* We expect to be idle already; but also want to be independent */
wait_for_suspend(gt);
@@ -352,11 +350,10 @@ void intel_gt_suspend_late(struct intel_gt *gt)
if (pm_suspend_target() == PM_SUSPEND_TO_IDLE)
return;
- with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
- intel_rps_disable(>->rps);
- intel_rc6_disable(>->rc6);
- intel_llc_disable(>->llc);
- }
+ intel_rps_disable(>->rps);
+ intel_rc6_disable(>->rc6);
+ intel_llc_disable(>->llc);
+
gt_sanitize(gt, false);
diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
index be94f863bdef..50f30a5295c4 100644
--- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
@@ -142,13 +142,15 @@ static int live_gt_clocks(void *arg)
static int live_gt_resume(void *arg)
{
struct intel_gt *gt = arg;
+ intel_wakeref_t wakeref = 0;
IGT_TIMEOUT(end_time);
int err;
/* Do several suspend/resume cycles to check we don't explode! */
do {
intel_gt_suspend_prepare(gt);
- intel_gt_suspend_late(gt);
+ with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+ intel_gt_suspend_late(gt);
if (gt->rc6.enabled) {
pr_err("rc6 still enabled after suspend!\n");
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index f2e7c82985ef..425ad2ef1644 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -652,17 +652,14 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
void intel_uc_suspend(struct intel_uc *uc)
{
struct intel_guc *guc = &uc->guc;
- intel_wakeref_t wakeref;
int err;
if (!intel_guc_is_ready(guc))
return;
- with_intel_runtime_pm(&uc_to_gt(uc)->i915->runtime_pm, wakeref) {
- err = intel_guc_suspend(guc);
- if (err)
- DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
- }
+ err = intel_guc_suspend(guc);
+ if (err)
+ DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
}
static int __uc_resume(struct intel_uc *uc, bool enable_communication)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 702e5b89be22..bbe1dac2341c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1173,9 +1173,12 @@ void i915_gem_driver_unregister(struct drm_i915_private *i915)
void i915_gem_driver_remove(struct drm_i915_private *dev_priv)
{
+ intel_wakeref_t wakeref;
+
intel_wakeref_auto_fini(&to_gt(dev_priv)->ggtt->userfault_wakeref);
- i915_gem_suspend_late(dev_priv);
+ with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
+ i915_gem_suspend_late(dev_priv);
intel_gt_driver_remove(to_gt(dev_priv));
dev_priv->uabi_engines = RB_ROOT;
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (2 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 03/11] drm/i915: Avoid rpm helpers in intel_gt_suspend_late Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 14:16 ` Gupta, Anshuman
2022-06-21 12:35 ` [Intel-gfx] [PATCH 05/11] drm/i915: Guard rpm helpers in gt helpers functions Tilak Tangudu
` (8 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Added is_intel_rpm_allowed function to query the runtime_pm
status and disllow during suspending and resuming.
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
drivers/gpu/drm/i915/intel_runtime_pm.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6ed5786bcd29..3759a8596084 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -320,6 +320,21 @@ untrack_all_intel_runtime_pm_wakerefs(struct intel_runtime_pm *rpm)
}
#endif
+static int intel_runtime_pm_status(struct intel_runtime_pm *rpm)
+{
+ return rpm->kdev->power.runtime_status;
+}
+
+bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm)
+{
+ int rpm_status;
+
+ rpm_status = intel_runtime_pm_status(rpm);
+ if (rpm_status == RPM_RESUMING || rpm_status == RPM_SUSPENDING)
+ return false;
+ else
+ return true;
+}
static void
intel_runtime_pm_acquire(struct intel_runtime_pm *rpm, bool wakelock)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.h b/drivers/gpu/drm/i915/intel_runtime_pm.h
index d9160e3ff4af..99418c3a934a 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.h
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
@@ -173,6 +173,7 @@ void intel_runtime_pm_init_early(struct intel_runtime_pm *rpm);
void intel_runtime_pm_enable(struct intel_runtime_pm *rpm);
void intel_runtime_pm_disable(struct intel_runtime_pm *rpm);
void intel_runtime_pm_driver_release(struct intel_runtime_pm *rpm);
+bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm);
intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm);
intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm *rpm);
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
2022-06-21 12:35 ` [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper Tilak Tangudu
@ 2022-06-21 14:16 ` Gupta, Anshuman
2022-06-21 14:22 ` Tangudu, Tilak
0 siblings, 1 reply; 26+ messages in thread
From: Gupta, Anshuman @ 2022-06-21 14:16 UTC (permalink / raw)
To: Tangudu, Tilak, intel-gfx@lists.freedesktop.org, Ewins, Jon,
Vivi, Rodrigo, Belgaumkar, Vinay, Wilson, Chris P,
Dixit, Ashutosh, Nilawar, Badal, Roper, Matthew D,
Gupta, saurabhg, Iddamsetty, Aravind, Sundaresan, Sujaritha
> -----Original Message-----
> From: Tangudu, Tilak <tilak.tangudu@intel.com>
> Sent: Tuesday, June 21, 2022 6:05 PM
> To: intel-gfx@lists.freedesktop.org; Ewins, Jon <jon.ewins@intel.com>; Vivi,
> Rodrigo <rodrigo.vivi@intel.com>; Belgaumkar, Vinay
> <vinay.belgaumkar@intel.com>; Wilson, Chris P <chris.p.wilson@intel.com>;
> Dixit, Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
> <badal.nilawar@intel.com>; Gupta, Anshuman <anshuman.gupta@intel.com>;
> Tangudu, Tilak <tilak.tangudu@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>; Gupta, saurabhg <saurabhg.gupta@intel.com>;
> Iddamsetty, Aravind <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> <sujaritha.sundaresan@intel.com>
> Subject: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
>
> Added is_intel_rpm_allowed function to query the runtime_pm status and
> disllow during suspending and resuming.
This seems a hack,
Not sure if we have better way to handle it.
May be check this in intel_pm_runtime_{get,put} to keep entire code simple ?
>
> Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
> ---
> drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
> drivers/gpu/drm/i915/intel_runtime_pm.h | 1 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6ed5786bcd29..3759a8596084 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -320,6 +320,21 @@ untrack_all_intel_runtime_pm_wakerefs(struct
> intel_runtime_pm *rpm) }
>
> #endif
> +static int intel_runtime_pm_status(struct intel_runtime_pm *rpm) {
> + return rpm->kdev->power.runtime_status; }
This is racy in principal, we need a kdev->power lock here.
Regards,
Anshuman Gupta.
> +
> +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm) {
> + int rpm_status;
> +
> + rpm_status = intel_runtime_pm_status(rpm);
> + if (rpm_status == RPM_RESUMING || rpm_status ==
> RPM_SUSPENDING)
> + return false;
> + else
> + return true;
> +}
>
> static void
> intel_runtime_pm_acquire(struct intel_runtime_pm *rpm, bool wakelock) diff --
> git a/drivers/gpu/drm/i915/intel_runtime_pm.h
> b/drivers/gpu/drm/i915/intel_runtime_pm.h
> index d9160e3ff4af..99418c3a934a 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.h
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
> @@ -173,6 +173,7 @@ void intel_runtime_pm_init_early(struct
> intel_runtime_pm *rpm); void intel_runtime_pm_enable(struct
> intel_runtime_pm *rpm); void intel_runtime_pm_disable(struct
> intel_runtime_pm *rpm); void intel_runtime_pm_driver_release(struct
> intel_runtime_pm *rpm);
> +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm);
>
> intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm);
> intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm
> *rpm);
> --
> 2.25.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
2022-06-21 14:16 ` Gupta, Anshuman
@ 2022-06-21 14:22 ` Tangudu, Tilak
2022-06-22 12:55 ` Jani Nikula
0 siblings, 1 reply; 26+ messages in thread
From: Tangudu, Tilak @ 2022-06-21 14:22 UTC (permalink / raw)
To: Gupta, Anshuman, intel-gfx@lists.freedesktop.org, Ewins, Jon,
Vivi, Rodrigo, Belgaumkar, Vinay, Wilson, Chris P,
Dixit, Ashutosh, Nilawar, Badal, Roper, Matthew D,
Gupta, saurabhg, Iddamsetty, Aravind, Sundaresan, Sujaritha
> -----Original Message-----
> From: Gupta, Anshuman <anshuman.gupta@intel.com>
> Sent: Tuesday, June 21, 2022 7:47 PM
> To: Tangudu, Tilak <tilak.tangudu@intel.com>; intel-gfx@lists.freedesktop.org;
> Ewins, Jon <jon.ewins@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> Belgaumkar, Vinay <vinay.belgaumkar@intel.com>; Wilson, Chris P
> <chris.p.wilson@intel.com>; Dixit, Ashutosh <ashutosh.dixit@intel.com>;
> Nilawar, Badal <badal.nilawar@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>; Gupta, saurabhg
> <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
> <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> <sujaritha.sundaresan@intel.com>
> Subject: RE: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
>
>
>
> > -----Original Message-----
> > From: Tangudu, Tilak <tilak.tangudu@intel.com>
> > Sent: Tuesday, June 21, 2022 6:05 PM
> > To: intel-gfx@lists.freedesktop.org; Ewins, Jon <jon.ewins@intel.com>;
> > Vivi, Rodrigo <rodrigo.vivi@intel.com>; Belgaumkar, Vinay
> > <vinay.belgaumkar@intel.com>; Wilson, Chris P
> > <chris.p.wilson@intel.com>; Dixit, Ashutosh
> > <ashutosh.dixit@intel.com>; Nilawar, Badal <badal.nilawar@intel.com>;
> > Gupta, Anshuman <anshuman.gupta@intel.com>; Tangudu, Tilak
> > <tilak.tangudu@intel.com>; Roper, Matthew D
> > <matthew.d.roper@intel.com>; Gupta, saurabhg
> > <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
> > <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> > <sujaritha.sundaresan@intel.com>
> > Subject: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
> >
> > Added is_intel_rpm_allowed function to query the runtime_pm status and
> > disllow during suspending and resuming.
> This seems a hack,
> Not sure if we have better way to handle it.
> May be check this in intel_pm_runtime_{get,put} to keep entire code simple ?
Yes, that would be simple without code refactoring.
Checked the same with Chris, he suggested unbalancing of wakeref might popup
If used at intel_pm_runtime_{get,put} . So used like this,
@Wilson, Chris P , Please comment .
@Vivi, Rodrigo , Any suggestion ?
> >
> > Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
> > drivers/gpu/drm/i915/intel_runtime_pm.h | 1 +
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 6ed5786bcd29..3759a8596084 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -320,6 +320,21 @@ untrack_all_intel_runtime_pm_wakerefs(struct
> > intel_runtime_pm *rpm) }
> >
> > #endif
> > +static int intel_runtime_pm_status(struct intel_runtime_pm *rpm) {
> > +return rpm->kdev->power.runtime_status; }
> This is racy in principal, we need a kdev->power lock here.
> Regards,
> Anshuman Gupta.
> > +
> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm) { int
> > +rpm_status;
> > +
> > +rpm_status = intel_runtime_pm_status(rpm); if (rpm_status ==
> > +RPM_RESUMING || rpm_status ==
> > RPM_SUSPENDING)
> > +return false;
> > +else
> > +return true;
> > +}
> >
> > static void
> > intel_runtime_pm_acquire(struct intel_runtime_pm *rpm, bool wakelock)
> > diff -- git a/drivers/gpu/drm/i915/intel_runtime_pm.h
> > b/drivers/gpu/drm/i915/intel_runtime_pm.h
> > index d9160e3ff4af..99418c3a934a 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.h
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
> > @@ -173,6 +173,7 @@ void intel_runtime_pm_init_early(struct
> > intel_runtime_pm *rpm); void intel_runtime_pm_enable(struct
> > intel_runtime_pm *rpm); void intel_runtime_pm_disable(struct
> > intel_runtime_pm *rpm); void intel_runtime_pm_driver_release(struct
> > intel_runtime_pm *rpm);
> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm);
> >
> > intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm);
> > intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm
> > *rpm);
> > --
> > 2.25.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
2022-06-21 14:22 ` Tangudu, Tilak
@ 2022-06-22 12:55 ` Jani Nikula
2022-06-22 20:40 ` Rodrigo Vivi
0 siblings, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2022-06-22 12:55 UTC (permalink / raw)
To: Tangudu, Tilak, Gupta, Anshuman, intel-gfx@lists.freedesktop.org,
Ewins, Jon, Vivi, Rodrigo, Belgaumkar, Vinay, Wilson, Chris P,
Dixit, Ashutosh, Nilawar, Badal, Roper, Matthew D,
Gupta, saurabhg, Iddamsetty, Aravind, Sundaresan, Sujaritha
On Tue, 21 Jun 2022, "Tangudu, Tilak" <tilak.tangudu@intel.com> wrote:
>> -----Original Message-----
>> From: Gupta, Anshuman <anshuman.gupta@intel.com>
>> Sent: Tuesday, June 21, 2022 7:47 PM
>> To: Tangudu, Tilak <tilak.tangudu@intel.com>; intel-gfx@lists.freedesktop.org;
>> Ewins, Jon <jon.ewins@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
>> Belgaumkar, Vinay <vinay.belgaumkar@intel.com>; Wilson, Chris P
>> <chris.p.wilson@intel.com>; Dixit, Ashutosh <ashutosh.dixit@intel.com>;
>> Nilawar, Badal <badal.nilawar@intel.com>; Roper, Matthew D
>> <matthew.d.roper@intel.com>; Gupta, saurabhg
>> <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
>> <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
>> <sujaritha.sundaresan@intel.com>
>> Subject: RE: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
>>
>>
>>
>> > -----Original Message-----
>> > From: Tangudu, Tilak <tilak.tangudu@intel.com>
>> > Sent: Tuesday, June 21, 2022 6:05 PM
>> > To: intel-gfx@lists.freedesktop.org; Ewins, Jon <jon.ewins@intel.com>;
>> > Vivi, Rodrigo <rodrigo.vivi@intel.com>; Belgaumkar, Vinay
>> > <vinay.belgaumkar@intel.com>; Wilson, Chris P
>> > <chris.p.wilson@intel.com>; Dixit, Ashutosh
>> > <ashutosh.dixit@intel.com>; Nilawar, Badal <badal.nilawar@intel.com>;
>> > Gupta, Anshuman <anshuman.gupta@intel.com>; Tangudu, Tilak
>> > <tilak.tangudu@intel.com>; Roper, Matthew D
>> > <matthew.d.roper@intel.com>; Gupta, saurabhg
>> > <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
>> > <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
>> > <sujaritha.sundaresan@intel.com>
>> > Subject: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
>> >
>> > Added is_intel_rpm_allowed function to query the runtime_pm status and
>> > disllow during suspending and resuming.
>> This seems a hack,
>> Not sure if we have better way to handle it.
>> May be check this in intel_pm_runtime_{get,put} to keep entire code simple ?
> Yes, that would be simple without code refactoring.
> Checked the same with Chris, he suggested unbalancing of wakeref might popup
> If used at intel_pm_runtime_{get,put} . So used like this,
> @Wilson, Chris P , Please comment .
> @Vivi, Rodrigo , Any suggestion ?
One option would be to track this in intel_wakeref_t, i.e. _get flags
the case in the returned wakeref and _put skips in that case.
BR,
Jani.
>
>> >
>> > Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
>> > drivers/gpu/drm/i915/intel_runtime_pm.h | 1 +
>> > 2 files changed, 16 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > index 6ed5786bcd29..3759a8596084 100644
>> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > @@ -320,6 +320,21 @@ untrack_all_intel_runtime_pm_wakerefs(struct
>> > intel_runtime_pm *rpm) }
>> >
>> > #endif
>> > +static int intel_runtime_pm_status(struct intel_runtime_pm *rpm) {
>> > +return rpm->kdev->power.runtime_status; }
>> This is racy in principal, we need a kdev->power lock here.
>> Regards,
>> Anshuman Gupta.
>> > +
>> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm) { int
>> > +rpm_status;
>> > +
>> > +rpm_status = intel_runtime_pm_status(rpm); if (rpm_status ==
>> > +RPM_RESUMING || rpm_status ==
>> > RPM_SUSPENDING)
>> > +return false;
>> > +else
>> > +return true;
>> > +}
>> >
>> > static void
>> > intel_runtime_pm_acquire(struct intel_runtime_pm *rpm, bool wakelock)
>> > diff -- git a/drivers/gpu/drm/i915/intel_runtime_pm.h
>> > b/drivers/gpu/drm/i915/intel_runtime_pm.h
>> > index d9160e3ff4af..99418c3a934a 100644
>> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.h
>> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
>> > @@ -173,6 +173,7 @@ void intel_runtime_pm_init_early(struct
>> > intel_runtime_pm *rpm); void intel_runtime_pm_enable(struct
>> > intel_runtime_pm *rpm); void intel_runtime_pm_disable(struct
>> > intel_runtime_pm *rpm); void intel_runtime_pm_driver_release(struct
>> > intel_runtime_pm *rpm);
>> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm);
>> >
>> > intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm);
>> > intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm
>> > *rpm);
>> > --
>> > 2.25.1
>>
>
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
2022-06-22 12:55 ` Jani Nikula
@ 2022-06-22 20:40 ` Rodrigo Vivi
2022-06-23 17:21 ` Tangudu, Tilak
0 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2022-06-22 20:40 UTC (permalink / raw)
To: Jani Nikula
Cc: intel-gfx@lists.freedesktop.org, Wilson, Chris P, Gupta, saurabhg
On Wed, Jun 22, 2022 at 03:55:03PM +0300, Jani Nikula wrote:
> On Tue, 21 Jun 2022, "Tangudu, Tilak" <tilak.tangudu@intel.com> wrote:
> >> -----Original Message-----
> >> From: Gupta, Anshuman <anshuman.gupta@intel.com>
> >> Sent: Tuesday, June 21, 2022 7:47 PM
> >> To: Tangudu, Tilak <tilak.tangudu@intel.com>; intel-gfx@lists.freedesktop.org;
> >> Ewins, Jon <jon.ewins@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> >> Belgaumkar, Vinay <vinay.belgaumkar@intel.com>; Wilson, Chris P
> >> <chris.p.wilson@intel.com>; Dixit, Ashutosh <ashutosh.dixit@intel.com>;
> >> Nilawar, Badal <badal.nilawar@intel.com>; Roper, Matthew D
> >> <matthew.d.roper@intel.com>; Gupta, saurabhg
> >> <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
> >> <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> >> <sujaritha.sundaresan@intel.com>
> >> Subject: RE: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
> >>
> >>
> >>
> >> > -----Original Message-----
> >> > From: Tangudu, Tilak <tilak.tangudu@intel.com>
> >> > Sent: Tuesday, June 21, 2022 6:05 PM
> >> > To: intel-gfx@lists.freedesktop.org; Ewins, Jon <jon.ewins@intel.com>;
> >> > Vivi, Rodrigo <rodrigo.vivi@intel.com>; Belgaumkar, Vinay
> >> > <vinay.belgaumkar@intel.com>; Wilson, Chris P
> >> > <chris.p.wilson@intel.com>; Dixit, Ashutosh
> >> > <ashutosh.dixit@intel.com>; Nilawar, Badal <badal.nilawar@intel.com>;
> >> > Gupta, Anshuman <anshuman.gupta@intel.com>; Tangudu, Tilak
> >> > <tilak.tangudu@intel.com>; Roper, Matthew D
> >> > <matthew.d.roper@intel.com>; Gupta, saurabhg
> >> > <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
> >> > <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> >> > <sujaritha.sundaresan@intel.com>
> >> > Subject: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
> >> >
> >> > Added is_intel_rpm_allowed function to query the runtime_pm status and
> >> > disllow during suspending and resuming.
> >> This seems a hack,
> >> Not sure if we have better way to handle it.
> >> May be check this in intel_pm_runtime_{get,put} to keep entire code simple ?
> > Yes, that would be simple without code refactoring.
> > Checked the same with Chris, he suggested unbalancing of wakeref might popup
> > If used at intel_pm_runtime_{get,put} . So used like this,
> > @Wilson, Chris P , Please comment .
> > @Vivi, Rodrigo , Any suggestion ?
>
> One option would be to track this in intel_wakeref_t, i.e. _get flags
> the case in the returned wakeref and _put skips in that case.
yeap, this seems to be the quick path at this moment...
Imre, do you see any other quick option?
In general I don't like much the big wakeref infra we end up
creating here because all of the historical unbalanced cases we got.
We should be able to get something cleaner and use the rpm infra as
other drivers are using, or improve in the rpm side itself whatever
we feel that we are missing to deal with these cases.
But back to this specific case/usage here we might need to duplicate
some functions to be called just from the inside the resuming/suspending
path... and/or moving the gets & puts upper on the stack...
The quick hacks will solve our short term problems and continue bloating
our rpm infra.
>
> BR,
> Jani.
>
>
> >
> >> >
> >> > Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
> >> > ---
> >> > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
> >> > drivers/gpu/drm/i915/intel_runtime_pm.h | 1 +
> >> > 2 files changed, 16 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> > index 6ed5786bcd29..3759a8596084 100644
> >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> > @@ -320,6 +320,21 @@ untrack_all_intel_runtime_pm_wakerefs(struct
> >> > intel_runtime_pm *rpm) }
> >> >
> >> > #endif
> >> > +static int intel_runtime_pm_status(struct intel_runtime_pm *rpm) {
> >> > +return rpm->kdev->power.runtime_status; }
> >> This is racy in principal, we need a kdev->power lock here.
> >> Regards,
> >> Anshuman Gupta.
> >> > +
> >> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm) { int
> >> > +rpm_status;
> >> > +
> >> > +rpm_status = intel_runtime_pm_status(rpm); if (rpm_status ==
> >> > +RPM_RESUMING || rpm_status ==
> >> > RPM_SUSPENDING)
> >> > +return false;
> >> > +else
> >> > +return true;
> >> > +}
> >> >
> >> > static void
> >> > intel_runtime_pm_acquire(struct intel_runtime_pm *rpm, bool wakelock)
> >> > diff -- git a/drivers/gpu/drm/i915/intel_runtime_pm.h
> >> > b/drivers/gpu/drm/i915/intel_runtime_pm.h
> >> > index d9160e3ff4af..99418c3a934a 100644
> >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.h
> >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
> >> > @@ -173,6 +173,7 @@ void intel_runtime_pm_init_early(struct
> >> > intel_runtime_pm *rpm); void intel_runtime_pm_enable(struct
> >> > intel_runtime_pm *rpm); void intel_runtime_pm_disable(struct
> >> > intel_runtime_pm *rpm); void intel_runtime_pm_driver_release(struct
> >> > intel_runtime_pm *rpm);
> >> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm);
> >> >
> >> > intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm);
> >> > intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm
> >> > *rpm);
> >> > --
> >> > 2.25.1
> >>
> >
>
> --
> Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
2022-06-22 20:40 ` Rodrigo Vivi
@ 2022-06-23 17:21 ` Tangudu, Tilak
2022-06-23 17:49 ` Jani Nikula
0 siblings, 1 reply; 26+ messages in thread
From: Tangudu, Tilak @ 2022-06-23 17:21 UTC (permalink / raw)
To: Vivi, Rodrigo, Jani Nikula
Cc: intel-gfx@lists.freedesktop.org, Wilson, Chris P, Gupta, saurabhg
> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Thursday, June 23, 2022 2:11 AM
> To: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Tangudu, Tilak <tilak.tangudu@intel.com>; Gupta, Anshuman
> <anshuman.gupta@intel.com>; intel-gfx@lists.freedesktop.org; Ewins, Jon
> <jon.ewins@intel.com>; Belgaumkar, Vinay <vinay.belgaumkar@intel.com>;
> Wilson, Chris P <chris.p.wilson@intel.com>; Dixit, Ashutosh
> <ashutosh.dixit@intel.com>; Nilawar, Badal <badal.nilawar@intel.com>;
> Roper, Matthew D <matthew.d.roper@intel.com>; Gupta, saurabhg
> <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
> <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> <sujaritha.sundaresan@intel.com>; Deak, Imre <imre.deak@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed
> helper
>
> On Wed, Jun 22, 2022 at 03:55:03PM +0300, Jani Nikula wrote:
> > On Tue, 21 Jun 2022, "Tangudu, Tilak" <tilak.tangudu@intel.com> wrote:
> > >> -----Original Message-----
> > >> From: Gupta, Anshuman <anshuman.gupta@intel.com>
> > >> Sent: Tuesday, June 21, 2022 7:47 PM
> > >> To: Tangudu, Tilak <tilak.tangudu@intel.com>;
> > >> intel-gfx@lists.freedesktop.org; Ewins, Jon <jon.ewins@intel.com>;
> > >> Vivi, Rodrigo <rodrigo.vivi@intel.com>; Belgaumkar, Vinay
> > >> <vinay.belgaumkar@intel.com>; Wilson, Chris P
> > >> <chris.p.wilson@intel.com>; Dixit, Ashutosh
> > >> <ashutosh.dixit@intel.com>; Nilawar, Badal
> > >> <badal.nilawar@intel.com>; Roper, Matthew D
> > >> <matthew.d.roper@intel.com>; Gupta, saurabhg
> > >> <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
> > >> <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> > >> <sujaritha.sundaresan@intel.com>
> > >> Subject: RE: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed
> > >> helper
> > >>
> > >>
> > >>
> > >> > -----Original Message-----
> > >> > From: Tangudu, Tilak <tilak.tangudu@intel.com>
> > >> > Sent: Tuesday, June 21, 2022 6:05 PM
> > >> > To: intel-gfx@lists.freedesktop.org; Ewins, Jon
> > >> > <jon.ewins@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> > >> > Belgaumkar, Vinay <vinay.belgaumkar@intel.com>; Wilson, Chris P
> > >> > <chris.p.wilson@intel.com>; Dixit, Ashutosh
> > >> > <ashutosh.dixit@intel.com>; Nilawar, Badal
> > >> > <badal.nilawar@intel.com>; Gupta, Anshuman
> > >> > <anshuman.gupta@intel.com>; Tangudu, Tilak
> > >> > <tilak.tangudu@intel.com>; Roper, Matthew D
> > >> > <matthew.d.roper@intel.com>; Gupta, saurabhg
> > >> > <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
> > >> > <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> > >> > <sujaritha.sundaresan@intel.com>
> > >> > Subject: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed
> > >> > helper
> > >> >
> > >> > Added is_intel_rpm_allowed function to query the runtime_pm
> > >> > status and disllow during suspending and resuming.
> > >> This seems a hack,
> > >> Not sure if we have better way to handle it.
> > >> May be check this in intel_pm_runtime_{get,put} to keep entire code
> simple ?
> > > Yes, that would be simple without code refactoring.
> > > Checked the same with Chris, he suggested unbalancing of wakeref
> > > might popup If used at intel_pm_runtime_{get,put} . So used like
> > > this, @Wilson, Chris P , Please comment .
> > > @Vivi, Rodrigo , Any suggestion ?
> >
> > One option would be to track this in intel_wakeref_t, i.e. _get flags
> > the case in the returned wakeref and _put skips in that case.
@Jani Nikula
I did not understand the suggestion, Can you please elaborate ?
Did you mean below or something more ? please help clarify.
8< ------------------------------
linux-desk:~/Code/drm-tip$ git diff
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 3759a8596084..ce272c569a89 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -369,12 +369,16 @@ static intel_wakeref_t __intel_runtime_pm_get(struct intel_runtime_pm *rpm,
runtime_pm);
int ret;
+ if (!is_intel_rpm_allowed(rpm))
+ goto out:
+
ret = pm_runtime_get_sync(rpm->kdev);
drm_WARN_ONCE(&i915->drm, ret < 0,
"pm_runtime_get_sync() failed: %d\n", ret);
intel_runtime_pm_acquire(rpm, wakelock);
+out:
return track_intel_runtime_pm_wakeref(rpm);
}
@@ -505,6 +509,9 @@ static void __intel_runtime_pm_put(struct intel_runtime_pm *rpm,
untrack_intel_runtime_pm_wakeref(rpm, wref);
+ if (!is_intel_rpm_allowed(rpm))
+ return;
+
intel_runtime_pm_release(rpm, wakelock);
pm_runtime_mark_last_busy(kdev);
---------------------------------------------------------- >8
Thanks
Tilak
>
> yeap, this seems to be the quick path at this moment...
>
> Imre, do you see any other quick option?
>
> In general I don't like much the big wakeref infra we end up creating here
> because all of the historical unbalanced cases we got.
> We should be able to get something cleaner and use the rpm infra as other
> drivers are using, or improve in the rpm side itself whatever we feel that we
> are missing to deal with these cases.
>
> But back to this specific case/usage here we might need to duplicate some
> functions to be called just from the inside the resuming/suspending path...
> and/or moving the gets & puts upper on the stack...
>
> The quick hacks will solve our short term problems and continue bloating our
> rpm infra.
>
> >
> > BR,
> > Jani.
> >
> >
> > >
> > >> >
> > >> > Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
> > >> > ---
> > >> > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
> > >> > drivers/gpu/drm/i915/intel_runtime_pm.h | 1 +
> > >> > 2 files changed, 16 insertions(+)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > >> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > >> > index 6ed5786bcd29..3759a8596084 100644
> > >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > >> > @@ -320,6 +320,21 @@ untrack_all_intel_runtime_pm_wakerefs(struct
> > >> > intel_runtime_pm *rpm) }
> > >> >
> > >> > #endif
> > >> > +static int intel_runtime_pm_status(struct intel_runtime_pm *rpm)
> > >> > +{ return rpm->kdev->power.runtime_status; }
> > >> This is racy in principal, we need a kdev->power lock here.
> > >> Regards,
> > >> Anshuman Gupta.
> > >> > +
> > >> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm) { int
> > >> > +rpm_status;
> > >> > +
> > >> > +rpm_status = intel_runtime_pm_status(rpm); if (rpm_status ==
> > >> > +RPM_RESUMING || rpm_status ==
> > >> > RPM_SUSPENDING)
> > >> > +return false;
> > >> > +else
> > >> > +return true;
> > >> > +}
> > >> >
> > >> > static void
> > >> > intel_runtime_pm_acquire(struct intel_runtime_pm *rpm, bool
> > >> > wakelock) diff -- git a/drivers/gpu/drm/i915/intel_runtime_pm.h
> > >> > b/drivers/gpu/drm/i915/intel_runtime_pm.h
> > >> > index d9160e3ff4af..99418c3a934a 100644
> > >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.h
> > >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
> > >> > @@ -173,6 +173,7 @@ void intel_runtime_pm_init_early(struct
> > >> > intel_runtime_pm *rpm); void intel_runtime_pm_enable(struct
> > >> > intel_runtime_pm *rpm); void intel_runtime_pm_disable(struct
> > >> > intel_runtime_pm *rpm); void
> > >> > intel_runtime_pm_driver_release(struct
> > >> > intel_runtime_pm *rpm);
> > >> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm);
> > >> >
> > >> > intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm
> > >> > *rpm); intel_wakeref_t intel_runtime_pm_get_if_in_use(struct
> > >> > intel_runtime_pm *rpm);
> > >> > --
> > >> > 2.25.1
> > >>
> > >
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper
2022-06-23 17:21 ` Tangudu, Tilak
@ 2022-06-23 17:49 ` Jani Nikula
0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2022-06-23 17:49 UTC (permalink / raw)
To: Tangudu, Tilak, Vivi, Rodrigo
Cc: intel-gfx@lists.freedesktop.org, Wilson, Chris P, Gupta, saurabhg
On Thu, 23 Jun 2022, "Tangudu, Tilak" <tilak.tangudu@intel.com> wrote:
>> -----Original Message-----
>> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Sent: Thursday, June 23, 2022 2:11 AM
>> To: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Tangudu, Tilak <tilak.tangudu@intel.com>; Gupta, Anshuman
>> <anshuman.gupta@intel.com>; intel-gfx@lists.freedesktop.org; Ewins, Jon
>> <jon.ewins@intel.com>; Belgaumkar, Vinay <vinay.belgaumkar@intel.com>;
>> Wilson, Chris P <chris.p.wilson@intel.com>; Dixit, Ashutosh
>> <ashutosh.dixit@intel.com>; Nilawar, Badal <badal.nilawar@intel.com>;
>> Roper, Matthew D <matthew.d.roper@intel.com>; Gupta, saurabhg
>> <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
>> <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
>> <sujaritha.sundaresan@intel.com>; Deak, Imre <imre.deak@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed
>> helper
>>
>> On Wed, Jun 22, 2022 at 03:55:03PM +0300, Jani Nikula wrote:
>> > On Tue, 21 Jun 2022, "Tangudu, Tilak" <tilak.tangudu@intel.com> wrote:
>> > >> -----Original Message-----
>> > >> From: Gupta, Anshuman <anshuman.gupta@intel.com>
>> > >> Sent: Tuesday, June 21, 2022 7:47 PM
>> > >> To: Tangudu, Tilak <tilak.tangudu@intel.com>;
>> > >> intel-gfx@lists.freedesktop.org; Ewins, Jon <jon.ewins@intel.com>;
>> > >> Vivi, Rodrigo <rodrigo.vivi@intel.com>; Belgaumkar, Vinay
>> > >> <vinay.belgaumkar@intel.com>; Wilson, Chris P
>> > >> <chris.p.wilson@intel.com>; Dixit, Ashutosh
>> > >> <ashutosh.dixit@intel.com>; Nilawar, Badal
>> > >> <badal.nilawar@intel.com>; Roper, Matthew D
>> > >> <matthew.d.roper@intel.com>; Gupta, saurabhg
>> > >> <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
>> > >> <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
>> > >> <sujaritha.sundaresan@intel.com>
>> > >> Subject: RE: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed
>> > >> helper
>> > >>
>> > >>
>> > >>
>> > >> > -----Original Message-----
>> > >> > From: Tangudu, Tilak <tilak.tangudu@intel.com>
>> > >> > Sent: Tuesday, June 21, 2022 6:05 PM
>> > >> > To: intel-gfx@lists.freedesktop.org; Ewins, Jon
>> > >> > <jon.ewins@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
>> > >> > Belgaumkar, Vinay <vinay.belgaumkar@intel.com>; Wilson, Chris P
>> > >> > <chris.p.wilson@intel.com>; Dixit, Ashutosh
>> > >> > <ashutosh.dixit@intel.com>; Nilawar, Badal
>> > >> > <badal.nilawar@intel.com>; Gupta, Anshuman
>> > >> > <anshuman.gupta@intel.com>; Tangudu, Tilak
>> > >> > <tilak.tangudu@intel.com>; Roper, Matthew D
>> > >> > <matthew.d.roper@intel.com>; Gupta, saurabhg
>> > >> > <saurabhg.gupta@intel.com>; Iddamsetty, Aravind
>> > >> > <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
>> > >> > <sujaritha.sundaresan@intel.com>
>> > >> > Subject: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed
>> > >> > helper
>> > >> >
>> > >> > Added is_intel_rpm_allowed function to query the runtime_pm
>> > >> > status and disllow during suspending and resuming.
>> > >> This seems a hack,
>> > >> Not sure if we have better way to handle it.
>> > >> May be check this in intel_pm_runtime_{get,put} to keep entire code
>> simple ?
>> > > Yes, that would be simple without code refactoring.
>> > > Checked the same with Chris, he suggested unbalancing of wakeref
>> > > might popup If used at intel_pm_runtime_{get,put} . So used like
>> > > this, @Wilson, Chris P , Please comment .
>> > > @Vivi, Rodrigo , Any suggestion ?
>> >
>> > One option would be to track this in intel_wakeref_t, i.e. _get flags
>> > the case in the returned wakeref and _put skips in that case.
>
> @Jani Nikula
>
> I did not understand the suggestion, Can you please elaborate ?
> Did you mean below or something more ? please help clarify.
The code below will lead to get/put inbalance if is_intel_rpm_allowed()
status changes between the get/put calls. I don't know how likely that
is, but if it happens it's nasty.
intel_wakeref_t is depot_stack_handle_t, which is actually just u32. We
already abuse -1 value to not track wakeref (when
CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n or track_intel_runtime_pm_wakeref()
fails.
It's a bit of a hack, but we could have __intel_runtime_pm_get() early
return -2 as the wakeref when !is_intel_rpm_allowed(), and
intel_runtime_pm_put() (both versions for both kconfig option values!)
ignore the put when the passed in wakeref == -2.
This requires no changes in the calling code anywhere, even though the
implementation is a hack. A pedantically correct implementation would
turn intel_wakeref_t into a struct that wraps depot_stack_handle_t
inside, and has a separate field for validity, but that probably has a
non-trivial code size penalty.
BR,
Jani.
>
> 8< ------------------------------
> linux-desk:~/Code/drm-tip$ git diff
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 3759a8596084..ce272c569a89 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -369,12 +369,16 @@ static intel_wakeref_t __intel_runtime_pm_get(struct intel_runtime_pm *rpm,
> runtime_pm);
> int ret;
>
> + if (!is_intel_rpm_allowed(rpm))
> + goto out:
> +
> ret = pm_runtime_get_sync(rpm->kdev);
> drm_WARN_ONCE(&i915->drm, ret < 0,
> "pm_runtime_get_sync() failed: %d\n", ret);
>
> intel_runtime_pm_acquire(rpm, wakelock);
>
> +out:
> return track_intel_runtime_pm_wakeref(rpm);
> }
>
> @@ -505,6 +509,9 @@ static void __intel_runtime_pm_put(struct intel_runtime_pm *rpm,
>
> untrack_intel_runtime_pm_wakeref(rpm, wref);
>
> + if (!is_intel_rpm_allowed(rpm))
> + return;
> +
> intel_runtime_pm_release(rpm, wakelock);
>
> pm_runtime_mark_last_busy(kdev);
> ---------------------------------------------------------- >8
>
> Thanks
> Tilak
>>
>> yeap, this seems to be the quick path at this moment...
>>
>> Imre, do you see any other quick option?
>>
>> In general I don't like much the big wakeref infra we end up creating here
>> because all of the historical unbalanced cases we got.
>> We should be able to get something cleaner and use the rpm infra as other
>> drivers are using, or improve in the rpm side itself whatever we feel that we
>> are missing to deal with these cases.
>>
>> But back to this specific case/usage here we might need to duplicate some
>> functions to be called just from the inside the resuming/suspending path...
>> and/or moving the gets & puts upper on the stack...
>>
>> The quick hacks will solve our short term problems and continue bloating our
>> rpm infra.
>>
>> >
>> > BR,
>> > Jani.
>> >
>> >
>> > >
>> > >> >
>> > >> > Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
>> > >> > ---
>> > >> > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
>> > >> > drivers/gpu/drm/i915/intel_runtime_pm.h | 1 +
>> > >> > 2 files changed, 16 insertions(+)
>> > >> >
>> > >> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > >> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > >> > index 6ed5786bcd29..3759a8596084 100644
>> > >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > >> > @@ -320,6 +320,21 @@ untrack_all_intel_runtime_pm_wakerefs(struct
>> > >> > intel_runtime_pm *rpm) }
>> > >> >
>> > >> > #endif
>> > >> > +static int intel_runtime_pm_status(struct intel_runtime_pm *rpm)
>> > >> > +{ return rpm->kdev->power.runtime_status; }
>> > >> This is racy in principal, we need a kdev->power lock here.
>> > >> Regards,
>> > >> Anshuman Gupta.
>> > >> > +
>> > >> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm) { int
>> > >> > +rpm_status;
>> > >> > +
>> > >> > +rpm_status = intel_runtime_pm_status(rpm); if (rpm_status ==
>> > >> > +RPM_RESUMING || rpm_status ==
>> > >> > RPM_SUSPENDING)
>> > >> > +return false;
>> > >> > +else
>> > >> > +return true;
>> > >> > +}
>> > >> >
>> > >> > static void
>> > >> > intel_runtime_pm_acquire(struct intel_runtime_pm *rpm, bool
>> > >> > wakelock) diff -- git a/drivers/gpu/drm/i915/intel_runtime_pm.h
>> > >> > b/drivers/gpu/drm/i915/intel_runtime_pm.h
>> > >> > index d9160e3ff4af..99418c3a934a 100644
>> > >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.h
>> > >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
>> > >> > @@ -173,6 +173,7 @@ void intel_runtime_pm_init_early(struct
>> > >> > intel_runtime_pm *rpm); void intel_runtime_pm_enable(struct
>> > >> > intel_runtime_pm *rpm); void intel_runtime_pm_disable(struct
>> > >> > intel_runtime_pm *rpm); void
>> > >> > intel_runtime_pm_driver_release(struct
>> > >> > intel_runtime_pm *rpm);
>> > >> > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm);
>> > >> >
>> > >> > intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm
>> > >> > *rpm); intel_wakeref_t intel_runtime_pm_get_if_in_use(struct
>> > >> > intel_runtime_pm *rpm);
>> > >> > --
>> > >> > 2.25.1
>> > >>
>> > >
>> >
>> > --
>> > Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Intel-gfx] [PATCH 05/11] drm/i915: Guard rpm helpers in gt helpers functions
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (3 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-22 12:52 ` Jani Nikula
2022-06-21 12:35 ` [Intel-gfx] [PATCH 06/11] drm/i915: Avoid rpm helpers in try_context_registration Tilak Tangudu
` (7 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Guard rpm helpers in gt_sanitize and intel_gt_set_wedged
with is_intel_rpm_allowed
Acquire rpm wakeref for higherlevel function i915_gem_resume
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 12 ++++++------
drivers/gpu/drm/i915/gt/intel_reset.c | 10 +++++++---
drivers/gpu/drm/i915/i915_driver.c | 4 +++-
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index be99b01a0984..9857b91194b7 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -163,12 +163,14 @@ static void gt_sanitize(struct intel_gt *gt, bool force)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
- intel_wakeref_t wakeref;
+ intel_wakeref_t wakeref = 0;
GT_TRACE(gt, "force:%s", str_yes_no(force));
/* Use a raw wakeref to avoid calling intel_display_power_get early */
- wakeref = intel_runtime_pm_get(gt->uncore->rpm);
+ if (is_intel_rpm_allowed(gt->uncore->rpm))
+ wakeref = intel_runtime_pm_get(gt->uncore->rpm);
+
intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
intel_gt_check_clock_frequency(gt);
@@ -207,7 +209,8 @@ static void gt_sanitize(struct intel_gt *gt, bool force)
intel_rps_sanitize(>->rps);
intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
- intel_runtime_pm_put(gt->uncore->rpm, wakeref);
+ if (wakeref)
+ intel_runtime_pm_put(gt->uncore->rpm, wakeref);
}
void intel_gt_pm_fini(struct intel_gt *gt)
@@ -226,7 +229,6 @@ int intel_gt_resume(struct intel_gt *gt)
return err;
GT_TRACE(gt, "\n");
-
/*
* After resume, we may need to poke into the pinned kernel
* contexts to paper over any damage caused by the sudden suspend.
@@ -259,10 +261,8 @@ int intel_gt_resume(struct intel_gt *gt)
for_each_engine(engine, gt, id) {
intel_engine_pm_get(engine);
-
engine->serial++; /* kernel context lost */
err = intel_engine_resume(engine);
-
intel_engine_pm_put(engine);
if (err) {
drm_err(>->i915->drm,
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index c8e05b48c14f..55a1fd38c7c4 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -901,12 +901,14 @@ static void __intel_gt_set_wedged(struct intel_gt *gt)
void intel_gt_set_wedged(struct intel_gt *gt)
{
- intel_wakeref_t wakeref;
+ intel_wakeref_t wakeref = 0;
if (test_bit(I915_WEDGED, >->reset.flags))
return;
- wakeref = intel_runtime_pm_get(gt->uncore->rpm);
+ if (is_intel_rpm_allowed(gt->uncore->rpm))
+ wakeref = intel_runtime_pm_get(gt->uncore->rpm);
+
mutex_lock(>->reset.mutex);
if (GEM_SHOW_DEBUG()) {
@@ -926,7 +928,9 @@ void intel_gt_set_wedged(struct intel_gt *gt)
__intel_gt_set_wedged(gt);
mutex_unlock(>->reset.mutex);
- intel_runtime_pm_put(gt->uncore->rpm, wakeref);
+
+ if (wakeref)
+ intel_runtime_pm_put(gt->uncore->rpm, wakeref);
}
static bool __intel_gt_unset_wedged(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index d26dcca7e654..60f6fcc6b71d 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1263,6 +1263,7 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
static int i915_drm_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ intel_wakeref_t wakeref;
int ret;
disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
@@ -1303,7 +1304,8 @@ static int i915_drm_resume(struct drm_device *dev)
if (HAS_DISPLAY(dev_priv))
drm_mode_config_reset(dev);
- i915_gem_resume(dev_priv);
+ with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
+ i915_gem_resume(dev_priv);
intel_modeset_init_hw(dev_priv);
intel_init_clock_gating(dev_priv);
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 05/11] drm/i915: Guard rpm helpers in gt helpers functions
2022-06-21 12:35 ` [Intel-gfx] [PATCH 05/11] drm/i915: Guard rpm helpers in gt helpers functions Tilak Tangudu
@ 2022-06-22 12:52 ` Jani Nikula
0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2022-06-22 12:52 UTC (permalink / raw)
To: Tilak Tangudu, intel-gfx, jon.ewins, rodrigo.vivi,
vinay.belgaumkar, chris.p.wilson, ashutosh.dixit, badal.nilawar,
anshuman.gupta, tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
On Tue, 21 Jun 2022, Tilak Tangudu <tilak.tangudu@intel.com> wrote:
> Guard rpm helpers in gt_sanitize and intel_gt_set_wedged
> with is_intel_rpm_allowed
>
> Acquire rpm wakeref for higherlevel function i915_gem_resume
>
> Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_gt_pm.c | 12 ++++++------
> drivers/gpu/drm/i915/gt/intel_reset.c | 10 +++++++---
> drivers/gpu/drm/i915/i915_driver.c | 4 +++-
> 3 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index be99b01a0984..9857b91194b7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -163,12 +163,14 @@ static void gt_sanitize(struct intel_gt *gt, bool force)
> {
> struct intel_engine_cs *engine;
> enum intel_engine_id id;
> - intel_wakeref_t wakeref;
> + intel_wakeref_t wakeref = 0;
We've got intel_wakeref_t to hide what it actually is. You shouldn't
assume you can assign 0 to it or use wakeref in an if condition. You
should treat it as opaque. You should assume the typedef could be
switched to a struct and you shouldn't have to change the code using it.
BR,
Jani.
>
> GT_TRACE(gt, "force:%s", str_yes_no(force));
>
> /* Use a raw wakeref to avoid calling intel_display_power_get early */
> - wakeref = intel_runtime_pm_get(gt->uncore->rpm);
> + if (is_intel_rpm_allowed(gt->uncore->rpm))
> + wakeref = intel_runtime_pm_get(gt->uncore->rpm);
> +
> intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
>
> intel_gt_check_clock_frequency(gt);
> @@ -207,7 +209,8 @@ static void gt_sanitize(struct intel_gt *gt, bool force)
> intel_rps_sanitize(>->rps);
>
> intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
> - intel_runtime_pm_put(gt->uncore->rpm, wakeref);
> + if (wakeref)
> + intel_runtime_pm_put(gt->uncore->rpm, wakeref);
> }
>
> void intel_gt_pm_fini(struct intel_gt *gt)
> @@ -226,7 +229,6 @@ int intel_gt_resume(struct intel_gt *gt)
> return err;
>
> GT_TRACE(gt, "\n");
> -
> /*
> * After resume, we may need to poke into the pinned kernel
> * contexts to paper over any damage caused by the sudden suspend.
> @@ -259,10 +261,8 @@ int intel_gt_resume(struct intel_gt *gt)
>
> for_each_engine(engine, gt, id) {
> intel_engine_pm_get(engine);
> -
> engine->serial++; /* kernel context lost */
> err = intel_engine_resume(engine);
> -
> intel_engine_pm_put(engine);
> if (err) {
> drm_err(>->i915->drm,
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index c8e05b48c14f..55a1fd38c7c4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -901,12 +901,14 @@ static void __intel_gt_set_wedged(struct intel_gt *gt)
>
> void intel_gt_set_wedged(struct intel_gt *gt)
> {
> - intel_wakeref_t wakeref;
> + intel_wakeref_t wakeref = 0;
>
> if (test_bit(I915_WEDGED, >->reset.flags))
> return;
>
> - wakeref = intel_runtime_pm_get(gt->uncore->rpm);
> + if (is_intel_rpm_allowed(gt->uncore->rpm))
> + wakeref = intel_runtime_pm_get(gt->uncore->rpm);
> +
> mutex_lock(>->reset.mutex);
>
> if (GEM_SHOW_DEBUG()) {
> @@ -926,7 +928,9 @@ void intel_gt_set_wedged(struct intel_gt *gt)
> __intel_gt_set_wedged(gt);
>
> mutex_unlock(>->reset.mutex);
> - intel_runtime_pm_put(gt->uncore->rpm, wakeref);
> +
> + if (wakeref)
> + intel_runtime_pm_put(gt->uncore->rpm, wakeref);
> }
>
> static bool __intel_gt_unset_wedged(struct intel_gt *gt)
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index d26dcca7e654..60f6fcc6b71d 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1263,6 +1263,7 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
> static int i915_drm_resume(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + intel_wakeref_t wakeref;
> int ret;
>
> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
> @@ -1303,7 +1304,8 @@ static int i915_drm_resume(struct drm_device *dev)
> if (HAS_DISPLAY(dev_priv))
> drm_mode_config_reset(dev);
>
> - i915_gem_resume(dev_priv);
> + with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
> + i915_gem_resume(dev_priv);
>
> intel_modeset_init_hw(dev_priv);
> intel_init_clock_gating(dev_priv);
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Intel-gfx] [PATCH 06/11] drm/i915: Avoid rpm helpers in try_context_registration
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (4 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 05/11] drm/i915: Guard rpm helpers in gt helpers functions Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 07/11] drm/i915: Guard rc6 helpers with is_intel_rpm_allowed Tilak Tangudu
` (6 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Guard rpm helpers in try_context_registration with
is_intel_rpm_allowed
Avoid rpm helpers in guc_init_engine_stats and
rpm helpers not needed at higher level functions.
as intel_guc_submission_enable is called from
intel_gt_init_hw (which already holds wakeref)
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 28 +++++++++++--------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index e62ea35513ea..1be469810154 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1364,18 +1364,16 @@ static int guc_action_enable_usage_stats(struct intel_guc *guc)
static void guc_init_engine_stats(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
- intel_wakeref_t wakeref;
+ int ret;
mod_delayed_work(system_highpri_wq, &guc->timestamp.work,
guc->timestamp.ping_delay);
- with_intel_runtime_pm(>->i915->runtime_pm, wakeref) {
- int ret = guc_action_enable_usage_stats(guc);
+ ret = guc_action_enable_usage_stats(guc);
- if (ret)
- drm_err(>->i915->drm,
- "Failed to enable usage stats: %d!\n", ret);
- }
+ if (ret)
+ drm_err(>->i915->drm,
+ "Failed to enable usage stats: %d!\n", ret);
}
void intel_guc_busyness_park(struct intel_gt *gt)
@@ -2478,7 +2476,7 @@ static int try_context_registration(struct intel_context *ce, bool loop)
struct intel_engine_cs *engine = ce->engine;
struct intel_runtime_pm *runtime_pm = engine->uncore->rpm;
struct intel_guc *guc = &engine->gt->uc.guc;
- intel_wakeref_t wakeref;
+ intel_wakeref_t wakeref = 0;
u32 ctx_id = ce->guc_id.id;
bool context_registered;
int ret = 0;
@@ -2522,13 +2520,19 @@ static int try_context_registration(struct intel_context *ce, bool loop)
* If stealing the guc_id, this ce has the same guc_id as the
* context whose guc_id was stolen.
*/
- with_intel_runtime_pm(runtime_pm, wakeref)
- ret = deregister_context(ce, ce->guc_id.id);
+ if (is_intel_rpm_allowed(runtime_pm))
+ wakeref = intel_runtime_pm_get(runtime_pm);
+ ret = deregister_context(ce, ce->guc_id.id);
+ if (wakeref)
+ intel_runtime_pm_put(runtime_pm, wakeref);
if (unlikely(ret == -ENODEV))
ret = 0; /* Will get registered later */
} else {
- with_intel_runtime_pm(runtime_pm, wakeref)
- ret = register_context(ce, loop);
+ if (is_intel_rpm_allowed(runtime_pm))
+ wakeref = intel_runtime_pm_get(runtime_pm);
+ ret = register_context(ce, loop);
+ if (wakeref)
+ intel_runtime_pm_put(runtime_pm, wakeref);
if (unlikely(ret == -EBUSY)) {
clr_ctx_id_mapping(guc, ctx_id);
} else if (unlikely(ret == -ENODEV)) {
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Intel-gfx] [PATCH 07/11] drm/i915: Guard rc6 helpers with is_intel_rpm_allowed
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (5 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 06/11] drm/i915: Avoid rpm helpers in try_context_registration Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 08/11] drm/i915: Guard rpm helpers in rpm_get/put Tilak Tangudu
` (5 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Guard intel_rc6_sanitize/intel_rc6_enable/intel_rc6_disable
rc6 helpers with is_intel_rpm_allowed as these
are called in intel_gt_resume/intel_gt_suspend_late.
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/gt/intel_rc6.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
index f8d0523f4c18..73e2fb9420a2 100644
--- a/drivers/gpu/drm/i915/gt/intel_rc6.c
+++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
@@ -507,9 +507,14 @@ static bool rc6_supported(struct intel_rc6 *rc6)
static void rpm_get(struct intel_rc6 *rc6)
{
+ struct drm_i915_private *i915 = rc6_to_i915(rc6);
+
GEM_BUG_ON(rc6->wakeref);
- pm_runtime_get_sync(rc6_to_i915(rc6)->drm.dev);
- rc6->wakeref = true;
+
+ if (is_intel_rpm_allowed(&i915->runtime_pm)) {
+ pm_runtime_get_sync(i915->drm.dev);
+ rc6->wakeref = true;
+ }
}
static void rpm_put(struct intel_rc6 *rc6)
@@ -623,7 +628,9 @@ void intel_rc6_enable(struct intel_rc6 *rc6)
return;
/* rc6 is ready, runtime-pm is go! */
- rpm_put(rc6);
+ if (rc6->wakeref)
+ rpm_put(rc6);
+
rc6->enabled = true;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Intel-gfx] [PATCH 08/11] drm/i915: Guard rpm helpers in rpm_get/put
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (6 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 07/11] drm/i915: Guard rc6 helpers with is_intel_rpm_allowed Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers Tilak Tangudu
` (4 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Guard rpm helpers in rpm_get/put with is_intel_rpm_allowed
to avoid rpm helpers in intel_engine/gt_pm_get/put
called from user_forcewake, intel_gt_resume, i915_ttm_accel_move
and intel_context_enter/exit_engine
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/intel_wakeref.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
index dfd87d082218..00a5335387a4 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.c
+++ b/drivers/gpu/drm/i915/intel_wakeref.c
@@ -11,15 +11,17 @@
static void rpm_get(struct intel_wakeref *wf)
{
- wf->wakeref = intel_runtime_pm_get(wf->rpm);
+ if (is_intel_rpm_allowed(wf->rpm))
+ wf->wakeref = intel_runtime_pm_get(wf->rpm);
}
static void rpm_put(struct intel_wakeref *wf)
{
intel_wakeref_t wakeref = fetch_and_zero(&wf->wakeref);
- intel_runtime_pm_put(wf->rpm, wakeref);
- INTEL_WAKEREF_BUG_ON(!wakeref);
+ if (wakeref)
+ intel_runtime_pm_put(wf->rpm, wakeref);
+
}
int __intel_wakeref_get_first(struct intel_wakeref *wf)
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (7 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 08/11] drm/i915: Guard rpm helpers in rpm_get/put Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 16:30 ` kernel test robot
` (3 more replies)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 10/11] drm/i915: Guard rpm helpers at gt_park/unpark Tilak Tangudu
` (3 subsequent siblings)
12 siblings, 4 replies; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Add i915_save/load_pci_state helpers which saves
pci config state and restores the saved state.
Signed-off-by: Iddamsetty Aravind <Aravind.Iddamsetty@intel.com>
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/i915_driver.c | 34 ++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_drv.h | 1 +
2 files changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 60f6fcc6b71d..669365c2958c 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -102,6 +102,40 @@
static const struct drm_driver i915_drm_driver;
+bool i915_save_pci_state(struct pci_dev *pdev)
+{
+ struct drm_i915_private *i915 = pci_get_drvdata(pdev);
+
+ if (pci_save_state(pdev))
+ return false;
+
+ kfree(i915->pci_state);
+
+ i915->pci_state = pci_store_saved_state(pdev);
+
+ if (!i915->pci_state) {
+ drm_err(&i915->drm, "Failed to store PCI saved state\n");
+ return false;
+ }
+
+ return true;
+}
+
+void i915_load_pci_state(struct pci_dev *pdev)
+{
+ struct drm_i915_private *i915 = pci_get_drvdata(pdev);
+ int ret;
+
+ if (!i915->pci_state)
+ return;
+
+ ret = pci_load_saved_state(pdev, i915->pci_state);
+ if (!ret) {
+ pci_restore_state(pdev);
+ } else {
+ drm_warn(&i915->drm, "Failed to load PCI state, err:%d\n", ret);
+ }
+}
static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
{
int domain = pci_domain_nr(to_pci_dev(dev_priv->drm.dev)->bus);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c22f29c3faa0..ec8c7a2af673 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -777,6 +777,7 @@ struct drm_i915_private {
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
*/
+ struct pci_saved_state *pci_state;
};
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers
2022-06-21 12:35 ` [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers Tilak Tangudu
@ 2022-06-21 16:30 ` kernel test robot
2022-06-21 19:44 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2022-06-21 16:30 UTC (permalink / raw)
To: Tilak Tangudu, intel-gfx, jon.ewins, rodrigo.vivi,
vinay.belgaumkar, chris.p.wilson, ashutosh.dixit, badal.nilawar,
anshuman.gupta, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Cc: kbuild-all
Hi Tilak,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Tilak-Tangudu/drm-i915-Add-D3Cold-Off-support-for-runtime-pm/20220621-202453
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-debian-10.3-kselftests (https://download.01.org/0day-ci/archive/20220622/202206220018.NY2T7g0S-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/ad0aa2eb6293edc066466ecf3b82ce2e4e0a9636
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Tilak-Tangudu/drm-i915-Add-D3Cold-Off-support-for-runtime-pm/20220621-202453
git checkout ad0aa2eb6293edc066466ecf3b82ce2e4e0a9636
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/i915_driver.c:108:6: warning: no previous prototype for 'i915_save_pci_state' [-Wmissing-prototypes]
108 | bool i915_save_pci_state(struct pci_dev *pdev)
| ^~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/i915_driver.c:127:6: warning: no previous prototype for 'i915_load_pci_state' [-Wmissing-prototypes]
127 | void i915_load_pci_state(struct pci_dev *pdev)
| ^~~~~~~~~~~~~~~~~~~
vim +/i915_save_pci_state +108 drivers/gpu/drm/i915/i915_driver.c
107
> 108 bool i915_save_pci_state(struct pci_dev *pdev)
109 {
110 struct drm_i915_private *i915 = pci_get_drvdata(pdev);
111
112 if (pci_save_state(pdev))
113 return false;
114
115 kfree(i915->pci_state);
116
117 i915->pci_state = pci_store_saved_state(pdev);
118
119 if (!i915->pci_state) {
120 drm_err(&i915->drm, "Failed to store PCI saved state\n");
121 return false;
122 }
123
124 return true;
125 }
126
> 127 void i915_load_pci_state(struct pci_dev *pdev)
128 {
129 struct drm_i915_private *i915 = pci_get_drvdata(pdev);
130 int ret;
131
132 if (!i915->pci_state)
133 return;
134
135 ret = pci_load_saved_state(pdev, i915->pci_state);
136 if (!ret) {
137 pci_restore_state(pdev);
138 } else {
139 drm_warn(&i915->drm, "Failed to load PCI state, err:%d\n", ret);
140 }
141 }
142 static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
143 {
144 int domain = pci_domain_nr(to_pci_dev(dev_priv->drm.dev)->bus);
145
146 dev_priv->bridge_dev =
147 pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0));
148 if (!dev_priv->bridge_dev) {
149 drm_err(&dev_priv->drm, "bridge device not found\n");
150 return -EIO;
151 }
152 return 0;
153 }
154
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers
2022-06-21 12:35 ` [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers Tilak Tangudu
2022-06-21 16:30 ` kernel test robot
@ 2022-06-21 19:44 ` kernel test robot
2022-06-21 22:57 ` kernel test robot
2022-06-22 8:35 ` kernel test robot
3 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2022-06-21 19:44 UTC (permalink / raw)
To: Tilak Tangudu, intel-gfx, jon.ewins, rodrigo.vivi,
vinay.belgaumkar, chris.p.wilson, ashutosh.dixit, badal.nilawar,
anshuman.gupta, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Cc: llvm, kbuild-all
Hi Tilak,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Tilak-Tangudu/drm-i915-Add-D3Cold-Off-support-for-runtime-pm/20220621-202453
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-randconfig-a004 (https://download.01.org/0day-ci/archive/20220622/202206220303.UvbFJUZD-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/ad0aa2eb6293edc066466ecf3b82ce2e4e0a9636
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Tilak-Tangudu/drm-i915-Add-D3Cold-Off-support-for-runtime-pm/20220621-202453
git checkout ad0aa2eb6293edc066466ecf3b82ce2e4e0a9636
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/i915_driver.c:108:6: warning: no previous prototype for function 'i915_save_pci_state' [-Wmissing-prototypes]
bool i915_save_pci_state(struct pci_dev *pdev)
^
drivers/gpu/drm/i915/i915_driver.c:108:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
bool i915_save_pci_state(struct pci_dev *pdev)
^
static
>> drivers/gpu/drm/i915/i915_driver.c:127:6: warning: no previous prototype for function 'i915_load_pci_state' [-Wmissing-prototypes]
void i915_load_pci_state(struct pci_dev *pdev)
^
drivers/gpu/drm/i915/i915_driver.c:127:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void i915_load_pci_state(struct pci_dev *pdev)
^
static
2 warnings generated.
vim +/i915_save_pci_state +108 drivers/gpu/drm/i915/i915_driver.c
107
> 108 bool i915_save_pci_state(struct pci_dev *pdev)
109 {
110 struct drm_i915_private *i915 = pci_get_drvdata(pdev);
111
112 if (pci_save_state(pdev))
113 return false;
114
115 kfree(i915->pci_state);
116
117 i915->pci_state = pci_store_saved_state(pdev);
118
119 if (!i915->pci_state) {
120 drm_err(&i915->drm, "Failed to store PCI saved state\n");
121 return false;
122 }
123
124 return true;
125 }
126
> 127 void i915_load_pci_state(struct pci_dev *pdev)
128 {
129 struct drm_i915_private *i915 = pci_get_drvdata(pdev);
130 int ret;
131
132 if (!i915->pci_state)
133 return;
134
135 ret = pci_load_saved_state(pdev, i915->pci_state);
136 if (!ret) {
137 pci_restore_state(pdev);
138 } else {
139 drm_warn(&i915->drm, "Failed to load PCI state, err:%d\n", ret);
140 }
141 }
142 static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
143 {
144 int domain = pci_domain_nr(to_pci_dev(dev_priv->drm.dev)->bus);
145
146 dev_priv->bridge_dev =
147 pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0));
148 if (!dev_priv->bridge_dev) {
149 drm_err(&dev_priv->drm, "bridge device not found\n");
150 return -EIO;
151 }
152 return 0;
153 }
154
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers
2022-06-21 12:35 ` [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers Tilak Tangudu
2022-06-21 16:30 ` kernel test robot
2022-06-21 19:44 ` kernel test robot
@ 2022-06-21 22:57 ` kernel test robot
2022-06-22 8:35 ` kernel test robot
3 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2022-06-21 22:57 UTC (permalink / raw)
To: Tilak Tangudu, intel-gfx, jon.ewins, rodrigo.vivi,
vinay.belgaumkar, chris.p.wilson, ashutosh.dixit, badal.nilawar,
anshuman.gupta, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Cc: kbuild-all
Hi Tilak,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Tilak-Tangudu/drm-i915-Add-D3Cold-Off-support-for-runtime-pm/20220621-202453
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-rhel-8.3-syz (https://download.01.org/0day-ci/archive/20220622/202206220601.aElPXis6-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/ad0aa2eb6293edc066466ecf3b82ce2e4e0a9636
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Tilak-Tangudu/drm-i915-Add-D3Cold-Off-support-for-runtime-pm/20220621-202453
git checkout ad0aa2eb6293edc066466ecf3b82ce2e4e0a9636
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/i915_driver.c:108:6: error: no previous prototype for 'i915_save_pci_state' [-Werror=missing-prototypes]
108 | bool i915_save_pci_state(struct pci_dev *pdev)
| ^~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/i915_driver.c:127:6: error: no previous prototype for 'i915_load_pci_state' [-Werror=missing-prototypes]
127 | void i915_load_pci_state(struct pci_dev *pdev)
| ^~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/i915_save_pci_state +108 drivers/gpu/drm/i915/i915_driver.c
107
> 108 bool i915_save_pci_state(struct pci_dev *pdev)
109 {
110 struct drm_i915_private *i915 = pci_get_drvdata(pdev);
111
112 if (pci_save_state(pdev))
113 return false;
114
115 kfree(i915->pci_state);
116
117 i915->pci_state = pci_store_saved_state(pdev);
118
119 if (!i915->pci_state) {
120 drm_err(&i915->drm, "Failed to store PCI saved state\n");
121 return false;
122 }
123
124 return true;
125 }
126
> 127 void i915_load_pci_state(struct pci_dev *pdev)
128 {
129 struct drm_i915_private *i915 = pci_get_drvdata(pdev);
130 int ret;
131
132 if (!i915->pci_state)
133 return;
134
135 ret = pci_load_saved_state(pdev, i915->pci_state);
136 if (!ret) {
137 pci_restore_state(pdev);
138 } else {
139 drm_warn(&i915->drm, "Failed to load PCI state, err:%d\n", ret);
140 }
141 }
142 static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
143 {
144 int domain = pci_domain_nr(to_pci_dev(dev_priv->drm.dev)->bus);
145
146 dev_priv->bridge_dev =
147 pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0));
148 if (!dev_priv->bridge_dev) {
149 drm_err(&dev_priv->drm, "bridge device not found\n");
150 return -EIO;
151 }
152 return 0;
153 }
154
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers
2022-06-21 12:35 ` [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers Tilak Tangudu
` (2 preceding siblings ...)
2022-06-21 22:57 ` kernel test robot
@ 2022-06-22 8:35 ` kernel test robot
3 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2022-06-22 8:35 UTC (permalink / raw)
To: Tilak Tangudu, intel-gfx, jon.ewins, rodrigo.vivi,
vinay.belgaumkar, chris.p.wilson, ashutosh.dixit, badal.nilawar,
anshuman.gupta, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Cc: kbuild-all
Hi Tilak,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Tilak-Tangudu/drm-i915-Add-D3Cold-Off-support-for-runtime-pm/20220621-202453
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220622/202206221620.m7i1Q9uZ-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-31-g4880bd19-dirty
# https://github.com/intel-lab-lkp/linux/commit/ad0aa2eb6293edc066466ecf3b82ce2e4e0a9636
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Tilak-Tangudu/drm-i915-Add-D3Cold-Off-support-for-runtime-pm/20220621-202453
git checkout ad0aa2eb6293edc066466ecf3b82ce2e4e0a9636
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/i915/i915_driver.c:108:6: sparse: sparse: symbol 'i915_save_pci_state' was not declared. Should it be static?
>> drivers/gpu/drm/i915/i915_driver.c:127:6: sparse: sparse: symbol 'i915_load_pci_state' was not declared. Should it be static?
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Intel-gfx] [PATCH 10/11] drm/i915: Guard rpm helpers at gt_park/unpark
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (8 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 09/11] drm/i915: Add i915_save/load_pci_state helpers Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 12:35 ` [Intel-gfx] [PATCH 11/11] drm/i915 : Add D3COLD OFF support Tilak Tangudu
` (2 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Guard rpm helpers at gt_park/unpark with is_intel_rpm_allowed
to guard (gt/engine)_pm_(get/put)
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 9 ++++++---
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 10 ++++++----
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 136cc44c3deb..e353aa0c649b 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1600,9 +1600,12 @@ bool intel_engines_are_idle(struct intel_gt *gt)
if (intel_gt_is_wedged(gt))
return true;
- /* Already parked (and passed an idleness test); must still be idle */
- if (!READ_ONCE(gt->awake))
- return true;
+ /* Ignore gt->awake when rpm is not allowed as wakeref is not held at gt_unpark */
+ if (is_intel_rpm_allowed(gt->uncore->rpm)) {
+ /* Already parked (and passed an idleness test); must still be idle */
+ if (!READ_ONCE(gt->awake))
+ return true;
+ }
for_each_engine(engine, gt, id) {
if (!intel_engine_is_idle(engine))
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 9857b91194b7..12117cf7eb94 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -81,8 +81,10 @@ static int __gt_unpark(struct intel_wakeref *wf)
* Work around it by grabbing a GT IRQ power domain whilst there is any
* GT activity, preventing any DC state transitions.
*/
- gt->awake = intel_display_power_get(i915, POWER_DOMAIN_GT_IRQ);
- GEM_BUG_ON(!gt->awake);
+ if (is_intel_rpm_allowed(gt->uncore->rpm)) {
+ gt->awake = intel_display_power_get(i915, POWER_DOMAIN_GT_IRQ);
+ GEM_BUG_ON(!gt->awake);
+ }
intel_rc6_unpark(>->rc6);
intel_rps_unpark(>->rps);
@@ -116,8 +118,8 @@ static int __gt_park(struct intel_wakeref *wf)
intel_synchronize_irq(i915);
/* Defer dropping the display power well for 100ms, it's slow! */
- GEM_BUG_ON(!wakeref);
- intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref);
+ if (wakeref)
+ intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Intel-gfx] [PATCH 11/11] drm/i915 : Add D3COLD OFF support
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (9 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 10/11] drm/i915: Guard rpm helpers at gt_park/unpark Tilak Tangudu
@ 2022-06-21 12:35 ` Tilak Tangudu
2022-06-21 13:15 ` Gupta, Anshuman
2022-06-21 13:24 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Add D3Cold-Off support for runtime-pm Patchwork
2022-06-23 17:35 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Add D3Cold-Off support for runtime-pm (rev2) Patchwork
12 siblings, 1 reply; 26+ messages in thread
From: Tilak Tangudu @ 2022-06-21 12:35 UTC (permalink / raw)
To: intel-gfx, jon.ewins, rodrigo.vivi, vinay.belgaumkar,
chris.p.wilson, ashutosh.dixit, badal.nilawar, anshuman.gupta,
tilak.tangudu, matthew.d.roper, saurabhg.gupta,
Aravind.Iddamsetty, Sujaritha.Sundaresan
Added lmem deep suspend/resume, which covers lmem
eviction and added GT/GUC deep suspend/resume
using i915_gem_backup_suspend, i915_gem_suspend_late
and i915_gem_resume.
Added HAS_D3COLD_OFF feature macro to use for
D3COLD OFF feature
Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
---
drivers/gpu/drm/i915/i915_driver.c | 96 ++++++++++++++++++------
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_pci.c | 1 +
drivers/gpu/drm/i915/intel_device_info.h | 1 +
4 files changed, 76 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 669365c2958c..1ca45d933a4a 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1609,9 +1609,21 @@ static int intel_runtime_suspend(struct device *kdev)
* We are safe here against re-faults, since the fault handler takes
* an RPM reference.
*/
- i915_gem_runtime_suspend(dev_priv);
+ if (HAS_D3COLD_OFF(dev_priv)) {
+ i915_gem_backup_suspend(dev_priv);
+ i915_ggtt_suspend(to_gt(dev_priv)->ggtt);
+ i915_gem_suspend_late(dev_priv);
+ } else {
+ i915_gem_runtime_suspend(dev_priv);
+ intel_gt_runtime_suspend(to_gt(dev_priv));
- intel_gt_runtime_suspend(to_gt(dev_priv));
+ /*
+ * FIXME: Temporary hammer to avoid freezing the machine on our DGFX
+ * This should be totally removed when we handle the pci states properly
+ * on runtime PM and on s2idle cases.
+ */
+ pci_d3cold_disable(pdev);
+ }
intel_runtime_pm_disable_interrupts(dev_priv);
@@ -1641,12 +1653,6 @@ static int intel_runtime_suspend(struct device *kdev)
drm_err(&dev_priv->drm,
"Unclaimed access detected prior to suspending\n");
- /*
- * FIXME: Temporary hammer to avoid freezing the machine on our DGFX
- * This should be totally removed when we handle the pci states properly
- * on runtime PM and on s2idle cases.
- */
- pci_d3cold_disable(pdev);
rpm->suspended = true;
/*
@@ -1662,14 +1668,18 @@ static int intel_runtime_suspend(struct device *kdev)
*/
intel_opregion_notify_adapter(dev_priv, PCI_D3hot);
} else {
- /*
- * current versions of firmware which depend on this opregion
- * notification have repurposed the D1 definition to mean
- * "runtime suspended" vs. what you would normally expect (D3)
- * to distinguish it from notifications that might be sent via
- * the suspend path.
- */
- intel_opregion_notify_adapter(dev_priv, PCI_D1);
+ if (HAS_D3COLD_OFF(dev_priv)) {
+ intel_opregion_suspend(dev_priv, PCI_D3cold);
+ } else {
+ /*
+ * current versions of firmware which depend on this opregion
+ * notification have repurposed the D1 definition to mean
+ * "runtime suspended" vs. what you would normally expect (D3)
+ * to distinguish it from notifications that might be sent via
+ * the suspend path.
+ */
+ intel_opregion_notify_adapter(dev_priv, PCI_D1);
+ }
}
assert_forcewakes_inactive(&dev_priv->uncore);
@@ -1677,6 +1687,12 @@ static int intel_runtime_suspend(struct device *kdev)
if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
intel_hpd_poll_enable(dev_priv);
+ if (HAS_D3COLD_OFF(dev_priv)) {
+ i915_save_pci_state(pdev);
+ pci_disable_device(pdev);
+ pci_set_power_state(pdev, PCI_D3cold);
+ }
+
drm_dbg(&dev_priv->drm, "Device suspended\n");
return 0;
}
@@ -1696,9 +1712,28 @@ static int intel_runtime_resume(struct device *kdev)
drm_WARN_ON_ONCE(&dev_priv->drm, atomic_read(&rpm->wakeref_count));
disable_rpm_wakeref_asserts(rpm);
- intel_opregion_notify_adapter(dev_priv, PCI_D0);
+ if (HAS_D3COLD_OFF(dev_priv)) {
+ ret = pci_set_power_state(pdev, PCI_D0);
+ if (ret) {
+ drm_err(&dev_priv->drm,
+ "failed to set PCI D0 power state (%d)\n", ret);
+ return ret;
+ }
+
+ i915_load_pci_state(pdev);
+
+ ret = pci_enable_device(pdev);
+ if (ret)
+ return ret;
+ pci_set_master(pdev);
+ intel_opregion_resume(dev_priv);
+ } else {
+ pci_d3cold_enable(pdev);
+ intel_opregion_notify_adapter(dev_priv, PCI_D0);
+ }
+
rpm->suspended = false;
- pci_d3cold_enable(pdev);
+
if (intel_uncore_unclaimed_mmio(&dev_priv->uncore))
drm_dbg(&dev_priv->drm,
"Unclaimed access during suspend, bios?\n");
@@ -1711,12 +1746,27 @@ static int intel_runtime_resume(struct device *kdev)
intel_runtime_pm_enable_interrupts(dev_priv);
- /*
- * No point of rolling back things in case of an error, as the best
- * we can do is to hope that things will still work (and disable RPM).
- */
- intel_gt_runtime_resume(to_gt(dev_priv));
+ if (HAS_D3COLD_OFF(dev_priv)) {
+ ret = i915_pcode_init(dev_priv);
+ if (ret)
+ return ret;
+ sanitize_gpu(dev_priv);
+ ret = i915_ggtt_enable_hw(dev_priv);
+ if (ret)
+ drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
+
+ i915_ggtt_resume(to_gt(dev_priv)->ggtt);
+
+ i915_gem_resume(dev_priv);
+
+ } else {
+ /*
+ * No point of rolling back things in case of an error, as the best
+ * we can do is to hope that things will still work (and disable RPM).
+ */
+ intel_gt_runtime_resume(to_gt(dev_priv));
+ }
/*
* On VLV/CHV display interrupts are part of the display
* power well, so hpd is reinitialized from there. For
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ec8c7a2af673..633d20c2372a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1300,6 +1300,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i))
#define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM)
+#define HAS_D3COLD_OFF(i915) (INTEL_INFO(dev_priv)->has_d3cold_off)
/*
* Platform has the dedicated compression control state for each lmem surfaces
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 5e51fc29bb8b..749ccb14fd6f 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1064,6 +1064,7 @@ static const struct intel_device_info xehpsdv_info = {
.has_guc_deprivilege = 1, \
.has_heci_pxp = 1, \
.needs_compact_pt = 1, \
+ .has_d3cold_off = 1, \
.has_media_ratio_mode = 1, \
.platform_engine_mask = \
BIT(RCS0) | BIT(BCS0) | \
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 08341174ee0a..495c12d65c3e 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -140,6 +140,7 @@ enum intel_ppgtt_type {
/* Keep has_* in alphabetical order */ \
func(has_64bit_reloc); \
func(has_64k_pages); \
+ func(has_d3cold_off); \
func(needs_compact_pt); \
func(gpu_reset_clobbers_display); \
func(has_reset_engine); \
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Intel-gfx] [PATCH 11/11] drm/i915 : Add D3COLD OFF support
2022-06-21 12:35 ` [Intel-gfx] [PATCH 11/11] drm/i915 : Add D3COLD OFF support Tilak Tangudu
@ 2022-06-21 13:15 ` Gupta, Anshuman
0 siblings, 0 replies; 26+ messages in thread
From: Gupta, Anshuman @ 2022-06-21 13:15 UTC (permalink / raw)
To: Tangudu, Tilak, intel-gfx@lists.freedesktop.org, Ewins, Jon,
Vivi, Rodrigo, Belgaumkar, Vinay, Wilson, Chris P,
Dixit, Ashutosh, Nilawar, Badal, Roper, Matthew D,
Gupta, saurabhg, Iddamsetty, Aravind, Sundaresan, Sujaritha
> -----Original Message-----
> From: Tangudu, Tilak <tilak.tangudu@intel.com>
> Sent: Tuesday, June 21, 2022 6:05 PM
> To: intel-gfx@lists.freedesktop.org; Ewins, Jon <jon.ewins@intel.com>; Vivi,
> Rodrigo <rodrigo.vivi@intel.com>; Belgaumkar, Vinay
> <vinay.belgaumkar@intel.com>; Wilson, Chris P <chris.p.wilson@intel.com>;
> Dixit, Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
> <badal.nilawar@intel.com>; Gupta, Anshuman <anshuman.gupta@intel.com>;
> Tangudu, Tilak <tilak.tangudu@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>; Gupta, saurabhg <saurabhg.gupta@intel.com>;
> Iddamsetty, Aravind <aravind.iddamsetty@intel.com>; Sundaresan, Sujaritha
> <sujaritha.sundaresan@intel.com>
> Subject: [PATCH 11/11] drm/i915 : Add D3COLD OFF support
>
> Added lmem deep suspend/resume, which covers lmem eviction and added
> GT/GUC deep suspend/resume using i915_gem_backup_suspend,
> i915_gem_suspend_late and i915_gem_resume.
>
> Added HAS_D3COLD_OFF feature macro to use for D3COLD OFF feature
We don't need such Macro HAS_D3COLD_OFF , checking bridge pme capability from D3Cold is sufficient,
As it is a pci feature. You can refer below patch.
https://patchwork.freedesktop.org/patch/489746/?series=104128&rev=3
>
> Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>
> ---
> drivers/gpu/drm/i915/i915_driver.c | 96 ++++++++++++++++++------
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_pci.c | 1 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> 4 files changed, 76 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> index 669365c2958c..1ca45d933a4a 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1609,9 +1609,21 @@ static int intel_runtime_suspend(struct device *kdev)
> * We are safe here against re-faults, since the fault handler takes
> * an RPM reference.
> */
> - i915_gem_runtime_suspend(dev_priv);
> + if (HAS_D3COLD_OFF(dev_priv)) {
> + i915_gem_backup_suspend(dev_priv);
> + i915_ggtt_suspend(to_gt(dev_priv)->ggtt);
> + i915_gem_suspend_late(dev_priv);
> + } else {
> + i915_gem_runtime_suspend(dev_priv);
> + intel_gt_runtime_suspend(to_gt(dev_priv));
>
> - intel_gt_runtime_suspend(to_gt(dev_priv));
> + /*
> + * FIXME: Temporary hammer to avoid freezing the machine on
> our DGFX
> + * This should be totally removed when we handle the pci states
> properly
> + * on runtime PM and on s2idle cases.
> + */
> + pci_d3cold_disable(pdev);
> + }
>
> intel_runtime_pm_disable_interrupts(dev_priv);
>
> @@ -1641,12 +1653,6 @@ static int intel_runtime_suspend(struct device *kdev)
> drm_err(&dev_priv->drm,
> "Unclaimed access detected prior to suspending\n");
>
> - /*
> - * FIXME: Temporary hammer to avoid freezing the machine on our
> DGFX
> - * This should be totally removed when we handle the pci states
> properly
> - * on runtime PM and on s2idle cases.
> - */
> - pci_d3cold_disable(pdev);
> rpm->suspended = true;
>
> /*
> @@ -1662,14 +1668,18 @@ static int intel_runtime_suspend(struct device
> *kdev)
> */
> intel_opregion_notify_adapter(dev_priv, PCI_D3hot);
> } else {
> - /*
> - * current versions of firmware which depend on this opregion
> - * notification have repurposed the D1 definition to mean
> - * "runtime suspended" vs. what you would normally expect
> (D3)
> - * to distinguish it from notifications that might be sent via
> - * the suspend path.
> - */
> - intel_opregion_notify_adapter(dev_priv, PCI_D1);
> + if (HAS_D3COLD_OFF(dev_priv)) {
> + intel_opregion_suspend(dev_priv, PCI_D3cold);
This is not needed, this opregion mbox is already depreciated.
> + } else {
> + /*
> + * current versions of firmware which depend on this
> opregion
> + * notification have repurposed the D1 definition to
> mean
> + * "runtime suspended" vs. what you would normally
> expect (D3)
> + * to distinguish it from notifications that might be sent
> via
> + * the suspend path.
> + */
> + intel_opregion_notify_adapter(dev_priv, PCI_D1);
> + }
> }
>
> assert_forcewakes_inactive(&dev_priv->uncore);
> @@ -1677,6 +1687,12 @@ static int intel_runtime_suspend(struct device *kdev)
> if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> intel_hpd_poll_enable(dev_priv);
>
> + if (HAS_D3COLD_OFF(dev_priv)) {
> + i915_save_pci_state(pdev);
PCI core do save/restore state , do we need this ?
> + pci_disable_device(pdev);
> + pci_set_power_state(pdev, PCI_D3cold);
We don't need to set the pci state as well, as though pci config state, it can set up to max d3hot.
> + }
> +
> drm_dbg(&dev_priv->drm, "Device suspended\n");
> return 0;
> }
> @@ -1696,9 +1712,28 @@ static int intel_runtime_resume(struct device *kdev)
> drm_WARN_ON_ONCE(&dev_priv->drm, atomic_read(&rpm-
> >wakeref_count));
> disable_rpm_wakeref_asserts(rpm);
>
> - intel_opregion_notify_adapter(dev_priv, PCI_D0);
> + if (HAS_D3COLD_OFF(dev_priv)) {
> + ret = pci_set_power_state(pdev, PCI_D0);
> + if (ret) {
> + drm_err(&dev_priv->drm,
> + "failed to set PCI D0 power state (%d)\n", ret);
> + return ret;
> + }
> +
> + i915_load_pci_state(pdev);
> +
> + ret = pci_enable_device(pdev);
> + if (ret)
> + return ret;
> + pci_set_master(pdev);
> + intel_opregion_resume(dev_priv);
> + } else {
> + pci_d3cold_enable(pdev);
> + intel_opregion_notify_adapter(dev_priv, PCI_D0);
> + }
> +
> rpm->suspended = false;
> - pci_d3cold_enable(pdev);
> +
> if (intel_uncore_unclaimed_mmio(&dev_priv->uncore))
> drm_dbg(&dev_priv->drm,
> "Unclaimed access during suspend, bios?\n"); @@ -
> 1711,12 +1746,27 @@ static int intel_runtime_resume(struct device *kdev)
>
> intel_runtime_pm_enable_interrupts(dev_priv);
>
> - /*
> - * No point of rolling back things in case of an error, as the best
> - * we can do is to hope that things will still work (and disable RPM).
> - */
> - intel_gt_runtime_resume(to_gt(dev_priv));
> + if (HAS_D3COLD_OFF(dev_priv)) {
> + ret = i915_pcode_init(dev_priv);
> + if (ret)
> + return ret;
>
> + sanitize_gpu(dev_priv);
> + ret = i915_ggtt_enable_hw(dev_priv);
> + if (ret)
> + drm_err(&dev_priv->drm, "failed to re-enable
> GGTT\n");
> +
> + i915_ggtt_resume(to_gt(dev_priv)->ggtt);
> +
> + i915_gem_resume(dev_priv);
> +
> + } else {
> + /*
> + * No point of rolling back things in case of an error, as the best
> + * we can do is to hope that things will still work (and disable
> RPM).
> + */
> + intel_gt_runtime_resume(to_gt(dev_priv));
> + }
> /*
> * On VLV/CHV display interrupts are part of the display
> * power well, so hpd is reinitialized from there. For diff --git
> a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index
> ec8c7a2af673..633d20c2372a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1300,6 +1300,7 @@ IS_SUBPLATFORM(const struct drm_i915_private
> *i915,
>
> #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i))
> #define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM)
> +#define HAS_D3COLD_OFF(i915) (INTEL_INFO(dev_priv)->has_d3cold_off)
>
> /*
> * Platform has the dedicated compression control state for each lmem surfaces
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 5e51fc29bb8b..749ccb14fd6f 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -1064,6 +1064,7 @@ static const struct intel_device_info xehpsdv_info = {
> .has_guc_deprivilege = 1, \
> .has_heci_pxp = 1, \
> .needs_compact_pt = 1, \
> + .has_d3cold_off = 1, \
> .has_media_ratio_mode = 1, \
> .platform_engine_mask = \
> BIT(RCS0) | BIT(BCS0) | \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> b/drivers/gpu/drm/i915/intel_device_info.h
> index 08341174ee0a..495c12d65c3e 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -140,6 +140,7 @@ enum intel_ppgtt_type {
> /* Keep has_* in alphabetical order */ \
> func(has_64bit_reloc); \
> func(has_64k_pages); \
> + func(has_d3cold_off); \
> func(needs_compact_pt); \
> func(gpu_reset_clobbers_display); \
> func(has_reset_engine); \
> --
> 2.25.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Add D3Cold-Off support for runtime-pm
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (10 preceding siblings ...)
2022-06-21 12:35 ` [Intel-gfx] [PATCH 11/11] drm/i915 : Add D3COLD OFF support Tilak Tangudu
@ 2022-06-21 13:24 ` Patchwork
2022-06-23 17:35 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Add D3Cold-Off support for runtime-pm (rev2) Patchwork
12 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2022-06-21 13:24 UTC (permalink / raw)
To: Tilak Tangudu; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Add D3Cold-Off support for runtime-pm
URL : https://patchwork.freedesktop.org/series/105427/
State : failure
== Summary ==
Error: make failed
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/i915/i915_driver.o
drivers/gpu/drm/i915/i915_driver.c:108:6: error: no previous prototype for ‘i915_save_pci_state’ [-Werror=missing-prototypes]
bool i915_save_pci_state(struct pci_dev *pdev)
^~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/i915_driver.c:127:6: error: no previous prototype for ‘i915_load_pci_state’ [-Werror=missing-prototypes]
void i915_load_pci_state(struct pci_dev *pdev)
^~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:249: recipe for target 'drivers/gpu/drm/i915/i915_driver.o' failed
make[4]: *** [drivers/gpu/drm/i915/i915_driver.o] Error 1
scripts/Makefile.build:466: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:466: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:466: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1843: recipe for target 'drivers' failed
make: *** [drivers] Error 2
^ permalink raw reply [flat|nested] 26+ messages in thread* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Add D3Cold-Off support for runtime-pm (rev2)
2022-06-21 12:35 [Intel-gfx] [PATCH 00/11] drm/i915: Add D3Cold-Off support for runtime-pm Tilak Tangudu
` (11 preceding siblings ...)
2022-06-21 13:24 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Add D3Cold-Off support for runtime-pm Patchwork
@ 2022-06-23 17:35 ` Patchwork
12 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2022-06-23 17:35 UTC (permalink / raw)
To: Tangudu, Tilak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Add D3Cold-Off support for runtime-pm (rev2)
URL : https://patchwork.freedesktop.org/series/105427/
State : failure
== Summary ==
Error: patch https://patchwork.freedesktop.org/api/1.0/series/105427/revisions/2/mbox/ not applied
Applying: drm/i915: Avoid rpm helpers in intel_guc_global_policies_update
Applying: drm/i915: Avoid rpm helpers in intel_guc_slpc_set_media_ratio_mode
Applying: drm/i915: Avoid rpm helpers in intel_gt_suspend_late
Applying: drm/i915: Added is_intel_rpm_allowed helper
error: patch failed: drivers/gpu/drm/i915/intel_runtime_pm.c:369
error: drivers/gpu/drm/i915/intel_runtime_pm.c: patch does not apply
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/intel_runtime_pm.c
Patch failed at 0004 drm/i915: Added is_intel_rpm_allowed helper
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
^ permalink raw reply [flat|nested] 26+ messages in thread