[PATCH i-g-t 2/3] lib/igt_psr: add support for PR selective update
Joshi, Kunal1
kunal1.joshi at intel.com
Mon Feb 19 09:01:16 UTC 2024
Hello Jouni,
On 2/19/2024 2:26 PM, Hogander, Jouni wrote:
> 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
Ok got your point.
Will add your comment as a comment so nobody gets confused.
It will switch to PSR1 incase of PSR and Panel Replay Full Frame update in case of Panel Replay.
Thanks and Regards
Kunal Joshi
>> 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