[PATCH] drm/i915: Handle runtime pm in the CRC setup code

* [PATCH] drm/i915: Handle runtime pm in the CRC setup code
@ 2014-11-24 15:23 Daniel Vetter
  2014-11-24 15:36 ` Damien Lespiau
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Daniel Vetter @ 2014-11-24 15:23 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter

The crc code doesn't handle anything really that could drop the
register state (by design so that we have less complexity). Which
means userspace may only start crc capture once the pipe is fully set
up.

With an i-g-t patch this will be the case, but there's still the
problem that this results in obscure unclaimed register write
failures. Which is a pain to debug.

So instead make sure we don't have the basic unclaimed register write
failure by grabbing runtime pm references. And reject completely
invalid requests with -EIO. This is still racy of course, but for a
test library we don't really care - if userspace shuts down the pipe
right afterwards the entire setup will be lost anyway.

References: https://bugs.freedesktop.org/show_bug.cgi?id=86092
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 319da61354b0..68a76e58e8fd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3309,6 +3309,14 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 	if (pipe_crc->source && source)
 		return -EINVAL;
 
+	intel_runtime_pm_get(dev_priv);
+
+	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PIPE(pipe))) {
+		DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
+		ret = -EIO;
+		goto out;
+	}
+
 	if (IS_GEN2(dev))
 		ret = i8xx_pipe_crc_ctl_reg(&source, &val);
 	else if (INTEL_INFO(dev)->gen < 5)
@@ -3321,7 +3329,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 		ret = ivb_pipe_crc_ctl_reg(dev, pipe, &source, &val);
 
 	if (ret != 0)
-		return ret;
+		goto out;
 
 	/* none -> real source transition */
 	if (source) {
@@ -3331,8 +3339,10 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 		pipe_crc->entries = kzalloc(sizeof(*pipe_crc->entries) *
 					    INTEL_PIPE_CRC_ENTRIES_NR,
 					    GFP_KERNEL);
-		if (!pipe_crc->entries)
-			return -ENOMEM;
+		if (!pipe_crc->entries) {
+			ret = -ENOMEM;
+			goto out;
+		}
 
 		/*
 		 * When IPS gets enabled, the pipe CRC changes. Since IPS gets
@@ -3383,8 +3393,10 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 
 		hsw_enable_ips(crtc);
 	}
+out:
+	intel_runtime_pm_get(dev_priv);
 
-	return 0;
+	return ret;
 }
 
 /*
-- 
2.1.1

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

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