[PATCH i-g-t 2/3] lib/igt_psr: add support for PR selective update
Hogander, Jouni
jouni.hogander at intel.com
Mon Feb 19 08:56:12 UTC 2024
On Mon, 2024-02-19 at 13:42 +0530, Joshi, Kunal1 wrote:
> Hello Jouni,
>
> On 2/19/2024 1:30 PM, Hogander, Jouni wrote:
> > On Sun, 2024-02-18 at 14:47 +0530, 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 at intel.com>
> > > Cc: Animesh Manna <animesh.manna at intel.com>
> > > Cc: Arun R Murthy <arun.r.murthy at intel.com>
> > > Signed-off-by: Kunal Joshi <kunal1.joshi at 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");
> > There is no need to change this function.
> > psr2_selective_fetch_check
> > (now selective_fetch_check) works for PR and PSR.
> >
> > BR,
> >
> > Jouni Högander
> Have modified selective_fetch_check to check for a particular output,
>
> bool selective_fetch_check(int debugfs_fd, igt_output_t *output)
> {
> char buf[PSR_STATUS_MAX_LEN];
> char debugfs_file[128] = {0};
>
> SET_DEBUGFS_PATH(output, debugfs_file);
> igt_debugfs_simple_read(debugfs_fd, debugfs_file, buf,
> sizeof(buf));
>
> return strstr(buf, "PSR2 selective fetch: enabled");
> }
>
> Will this be not required?
No it has to support providing ouput as parameter. My point is that
selective_fetch_check works for PR as well. and:
psr_set(drm_fd, debugfs_fd, PSR_MODE_1, output);
Will switch to Panel Replay Full Frame update for Panel Replay and PSR1
for PSR. If we ever need to take into account panel supporting both I
think we should do that by adding disable flag for Panel Replay
instead of setting the mode. See:
https://patchwork.freedesktop.org/series/128193/
and specifically:
https://patchwork.freedesktop.org/patch/575161/?series=128193&rev=3
In practice no need to touch contents of
i915_pr_psr2_sel_fetch_to_pr_psr1 at all. It will switch to PSR1 in
case of PSR and Panel Replay Full Frame update in case of Panel
Replay.
Also psr_set should use same values for Panel Replay as are used for
PSR. I'm sorry if I have instructed otherwise earlier. This has somehow
evolved during writing the code.
BR,
Jouni Högander
>
> Thanks and Regards
> Kunal Joshi
> >
> > > 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);
> > > }
> > >
> > > @@ -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.dr
> > > m_fd
> > > , NULL);
> > > igt_display_fini(&display);
> > > drm_close_driver(display.drm_fd);
> > > }
More information about the igt-dev
mailing list