Re: [PATCH i-g-t 2/3] lib/igt_psr: add support for PR selective update

["Hogander, Jouni" <jouni.hogander@intel.com>]

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

On Mon, 2024-02-19 at 15:31 +0530, Joshi, Kunal1 wrote:

Hello Jouni,

On 2/18/2024 2:47 PM, Kunal Joshi wrote:


Extend the tests to cover panel replay selective fetch feature.


From kms_psr2_sf test point of view we have check_pr_psr2_sel_fetch_support

function to check if PR/PSR2 selective fetch is supported for an output

if output supports selective fetch then we check we enter DEEP_SLEEP mode

in run function


v2: fixed dynamic test name

v3: use check_psr2_support (Jouni)

v4: split patches (Jouni)


Cc: Jouni Högander <jouni.hogander@intel.com><mailto:jouni.hogander@intel.com>

Cc: Animesh Manna <animesh.manna@intel.com><mailto:animesh.manna@intel.com>

Cc: Arun R Murthy <arun.r.murthy@intel.com><mailto:arun.r.murthy@intel.com>

Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com><mailto:kunal1.joshi@intel.com>

---

 lib/igt_psr.c             | 36 ++++++++++++++++++++++++++----------

 lib/igt_psr.h             |  6 +++---

 tests/kms_cursor_legacy.c |  4 ++--

 3 files changed, 31 insertions(+), 15 deletions(-)


diff --git a/lib/igt_psr.c b/lib/igt_psr.c

index cad8cce05..9accd2047 100644

--- a/lib/igt_psr.c

+++ b/lib/igt_psr.c

@@ -41,7 +41,7 @@ bool psr_disabled_check(int debugfs_fd)

        return strstr(buf, "PSR mode: disabled\n");

 }



-bool psr2_selective_fetch_check(int debugfs_fd, igt_output_t *output)

+bool selective_fetch_check(int debugfs_fd, igt_output_t *output)

 {

        char buf[PSR_STATUS_MAX_LEN];

        char debugfs_file[128] = {0};

@@ -248,7 +248,9 @@ bool psr_sink_support(int device, int debugfs_fd, enum psr_mode mode, igt_output

                       (strstr(line, "PSR = yes") &&

                       (strstr(line, "[0x03]") || strstr(line, "[0x04]")));

        case PR_MODE:

-               return strstr(line, "Panel Replay = yes");

+               return strstr(line, "Panel Replay = yes, Panel Replay Selective Update = no");

+       case PR_MODE_SEL_FETCH:

+               return strstr(line, "Panel Replay = yes, Panel Replay Selective Update = yes");

        default:

                igt_assert_f(false, "Invalid psr mode\n");

                return false;

@@ -317,7 +319,7 @@ bool i915_psr2_selective_fetch_check(int drm_fd, igt_output_t *output)

                return false;



        debugfs_fd = igt_debugfs_dir(drm_fd);

-       ret = psr2_selective_fetch_check(debugfs_fd, output);

+       ret = selective_fetch_check(debugfs_fd, output);

        close(debugfs_fd);



        return ret;

@@ -334,17 +336,24 @@ bool i915_psr2_selective_fetch_check(int drm_fd, igt_output_t *output)

  * Returns:

  * True if PSR mode changed to PSR1, false otherwise.

  */

-bool i915_psr2_sel_fetch_to_psr1(int drm_fd, igt_output_t *output)

+bool i915_pr_psr2_sel_fetch_to_pr_psr1(int drm_fd, igt_output_t *output)

 {

        int debugfs_fd;

        bool ret = false;

+       enum psr_mode mode;



        if (!is_intel_device(drm_fd))

                return ret;



        debugfs_fd = igt_debugfs_dir(drm_fd);

-       if (psr2_selective_fetch_check(debugfs_fd, output)) {

-               psr_set(drm_fd, debugfs_fd, PSR_MODE_1, output);

+       if (selective_fetch_check(debugfs_fd, output)) {

+               mode = psr_get_mode(debugfs_fd, output);

+               if (mode == PR_MODE_SEL_FETCH)

+                       psr_set(drm_fd, debugfs_fd, PR_MODE, output);

+               else if (mode == PSR_MODE_2_SEL_FETCH)

+                       psr_set(drm_fd, debugfs_fd, PSR_MODE_1, output);

+               else

+                       igt_assert("switch not possible from current psr mode\n");

                ret = true;

        }



@@ -358,12 +367,17 @@ bool i915_psr2_sel_fetch_to_psr1(int drm_fd, igt_output_t *output)

  * Restore PSR2 selective fetch after tests were executed, this function should

  * only be called if i915_psr2_sel_fetch_to_psr1() returned true.

  */

-void i915_psr2_sel_fetch_restore(int drm_fd, igt_output_t *output)

+void i915_pr_psr2_sel_fetch_restore(int drm_fd, igt_output_t *output)

 {

        int debugfs_fd;

+       enum psr_mode mode;



        debugfs_fd = igt_debugfs_dir(drm_fd);

-       psr_set(drm_fd, debugfs_fd, PSR_MODE_2_SEL_FETCH, output);

+       mode = psr_get_mode(debugfs_fd, output);

+       if (mode == PR_MODE)

+               psr_set(drm_fd, debugfs_fd, PR_MODE_SEL_FETCH, output);

+       else

+               psr_set(drm_fd, debugfs_fd, PSR_MODE_2_SEL_FETCH, output);

        close(debugfs_fd);

 }

What do you think of this change, do we need to care about PR/PSR2 Selective fetch or just
write PSR_MODE_2_SEL_FETCH?

Good point. I think this function can be left untouched as well.

BR,

Jouni Högander

Thanks and Regards Kunal Joshi




@@ -389,11 +403,13 @@ enum psr_mode psr_get_mode(int debugfs_fd, igt_output_t *output)



        if (strstr(buf, "Panel Replay Enabled"))

                return PR_MODE;

+       else if (strstr(buf, "Panel Replay Selective Update Enabled"))

+               return PR_MODE_SEL_FETCH;

        else if (strstr(buf, "PSR2 selective fetch: enabled"))

                return PSR_MODE_2_SEL_FETCH;

-       else if (strstr(buf, "PSR2 enabled"))

+       else if (strstr(buf, "PSR2"))

                return PSR_MODE_2;

-       else if (strstr(buf, "PSR1 enabled"))

+       else if (strstr(buf, "PSR1"))

                return PSR_MODE_1;



        return PSR_DISABLED;

diff --git a/lib/igt_psr.h b/lib/igt_psr.h

index 372bef2b2..36ba7f068 100644

--- a/lib/igt_psr.h

+++ b/lib/igt_psr.h

@@ -46,7 +46,7 @@ enum fbc_mode {

 };



 bool psr_disabled_check(int debugfs_fd);

-bool psr2_selective_fetch_check(int debugfs_fd, igt_output_t *output);

+bool selective_fetch_check(int debugfs_fd, igt_output_t *output);

 bool psr_wait_entry(int debugfs_fd, enum psr_mode mode, igt_output_t *output);

 bool psr_wait_update(int debugfs_fd, enum psr_mode mode, igt_output_t *output);

 bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode, igt_output_t *output);

@@ -59,8 +59,8 @@ enum psr_mode psr_get_mode(int debugfs_fd, igt_output_t *output);



 bool i915_psr2_selective_fetch_check(int drm_fd, igt_output_t *output);



-bool i915_psr2_sel_fetch_to_psr1(int drm_fd, igt_output_t *output);

-void i915_psr2_sel_fetch_restore(int drm_fd, igt_output_t *output);

+bool i915_pr_psr2_sel_fetch_to_pr_psr1(int drm_fd, igt_output_t *output);

+void i915_pr_psr2_sel_fetch_restore(int drm_fd, igt_output_t *output);

 bool is_psr_enable_possible(int drm_fd, enum psr_mode mode);



 #endif

diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c

index a430f735a..91e5e9b07 100644

--- a/tests/kms_cursor_legacy.c

+++ b/tests/kms_cursor_legacy.c

@@ -1849,7 +1849,7 @@ igt_main

                 * page flip with cursor legacy APIS when Intel's PSR2 selective

                 * fetch is enabled, so switching PSR1 for this whole test.

                 */

-               intel_psr2_restore = i915_psr2_sel_fetch_to_psr1(display.drm_fd, NULL);

+               intel_psr2_restore = i915_pr_psr2_sel_fetch_to_pr_psr1(display.drm_fd, NULL);

        }



        igt_describe("Test checks how many cursor updates we can fit between vblanks "

@@ -2074,7 +2074,7 @@ igt_main



        igt_fixture {

                if (intel_psr2_restore)

-                       i915_psr2_sel_fetch_restore(display.drm_fd, NULL);

+                       i915_pr_psr2_sel_fetch_restore(display.drm_fd, NULL);

                igt_display_fini(&display);

                drm_close_driver(display.drm_fd);

        }


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