Re: [PATCH i-g-t 1/2] lib/igt_psr.c: add support for panel replay sf

["Joshi, Kunal1" <kunal1.joshi@intel.com>]

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

Hello Jouni,

On 2/5/2024 5:30 PM, Hogander, Jouni wrote:
> On Mon, 2024-01-22 at 13:19 +0530, Kunal Joshi wrote:
>> modify functions in igt_psr to extend support for validating
>> panel replay selective fetch.
>>
>> 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             | 46 ++++++++++++++++++++++++++-----------
>> --
>>   lib/igt_psr.h             |  6 ++---
>>   tests/intel/kms_psr2_sf.c |  8 ++++---
>>   tests/kms_async_flips.c   |  4 ++--
>>   tests/kms_cursor_legacy.c |  2 +-
>>   5 files changed, 42 insertions(+), 24 deletions(-)
>>
>> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
>> index 663bac163..1123c8d98 100644
>> --- a/lib/igt_psr.c
>> +++ b/lib/igt_psr.c
>> @@ -37,14 +37,21 @@ bool psr_disabled_check(int debugfs_fd)
>>          return strstr(buf, "PSR mode: disabled\n");
>>   }
>>   
>> -bool psr2_selective_fetch_check(int debugfs_fd)
>> +enum psr_mode selective_fetch_check(int debugfs_fd, igt_output_t
>> *output)
>>   {
>> +       char debugfs_file[128] = {0};
>>          char buf[PSR_STATUS_MAX_LEN];
>>   
>> -       igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status",
>> buf,
>> -                               sizeof(buf));
>> +       if (output)
>> +               sprintf(debugfs_file, "%s/i915_psr_status", output-
>>> name);
>> +       else
>> +               sprintf(debugfs_file, "%s", "i915_edp_psr_status");
>>   
>> -       return strstr(buf, "PSR2 selective fetch: enabled");
>> +       igt_debugfs_simple_read(debugfs_fd, debugfs_file, buf,
>> +                                                       sizeof(buf));
>> +
>> +       return strstr(buf, "PSR2 selective fetch: enabled") ?
>> PSR_MODE_2_SEL_FETCH :
>> +                  strstr(buf, "Panel Replay Selective Update
>> Enabled") ? PR_MODE_SEL_FETCH : PSR_DISABLED;
> There is no need to change this function. "PSR2 selective fetch:
> enable" is valid check for both of the cases. we will have in Panel
> Replay case:
>
> PSR mode: Panel Replay Selective Update Enabled
> Source PSR/PanelReplay ctl: enabled [0xc2004a99]
> Source PSR/PanelReplay status: IDLE [0x04000000]
> Busy frontbuffer bits: 0x00000000
> Performance counter: 0
> Frame:	PSR2 SU blocks:
> 0	0
> 1	0
> 2	0
> 3	0
> 4	0
> 5	0
> 6	0
> 7	0
> PSR2 selective fetch: enabled
>
> See:
>
> https://patchwork.freedesktop.org/patch/575163/?series=128193&rev=3
>
> BR,
>
> Jouni Högander
Thanks for checking this out, have float new series addressing your 
review comments.
>>   }
>>   
>>   static bool psr_active_check(int debugfs_fd, enum psr_mode mode,
>> igt_output_t *output)
>> @@ -246,6 +253,8 @@ bool psr_sink_support(int device, int debugfs_fd,
>> enum psr_mode mode, igt_output
>>                         (strstr(line, "[0x03]") || strstr(line,
>> "[0x04]")));
>>          case PR_MODE:
>>                  return strstr(line, "Panel Replay = yes");
>> +       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;
>> @@ -305,7 +314,7 @@ void psr_print_debugfs(int debugfs_fd)
>>          igt_info("%s", buf);
>>   }
>>   
>> -bool i915_psr2_selective_fetch_check(int drm_fd)
>> +bool i915_psr2_selective_fetch_check(int drm_fd, igt_output_t
>> *output)
>>   {
>>          int debugfs_fd;
>>          bool ret;
>> @@ -314,24 +323,24 @@ bool i915_psr2_selective_fetch_check(int
>> drm_fd)
>>                  return false;
>>   
>>          debugfs_fd = igt_debugfs_dir(drm_fd);
>> -       ret = psr2_selective_fetch_check(debugfs_fd);
>> +       ret = selective_fetch_check(debugfs_fd, output) !=
>> PSR_DISABLED;
>>          close(debugfs_fd);
>>   
>>          return ret;
>>   }
>>   
>> -/**
>> - * i915_psr2_sel_fetch_to_psr1
>> +/*
>> + * i915_pr_psr2_sel_fetch_to_pr_psr1
>>    *
>> - * Check if PSR2 selective fetch is enabled, if yes switch to PSR1
>> and returns
>> + * Check if PR/PSR2 selective fetch is enabled, if yes switch to
>> PR/PSR1 and returns
>>    * true otherwise returns false.
>> - * This function should be called from tests that are not compatible
>> with PSR2
>> - * selective fetch.
>>    *
>> + * @param drm_fd The file descriptor of the DRM device.
>> + * @param output The output for which the conversion is performed.
>>    * Returns:
>> - * True if PSR mode changed to PSR1, false otherwise.
>> + *     True if the conversion was successful, false otherwise.
>>    */
>> -bool i915_psr2_sel_fetch_to_psr1(int drm_fd)
>> +bool i915_pr_psr2_sel_fetch_to_pr_psr1(int drm_fd, igt_output_t
>> *output)
>>   {
>>          int debugfs_fd;
>>          bool ret = false;
>> @@ -340,11 +349,18 @@ bool i915_psr2_sel_fetch_to_psr1(int drm_fd)
>>                  return ret;
>>   
>>          debugfs_fd = igt_debugfs_dir(drm_fd);
>> -       if (psr2_selective_fetch_check(debugfs_fd)) {
>> +       switch (selective_fetch_check(debugfs_fd, output)) {
>> +       case PSR_MODE_2_SEL_FETCH:
>>                  psr_set(drm_fd, debugfs_fd, PSR_MODE_1);
>>                  ret = true;
>> +               break;
>> +       case PR_MODE_SEL_FETCH:
>> +               psr_set(drm_fd, debugfs_fd, PR_MODE);
>> +               ret = true;
>> +               break;
>> +       default:
>> +               ret = false;
>>          }
>> -
>>          close(debugfs_fd);
>>          return ret;
>>   }
>> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
>> index 36711c0d4..5dc70f23e 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);
>> +enum psr_mode 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);
>> @@ -57,9 +57,9 @@ bool psr2_wait_su(int debugfs_fd, uint16_t
>> *num_su_blocks);
>>   void psr_print_debugfs(int debugfs_fd);
>>   enum psr_mode psr_get_mode(int debugfs_fd);
>>   
>> -bool i915_psr2_selective_fetch_check(int drm_fd);
>> +bool i915_psr2_selective_fetch_check(int drm_fd, igt_output_t
>> *output);
>>   
>> -bool i915_psr2_sel_fetch_to_psr1(int drm_fd);
>> +bool i915_pr_psr2_sel_fetch_to_pr_psr1(int drm_fd, igt_output_t
>> *output);
>>   void i915_psr2_sel_fetch_restore(int drm_fd);
>>   
>>   #endif
>> diff --git a/tests/intel/kms_psr2_sf.c b/tests/intel/kms_psr2_sf.c
>> index ecf9ad77f..c826cd7c3 100644
>> --- a/tests/intel/kms_psr2_sf.c
>> +++ b/tests/intel/kms_psr2_sf.c
>> @@ -994,6 +994,7 @@ igt_main
>>          int fbc_status[] = {FBC_DISABLED, FBC_ENABLED};
>>   
>>          igt_fixture {
>> +               bool pr_or_psr2_selective_fetch_supported = false;
>>                  drmModeResPtr res;
>>   
>>                  data.drm_fd = drm_open_driver_master(DRIVER_INTEL |
>> DRIVER_XE);
>> @@ -1026,10 +1027,9 @@ igt_main
>>                  igt_info("Big framebuffer size %dx%d\n",
>>                           data.big_fb_width, data.big_fb_height);
>>   
>> -
>>                 igt_require_f(psr2_selective_fetch_check(data.debugfs_f
>> d),
>> -                             "PSR2 selective fetch not enabled\n");
>> -
>>                  for_each_pipe_with_valid_output(&data.display,
>> data.pipe, data.output) {
>> +                       pr_or_psr2_selective_fetch_supported |=
>> (selective_fetch_check(data.debugfs_fd,
>> +
>>                                      data.output) != PSR_DISABLED);
>>                          coexist_features[n_pipes] = 0;
>>                          if (check_psr2_support(&data)) {
>>                                  pipes[n_pipes] = data.pipe;
>> @@ -1041,6 +1041,8 @@ igt_main
>>                                  n_pipes++;
>>                          }
>>                  }
>> +               igt_require_f(pr_or_psr2_selective_fetch_supported,
>> +                                         "PR/PSR2 selective fetch
>> not supported\n");
>>          }
>>   
>>          for (y = 0; y < ARRAY_SIZE(fbc_status); y++) {
>> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
>> index a0349fa03..0ab8ea429 100644
>> --- a/tests/kms_async_flips.c
>> +++ b/tests/kms_async_flips.c
>> @@ -391,7 +391,7 @@ static void test_cursor(data_t *data)
>>           * necessary, causing the async flip to fail because async
>> flip is not
>>           * supported in cursor plane.
>>           */
>> -       igt_skip_on_f(i915_psr2_selective_fetch_check(data->drm_fd),
>> +       igt_skip_on_f(i915_pr_psr2_sel_fetch_to_pr_psr1(data->drm_fd,
>> NULL),
>>                        "PSR2 sel fetch causes cursor to be added to
>> primary plane " \
>>                        "pages flips and async flip is not supported in
>> cursor\n");
>>   
>> @@ -704,7 +704,7 @@ igt_main
>>                   * necessary, causing the async flip to fail because
>> async flip is not
>>                   * supported in cursor plane.
>>                   */
>> -
>>                 igt_skip_on_f(i915_psr2_selective_fetch_check(data.drm_
>> fd),
>> +               igt_skip_on_f(i915_pr_psr2_sel_fetch_to_pr_psr1(data.
>> drm_fd, NULL),
>>                                "PSR2 sel fetch causes cursor to be
>> added to primary plane " \
>>                                "pages flips and async flip is not
>> supported in cursor\n");
>>   
>> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
>> index 0017659d4..f453e2998 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);
>> +               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 "
Regards
Kunal Joshi

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