[igt-dev] [PATCH i-g-t RFC] tests/i915/i915_pm_dc: Fix DC9 test

[Bhatt, Jigar]

Hi Anshuman,

-----Original Message-----
From: Gupta, Anshuman <anshuman.gupta at intel.com> 
Sent: Wednesday, August 4, 2021 11:21 AM
To: Bhatt, Jigar <jigar.bhatt at intel.com>; igt-dev at lists.freedesktop.org
Cc: Varide, Nischal <nischal.varide at intel.com>; Shankar, Uma <uma.shankar at intel.com>
Subject: RE: [PATCH i-g-t RFC] tests/i915/i915_pm_dc: Fix DC9 test



> -----Original Message-----
> From: Bhatt, Jigar <jigar.bhatt at intel.com>
> Sent: Thursday, July 29, 2021 6:50 PM
> To: igt-dev at lists.freedesktop.org
> Cc: Bhatt, Jigar <jigar.bhatt at intel.com>; Varide, Nischal 
> <nischal.varide at intel.com>; Gupta, Anshuman 
> <anshuman.gupta at intel.com>; Shankar, Uma <uma.shankar at intel.com>
> Subject: [PATCH i-g-t RFC] tests/i915/i915_pm_dc: Fix DC9 test
> 
Please use the standard patch header format.
> 1. Currently, check_dc9 is being called with reference
> (previous) counter being read after dpms_off call.
> At this time, already the counter is 0.
> We need to read the counter before dpms_off is called so that it holds 
> the previous value which had incremented while testing shallow states (DC5 or DC6).
> 
> 2. Check for a condition dc counter to be reset along with system 
> should in runtime suspend state :
> currect_dc < prev_dc && i915 runtime status == SUSPENDED.
> If above condition pass, test is passed otherwise test is failed.
> 
> 3. adding dc9_dpms_off()
> 
> Signed-off-by: Jigar Bhatt <jigar.bhatt at intel.com>
> ---
>  tests/i915/i915_pm_dc.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c index
> 9d0a15d81..a6fddb617 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -358,6 +358,15 @@ static void dpms_off(data_t *data)
>     (IGT_RUNTIME_PM_STATUS_SUSPENDED));
>  }
> 
> +static void dc9_dpms_off(data_t *data) { for (int i = 0; i < 
> +data->display.n_outputs; i++) { 
> +kmstest_set_connector_dpms(data->drm_fd,
> +   data-
> >display.outputs[i].config.connector,
> +   DRM_MODE_DPMS_OFF);
> +}
> +}
> +
>  static void dpms_on(data_t *data)
>  {
>  for (int i = 0; i < data->display.n_outputs; i++) { @@ -400,8 +409,11 
> @@ static bool check_dc9(uint32_t debugfs_fd, int prev_dc, bool dc6_supported, int
>   * so we rely on dc5/dc6 counter reset to check if display engine was 
> in DC9.
>   */
>  return igt_wait(dc6_supported ? read_dc_counter(debugfs_fd,
> CHECK_DC6) <
> -prev_dc : read_dc_counter(debugfs_fd, CHECK_DC5) < -prev_dc, seconds, 
> 100);
> +prev_dc &&
> +
> igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED)
Don't use igt_wait_for_pm_status here , it waits for 10 seconds internally.
We just need i915 runtime pm status and that should check against suspended.
This will waste may CI cycles.
Br,
Anshuman

If we are not using this igt_wait_for_status then it will check status only one time and mostly it has active status.
and after dpms_off it will take some time to be suspended that's why this igt_wait_for_status has
max 10 sec wait loop if status == SUSPENDED then it will automatically getting out of waiting time(10 sec).

If we only depend on i915 runtime status after immediate DPMS_OFF then it will take some milliseconds for getting SUSPENDED.

Thanks and Regards,
Jigar Bhatt
> +: read_dc_counter(debugfs_fd, CHECK_DC5) < prev_dc
> &&
> +
> igt_wait_for_pm_status(IGT_RUNTIME_STATUS_PM_SUSPENDED)
> +, seconds, 100);
>  }
> 
>  static void setup_dc9_dpms(data_t *data, int prev_dc, bool 
> dc6_supported) @@ -418,15 +430,16 @@ static void setup_dc9_dpms(data_t 
> *data, int prev_dc, bool dc6_supported)  static void 
> test_dc9_dpms(data_t *data)  {  bool dc6_supported;
> +int prev_dc;
> 
>  require_dc_counter(data->debugfs_fd, CHECK_DC5);  dc6_supported = 
> support_dc6(data->debugfs_fd);  setup_dc9_dpms(data, dc6_supported ? 
> read_dc_counter(data-
> >debugfs_fd, CHECK_DC6) :
>  read_dc_counter(data->debugfs_fd, CHECK_DC5), dc6_supported); 
> -dpms_off(data); -igt_assert_f(check_dc9(data->debugfs_fd, 
> dc6_supported ?
> -read_dc_counter(data->debugfs_fd,
> CHECK_DC6) :
> -read_dc_counter(data->debugfs_fd,
> CHECK_DC5),
> +prev_dc = dc6_supported ? read_dc_counter(data->debugfs_fd,
> CHECK_DC6) :
> +read_dc_counter(data->debugfs_fd, CHECK_DC5); dc9_dpms_off(data); 
> +igt_assert_f(check_dc9(data->debugfs_fd, prev_dc,
>  dc6_supported, 3000), "Not in DC9\n");  dpms_on(data);  }
> --
> 2.25.1