[Intel-gfx] [PATCH 7/7] drm/i915/psr: Disable DRRS if enabled when enabling PSR from debugfs

[Maarten Lankhorst]

Op 09-11-18 om 21:20 schreef José Roberto de Souza:
> If panel supports DRRS and PSR and if driver is loaded without PSR
> enabled, driver will enable DRRS as expected but if PSR is enabled by
> debugfs latter it will keep PSR and DRRS enabled causing possible
> problems as DRRS will lower the refresh rate while PSR enabled.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108341
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 853e3f1370a0..bfc6a08b5cf4 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -904,8 +904,11 @@ int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
>  
>  	intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
>  
> -	if (dev_priv->psr.prepared && enable)
> +	if (dev_priv->psr.prepared && enable) {
> +		if (crtc_state)
> +			intel_edp_drrs_disable(dp, crtc_state);
>  		intel_psr_enable_locked(dev_priv, crtc_state);
> +	}
>  
>  	mutex_unlock(&dev_priv->psr.lock);
>  	return ret;

I've considered this, but I thought it was a feature, not a bug. It's a pain to track
how we handle this as intended.

kms_frontbuffer_tracking is also controlling DRRS during the test, so perhaps simply
fix the test?

It seems the no_drrs test simply checks that if PSR is enabled, we don't have drrs
enabled. We probably care about the default configuration, so I would simply disable
the pipe, update the PSR flag, and then start running the tests. Else the only thing
we test is that debugfs disables DRRS. Not that the default modeset path prevents
PSR and DRRS simultaneously.

~Maarten

Maybe something like below?

Perhaps move the drrs manipulation functions from kms_frontbuffer_tracking to lib/kms_psr.c

----8<-------
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 9767f475bf23..ffc356df06ce 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -414,9 +414,6 @@ int main(int argc, char *argv[])
 		kmstest_set_vt_graphics_mode();
 		data.devid = intel_get_drm_devid(data.drm_fd);
 
-		if (!data.with_psr_disabled)
-			psr_enable(data.debugfs_fd);
-
 		igt_require_f(sink_support(&data),
 			      "Sink does not support PSR\n");
 
@@ -428,18 +425,25 @@ int main(int argc, char *argv[])
 	}
 
 	igt_subtest("basic") {
-		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
-		igt_assert(psr_wait_entry_if_enabled(&data));
-		test_cleanup(&data);
-	}
+		/* Disable display to get a default setup. */
+		igt_display_commit2(&data.display, data.display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
+
+		if (!data.with_psr_disabled)
+			psr_enable(data.debugfs_fd);
 
-	igt_subtest("no_drrs") {
 		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
 		igt_assert(psr_wait_entry_if_enabled(&data));
 		igt_assert(drrs_disabled(&data));
 		test_cleanup(&data);
 	}
 
+	igt_fixture {
+		drrs_disable();
+
+		if (!data.with_psr_disabled)
+			psr_enable(data.debugfs_fd);
+	}
+
 	for (op = PAGE_FLIP; op <= RENDER; op++) {
 		igt_subtest_f("primary_%s", op_str(op)) {
 			data.op = op;