[igt-dev] [PATCH i-g-t] tests/kms_cursor_legacy: Disable Intel's PSR2 selective fetch in this test

[Hogander, Jouni]

Hello Jose,

Couple of comments below.

On Tue, 2021-11-23 at 09:10 -0800, José Roberto de Souza wrote:
> When doing a primary or sprite plane flip, PSR2 selective fetch code
> also adds all the planes including cursor that overlaps with the
> area being updated, so this causes legacy cursor API calls to
> wait for a pending atomic commit to finish causing tests that do
> checks with vblank counters.
> 
> So here when running in an Intel platform that has PSR2 selective
> fetch enabled, it will switch to PSR1 before executing the subtests.
> Because what this whole test mostly wants to do, is check if
> userspace
> can do asynchronous cursors updates.

If the testcase can't be run with PSR2 selective fetch I would expect
PSR2 selective fetch is either disabled or this testcase is skipped
rather than switching to PSR1. Do you have some specific reason for
choosing switching to PSR1?

> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2346
> Cc: Mika Kahola <mika.kahola at intel.com>
> Cc: Jouni Hogander <jouni.hogander at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  lib/igt_psr.c             | 31 +++++++++++++++++++++++++++++++
>  lib/igt_psr.h             |  2 ++
>  tests/kms_cursor_legacy.c | 10 ++++++++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 8fa8b192f..554fe243c 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -280,3 +280,34 @@ bool i915_psr2_selective_fetch_check(int drm_fd)
>  
>  	return ret;
>  }
> +
> +/*
> + * Check if PSR2 selective fetch is enabled, if yes switch to PSR1
> and return
> + * true otherwise return false.
> + */
> +bool i915_psr2_selective_fetch_disable(int drm_fd)
> +{
> +	int debugfs_fd;
> +	bool ret = false;
> +
> +	if (!is_i915_device(drm_fd))
> +		return ret;
> +
> +	debugfs_fd = igt_debugfs_dir(drm_fd);
> +	if (psr2_selective_fetch_check(debugfs_fd)) {
> +		psr_set(drm_fd, debugfs_fd, PSR_MODE_1);
> +		ret = true;
> +	}
> +
> +	close(debugfs_fd);
> +	return ret;
> +}

This function is named as i915_psr2_selective_fetch_disable. I would
expect it disables psr2 selective fetch rather than switches to psr1.

> +
> +void i915_psr2_selective_fetch_restore(int drm_fd)
> +{
> +	int debugfs_fd;
> +
> +	debugfs_fd = igt_debugfs_dir(drm_fd);
> +	psr_set(drm_fd, debugfs_fd, PSR_MODE_2_SEL_FETCH);
> +	close(debugfs_fd);
> +}
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index a075f336d..0f60a1011 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -48,5 +48,7 @@ bool psr2_wait_su(int debugfs_fd, uint16_t
> *num_su_blocks);
>  void psr_print_debugfs(int debugfs_fd);
>  
>  bool i915_psr2_selective_fetch_check(int drm_fd);
> +bool i915_psr2_selective_fetch_disable(int drm_fd);
> +void i915_psr2_selective_fetch_restore(int drm_fd);
>  
>  #endif
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index cd2f84984..e2490902e 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -27,6 +27,7 @@
>  
>  #include "i915/gem.h"
>  #include "igt.h"
> +#include "igt_psr.h"
>  #include "igt_rand.h"
>  #include "igt_stats.h"
>  
> @@ -1421,6 +1422,7 @@ igt_main
>  {
>  	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
>  	igt_display_t display = { .drm_fd = -1 };
> +	bool intel_psr2_restore = false;
>  	int i;
>  
>  	igt_fixture {
> @@ -1428,6 +1430,12 @@ igt_main
>  		kmstest_set_vt_graphics_mode();
>  
>  		igt_display_require(&display, display.drm_fd);
> +		/*
> +		 * Not possible to evade vblank after a primary or
> sprite plane
> +		 * 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_selective_fetch_disable(display.drm_fd);
>  	}
>  
>  	/*Test description for pipe-* and all-pipe-* tests*/
> @@ -1609,6 +1617,8 @@ igt_main
>  		}
>  
>  	igt_fixture {
> +		if (intel_psr2_restore)
> +			i915_psr2_selective_fetch_restore(display.drm_f
> d);
>  		igt_display_fini(&display);
>  	}
>  }