<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div>On Mon, 2024-02-19 at 15:31 +0530, Joshi, Kunal1 wrote:</div>
<blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
<p>Hello Jouni,<br>
</p>
<div class="moz-cite-prefix">On 2/18/2024 2:47 PM, Kunal Joshi wrote:<br>
</div>
<div><br>
</div>
<blockquote type="cite" cite="mid:20240218091704.2259937-3-kunal1.joshi@intel.com" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
<pre>Extend the tests to cover panel replay selective fetch feature.</pre>
<pre><br></pre>
<pre>From kms_psr2_sf test point of view we have check_pr_psr2_sel_fetch_support</pre>
<pre>function to check if PR/PSR2 selective fetch is supported for an output</pre>
<pre>if output supports selective fetch then we check we enter DEEP_SLEEP mode</pre>
<pre>in run function</pre>
<pre><br></pre>
<pre>v2: fixed dynamic test name</pre>
<pre>v3: use check_psr2_support (Jouni)</pre>
<pre>v4: split patches (Jouni)</pre>
<pre><br></pre>
<pre>Cc: Jouni Högander <a class="moz-txt-link-rfc2396E" href="mailto:jouni.hogander@intel.com"><jouni.hogander@intel.com></a></pre>
<pre>Cc: Animesh Manna <a class="moz-txt-link-rfc2396E" href="mailto:animesh.manna@intel.com"><animesh.manna@intel.com></a></pre>
<pre>Cc: Arun R Murthy <a class="moz-txt-link-rfc2396E" href="mailto:arun.r.murthy@intel.com"><arun.r.murthy@intel.com></a></pre>
<pre>Signed-off-by: Kunal Joshi <a class="moz-txt-link-rfc2396E" href="mailto:kunal1.joshi@intel.com"><kunal1.joshi@intel.com></a></pre>
<pre>---</pre>
<pre> lib/igt_psr.c             | 36 ++++++++++++++++++++++++++----------</pre>
<pre> lib/igt_psr.h             |  6 +++---</pre>
<pre> tests/kms_cursor_legacy.c |  4 ++--</pre>
<pre> 3 files changed, 31 insertions(+), 15 deletions(-)</pre>
<pre><br></pre>
<pre>diff --git a/lib/igt_psr.c b/lib/igt_psr.c</pre>
<pre>index cad8cce05..9accd2047 100644</pre>
<pre>--- a/lib/igt_psr.c</pre>
<pre>+++ b/lib/igt_psr.c</pre>
<pre>@@ -41,7 +41,7 @@ bool psr_disabled_check(int debugfs_fd)</pre>
<pre>     return strstr(buf, "PSR mode: disabled\n");</pre>
<pre> }</pre>
<pre> </pre>
<pre>-bool psr2_selective_fetch_check(int debugfs_fd, igt_output_t *output)</pre>
<pre>+bool selective_fetch_check(int debugfs_fd, igt_output_t *output)</pre>
<pre> {</pre>
<pre>     char buf[PSR_STATUS_MAX_LEN];</pre>
<pre>     char debugfs_file[128] = {0};</pre>
<pre>@@ -248,7 +248,9 @@ bool psr_sink_support(int device, int debugfs_fd, enum psr_mode mode, igt_output</pre>
<pre>                    (strstr(line, "PSR = yes") &&</pre>
<pre>                    (strstr(line, "[0x03]") || strstr(line, "[0x04]")));</pre>
<pre>     case PR_MODE:</pre>
<pre>-            return strstr(line, "Panel Replay = yes");</pre>
<pre>+            return strstr(line, "Panel Replay = yes, Panel Replay Selective Update = no");</pre>
<pre>+    case PR_MODE_SEL_FETCH:</pre>
<pre>+            return strstr(line, "Panel Replay = yes, Panel Replay Selective Update = yes");</pre>
<pre>     default:</pre>
<pre>             igt_assert_f(false, "Invalid psr mode\n");</pre>
<pre>             return false;</pre>
<pre>@@ -317,7 +319,7 @@ bool i915_psr2_selective_fetch_check(int drm_fd, igt_output_t *output)</pre>
<pre>             return false;</pre>
<pre> </pre>
<pre>     debugfs_fd = igt_debugfs_dir(drm_fd);</pre>
<pre>-    ret = psr2_selective_fetch_check(debugfs_fd, output);</pre>
<pre>+    ret = selective_fetch_check(debugfs_fd, output);</pre>
<pre>     close(debugfs_fd);</pre>
<pre> </pre>
<pre>     return ret;</pre>
<pre>@@ -334,17 +336,24 @@ bool i915_psr2_selective_fetch_check(int drm_fd, igt_output_t *output)</pre>
<pre>  * Returns:</pre>
<pre>  * True if PSR mode changed to PSR1, false otherwise.</pre>
<pre>  */</pre>
<pre>-bool i915_psr2_sel_fetch_to_psr1(int drm_fd, igt_output_t *output)</pre>
<pre>+bool i915_pr_psr2_sel_fetch_to_pr_psr1(int drm_fd, igt_output_t *output)</pre>
<pre> {</pre>
<pre>     int debugfs_fd;</pre>
<pre>     bool ret = false;</pre>
<pre>+    enum psr_mode mode;</pre>
<pre> </pre>
<pre>     if (!is_intel_device(drm_fd))</pre>
<pre>             return ret;</pre>
<pre> </pre>
<pre>     debugfs_fd = igt_debugfs_dir(drm_fd);</pre>
<pre>-    if (psr2_selective_fetch_check(debugfs_fd, output)) {</pre>
<pre>-            psr_set(drm_fd, debugfs_fd, PSR_MODE_1, output);</pre>
<pre>+    if (selective_fetch_check(debugfs_fd, output)) {</pre>
<pre>+            mode = psr_get_mode(debugfs_fd, output);</pre>
<pre>+            if (mode == PR_MODE_SEL_FETCH)</pre>
<pre>+                    psr_set(drm_fd, debugfs_fd, PR_MODE, output);</pre>
<pre>+            else if (mode == PSR_MODE_2_SEL_FETCH)</pre>
<pre>+                    psr_set(drm_fd, debugfs_fd, PSR_MODE_1, output);</pre>
<pre>+            else</pre>
<pre>+                    igt_assert("switch not possible from current psr mode\n");</pre>
<pre>             ret = true;</pre>
<pre>     }</pre>
<pre> </pre>
<pre>@@ -358,12 +367,17 @@ bool i915_psr2_sel_fetch_to_psr1(int drm_fd, igt_output_t *output)</pre>
<pre>  * Restore PSR2 selective fetch after tests were executed, this function should</pre>
<pre>  * only be called if i915_psr2_sel_fetch_to_psr1() returned true.</pre>
<pre>  */</pre>
<pre>-void i915_psr2_sel_fetch_restore(int drm_fd, igt_output_t *output)</pre>
<pre>+void i915_pr_psr2_sel_fetch_restore(int drm_fd, igt_output_t *output)</pre>
<pre> {</pre>
<pre>     int debugfs_fd;</pre>
<pre>+    enum psr_mode mode;</pre>
<pre> </pre>
<pre>     debugfs_fd = igt_debugfs_dir(drm_fd);</pre>
<pre>-    psr_set(drm_fd, debugfs_fd, PSR_MODE_2_SEL_FETCH, output);</pre>
<pre>+    mode = psr_get_mode(debugfs_fd, output);</pre>
<pre>+    if (mode == PR_MODE)</pre>
<pre>+            psr_set(drm_fd, debugfs_fd, PR_MODE_SEL_FETCH, output);</pre>
<pre>+    else</pre>
<pre>+            psr_set(drm_fd, debugfs_fd, PSR_MODE_2_SEL_FETCH, output);</pre>
<pre>     close(debugfs_fd);</pre>
<pre> }</pre>
</blockquote>
<p>What do you think of this change, do we need to care about PR/PSR2 Selective fetch or just<br>
write <span style="white-space: pre-wrap">PSR_MODE_2_SEL_FETCH?</span></p>
<p><span style="white-space: pre-wrap"></span></p>
</blockquote>
<div>Good point. I think this function can be left untouched as well.</div>
<div><br>
</div>
<div>BR,</div>
<div><br>
</div>
<div>Jouni Högander</div>
<blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
<p><span style="white-space: pre-wrap">Thanks and Regards Kunal Joshi </span></p>
<div><br>
</div>
<blockquote type="cite" cite="mid:20240218091704.2259937-3-kunal1.joshi@intel.com" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
<pre> </pre>
<pre>@@ -389,11 +403,13 @@ enum psr_mode psr_get_mode(int debugfs_fd, igt_output_t *output)</pre>
<pre> </pre>
<pre>     if (strstr(buf, "Panel Replay Enabled"))</pre>
<pre>             return PR_MODE;</pre>
<pre>+    else if (strstr(buf, "Panel Replay Selective Update Enabled"))</pre>
<pre>+            return PR_MODE_SEL_FETCH;</pre>
<pre>     else if (strstr(buf, "PSR2 selective fetch: enabled"))</pre>
<pre>             return PSR_MODE_2_SEL_FETCH;</pre>
<pre>-    else if (strstr(buf, "PSR2 enabled"))</pre>
<pre>+    else if (strstr(buf, "PSR2"))</pre>
<pre>             return PSR_MODE_2;</pre>
<pre>-    else if (strstr(buf, "PSR1 enabled"))</pre>
<pre>+    else if (strstr(buf, "PSR1"))</pre>
<pre>             return PSR_MODE_1;</pre>
<pre> </pre>
<pre>     return PSR_DISABLED;</pre>
<pre>diff --git a/lib/igt_psr.h b/lib/igt_psr.h</pre>
<pre>index 372bef2b2..36ba7f068 100644</pre>
<pre>--- a/lib/igt_psr.h</pre>
<pre>+++ b/lib/igt_psr.h</pre>
<pre>@@ -46,7 +46,7 @@ enum fbc_mode {</pre>
<pre> };</pre>
<pre> </pre>
<pre> bool psr_disabled_check(int debugfs_fd);</pre>
<pre>-bool psr2_selective_fetch_check(int debugfs_fd, igt_output_t *output);</pre>
<pre>+bool selective_fetch_check(int debugfs_fd, igt_output_t *output);</pre>
<pre> bool psr_wait_entry(int debugfs_fd, enum psr_mode mode, igt_output_t *output);</pre>
<pre> bool psr_wait_update(int debugfs_fd, enum psr_mode mode, igt_output_t *output);</pre>
<pre> bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode, igt_output_t *output);</pre>
<pre>@@ -59,8 +59,8 @@ enum psr_mode psr_get_mode(int debugfs_fd, igt_output_t *output);</pre>
<pre> </pre>
<pre> bool i915_psr2_selective_fetch_check(int drm_fd, igt_output_t *output);</pre>
<pre> </pre>
<pre>-bool i915_psr2_sel_fetch_to_psr1(int drm_fd, igt_output_t *output);</pre>
<pre>-void i915_psr2_sel_fetch_restore(int drm_fd, igt_output_t *output);</pre>
<pre>+bool i915_pr_psr2_sel_fetch_to_pr_psr1(int drm_fd, igt_output_t *output);</pre>
<pre>+void i915_pr_psr2_sel_fetch_restore(int drm_fd, igt_output_t *output);</pre>
<pre> bool is_psr_enable_possible(int drm_fd, enum psr_mode mode);</pre>
<pre> </pre>
<pre> #endif</pre>
<pre>diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c</pre>
<pre>index a430f735a..91e5e9b07 100644</pre>
<pre>--- a/tests/kms_cursor_legacy.c</pre>
<pre>+++ b/tests/kms_cursor_legacy.c</pre>
<pre>@@ -1849,7 +1849,7 @@ igt_main</pre>
<pre>              * page flip with cursor legacy APIS when Intel's PSR2 selective</pre>
<pre>              * fetch is enabled, so switching PSR1 for this whole test.</pre>
<pre>              */</pre>
<pre>-            intel_psr2_restore = i915_psr2_sel_fetch_to_psr1(display.drm_fd, NULL);</pre>
<pre>+            intel_psr2_restore = i915_pr_psr2_sel_fetch_to_pr_psr1(display.drm_fd, NULL);</pre>
<pre>     }</pre>
<pre> </pre>
<pre>     igt_describe("Test checks how many cursor updates we can fit between vblanks "</pre>
<pre>@@ -2074,7 +2074,7 @@ igt_main</pre>
<pre> </pre>
<pre>     igt_fixture {</pre>
<pre>             if (intel_psr2_restore)</pre>
<pre>-                    i915_psr2_sel_fetch_restore(display.drm_fd, NULL);</pre>
<pre>+                    i915_pr_psr2_sel_fetch_restore(display.drm_fd, NULL);</pre>
<pre>             igt_display_fini(&display);</pre>
<pre>             drm_close_driver(display.drm_fd);</pre>
<pre>     }</pre>
</blockquote>
<div></div>
</blockquote>
<div><br>
</div>
<div><span></span></div>
</body>
</html>