Skip to content

Commit 0e6e0be

Browse files
ickleickle
committed
drm/i915: Markup paired operations on display power domains
The majority of runtime-pm operations are bounded and scoped within a function; these are easy to verify that the wakeref are handled correctly. We can employ the compiler to help us, and reduce the number of wakerefs tracked when debugging, by passing around cookies provided by the various rpm_get functions to their rpm_put counterpart. This makes the pairing explicit, and given the required wakeref cookie the compiler can verify that we pass an initialised value to the rpm_put (quite handy for double checking error paths). Signed-off-by: Chris Wilson <[email protected]> Cc: Jani Nikula <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent d4225a5 commit 0e6e0be

23 files changed

+347
-190
lines changed

‎drivers/gpu/drm/i915/i915_debugfs.c

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -626,10 +626,12 @@ static void gen8_display_interrupt_info(struct seq_file *m)
626626

627627
for_each_pipe(dev_priv, pipe) {
628628
enum intel_display_power_domain power_domain;
629+
intel_wakeref_t wakeref;
629630

630631
power_domain = POWER_DOMAIN_PIPE(pipe);
631-
if (!intel_display_power_get_if_enabled(dev_priv,
632-
power_domain)) {
632+
wakeref = intel_display_power_get_if_enabled(dev_priv,
633+
power_domain);
634+
if (!wakeref) {
633635
seq_printf(m, "Pipe %c power disabled\n",
634636
pipe_name(pipe));
635637
continue;
@@ -644,7 +646,7 @@ static void gen8_display_interrupt_info(struct seq_file *m)
644646
pipe_name(pipe),
645647
I915_READ(GEN8_DE_PIPE_IER(pipe)));
646648

647-
intel_display_power_put(dev_priv, power_domain);
649+
intel_display_power_put(dev_priv, power_domain, wakeref);
648650
}
649651

650652
seq_printf(m, "Display Engine port interrupt mask:\t%08x\n",
@@ -680,6 +682,8 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
680682
wakeref = intel_runtime_pm_get(dev_priv);
681683

682684
if (IS_CHERRYVIEW(dev_priv)) {
685+
intel_wakeref_t pref;
686+
683687
seq_printf(m, "Master Interrupt Control:\t%08x\n",
684688
I915_READ(GEN8_MASTER_IRQ));
685689

@@ -695,8 +699,9 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
695699
enum intel_display_power_domain power_domain;
696700

697701
power_domain = POWER_DOMAIN_PIPE(pipe);
698-
if (!intel_display_power_get_if_enabled(dev_priv,
699-
power_domain)) {
702+
pref = intel_display_power_get_if_enabled(dev_priv,
703+
power_domain);
704+
if (!pref) {
700705
seq_printf(m, "Pipe %c power disabled\n",
701706
pipe_name(pipe));
702707
continue;
@@ -706,17 +711,17 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
706711
pipe_name(pipe),
707712
I915_READ(PIPESTAT(pipe)));
708713

709-
intel_display_power_put(dev_priv, power_domain);
714+
intel_display_power_put(dev_priv, power_domain, pref);
710715
}
711716

712-
intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
717+
pref = intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
713718
seq_printf(m, "Port hotplug:\t%08x\n",
714719
I915_READ(PORT_HOTPLUG_EN));
715720
seq_printf(m, "DPFLIPSTAT:\t%08x\n",
716721
I915_READ(VLV_DPFLIPSTAT));
717722
seq_printf(m, "DPINVGTT:\t%08x\n",
718723
I915_READ(DPINVGTT));
719-
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
724+
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT, pref);
720725

721726
for (i = 0; i < 4; i++) {
722727
seq_printf(m, "GT Interrupt IMR %d:\t%08x\n",
@@ -779,10 +784,12 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
779784
I915_READ(VLV_IMR));
780785
for_each_pipe(dev_priv, pipe) {
781786
enum intel_display_power_domain power_domain;
787+
intel_wakeref_t pref;
782788

783789
power_domain = POWER_DOMAIN_PIPE(pipe);
784-
if (!intel_display_power_get_if_enabled(dev_priv,
785-
power_domain)) {
790+
pref = intel_display_power_get_if_enabled(dev_priv,
791+
power_domain);
792+
if (!pref) {
786793
seq_printf(m, "Pipe %c power disabled\n",
787794
pipe_name(pipe));
788795
continue;
@@ -791,7 +798,7 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
791798
seq_printf(m, "Pipe %c stat:\t%08x\n",
792799
pipe_name(pipe),
793800
I915_READ(PIPESTAT(pipe)));
794-
intel_display_power_put(dev_priv, power_domain);
801+
intel_display_power_put(dev_priv, power_domain, pref);
795802
}
796803

797804
seq_printf(m, "Master IER:\t%08x\n",
@@ -1709,8 +1716,7 @@ static int i915_sr_status(struct seq_file *m, void *unused)
17091716
intel_wakeref_t wakeref;
17101717
bool sr_enabled = false;
17111718

1712-
wakeref = intel_runtime_pm_get(dev_priv);
1713-
intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
1719+
wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
17141720

17151721
if (INTEL_GEN(dev_priv) >= 9)
17161722
/* no global SR status; inspect per-plane WM */;
@@ -1726,8 +1732,7 @@ static int i915_sr_status(struct seq_file *m, void *unused)
17261732
else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
17271733
sr_enabled = I915_READ(FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
17281734

1729-
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
1730-
intel_runtime_pm_put(dev_priv, wakeref);
1735+
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT, wakeref);
17311736

17321737
seq_printf(m, "self-refresh: %s\n", enableddisabled(sr_enabled));
17331738

‎drivers/gpu/drm/i915/i915_drv.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ struct intel_csr {
344344
uint32_t mmiodata[8];
345345
uint32_t dc_state;
346346
uint32_t allowed_dc_mask;
347+
intel_wakeref_t wakeref;
347348
};
348349

349350
enum i915_cache_level {
@@ -1982,6 +1983,7 @@ struct drm_i915_private {
19821983
* is a slight delay before we do so.
19831984
*/
19841985
intel_wakeref_t awake;
1986+
intel_wakeref_t power;
19851987

19861988
/**
19871989
* The number of times we have woken up.

‎drivers/gpu/drm/i915/i915_gem.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
176176
if (INTEL_GEN(i915) >= 6)
177177
gen6_rps_idle(i915);
178178

179-
intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ);
179+
intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ, i915->gt.power);
180180

181181
intel_runtime_pm_put(i915, wakeref);
182182

@@ -221,7 +221,7 @@ void i915_gem_unpark(struct drm_i915_private *i915)
221221
* Work around it by grabbing a GT IRQ power domain whilst there is any
222222
* GT activity, preventing any DC state transitions.
223223
*/
224-
intel_display_power_get(i915, POWER_DOMAIN_GT_IRQ);
224+
i915->gt.power = intel_display_power_get(i915, POWER_DOMAIN_GT_IRQ);
225225

226226
if (unlikely(++i915->gt.epoch == 0)) /* keep 0 as invalid */
227227
i915->gt.epoch = 1;

‎drivers/gpu/drm/i915/icl_dsi.c

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -337,9 +337,11 @@ static void gen11_dsi_enable_io_power(struct intel_encoder *encoder)
337337
}
338338

339339
for_each_dsi_port(port, intel_dsi->ports) {
340-
intel_display_power_get(dev_priv, port == PORT_A ?
341-
POWER_DOMAIN_PORT_DDI_A_IO :
342-
POWER_DOMAIN_PORT_DDI_B_IO);
340+
intel_dsi->io_wakeref[port] =
341+
intel_display_power_get(dev_priv,
342+
port == PORT_A ?
343+
POWER_DOMAIN_PORT_DDI_A_IO :
344+
POWER_DOMAIN_PORT_DDI_B_IO);
343345
}
344346
}
345347

@@ -1125,10 +1127,18 @@ static void gen11_dsi_disable_io_power(struct intel_encoder *encoder)
11251127
enum port port;
11261128
u32 tmp;
11271129

1128-
intel_display_power_put(dev_priv, POWER_DOMAIN_PORT_DDI_A_IO);
1129-
1130-
if (intel_dsi->dual_link)
1131-
intel_display_power_put(dev_priv, POWER_DOMAIN_PORT_DDI_B_IO);
1130+
for_each_dsi_port(port, intel_dsi->ports) {
1131+
intel_wakeref_t wakeref;
1132+
1133+
wakeref = fetch_and_zero(&intel_dsi->io_wakeref[port]);
1134+
if (wakeref) {
1135+
intel_display_power_put(dev_priv,
1136+
port == PORT_A ?
1137+
POWER_DOMAIN_PORT_DDI_A_IO :
1138+
POWER_DOMAIN_PORT_DDI_B_IO,
1139+
wakeref);
1140+
}
1141+
}
11321142

11331143
/* set mode to DDI */
11341144
for_each_dsi_port(port, intel_dsi->ports) {
@@ -1229,13 +1239,15 @@ static bool gen11_dsi_get_hw_state(struct intel_encoder *encoder,
12291239
{
12301240
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
12311241
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
1232-
u32 tmp;
1233-
enum port port;
12341242
enum transcoder dsi_trans;
1243+
intel_wakeref_t wakeref;
1244+
enum port port;
12351245
bool ret = false;
1246+
u32 tmp;
12361247

1237-
if (!intel_display_power_get_if_enabled(dev_priv,
1238-
encoder->power_domain))
1248+
wakeref = intel_display_power_get_if_enabled(dev_priv,
1249+
encoder->power_domain);
1250+
if (!wakeref)
12391251
return false;
12401252

12411253
for_each_dsi_port(port, intel_dsi->ports) {
@@ -1260,7 +1272,7 @@ static bool gen11_dsi_get_hw_state(struct intel_encoder *encoder,
12601272
ret = tmp & PIPECONF_ENABLE;
12611273
}
12621274
out:
1263-
intel_display_power_put(dev_priv, encoder->power_domain);
1275+
intel_display_power_put(dev_priv, encoder->power_domain, wakeref);
12641276
return ret;
12651277
}
12661278

‎drivers/gpu/drm/i915/intel_audio.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,8 @@ static void i915_audio_component_get_power(struct device *kdev)
748748

749749
static void i915_audio_component_put_power(struct device *kdev)
750750
{
751-
intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
751+
intel_display_power_put_unchecked(kdev_to_i915(kdev),
752+
POWER_DOMAIN_AUDIO);
752753
}
753754

754755
static void i915_audio_component_codec_wake_override(struct device *kdev,

‎drivers/gpu/drm/i915/intel_cdclk.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,7 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
520520
{
521521
int cdclk = cdclk_state->cdclk;
522522
u32 val, cmd = cdclk_state->voltage_level;
523+
intel_wakeref_t wakeref;
523524

524525
switch (cdclk) {
525526
case 400000:
@@ -539,7 +540,7 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
539540
* a system suspend. So grab the PIPE-A domain, which covers
540541
* the HW blocks needed for the following programming.
541542
*/
542-
intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
543+
wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
543544

544545
mutex_lock(&dev_priv->pcu_lock);
545546
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
@@ -593,14 +594,15 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
593594

594595
vlv_program_pfi_credits(dev_priv);
595596

596-
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
597+
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A, wakeref);
597598
}
598599

599600
static void chv_set_cdclk(struct drm_i915_private *dev_priv,
600601
const struct intel_cdclk_state *cdclk_state)
601602
{
602603
int cdclk = cdclk_state->cdclk;
603604
u32 val, cmd = cdclk_state->voltage_level;
605+
intel_wakeref_t wakeref;
604606

605607
switch (cdclk) {
606608
case 333333:
@@ -619,7 +621,7 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv,
619621
* a system suspend. So grab the PIPE-A domain, which covers
620622
* the HW blocks needed for the following programming.
621623
*/
622-
intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
624+
wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
623625

624626
mutex_lock(&dev_priv->pcu_lock);
625627
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
@@ -637,7 +639,7 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv,
637639

638640
vlv_program_pfi_credits(dev_priv);
639641

640-
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
642+
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A, wakeref);
641643
}
642644

643645
static int bdw_calc_cdclk(int min_cdclk)

‎drivers/gpu/drm/i915/intel_crt.c

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,17 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
8383
{
8484
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
8585
struct intel_crt *crt = intel_encoder_to_crt(encoder);
86+
intel_wakeref_t wakeref;
8687
bool ret;
8788

88-
if (!intel_display_power_get_if_enabled(dev_priv,
89-
encoder->power_domain))
89+
wakeref = intel_display_power_get_if_enabled(dev_priv,
90+
encoder->power_domain);
91+
if (!wakeref)
9092
return false;
9193

9294
ret = intel_crt_port_enabled(dev_priv, crt->adpa_reg, pipe);
9395

94-
intel_display_power_put(dev_priv, encoder->power_domain);
96+
intel_display_power_put(dev_priv, encoder->power_domain, wakeref);
9597

9698
return ret;
9799
}
@@ -776,6 +778,7 @@ intel_crt_detect(struct drm_connector *connector,
776778
struct drm_i915_private *dev_priv = to_i915(connector->dev);
777779
struct intel_crt *crt = intel_attached_crt(connector);
778780
struct intel_encoder *intel_encoder = &crt->base;
781+
intel_wakeref_t wakeref;
779782
int status, ret;
780783
struct intel_load_detect_pipe tmp;
781784

@@ -784,15 +787,17 @@ intel_crt_detect(struct drm_connector *connector,
784787
force);
785788

786789
if (i915_modparams.load_detect_test) {
787-
intel_display_power_get(dev_priv, intel_encoder->power_domain);
790+
wakeref = intel_display_power_get(dev_priv,
791+
intel_encoder->power_domain);
788792
goto load_detect;
789793
}
790794

791795
/* Skip machines without VGA that falsely report hotplug events */
792796
if (dmi_check_system(intel_spurious_crt_detect))
793797
return connector_status_disconnected;
794798

795-
intel_display_power_get(dev_priv, intel_encoder->power_domain);
799+
wakeref = intel_display_power_get(dev_priv,
800+
intel_encoder->power_domain);
796801

797802
if (I915_HAS_HOTPLUG(dev_priv)) {
798803
/* We can not rely on the HPD pin always being correctly wired
@@ -847,7 +852,7 @@ intel_crt_detect(struct drm_connector *connector,
847852
}
848853

849854
out:
850-
intel_display_power_put(dev_priv, intel_encoder->power_domain);
855+
intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
851856
return status;
852857
}
853858

@@ -857,10 +862,12 @@ static int intel_crt_get_modes(struct drm_connector *connector)
857862
struct drm_i915_private *dev_priv = to_i915(dev);
858863
struct intel_crt *crt = intel_attached_crt(connector);
859864
struct intel_encoder *intel_encoder = &crt->base;
860-
int ret;
865+
intel_wakeref_t wakeref;
861866
struct i2c_adapter *i2c;
867+
int ret;
862868

863-
intel_display_power_get(dev_priv, intel_encoder->power_domain);
869+
wakeref = intel_display_power_get(dev_priv,
870+
intel_encoder->power_domain);
864871

865872
i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin);
866873
ret = intel_crt_ddc_get_modes(connector, i2c);
@@ -872,7 +879,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
872879
ret = intel_crt_ddc_get_modes(connector, i2c);
873880

874881
out:
875-
intel_display_power_put(dev_priv, intel_encoder->power_domain);
882+
intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
876883

877884
return ret;
878885
}

0 commit comments

Comments
 (0)