[i-g-t,2/8] tests: Share the code handling PSR debugfs parsing

Submitted by Souza, Jose on Dec. 4, 2018, 11:09 p.m.

Details

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

Commit Message

Souza, Jose Dec. 4, 2018, 11:09 p.m.
The same code checking if sink supports PSR was spread into 3 tests,
better move it to lib and reuse.
Also kms_fbcon_fbt was doing its own handling to wait for PSR to get
enabled while it is already available in lib.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c                    |  9 +++++++++
 lib/igt_psr.h                    |  1 +
 tests/kms_fbcon_fbt.c            | 26 ++------------------------
 tests/kms_frontbuffer_tracking.c |  8 +-------
 tests/kms_psr.c                  |  8 +-------
 5 files changed, 14 insertions(+), 38 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index c7bc523c..d68f4b49 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -131,3 +131,12 @@  bool psr_disable(int debugfs_fd)
 {
 	return psr_set(debugfs_fd, false);
 }
+
+bool psr_supported(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				sizeof(buf));
+	return strstr(buf, "Sink_Support: yes\n");
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index b9693822..a4fcf325 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -32,5 +32,6 @@  bool psr_wait_entry(int debugfs_fd);
 bool psr_wait_exit(int debugfs_fd);
 bool psr_enable(int debugfs_fd);
 bool psr_disable(int debugfs_fd);
+bool psr_supported(int debugfs_fd);
 
 #endif
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 24d3ad90..32407709 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -168,19 +168,6 @@  static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
 	igt_assert_eq(rc, 0);
 }
 
-static bool psr_supported_on_chipset(int debugfs_fd)
-{
-	char buf[256];
-	int ret;
-
-	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status",
-				      buf, sizeof(buf));
-	if (ret < 0)
-		return false;
-
-	return strstr(buf, "Sink_Support: yes\n");
-}
-
 static bool connector_can_psr(drmModeConnectorPtr connector)
 {
 	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
@@ -195,18 +182,9 @@  static void psr_print_status(int debugfs_fd)
 	igt_debug("PSR status: %s\n", buf);
 }
 
-static bool psr_is_enabled(int debugfs_fd)
-{
-	char buf[256];
-
-	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
-				sizeof(buf));
-	return strstr(buf, "\nHW Enabled & Active bit: yes\n");
-}
-
 static bool psr_wait_until_enabled(int debugfs_fd)
 {
-	bool r = igt_wait(psr_is_enabled(debugfs_fd), 5000, 1);
+	bool r = psr_wait_entry(debugfs_fd);
 
 	psr_print_status(debugfs_fd);
 	return r;
@@ -239,7 +217,7 @@  struct feature {
 	.connector_possible_fn = connector_can_fbc,
 	.enable = fbc_modparam_enable,
 }, psr = {
-	.supported_on_chipset = psr_supported_on_chipset,
+	.supported_on_chipset = psr_supported,
 	.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 276ef83c..c419557e 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1419,13 +1419,7 @@  static void teardown_fbc(void)
 
 static bool psr_sink_has_support(void)
 {
-	char buf[256];
-
-	debugfs_read("i915_edp_psr_status", buf);
-	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
-		return false;
-
-	return strstr(buf, "Sink_Support: yes\n");
+	return psr_supported(drm.debugfs);
 }
 
 static void setup_psr(void)
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index d00e552f..116fe409 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -191,13 +191,7 @@  static void fill_render(data_t *data, uint32_t handle, unsigned char color)
 
 static bool sink_support(data_t *data)
 {
-	char buf[512];
-
-	igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
-			 buf, sizeof(buf));
-
-	return data->with_psr_disabled ||
-		strstr(buf, "Sink_Support: yes\n");
+	return data->with_psr_disabled || psr_supported(data->debugfs_fd);
 }
 
 static bool psr_wait_entry_if_enabled(data_t *data)

Comments

Dhinakaran Pandiyan Dec. 11, 2018, 11:06 p.m.
On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> The same code checking if sink supports PSR was spread into 3 tests,
> better move it to lib and reuse.
Agreed.

> Also kms_fbcon_fbt was doing its own handling to wait for PSR to get
> enabled while it is already available in lib.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c                    |  9 +++++++++
>  lib/igt_psr.h                    |  1 +
>  tests/kms_fbcon_fbt.c            | 26 ++------------------------
>  tests/kms_frontbuffer_tracking.c |  8 +-------
>  tests/kms_psr.c                  |  8 +-------
>  5 files changed, 14 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index c7bc523c..d68f4b49 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -131,3 +131,12 @@ bool psr_disable(int debugfs_fd)
>  {
>  	return psr_set(debugfs_fd, false);
>  }
> +
> +bool psr_supported(int debugfs_fd)
psr_sink_support() in order to be descriptive about what support we are
referring to.

> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +
> +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> +				sizeof(buf));
> +	return strstr(buf, "Sink_Support: yes\n");
> +}
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index b9693822..a4fcf325 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -32,5 +32,6 @@ bool psr_wait_entry(int debugfs_fd);
>  bool psr_wait_exit(int debugfs_fd);
>  bool psr_enable(int debugfs_fd);
>  bool psr_disable(int debugfs_fd);
> +bool psr_supported(int debugfs_fd);
>  
>  #endif
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index 24d3ad90..32407709 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -168,19 +168,6 @@ static void set_mode_for_one_screen(struct
> drm_info *drm, struct igt_fb *fb,
>  	igt_assert_eq(rc, 0);
>  }
>  
> -static bool psr_supported_on_chipset(int debugfs_fd)
> -{
> -	char buf[256];
> -	int ret;
> -
> -	ret = igt_debugfs_simple_read(debugfs_fd,
> "i915_edp_psr_status",
> -				      buf, sizeof(buf));
> -	if (ret < 0)
> -		return false;
> -
> -	return strstr(buf, "Sink_Support: yes\n");
> -}
> -
>  static bool connector_can_psr(drmModeConnectorPtr connector)
>  {
>  	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
> @@ -195,18 +182,9 @@ static void psr_print_status(int debugfs_fd)
>  	igt_debug("PSR status: %s\n", buf);
>  }
>  
> -static bool psr_is_enabled(int debugfs_fd)
> -{
> -	char buf[256];
> -
> -	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> -				sizeof(buf));
> -	return strstr(buf, "\nHW Enabled & Active bit: yes\n");
> -}
> -
>  static bool psr_wait_until_enabled(int debugfs_fd)
>  {
> -	bool r = igt_wait(psr_is_enabled(debugfs_fd), 5000, 1);
> +	bool r = psr_wait_entry(debugfs_fd);
Please split this change.

>  
>  	psr_print_status(debugfs_fd);
>  	return r;
> @@ -239,7 +217,7 @@ struct feature {
>  	.connector_possible_fn = connector_can_fbc,
>  	.enable = fbc_modparam_enable,
>  }, psr = {
> -	.supported_on_chipset = psr_supported_on_chipset,
> +	.supported_on_chipset = psr_supported,
>  	.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 276ef83c..c419557e 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -1419,13 +1419,7 @@ static void teardown_fbc(void)
>  
>  static bool psr_sink_has_support(void)
>  {
> -	char buf[256];
> -
> -	debugfs_read("i915_edp_psr_status", buf);
> -	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
> -		return false;
> -
> -	return strstr(buf, "Sink_Support: yes\n");
> +	return psr_supported(drm.debugfs);
Suggest inlining at the only call site.

>  }
>  
>  static void setup_psr(void)
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index d00e552f..116fe409 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -191,13 +191,7 @@ static void fill_render(data_t *data, uint32_t
> handle, unsigned char color)
>  
>  static bool sink_support(data_t *data)
>  {
> -	char buf[512];
> -
> -	igt_debugfs_simple_read(data->debugfs_fd,
> "i915_edp_psr_status",
> -			 buf, sizeof(buf));
> -
> -	return data->with_psr_disabled ||
> -		strstr(buf, "Sink_Support: yes\n");
> +	return data->with_psr_disabled || psr_supported(data-
> >debugfs_fd);
>  }
>  
>  static bool psr_wait_entry_if_enabled(data_t *data)
Souza, Jose Dec. 13, 2018, 7:53 p.m.
On Tue, 2018-12-11 at 15:06 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> > The same code checking if sink supports PSR was spread into 3
> > tests,
> > better move it to lib and reuse.
> Agreed.
> 
> > Also kms_fbcon_fbt was doing its own handling to wait for PSR to
> > get
> > enabled while it is already available in lib.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  lib/igt_psr.c                    |  9 +++++++++
> >  lib/igt_psr.h                    |  1 +
> >  tests/kms_fbcon_fbt.c            | 26 ++------------------------
> >  tests/kms_frontbuffer_tracking.c |  8 +-------
> >  tests/kms_psr.c                  |  8 +-------
> >  5 files changed, 14 insertions(+), 38 deletions(-)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index c7bc523c..d68f4b49 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -131,3 +131,12 @@ bool psr_disable(int debugfs_fd)
> >  {
> >  	return psr_set(debugfs_fd, false);
> >  }
> > +
> > +bool psr_supported(int debugfs_fd)
> psr_sink_support() in order to be descriptive about what support we
> are
> referring to.

Done

> 
> > +{
> > +	char buf[PSR_STATUS_MAX_LEN];
> > +
> > +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> > +				sizeof(buf));
> > +	return strstr(buf, "Sink_Support: yes\n");
> > +}
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > index b9693822..a4fcf325 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -32,5 +32,6 @@ bool psr_wait_entry(int debugfs_fd);
> >  bool psr_wait_exit(int debugfs_fd);
> >  bool psr_enable(int debugfs_fd);
> >  bool psr_disable(int debugfs_fd);
> > +bool psr_supported(int debugfs_fd);
> >  
> >  #endif
> > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> > index 24d3ad90..32407709 100644
> > --- a/tests/kms_fbcon_fbt.c
> > +++ b/tests/kms_fbcon_fbt.c
> > @@ -168,19 +168,6 @@ static void set_mode_for_one_screen(struct
> > drm_info *drm, struct igt_fb *fb,
> >  	igt_assert_eq(rc, 0);
> >  }
> >  
> > -static bool psr_supported_on_chipset(int debugfs_fd)
> > -{
> > -	char buf[256];
> > -	int ret;
> > -
> > -	ret = igt_debugfs_simple_read(debugfs_fd,
> > "i915_edp_psr_status",
> > -				      buf, sizeof(buf));
> > -	if (ret < 0)
> > -		return false;
> > -
> > -	return strstr(buf, "Sink_Support: yes\n");
> > -}
> > -
> >  static bool connector_can_psr(drmModeConnectorPtr connector)
> >  {
> >  	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
> > @@ -195,18 +182,9 @@ static void psr_print_status(int debugfs_fd)
> >  	igt_debug("PSR status: %s\n", buf);
> >  }
> >  
> > -static bool psr_is_enabled(int debugfs_fd)
> > -{
> > -	char buf[256];
> > -
> > -	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> > -				sizeof(buf));
> > -	return strstr(buf, "\nHW Enabled & Active bit: yes\n");
> > -}
> > -
> >  static bool psr_wait_until_enabled(int debugfs_fd)
> >  {
> > -	bool r = igt_wait(psr_is_enabled(debugfs_fd), 5000, 1);
> > +	bool r = psr_wait_entry(debugfs_fd);
> Please split this change.

Done and changing commit message from this patch

> 
> >  
> >  	psr_print_status(debugfs_fd);
> >  	return r;
> > @@ -239,7 +217,7 @@ struct feature {
> >  	.connector_possible_fn = connector_can_fbc,
> >  	.enable = fbc_modparam_enable,
> >  }, psr = {
> > -	.supported_on_chipset = psr_supported_on_chipset,
> > +	.supported_on_chipset = psr_supported,
> >  	.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 276ef83c..c419557e 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -1419,13 +1419,7 @@ static void teardown_fbc(void)
> >  
> >  static bool psr_sink_has_support(void)
> >  {
> > -	char buf[256];
> > -
> > -	debugfs_read("i915_edp_psr_status", buf);
> > -	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
> > -		return false;
> > -
> > -	return strstr(buf, "Sink_Support: yes\n");
> > +	return psr_supported(drm.debugfs);
> Suggest inlining at the only call site.

Done

> 
> >  }
> >  
> >  static void setup_psr(void)
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index d00e552f..116fe409 100644
> > --- a/tests/kms_psr.c
> > +++ b/tests/kms_psr.c
> > @@ -191,13 +191,7 @@ static void fill_render(data_t *data, uint32_t
> > handle, unsigned char color)
> >  
> >  static bool sink_support(data_t *data)
> >  {
> > -	char buf[512];
> > -
> > -	igt_debugfs_simple_read(data->debugfs_fd,
> > "i915_edp_psr_status",
> > -			 buf, sizeof(buf));
> > -
> > -	return data->with_psr_disabled ||
> > -		strstr(buf, "Sink_Support: yes\n");
> > +	return data->with_psr_disabled || psr_supported(data-
> > > debugfs_fd);
> >  }
> >  
> >  static bool psr_wait_entry_if_enabled(data_t *data)