[igt-dev] [PATCH i-g-t 0/5] extend kms_psr test to validate pr

[igt-dev] [PATCH i-g-t 0/5] extend kms_psr test to validate pr

[Kunal Joshi]

Panel Replay behaves same as PSR for DP connector,
modify kms_psr test to validate pr feature as well

Kunal Joshi (5):
  lib/igt_psr: added enum for panel replay
  lib/igt_psr: modified function for pr compaitability
  lib/igt_psr: added function to check output supports pr
  lib/igt_psr: added function to check active dp pr
  tests/intel/kms_psr: made test compaitable with pr

 lib/igt_psr.c         | 104 ++++++++---
 lib/igt_psr.h         |   5 +-
 tests/intel/kms_psr.c | 402 ++++++++++++++++++++++++++++++++++--------
 3 files changed, 414 insertions(+), 97 deletions(-)

-- 
2.25.1

[igt-dev] [PATCH i-g-t 1/5] lib/igt_psr: added enum for panel replay

[Kunal Joshi]

Added enum for PR

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_psr.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 12ffc9d6c..243154897 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -34,6 +34,8 @@ enum psr_mode {
 	PSR_MODE_1,
 	PSR_MODE_2,
 	PSR_MODE_2_SEL_FETCH,
+	PR_MODE,
+	PR_MODE_SEL_FETCH,
 	PSR_DISABLED,
 };
 
-- 
2.25.1

[igt-dev] [PATCH i-g-t 2/5] lib/igt_psr: modified function for pr compaitability

[Kunal Joshi]

modified psr_active_check and psr_sink_support for
pr cases.

note :- this will be usefull for eDP PR not needed for DP PR

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_psr.c | 64 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 25 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 13f7c567d..acacfa31f 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -50,7 +50,7 @@ bool psr2_selective_fetch_check(int debugfs_fd)
 static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
 {
 	char buf[PSR_STATUS_MAX_LEN];
-	const char *state = mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
+	const char *state = mode == PSR_MODE_1 || PR_MODE ? "SRDENT" : "DEEP_SLEEP";
 	int ret;
 
 	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status",
@@ -162,6 +162,12 @@ static bool psr_set(int device, int debugfs_fd, int mode)
 		case PSR_MODE_2_SEL_FETCH:
 			debug_val = "0x4";
 			break;
+		case PR_MODE:
+			debug_val = "0x5";
+			break;
+		case PR_MODE_SEL_FETCH:
+			debug_val = "0x6";
+			break;
 		default:
 			/* Disables PSR */
 			debug_val = "0x1";
@@ -194,29 +200,35 @@ bool psr_disable(int device, int debugfs_fd)
 
 bool psr_sink_support(int device, 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));
-	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
-		 *
-		 * or
-		 *
-		 * PSR version 0x4 that is PSR2 + SU w/ Y-coordinate and SU
-		 * Region Early Transport to support PSR2 (eDP 1.5)
-		 */
-		return strstr(buf, "Sink support: yes [0x03]") ||
-		       strstr(buf, "Sink support: yes [0x04]");
+	char *line;
+    char buf[PSR_STATUS_MAX_LEN];
+    int ret;
+
+    ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+                                  sizeof(buf));
+    if (ret < 1)
+        return false;
+
+    line = strstr(buf, "Sink support: ");
+    if (!line)
+        return false;
+
+    if (mode == PSR_MODE_1)
+        return strstr(line, "PSR = yes");
+    else if (mode == PSR_MODE_2 || mode == PSR_MODE_2_SEL_FETCH)
+        /*
+         * i915 requires PSR version 0x03 that is PSR2 + SU with
+         * Y-coordinate to support PSR2
+         *
+         * or
+         *
+         * PSR version 0x4 that is PSR2 + SU w/ Y-coordinate and SU
+         * Region Early Transport to support PSR2 (eDP 1.5)
+         */
+        return strstr(line, "PSR = yes") &&
+               (strstr(line, "[0x03]") || strstr(line, "[0x04]"));
+    else
+        return strstr(line, "Panel Replay = yes");
 }
 
 #define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
@@ -350,7 +362,9 @@ enum psr_mode psr_get_mode(int debugfs_fd)
 		return PSR_DISABLED;
 	}
 
-	if (strstr(buf, "PSR2 selective fetch: enabled"))
+	if (strstr(buf, "Panel Replay Enabled"))
+		return PR_MODE;
+	else if (strstr(buf, "PSR2 selective fetch: enabled"))
 		return PSR_MODE_2_SEL_FETCH;
 	else if (strstr(buf, "PSR2 enabled"))
 		return PSR_MODE_2;
-- 
2.25.1

[igt-dev] [PATCH i-g-t 3/5] lib/igt_psr: added function to check output supports pr

[Kunal Joshi]

added function to check output supports PR

Note :- New debugfs interfaces which are used
        will be added by below patch in the driver
        https://patchwork.freedesktop.org/series/94470/

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_psr.c | 17 +++++++++++++++++
 lib/igt_psr.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index acacfa31f..4c912a064 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -22,6 +22,7 @@
  */
 
 #include "drmtest.h"
+#include "igt_kms.h"
 #include "igt_params.h"
 #include "igt_psr.h"
 #include "igt_sysfs.h"
@@ -373,3 +374,19 @@ enum psr_mode psr_get_mode(int debugfs_fd)
 
 	return PSR_DISABLED;
 }
+
+bool output_supports_pr(int debugfs_fd, igt_output_t *output)
+{
+	char debugfs_file[128] = {0};
+	char buf[PSR_STATUS_MAX_LEN];
+
+	if (!strstr(output->name,"DP"))
+		return false;
+
+        sprintf(debugfs_file, "%s/i915_psr_status", output->name);
+
+        igt_debugfs_simple_read(debugfs_fd, debugfs_file,
+				buf, sizeof(buf));
+
+        return strstr(buf, "Panel Replay = yes");
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 243154897..8e36fcf2f 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -55,5 +55,6 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
 
 bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
 void i915_psr2_sel_fetch_restore(int drm_fd);
+bool output_supports_pr(int debugfs_fd, igt_output_t *output);
 
 #endif
-- 
2.25.1

Re: [igt-dev] [PATCH i-g-t 3/5] lib/igt_psr: added function to check output supports pr

[Hogander, Jouni]

On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> added function to check output supports PR
> 
> Note :- New debugfs interfaces which are used
>         will be added by below patch in the driver
>         https://patchwork.freedesktop.org/series/94470/
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  lib/igt_psr.c | 17 +++++++++++++++++
>  lib/igt_psr.h |  1 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index acacfa31f..4c912a064 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -22,6 +22,7 @@
>   */
>  
>  #include "drmtest.h"
> +#include "igt_kms.h"
>  #include "igt_params.h"
>  #include "igt_psr.h"
>  #include "igt_sysfs.h"
> @@ -373,3 +374,19 @@ enum psr_mode psr_get_mode(int debugfs_fd)
>  
>         return PSR_DISABLED;
>  }
> +
> +bool output_supports_pr(int debugfs_fd, igt_output_t *output)
> +{
> +       char debugfs_file[128] = {0};
> +       char buf[PSR_STATUS_MAX_LEN];
> +
> +       if (!strstr(output->name,"DP"))
> +               return false;
> +
> +        sprintf(debugfs_file, "%s/i915_psr_status", output->name);
> +
> +        igt_debugfs_simple_read(debugfs_fd, debugfs_file,
> +                               buf, sizeof(buf));
> +
> +        return strstr(buf, "Panel Replay = yes");
> +}

Why do you think psr_sink_support couldn't work for DP as well?

BR,

Jouni Högander

> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index 243154897..8e36fcf2f 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -55,5 +55,6 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
>  
>  bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
>  void i915_psr2_sel_fetch_restore(int drm_fd);
> +bool output_supports_pr(int debugfs_fd, igt_output_t *output);
>  
>  #endif

Re: [igt-dev] [PATCH i-g-t 3/5] lib/igt_psr: added function to check output supports pr

[Joshi, Kunal1]

Hello Jouni,

-----Original Message-----
From: Hogander, Jouni <jouni.hogander@intel.com> 
Sent: Friday, October 20, 2023 5:43 PM
To: Joshi, Kunal1 <kunal1.joshi@intel.com>; igt-dev@lists.freedesktop.org
Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh <animesh.manna@intel.com>
Subject: Re: [PATCH i-g-t 3/5] lib/igt_psr: added function to check output supports pr

On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> added function to check output supports PR
> 
> Note :- New debugfs interfaces which are used
>         will be added by below patch in the driver
>         https://patchwork.freedesktop.org/series/94470/
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  lib/igt_psr.c | 17 +++++++++++++++++
>  lib/igt_psr.h |  1 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c index acacfa31f..4c912a064 
> 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -22,6 +22,7 @@
>   */
>  
>  #include "drmtest.h"
> +#include "igt_kms.h"
>  #include "igt_params.h"
>  #include "igt_psr.h"
>  #include "igt_sysfs.h"
> @@ -373,3 +374,19 @@ enum psr_mode psr_get_mode(int debugfs_fd)
>  
>         return PSR_DISABLED;
>  }
> +
> +bool output_supports_pr(int debugfs_fd, igt_output_t *output) {
> +       char debugfs_file[128] = {0};
> +       char buf[PSR_STATUS_MAX_LEN];
> +
> +       if (!strstr(output->name,"DP"))
> +               return false;
> +
> +        sprintf(debugfs_file, "%s/i915_psr_status", output->name);
> +
> +        igt_debugfs_simple_read(debugfs_fd, debugfs_file,
> +                               buf, sizeof(buf));
> +
> +        return strstr(buf, "Panel Replay = yes"); }

> Why do you think psr_sink_support couldn't work for DP as well?
>
> BR,

> Jouni Högander

Jouni psr_sink_support won't work for DP, psr_sink_support is modified
for eDP only. Changes are based on driver changes done in
https://patchwork.freedesktop.org/series/94470/


> diff --git a/lib/igt_psr.h b/lib/igt_psr.h index 243154897..8e36fcf2f 
> 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -55,5 +55,6 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
>  
>  bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
>  void i915_psr2_sel_fetch_restore(int drm_fd);
> +bool output_supports_pr(int debugfs_fd, igt_output_t *output);
>  
>  #endif

Re: [igt-dev] [PATCH i-g-t 3/5] lib/igt_psr: added function to check output supports pr

[Hogander, Jouni]

On Wed, 2023-10-25 at 06:01 +0000, Joshi, Kunal1 wrote:
> Hello Jouni,
> 
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com> 
> Sent: Friday, October 20, 2023 5:43 PM
> To: Joshi, Kunal1 <kunal1.joshi@intel.com>;
> igt-dev@lists.freedesktop.org
> Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh
> <animesh.manna@intel.com>
> Subject: Re: [PATCH i-g-t 3/5] lib/igt_psr: added function to check
> output supports pr
> 
> On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> > added function to check output supports PR
> > 
> > Note :- New debugfs interfaces which are used
> >         will be added by below patch in the driver
> >         https://patchwork.freedesktop.org/series/94470/
> > 
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> > ---
> >  lib/igt_psr.c | 17 +++++++++++++++++
> >  lib/igt_psr.h |  1 +
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c index
> > acacfa31f..4c912a064 
> > 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -22,6 +22,7 @@
> >   */
> >  
> >  #include "drmtest.h"
> > +#include "igt_kms.h"
> >  #include "igt_params.h"
> >  #include "igt_psr.h"
> >  #include "igt_sysfs.h"
> > @@ -373,3 +374,19 @@ enum psr_mode psr_get_mode(int debugfs_fd)
> >  
> >         return PSR_DISABLED;
> >  }
> > +
> > +bool output_supports_pr(int debugfs_fd, igt_output_t *output) {
> > +       char debugfs_file[128] = {0};
> > +       char buf[PSR_STATUS_MAX_LEN];
> > +
> > +       if (!strstr(output->name,"DP"))
> > +               return false;
> > +
> > +        sprintf(debugfs_file, "%s/i915_psr_status", output->name);
> > +
> > +        igt_debugfs_simple_read(debugfs_fd, debugfs_file,
> > +                               buf, sizeof(buf));
> > +
> > +        return strstr(buf, "Panel Replay = yes"); }
> 
> > Why do you think psr_sink_support couldn't work for DP as well?
> > 
> > BR,
> 
> > Jouni Högander
> 
> Jouni psr_sink_support won't work for DP, psr_sink_support is
> modified
> for eDP only. Changes are based on driver changes done in
> https://patchwork.freedesktop.org/series/94470/

Ok but instead of adding new you could modify psr_sink_support to
accept PR_MODE and PR_MODE_SEL_FETCH?

BR,

Jouni Högander

> 
> 
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h index
> > 243154897..8e36fcf2f 
> > 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -55,5 +55,6 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
> >  
> >  bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
> >  void i915_psr2_sel_fetch_restore(int drm_fd);
> > +bool output_supports_pr(int debugfs_fd, igt_output_t *output);
> >  
> >  #endif
> 

Re: [igt-dev] [PATCH i-g-t 3/5] lib/igt_psr: added function to check output supports pr

[Joshi, Kunal1]

Hello Jouni,

-----Original Message-----
From: Hogander, Jouni <jouni.hogander@intel.com> 
Sent: Wednesday, October 25, 2023 12:36 PM
To: Joshi, Kunal1 <kunal1.joshi@intel.com>; igt-dev@lists.freedesktop.org
Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh <animesh.manna@intel.com>
Subject: Re: [PATCH i-g-t 3/5] lib/igt_psr: added function to check output supports pr

On Wed, 2023-10-25 at 06:01 +0000, Joshi, Kunal1 wrote:
> Hello Jouni,
> 
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Friday, October 20, 2023 5:43 PM
> To: Joshi, Kunal1 <kunal1.joshi@intel.com>; 
> igt-dev@lists.freedesktop.org
> Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh 
> <animesh.manna@intel.com>
> Subject: Re: [PATCH i-g-t 3/5] lib/igt_psr: added function to check 
> output supports pr
> 
> On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> > added function to check output supports PR
> > 
> > Note :- New debugfs interfaces which are used
> >         will be added by below patch in the driver
> >         https://patchwork.freedesktop.org/series/94470/
> > 
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> > ---
> >  lib/igt_psr.c | 17 +++++++++++++++++
> >  lib/igt_psr.h |  1 +
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c index
> > acacfa31f..4c912a064
> > 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -22,6 +22,7 @@
> >   */
> >  
> >  #include "drmtest.h"
> > +#include "igt_kms.h"
> >  #include "igt_params.h"
> >  #include "igt_psr.h"
> >  #include "igt_sysfs.h"
> > @@ -373,3 +374,19 @@ enum psr_mode psr_get_mode(int debugfs_fd)
> >  
> >         return PSR_DISABLED;
> >  }
> > +
> > +bool output_supports_pr(int debugfs_fd, igt_output_t *output) {
> > +       char debugfs_file[128] = {0};
> > +       char buf[PSR_STATUS_MAX_LEN];
> > +
> > +       if (!strstr(output->name,"DP"))
> > +               return false;
> > +
> > +        sprintf(debugfs_file, "%s/i915_psr_status", output->name);
> > +
> > +        igt_debugfs_simple_read(debugfs_fd, debugfs_file,
> > +                               buf, sizeof(buf));
> > +
> > +        return strstr(buf, "Panel Replay = yes"); }
> 
> > Why do you think psr_sink_support couldn't work for DP as well?
> > 
> > BR,
> 
> > Jouni Högander
> 
> Jouni psr_sink_support won't work for DP, psr_sink_support is modified 
> for eDP only. Changes are based on driver changes done in 
> https://patchwork.freedesktop.org/series/94470/

> Ok but instead of adding new you could modify psr_sink_support to accept PR_MODE
> PR_MODE_SEL_FETCH?
>
> BR,
>
> Jouni Högander

For psr debugfs is
/sys/kernel/debug/dri/0/i915_edp_psr_status

For DP Pr it is
/sys/kernel/debug/dri/0/DP-X/i915)psr_status

Certainly I can modify to handle PR for DP as well in
psr_sink_support, by below

- bool psr_sink_support(int device, int debugfs_fd, enum psr_mode mode)
+  bool psr_sink_support(int device, int debugfs_fd, enum psr_mode mode, igt_output *output)

can I proceed with above modification?

 
> 
> 
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h index 
> > 243154897..8e36fcf2f
> > 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -55,5 +55,6 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
> >  
> >  bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
> >  void i915_psr2_sel_fetch_restore(int drm_fd);
> > +bool output_supports_pr(int debugfs_fd, igt_output_t *output);
> >  
> >  #endif
> 

Thanks and Regards
Kunal Joshi

[igt-dev] [PATCH i-g-t 4/5] lib/igt_psr: added function to check active dp pr

[Kunal Joshi]

Added function to check if pr is active on a output.
assuming SRDENT is the state to compare when pr is active

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_psr.c | 23 +++++++++++++++++++++++
 lib/igt_psr.h |  2 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 4c912a064..683379db6 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -390,3 +390,26 @@ bool output_supports_pr(int debugfs_fd, igt_output_t *output)
 
         return strstr(buf, "Panel Replay = yes");
 }
+
+bool dp_pr_active_check(int debugfs_fd, igt_output_t *output)
+{
+        char debugfs_file[128] = {0};
+        char buf[PSR_STATUS_MAX_LEN];
+        const char *state = "SRDENT";
+        int ret;
+
+        if (!strstr(output->name,"DP"))
+                return false;
+
+        sprintf(debugfs_file, "%s/i915_sink_status", output->name);
+
+        ret = igt_debugfs_simple_read(debugfs_fd, debugfs_file,
+                                     buf, sizeof(buf));
+        if (ret < 0) {
+                igt_info("Could not read i915_edp_psr_status: %s\n",
+                         strerror(-ret));
+                return false;
+        }
+        igt_skip_on(strstr(buf, "PSR sink not reliable: yes"));
+        return strstr(buf, state);
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 8e36fcf2f..dc603ffc3 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -56,5 +56,5 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
 bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
 void i915_psr2_sel_fetch_restore(int drm_fd);
 bool output_supports_pr(int debugfs_fd, igt_output_t *output);
-
+bool dp_pr_active_check(int debugfs_fd, igt_output_t *output);
 #endif
-- 
2.25.1

Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_psr: added function to check active dp pr

[Hogander, Jouni]

On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> Added function to check if pr is active on a output.
> assuming SRDENT is the state to compare when pr is active
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  lib/igt_psr.c | 23 +++++++++++++++++++++++
>  lib/igt_psr.h |  2 +-
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 4c912a064..683379db6 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -390,3 +390,26 @@ bool output_supports_pr(int debugfs_fd,
> igt_output_t *output)
>  
>          return strstr(buf, "Panel Replay = yes");
>  }
> +
> +bool dp_pr_active_check(int debugfs_fd, igt_output_t *output)
> +{
> +        char debugfs_file[128] = {0};
> +        char buf[PSR_STATUS_MAX_LEN];
> +        const char *state = "SRDENT";
> +        int ret;
> +
> +        if (!strstr(output->name,"DP"))
> +                return false;
> +
> +        sprintf(debugfs_file, "%s/i915_sink_status", output->name);
> +
> +        ret = igt_debugfs_simple_read(debugfs_fd, debugfs_file,
> +                                     buf, sizeof(buf));
> +        if (ret < 0) {
> +                igt_info("Could not read i915_edp_psr_status: %s\n",
> +                         strerror(-ret));
> +                return false;
> +        }
> +        igt_skip_on(strstr(buf, "PSR sink not reliable: yes"));
> +        return strstr(buf, state);
> +}

Why did you choose to write new instead of modifying existing
psr_sink_support ?

BR,

Jouni Högander
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index 8e36fcf2f..dc603ffc3 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -56,5 +56,5 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
>  bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
>  void i915_psr2_sel_fetch_restore(int drm_fd);
>  bool output_supports_pr(int debugfs_fd, igt_output_t *output);
> -
> +bool dp_pr_active_check(int debugfs_fd, igt_output_t *output);
>  #endif

Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_psr: added function to check active dp pr

[Joshi, Kunal1]

Hello Jouni,

-----Original Message-----
From: Hogander, Jouni <jouni.hogander@intel.com> 
Sent: Friday, October 20, 2023 5:46 PM
To: Joshi, Kunal1 <kunal1.joshi@intel.com>; igt-dev@lists.freedesktop.org
Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh <animesh.manna@intel.com>
Subject: Re: [PATCH i-g-t 4/5] lib/igt_psr: added function to check active dp pr

On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> Added function to check if pr is active on a output.
> assuming SRDENT is the state to compare when pr is active
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  lib/igt_psr.c | 23 +++++++++++++++++++++++
>  lib/igt_psr.h |  2 +-
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c index 4c912a064..683379db6 
> 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -390,3 +390,26 @@ bool output_supports_pr(int debugfs_fd, 
> igt_output_t *output)
>  
>          return strstr(buf, "Panel Replay = yes");
>  }
> +
> +bool dp_pr_active_check(int debugfs_fd, igt_output_t *output) {
> +        char debugfs_file[128] = {0};
> +        char buf[PSR_STATUS_MAX_LEN];
> +        const char *state = "SRDENT";
> +        int ret;
> +
> +        if (!strstr(output->name,"DP"))
> +                return false;
> +
> +        sprintf(debugfs_file, "%s/i915_sink_status", output->name);
> +
> +        ret = igt_debugfs_simple_read(debugfs_fd, debugfs_file,
> +                                     buf, sizeof(buf));
> +        if (ret < 0) {
> +                igt_info("Could not read i915_edp_psr_status: %s\n",
> +                         strerror(-ret));
> +                return false;
> +        }
> +        igt_skip_on(strstr(buf, "PSR sink not reliable: yes"));
> +        return strstr(buf, state);
> +}

> Why did you choose to write new instead of modifying existing c ?
>
> BR
For DP pr we are using different debugfs directory to read,
so thought of creating new function for clear separation,

But can modify psr_sink_support if you think that will be better way.

Jouni Högander
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h index 8e36fcf2f..dc603ffc3 
> 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -56,5 +56,5 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
>  bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
>  void i915_psr2_sel_fetch_restore(int drm_fd);
>  bool output_supports_pr(int debugfs_fd, igt_output_t *output);
> -
> +bool dp_pr_active_check(int debugfs_fd, igt_output_t *output);
>  #endif

Thanks and Regards
Kunal Joshi

Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_psr: added function to check active dp pr

[Hogander, Jouni]

On Wed, 2023-10-25 at 06:16 +0000, Joshi, Kunal1 wrote:
> Hello Jouni,
> 
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com> 
> Sent: Friday, October 20, 2023 5:46 PM
> To: Joshi, Kunal1 <kunal1.joshi@intel.com>;
> igt-dev@lists.freedesktop.org
> Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh
> <animesh.manna@intel.com>
> Subject: Re: [PATCH i-g-t 4/5] lib/igt_psr: added function to check
> active dp pr
> 
> On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> > Added function to check if pr is active on a output.
> > assuming SRDENT is the state to compare when pr is active
> > 
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> > ---
> >  lib/igt_psr.c | 23 +++++++++++++++++++++++
> >  lib/igt_psr.h |  2 +-
> >  2 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c index
> > 4c912a064..683379db6 
> > 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -390,3 +390,26 @@ bool output_supports_pr(int debugfs_fd, 
> > igt_output_t *output)
> >  
> >          return strstr(buf, "Panel Replay = yes");
> >  }
> > +
> > +bool dp_pr_active_check(int debugfs_fd, igt_output_t *output) {
> > +        char debugfs_file[128] = {0};
> > +        char buf[PSR_STATUS_MAX_LEN];
> > +        const char *state = "SRDENT";
> > +        int ret;
> > +
> > +        if (!strstr(output->name,"DP"))
> > +                return false;
> > +
> > +        sprintf(debugfs_file, "%s/i915_sink_status", output-
> > >name);
> > +
> > +        ret = igt_debugfs_simple_read(debugfs_fd, debugfs_file,
> > +                                     buf, sizeof(buf));
> > +        if (ret < 0) {
> > +                igt_info("Could not read i915_edp_psr_status:
> > %s\n",
> > +                         strerror(-ret));
> > +                return false;
> > +        }
> > +        igt_skip_on(strstr(buf, "PSR sink not reliable: yes"));
> > +        return strstr(buf, state);
> > +}
> 
> > Why did you choose to write new instead of modifying existing c ?
> > 
> > BR
> For DP pr we are using different debugfs directory to read,
> so thought of creating new function for clear separation,
> 
> But can modify psr_sink_support if you think that will be better way.

I think that would be better approach. Also eases your modifications in
kms_psr.c

BR,

Jouni Högander

> 
> Jouni Högander
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h index
> > 8e36fcf2f..dc603ffc3 
> > 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -56,5 +56,5 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
> >  bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
> >  void i915_psr2_sel_fetch_restore(int drm_fd);
> >  bool output_supports_pr(int debugfs_fd, igt_output_t *output);
> > -
> > +bool dp_pr_active_check(int debugfs_fd, igt_output_t *output);
> >  #endif
> 
> Thanks and Regards
> Kunal Joshi
> 

Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_psr: added function to check active dp pr

[Joshi, Kunal1]

Hello Jouni,

-----Original Message-----
From: Hogander, Jouni <jouni.hogander@intel.com> 
Sent: Wednesday, October 25, 2023 12:38 PM
To: Joshi, Kunal1 <kunal1.joshi@intel.com>; igt-dev@lists.freedesktop.org
Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh <animesh.manna@intel.com>
Subject: Re: [PATCH i-g-t 4/5] lib/igt_psr: added function to check active dp pr

On Wed, 2023-10-25 at 06:16 +0000, Joshi, Kunal1 wrote:
> Hello Jouni,
> 
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Friday, October 20, 2023 5:46 PM
> To: Joshi, Kunal1 <kunal1.joshi@intel.com>; 
> igt-dev@lists.freedesktop.org
> Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh 
> <animesh.manna@intel.com>
> Subject: Re: [PATCH i-g-t 4/5] lib/igt_psr: added function to check 
> active dp pr
> 
> On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> > Added function to check if pr is active on a output.
> > assuming SRDENT is the state to compare when pr is active
> > 
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> > ---
> >  lib/igt_psr.c | 23 +++++++++++++++++++++++
> >  lib/igt_psr.h |  2 +-
> >  2 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c index
> > 4c912a064..683379db6
> > 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -390,3 +390,26 @@ bool output_supports_pr(int debugfs_fd, 
> > igt_output_t *output)
> >  
> >          return strstr(buf, "Panel Replay = yes");
> >  }
> > +
> > +bool dp_pr_active_check(int debugfs_fd, igt_output_t *output) {
> > +        char debugfs_file[128] = {0};
> > +        char buf[PSR_STATUS_MAX_LEN];
> > +        const char *state = "SRDENT";
> > +        int ret;
> > +
> > +        if (!strstr(output->name,"DP"))
> > +                return false;
> > +
> > +        sprintf(debugfs_file, "%s/i915_sink_status", output-
> > >name);
> > +
> > +        ret = igt_debugfs_simple_read(debugfs_fd, debugfs_file,
> > +                                     buf, sizeof(buf));
> > +        if (ret < 0) {
> > +                igt_info("Could not read i915_edp_psr_status:
> > %s\n",
> > +                         strerror(-ret));
> > +                return false;
> > +        }
> > +        igt_skip_on(strstr(buf, "PSR sink not reliable: yes"));
> > +        return strstr(buf, state);
> > +}
> 
> > Why did you choose to write new instead of modifying existing c ?
> > 
> > BR
> For DP pr we are using different debugfs directory to read, so thought 
> of creating new function for clear separation,
> 
> But can modify psr_sink_support if you think that will be better way.
>
> I think that would be better approach. Also eases your modifications in kms_psr.c
>
> BR,
>
> Jouni Högander

Sure Jouni, thanks for the suggestion 😊

> 
> Jouni Högander
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h index
> > 8e36fcf2f..dc603ffc3
> > 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -56,5 +56,5 @@ bool i915_psr2_selective_fetch_check(int drm_fd);
> >  bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
> >  void i915_psr2_sel_fetch_restore(int drm_fd);
> >  bool output_supports_pr(int debugfs_fd, igt_output_t *output);
> > -
> > +bool dp_pr_active_check(int debugfs_fd, igt_output_t *output);
> >  #endif
> 
> Thanks and Regards
> Kunal Joshi
> 

Thanks and Regards
Kunal Joshi

[igt-dev] [PATCH i-g-t 5/5] tests/intel/kms_psr: made test compaitable with pr

[Kunal Joshi]

Modified kms_psr to have support for DP pr.

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 tests/intel/kms_psr.c | 402 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 331 insertions(+), 71 deletions(-)

diff --git a/tests/intel/kms_psr.c b/tests/intel/kms_psr.c
index ffecc5222..b14267dce 100644
--- a/tests/intel/kms_psr.c
+++ b/tests/intel/kms_psr.c
@@ -229,6 +229,102 @@
  * @plane_move:         Move plane position
  */
 
+/**
+ * SUBTEST: pr_dpms
+ * Description: Check if pr is detecting changes when rendering operation
+ *              is performed  with dpms enabled or disabled
+ * Driver requirement: i915, xe
+ * Functionality: dpms, pr
+ * Mega feature: Panel Replay
+ * Test category: functionality test
+ *
+ * SUBTEST: pr_no_drrs
+ * Description: Check if pr is detecting changes when drrs is disabled
+ * Driver requirement: i915, xe
+ * Functionality: drrs, pr
+ * Mega feature: Panel Replay
+ * Test category: functionality test
+ *
+ * SUBTEST: pr_suspend
+ * Description: Check if pr is detecting changes when plane operation is
+ *              performed with suspend resume cycles
+ * Driver requirement: i915, xe
+ * Functionality: pr, suspend
+ * Mega feature: Panel Replay
+ * Test category: functionality test
+ *
+ * SUBTEST: pr_basic
+ * Description: Basic check for pr if it is detecting changes made in planes
+ * Driver requirement: i915, xe
+ * Functionality: pr
+ * Mega feature: Panel Replay
+ * Test category: functionality test
+ *
+ * SUBTEST: pr_%s_%s
+ * Description: Check if pr is detecting memory mapping, rendering and plane
+ *              operations performed on %arg[1] planes
+ * Driver requirement: i915
+ * Functionality: kms_core, plane, pr
+ * Mega feature: Panel Replay
+ * Test category: functionality test
+ *
+ * arg[1]:
+ *
+ * @cursor:             Cursor plane
+ * @primary:            Primary plane
+ * @sprite:             Sprite plane
+ *
+ * arg[2]:
+ *
+ * @mmap_cpu:           MMAP CPU
+ * @mmap_gtt:           MMAP GTT
+ */
+
+/**
+ * SUBTEST: pr_primary_page_flip
+ * Description: Check if pr is detecting memory mapping, rendering and plane
+ *              operations performed on primary planes
+ * Driver requirement: i915, xe
+ * Functionality: plane, pr
+ * Mega feature: Panel Replay
+ * Test category: functionality test
+ *
+ * SUBTEST: pr_primary_%s
+ * Description: Check if pr is detecting memory mapping, rendering and plane
+ *              operations performed on primary planes
+ * Driver requirement: i915, xe
+ * Functionality: kms_core, plane, pr
+ * Mega feature: Panel Replay
+ * Test category: functionality test
+ *
+ * arg[1]:
+ *
+ * @blt:                Blitter
+ * @render:             Render
+ */
+
+/**
+ * SUBTEST: pr_%s_%s
+ * Description: Check if pr is detecting memory mapping, rendering and plane
+ *              operations performed on %arg[1] planes
+ * Driver requirement: i915, xe
+ * Functionality: kms_core, plane, pr
+ * Mega feature: Panel Replay
+ * Test category: functionality test
+ *
+ * arg[1]:
+ *
+ * @cursor:             Cursor plane
+ * @sprite:             Sprite plane
+ *
+ * arg[2]:
+ *
+ * @blt:                Blitter
+ * @render:             Render
+ * @plane_onoff:        Plane On off
+ * @plane_move:         Move plane position
+ */
+
 enum operations {
 	PAGE_FLIP,
 	MMAP_GTT,
@@ -272,6 +368,9 @@ typedef struct {
 	igt_output_t *output;
 	bool with_psr_disabled;
 	bool supports_psr2;
+        bool supports_psr;
+	bool supports_pr;
+	igt_output_t **pr_outputs;
 } data_t;
 
 static void create_cursor_fb(data_t *data)
@@ -316,7 +415,6 @@ static void setup_output(data_t *data)
 static void display_init(data_t *data)
 {
 	igt_display_require(&data->display, data->drm_fd);
-	setup_output(data);
 }
 
 static void display_fini(data_t *data)
@@ -444,6 +542,15 @@ static bool psr_wait_entry_if_enabled(data_t *data)
 	return psr_wait_entry(data->debugfs_fd, data->op_psr_mode);
 }
 
+static bool pr_wait_entry(data_t *data)
+{
+        if (data->with_psr_disabled)
+                return true;
+
+        return igt_wait(dp_pr_active_check(data->debugfs_fd,
+					   data->output), 500, 20);
+}
+
 static bool psr_wait_update_if_enabled(data_t *data)
 {
 	if (data->with_psr_disabled)
@@ -604,7 +711,10 @@ static void run_test(data_t *data)
 		expected = "screen GREEN";
 		break;
 	}
-	igt_assert(psr_wait_update_if_enabled(data));
+	if(strstr(data->output->name, "DP"))
+		igt_assert(pr_wait_entry(data));
+	else
+		igt_assert(psr_wait_update_if_enabled(data));
 	manual(expected);
 }
 
@@ -687,6 +797,7 @@ static void test_setup(data_t *data)
 {
 	drmModeConnectorPtr connector;
 	bool psr_entered = false;
+	bool pr_entered = false;
 
 	igt_require_f(data->output,
 		      "No available output found\n");
@@ -703,15 +814,23 @@ static void test_setup(data_t *data)
 		if (data->op_psr_mode == PSR_MODE_2)
 			igt_require(data->supports_psr2);
 
-		psr_enable_if_enabled(data);
-		setup_test_plane(data, data->test_plane_id);
-		if (psr_wait_entry_if_enabled(data)) {
-			psr_entered = true;
-			break;
+		if (strstr(data->output->name, "DP"))
+		{
+			setup_test_plane(data, data->test_plane_id);
+			pr_entered = pr_wait_entry(data);
+			if(pr_entered)
+				break;
+		}
+		else {
+			psr_enable_if_enabled(data);
+			setup_test_plane(data, data->test_plane_id);
+			if(psr_wait_entry_if_enabled(data)) {
+				psr_entered = true;
+				break;
+			}
 		}
 	}
-
-	igt_assert(psr_entered);
+	igt_assert(psr_entered || pr_entered);
 }
 
 static void dpms_off_on(data_t *data)
@@ -748,10 +867,15 @@ data_t data = {};
 igt_main_args("", long_options, help_str, opt_handler, &data)
 {
 	enum operations op;
-	const char *append_subtest_name[2] = {
+	const char *append_subtest_name[3] = {
 		"",
-		"psr2_"
+		"psr2_",
+		"pr_"
 	};
+	int pr_output_count;
+	igt_output_t *out;
+	int modes[] = {PSR_MODE_1, PSR_MODE_1, PR_MODE};
+	int z;
 
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_XE);
@@ -759,103 +883,239 @@ igt_main_args("", long_options, help_str, opt_handler, &data)
 		kmstest_set_vt_graphics_mode();
 		data.devid = intel_get_drm_devid(data.drm_fd);
 
-		igt_require_f(sink_support(&data, PSR_MODE_1),
-			      "Sink does not support PSR\n");
-
+		pr_output_count = 0;
+		data.pr_outputs = (igt_output_t **)malloc(sizeof(igt_output_t *) * data.display.n_outputs);
+		for_each_connected_output(&data.display, out)
+			if(output_supports_pr(data.debugfs_fd, out))
+				data.pr_outputs[pr_output_count++] = out;
+		data.supports_psr = sink_support(&data, PSR_MODE_1);
 		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
+		data.supports_pr = pr_output_count > 0 ? true : false;
+
+		igt_require_f(data.supports_psr || data.supports_pr,
+			      "Sink does not support PSR/PR\n");
 		data.bops = buf_ops_create(data.drm_fd);
 		display_init(&data);
 	}
 
-	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <= PSR_MODE_2;
-	     data.op_psr_mode++) {
-
+	for (z = 0; z < ARRAY_SIZE(modes); z++) {
+		data.op_psr_mode = modes[z];
 		igt_describe("Basic check for psr if it is detecting changes made in planes");
-		igt_subtest_f("%sbasic", append_subtest_name[data.op_psr_mode]) {
-			data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
-			test_setup(&data);
-			test_cleanup(&data);
+		igt_subtest_with_dynamic_f("%sbasic", append_subtest_name[z]) {
+			if (data.op_psr_mode == PR_MODE)
+			{
+				for(int k = 0; k < pr_output_count; k++) {
+					igt_dynamic_f("%s", data.pr_outputs[k]->name)
+					{
+						data.output = data.pr_outputs[k];
+						data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+						test_setup(&data);
+						test_cleanup(&data);
+					}
+				}
+			}
+			else {
+				setup_output(&data);
+				igt_dynamic_f("%s", data.output->name) {
+					data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+					test_setup(&data);
+					test_cleanup(&data);
+				}
+			}
 		}
 
 		igt_describe("Check if psr is detecting changes when drrs is disabled");
-		igt_subtest_f("%sno_drrs", append_subtest_name[data.op_psr_mode]) {
-			data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
-			test_setup(&data);
-			igt_assert(drrs_disabled(&data));
-			test_cleanup(&data);
+		igt_subtest_with_dynamic_f("%sno_drrs", append_subtest_name[z]) {
+                        if (data.op_psr_mode == PR_MODE)
+                        {
+                                for(int k = 0; k < pr_output_count; k++) {
+                                        igt_dynamic_f("%s", data.pr_outputs[k]->name)
+                                        {
+						data.output = data.pr_outputs[k];
+						data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+						test_setup(&data);
+						igt_assert(drrs_disabled(&data));
+						test_cleanup(&data);
+
+                                        }
+				}
+                        }
+			else {
+				setup_output(&data);
+				igt_dynamic_f("%s", data.output->name) {
+					data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+					test_setup(&data);
+					igt_assert(drrs_disabled(&data));
+					test_cleanup(&data);
+				}
+			}
 		}
 
 		for (op = PAGE_FLIP; op <= RENDER; op++) {
 			igt_describe("Check if psr is detecting page-flipping,memory mapping and "
 					"rendering operations performed on primary planes");
-			igt_subtest_f("%sprimary_%s",
-				      append_subtest_name[data.op_psr_mode],
+			igt_subtest_with_dynamic_f("%sprimary_%s",
+				      append_subtest_name[z],
 				      op_str(op)) {
 				igt_skip_on(is_xe_device(data.drm_fd) &&
 					    (op == MMAP_CPU || op == MMAP_GTT));
-
-				data.op = op;
-				data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
-				test_setup(&data);
-				run_test(&data);
-				test_cleanup(&data);
+				if (data.op_psr_mode == PR_MODE)
+				{
+					for(int k = 0; k < pr_output_count; k++) {
+						igt_dynamic_f("%s", data.pr_outputs[k]->name)
+						{
+							data.output = data.pr_outputs[k];
+							data.op = op;
+							data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+							test_setup(&data);
+							run_test(&data);
+							test_cleanup(&data);
+						}
+					}
+				}
+				else {
+					setup_output(&data);
+					igt_dynamic_f("%s", data.output->name) {
+						data.op = op;
+						data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+						test_setup(&data);
+						run_test(&data);
+						test_cleanup(&data);
+					}
+				}
 			}
 		}
 
 		for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
 			igt_describe("Check if psr is detecting memory mapping,rendering "
 					"and plane operations performed on sprite planes");
-			igt_subtest_f("%ssprite_%s",
-				      append_subtest_name[data.op_psr_mode],
+			igt_subtest_with_dynamic_f("%ssprite_%s",
+				      append_subtest_name[z],
 				      op_str(op)) {
 				igt_skip_on(is_xe_device(data.drm_fd) &&
 					    (op == MMAP_CPU || op == MMAP_GTT));
-
-				data.op = op;
-				data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
-				test_setup(&data);
-				run_test(&data);
-				test_cleanup(&data);
+				if (data.op_psr_mode == PR_MODE)
+				{
+					for(int k = 0; k < pr_output_count; k++) {
+						igt_dynamic_f("%s", data.pr_outputs[k]->name)
+						{
+							data.output = data.pr_outputs[k];
+							data.op = op;
+							data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
+							test_setup(&data);
+							run_test(&data);
+							test_cleanup(&data);
+						}
+					}
+				}
+				else {
+					setup_output(&data);
+					igt_dynamic_f("%s", data.output->name) {
+						data.op = op;
+						data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
+						test_setup(&data);
+						run_test(&data);
+						test_cleanup(&data);
+					}
+				}
 			}
 
 			igt_describe("Check if psr is detecting memory mapping, rendering "
-					"and plane operations performed on cursor planes");
-			igt_subtest_f("%scursor_%s",
-				      append_subtest_name[data.op_psr_mode],
-				      op_str(op)) {
+				     "and plane operations performed on cursor planes");
+			igt_subtest_with_dynamic_f("%scursor_%s",
+					           append_subtest_name[z],
+						   op_str(op)) {
 				igt_skip_on(is_xe_device(data.drm_fd) &&
-					    (op == MMAP_CPU || op == MMAP_GTT));
-
-				data.op = op;
-				data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
-				test_setup(&data);
-				run_test(&data);
-				test_cleanup(&data);
+					   (op == MMAP_CPU || op == MMAP_GTT));
+				if (data.op_psr_mode == PR_MODE)
+				{
+					for(int k = 0; k < pr_output_count; k++) {
+						igt_dynamic_f("%s", data.pr_outputs[k]->name)
+						{
+							data.output = data.pr_outputs[k];
+							data.op = op;
+							data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
+							test_setup(&data);
+							run_test(&data);
+							test_cleanup(&data);
+						}
+					}
+				}
+				else {
+					setup_output(&data);
+					igt_dynamic_f("%s", data.output->name) {
+						data.op = op;
+						data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
+						test_setup(&data);
+						run_test(&data);
+						test_cleanup(&data);
+					}
+				}
 			}
 		}
 
-		igt_describe("Check if psr is detecting changes when rendering operation is performed"
-				"  with dpms enabled or disabled");
-		igt_subtest_f("%sdpms", append_subtest_name[data.op_psr_mode]) {
-			data.op = RENDER;
-			data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
-			test_setup(&data);
-			dpms_off_on(&data);
-			run_test(&data);
-			test_cleanup(&data);
+		igt_describe("Check if psr is detecting changes when rendering operation is performed "
+			     "with dpms enabled or disabled");
+		igt_subtest_with_dynamic_f("%sdpms", append_subtest_name[z]) {
+			if (data.op_psr_mode == PR_MODE)
+			{
+				for(int k = 0; k < pr_output_count; k++) {
+					igt_dynamic_f("%s", data.pr_outputs[k]->name)
+					{
+						data.output = data.pr_outputs[k];
+						data.op = RENDER;
+						data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+						test_setup(&data);
+						dpms_off_on(&data);
+						run_test(&data);
+						test_cleanup(&data);
+					}
+				}
+			}
+			else {
+				setup_output(&data);
+				igt_dynamic_f("%s", data.output->name) {
+					data.op = RENDER;
+					data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+					test_setup(&data);
+					dpms_off_on(&data);
+					run_test(&data);
+					test_cleanup(&data);
+				}
+			}
 		}
 
 		igt_describe("Check if psr is detecting changes when plane operation is performed "
 				"with suspend resume cycles");
-		igt_subtest_f("%ssuspend", append_subtest_name[data.op_psr_mode]) {
-			data.op = PLANE_ONOFF;
-			data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
-			test_setup(&data);
-			igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
-						      SUSPEND_TEST_NONE);
-			igt_assert(psr_wait_entry_if_enabled(&data));
-			run_test(&data);
-			test_cleanup(&data);
+		igt_subtest_with_dynamic_f("%ssuspend", append_subtest_name[z]) {
+			if (data.op_psr_mode == PR_MODE)
+			{
+				for(int k = 0; k < pr_output_count; k++) {
+					igt_dynamic_f("%s", data.pr_outputs[k]->name)
+					{
+						data.output = data.pr_outputs[k];
+						data.op = PLANE_ONOFF;
+						data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
+						test_setup(&data);
+						igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
+									      SUSPEND_TEST_NONE);
+						igt_assert(pr_wait_entry(&data));
+						run_test(&data);
+						test_cleanup(&data);
+					}
+				}
+			}
+			else {
+				setup_output(&data);
+				igt_dynamic_f("%s", data.output->name) {
+					data.op = RENDER;
+					data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
+					test_setup(&data);
+					dpms_off_on(&data);
+					run_test(&data);
+					test_cleanup(&data);
+				}
+			}
 		}
 	}
 
-- 
2.25.1

Re: [igt-dev] [PATCH i-g-t 5/5] tests/intel/kms_psr: made test compaitable with pr

[Hogander, Jouni]

On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> Modified kms_psr to have support for DP pr.
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  tests/intel/kms_psr.c | 402 ++++++++++++++++++++++++++++++++++------
> --
>  1 file changed, 331 insertions(+), 71 deletions(-)
> 
> diff --git a/tests/intel/kms_psr.c b/tests/intel/kms_psr.c
> index ffecc5222..b14267dce 100644
> --- a/tests/intel/kms_psr.c
> +++ b/tests/intel/kms_psr.c
> @@ -229,6 +229,102 @@
>   * @plane_move:         Move plane position
>   */
>  
> +/**
> + * SUBTEST: pr_dpms
> + * Description: Check if pr is detecting changes when rendering
> operation
> + *              is performed  with dpms enabled or disabled
> + * Driver requirement: i915, xe
> + * Functionality: dpms, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_no_drrs
> + * Description: Check if pr is detecting changes when drrs is
> disabled
> + * Driver requirement: i915, xe
> + * Functionality: drrs, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_suspend
> + * Description: Check if pr is detecting changes when plane
> operation is
> + *              performed with suspend resume cycles
> + * Driver requirement: i915, xe
> + * Functionality: pr, suspend
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_basic
> + * Description: Basic check for pr if it is detecting changes made
> in planes
> + * Driver requirement: i915, xe
> + * Functionality: pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_%s_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on %arg[1] planes
> + * Driver requirement: i915
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @cursor:             Cursor plane
> + * @primary:            Primary plane
> + * @sprite:             Sprite plane
> + *
> + * arg[2]:
> + *
> + * @mmap_cpu:           MMAP CPU
> + * @mmap_gtt:           MMAP GTT
> + */
> +
> +/**
> + * SUBTEST: pr_primary_page_flip
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on primary planes
> + * Driver requirement: i915, xe
> + * Functionality: plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_primary_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on primary planes
> + * Driver requirement: i915, xe
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @blt:                Blitter
> + * @render:             Render
> + */
> +
> +/**
> + * SUBTEST: pr_%s_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on %arg[1] planes
> + * Driver requirement: i915, xe
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @cursor:             Cursor plane
> + * @sprite:             Sprite plane
> + *
> + * arg[2]:
> + *
> + * @blt:                Blitter
> + * @render:             Render
> + * @plane_onoff:        Plane On off
> + * @plane_move:         Move plane position
> + */
> +
>  enum operations {
>         PAGE_FLIP,
>         MMAP_GTT,
> @@ -272,6 +368,9 @@ typedef struct {
>         igt_output_t *output;
>         bool with_psr_disabled;
>         bool supports_psr2;
> +        bool supports_psr;
> +       bool supports_pr;
> +       igt_output_t **pr_outputs;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data)
> @@ -316,7 +415,6 @@ static void setup_output(data_t *data)
>  static void display_init(data_t *data)
>  {
>         igt_display_require(&data->display, data->drm_fd);
> -       setup_output(data);
>  }
>  
>  static void display_fini(data_t *data)
> @@ -444,6 +542,15 @@ static bool psr_wait_entry_if_enabled(data_t
> *data)
>         return psr_wait_entry(data->debugfs_fd, data->op_psr_mode);
>  }
>  
> +static bool pr_wait_entry(data_t *data)
> +{
> +        if (data->with_psr_disabled)
> +                return true;
> +
> +        return igt_wait(dp_pr_active_check(data->debugfs_fd,
> +                                          data->output), 500, 20);
> +}
> +
>  static bool psr_wait_update_if_enabled(data_t *data)
>  {
>         if (data->with_psr_disabled)
> @@ -604,7 +711,10 @@ static void run_test(data_t *data)
>                 expected = "screen GREEN";
>                 break;
>         }
> -       igt_assert(psr_wait_update_if_enabled(data));
> +       if(strstr(data->output->name, "DP"))
> +               igt_assert(pr_wait_entry(data));
> +       else
> +               igt_assert(psr_wait_update_if_enabled(data));
>         manual(expected);
>  }
>  
> @@ -687,6 +797,7 @@ static void test_setup(data_t *data)
>  {
>         drmModeConnectorPtr connector;
>         bool psr_entered = false;
> +       bool pr_entered = false;
>  
>         igt_require_f(data->output,
>                       "No available output found\n");
> @@ -703,15 +814,23 @@ static void test_setup(data_t *data)
>                 if (data->op_psr_mode == PSR_MODE_2)
>                         igt_require(data->supports_psr2);
>  
> -               psr_enable_if_enabled(data);
> -               setup_test_plane(data, data->test_plane_id);
> -               if (psr_wait_entry_if_enabled(data)) {
> -                       psr_entered = true;
> -                       break;
> +               if (strstr(data->output->name, "DP"))
> +               {
> +                       setup_test_plane(data, data->test_plane_id);
> +                       pr_entered = pr_wait_entry(data);
> +                       if(pr_entered)
> +                               break;
> +               }
> +               else {
> +                       psr_enable_if_enabled(data);
> +                       setup_test_plane(data, data->test_plane_id);
> +                       if(psr_wait_entry_if_enabled(data)) {
> +                               psr_entered = true;
> +                               break;
> +                       }
>                 }
>         }
> -
> -       igt_assert(psr_entered);
> +       igt_assert(psr_entered || pr_entered);
>  }
>  
>  static void dpms_off_on(data_t *data)
> @@ -748,10 +867,15 @@ data_t data = {};
>  igt_main_args("", long_options, help_str, opt_handler, &data)
>  {
>         enum operations op;
> -       const char *append_subtest_name[2] = {
> +       const char *append_subtest_name[3] = {
>                 "",
> -               "psr2_"
> +               "psr2_",
> +               "pr_"
>         };
> +       int pr_output_count;
> +       igt_output_t *out;
> +       int modes[] = {PSR_MODE_1, PSR_MODE_1, PR_MODE};

PSR_MODE_1 is listed twice.

> +       int z;
>  
>         igt_fixture {
>                 data.drm_fd = drm_open_driver_master(DRIVER_INTEL |
> DRIVER_XE);
> @@ -759,103 +883,239 @@ igt_main_args("", long_options, help_str,
> opt_handler, &data)
>                 kmstest_set_vt_graphics_mode();
>                 data.devid = intel_get_drm_devid(data.drm_fd);
>  
> -               igt_require_f(sink_support(&data, PSR_MODE_1),
> -                             "Sink does not support PSR\n");
> -
> +               pr_output_count = 0;
> +               data.pr_outputs = (igt_output_t
> **)malloc(sizeof(igt_output_t *) * data.display.n_outputs);
> +               for_each_connected_output(&data.display, out)
> +                       if(output_supports_pr(data.debugfs_fd, out))
> +                               data.pr_outputs[pr_output_count++] =
> out;
> +               data.supports_psr = sink_support(&data, PSR_MODE_1);
>                 data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> +               data.supports_pr = pr_output_count > 0 ? true :
> false;
> +
> +               igt_require_f(data.supports_psr || data.supports_pr,
> +                             "Sink does not support PSR/PR\n");
>                 data.bops = buf_ops_create(data.drm_fd);
>                 display_init(&data);
>         }
>  
> -       for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> PSR_MODE_2;
> -            data.op_psr_mode++) {
> -
> +       for (z = 0; z < ARRAY_SIZE(modes); z++) {
> +               data.op_psr_mode = modes[z];

Why are you changing this? To my opinion original is more clear...

>                 igt_describe("Basic check for psr if it is detecting
> changes made in planes");
> -               igt_subtest_f("%sbasic",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%sbasic",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }

I would suggest using outputs and output_count instead of pr_* and drop
setup_output for PSR as wll. Then you will have 1 output for PSR and
many outpus for PR. This way there is no need to duplicate so much.
Same comment applies on each test below.

BR,

Jouni Högander

>                 }
>  
>                 igt_describe("Check if psr is detecting changes when
> drrs is disabled");
> -               igt_subtest_f("%sno_drrs",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       igt_assert(drrs_disabled(&data));
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%sno_drrs",
> append_subtest_name[z]) {
> +                        if (data.op_psr_mode == PR_MODE)
> +                        {
> +                                for(int k = 0; k < pr_output_count;
> k++) {
> +                                        igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                        {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               igt_assert(drrs_disab
> led(&data));
> +                                               test_cleanup(&data);
> +
> +                                        }
> +                               }
> +                        }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       igt_assert(drrs_disabled(&dat
> a));
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>  
>                 for (op = PAGE_FLIP; op <= RENDER; op++) {
>                         igt_describe("Check if psr is detecting page-
> flipping,memory mapping and "
>                                         "rendering operations
> performed on primary planes");
> -                       igt_subtest_f("%sprimary_%s",
> -                                    
> append_subtest_name[data.op_psr_mode],
> +                       igt_subtest_with_dynamic_f("%sprimary_%s",
> +                                     append_subtest_name[z],
>                                       op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
>                                             (op == MMAP_CPU || op ==
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_PRIMARY;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>                 }
>  
>                 for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>                         igt_describe("Check if psr is detecting
> memory mapping,rendering "
>                                         "and plane operations
> performed on sprite planes");
> -                       igt_subtest_f("%ssprite_%s",
> -                                    
> append_subtest_name[data.op_psr_mode],
> +                       igt_subtest_with_dynamic_f("%ssprite_%s",
> +                                     append_subtest_name[z],
>                                       op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
>                                             (op == MMAP_CPU || op ==
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id =
> DRM_PLANE_TYPE_OVERLAY;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_OVERLAY;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_OVERLAY;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>  
>                         igt_describe("Check if psr is detecting
> memory mapping, rendering "
> -                                       "and plane operations
> performed on cursor planes");
> -                       igt_subtest_f("%scursor_%s",
> -                                    
> append_subtest_name[data.op_psr_mode],
> -                                     op_str(op)) {
> +                                    "and plane operations performed
> on cursor planes");
> +                       igt_subtest_with_dynamic_f("%scursor_%s",
> +                                                 
> append_subtest_name[z],
> +                                                  op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
> -                                           (op == MMAP_CPU || op ==
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id =
> DRM_PLANE_TYPE_CURSOR;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                                          (op == MMAP_CPU || op ==
> MMAP_GTT));
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_CURSOR;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_CURSOR;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>                 }
>  
> -               igt_describe("Check if psr is detecting changes when
> rendering operation is performed"
> -                               "  with dpms enabled or disabled");
> -               igt_subtest_f("%sdpms",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.op = RENDER;
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       dpms_off_on(&data);
> -                       run_test(&data);
> -                       test_cleanup(&data);
> +               igt_describe("Check if psr is detecting changes when
> rendering operation is performed "
> +                            "with dpms enabled or disabled");
> +               igt_subtest_with_dynamic_f("%sdpms",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.op = RENDER;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               dpms_off_on(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.op = RENDER;
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       dpms_off_on(&data);
> +                                       run_test(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>  
>                 igt_describe("Check if psr is detecting changes when
> plane operation is performed "
>                                 "with suspend resume cycles");
> -               igt_subtest_f("%ssuspend",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.op = PLANE_ONOFF;
> -                       data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> -                       test_setup(&data);
> -
>                        igt_system_suspend_autoresume(SUSPEND_STATE_MEM
> ,
> -                                                    
> SUSPEND_TEST_NONE);
> -                       igt_assert(psr_wait_entry_if_enabled(&data));
> -                       run_test(&data);
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%ssuspend",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.op =
> PLANE_ONOFF;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_CURSOR;
> +                                               test_setup(&data);
> +                                               igt_system_suspend_au
> toresume(SUSPEND_STATE_MEM,
> +                                                                    
>          SUSPEND_TEST_NONE);
> +                                               igt_assert(pr_wait_en
> try(&data));
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.op = RENDER;
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       dpms_off_on(&data);
> +                                       run_test(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>         }
>  

Re: [igt-dev] [PATCH i-g-t 5/5] tests/intel/kms_psr: made test compaitable with pr

[Joshi, Kunal1]

Hello Jouni,

-----Original Message-----
From: Hogander, Jouni <jouni.hogander@intel.com> 
Sent: Monday, October 23, 2023 12:24 PM
To: Joshi, Kunal1 <kunal1.joshi@intel.com>; igt-dev@lists.freedesktop.org
Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Manna, Animesh <animesh.manna@intel.com>
Subject: Re: [PATCH i-g-t 5/5] tests/intel/kms_psr: made test compaitable with pr

On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> Modified kms_psr to have support for DP pr.
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  tests/intel/kms_psr.c | 402 ++++++++++++++++++++++++++++++++++------
> --
>  1 file changed, 331 insertions(+), 71 deletions(-)
> 
> diff --git a/tests/intel/kms_psr.c b/tests/intel/kms_psr.c index 
> ffecc5222..b14267dce 100644
> --- a/tests/intel/kms_psr.c
> +++ b/tests/intel/kms_psr.c
> @@ -229,6 +229,102 @@
>   * @plane_move:         Move plane position
>   */
>  
> +/**
> + * SUBTEST: pr_dpms
> + * Description: Check if pr is detecting changes when rendering
> operation
> + *              is performed  with dpms enabled or disabled
> + * Driver requirement: i915, xe
> + * Functionality: dpms, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_no_drrs
> + * Description: Check if pr is detecting changes when drrs is
> disabled
> + * Driver requirement: i915, xe
> + * Functionality: drrs, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_suspend
> + * Description: Check if pr is detecting changes when plane
> operation is
> + *              performed with suspend resume cycles
> + * Driver requirement: i915, xe
> + * Functionality: pr, suspend
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_basic
> + * Description: Basic check for pr if it is detecting changes made
> in planes
> + * Driver requirement: i915, xe
> + * Functionality: pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_%s_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on %arg[1] planes
> + * Driver requirement: i915
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @cursor:             Cursor plane
> + * @primary:            Primary plane
> + * @sprite:             Sprite plane
> + *
> + * arg[2]:
> + *
> + * @mmap_cpu:           MMAP CPU
> + * @mmap_gtt:           MMAP GTT
> + */
> +
> +/**
> + * SUBTEST: pr_primary_page_flip
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on primary planes
> + * Driver requirement: i915, xe
> + * Functionality: plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_primary_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on primary planes
> + * Driver requirement: i915, xe
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @blt:                Blitter
> + * @render:             Render
> + */
> +
> +/**
> + * SUBTEST: pr_%s_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on %arg[1] planes
> + * Driver requirement: i915, xe
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @cursor:             Cursor plane
> + * @sprite:             Sprite plane
> + *
> + * arg[2]:
> + *
> + * @blt:                Blitter
> + * @render:             Render
> + * @plane_onoff:        Plane On off
> + * @plane_move:         Move plane position */
> +
>  enum operations {
>         PAGE_FLIP,
>         MMAP_GTT,
> @@ -272,6 +368,9 @@ typedef struct {
>         igt_output_t *output;
>         bool with_psr_disabled;
>         bool supports_psr2;
> +        bool supports_psr;
> +       bool supports_pr;
> +       igt_output_t **pr_outputs;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data) @@ -316,7 +415,6 @@ static 
> void setup_output(data_t *data)
>  static void display_init(data_t *data)
>  {
>         igt_display_require(&data->display, data->drm_fd);
> -       setup_output(data);
>  }
>  
>  static void display_fini(data_t *data) @@ -444,6 +542,15 @@ static 
> bool psr_wait_entry_if_enabled(data_t
> *data)
>         return psr_wait_entry(data->debugfs_fd, data->op_psr_mode);
>  }
>  
> +static bool pr_wait_entry(data_t *data) {
> +        if (data->with_psr_disabled)
> +                return true;
> +
> +        return igt_wait(dp_pr_active_check(data->debugfs_fd,
> +                                          data->output), 500, 20); }
> +
>  static bool psr_wait_update_if_enabled(data_t *data)
>  {
>         if (data->with_psr_disabled)
> @@ -604,7 +711,10 @@ static void run_test(data_t *data)
>                 expected = "screen GREEN";
>                 break;
>         }
> -       igt_assert(psr_wait_update_if_enabled(data));
> +       if(strstr(data->output->name, "DP"))
> +               igt_assert(pr_wait_entry(data));
> +       else
> +               igt_assert(psr_wait_update_if_enabled(data));
>         manual(expected);
>  }
>  
> @@ -687,6 +797,7 @@ static void test_setup(data_t *data)
>  {
>         drmModeConnectorPtr connector;
>         bool psr_entered = false;
> +       bool pr_entered = false;
>  
>         igt_require_f(data->output,
>                       "No available output found\n"); @@ -703,15 
> +814,23 @@ static void test_setup(data_t *data)
>                 if (data->op_psr_mode == PSR_MODE_2)
>                         igt_require(data->supports_psr2);
>  
> -               psr_enable_if_enabled(data);
> -               setup_test_plane(data, data->test_plane_id);
> -               if (psr_wait_entry_if_enabled(data)) {
> -                       psr_entered = true;
> -                       break;
> +               if (strstr(data->output->name, "DP"))
> +               {
> +                       setup_test_plane(data, data->test_plane_id);
> +                       pr_entered = pr_wait_entry(data);
> +                       if(pr_entered)
> +                               break;
> +               }
> +               else {
> +                       psr_enable_if_enabled(data);
> +                       setup_test_plane(data, data->test_plane_id);
> +                       if(psr_wait_entry_if_enabled(data)) {
> +                               psr_entered = true;
> +                               break;
> +                       }
>                 }
>         }
> -
> -       igt_assert(psr_entered);
> +       igt_assert(psr_entered || pr_entered);
>  }
>  
>  static void dpms_off_on(data_t *data) @@ -748,10 +867,15 @@ data_t 
> data = {};
>  igt_main_args("", long_options, help_str, opt_handler, &data)
>  {
>         enum operations op;
> -       const char *append_subtest_name[2] = {
> +       const char *append_subtest_name[3] = {
>                 "",
> -               "psr2_"
> +               "psr2_",
> +               "pr_"
>         };
> +       int pr_output_count;
> +       igt_output_t *out;
> +       int modes[] = {PSR_MODE_1, PSR_MODE_1, PR_MODE};

PSR_MODE_1 is listed twice.

> +       int z;
>  
>         igt_fixture {
>                 data.drm_fd = drm_open_driver_master(DRIVER_INTEL | 
> DRIVER_XE); @@ -759,103 +883,239 @@ igt_main_args("", long_options, 
> help_str, opt_handler, &data)
>                 kmstest_set_vt_graphics_mode();
>                 data.devid = intel_get_drm_devid(data.drm_fd);
>  
> -               igt_require_f(sink_support(&data, PSR_MODE_1),
> -                             "Sink does not support PSR\n");
> -
> +               pr_output_count = 0;
> +               data.pr_outputs = (igt_output_t
> **)malloc(sizeof(igt_output_t *) * data.display.n_outputs);
> +               for_each_connected_output(&data.display, out)
> +                       if(output_supports_pr(data.debugfs_fd, out))
> +                               data.pr_outputs[pr_output_count++] =
> out;
> +               data.supports_psr = sink_support(&data, PSR_MODE_1);
>                 data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> +               data.supports_pr = pr_output_count > 0 ? true :
> false;
> +
> +               igt_require_f(data.supports_psr || data.supports_pr,
> +                             "Sink does not support PSR/PR\n");
>                 data.bops = buf_ops_create(data.drm_fd);
>                 display_init(&data);
>         }
>  
> -       for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <= 
> PSR_MODE_2;
> -            data.op_psr_mode++) {
> -
> +       for (z = 0; z < ARRAY_SIZE(modes); z++) {
> +               data.op_psr_mode = modes[z];

Why are you changing this? To my opinion original is more clear...

>                 igt_describe("Basic check for psr if it is detecting 
> changes made in planes");
> -               igt_subtest_f("%sbasic",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%sbasic",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }

> I would suggest using outputs and output_count instead of pr_* and drop setup_output for
> PSR as wll. Then you will have 1 output for PSR and many outpus for PR. This way there is no 
> need to duplicate so much.
> Same comment applies on each test below.
>
> BR,
>
> Jouni Högander

Sure agree, will address in next revision.

>                 }
>  
>                 igt_describe("Check if psr is detecting changes when 
> drrs is disabled");
> -               igt_subtest_f("%sno_drrs",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       igt_assert(drrs_disabled(&data));
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%sno_drrs",
> append_subtest_name[z]) {
> +                        if (data.op_psr_mode == PR_MODE)
> +                        {
> +                                for(int k = 0; k < pr_output_count;
> k++) {
> +                                        igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                        {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               igt_assert(drrs_disab
> led(&data));
> +                                               test_cleanup(&data);
> +
> +                                        }
> +                               }
> +                        }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       igt_assert(drrs_disabled(&dat
> a));
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>  
>                 for (op = PAGE_FLIP; op <= RENDER; op++) {
>                         igt_describe("Check if psr is detecting page- 
> flipping,memory mapping and "
>                                         "rendering operations 
> performed on primary planes");
> -                       igt_subtest_f("%sprimary_%s",
> -
> append_subtest_name[data.op_psr_mode],
> +                       igt_subtest_with_dynamic_f("%sprimary_%s",
> +                                     append_subtest_name[z],
>                                       op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
>                                             (op == MMAP_CPU || op == 
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id = 
> DRM_PLANE_TYPE_PRIMARY;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_PRIMARY;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>                 }
>  
>                 for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>                         igt_describe("Check if psr is detecting memory 
> mapping,rendering "
>                                         "and plane operations 
> performed on sprite planes");
> -                       igt_subtest_f("%ssprite_%s",
> -
> append_subtest_name[data.op_psr_mode],
> +                       igt_subtest_with_dynamic_f("%ssprite_%s",
> +                                     append_subtest_name[z],
>                                       op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
>                                             (op == MMAP_CPU || op == 
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id = 
> DRM_PLANE_TYPE_OVERLAY;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_OVERLAY;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_OVERLAY;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>  
>                         igt_describe("Check if psr is detecting memory 
> mapping, rendering "
> -                                       "and plane operations 
> performed on cursor planes");
> -                       igt_subtest_f("%scursor_%s",
> -
> append_subtest_name[data.op_psr_mode],
> -                                     op_str(op)) {
> +                                    "and plane operations performed
> on cursor planes");
> +                       igt_subtest_with_dynamic_f("%scursor_%s",
> +                                                 
> append_subtest_name[z],
> +                                                  op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
> -                                           (op == MMAP_CPU || op == 
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id = 
> DRM_PLANE_TYPE_CURSOR;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                                          (op == MMAP_CPU || op ==
> MMAP_GTT));
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_CURSOR;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_CURSOR;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>                 }
>  
> -               igt_describe("Check if psr is detecting changes when 
> rendering operation is performed"
> -                               "  with dpms enabled or disabled");
> -               igt_subtest_f("%sdpms",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.op = RENDER;
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       dpms_off_on(&data);
> -                       run_test(&data);
> -                       test_cleanup(&data);
> +               igt_describe("Check if psr is detecting changes when
> rendering operation is performed "
> +                            "with dpms enabled or disabled");
> +               igt_subtest_with_dynamic_f("%sdpms",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.op = RENDER;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               dpms_off_on(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.op = RENDER;
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       dpms_off_on(&data);
> +                                       run_test(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>  
>                 igt_describe("Check if psr is detecting changes when 
> plane operation is performed "
>                                 "with suspend resume cycles");
> -               igt_subtest_f("%ssuspend",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.op = PLANE_ONOFF;
> -                       data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> -                       test_setup(&data);
> -
>                        igt_system_suspend_autoresume(SUSPEND_STATE_MEM
> ,
> - SUSPEND_TEST_NONE);
> -                       igt_assert(psr_wait_entry_if_enabled(&data));
> -                       run_test(&data);
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%ssuspend",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.op =
> PLANE_ONOFF;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_CURSOR;
> +                                               test_setup(&data);
> +                                               igt_system_suspend_au
> toresume(SUSPEND_STATE_MEM,
> +                                                                    
>          SUSPEND_TEST_NONE);
> +                                               igt_assert(pr_wait_en
> try(&data));
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.op = RENDER;
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       dpms_off_on(&data);
> +                                       run_test(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>         }
>  
Thanks and Regards
Kunal Joshi

Re: [igt-dev] [PATCH i-g-t 5/5] tests/intel/kms_psr: made test compaitable with pr

[Joshi, Kunal1]

Hello Jouni,

Missed commenting on one of the review comments,
PSR_MODE_1 repeated should be PSR_MODE_2,
>  
> -       for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <= 
> PSR_MODE_2;
> -            data.op_psr_mode++) {
> -
> +       for (z = 0; z < ARRAY_SIZE(modes); z++) {
> +               data.op_psr_mode = modes[z];

> Why are you changing this? To my opinion original is more clear...

This doesn't do anything for PSR_MODE_2_SEL_FETCH, so had created new array
for all the modes that need to be tested
int modes[] = {PSR_MODE_1, PSR_MODE_2, PR_MODE};

	PSR_MODE_1,
 	PSR_MODE_2,
 	PSR_MODE_2_SEL_FETCH,
+	PR_MODE,
+	PR_MODE_SEL_FETCH,
 	PSR_DISABLED,

[igt-dev] ✗ Fi.CI.BAT: failure for extend kms_psr test to validate pr

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 8925 bytes --]

== Series Details ==

Series: extend kms_psr test to validate pr
URL   : https://patchwork.freedesktop.org/series/125379/
State : failure

== Summary ==

CI Bug Log - changes from IGT_7551 -> IGTPW_10037
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_10037 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_10037, please notify your bug team (lgci.bug.filing@intel.com) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/index.html

Participating hosts (36 -> 36)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (1): fi-hsw-4770 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_10037:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_psr@sprite_plane_onoff:
    - bat-jsl-1:          [PASS][1] -> [SKIP][2] +3 other tests skip
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-jsl-1/igt@kms_psr@sprite_plane_onoff.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-jsl-1/igt@kms_psr@sprite_plane_onoff.html
    - bat-jsl-3:          [PASS][3] -> [SKIP][4] +3 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-jsl-3/igt@kms_psr@sprite_plane_onoff.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-jsl-3/igt@kms_psr@sprite_plane_onoff.html

  
#### Warnings ####

  * igt@kms_psr@cursor_plane_move:
    - bat-adlm-1:         [SKIP][5] ([i915#1072]) -> [SKIP][6] +3 other tests skip
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-adlm-1/igt@kms_psr@cursor_plane_move.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-adlm-1/igt@kms_psr@cursor_plane_move.html
    - fi-tgl-1115g4:      [SKIP][7] ([fdo#110189]) -> [SKIP][8] +3 other tests skip
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/fi-tgl-1115g4/igt@kms_psr@cursor_plane_move.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/fi-tgl-1115g4/igt@kms_psr@cursor_plane_move.html
    - bat-rpls-1:         [SKIP][9] ([i915#1072]) -> [SKIP][10] +3 other tests skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-rpls-1/igt@kms_psr@cursor_plane_move.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-rpls-1/igt@kms_psr@cursor_plane_move.html
    - bat-mtlp-6:         [SKIP][11] ([i915#1072]) -> [SKIP][12] +3 other tests skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-mtlp-6/igt@kms_psr@cursor_plane_move.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-mtlp-6/igt@kms_psr@cursor_plane_move.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-adlp-9:         [SKIP][13] ([i915#1072]) -> [SKIP][14] +3 other tests skip
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-adlp-9/igt@kms_psr@sprite_plane_onoff.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-adlp-9/igt@kms_psr@sprite_plane_onoff.html
    - fi-rkl-11600:       [SKIP][15] ([i915#1072]) -> [SKIP][16] +3 other tests skip
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/fi-rkl-11600/igt@kms_psr@sprite_plane_onoff.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/fi-rkl-11600/igt@kms_psr@sprite_plane_onoff.html
    - bat-adls-5:         [SKIP][17] ([i915#1072]) -> [SKIP][18] +3 other tests skip
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-adls-5/igt@kms_psr@sprite_plane_onoff.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-adls-5/igt@kms_psr@sprite_plane_onoff.html
    - bat-dg1-5:          [SKIP][19] ([i915#1072] / [i915#4078]) -> [SKIP][20] +3 other tests skip
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-dg1-5/igt@kms_psr@sprite_plane_onoff.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-dg1-5/igt@kms_psr@sprite_plane_onoff.html
    - bat-dg2-9:          [SKIP][21] ([i915#1072]) -> [SKIP][22] +3 other tests skip
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-dg2-9/igt@kms_psr@sprite_plane_onoff.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-dg2-9/igt@kms_psr@sprite_plane_onoff.html
    - bat-dg2-11:         [SKIP][23] ([i915#1072]) -> [SKIP][24] +3 other tests skip
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-dg2-11/igt@kms_psr@sprite_plane_onoff.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-dg2-11/igt@kms_psr@sprite_plane_onoff.html

  
Known issues
------------

  Here are the changes found in IGTPW_10037 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][25] ([fdo#109271] / [i915#2190])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][26] ([fdo#109271] / [i915#4613]) +3 other tests skip
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][27] ([i915#1886])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@kms_dsc@dsc-basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][28] ([fdo#109271]) +13 other tests skip
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/fi-kbl-soraka/igt@kms_dsc@dsc-basic.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-dg2-9:          NOTRUN -> [SKIP][29] ([i915#1845] / [i915#9197])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-dg2-9/igt@kms_pipe_crc_basic@suspend-read-crc.html

  * igt@kms_psr@cursor_plane_move:
    - fi-skl-6600u:       [PASS][30] -> [SKIP][31] ([fdo#109271]) +3 other tests skip
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/fi-skl-6600u/igt@kms_psr@cursor_plane_move.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/fi-skl-6600u/igt@kms_psr@cursor_plane_move.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0@smem:
    - bat-dg2-9:          [INCOMPLETE][32] ([i915#9275]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7551/bat-dg2-9/igt@gem_exec_suspend@basic-s0@smem.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/bat-dg2-9/igt@gem_exec_suspend@basic-s0@smem.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359
  [i915#7952]: https://gitlab.freedesktop.org/drm/intel/issues/7952
  [i915#8981]: https://gitlab.freedesktop.org/drm/intel/issues/8981
  [i915#9197]: https://gitlab.freedesktop.org/drm/intel/issues/9197
  [i915#9275]: https://gitlab.freedesktop.org/drm/intel/issues/9275


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7551 -> IGTPW_10037

  CI-20190529: 20190529
  CI_DRM_13777: 1b4fd688d213556268c50f853746c94c9a0cfee7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10037: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/index.html
  IGT_7551: 15e7d92ca5f98d10feffa27a76724c33cbd68da5 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

+igt@kms_psr@pr_basic
+igt@kms_psr@pr_cursor_blt
+igt@kms_psr@pr_cursor_mmap_cpu
+igt@kms_psr@pr_cursor_mmap_gtt
+igt@kms_psr@pr_cursor_plane_move
+igt@kms_psr@pr_cursor_plane_onoff
+igt@kms_psr@pr_cursor_render
+igt@kms_psr@pr_dpms
+igt@kms_psr@pr_no_drrs
+igt@kms_psr@pr_primary_blt
+igt@kms_psr@pr_primary_mmap_cpu
+igt@kms_psr@pr_primary_mmap_gtt
+igt@kms_psr@pr_primary_page_flip
+igt@kms_psr@pr_primary_render
+igt@kms_psr@pr_sprite_blt
+igt@kms_psr@pr_sprite_mmap_cpu
+igt@kms_psr@pr_sprite_mmap_gtt
+igt@kms_psr@pr_sprite_plane_move
+igt@kms_psr@pr_sprite_plane_onoff
+igt@kms_psr@pr_sprite_render
+igt@kms_psr@pr_suspend
-igt@xe_access_counter@invalid-param

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/index.html

[-- Attachment #2: Type: text/html, Size: 10574 bytes --]

[igt-dev] ✗ CI.xeBAT: failure for extend kms_psr test to validate pr

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3309 bytes --]

== Series Details ==

Series: extend kms_psr test to validate pr
URL   : https://patchwork.freedesktop.org/series/125379/
State : failure

== Summary ==

CI Bug Log - changes from XEIGT_7551_BAT -> XEIGTPW_10037_BAT
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with XEIGTPW_10037_BAT absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in XEIGTPW_10037_BAT, please notify your bug team (lgci.bug.filing@intel.com) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (4 -> 3)
------------------------------

  Missing    (1): bat-dg2-oem2 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in XEIGTPW_10037_BAT:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_psr@primary_page_flip:
    - bat-adlp-7:         [PASS][1] -> [SKIP][2] +2 other tests skip
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7551/bat-adlp-7/igt@kms_psr@primary_page_flip.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10037/bat-adlp-7/igt@kms_psr@primary_page_flip.html

  
#### Warnings ####

  * igt@kms_psr@cursor_plane_move:
    - bat-atsm-2:         [SKIP][3] ([Intel XE#535]) -> [SKIP][4] +2 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7551/bat-atsm-2/igt@kms_psr@cursor_plane_move.html
   [4]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10037/bat-atsm-2/igt@kms_psr@cursor_plane_move.html

  
Known issues
------------

  Here are the changes found in XEIGTPW_10037_BAT that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1:
    - bat-adlp-7:         [FAIL][5] ([Intel XE#480]) -> [PASS][6] +2 other tests pass
   [5]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7551/bat-adlp-7/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html
   [6]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10037/bat-adlp-7/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html

  * {igt@xe_create@create-execqueues-noleak}:
    - bat-adlp-7:         [FAIL][7] ([Intel XE#524]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7551/bat-adlp-7/igt@xe_create@create-execqueues-noleak.html
   [8]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10037/bat-adlp-7/igt@xe_create@create-execqueues-noleak.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [Intel XE#480]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/480
  [Intel XE#524]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/524
  [Intel XE#535]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/535


Build changes
-------------

  * IGT: IGT_7551 -> IGTPW_10037

  IGTPW_10037: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10037/index.html
  IGT_7551: 15e7d92ca5f98d10feffa27a76724c33cbd68da5 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  xe-442-a0a80950176b39dbc76f8faa92fddf6caaa06191: a0a80950176b39dbc76f8faa92fddf6caaa06191

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10037/index.html

[-- Attachment #2: Type: text/html, Size: 3981 bytes --]