All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions
@ 2015-12-31 12:45 Gabriel Feceoru
  2015-12-31 13:20 ` ✗ failure: Fi.CI.BAT Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Gabriel Feceoru @ 2015-12-31 12:45 UTC (permalink / raw)
  To: intel-gfx

This gets rid of errors like:

[  906.286213] ------------[ cut here ]------------
[  906.286233] WARNING: CPU: 0 PID: 12252 at drivers/gpu/drm/i915/intel_drv.h:1457 gen6_read32+0x1ca/0x1e0 [i915]()
[  906.286234] RPM wakelock ref not held during HW access
[  906.286235] Modules linked in:
[  906.286236]  snd_hda_intel i915 drm_kms_helper drm msr snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec snd_hwdep x86_pkg_temp_thermal snd_hda_core i2c_algo_bit ehci_pci syscopyarea sysfillrect sysimgblt fb_sys_fops ehci_hcd r8169 xhci_pci mii xhci_hcd video [last unloaded: drm]
[  906.286246] CPU: 0 PID: 12252 Comm: kms_pipe_crc_ba Tainted: G     U  W       4.4.0-rc6+ #45
[  906.286247] Hardware name: Dell Inc. Inspiron 3847/088DT1       , BIOS A06 01/15/2015
[  906.286248]  ffffffffc022c098 ffff880210dbbae0 ffffffff813e0e4f ffff880210dbbb28
[  906.286250]  ffff880210dbbb18 ffffffff8105f5f2 ffff8801fff40000 0000000000046040
[  906.286251]  ffff8801fff49170 ffff8801fff49170 ffff8801fff40000 ffff880210dbbb78
[  906.286253] Call Trace:
[  906.286256]  [<ffffffff813e0e4f>] dump_stack+0x44/0x55
[  906.286259]  [<ffffffff8105f5f2>] warn_slowpath_common+0x82/0xc0
[  906.286261]  [<ffffffff8105f67c>] warn_slowpath_fmt+0x4c/0x50
[  906.286270]  [<ffffffffc007e29c>] ? drm_property_free_blob+0x8c/0xb0 [drm]
[  906.286280]  [<ffffffffc01a32ea>] gen6_read32+0x1ca/0x1e0 [i915]
[  906.286283]  [<ffffffff8172cd12>] ? mutex_lock+0x12/0x30
[  906.286294]  [<ffffffffc01e0ef0>] hsw_ddi_wrpll_get_hw_state+0x40/0x50 [i915]
[  906.286304]  [<ffffffffc01c3fa1>] intel_atomic_commit+0xd41/0x1740 [i915]
[  906.286312]  [<ffffffffc008c597>] drm_atomic_commit+0x37/0x60 [drm]
[  906.286316]  [<ffffffffc01373e6>] drm_atomic_helper_set_config+0x76/0xb0 [drm_kms_helper]
[  906.286323]  [<ffffffffc008ac0a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
[  906.286329]  [<ffffffffc007b852>] drm_mode_set_config_internal+0x62/0x100 [drm]
[  906.286335]  [<ffffffffc007fcfd>] drm_mode_setcrtc+0x3cd/0x4e0 [drm]
[  906.286339]  [<ffffffffc0071762>] drm_ioctl+0x152/0x540 [drm]
[  906.286341]  [<ffffffff8109d914>] ? __wake_up+0x44/0x50
[  906.286346]  [<ffffffffc007f930>] ? drm_mode_setplane+0x1b0/0x1b0 [drm]
[  906.286348]  [<ffffffff811dc844>] ? mntput+0x24/0x40
[  906.286350]  [<ffffffff811bfe22>] ? __fput+0x172/0x1e0
[  906.286352]  [<ffffffff811d0758>] do_vfs_ioctl+0x288/0x460
[  906.286353]  [<ffffffff811bfece>] ? ____fput+0xe/0x10

Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 48 ++++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..4ba9f2c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2014,15 +2014,18 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 	enum intel_display_power_domain power_domain;
 	u32 tmp;
 	int i;
+	bool ret = false;
 
 	power_domain = intel_display_port_power_domain(encoder);
 	if (!intel_display_power_is_enabled(dev_priv, power_domain))
-		return false;
+		return ret;
+
+	intel_runtime_pm_get(dev_priv);
 
 	tmp = I915_READ(DDI_BUF_CTL(port));
 
 	if (!(tmp & DDI_BUF_CTL_ENABLE))
-		return false;
+		goto out;
 
 	if (port == PORT_A) {
 		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
@@ -2040,7 +2043,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 			break;
 		}
 
-		return true;
+		ret = true;
+		goto out;
 	} else {
 		for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) {
 			tmp = I915_READ(TRANS_DDI_FUNC_CTL(i));
@@ -2048,17 +2052,19 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 			if ((tmp & TRANS_DDI_PORT_MASK)
 			    == TRANS_DDI_SELECT_PORT(port)) {
 				if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST)
-					return false;
+					goto out;
 
 				*pipe = i;
-				return true;
+				ret = true;
+				goto out;
 			}
 		}
 	}
 
 	DRM_DEBUG_KMS("No pipe for ddi port %c found\n", port_name(port));
-
-	return false;
+out:
+	intel_runtime_pm_put(dev_priv);
+	return ret;
 }
 
 void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
@@ -2510,7 +2516,10 @@ static bool hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
 	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
 		return false;
 
+	intel_runtime_pm_get(dev_priv);
 	val = I915_READ(WRPLL_CTL(pll->id));
+	intel_runtime_pm_put(dev_priv);
+
 	hw_state->wrpll = val;
 
 	return val & WRPLL_PLL_ENABLE;
@@ -2525,7 +2534,10 @@ static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
 	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
 		return false;
 
+	intel_runtime_pm_get(dev_priv);
 	val = I915_READ(SPLL_CTL);
+	intel_runtime_pm_put(dev_priv);
+
 	hw_state->spll = val;
 
 	return val & SPLL_PLL_ENABLE;
@@ -2644,16 +2656,19 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	uint32_t val;
 	unsigned int dpll;
 	const struct skl_dpll_regs *regs = skl_dpll_regs;
+	bool ret = false;
 
 	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
-		return false;
+		return ret;
 
 	/* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */
 	dpll = pll->id + 1;
 
+	intel_runtime_pm_get(dev_priv);
+
 	val = I915_READ(regs[pll->id].ctl);
 	if (!(val & LCPLL_PLL_ENABLE))
-		return false;
+		goto out;
 
 	val = I915_READ(DPLL_CTRL1);
 	hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f;
@@ -2664,6 +2679,9 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 		hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2);
 	}
 
+	ret = true;
+out:
+	intel_runtime_pm_put(dev_priv);
 	return true;
 }
 
@@ -2931,13 +2949,16 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 {
 	enum port port = (enum port)pll->id;	/* 1:1 port->PLL mapping */
 	uint32_t val;
+	bool ret = false;
 
 	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
-		return false;
+		return ret;
+
+	intel_runtime_pm_get(dev_priv);
 
 	val = I915_READ(BXT_PORT_PLL_ENABLE(port));
 	if (!(val & PORT_PLL_ENABLE))
-		return false;
+		goto out;
 
 	hw_state->ebb0 = I915_READ(BXT_PORT_PLL_EBB_0(port));
 	hw_state->ebb0 &= PORT_PLL_P1_MASK | PORT_PLL_P2_MASK;
@@ -2984,7 +3005,10 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				 I915_READ(BXT_PORT_PCS_DW12_LN23(port)));
 	hw_state->pcsdw12 &= LANE_STAGGER_MASK | LANESTAGGER_STRAP_OVRD;
 
-	return true;
+	ret = true;
+out:
+	intel_runtime_pm_put(dev_priv);
+	return ret;
 }
 
 static void bxt_shared_dplls_init(struct drm_i915_private *dev_priv)
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ failure: Fi.CI.BAT
  2015-12-31 12:45 [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
@ 2015-12-31 13:20 ` Patchwork
  2015-12-31 15:52 ` [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2015-12-31 13:20 UTC (permalink / raw)
  To: Feceoru, Gabriel; +Cc: intel-gfx

== Summary ==

Built on 79686f613b3955a4ed09cee936e7f70ec4e61b67 drm-intel-nightly: 2015y-12m-30d-11h-59m-54s UTC integration manifest

Test gem_basic:
        Subgroup create-close:
                pass       -> DMESG-WARN (skl-i7k-2)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i7k-2)
Test gem_ctx_param_basic:
        Subgroup basic:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup invalid-param-set:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup non-root-set-no-zeromap:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup root-set-no-zeromap-disabled:
                pass       -> DMESG-WARN (skl-i7k-2)
Test gem_mmap:
        Subgroup basic:
                pass       -> DMESG-WARN (skl-i7k-2)
Test gem_mmap_gtt:
        Subgroup basic-read:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup basic-write:
                pass       -> DMESG-WARN (skl-i7k-2)
Test gem_storedw_loop:
        Subgroup basic-render:
                dmesg-warn -> PASS       (skl-i5k-2)
                dmesg-warn -> PASS       (skl-i7k-2)
Test kms_addfb_basic:
        Subgroup addfb25-modifier-no-flag:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup addfb25-x-tiled-mismatch:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup bad-pitch-1024:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup bad-pitch-63:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup bad-pitch-999:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup clobberred-modifier:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup too-high:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup too-wide:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup unused-offsets:
                pass       -> DMESG-WARN (skl-i7k-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
                dmesg-warn -> PASS       (ilk-hp8440p)
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (bsw-nuc-2)
                dmesg-warn -> PASS       (hsw-brixbox)
                dmesg-warn -> PASS       (bdw-nuci7)
                dmesg-warn -> PASS       (ilk-hp8440p)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (snb-x220t)
                pass       -> DMESG-WARN (bdw-ultra)
                pass       -> DMESG-FAIL (skl-i7k-2)
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> SKIP       (snb-x220t)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-WARN (skl-i5k-2)
                dmesg-warn -> PASS       (snb-x220t)
                pass       -> DMESG-WARN (skl-i7k-2)
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (snb-dellxps)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-FAIL (skl-i7k-2)
        Subgroup read-crc-pipe-c:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                dmesg-warn -> PASS       (bdw-ultra)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                dmesg-warn -> PASS       (snb-dellxps)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (skl-i7k-2)
Test prime_self_import:
        Subgroup basic-with_two_bos:
                pass       -> DMESG-WARN (skl-i7k-2)

bdw-nuci7        total:132  pass:122  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultra        total:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc          total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:127  dwarn:1   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
hsw-xps12        total:132  pass:125  dwarn:3   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:100  dwarn:0   dfail:0   fail:0   skip:35 
ivb-t430s        total:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2        total:135  pass:108  dwarn:19  dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:89   dwarn:34  dfail:3   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:120  dwarn:2   dfail:0   fail:1   skip:12 

Results at /archive/results/CI_IGT_test/Patchwork_981/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
  2015-12-31 12:45 [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
  2015-12-31 13:20 ` ✗ failure: Fi.CI.BAT Patchwork
@ 2015-12-31 15:52 ` Gabriel Feceoru
  2016-01-04 14:19   ` Maarten Lankhorst
  2016-01-04 15:05   ` Ville Syrjälä
  2015-12-31 16:20 ` ✗ warning: Fi.CI.BAT Patchwork
  2016-01-04 12:55 ` [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions Marius Vlad
  3 siblings, 2 replies; 10+ messages in thread
From: Gabriel Feceoru @ 2015-12-31 15:52 UTC (permalink / raw)
  To: intel-gfx

This gets rid of errors like:

[  906.286213] ------------[ cut here ]------------
[  906.286233] WARNING: CPU: 0 PID: 12252 at drivers/gpu/drm/i915/intel_drv.h:1457 gen6_read32+0x1ca/0x1e0 [i915]()
[  906.286234] RPM wakelock ref not held during HW access
[  906.286235] Modules linked in:
[  906.286236]  snd_hda_intel i915 drm_kms_helper drm msr snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec snd_hwdep x86_pkg_temp_thermal snd_hda_core i2c_algo_bit ehci_pci syscopyarea sysfillrect sysimgblt fb_sys_fops ehci_hcd r8169 xhci_pci mii xhci_hcd video [last unloaded: drm]
[  906.286246] CPU: 0 PID: 12252 Comm: kms_pipe_crc_ba Tainted: G     U  W       4.4.0-rc6+ #45
[  906.286247] Hardware name: Dell Inc. Inspiron 3847/088DT1       , BIOS A06 01/15/2015
[  906.286248]  ffffffffc022c098 ffff880210dbbae0 ffffffff813e0e4f ffff880210dbbb28
[  906.286250]  ffff880210dbbb18 ffffffff8105f5f2 ffff8801fff40000 0000000000046040
[  906.286251]  ffff8801fff49170 ffff8801fff49170 ffff8801fff40000 ffff880210dbbb78
[  906.286253] Call Trace:
[  906.286256]  [<ffffffff813e0e4f>] dump_stack+0x44/0x55
[  906.286259]  [<ffffffff8105f5f2>] warn_slowpath_common+0x82/0xc0
[  906.286261]  [<ffffffff8105f67c>] warn_slowpath_fmt+0x4c/0x50
[  906.286270]  [<ffffffffc007e29c>] ? drm_property_free_blob+0x8c/0xb0 [drm]
[  906.286280]  [<ffffffffc01a32ea>] gen6_read32+0x1ca/0x1e0 [i915]
[  906.286283]  [<ffffffff8172cd12>] ? mutex_lock+0x12/0x30
[  906.286294]  [<ffffffffc01e0ef0>] hsw_ddi_wrpll_get_hw_state+0x40/0x50 [i915]
[  906.286304]  [<ffffffffc01c3fa1>] intel_atomic_commit+0xd41/0x1740 [i915]
[  906.286312]  [<ffffffffc008c597>] drm_atomic_commit+0x37/0x60 [drm]
[  906.286316]  [<ffffffffc01373e6>] drm_atomic_helper_set_config+0x76/0xb0 [drm_kms_helper]
[  906.286323]  [<ffffffffc008ac0a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
[  906.286329]  [<ffffffffc007b852>] drm_mode_set_config_internal+0x62/0x100 [drm]
[  906.286335]  [<ffffffffc007fcfd>] drm_mode_setcrtc+0x3cd/0x4e0 [drm]
[  906.286339]  [<ffffffffc0071762>] drm_ioctl+0x152/0x540 [drm]
[  906.286341]  [<ffffffff8109d914>] ? __wake_up+0x44/0x50
[  906.286346]  [<ffffffffc007f930>] ? drm_mode_setplane+0x1b0/0x1b0 [drm]
[  906.286348]  [<ffffffff811dc844>] ? mntput+0x24/0x40
[  906.286350]  [<ffffffff811bfe22>] ? __fput+0x172/0x1e0
[  906.286352]  [<ffffffff811d0758>] do_vfs_ioctl+0x288/0x460
[  906.286353]  [<ffffffff811bfece>] ? ____fput+0xe/0x10

v2:
    Fixed return value in skl_ddi_pll_get_hw_state

Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 50 +++++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..e92ed7a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2014,15 +2014,18 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 	enum intel_display_power_domain power_domain;
 	u32 tmp;
 	int i;
+	bool ret = false;
 
 	power_domain = intel_display_port_power_domain(encoder);
 	if (!intel_display_power_is_enabled(dev_priv, power_domain))
-		return false;
+		return ret;
+
+	intel_runtime_pm_get(dev_priv);
 
 	tmp = I915_READ(DDI_BUF_CTL(port));
 
 	if (!(tmp & DDI_BUF_CTL_ENABLE))
-		return false;
+		goto out;
 
 	if (port == PORT_A) {
 		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
@@ -2040,7 +2043,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 			break;
 		}
 
-		return true;
+		ret = true;
+		goto out;
 	} else {
 		for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) {
 			tmp = I915_READ(TRANS_DDI_FUNC_CTL(i));
@@ -2048,17 +2052,19 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 			if ((tmp & TRANS_DDI_PORT_MASK)
 			    == TRANS_DDI_SELECT_PORT(port)) {
 				if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST)
-					return false;
+					goto out;
 
 				*pipe = i;
-				return true;
+				ret = true;
+				goto out;
 			}
 		}
 	}
 
 	DRM_DEBUG_KMS("No pipe for ddi port %c found\n", port_name(port));
-
-	return false;
+out:
+	intel_runtime_pm_put(dev_priv);
+	return ret;
 }
 
 void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
@@ -2510,7 +2516,10 @@ static bool hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
 	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
 		return false;
 
+	intel_runtime_pm_get(dev_priv);
 	val = I915_READ(WRPLL_CTL(pll->id));
+	intel_runtime_pm_put(dev_priv);
+
 	hw_state->wrpll = val;
 
 	return val & WRPLL_PLL_ENABLE;
@@ -2525,7 +2534,10 @@ static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
 	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
 		return false;
 
+	intel_runtime_pm_get(dev_priv);
 	val = I915_READ(SPLL_CTL);
+	intel_runtime_pm_put(dev_priv);
+
 	hw_state->spll = val;
 
 	return val & SPLL_PLL_ENABLE;
@@ -2644,16 +2656,19 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	uint32_t val;
 	unsigned int dpll;
 	const struct skl_dpll_regs *regs = skl_dpll_regs;
+	bool ret = false;
 
 	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
-		return false;
+		return ret;
 
 	/* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */
 	dpll = pll->id + 1;
 
+	intel_runtime_pm_get(dev_priv);
+
 	val = I915_READ(regs[pll->id].ctl);
 	if (!(val & LCPLL_PLL_ENABLE))
-		return false;
+		goto out;
 
 	val = I915_READ(DPLL_CTRL1);
 	hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f;
@@ -2664,7 +2679,10 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 		hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2);
 	}
 
-	return true;
+	ret = true;
+out:
+	intel_runtime_pm_put(dev_priv);
+	return ret;
 }
 
 static void skl_shared_dplls_init(struct drm_i915_private *dev_priv)
@@ -2931,13 +2949,16 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 {
 	enum port port = (enum port)pll->id;	/* 1:1 port->PLL mapping */
 	uint32_t val;
+	bool ret = false;
 
 	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
-		return false;
+		return ret;
+
+	intel_runtime_pm_get(dev_priv);
 
 	val = I915_READ(BXT_PORT_PLL_ENABLE(port));
 	if (!(val & PORT_PLL_ENABLE))
-		return false;
+		goto out;
 
 	hw_state->ebb0 = I915_READ(BXT_PORT_PLL_EBB_0(port));
 	hw_state->ebb0 &= PORT_PLL_P1_MASK | PORT_PLL_P2_MASK;
@@ -2984,7 +3005,10 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				 I915_READ(BXT_PORT_PCS_DW12_LN23(port)));
 	hw_state->pcsdw12 &= LANE_STAGGER_MASK | LANESTAGGER_STRAP_OVRD;
 
-	return true;
+	ret = true;
+out:
+	intel_runtime_pm_put(dev_priv);
+	return ret;
 }
 
 static void bxt_shared_dplls_init(struct drm_i915_private *dev_priv)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ warning: Fi.CI.BAT
  2015-12-31 12:45 [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
  2015-12-31 13:20 ` ✗ failure: Fi.CI.BAT Patchwork
  2015-12-31 15:52 ` [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
@ 2015-12-31 16:20 ` Patchwork
  2016-01-04 12:55 ` [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions Marius Vlad
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2015-12-31 16:20 UTC (permalink / raw)
  To: Feceoru, Gabriel; +Cc: intel-gfx

== Summary ==

Built on 79686f613b3955a4ed09cee936e7f70ec4e61b67 drm-intel-nightly: 2015y-12m-30d-11h-59m-54s UTC integration manifest

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ilk-hp8440p)
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (bsw-nuc-2)
                dmesg-warn -> PASS       (bdw-nuci7)
                dmesg-warn -> PASS       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a:
                dmesg-warn -> PASS       (snb-x220t)
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> DMESG-WARN (snb-dellxps)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (snb-x220t)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                dmesg-warn -> PASS       (snb-dellxps)

bdw-nuci7        total:132  pass:122  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultra        total:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc          total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:126  dwarn:2   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:100  dwarn:0   dfail:0   fail:0   skip:35 
ivb-t430s        total:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2        total:135  pass:122  dwarn:5   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:124  dwarn:3   dfail:0   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_987/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions
  2015-12-31 12:45 [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
                   ` (2 preceding siblings ...)
  2015-12-31 16:20 ` ✗ warning: Fi.CI.BAT Patchwork
@ 2016-01-04 12:55 ` Marius Vlad
  3 siblings, 0 replies; 10+ messages in thread
From: Marius Vlad @ 2016-01-04 12:55 UTC (permalink / raw)
  To: Gabriel Feceoru; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 18766 bytes --]

Hi,
	I'm still seeing these warning(s) with both this patch and
	the one for i915_driver_unload() (http://patchwork.freedesktop.org/patch/69227/),
	in more than two cases:

	$ tests/kms_flip --run-subtest basic-flip-vs-wf_vblank

	[  226.580012] ------------[ cut here ]------------
	[  226.580049] WARNING: CPU: 0 PID: 2077 at drivers/gpu/drm/i915/intel_drv.h:1457 gen6_read32+0x1ca/0x1e0 [i915]()
	[  226.580051] RPM wakelock ref not held during HW access
	[  226.580052] Modules linked in:
	[  226.580054]  snd_hda_intel i915 drm_kms_helper drm msr bnep binfmt_misc arc4 ath9k ath9k_common ath9k_hw ath mac80211 snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel snd_hda_core joydev kvm snd_hwdep input_leds dcdbas dell_smm_hwmon snd_pcm snd_seq_midi cfg80211 snd_seq_midi_event snd_rawmidi irqbypass snd_seq ath3k btusb btrtl btbcm btintel crct10dif_pclmul crc32_pclmul bluetooth aesni_intel aes_x86_64 snd_seq_device lrw snd_timer gf128mul glue_helper snd mei_me ablk_helper cryptd mei shpchp soundcore serio_raw lpc_ich mac_hid parport_pc ppdev lp parport autofs4 hid_generic usbhid hid i2c_algo_bit syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse r8169 ahci libahci mii video [last unloaded: drm]

	[  226.580112] CPU: 0 PID: 2077 Comm: kms_flip Tainted: G     U  W       4.4.0-rc8+ #40
	[  226.580114] Hardware name: Dell Inc. Inspiron 3847/088DT1       , BIOS A06 01/15/2015
	[  226.580116]  0000000000000000 00000000291e9ecc ffff880207e27a80 ffffffff813c5104
	[  226.580119]  ffff880207e27ac8 ffff880207e27ab8 ffffffff8107d5d2 ffff880213080000
	[  226.580122]  000000000006f400 ffff8800d6fe6000 ffff880207d0b000 ffff880213080000
	[  226.580125] Call Trace:
	[  226.580133]  [<ffffffff813c5104>] dump_stack+0x44/0x60
	[  226.580138]  [<ffffffff8107d5d2>] warn_slowpath_common+0x82/0xc0
	[  226.580140]  [<ffffffff8107d66c>] warn_slowpath_fmt+0x5c/0x80
	[  226.580162]  [<ffffffffc081c3ba>] gen6_read32+0x1ca/0x1e0 [i915]
	[  226.580184]  [<ffffffffc082fa83>] haswell_get_pipe_config+0x83/0x710 [i915]
	[  226.580202]  [<ffffffffc015c9ee>] ? drm_property_unreference_blob+0x8e/0xc0 [drm]
	[  226.580226]  [<ffffffffc083d6d4>] intel_atomic_commit+0xcb4/0x17f0 [i915]
	[  226.580243]  [<ffffffffc016a256>] ? drm_atomic_check_only+0x196/0x5b0 [drm]
	[  226.580257]  [<ffffffffc016a6a7>] drm_atomic_commit+0x37/0x60 [drm]
	[  226.580265]  [<ffffffffc0089696>] drm_atomic_helper_set_config+0x76/0xb0 [drm_kms_helper]
	[  226.580280]  [<ffffffffc016957a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
	[  226.580292]  [<ffffffffc0159ae2>] drm_mode_set_config_internal+0x62/0x100 [drm]
	[  226.580305]  [<ffffffffc015e3b0>] drm_mode_setcrtc+0x3e0/0x500 [drm]
	[  226.580314]  [<ffffffffc014f7b2>] drm_ioctl+0x152/0x540 [drm]
	[  226.580319]  [<ffffffff8118eaa3>] ? free_pages+0x13/0x20
	[  226.580330]  [<ffffffffc015dfd0>] ? drm_mode_setplane+0x1b0/0x1b0 [drm]
	[  226.580348]  [<ffffffffc06864a2>] ? ieee80211_channel_switch+0x342/0x9d0 [mac80211]
	[  226.580353]  [<ffffffff81217478>] do_vfs_ioctl+0x298/0x480
	[  226.580358]  [<ffffffff811bfc19>] ? do_munmap+0x349/0x480
	[  226.580362]  [<ffffffff812176d9>] SyS_ioctl+0x79/0x90
	[  226.580366]  [<ffffffff817f38f6>] entry_SYSCALL_64_fastpath+0x16/0x75
	[  226.580368] ---[ end trace 964f3949c8ea57e4 ]---

	$ tests/kms_pipe_crc_basic --run-subtest suspend-read-crc-pipe-A

	[  392.470207] Suspending console(s) (use no_console_suspend to debug)
	[  392.470494] sd 0:0:0:0: [sda] Synchronizing SCSI cache
	[  392.472230] sd 0:0:0:0: [sda] Stopping disk
	[  393.248866]  cache: parent cpu1 should not be sleeping
	[  393.268891]  cache: parent cpu2 should not be sleeping
	[  393.284919]  cache: parent cpu3 should not be sleeping
	[  393.373952] sd 0:0:0:0: [sda] Starting disk
	[  396.273511] pciehp 0000:00:1c.0:pcie04: link training error: status 0x1001
	[  396.273513] pciehp 0000:00:1c.0:pcie04: Failed to check link status
	[  396.504341] ------------[ cut here ]------------
	[  396.504363] WARNING: CPU: 0 PID: 2719 at drivers/gpu/drm/i915/intel_drv.h:1457 gen6_read32+0x1ca/0x1e0 [i915]()
	[  396.504364] RPM wakelock ref not held during HW access
	[  396.504365] Modules linked in:
	[  396.504366]  snd_hda_intel i915 drm_kms_helper drm msr bnep binfmt_misc arc4 ath9k ath9k_common ath9k_hw ath mac80211 snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel snd_hda_core joydev kvm snd_hwdep input_leds dcdbas dell_smm_hwmon snd_pcm snd_seq_midi cfg80211 snd_seq_midi_event snd_rawmidi irqbypass snd_seq ath3k btusb btrtl btbcm btintel crct10dif_pclmul crc32_pclmul bluetooth aesni_intel aes_x86_64 snd_seq_device lrw snd_timer gf128mul glue_helper snd mei_me ablk_helper cryptd mei shpchp soundcore serio_raw lpc_ich mac_hid parport_pc ppdev lp parport autofs4 hid_generic usbhid hid i2c_algo_bit syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse r8169 ahci libahci mii video [last unloaded: drm]

	[  396.504397] CPU: 0 PID: 2719 Comm: kms_pipe_crc_ba Tainted: G     U  W       4.4.0-rc8+ #40
	[  396.504398] Hardware name: Dell Inc. Inspiron 3847/088DT1       , BIOS A06 01/15/2015
	[  396.504399]  0000000000000000 0000000013c77361 ffff8800d4bb3a80 ffffffff813c5104
	[  396.504401]  ffff8800d4bb3ac8 ffff8800d4bb3ab8 ffffffff8107d5d2 ffff8801f5ba0000
	[  396.504402]  000000000006f400 ffff8800d6fe4800 ffff8801f53fe000 ffff8801f5ba0000
	[  396.504404] Call Trace:
	[  396.504409]  [<ffffffff813c5104>] dump_stack+0x44/0x60
	[  396.504411]  [<ffffffff8107d5d2>] warn_slowpath_common+0x82/0xc0
	[  396.504412]  [<ffffffff8107d66c>] warn_slowpath_fmt+0x5c/0x80
	[  396.504423]  [<ffffffffc081c3ba>] gen6_read32+0x1ca/0x1e0 [i915]
	[  396.504434]  [<ffffffffc082fa83>] haswell_get_pipe_config+0x83/0x710 [i915]
	[  396.504443]  [<ffffffffc008f9ee>] ? drm_property_unreference_blob+0x8e/0xc0 [drm]
	[  396.504454]  [<ffffffffc083d6d4>] intel_atomic_commit+0xcb4/0x17f0 [i915]
	[  396.504463]  [<ffffffffc009d256>] ? drm_atomic_check_only+0x196/0x5b0 [drm]
	[  396.504470]  [<ffffffffc009d6a7>] drm_atomic_commit+0x37/0x60 [drm]
	[  396.504475]  [<ffffffffc0156696>] drm_atomic_helper_set_config+0x76/0xb0 [drm_kms_helper]
	[  396.504482]  [<ffffffffc009c57a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
	[  396.504488]  [<ffffffffc008cae2>] drm_mode_set_config_internal+0x62/0x100 [drm]
	[  396.504495]  [<ffffffffc00913b0>] drm_mode_setcrtc+0x3e0/0x500 [drm]
	[  396.504499]  [<ffffffffc00827b2>] drm_ioctl+0x152/0x540 [drm]
	[  396.504504]  [<ffffffffc0090fd0>] ? drm_mode_setplane+0x1b0/0x1b0 [drm]
	[  396.504507]  [<ffffffff81224214>] ? mntput+0x24/0x40
	[  396.504509]  [<ffffffff81217478>] do_vfs_ioctl+0x298/0x480
	[  396.504510]  [<ffffffff81205cae>] ? ____fput+0xe/0x10
	[  396.504512]  [<ffffffff8109aaf8>] ? task_work_run+0x78/0x90
	[  396.504514]  [<ffffffff812176d9>] SyS_ioctl+0x79/0x90
	[  396.504516]  [<ffffffff817f38f6>] entry_SYSCALL_64_fastpath+0x16/0x75
	[  396.504517] ---[ end trace 964f3949c8ea57e5 ]---


	Running a test from igt when the monitor resumes from suspend/standby
	-- this can be easily trigger by using a consoleblank value much smaller 
	than the default (30 minutes) and using something like:
	$ tests/pm_rpm --run-subtest basic-rte

	[  240.707717] WARNING: CPU: 0 PID: 4 at drivers/gpu/drm/i915/intel_drv.h:1457 gen6_read32+0x1ca/0x1e0 [i915]()
	[  240.707718] RPM wakelock ref not held during HW access
	[  240.707740] Modules linked in: msr snd_hda_intel i915 drm_kms_helper
	drm bnep binfmt_misc arc4 ath9k ath9k_common ath9k_hw ath mac80211
	snd_hda_codec_realtek snd_hda_codec_hdmi snd_hda_codec_generic intel_rapl
	snd_hda_codec iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel
	snd_hda_core snd_hwdep snd_pcm kvm ath3k snd_seq_midi btusb snd_seq_midi_event
	btrtl snd_rawmidi btbcm cfg80211 snd_seq btintel bluetooth irqbypass
	snd_seq_device snd_timer crct10dif_pclmul dcdbas crc32_pclmul dell_smm_hwmon
	joydev snd aesni_intel aes_x86_64 input_leds lrw gf128mul glue_helper mei_me
	ablk_helper cryptd mei serio_raw shpchp soundcore lpc_ich mac_hid parport_pc
	ppdev lp parport autofs4 hid_generic usbhid hid i2c_algo_bit syscopyarea
	sysfillrect sysimgblt fb_sys_fops psmouse ahci r8169 libahci mii
	[  240.707740]  video [last unloaded: drm]
	[  240.707742] CPU: 0 PID: 4 Comm: kworker/0:0 Tainted: G        W       4.4.0-rc8+ #40
	[  240.707743] Hardware name: Dell Inc. Inspiron 3847/088DT1       , BIOS A06 01/15/2015
	[  240.707747] Workqueue: events console_callback
	[  240.707748]  0000000000000000 000000001b5b9d8a ffff88021621f988 ffffffff813c5104
	[  240.707749]  ffff88021621f9d0 ffff88021621f9c0 ffffffff8107d5d2 ffff8800d0810000
	[  240.707750]  000000000006f400 ffff8800c02af800 ffff8801f689e000 ffff8800d0810000
	[  240.707750] Call Trace:
	[  240.707754]  [<ffffffff813c5104>] dump_stack+0x44/0x60
	[  240.707757]  [<ffffffff8107d5d2>] warn_slowpath_common+0x82/0xc0
	[  240.707758]  [<ffffffff8107d66c>] warn_slowpath_fmt+0x5c/0x80
	[  240.707770]  [<ffffffffc08063ba>] gen6_read32+0x1ca/0x1e0 [i915]
	[  240.707780]  [<ffffffffc0819a83>] haswell_get_pipe_config+0x83/0x710 [i915]
	[  240.707791]  [<ffffffffc08276d4>] intel_atomic_commit+0xcb4/0x17f0 [i915]
	[  240.707801]  [<ffffffffc0152256>] ? drm_atomic_check_only+0x196/0x5b0 [drm]
	[  240.707808]  [<ffffffffc01526a7>] drm_atomic_commit+0x37/0x60 [drm]
	[  240.707812]  [<ffffffffc0083d7d>] drm_atomic_helper_connector_dpms+0xed/0x1a0 [drm_kms_helper]
	[  240.707815]  [<ffffffffc0087417>] drm_fb_helper_dpms.isra.8+0x97/0xe0 [drm_kms_helper]
	[  240.707818]  [<ffffffffc008749d>] drm_fb_helper_blank+0x3d/0x90 [drm_kms_helper]
	[  240.707829]  [<ffffffffc083e7fa>] intel_fbdev_blank+0x1a/0x60 [i915]
	[  240.707831]  [<ffffffff81447366>] fb_blank+0x66/0xc0
	[  240.707832]  [<ffffffff8143d912>] fbcon_blank+0x292/0x350
	[  240.707834]  [<ffffffff810b5814>] ? set_next_entity+0xa4/0x870
	[  240.707836]  [<ffffffff810e8061>] ? internal_add_timer+0x71/0x80
	[  240.707837]  [<ffffffff810e7e64>] ? lock_timer_base.isra.22+0x54/0x70
	[  240.707838]  [<ffffffff810e7f6e>] ? try_to_del_timer_sync+0x5e/0x90
	[  240.707839]  [<ffffffff814d4620>] do_blank_screen+0x1c0/0x260
	[  240.707840]  [<ffffffff814d5437>] console_callback+0x77/0x180
	[  240.707842]  [<ffffffff817ef1f6>] ? __schedule+0x386/0x9c0
	[  240.707844]  [<ffffffff8109625a>] process_one_work+0x1aa/0x440
	[  240.707845]  [<ffffffff8109653b>] worker_thread+0x4b/0x4c0
	[  240.707845]  [<ffffffff810964f0>] ? process_one_work+0x440/0x440
	[  240.707847]  [<ffffffff8109c698>] kthread+0xd8/0xf0
	[  240.707848]  [<ffffffff8109c5c0>] ? kthread_create_on_node+0x1a0/0x1a0
	[  240.707849]  [<ffffffff817f3c8f>] ret_from_fork+0x3f/0x70
	[  240.707851]  [<ffffffff8109c5c0>] ? kthread_create_on_node+0x1a0/0x1a0

	Adding intel_runtime_pm_get(), _put() around I915_READ() seem to solve
	this issue.

	Most likely the following must be addressed as well:

	- i9xx_get_pipe_config()
	- ironlake_get_pipe_config()

	CC: Added Imre/Daniel as well.

On Thu, Dec 31, 2015 at 02:45:49PM +0200, Gabriel Feceoru wrote:
> This gets rid of errors like:
> 
> [  906.286213] ------------[ cut here ]------------
> [  906.286233] WARNING: CPU: 0 PID: 12252 at drivers/gpu/drm/i915/intel_drv.h:1457 gen6_read32+0x1ca/0x1e0 [i915]()
> [  906.286234] RPM wakelock ref not held during HW access
> [  906.286235] Modules linked in:
> [  906.286236]  snd_hda_intel i915 drm_kms_helper drm msr snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec snd_hwdep x86_pkg_temp_thermal snd_hda_core i2c_algo_bit ehci_pci syscopyarea sysfillrect sysimgblt fb_sys_fops ehci_hcd r8169 xhci_pci mii xhci_hcd video [last unloaded: drm]
> [  906.286246] CPU: 0 PID: 12252 Comm: kms_pipe_crc_ba Tainted: G     U  W       4.4.0-rc6+ #45
> [  906.286247] Hardware name: Dell Inc. Inspiron 3847/088DT1       , BIOS A06 01/15/2015
> [  906.286248]  ffffffffc022c098 ffff880210dbbae0 ffffffff813e0e4f ffff880210dbbb28
> [  906.286250]  ffff880210dbbb18 ffffffff8105f5f2 ffff8801fff40000 0000000000046040
> [  906.286251]  ffff8801fff49170 ffff8801fff49170 ffff8801fff40000 ffff880210dbbb78
> [  906.286253] Call Trace:
> [  906.286256]  [<ffffffff813e0e4f>] dump_stack+0x44/0x55
> [  906.286259]  [<ffffffff8105f5f2>] warn_slowpath_common+0x82/0xc0
> [  906.286261]  [<ffffffff8105f67c>] warn_slowpath_fmt+0x4c/0x50
> [  906.286270]  [<ffffffffc007e29c>] ? drm_property_free_blob+0x8c/0xb0 [drm]
> [  906.286280]  [<ffffffffc01a32ea>] gen6_read32+0x1ca/0x1e0 [i915]
> [  906.286283]  [<ffffffff8172cd12>] ? mutex_lock+0x12/0x30
> [  906.286294]  [<ffffffffc01e0ef0>] hsw_ddi_wrpll_get_hw_state+0x40/0x50 [i915]
> [  906.286304]  [<ffffffffc01c3fa1>] intel_atomic_commit+0xd41/0x1740 [i915]
> [  906.286312]  [<ffffffffc008c597>] drm_atomic_commit+0x37/0x60 [drm]
> [  906.286316]  [<ffffffffc01373e6>] drm_atomic_helper_set_config+0x76/0xb0 [drm_kms_helper]
> [  906.286323]  [<ffffffffc008ac0a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
> [  906.286329]  [<ffffffffc007b852>] drm_mode_set_config_internal+0x62/0x100 [drm]
> [  906.286335]  [<ffffffffc007fcfd>] drm_mode_setcrtc+0x3cd/0x4e0 [drm]
> [  906.286339]  [<ffffffffc0071762>] drm_ioctl+0x152/0x540 [drm]
> [  906.286341]  [<ffffffff8109d914>] ? __wake_up+0x44/0x50
> [  906.286346]  [<ffffffffc007f930>] ? drm_mode_setplane+0x1b0/0x1b0 [drm]
> [  906.286348]  [<ffffffff811dc844>] ? mntput+0x24/0x40
> [  906.286350]  [<ffffffff811bfe22>] ? __fput+0x172/0x1e0
> [  906.286352]  [<ffffffff811d0758>] do_vfs_ioctl+0x288/0x460
> [  906.286353]  [<ffffffff811bfece>] ? ____fput+0xe/0x10
> 
> Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 48 ++++++++++++++++++++++++++++++----------
>  1 file changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e6408e5..4ba9f2c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2014,15 +2014,18 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  	enum intel_display_power_domain power_domain;
>  	u32 tmp;
>  	int i;
> +	bool ret = false;
>  
>  	power_domain = intel_display_port_power_domain(encoder);
>  	if (!intel_display_power_is_enabled(dev_priv, power_domain))
> -		return false;
> +		return ret;
> +
> +	intel_runtime_pm_get(dev_priv);
>  
>  	tmp = I915_READ(DDI_BUF_CTL(port));
>  
>  	if (!(tmp & DDI_BUF_CTL_ENABLE))
> -		return false;
> +		goto out;
>  
>  	if (port == PORT_A) {
>  		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> @@ -2040,7 +2043,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  			break;
>  		}
>  
> -		return true;
> +		ret = true;
> +		goto out;
>  	} else {
>  		for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) {
>  			tmp = I915_READ(TRANS_DDI_FUNC_CTL(i));
> @@ -2048,17 +2052,19 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  			if ((tmp & TRANS_DDI_PORT_MASK)
>  			    == TRANS_DDI_SELECT_PORT(port)) {
>  				if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST)
> -					return false;
> +					goto out;
>  
>  				*pipe = i;
> -				return true;
> +				ret = true;
> +				goto out;
>  			}
>  		}
>  	}
>  
>  	DRM_DEBUG_KMS("No pipe for ddi port %c found\n", port_name(port));
> -
> -	return false;
> +out:
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
>  }
>  
>  void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> @@ -2510,7 +2516,10 @@ static bool hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
>  	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
>  		return false;
>  
> +	intel_runtime_pm_get(dev_priv);
>  	val = I915_READ(WRPLL_CTL(pll->id));
> +	intel_runtime_pm_put(dev_priv);
> +
>  	hw_state->wrpll = val;
>  
>  	return val & WRPLL_PLL_ENABLE;
> @@ -2525,7 +2534,10 @@ static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
>  	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
>  		return false;
>  
> +	intel_runtime_pm_get(dev_priv);
>  	val = I915_READ(SPLL_CTL);
> +	intel_runtime_pm_put(dev_priv);
> +
>  	hw_state->spll = val;
>  
>  	return val & SPLL_PLL_ENABLE;
> @@ -2644,16 +2656,19 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  	uint32_t val;
>  	unsigned int dpll;
>  	const struct skl_dpll_regs *regs = skl_dpll_regs;
> +	bool ret = false;
>  
>  	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
> -		return false;
> +		return ret;
>  
>  	/* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */
>  	dpll = pll->id + 1;
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	val = I915_READ(regs[pll->id].ctl);
>  	if (!(val & LCPLL_PLL_ENABLE))
> -		return false;
> +		goto out;
>  
>  	val = I915_READ(DPLL_CTRL1);
>  	hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f;
> @@ -2664,6 +2679,9 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  		hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2);
>  	}
>  
> +	ret = true;
> +out:
> +	intel_runtime_pm_put(dev_priv);
>  	return true;
>  }
>  
> @@ -2931,13 +2949,16 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  {
>  	enum port port = (enum port)pll->id;	/* 1:1 port->PLL mapping */
>  	uint32_t val;
> +	bool ret = false;
>  
>  	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
> -		return false;
> +		return ret;
> +
> +	intel_runtime_pm_get(dev_priv);
>  
>  	val = I915_READ(BXT_PORT_PLL_ENABLE(port));
>  	if (!(val & PORT_PLL_ENABLE))
> -		return false;
> +		goto out;
>  
>  	hw_state->ebb0 = I915_READ(BXT_PORT_PLL_EBB_0(port));
>  	hw_state->ebb0 &= PORT_PLL_P1_MASK | PORT_PLL_P2_MASK;
> @@ -2984,7 +3005,10 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				 I915_READ(BXT_PORT_PCS_DW12_LN23(port)));
>  	hw_state->pcsdw12 &= LANE_STAGGER_MASK | LANESTAGGER_STRAP_OVRD;
>  
> -	return true;
> +	ret = true;
> +out:
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
>  }
>  
>  static void bxt_shared_dplls_init(struct drm_i915_private *dev_priv)
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
  2015-12-31 15:52 ` [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
@ 2016-01-04 14:19   ` Maarten Lankhorst
  2016-01-04 15:05   ` Ville Syrjälä
  1 sibling, 0 replies; 10+ messages in thread
From: Maarten Lankhorst @ 2016-01-04 14:19 UTC (permalink / raw)
  To: Gabriel Feceoru, intel-gfx

Hey,

Op 31-12-15 om 16:52 schreef Gabriel Feceoru:
> This gets rid of errors like:
>
> [  906.286213] ------------[ cut here ]------------
> [  906.286233] WARNING: CPU: 0 PID: 12252 at drivers/gpu/drm/i915/intel_drv.h:1457 gen6_read32+0x1ca/0x1e0 [i915]()
> [  906.286234] RPM wakelock ref not held during HW access
> [  906.286235] Modules linked in:
> [  906.286236]  snd_hda_intel i915 drm_kms_helper drm msr snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec snd_hwdep x86_pkg_temp_thermal snd_hda_core i2c_algo_bit ehci_pci syscopyarea sysfillrect sysimgblt fb_sys_fops ehci_hcd r8169 xhci_pci mii xhci_hcd video [last unloaded: drm]
> [  906.286246] CPU: 0 PID: 12252 Comm: kms_pipe_crc_ba Tainted: G     U  W       4.4.0-rc6+ #45
> [  906.286247] Hardware name: Dell Inc. Inspiron 3847/088DT1       , BIOS A06 01/15/2015
> [  906.286248]  ffffffffc022c098 ffff880210dbbae0 ffffffff813e0e4f ffff880210dbbb28
> [  906.286250]  ffff880210dbbb18 ffffffff8105f5f2 ffff8801fff40000 0000000000046040
> [  906.286251]  ffff8801fff49170 ffff8801fff49170 ffff8801fff40000 ffff880210dbbb78
> [  906.286253] Call Trace:
> [  906.286256]  [<ffffffff813e0e4f>] dump_stack+0x44/0x55
> [  906.286259]  [<ffffffff8105f5f2>] warn_slowpath_common+0x82/0xc0
> [  906.286261]  [<ffffffff8105f67c>] warn_slowpath_fmt+0x4c/0x50
> [  906.286270]  [<ffffffffc007e29c>] ? drm_property_free_blob+0x8c/0xb0 [drm]
> [  906.286280]  [<ffffffffc01a32ea>] gen6_read32+0x1ca/0x1e0 [i915]
> [  906.286283]  [<ffffffff8172cd12>] ? mutex_lock+0x12/0x30
> [  906.286294]  [<ffffffffc01e0ef0>] hsw_ddi_wrpll_get_hw_state+0x40/0x50 [i915]
> [  906.286304]  [<ffffffffc01c3fa1>] intel_atomic_commit+0xd41/0x1740 [i915]
> [  906.286312]  [<ffffffffc008c597>] drm_atomic_commit+0x37/0x60 [drm]
> [  906.286316]  [<ffffffffc01373e6>] drm_atomic_helper_set_config+0x76/0xb0 [drm_kms_helper]
> [  906.286323]  [<ffffffffc008ac0a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
> [  906.286329]  [<ffffffffc007b852>] drm_mode_set_config_internal+0x62/0x100 [drm]
> [  906.286335]  [<ffffffffc007fcfd>] drm_mode_setcrtc+0x3cd/0x4e0 [drm]
> [  906.286339]  [<ffffffffc0071762>] drm_ioctl+0x152/0x540 [drm]
> [  906.286341]  [<ffffffff8109d914>] ? __wake_up+0x44/0x50
> [  906.286346]  [<ffffffffc007f930>] ? drm_mode_setplane+0x1b0/0x1b0 [drm]
> [  906.286348]  [<ffffffff811dc844>] ? mntput+0x24/0x40
> [  906.286350]  [<ffffffff811bfe22>] ? __fput+0x172/0x1e0
> [  906.286352]  [<ffffffff811d0758>] do_vfs_ioctl+0x288/0x460
> [  906.286353]  [<ffffffff811bfece>] ? ____fput+0xe/0x10
>
> v2:
>     Fixed return value in skl_ddi_pll_get_hw_state
>
> Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
>
Why do this in the get_hw_state functions? Seems more like it could be held by intel_modeset_check_state.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
  2015-12-31 15:52 ` [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
  2016-01-04 14:19   ` Maarten Lankhorst
@ 2016-01-04 15:05   ` Ville Syrjälä
  2016-01-05 13:54     ` Daniel Vetter
  2016-01-07  8:38     ` Joonas Lahtinen
  1 sibling, 2 replies; 10+ messages in thread
From: Ville Syrjälä @ 2016-01-04 15:05 UTC (permalink / raw)
  To: Gabriel Feceoru; +Cc: intel-gfx

On Thu, Dec 31, 2015 at 05:52:07PM +0200, Gabriel Feceoru wrote:
> This gets rid of errors like:
> 
> [  906.286213] ------------[ cut here ]------------
> [  906.286233] WARNING: CPU: 0 PID: 12252 at drivers/gpu/drm/i915/intel_drv.h:1457 gen6_read32+0x1ca/0x1e0 [i915]()
> [  906.286234] RPM wakelock ref not held during HW access
> [  906.286235] Modules linked in:
> [  906.286236]  snd_hda_intel i915 drm_kms_helper drm msr snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec snd_hwdep x86_pkg_temp_thermal snd_hda_core i2c_algo_bit ehci_pci syscopyarea sysfillrect sysimgblt fb_sys_fops ehci_hcd r8169 xhci_pci mii xhci_hcd video [last unloaded: drm]
> [  906.286246] CPU: 0 PID: 12252 Comm: kms_pipe_crc_ba Tainted: G     U  W       4.4.0-rc6+ #45
> [  906.286247] Hardware name: Dell Inc. Inspiron 3847/088DT1       , BIOS A06 01/15/2015
> [  906.286248]  ffffffffc022c098 ffff880210dbbae0 ffffffff813e0e4f ffff880210dbbb28
> [  906.286250]  ffff880210dbbb18 ffffffff8105f5f2 ffff8801fff40000 0000000000046040
> [  906.286251]  ffff8801fff49170 ffff8801fff49170 ffff8801fff40000 ffff880210dbbb78
> [  906.286253] Call Trace:
> [  906.286256]  [<ffffffff813e0e4f>] dump_stack+0x44/0x55
> [  906.286259]  [<ffffffff8105f5f2>] warn_slowpath_common+0x82/0xc0
> [  906.286261]  [<ffffffff8105f67c>] warn_slowpath_fmt+0x4c/0x50
> [  906.286270]  [<ffffffffc007e29c>] ? drm_property_free_blob+0x8c/0xb0 [drm]
> [  906.286280]  [<ffffffffc01a32ea>] gen6_read32+0x1ca/0x1e0 [i915]
> [  906.286283]  [<ffffffff8172cd12>] ? mutex_lock+0x12/0x30
> [  906.286294]  [<ffffffffc01e0ef0>] hsw_ddi_wrpll_get_hw_state+0x40/0x50 [i915]
> [  906.286304]  [<ffffffffc01c3fa1>] intel_atomic_commit+0xd41/0x1740 [i915]
> [  906.286312]  [<ffffffffc008c597>] drm_atomic_commit+0x37/0x60 [drm]
> [  906.286316]  [<ffffffffc01373e6>] drm_atomic_helper_set_config+0x76/0xb0 [drm_kms_helper]
> [  906.286323]  [<ffffffffc008ac0a>] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm]
> [  906.286329]  [<ffffffffc007b852>] drm_mode_set_config_internal+0x62/0x100 [drm]
> [  906.286335]  [<ffffffffc007fcfd>] drm_mode_setcrtc+0x3cd/0x4e0 [drm]
> [  906.286339]  [<ffffffffc0071762>] drm_ioctl+0x152/0x540 [drm]
> [  906.286341]  [<ffffffff8109d914>] ? __wake_up+0x44/0x50
> [  906.286346]  [<ffffffffc007f930>] ? drm_mode_setplane+0x1b0/0x1b0 [drm]
> [  906.286348]  [<ffffffff811dc844>] ? mntput+0x24/0x40
> [  906.286350]  [<ffffffff811bfe22>] ? __fput+0x172/0x1e0
> [  906.286352]  [<ffffffff811d0758>] do_vfs_ioctl+0x288/0x460
> [  906.286353]  [<ffffffff811bfece>] ? ____fput+0xe/0x10
> 
> v2:
>     Fixed return value in skl_ddi_pll_get_hw_state
> 
> Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 50 +++++++++++++++++++++++++++++-----------
>  1 file changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e6408e5..e92ed7a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2014,15 +2014,18 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  	enum intel_display_power_domain power_domain;
>  	u32 tmp;
>  	int i;
> +	bool ret = false;
>  
>  	power_domain = intel_display_port_power_domain(encoder);
>  	if (!intel_display_power_is_enabled(dev_priv, power_domain))
> -		return false;
> +		return ret;

I think what we really need here is some kind of
intel_display_power_get_unless_zero() thing. We need to make sure not
only that the rpm reference is held when reading out the state, but also
that the relevant power well(s) remain enabled while we're reading out
the state.


> +
> +	intel_runtime_pm_get(dev_priv);
>  
>  	tmp = I915_READ(DDI_BUF_CTL(port));
>  
>  	if (!(tmp & DDI_BUF_CTL_ENABLE))
> -		return false;
> +		goto out;
>  
>  	if (port == PORT_A) {
>  		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> @@ -2040,7 +2043,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  			break;
>  		}
>  
> -		return true;
> +		ret = true;
> +		goto out;
>  	} else {
>  		for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) {
>  			tmp = I915_READ(TRANS_DDI_FUNC_CTL(i));
> @@ -2048,17 +2052,19 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  			if ((tmp & TRANS_DDI_PORT_MASK)
>  			    == TRANS_DDI_SELECT_PORT(port)) {
>  				if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST)
> -					return false;
> +					goto out;
>  
>  				*pipe = i;
> -				return true;
> +				ret = true;
> +				goto out;
>  			}
>  		}
>  	}
>  
>  	DRM_DEBUG_KMS("No pipe for ddi port %c found\n", port_name(port));
> -
> -	return false;
> +out:
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
>  }
>  
>  void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> @@ -2510,7 +2516,10 @@ static bool hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
>  	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
>  		return false;
>  
> +	intel_runtime_pm_get(dev_priv);
>  	val = I915_READ(WRPLL_CTL(pll->id));
> +	intel_runtime_pm_put(dev_priv);
> +
>  	hw_state->wrpll = val;
>  
>  	return val & WRPLL_PLL_ENABLE;
> @@ -2525,7 +2534,10 @@ static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
>  	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
>  		return false;
>  
> +	intel_runtime_pm_get(dev_priv);
>  	val = I915_READ(SPLL_CTL);
> +	intel_runtime_pm_put(dev_priv);
> +
>  	hw_state->spll = val;
>  
>  	return val & SPLL_PLL_ENABLE;
> @@ -2644,16 +2656,19 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  	uint32_t val;
>  	unsigned int dpll;
>  	const struct skl_dpll_regs *regs = skl_dpll_regs;
> +	bool ret = false;
>  
>  	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
> -		return false;
> +		return ret;
>  
>  	/* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */
>  	dpll = pll->id + 1;
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	val = I915_READ(regs[pll->id].ctl);
>  	if (!(val & LCPLL_PLL_ENABLE))
> -		return false;
> +		goto out;
>  
>  	val = I915_READ(DPLL_CTRL1);
>  	hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f;
> @@ -2664,7 +2679,10 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  		hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2);
>  	}
>  
> -	return true;
> +	ret = true;
> +out:
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
>  }
>  
>  static void skl_shared_dplls_init(struct drm_i915_private *dev_priv)
> @@ -2931,13 +2949,16 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  {
>  	enum port port = (enum port)pll->id;	/* 1:1 port->PLL mapping */
>  	uint32_t val;
> +	bool ret = false;
>  
>  	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
> -		return false;
> +		return ret;
> +
> +	intel_runtime_pm_get(dev_priv);
>  
>  	val = I915_READ(BXT_PORT_PLL_ENABLE(port));
>  	if (!(val & PORT_PLL_ENABLE))
> -		return false;
> +		goto out;
>  
>  	hw_state->ebb0 = I915_READ(BXT_PORT_PLL_EBB_0(port));
>  	hw_state->ebb0 &= PORT_PLL_P1_MASK | PORT_PLL_P2_MASK;
> @@ -2984,7 +3005,10 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				 I915_READ(BXT_PORT_PCS_DW12_LN23(port)));
>  	hw_state->pcsdw12 &= LANE_STAGGER_MASK | LANESTAGGER_STRAP_OVRD;
>  
> -	return true;
> +	ret = true;
> +out:
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
>  }
>  
>  static void bxt_shared_dplls_init(struct drm_i915_private *dev_priv)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
  2016-01-04 15:05   ` Ville Syrjälä
@ 2016-01-05 13:54     ` Daniel Vetter
  2016-01-05 14:05       ` Marius Vlad
  2016-01-07  8:38     ` Joonas Lahtinen
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2016-01-05 13:54 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Mon, Jan 4, 2016 at 4:05 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> I think what we really need here is some kind of
> intel_display_power_get_unless_zero() thing. We need to make sure not
> only that the rpm reference is held when reading out the state, but also
> that the relevant power well(s) remain enabled while we're reading out
> the state.

Yeah, we need to check whether a given piece of hw is enabled or not.
All the get_hw_state funcs do that, using
intel_display_power_is_enabled. But it looks like
intel_ddi_get_hw_state partially gets this wrong. That would also
address the same backtrace in module unload.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
  2016-01-05 13:54     ` Daniel Vetter
@ 2016-01-05 14:05       ` Marius Vlad
  0 siblings, 0 replies; 10+ messages in thread
From: Marius Vlad @ 2016-01-05 14:05 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 1141 bytes --]

On Tue, Jan 05, 2016 at 02:54:31PM +0100, Daniel Vetter wrote:
> On Mon, Jan 4, 2016 at 4:05 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > I think what we really need here is some kind of
> > intel_display_power_get_unless_zero() thing. We need to make sure not
> > only that the rpm reference is held when reading out the state, but also
> > that the relevant power well(s) remain enabled while we're reading out
> > the state.
> 
> Yeah, we need to check whether a given piece of hw is enabled or not.
> All the get_hw_state funcs do that, using
> intel_display_power_is_enabled. But it looks like
> intel_ddi_get_hw_state partially gets this wrong. That would also
> address the same backtrace in module unload.

Tried __only__ this patch but it doesn't fix the trace in driver unload, had
the same hunch....

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
  2016-01-04 15:05   ` Ville Syrjälä
  2016-01-05 13:54     ` Daniel Vetter
@ 2016-01-07  8:38     ` Joonas Lahtinen
  1 sibling, 0 replies; 10+ messages in thread
From: Joonas Lahtinen @ 2016-01-07  8:38 UTC (permalink / raw)
  To: Ville Syrjälä, Gabriel Feceoru, Daniel Vetter; +Cc: intel-gfx

On ma, 2016-01-04 at 17:05 +0200, Ville Syrjälä wrote:
> On Thu, Dec 31, 2015 at 05:52:07PM +0200, Gabriel Feceoru wrote

<SNIP>

> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2014,15 +2014,18 @@ bool intel_ddi_get_hw_state(struct
> > intel_encoder *encoder,
> >  	enum intel_display_power_domain power_domain;
> >  	u32 tmp;
> >  	int i;
> > +	bool ret = false;
> >  
> >  	power_domain = intel_display_port_power_domain(encoder);
> >  	if (!intel_display_power_is_enabled(dev_priv,
> > power_domain))
> > -		return false;
> > +		return ret;
> 
> I think what we really need here is some kind of
> intel_display_power_get_unless_zero() thing. We need to make sure not
> only that the rpm reference is held when reading out the state, but
> also
> that the relevant power well(s) remain enabled while we're reading
> out
> the state.
> 

Once this gets merged to our trees, should be rather easy to add:

PM / runtime: Add new helper for conditional usage count incrementation

https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit
/?h=bleeding-edge&id=a436b6a19f57656a6557439523923d89eb4a880d

Regards, Joonas

> 
> > +
> > +	intel_runtime_pm_get(dev_priv);
> >  
> >  	tmp = I915_READ(DDI_BUF_CTL(port));
> >  
> >  	if (!(tmp & DDI_BUF_CTL_ENABLE))
> > -		return false;
> > +		goto out;
> >  
> >  	if (port == PORT_A) {
> >  		tmp =
> > I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> > @@ -2040,7 +2043,8 @@ bool intel_ddi_get_hw_state(struct
> > intel_encoder *encoder,
> >  			break;
> >  		}
> >  
> > -		return true;
> > +		ret = true;
> > +		goto out;
> >  	} else {
> >  		for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) {
> >  			tmp = I915_READ(TRANS_DDI_FUNC_CTL(i));
> > @@ -2048,17 +2052,19 @@ bool intel_ddi_get_hw_state(struct
> > intel_encoder *encoder,
> >  			if ((tmp & TRANS_DDI_PORT_MASK)
> >  			    == TRANS_DDI_SELECT_PORT(port)) {
> >  				if ((tmp &
> > TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST)
> > -					return false;
> > +					goto out;
> >  
> >  				*pipe = i;
> > -				return true;
> > +				ret = true;
> > +				goto out;
> >  			}
> >  		}
> >  	}
> >  
> >  	DRM_DEBUG_KMS("No pipe for ddi port %c found\n",
> > port_name(port));
> > -
> > -	return false;
> > +out:
> > +	intel_runtime_pm_put(dev_priv);
> > +	return ret;
> >  }
> >  
> >  void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> > @@ -2510,7 +2516,10 @@ static bool
> > hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
> >  	if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PLLS))
> >  		return false;
> >  
> > +	intel_runtime_pm_get(dev_priv);
> >  	val = I915_READ(WRPLL_CTL(pll->id));
> > +	intel_runtime_pm_put(dev_priv);
> > +
> >  	hw_state->wrpll = val;
> >  
> >  	return val & WRPLL_PLL_ENABLE;
> > @@ -2525,7 +2534,10 @@ static bool hsw_ddi_spll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >  	if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PLLS))
> >  		return false;
> >  
> > +	intel_runtime_pm_get(dev_priv);
> >  	val = I915_READ(SPLL_CTL);
> > +	intel_runtime_pm_put(dev_priv);
> > +
> >  	hw_state->spll = val;
> >  
> >  	return val & SPLL_PLL_ENABLE;
> > @@ -2644,16 +2656,19 @@ static bool skl_ddi_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >  	uint32_t val;
> >  	unsigned int dpll;
> >  	const struct skl_dpll_regs *regs = skl_dpll_regs;
> > +	bool ret = false;
> >  
> >  	if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PLLS))
> > -		return false;
> > +		return ret;
> >  
> >  	/* DPLL0 is not part of the shared DPLLs, so pll->id is 0
> > for DPLL1 */
> >  	dpll = pll->id + 1;
> >  
> > +	intel_runtime_pm_get(dev_priv);
> > +
> >  	val = I915_READ(regs[pll->id].ctl);
> >  	if (!(val & LCPLL_PLL_ENABLE))
> > -		return false;
> > +		goto out;
> >  
> >  	val = I915_READ(DPLL_CTRL1);
> >  	hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f;
> > @@ -2664,7 +2679,10 @@ static bool skl_ddi_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >  		hw_state->cfgcr2 = I915_READ(regs[pll
> > ->id].cfgcr2);
> >  	}
> >  
> > -	return true;
> > +	ret = true;
> > +out:
> > +	intel_runtime_pm_put(dev_priv);
> > +	return ret;
> >  }
> >  
> >  static void skl_shared_dplls_init(struct drm_i915_private
> > *dev_priv)
> > @@ -2931,13 +2949,16 @@ static bool bxt_ddi_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >  {
> >  	enum port port = (enum port)pll->id;	/* 1:1 port
> > ->PLL mapping */
> >  	uint32_t val;
> > +	bool ret = false;
> >  
> >  	if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PLLS))
> > -		return false;
> > +		return ret;
> > +
> > +	intel_runtime_pm_get(dev_priv);
> >  
> >  	val = I915_READ(BXT_PORT_PLL_ENABLE(port));
> >  	if (!(val & PORT_PLL_ENABLE))
> > -		return false;
> > +		goto out;
> >  
> >  	hw_state->ebb0 = I915_READ(BXT_PORT_PLL_EBB_0(port));
> >  	hw_state->ebb0 &= PORT_PLL_P1_MASK | PORT_PLL_P2_MASK;
> > @@ -2984,7 +3005,10 @@ static bool bxt_ddi_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >  				 I915_READ(BXT_PORT_PCS_DW12_LN23(
> > port)));
> >  	hw_state->pcsdw12 &= LANE_STAGGER_MASK |
> > LANESTAGGER_STRAP_OVRD;
> >  
> > -	return true;
> > +	ret = true;
> > +out:
> > +	intel_runtime_pm_put(dev_priv);
> > +	return ret;
> >  }
> >  
> >  static void bxt_shared_dplls_init(struct drm_i915_private
> > *dev_priv)
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-01-07  8:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-31 12:45 [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
2015-12-31 13:20 ` ✗ failure: Fi.CI.BAT Patchwork
2015-12-31 15:52 ` [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions Gabriel Feceoru
2016-01-04 14:19   ` Maarten Lankhorst
2016-01-04 15:05   ` Ville Syrjälä
2016-01-05 13:54     ` Daniel Vetter
2016-01-05 14:05       ` Marius Vlad
2016-01-07  8:38     ` Joonas Lahtinen
2015-12-31 16:20 ` ✗ warning: Fi.CI.BAT Patchwork
2016-01-04 12:55 ` [PATCH] drm/i915: Add RPM references in the *_get_hw_state functions Marius Vlad

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.