* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-10-29 22:14 Anusha Srivatsa
2018-10-29 23:53 ` ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev6) Patchwork
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Anusha Srivatsa @ 2018-10-29 22:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
v6: (Anusha Srivatsa)
- rebased.Use INTEL_GEN consistently.
- Simplify the code (Rodrigo)
v7: rebased. Change order according to platforms(Jyoti)
v8: rebased. Change the check from platform specific to
HAS_PCH_SPLIT(). Add comment in code to be more clear.(Rodrigo)
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Tested-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 32 ++++++++++++++++---------
3 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ad13da61d7a..6bdcd5a3d7b7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2157,7 +2157,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2924,7 +2924,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -3009,7 +3012,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 268afb6d2746..e4eaa40bd5f1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1695,6 +1695,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
@@ -2045,6 +2046,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5f5416eb9644..b1901a6c17be 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -633,8 +635,13 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ /*
+ * Power sequencer reset is not needed on
+ * platforms with South Display Engine on PCH,
+ * because PPS registers are always on.
+ */
+ if (!HAS_PCH_SPLIT(dev_priv))
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -716,7 +723,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2978,17 +2985,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
+ if (INTEL_GEN(dev_priv) >= 11) {
max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
+ } else if (IS_GEN9_LP(dev_priv)) {
+ max_dc = 1;
+ mask = DC_STATE_EN_DC9;
} else {
max_dc = 0;
mask = 0;
@@ -3539,8 +3549,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3592,7 +3602,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
intel_csr_load_program(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev6)
2018-10-29 22:14 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
@ 2018-10-29 23:53 ` Patchwork
2018-10-30 7:12 ` ✓ Fi.CI.IGT: " Patchwork
2018-10-30 17:56 ` [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Rodrigo Vivi
2 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-10-29 23:53 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev6)
URL : https://patchwork.freedesktop.org/series/49447/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5052 -> Patchwork_10638 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/49447/revisions/6/mbox/
== Known issues ==
Here are the changes found in Patchwork_10638 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_contexts:
fi-icl-u: NOTRUN -> DMESG-FAIL (fdo#108569)
igt@kms_flip@basic-flip-vs-modeset:
fi-skl-6700hq: PASS -> DMESG-WARN (fdo#105998)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362) +1
==== Possible fixes ====
igt@drv_module_reload@basic-reload:
fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS
igt@gem_exec_suspend@basic-s3:
fi-icl-u: INCOMPLETE (fdo#107713) -> PASS
igt@kms_flip@basic-flip-vs-dpms:
fi-skl-6700hq: DMESG-WARN (fdo#105998) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-b:
fi-byt-clapper: FAIL (fdo#107362) -> PASS
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
== Participating hosts (49 -> 44) ==
Missing (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_5052 -> Patchwork_10638
CI_DRM_5052: 24b6ea5d5f299c5e9e6b4d92068651f7f1555bd0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4699: 1270ec553741ac20c45178d2b26f9a9562ea565f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10638: 400a4e829380666942c2193967b231dd0b9275b1 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
400a4e829380 drm/i915/icl: Enable DC9 as lowest possible state during screen-off
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10638/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev6)
2018-10-29 22:14 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
2018-10-29 23:53 ` ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev6) Patchwork
@ 2018-10-30 7:12 ` Patchwork
2018-10-30 17:56 ` [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Rodrigo Vivi
2 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-10-30 7:12 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev6)
URL : https://patchwork.freedesktop.org/series/49447/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5052_full -> Patchwork_10638_full =
== Summary - SUCCESS ==
No regressions found.
== Known issues ==
Here are the changes found in Patchwork_10638_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_schedule@pi-ringfull-bsd:
shard-skl: NOTRUN -> FAIL (fdo#103158)
igt@gem_tiled_blits@interruptible:
shard-apl: PASS -> INCOMPLETE (fdo#103927)
igt@gem_userptr_blits@readonly-unsync:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108074)
igt@kms_busy@extended-modeset-hang-newfb-render-a:
shard-skl: NOTRUN -> DMESG-WARN (fdo#107956) +1
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
shard-snb: NOTRUN -> DMESG-WARN (fdo#107956) +1
igt@kms_chv_cursor_fail@pipe-c-128x128-top-edge:
shard-skl: NOTRUN -> FAIL (fdo#104671)
igt@kms_color@pipe-c-degamma:
shard-apl: PASS -> FAIL (fdo#104782)
igt@kms_cursor_crc@cursor-256x256-sliding:
shard-glk: PASS -> FAIL (fdo#103232) +3
igt@kms_cursor_crc@cursor-256x85-onscreen:
shard-apl: NOTRUN -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-256x85-random:
shard-apl: PASS -> FAIL (fdo#103232) +1
igt@kms_flip@2x-wf_vblank-ts-check:
shard-snb: SKIP -> INCOMPLETE (fdo#105411)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
shard-glk: PASS -> FAIL (fdo#103167) +2
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
shard-apl: PASS -> FAIL (fdo#103167)
igt@kms_frontbuffer_tracking@fbc-stridechange:
shard-skl: NOTRUN -> FAIL (fdo#105683)
igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-wc:
shard-skl: NOTRUN -> FAIL (fdo#103167) +1
igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
shard-skl: NOTRUN -> FAIL (fdo#108145, fdo#107815)
igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
shard-apl: NOTRUN -> FAIL (fdo#108145)
igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
shard-skl: NOTRUN -> FAIL (fdo#108145)
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-apl: PASS -> FAIL (fdo#103166) +2
igt@kms_setmode@basic:
shard-kbl: PASS -> FAIL (fdo#99912)
shard-snb: NOTRUN -> FAIL (fdo#99912)
igt@pm_rpm@gem-evict-pwrite:
shard-skl: NOTRUN -> INCOMPLETE (fdo#107807)
==== Possible fixes ====
igt@gem_eio@in-flight-contexts-1us:
shard-glk: FAIL (fdo#105957) -> PASS
igt@kms_busy@extended-modeset-hang-newfb-render-c:
shard-kbl: DMESG-WARN (fdo#107956) -> PASS
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
shard-hsw: DMESG-WARN (fdo#107956) -> PASS
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-glk: FAIL (fdo#108145) -> PASS
igt@kms_color@pipe-a-degamma:
shard-apl: FAIL (fdo#108145, fdo#104782) -> PASS
igt@kms_cursor_crc@cursor-128x128-suspend:
shard-apl: FAIL (fdo#103191, fdo#103232) -> PASS
igt@kms_cursor_crc@cursor-256x256-random:
shard-glk: FAIL (fdo#103232) -> PASS
igt@kms_cursor_crc@cursor-64x21-sliding:
shard-apl: FAIL (fdo#103232) -> PASS +1
igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
shard-glk: DMESG-WARN (fdo#106538, fdo#105763) -> PASS
igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled:
shard-glk: FAIL (fdo#103184) -> PASS +1
igt@kms_flip@plain-flip-fb-recreate:
shard-skl: FAIL (fdo#100368) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
shard-apl: FAIL (fdo#103167) -> PASS +2
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
shard-glk: FAIL (fdo#103167) -> PASS
igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
shard-apl: INCOMPLETE (fdo#103927) -> PASS +1
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: FAIL (fdo#103166) -> PASS
igt@kms_vblank@pipe-c-wait-busy:
shard-kbl: DMESG-WARN (fdo#103558, fdo#105602) -> PASS +3
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105683 https://bugs.freedesktop.org/show_bug.cgi?id=105683
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#105957 https://bugs.freedesktop.org/show_bug.cgi?id=105957
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 6) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_5052 -> Patchwork_10638
CI_DRM_5052: 24b6ea5d5f299c5e9e6b4d92068651f7f1555bd0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4699: 1270ec553741ac20c45178d2b26f9a9562ea565f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10638: 400a4e829380666942c2193967b231dd0b9275b1 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10638/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-29 22:14 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
2018-10-29 23:53 ` ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev6) Patchwork
2018-10-30 7:12 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-10-30 17:56 ` Rodrigo Vivi
2 siblings, 0 replies; 18+ messages in thread
From: Rodrigo Vivi @ 2018-10-30 17:56 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Mon, Oct 29, 2018 at 03:14:10PM -0700, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
>
> v6: (Anusha Srivatsa)
> - rebased.Use INTEL_GEN consistently.
> - Simplify the code (Rodrigo)
>
> v7: rebased. Change order according to platforms(Jyoti)
>
> v8: rebased. Change the check from platform specific to
> HAS_PCH_SPLIT(). Add comment in code to be more clear.(Rodrigo)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Tested-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
pushed to dinq, thanks for the patch, answers and tests
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 32 ++++++++++++++++---------
> 3 files changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ad13da61d7a..6bdcd5a3d7b7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2157,7 +2157,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2924,7 +2924,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -3009,7 +3012,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 268afb6d2746..e4eaa40bd5f1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1695,6 +1695,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> @@ -2045,6 +2046,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 5f5416eb9644..b1901a6c17be 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -633,8 +635,13 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + /*
> + * Power sequencer reset is not needed on
> + * platforms with South Display Engine on PCH,
> + * because PPS registers are always on.
> + */
> + if (!HAS_PCH_SPLIT(dev_priv))
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -716,7 +723,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2978,17 +2985,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> + mask = 0;
> + } else if (IS_GEN9_LP(dev_priv)) {
> + max_dc = 1;
> + mask = DC_STATE_EN_DC9;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3539,8 +3549,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3592,7 +3602,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-10-24 23:01 Anusha Srivatsa
2018-10-24 23:22 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: Anusha Srivatsa @ 2018-10-24 23:01 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
v6: (Anusha Srivatsa)
- rebased.Use INTEL_GEN consistently.
- Simplify the code (Rodrigo)
v7: rebased. Change order according to platforms(Jyoti)
v8: rebased. Change the check from platform specific to
HAS_PCH_SPLIT(). Add comment in code to be more clear.(Rodrigo)
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 31 ++++++++++++++++---------
3 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index baac35f698f9..6691b9ee95db 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0e9a926fca04..529ff19a5e48 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
@@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5f5416eb9644..31d08f452dcb 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -633,8 +635,12 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ /* In case of Gen11+, PPS registers are always on, due
+ * to PCH split. Reset power sequencer on platforms
+ * without the PCH split.
+ */
+ if (!HAS_PCH_SPLIT(dev_priv))
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -716,7 +722,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2978,17 +2984,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
+ if (INTEL_GEN(dev_priv) >= 11) {
max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
+ } else if (IS_GEN9_LP(dev_priv)) {
+ max_dc = 1;
+ mask = DC_STATE_EN_DC9;
} else {
max_dc = 0;
mask = 0;
@@ -3539,8 +3548,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3592,7 +3601,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
intel_csr_load_program(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-24 23:01 Anusha Srivatsa
@ 2018-10-24 23:22 ` Rodrigo Vivi
0 siblings, 0 replies; 18+ messages in thread
From: Rodrigo Vivi @ 2018-10-24 23:22 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Wed, Oct 24, 2018 at 04:01:09PM -0700, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
>
> v6: (Anusha Srivatsa)
> - rebased.Use INTEL_GEN consistently.
> - Simplify the code (Rodrigo)
>
> v7: rebased. Change order according to platforms(Jyoti)
>
> v8: rebased. Change the check from platform specific to
> HAS_PCH_SPLIT(). Add comment in code to be more clear.(Rodrigo)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 31 ++++++++++++++++---------
> 3 files changed, 40 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index baac35f698f9..6691b9ee95db 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0e9a926fca04..529ff19a5e48 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 5f5416eb9644..31d08f452dcb 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -633,8 +635,12 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + /* In case of Gen11+, PPS registers are always on, due
/*
* In case...
(but can be fixed while merging.
> + * to PCH split. Reset power sequencer on platforms
> + * without the PCH split.
> + */
What about:
/*
* Power sequencer reset is not needed on platforms with
* South Display Engine on PCH, because PPS registers are always on.
*/
with or without accepting this bikeshed
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> + if (!HAS_PCH_SPLIT(dev_priv))
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -716,7 +722,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2978,17 +2984,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> + mask = 0;
> + } else if (IS_GEN9_LP(dev_priv)) {
> + max_dc = 1;
> + mask = DC_STATE_EN_DC9;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3539,8 +3548,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3592,7 +3601,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-10-23 18:32 Anusha Srivatsa
2018-10-23 18:44 ` Srivatsa, Anusha
2018-10-23 18:52 ` Rodrigo Vivi
0 siblings, 2 replies; 18+ messages in thread
From: Anusha Srivatsa @ 2018-10-23 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
v6: (Anusha Srivatsa)
- rebased.Use INTEL_GEN consistently.
- Simplify the code (Rodrigo)
v7: rebased. Change order according to platforms(Jyoti)
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
3 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index baac35f698f9..6691b9ee95db 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0e9a926fca04..529ff19a5e48 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
@@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5f5416eb9644..ef08313cf359 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ if (INTEL_GEN(dev_priv) < 11)
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
+ if (INTEL_GEN(dev_priv) >= 11) {
max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
+ } else if (IS_GEN9_LP(dev_priv)) {
+ max_dc = 1;
+ mask = DC_STATE_EN_DC9;
} else {
max_dc = 0;
mask = 0;
@@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
intel_csr_load_program(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 18:32 Anusha Srivatsa
@ 2018-10-23 18:44 ` Srivatsa, Anusha
2018-10-23 18:52 ` Rodrigo Vivi
1 sibling, 0 replies; 18+ messages in thread
From: Srivatsa, Anusha @ 2018-10-23 18:44 UTC (permalink / raw)
To: intel-gfx; +Cc: Vivi, Rodrigo
Rodrigo, this patch is tested by Jyoti. Can you review it?
Anusha
>-----Original Message-----
>From: Srivatsa, Anusha
>Sent: Tuesday, October 23, 2018 11:32 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Manna, Animesh <animesh.manna@intel.com>; Deak, Imre
><imre.deak@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Ausmus, James
><james.ausmus@intel.com>; Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Subject: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-
>off
>
>From: Animesh Manna <animesh.manna@intel.com>
>
>ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
>DC5/6 when appropriate.
>
>v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
>v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
>v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
>v5: (Anusha Srivatsa)
>- rebased against the latest upstream changes.
>
>v6: (Anusha Srivatsa)
>- rebased.Use INTEL_GEN consistently.
>- Simplify the code (Rodrigo)
>
>v7: rebased. Change order according to platforms(Jyoti)
>
>Cc: Imre Deak <imre.deak@intel.com>
>Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>Signed-off-by: James Ausmus <james.ausmus@intel.com>
>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> 3 files changed, 36 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>index baac35f698f9..6691b9ee95db 100644
>--- a/drivers/gpu/drm/i915/i915_drv.c
>+++ b/drivers/gpu/drm/i915/i915_drv.c
>@@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device
>*dev)
>
> intel_uncore_resume_early(dev_priv);
>
>- if (IS_GEN9_LP(dev_priv)) {
>+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@ -
>2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
>- if (IS_GEN9_LP(dev_priv)) {
>+ if (INTEL_GEN(dev_priv) >= 11) {
>+ icl_display_core_uninit(dev_priv);
>+ bxt_enable_dc9(dev_priv);
>+ } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@ -
>3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend,
>bios?\n");
>
>- if (IS_GEN9_LP(dev_priv)) {
>+ if (INTEL_GEN(dev_priv) >= 11) {
>+ bxt_disable_dc9(dev_priv);
>+ icl_display_core_init(dev_priv, true);
>+ if (dev_priv->csr.dmc_payload) {
>+ if (dev_priv->csr.allowed_dc_mask &
>+ DC_STATE_EN_UPTO_DC6)
>+ skl_enable_dc6(dev_priv);
>+ else if (dev_priv->csr.allowed_dc_mask &
>+ DC_STATE_EN_UPTO_DC5)
>+ gen9_enable_dc5(dev_priv);
>+ }
>+ } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>index 0e9a926fca04..529ff19a5e48 100644
>--- a/drivers/gpu/drm/i915/intel_drv.h
>+++ b/drivers/gpu/drm/i915/intel_drv.h
>@@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private
>*dev_priv); void bxt_disable_dc9(struct drm_i915_private *dev_priv); void
>gen9_enable_dc5(struct drm_i915_private *dev_priv); unsigned int
>skl_cdclk_get_vco(unsigned int freq);
>+void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config); void
>intel_dp_set_m_n(const struct intel_crtc_state *crtc_state, @@ -1989,6 +1990,8
>@@ int intel_power_domains_init(struct drm_i915_private *); void
>intel_power_domains_cleanup(struct drm_i915_private *dev_priv); void
>intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
>void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
>+void icl_display_core_init(struct drm_i915_private *dev_priv, bool
>+resume); void icl_display_core_uninit(struct drm_i915_private
>+*dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv); void
>intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
>diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
>b/drivers/gpu/drm/i915/intel_runtime_pm.c
>index 5f5416eb9644..ef08313cf359 100644
>--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>@@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private
>*dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
>- if (IS_GEN9_LP(dev_priv))
>+ if (INTEL_GEN(dev_priv) >= 11)
>+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
>+ else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
>@@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
>-
>- intel_power_sequencer_reset(dev_priv);
>+ if (INTEL_GEN(dev_priv) < 11)
>+ intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9); }
>
>@@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct
>drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
>-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
>+void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
>@@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct
>drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
>- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
>+ if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
>- mask = 0;
>- } else if (IS_GEN9_LP(dev_priv)) {
>- max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
>+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
>+ max_dc = 2;
>+ mask = 0;
>+ } else if (IS_GEN9_LP(dev_priv)) {
>+ max_dc = 1;
>+ mask = DC_STATE_EN_DC9;
> } else {
> max_dc = 0;
> mask = 0;
>@@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct
>drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
>-static void icl_display_core_init(struct drm_i915_private *dev_priv,
>- bool resume)
>+void icl_display_core_init(struct drm_i915_private *dev_priv,
>+ bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv-
>>power_domains;
> struct i915_power_well *well;
>@@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct
>drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
>-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
>+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv-
>>power_domains;
> struct i915_power_well *well;
>--
>2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 18:32 Anusha Srivatsa
2018-10-23 18:44 ` Srivatsa, Anusha
@ 2018-10-23 18:52 ` Rodrigo Vivi
2018-10-23 20:01 ` James Ausmus
1 sibling, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-10-23 18:52 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Tue, Oct 23, 2018 at 11:32:21AM -0700, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
>
> v6: (Anusha Srivatsa)
> - rebased.Use INTEL_GEN consistently.
> - Simplify the code (Rodrigo)
>
> v7: rebased. Change order according to platforms(Jyoti)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> 3 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index baac35f698f9..6691b9ee95db 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0e9a926fca04..529ff19a5e48 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 5f5416eb9644..ef08313cf359 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + if (INTEL_GEN(dev_priv) < 11)
> + intel_power_sequencer_reset(dev_priv);
I'm sorry if this was discussed already, but why is this only
needed on BXT?
Could we have a comment here or mention on commit message?
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> + mask = 0;
> + } else if (IS_GEN9_LP(dev_priv)) {
> + max_dc = 1;
> + mask = DC_STATE_EN_DC9;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 18:52 ` Rodrigo Vivi
@ 2018-10-23 20:01 ` James Ausmus
2018-10-23 20:34 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: James Ausmus @ 2018-10-23 20:01 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Oct 23, 2018 at 11:52:31AM -0700, Rodrigo Vivi wrote:
> On Tue, Oct 23, 2018 at 11:32:21AM -0700, Anusha Srivatsa wrote:
> > From: Animesh Manna <animesh.manna@intel.com>
> >
> > ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> > DC5/6 when appropriate.
> >
> > v2: (James Ausmus)
> > - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> > i915_drm_suspend_early
> > - Add DC9 to gen9_dc_mask for ICL
> > - Re-order GEN checks for newest platform first
> > - Use INTEL_GEN instead of INTEL_INFO->gen
> > - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> > - Consolidate GEN checks
> >
> > v3: (James Ausmus)
> > - Also allow DC6 for ICL (Imre, Art)
> > - Simplify !(GEN >= 11) to GEN < 11 (Imre)
> >
> > v4: (James Ausmus)
> > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > PPS regs are Always On
> > - Rebase against upstream changes
> >
> > v5: (Anusha Srivatsa)
> > - rebased against the latest upstream changes.
> >
> > v6: (Anusha Srivatsa)
> > - rebased.Use INTEL_GEN consistently.
> > - Simplify the code (Rodrigo)
> >
> > v7: rebased. Change order according to platforms(Jyoti)
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> > 3 files changed, 36 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index baac35f698f9..6691b9ee95db 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> >
> > intel_uncore_resume_early(dev_priv);
> >
> > - if (IS_GEN9_LP(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> > gen9_sanitize_dc_state(dev_priv);
> > bxt_disable_dc9(dev_priv);
> > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> > intel_uncore_suspend(dev_priv);
> >
> > ret = 0;
> > - if (IS_GEN9_LP(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > + icl_display_core_uninit(dev_priv);
> > + bxt_enable_dc9(dev_priv);
> > + } else if (IS_GEN9_LP(dev_priv)) {
> > bxt_display_core_uninit(dev_priv);
> > bxt_enable_dc9(dev_priv);
> > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> > if (intel_uncore_unclaimed_mmio(dev_priv))
> > DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
> >
> > - if (IS_GEN9_LP(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > + bxt_disable_dc9(dev_priv);
> > + icl_display_core_init(dev_priv, true);
> > + if (dev_priv->csr.dmc_payload) {
> > + if (dev_priv->csr.allowed_dc_mask &
> > + DC_STATE_EN_UPTO_DC6)
> > + skl_enable_dc6(dev_priv);
> > + else if (dev_priv->csr.allowed_dc_mask &
> > + DC_STATE_EN_UPTO_DC5)
> > + gen9_enable_dc5(dev_priv);
> > + }
> > + } else if (IS_GEN9_LP(dev_priv)) {
> > bxt_disable_dc9(dev_priv);
> > bxt_display_core_init(dev_priv, true);
> > if (dev_priv->csr.dmc_payload &&
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 0e9a926fca04..529ff19a5e48 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> > void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> > void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> > unsigned int skl_cdclk_get_vco(unsigned int freq);
> > +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> > void intel_dp_get_m_n(struct intel_crtc *crtc,
> > struct intel_crtc_state *pipe_config);
> > void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> > @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> > void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> > void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> > void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> > +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> > +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> > void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> > void intel_power_domains_disable(struct drm_i915_private *dev_priv);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 5f5416eb9644..ef08313cf359 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> > u32 mask;
> >
> > mask = DC_STATE_EN_UPTO_DC5;
> > - if (IS_GEN9_LP(dev_priv))
> > + if (INTEL_GEN(dev_priv) >= 11)
> > + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> > + else if (IS_GEN9_LP(dev_priv))
> > mask |= DC_STATE_EN_DC9;
> > else
> > mask |= DC_STATE_EN_UPTO_DC6;
> > @@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > assert_can_enable_dc9(dev_priv);
> >
> > DRM_DEBUG_KMS("Enabling DC9\n");
> > -
> > - intel_power_sequencer_reset(dev_priv);
> > + if (INTEL_GEN(dev_priv) < 11)
> > + intel_power_sequencer_reset(dev_priv);
>
> I'm sorry if this was discussed already, but why is this only
> needed on BXT?
>
> Could we have a comment here or mention on commit message?
It is in the commit message! ;)
<snip>
> > v4: (James Ausmus)
> > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > PPS regs are Always On
</snip>
Enough, or need more?
-James
>
> > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> > }
> >
> > @@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> > assert_csr_loaded(dev_priv);
> > }
> >
> > -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > {
> > assert_can_enable_dc6(dev_priv);
> >
> > @@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> > int requested_dc;
> > int max_dc;
> >
> > - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > max_dc = 2;
> > - mask = 0;
> > - } else if (IS_GEN9_LP(dev_priv)) {
> > - max_dc = 1;
> > /*
> > * DC9 has a separate HW flow from the rest of the DC states,
> > * not depending on the DMC firmware. It's needed by system
> > * suspend/resume, so allow it unconditionally.
> > */
> > mask = DC_STATE_EN_DC9;
> > + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> > + max_dc = 2;
> > + mask = 0;
> > + } else if (IS_GEN9_LP(dev_priv)) {
> > + max_dc = 1;
> > + mask = DC_STATE_EN_DC9;
> > } else {
> > max_dc = 0;
> > mask = 0;
> > @@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> > I915_WRITE(CHICKEN_MISC_2, val);
> > }
> >
> > -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > - bool resume)
> > +void icl_display_core_init(struct drm_i915_private *dev_priv,
> > + bool resume)
> > {
> > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > struct i915_power_well *well;
> > @@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > intel_csr_load_program(dev_priv);
> > }
> >
> > -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > {
> > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > struct i915_power_well *well;
> > --
> > 2.17.1
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 20:01 ` James Ausmus
@ 2018-10-23 20:34 ` Rodrigo Vivi
2018-10-23 20:38 ` James Ausmus
0 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-10-23 20:34 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
On Tue, Oct 23, 2018 at 01:01:11PM -0700, James Ausmus wrote:
> On Tue, Oct 23, 2018 at 11:52:31AM -0700, Rodrigo Vivi wrote:
> > On Tue, Oct 23, 2018 at 11:32:21AM -0700, Anusha Srivatsa wrote:
> > > From: Animesh Manna <animesh.manna@intel.com>
> > >
> > > ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> > > DC5/6 when appropriate.
> > >
> > > v2: (James Ausmus)
> > > - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> > > i915_drm_suspend_early
> > > - Add DC9 to gen9_dc_mask for ICL
> > > - Re-order GEN checks for newest platform first
> > > - Use INTEL_GEN instead of INTEL_INFO->gen
> > > - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> > > - Consolidate GEN checks
> > >
> > > v3: (James Ausmus)
> > > - Also allow DC6 for ICL (Imre, Art)
> > > - Simplify !(GEN >= 11) to GEN < 11 (Imre)
> > >
> > > v4: (James Ausmus)
> > > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > > PPS regs are Always On
> > > - Rebase against upstream changes
> > >
> > > v5: (Anusha Srivatsa)
> > > - rebased against the latest upstream changes.
> > >
> > > v6: (Anusha Srivatsa)
> > > - rebased.Use INTEL_GEN consistently.
> > > - Simplify the code (Rodrigo)
> > >
> > > v7: rebased. Change order according to platforms(Jyoti)
> > >
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> > > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> > > 3 files changed, 36 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index baac35f698f9..6691b9ee95db 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> > >
> > > intel_uncore_resume_early(dev_priv);
> > >
> > > - if (IS_GEN9_LP(dev_priv)) {
> > > + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> > > gen9_sanitize_dc_state(dev_priv);
> > > bxt_disable_dc9(dev_priv);
> > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> > > intel_uncore_suspend(dev_priv);
> > >
> > > ret = 0;
> > > - if (IS_GEN9_LP(dev_priv)) {
> > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > + icl_display_core_uninit(dev_priv);
> > > + bxt_enable_dc9(dev_priv);
> > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > bxt_display_core_uninit(dev_priv);
> > > bxt_enable_dc9(dev_priv);
> > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> > > if (intel_uncore_unclaimed_mmio(dev_priv))
> > > DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
> > >
> > > - if (IS_GEN9_LP(dev_priv)) {
> > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > + bxt_disable_dc9(dev_priv);
> > > + icl_display_core_init(dev_priv, true);
> > > + if (dev_priv->csr.dmc_payload) {
> > > + if (dev_priv->csr.allowed_dc_mask &
> > > + DC_STATE_EN_UPTO_DC6)
> > > + skl_enable_dc6(dev_priv);
> > > + else if (dev_priv->csr.allowed_dc_mask &
> > > + DC_STATE_EN_UPTO_DC5)
> > > + gen9_enable_dc5(dev_priv);
> > > + }
> > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > bxt_disable_dc9(dev_priv);
> > > bxt_display_core_init(dev_priv, true);
> > > if (dev_priv->csr.dmc_payload &&
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 0e9a926fca04..529ff19a5e48 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> > > void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> > > void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> > > unsigned int skl_cdclk_get_vco(unsigned int freq);
> > > +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> > > void intel_dp_get_m_n(struct intel_crtc *crtc,
> > > struct intel_crtc_state *pipe_config);
> > > void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> > > @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> > > void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> > > void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> > > void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> > > +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> > > +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> > > void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> > > void intel_power_domains_disable(struct drm_i915_private *dev_priv);
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 5f5416eb9644..ef08313cf359 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> > > u32 mask;
> > >
> > > mask = DC_STATE_EN_UPTO_DC5;
> > > - if (IS_GEN9_LP(dev_priv))
> > > + if (INTEL_GEN(dev_priv) >= 11)
> > > + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> > > + else if (IS_GEN9_LP(dev_priv))
> > > mask |= DC_STATE_EN_DC9;
> > > else
> > > mask |= DC_STATE_EN_UPTO_DC6;
> > > @@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > > assert_can_enable_dc9(dev_priv);
> > >
> > > DRM_DEBUG_KMS("Enabling DC9\n");
> > > -
> > > - intel_power_sequencer_reset(dev_priv);
> > > + if (INTEL_GEN(dev_priv) < 11)
> > > + intel_power_sequencer_reset(dev_priv);
> >
> > I'm sorry if this was discussed already, but why is this only
> > needed on BXT?
> >
> > Could we have a comment here or mention on commit message?
>
> It is in the commit message! ;)
ops... my bad. sorry.
I didn't read the revision history. :/
>
> <snip>
> > > v4: (James Ausmus)
> > > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > > PPS regs are Always On
Is this because it is gen11+ or because PPS regs on icl is on PCH and BXT
doesn't have it?
in other words: Is the gen check better here or should we use HAS_PCH_SPLIT ?
and is this a possible thing to happen on the future?
> </snip>
>
> Enough, or need more?
I believe a comment close to the if is more appropriated to remind
us what to do in the future. ;)
Thanks,
Rodrigo.
>
> -James
>
> >
> > > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> > > }
> > >
> > > @@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> > > assert_csr_loaded(dev_priv);
> > > }
> > >
> > > -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > > +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > > {
> > > assert_can_enable_dc6(dev_priv);
> > >
> > > @@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> > > int requested_dc;
> > > int max_dc;
> > >
> > > - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > max_dc = 2;
> > > - mask = 0;
> > > - } else if (IS_GEN9_LP(dev_priv)) {
> > > - max_dc = 1;
> > > /*
> > > * DC9 has a separate HW flow from the rest of the DC states,
> > > * not depending on the DMC firmware. It's needed by system
> > > * suspend/resume, so allow it unconditionally.
> > > */
> > > mask = DC_STATE_EN_DC9;
> > > + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> > > + max_dc = 2;
> > > + mask = 0;
> > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > + max_dc = 1;
> > > + mask = DC_STATE_EN_DC9;
> > > } else {
> > > max_dc = 0;
> > > mask = 0;
> > > @@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > I915_WRITE(CHICKEN_MISC_2, val);
> > > }
> > >
> > > -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > - bool resume)
> > > +void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > + bool resume)
> > > {
> > > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > struct i915_power_well *well;
> > > @@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > intel_csr_load_program(dev_priv);
> > > }
> > >
> > > -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > {
> > > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > struct i915_power_well *well;
> > > --
> > > 2.17.1
> > >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 20:34 ` Rodrigo Vivi
@ 2018-10-23 20:38 ` James Ausmus
0 siblings, 0 replies; 18+ messages in thread
From: James Ausmus @ 2018-10-23 20:38 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Oct 23, 2018 at 01:34:54PM -0700, Rodrigo Vivi wrote:
> On Tue, Oct 23, 2018 at 01:01:11PM -0700, James Ausmus wrote:
> > On Tue, Oct 23, 2018 at 11:52:31AM -0700, Rodrigo Vivi wrote:
> > > On Tue, Oct 23, 2018 at 11:32:21AM -0700, Anusha Srivatsa wrote:
> > > > From: Animesh Manna <animesh.manna@intel.com>
> > > >
> > > > ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> > > > DC5/6 when appropriate.
> > > >
> > > > v2: (James Ausmus)
> > > > - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> > > > i915_drm_suspend_early
> > > > - Add DC9 to gen9_dc_mask for ICL
> > > > - Re-order GEN checks for newest platform first
> > > > - Use INTEL_GEN instead of INTEL_INFO->gen
> > > > - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> > > > - Consolidate GEN checks
> > > >
> > > > v3: (James Ausmus)
> > > > - Also allow DC6 for ICL (Imre, Art)
> > > > - Simplify !(GEN >= 11) to GEN < 11 (Imre)
> > > >
> > > > v4: (James Ausmus)
> > > > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > > > PPS regs are Always On
> > > > - Rebase against upstream changes
> > > >
> > > > v5: (Anusha Srivatsa)
> > > > - rebased against the latest upstream changes.
> > > >
> > > > v6: (Anusha Srivatsa)
> > > > - rebased.Use INTEL_GEN consistently.
> > > > - Simplify the code (Rodrigo)
> > > >
> > > > v7: rebased. Change order according to platforms(Jyoti)
> > > >
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> > > > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> > > > 3 files changed, 36 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index baac35f698f9..6691b9ee95db 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> > > >
> > > > intel_uncore_resume_early(dev_priv);
> > > >
> > > > - if (IS_GEN9_LP(dev_priv)) {
> > > > + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> > > > gen9_sanitize_dc_state(dev_priv);
> > > > bxt_disable_dc9(dev_priv);
> > > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > > @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> > > > intel_uncore_suspend(dev_priv);
> > > >
> > > > ret = 0;
> > > > - if (IS_GEN9_LP(dev_priv)) {
> > > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > > + icl_display_core_uninit(dev_priv);
> > > > + bxt_enable_dc9(dev_priv);
> > > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > > bxt_display_core_uninit(dev_priv);
> > > > bxt_enable_dc9(dev_priv);
> > > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > > @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> > > > if (intel_uncore_unclaimed_mmio(dev_priv))
> > > > DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
> > > >
> > > > - if (IS_GEN9_LP(dev_priv)) {
> > > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > > + bxt_disable_dc9(dev_priv);
> > > > + icl_display_core_init(dev_priv, true);
> > > > + if (dev_priv->csr.dmc_payload) {
> > > > + if (dev_priv->csr.allowed_dc_mask &
> > > > + DC_STATE_EN_UPTO_DC6)
> > > > + skl_enable_dc6(dev_priv);
> > > > + else if (dev_priv->csr.allowed_dc_mask &
> > > > + DC_STATE_EN_UPTO_DC5)
> > > > + gen9_enable_dc5(dev_priv);
> > > > + }
> > > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > > bxt_disable_dc9(dev_priv);
> > > > bxt_display_core_init(dev_priv, true);
> > > > if (dev_priv->csr.dmc_payload &&
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 0e9a926fca04..529ff19a5e48 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> > > > void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> > > > void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> > > > unsigned int skl_cdclk_get_vco(unsigned int freq);
> > > > +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> > > > void intel_dp_get_m_n(struct intel_crtc *crtc,
> > > > struct intel_crtc_state *pipe_config);
> > > > void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> > > > @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> > > > void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> > > > void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> > > > void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> > > > +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> > > > +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> > > > void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> > > > void intel_power_domains_disable(struct drm_i915_private *dev_priv);
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > index 5f5416eb9644..ef08313cf359 100644
> > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> > > > u32 mask;
> > > >
> > > > mask = DC_STATE_EN_UPTO_DC5;
> > > > - if (IS_GEN9_LP(dev_priv))
> > > > + if (INTEL_GEN(dev_priv) >= 11)
> > > > + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> > > > + else if (IS_GEN9_LP(dev_priv))
> > > > mask |= DC_STATE_EN_DC9;
> > > > else
> > > > mask |= DC_STATE_EN_UPTO_DC6;
> > > > @@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > > > assert_can_enable_dc9(dev_priv);
> > > >
> > > > DRM_DEBUG_KMS("Enabling DC9\n");
> > > > -
> > > > - intel_power_sequencer_reset(dev_priv);
> > > > + if (INTEL_GEN(dev_priv) < 11)
> > > > + intel_power_sequencer_reset(dev_priv);
> > >
> > > I'm sorry if this was discussed already, but why is this only
> > > needed on BXT?
> > >
> > > Could we have a comment here or mention on commit message?
> >
> > It is in the commit message! ;)
>
> ops... my bad. sorry.
>
> I didn't read the revision history. :/
>
> >
> > <snip>
> > > > v4: (James Ausmus)
> > > > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > > > PPS regs are Always On
>
> Is this because it is gen11+ or because PPS regs on icl is on PCH and BXT
> doesn't have it?
>
> in other words: Is the gen check better here or should we use HAS_PCH_SPLIT ?
>
> and is this a possible thing to happen on the future?
It is due to the PCH split - good call. I saw no sign in BSpec of this
changing in the future, though, so not sure whether GEN >= 11, or
HAS_PCH_SPLIT is the better check.
>
> > </snip>
> >
> > Enough, or need more?
>
> I believe a comment close to the if is more appropriated to remind
> us what to do in the future. ;)
Agreed. :)
Thanks!
-James
>
> Thanks,
> Rodrigo.
>
> >
> > -James
> >
> > >
> > > > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> > > > }
> > > >
> > > > @@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> > > > assert_csr_loaded(dev_priv);
> > > > }
> > > >
> > > > -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > > > +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > > > {
> > > > assert_can_enable_dc6(dev_priv);
> > > >
> > > > @@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> > > > int requested_dc;
> > > > int max_dc;
> > > >
> > > > - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> > > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > > max_dc = 2;
> > > > - mask = 0;
> > > > - } else if (IS_GEN9_LP(dev_priv)) {
> > > > - max_dc = 1;
> > > > /*
> > > > * DC9 has a separate HW flow from the rest of the DC states,
> > > > * not depending on the DMC firmware. It's needed by system
> > > > * suspend/resume, so allow it unconditionally.
> > > > */
> > > > mask = DC_STATE_EN_DC9;
> > > > + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> > > > + max_dc = 2;
> > > > + mask = 0;
> > > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > > + max_dc = 1;
> > > > + mask = DC_STATE_EN_DC9;
> > > > } else {
> > > > max_dc = 0;
> > > > mask = 0;
> > > > @@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > > I915_WRITE(CHICKEN_MISC_2, val);
> > > > }
> > > >
> > > > -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > > - bool resume)
> > > > +void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > > + bool resume)
> > > > {
> > > > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > > struct i915_power_well *well;
> > > > @@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > > intel_csr_load_program(dev_priv);
> > > > }
> > > >
> > > > -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > > +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > > {
> > > > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > > struct i915_power_well *well;
> > > > --
> > > > 2.17.1
> > > >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-09-19 20:06 Anusha Srivatsa
2018-10-23 10:01 ` Yadav, Jyoti R
0 siblings, 1 reply; 18+ messages in thread
From: Anusha Srivatsa @ 2018-09-19 20:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
v6: (Anusha Srivatsa)
- rebased.Use INTEL_GEN consistently.
- Simplify the code (Rodrigo)
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
3 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 44e2c0f5ec50..036f33fa5626 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2155,7 +2155,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2922,7 +2922,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -3007,7 +3010,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf1c38728a59..f0385fe5bb15 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
@@ -1966,6 +1967,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 0fdabce647ab..5271ca9418de 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ if (INTEL_GEN(dev_priv) < 11)
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2969,17 +2971,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
+ if (INTEL_GEN(dev_priv) >= 11) {
max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN9_LP(dev_priv)) {
+ max_dc = 1;
+ mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
} else {
max_dc = 0;
mask = 0;
@@ -3514,8 +3519,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3569,7 +3574,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
intel_csr_load_program(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-09-19 20:06 Anusha Srivatsa
@ 2018-10-23 10:01 ` Yadav, Jyoti R
0 siblings, 0 replies; 18+ messages in thread
From: Yadav, Jyoti R @ 2018-10-23 10:01 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
On 9/20/2018 1:36 AM, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
>
> v6: (Anusha Srivatsa)
> - rebased.Use INTEL_GEN consistently.
> - Simplify the code (Rodrigo)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> 3 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 44e2c0f5ec50..036f33fa5626 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2155,7 +2155,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2922,7 +2922,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -3007,7 +3010,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf1c38728a59..f0385fe5bb15 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> @@ -1966,6 +1967,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 0fdabce647ab..5271ca9418de 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + if (INTEL_GEN(dev_priv) < 11)
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2969,17 +2971,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN9_LP(dev_priv)) {
> + max_dc = 1;
> + mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> +
Please double confirm that Gen checks are in order. I could see see Gen
9 coming before Gen10. Usually we prefer to put new Gens first.
Also i tested this patch on ICL B1 HW and could see that display is
entering into Dc9 when DMC FW is loaded.
With this we can add "Tested-by" tag with my name for this patch.
> mask = 0;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3514,8 +3519,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3569,7 +3574,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-09-13 19:31 Anusha Srivatsa
2018-09-13 20:13 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: Anusha Srivatsa @ 2018-09-13 19:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 ++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 29 +++++++++++++++----------
3 files changed, 37 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2ddf8538cb47..86a83e0a7ef2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1855,7 +1855,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2622,7 +2622,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (IS_ICELAKE(dev_priv)) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2707,7 +2710,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (IS_ICELAKE(dev_priv)) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf1c38728a59..f0385fe5bb15 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
@@ -1966,6 +1967,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 480dadb1047b..3e2c936217f8 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ if (INTEL_GEN(dev_priv) < 11)
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2968,17 +2970,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
- max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11)
+ max_dc = 2;
+ else
+ max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
} else {
max_dc = 0;
mask = 0;
@@ -3513,8 +3518,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3565,7 +3570,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
icl_mbus_init(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-09-13 19:31 Anusha Srivatsa
@ 2018-09-13 20:13 ` Rodrigo Vivi
2018-09-17 18:45 ` Srivatsa, Anusha
0 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-09-13 20:13 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Thu, Sep 13, 2018 at 12:31:09PM -0700, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
First concern with this patch is regarding the tests...
How is this getting tested? Are you able to see DC6 and DC9?
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 ++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 29 +++++++++++++++----------
> 3 files changed, 37 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2ddf8538cb47..86a83e0a7ef2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1855,7 +1855,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2622,7 +2622,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (IS_ICELAKE(dev_priv)) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2707,7 +2710,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (IS_ICELAKE(dev_priv)) {
commit message mention the use of INTEL_GEN instead of ICELAKE,
but it seems we are missing some replacements here....
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf1c38728a59..f0385fe5bb15 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> @@ -1966,6 +1967,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 480dadb1047b..3e2c936217f8 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + if (INTEL_GEN(dev_priv) < 11)
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2968,17 +2970,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> - max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11)
> + max_dc = 2;
This is supper confusing... Could we create a new full block for INTEL_GEN >= 11?
> + else
> + max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> + mask = 0;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3513,8 +3518,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3565,7 +3570,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> icl_mbus_init(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-09-13 20:13 ` Rodrigo Vivi
@ 2018-09-17 18:45 ` Srivatsa, Anusha
0 siblings, 0 replies; 18+ messages in thread
From: Srivatsa, Anusha @ 2018-09-17 18:45 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx
>-----Original Message-----
>From: Vivi, Rodrigo
>Sent: Thursday, September 13, 2018 1:14 PM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Manna, Animesh
><animesh.manna@intel.com>; Deak, Imre <imre.deak@intel.com>; Ausmus,
>James <james.ausmus@intel.com>
>Subject: Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during
>screen-off
>
>On Thu, Sep 13, 2018 at 12:31:09PM -0700, Anusha Srivatsa wrote:
>> From: Animesh Manna <animesh.manna@intel.com>
>>
>> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and
>> enable
>> DC5/6 when appropriate.
>>
>> v2: (James Ausmus)
>> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
>> i915_drm_suspend_early
>> - Add DC9 to gen9_dc_mask for ICL
>> - Re-order GEN checks for newest platform first
>> - Use INTEL_GEN instead of INTEL_INFO->gen
>> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
>> - Consolidate GEN checks
>>
>> v3: (James Ausmus)
>> - Also allow DC6 for ICL (Imre, Art)
>> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>>
>> v4: (James Ausmus)
>> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
>> PPS regs are Always On
>> - Rebase against upstream changes
>>
>> v5: (Anusha Srivatsa)
>> - rebased against the latest upstream changes.
>
>First concern with this patch is regarding the tests...
>How is this getting tested? Are you able to see DC6 and DC9?
>
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> Signed-off-by: James Ausmus <james.ausmus@intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 20 ++++++++++++++---
>> drivers/gpu/drm/i915/intel_drv.h | 3 +++
>> drivers/gpu/drm/i915/intel_runtime_pm.c | 29
>> +++++++++++++++----------
>> 3 files changed, 37 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c index 2ddf8538cb47..86a83e0a7ef2
>> 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1855,7 +1855,7 @@ static int i915_drm_resume_early(struct
>> drm_device *dev)
>>
>> intel_uncore_resume_early(dev_priv);
>>
>> - if (IS_GEN9_LP(dev_priv)) {
>> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
>> gen9_sanitize_dc_state(dev_priv);
>> bxt_disable_dc9(dev_priv);
>> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@
>> -2622,7 +2622,10 @@ static int intel_runtime_suspend(struct device *kdev)
>> intel_uncore_suspend(dev_priv);
>>
>> ret = 0;
>> - if (IS_GEN9_LP(dev_priv)) {
>> + if (IS_ICELAKE(dev_priv)) {
>> + icl_display_core_uninit(dev_priv);
>> + bxt_enable_dc9(dev_priv);
>> + } else if (IS_GEN9_LP(dev_priv)) {
>> bxt_display_core_uninit(dev_priv);
>> bxt_enable_dc9(dev_priv);
>> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@
>> -2707,7 +2710,18 @@ static int intel_runtime_resume(struct device *kdev)
>> if (intel_uncore_unclaimed_mmio(dev_priv))
>> DRM_DEBUG_DRIVER("Unclaimed access during suspend,
>bios?\n");
>>
>> - if (IS_GEN9_LP(dev_priv)) {
>> + if (IS_ICELAKE(dev_priv)) {
>
>commit message mention the use of INTEL_GEN instead of ICELAKE, but it seems
>we are missing some replacements here....
Actually double checked with the internal version of this patch, seems like IS_ICELAKE was what was used in the code...
Maybe change the commit log?
>
>> + bxt_disable_dc9(dev_priv);
>> + icl_display_core_init(dev_priv, true);
>> + if (dev_priv->csr.dmc_payload) {
>> + if (dev_priv->csr.allowed_dc_mask &
>> + DC_STATE_EN_UPTO_DC6)
>> + skl_enable_dc6(dev_priv);
>> + else if (dev_priv->csr.allowed_dc_mask &
>> + DC_STATE_EN_UPTO_DC5)
>> + gen9_enable_dc5(dev_priv);
>> + }
>> + } else if (IS_GEN9_LP(dev_priv)) {
>> bxt_disable_dc9(dev_priv);
>> bxt_display_core_init(dev_priv, true);
>> if (dev_priv->csr.dmc_payload &&
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index bf1c38728a59..f0385fe5bb15 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private
>> *dev_priv); void bxt_disable_dc9(struct drm_i915_private *dev_priv);
>> void gen9_enable_dc5(struct drm_i915_private *dev_priv); unsigned int
>> skl_cdclk_get_vco(unsigned int freq);
>> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
>> void intel_dp_get_m_n(struct intel_crtc *crtc,
>> struct intel_crtc_state *pipe_config); void
>> intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n); @@
>> -1966,6 +1967,8 @@ int intel_power_domains_init(struct
>> drm_i915_private *); void intel_power_domains_cleanup(struct
>> drm_i915_private *dev_priv); void intel_power_domains_init_hw(struct
>> drm_i915_private *dev_priv, bool resume); void
>> intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
>> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool
>> +resume); void icl_display_core_uninit(struct drm_i915_private
>> +*dev_priv);
>> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
>> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>>
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index 480dadb1047b..3e2c936217f8 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private
>*dev_priv)
>> u32 mask;
>>
>> mask = DC_STATE_EN_UPTO_DC5;
>> - if (IS_GEN9_LP(dev_priv))
>> + if (INTEL_GEN(dev_priv) >= 11)
>> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
>> + else if (IS_GEN9_LP(dev_priv))
>> mask |= DC_STATE_EN_DC9;
>> else
>> mask |= DC_STATE_EN_UPTO_DC6;
>> @@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private
>*dev_priv)
>> assert_can_enable_dc9(dev_priv);
>>
>> DRM_DEBUG_KMS("Enabling DC9\n");
>> -
>> - intel_power_sequencer_reset(dev_priv);
>> + if (INTEL_GEN(dev_priv) < 11)
>> + intel_power_sequencer_reset(dev_priv);
>> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9); }
>>
>> @@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct
>drm_i915_private *dev_priv)
>> assert_csr_loaded(dev_priv);
>> }
>>
>> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
>> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
>> {
>> assert_can_enable_dc6(dev_priv);
>>
>> @@ -2968,17 +2970,20 @@ static uint32_t get_allowed_dc_mask(const struct
>drm_i915_private *dev_priv,
>> int requested_dc;
>> int max_dc;
>>
>> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
>> - max_dc = 2;
>> - mask = 0;
>> - } else if (IS_GEN9_LP(dev_priv)) {
>> - max_dc = 1;
>> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
>> + if (INTEL_GEN(dev_priv) >= 11)
>> + max_dc = 2;
>
>This is supper confusing... Could we create a new full block for INTEL_GEN >= 11?
Yes...that sounds good.
Anusha
>
>> + else
>> + max_dc = 1;
>> /*
>> * DC9 has a separate HW flow from the rest of the DC states,
>> * not depending on the DMC firmware. It's needed by system
>> * suspend/resume, so allow it unconditionally.
>> */
>> mask = DC_STATE_EN_DC9;
>> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
>> + max_dc = 2;
>> + mask = 0;
>> } else {
>> max_dc = 0;
>> mask = 0;
>> @@ -3513,8 +3518,8 @@ static void cnl_display_core_uninit(struct
>drm_i915_private *dev_priv)
>> I915_WRITE(CHICKEN_MISC_2, val);
>> }
>>
>> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
>> - bool resume)
>> +void icl_display_core_init(struct drm_i915_private *dev_priv,
>> + bool resume)
>> {
>> struct i915_power_domains *power_domains = &dev_priv-
>>power_domains;
>> struct i915_power_well *well;
>> @@ -3565,7 +3570,7 @@ static void icl_display_core_init(struct
>drm_i915_private *dev_priv,
>> icl_mbus_init(dev_priv);
>> }
>>
>> -static void icl_display_core_uninit(struct drm_i915_private
>> *dev_priv)
>> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
>> {
>> struct i915_power_domains *power_domains = &dev_priv-
>>power_domains;
>> struct i915_power_well *well;
>> --
>> 2.17.1
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-09-10 19:10 Anusha Srivatsa
0 siblings, 0 replies; 18+ messages in thread
From: Anusha Srivatsa @ 2018-09-10 19:10 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
3 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5dd7fc582e6f..2b1c1b9e0077 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1855,7 +1855,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2622,7 +2622,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (IS_ICELAKE(dev_priv)) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2707,7 +2710,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (IS_ICELAKE(dev_priv)) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f5731215210a..3a06f58a3459 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1625,6 +1625,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
@@ -1962,6 +1963,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 480dadb1047b..33ad0b751576 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ if (INTEL_GEN(dev_priv) < 11)
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2968,17 +2970,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
- max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11)
+ max_dc = 2;
+ else
+ max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
} else {
max_dc = 0;
mask = 0;
@@ -3513,7 +3518,7 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
+void icl_display_core_init(struct drm_i915_private *dev_priv,
bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
@@ -3565,7 +3570,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
icl_mbus_init(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-10-30 17:56 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-29 22:14 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
2018-10-29 23:53 ` ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev6) Patchwork
2018-10-30 7:12 ` ✓ Fi.CI.IGT: " Patchwork
2018-10-30 17:56 ` [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Rodrigo Vivi
-- strict thread matches above, loose matches on Subject: below --
2018-10-24 23:01 Anusha Srivatsa
2018-10-24 23:22 ` Rodrigo Vivi
2018-10-23 18:32 Anusha Srivatsa
2018-10-23 18:44 ` Srivatsa, Anusha
2018-10-23 18:52 ` Rodrigo Vivi
2018-10-23 20:01 ` James Ausmus
2018-10-23 20:34 ` Rodrigo Vivi
2018-10-23 20:38 ` James Ausmus
2018-09-19 20:06 Anusha Srivatsa
2018-10-23 10:01 ` Yadav, Jyoti R
2018-09-13 19:31 Anusha Srivatsa
2018-09-13 20:13 ` Rodrigo Vivi
2018-09-17 18:45 ` Srivatsa, Anusha
2018-09-10 19:10 Anusha Srivatsa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.