Re: [PATCH i-g-t v13] tests/kms_frontbuffer_tracking: Including DRRS test coverage

[Ramalingam C <ramalingam.c@intel.com>]

Summary from IRC discussions with Rodrigo:

drrs_disable will get called on all test setups(including the drrs 
non-capable). So we cant fail/skip the sub_test based on the return 
value from drrs_set(0)

drrs_enable will get called only when the test setup is capable of drrs. 
And also after the request for enabling drrs, drrs state is verified 
through i915_drrs_status.

So functionally it is not necessary to validate the debugfs_write return 
value. But no harm in double checking the error state for drrs_set(1). 
Rodrigo, as per above understanding i have mentioned the expected deltas 
inline. If you confirm, we will submit the next version. Thanks for the 
time.


On Wednesday 10 January 2018 08:17 PM, Lohith BS wrote:
> Dynamic Refresh Rate Switch(DRRS) is used to switch the panel's
> refresh rate to the lowest vrefresh supported by panel, when frame is
> not flipped for more than a Sec.
>
> In kernel, DRRS uses the front buffer tracking infrastructure.
> Hence DRRS test coverage is added along with other frontbuffer tracking
> based features such as FBC and PSR tests.
>
> Here, we are testing DRRS with other features in all possible
> combinations, in all required test cases, to capture any possible
> regression.
>
> v2: Addressed the comments and suggestions from Vlad, Marius.
>      The signoff details from the earlier work are also included.
>
> v3: Modified vblank rate calculation by using reply-sequence,
>      provided by drmWaitVBlank, as suggested by Chris Wilson.
>
> v4: As suggested from Chris Wilson and Daniel Vetter
>      1) Avoided using pthread for calculating vblank refresh rate,
>         instead used drmWaitVBlank reply sequence.
>      2) Avoided using kernel-specific info like transitional delays,
>         instead polling mechanism with timeout is used.
>      3) Included edp-DRRS as a subtest in kms_frontbuffer_tracking.c,
>         instead of having a separate test.
>
> v5: This patch adds DRRS as a new feature in the
>      kms_frontbuffer_tracking IGT.
>      DRRS switch to lower vrefresh rate is tested at slow-draw subtest.
>
>      Note:
>      1) Currently kernel doesn't have support to enable and disable
>         the DRRS feature dynamically(as in case of PSR). Hence if the
>         panel supports DRRS it will be enabled by default.
>
>      This is in continuation of last patch
> 	"https://patchwork.freedesktop.org/patch/162726/"
>
> v6: This patch adds runtime enable and disable feature for testing DRRS
>
> v7: This patch adds runtime enable and disable feature for testing DRRS
>      through debugfs entry "i915_drrs_ctl".
>
> v8: Commit message is updated to reflect current implementation.
>
> v9: Addressed Paulo Zanoni comments.
>      Check for DRRS_LOW at tests with OFFSCREEN + FBS_INDIVIDUAL.
>
> v10: Corrected DRRS state expectation in suspend related subtests.
>
> v11: Removing the global flag is_psr_drrs_combo [Rodrigo].
>
> v12: Rewriting the DRRS inactive deduction [Rodrigo].
>
> v13: En/Dis-able DRRS only when DRRS is capable on the setup.
>
> Signed-off-by: Lohith BS <lohith.bs@intel.com>
> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
> ---
>   tests/kms_frontbuffer_tracking.c | 172 +++++++++++++++++++++++++++++++++++++--
>   1 file changed, 164 insertions(+), 8 deletions(-)
>
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index 1601cab..6b2299b 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -34,7 +34,7 @@
>   
>   
>   IGT_TEST_DESCRIPTION("Test the Kernel's frontbuffer tracking mechanism and "
> -		     "its related features: FBC and PSR");
> +		     "its related features: FBC, PSR and DRRS");
>   
>   /*
>    * One of the aspects of this test is that, for every subtest, we try different
> @@ -105,8 +105,9 @@ struct test_mode {
>   		FEATURE_NONE  = 0,
>   		FEATURE_FBC   = 1,
>   		FEATURE_PSR   = 2,
> -		FEATURE_COUNT = 4,
> -		FEATURE_DEFAULT = 4,
> +		FEATURE_DRRS  = 4,
> +		FEATURE_COUNT = 8,
> +		FEATURE_DEFAULT = 8,
>   	} feature;
>   
>   	/* Possible pixel formats. We just use FORMAT_DEFAULT for most tests and
> @@ -182,6 +183,13 @@ struct {
>   	.can_test = false,
>   };
>   
> +#define MAX_DRRS_STATUS_BUF_LEN 256
> +
> +struct {
> +	bool can_test;
> +} drrs = {
> +	.can_test = false,
> +};
>   
>   #define SINK_CRC_SIZE 12
>   typedef struct {
> @@ -790,7 +798,14 @@ static void __debugfs_read(const char *param, char *buf, int len)
>   	buf[len] = '\0';
>   }
>   
> +static void __debugfs_write(const char *param, char *buf, int len)
> +{
> +	igt_assert_eq(igt_sysfs_write(drm.debugfs, param, buf, len - 1),
> +		      len - 1);
> +}
> +
redefining the __debugfs_write() as below:

static int __debugfs_write(const char *param, char *buf, int len)
{
	return igt_sysfs_write(drm.debugfs, param, buf, len - 1);
}

>   #define debugfs_read(p, arr) __debugfs_read(p, arr, sizeof(arr))
> +#define debugfs_write(p, arr) __debugfs_write(p, arr, sizeof(arr))
>   
>   static bool fbc_is_enabled(void)
>   {
> @@ -825,6 +840,62 @@ static void psr_print_status(void)
>   	igt_info("PSR status:\n%s\n", buf);
>   }
>   
> +void drrs_set(unsigned int val)
> +{
> +	char buf[2];
> +
> +	if (!drrs.can_test)
> +		return;
removing above two lines.
> +
> +	igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val);
> +	snprintf(buf, sizeof(buf), "%d", val);
> +	debugfs_write("i915_drrs_ctl", buf);
  replacing above line with below code

	ret = debugfs_write("i915_drrs_ctl", buf);

	/* drrs_enable can't fail, as that is called on drrs capable setup only. */
	if (val)
		igt_assert_eq(ret, sizeof(buf) - 1);

--Ram

> +}
> +
> +static bool is_drrs_high(void)
> +{
> +	char buf[MAX_DRRS_STATUS_BUF_LEN];
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx