[i-g-t,08/10] lib/psr: Add PSR2 support to the remaning psr functions

Submitted by Souza, Jose on Jan. 12, 2019, 1:46 a.m.

Details

Message ID 20190112014607.13446-8-jose.souza@intel.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Souza, Jose Jan. 12, 2019, 1:46 a.m.
Add the mode parameter to psr_enable() and psr_sink_support() so PSR1
and PSR2 can be tested separated.
For now all PSR tests will run only with PSR1 and the tests for PSR2
will come in the future.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c                    | 39 +++++++++++++++++++++++++-------
 lib/igt_psr.h                    |  4 ++--
 tests/kms_fbcon_fbt.c            |  9 ++++++--
 tests/kms_frontbuffer_tracking.c |  4 ++--
 tests/kms_psr.c                  |  5 ++--
 5 files changed, 45 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 5cc0fbc2..d7028f6c 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -83,9 +83,10 @@  static void restore_psr_debugfs(int sig)
 	psr_write(psr_restore_debugfs_fd, "0");
 }
 
-static bool psr_set(int debugfs_fd, bool enable)
+static bool psr_set(int debugfs_fd, enum psr_mode mode)
 {
 	int ret;
+	const char *debug_val;
 
 	ret = has_psr_debugfs(debugfs_fd);
 	if (ret == -ENODEV) {
@@ -96,7 +97,18 @@  static bool psr_set(int debugfs_fd, bool enable)
 		return false;
 	}
 
-	ret = psr_write(debugfs_fd, enable ? "0x3" : "0x1");
+	switch (mode) {
+	case PSR_MODE_1:
+		debug_val = "0x3";
+		break;
+	case PSR_MODE_2:
+		debug_val = "0x2";
+		break;
+	default:
+		debug_val = "0x1";
+	}
+
+	ret = psr_write(debugfs_fd, debug_val);
 	igt_assert(ret > 0);
 
 	/* Restore original value on exit */
@@ -109,23 +121,34 @@  static bool psr_set(int debugfs_fd, bool enable)
 	return ret;
 }
 
-bool psr_enable(int debugfs_fd)
+bool psr_enable(int debugfs_fd, enum psr_mode mode)
 {
-	return psr_set(debugfs_fd, true);
+	return psr_set(debugfs_fd, mode);
 }
 
 bool psr_disable(int debugfs_fd)
 {
-	return psr_set(debugfs_fd, false);
+	/* Any mode different than PSR_MODE_1/2 will disable PSR */
+	return psr_set(debugfs_fd, PSR_MODE_2 + 1);
 }
 
-bool psr_sink_support(int debugfs_fd)
+bool psr_sink_support(int debugfs_fd, enum psr_mode mode)
 {
 	char buf[PSR_STATUS_MAX_LEN];
 	int ret;
 
 	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
 				      sizeof(buf));
-	return ret > 0 && (strstr(buf, "Sink_Support: yes\n") ||
-			   strstr(buf, "Sink support: yes"));
+	if (ret < 1)
+		return false;
+
+	if (mode == PSR_MODE_1)
+		return strstr(buf, "Sink_Support: yes\n") ||
+		       strstr(buf, "Sink support: yes");
+	else
+		/*
+		 * i915 requires PSR version 0x03 that is PSR2 + SU with
+		 * Y-coordinate to support PSR2
+		 */
+		return strstr(buf, "Sink support: yes [0x03]");
 }
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 4fff77ec..7e7017bf 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -37,8 +37,8 @@  enum psr_mode {
 
 bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
 bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
-bool psr_enable(int debugfs_fd);
+bool psr_enable(int debugfs_fd, enum psr_mode);
 bool psr_disable(int debugfs_fd);
-bool psr_sink_support(int debugfs_fd);
+bool psr_sink_support(int debugfs_fd, enum psr_mode);
 
 #endif
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 2823b47a..9d0d5a36 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -199,6 +199,11 @@  static bool psr_wait_until_enabled(int debugfs_fd)
 	return r;
 }
 
+static bool psr_supported_on_chipset(int debugfs_fd)
+{
+	return psr_sink_support(debugfs_fd, PSR_MODE_1);
+}
+
 static void disable_features(int debugfs_fd)
 {
 	igt_set_module_param_int("enable_fbc", 0);
@@ -212,7 +217,7 @@  static inline void fbc_modparam_enable(int debugfs_fd)
 
 static inline void psr_debugfs_enable(int debugfs_fd)
 {
-	psr_enable(debugfs_fd);
+	psr_enable(debugfs_fd, PSR_MODE_1);
 }
 
 struct feature {
@@ -226,7 +231,7 @@  struct feature {
 	.connector_possible_fn = connector_can_fbc,
 	.enable = fbc_modparam_enable,
 }, psr = {
-	.supported_on_chipset = psr_sink_support,
+	.supported_on_chipset = psr_supported_on_chipset,
 	.wait_until_enabled = psr_wait_until_enabled,
 	.connector_possible_fn = connector_can_psr,
 	.enable = psr_debugfs_enable,
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index ed9a039a..609f7b41 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1425,7 +1425,7 @@  static void setup_psr(void)
 		return;
 	}
 
-	if (!psr_sink_support(drm.debugfs)) {
+	if (!psr_sink_support(drm.debugfs, PSR_MODE_1)) {
 		igt_info("Can't test PSR: not supported by sink.\n");
 		return;
 	}
@@ -1722,7 +1722,7 @@  static bool enable_features_for_test(const struct test_mode *t)
 	if (t->feature & FEATURE_FBC)
 		fbc_enable();
 	if (t->feature & FEATURE_PSR)
-		ret = psr_enable(drm.debugfs);
+		ret = psr_enable(drm.debugfs, PSR_MODE_1);
 	if (t->feature & FEATURE_DRRS)
 		drrs_enable();
 
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 23d000bd..c31dc317 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -191,7 +191,8 @@  static void fill_render(data_t *data, uint32_t handle, unsigned char color)
 
 static bool sink_support(data_t *data)
 {
-	return data->with_psr_disabled || psr_sink_support(data->debugfs_fd);
+	return data->with_psr_disabled ||
+	       psr_sink_support(data->debugfs_fd, PSR_MODE_1);
 }
 
 static bool psr_wait_entry_if_enabled(data_t *data)
@@ -410,7 +411,7 @@  int main(int argc, char *argv[])
 		data.devid = intel_get_drm_devid(data.drm_fd);
 
 		if (!data.with_psr_disabled)
-			psr_enable(data.debugfs_fd);
+			psr_enable(data.debugfs_fd, PSR_MODE_1);
 
 		igt_require_f(sink_support(&data),
 			      "Sink does not support PSR\n");

Comments

Dhinakaran Pandiyan Jan. 16, 2019, 5:33 a.m.
On Fri, 2019-01-11 at 17:46 -0800, José Roberto de Souza wrote:
> Add the mode parameter to psr_enable() and psr_sink_support() so PSR1
> and PSR2 can be tested separated.
> For now all PSR tests will run only with PSR1 and the tests for PSR2
> will come in the future.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c                    | 39 +++++++++++++++++++++++++-----
> --
>  lib/igt_psr.h                    |  4 ++--
>  tests/kms_fbcon_fbt.c            |  9 ++++++--
>  tests/kms_frontbuffer_tracking.c |  4 ++--
>  tests/kms_psr.c                  |  5 ++--
>  5 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 5cc0fbc2..d7028f6c 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -83,9 +83,10 @@ static void restore_psr_debugfs(int sig)
>  	psr_write(psr_restore_debugfs_fd, "0");
>  }
>  
> -static bool psr_set(int debugfs_fd, bool enable)
> +static bool psr_set(int debugfs_fd, enum psr_mode mode)
>  {
>  	int ret;
> +	const char *debug_val;
>  
>  	ret = has_psr_debugfs(debugfs_fd);
>  	if (ret == -ENODEV) {
> @@ -96,7 +97,18 @@ static bool psr_set(int debugfs_fd, bool enable)
>  		return false;
>  	}
>  
> -	ret = psr_write(debugfs_fd, enable ? "0x3" : "0x1");
> +	switch (mode) {
> +	case PSR_MODE_1:
> +		debug_val = "0x3";
> +		break;
> +	case PSR_MODE_2:
> +		debug_val = "0x2";
> +		break;
> +	default:
> +		debug_val = "0x1";
> +	}
> +
> +	ret = psr_write(debugfs_fd, debug_val);
>  	igt_assert(ret > 0);
>  
>  	/* Restore original value on exit */
> @@ -109,23 +121,34 @@ static bool psr_set(int debugfs_fd, bool
> enable)
>  	return ret;
>  }
>  
> -bool psr_enable(int debugfs_fd)
> +bool psr_enable(int debugfs_fd, enum psr_mode mode)
>  {
> -	return psr_set(debugfs_fd, true);
> +	return psr_set(debugfs_fd, mode);
>  }
>  
>  bool psr_disable(int debugfs_fd)
>  {
> -	return psr_set(debugfs_fd, false);
> +	/* Any mode different than PSR_MODE_1/2 will disable PSR */
Please consider adding PSR_MODE_DISABLE to get rid of this comment.

> +	return psr_set(debugfs_fd, PSR_MODE_2 + 1);
>  }
>  
> -bool psr_sink_support(int debugfs_fd)
> +bool psr_sink_support(int debugfs_fd, enum psr_mode mode)
>  {
>  	char buf[PSR_STATUS_MAX_LEN];
>  	int ret;
>  
>  	ret = igt_debugfs_simple_read(debugfs_fd,
> "i915_edp_psr_status", buf,
>  				      sizeof(buf));
> -	return ret > 0 && (strstr(buf, "Sink_Support: yes\n") ||
> -			   strstr(buf, "Sink support: yes"));
> +	if (ret < 1)
> +		return false;
> +
> +	if (mode == PSR_MODE_1)
> +		return strstr(buf, "Sink_Support: yes\n") ||
> +		       strstr(buf, "Sink support: yes");
> +	else
> +		/*
> +		 * i915 requires PSR version 0x03 that is PSR2 + SU
> with
> +		 * Y-coordinate to support PSR2
> +		 */
> +		return strstr(buf, "Sink support: yes [0x03]");
For some reason, I thought we also print whether the sink supports PSR1
or PSR2 in debugfs. Hope it's not too late, did we ever consider

Sink support: {No, PSR1, PSR2} [<PSR dpcd version>]?

>  }
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index 4fff77ec..7e7017bf 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -37,8 +37,8 @@ enum psr_mode {
>  
>  bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
>  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
> -bool psr_enable(int debugfs_fd);
> +bool psr_enable(int debugfs_fd, enum psr_mode);
>  bool psr_disable(int debugfs_fd);
> -bool psr_sink_support(int debugfs_fd);
> +bool psr_sink_support(int debugfs_fd, enum psr_mode);
>  
>  #endif
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index 2823b47a..9d0d5a36 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -199,6 +199,11 @@ static bool psr_wait_until_enabled(int
> debugfs_fd)
>  	return r;
>  }
>  
> +static bool psr_supported_on_chipset(int debugfs_fd)
> +{
> +	return psr_sink_support(debugfs_fd, PSR_MODE_1);
> +}
> +
>  static void disable_features(int debugfs_fd)
>  {
>  	igt_set_module_param_int("enable_fbc", 0);
> @@ -212,7 +217,7 @@ static inline void fbc_modparam_enable(int
> debugfs_fd)
>  
>  static inline void psr_debugfs_enable(int debugfs_fd)
>  {
> -	psr_enable(debugfs_fd);
> +	psr_enable(debugfs_fd, PSR_MODE_1);
>  }
>  
>  struct feature {
> @@ -226,7 +231,7 @@ struct feature {
>  	.connector_possible_fn = connector_can_fbc,
>  	.enable = fbc_modparam_enable,
>  }, psr = {
> -	.supported_on_chipset = psr_sink_support,
> +	.supported_on_chipset = psr_supported_on_chipset,
>  	.wait_until_enabled = psr_wait_until_enabled,
>  	.connector_possible_fn = connector_can_psr,
>  	.enable = psr_debugfs_enable,
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index ed9a039a..609f7b41 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -1425,7 +1425,7 @@ static void setup_psr(void)
>  		return;
>  	}
>  
> -	if (!psr_sink_support(drm.debugfs)) {
> +	if (!psr_sink_support(drm.debugfs, PSR_MODE_1)) {
>  		igt_info("Can't test PSR: not supported by sink.\n");
>  		return;
>  	}
> @@ -1722,7 +1722,7 @@ static bool enable_features_for_test(const
> struct test_mode *t)
>  	if (t->feature & FEATURE_FBC)
>  		fbc_enable();
>  	if (t->feature & FEATURE_PSR)
> -		ret = psr_enable(drm.debugfs);
> +		ret = psr_enable(drm.debugfs, PSR_MODE_1);
>  	if (t->feature & FEATURE_DRRS)
>  		drrs_enable();
>  
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 23d000bd..c31dc317 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -191,7 +191,8 @@ static void fill_render(data_t *data, uint32_t
> handle, unsigned char color)
>  
>  static bool sink_support(data_t *data)
>  {
> -	return data->with_psr_disabled || psr_sink_support(data-
> >debugfs_fd);
> +	return data->with_psr_disabled ||
> +	       psr_sink_support(data->debugfs_fd, PSR_MODE_1);
>  }
>  
>  static bool psr_wait_entry_if_enabled(data_t *data)
> @@ -410,7 +411,7 @@ int main(int argc, char *argv[])
>  		data.devid = intel_get_drm_devid(data.drm_fd);
>  
Store it in struct data so that the value stays consistent throughout
the test case?
>  		if (!data.with_psr_disabled)
> -			psr_enable(data.debugfs_fd);
> +			psr_enable(data.debugfs_fd, PSR_MODE_1);
>  
>  		igt_require_f(sink_support(&data),
>  			      "Sink does not support PSR\n");