[igt-dev] [PATCH i-g-t v6 1/8] kms_content_protection: Tests are defined by flags

[Shankar, Uma]

>-----Original Message-----
>From: C, Ramalingam
>Sent: Monday, May 6, 2019 11:56 AM
>To: Shankar, Uma <uma.shankar at intel.com>
>Cc: igt-dev at lists.freedesktop.org; Vetter, Daniel <daniel.vetter at intel.com>; Hiler,
>Arkadiusz <arkadiusz.hiler at intel.com>
>Subject: Re: [igt-dev] [PATCH i-g-t v6 1/8] kms_content_protection: Tests are defined
>by flags
>
>On 2019-05-06 at 11:31:28 +0530, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: igt-dev [mailto:igt-dev-bounces at lists.freedesktop.org] On
>> >Behalf Of Ramalingam C
>> >Sent: Thursday, May 2, 2019 6:46 PM
>> >To: igt-dev at lists.freedesktop.org; Vetter, Daniel
>> ><daniel.vetter at intel.com>; Hiler, Arkadiusz
>> ><arkadiusz.hiler at intel.com>
>> >Subject: [igt-dev] [PATCH i-g-t v6 1/8] kms_content_protection: Tests
>> >are defined by flags
>> >
>> >Considering increase of subtests for kms_content_protection, tests
>> >are defined through flags.
>> >
>> >Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
>> >Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>> >---
>> > tests/kms_content_protection.c | 19 +++++++++++--------
>> > 1 file changed, 11 insertions(+), 8 deletions(-)
>> >
>> >diff --git a/tests/kms_content_protection.c
>> >b/tests/kms_content_protection.c index ae6ab497ea21..051a3dfec5ba
>> >100644
>> >--- a/tests/kms_content_protection.c
>> >+++ b/tests/kms_content_protection.c
>> >@@ -34,8 +34,11 @@ struct data {
>> > 	int drm_fd;
>> > 	igt_display_t display;
>> > 	struct igt_fb red, green;
>> >+	unsigned int cp_tests;
>> > } data;
>> >
>> >+#define CP_DPMS					(1 << 0)
>> >+
>> > #define CP_UNDESIRED				0
>> > #define CP_DESIRED				1
>> > #define CP_ENABLED				2
>> >@@ -240,8 +243,7 @@ static void test_cp_lic(igt_output_t *output)  }
>> >
>> > static void test_content_protection_on_output(igt_output_t *output,
>> >-					      enum igt_commit_style s,
>> >-					      bool dpms_test)
>> >+					      enum igt_commit_style s)
>> > {
>> > 	igt_display_t *display = &data.display;
>> > 	igt_plane_t *primary;
>> >@@ -265,7 +267,7 @@ static void
>> >test_content_protection_on_output(igt_output_t
>> >*output,
>> > 		test_cp_enable_with_retry(output, s, 3);
>> > 		test_cp_lic(output);
>> >
>> >-		if (dpms_test) {
>> >+		if (data.cp_tests & CP_DPMS) {
>> > 			igt_pipe_set_prop_value(display, pipe,
>> > 						IGT_CRTC_ACTIVE, 0);
>> > 			igt_display_commit2(display, s);
>> >@@ -324,7 +326,7 @@ static bool sink_hdcp_capable(igt_output_t
>> >*output)
>> >
>> >
>> > static void
>> >-test_content_protection(enum igt_commit_style s, bool dpms_test)
>> >+test_content_protection(enum igt_commit_style s)
>> > {
>> > 	igt_display_t *display = &data.display;
>> > 	igt_output_t *output;
>> >@@ -341,7 +343,7 @@ test_content_protection(enum igt_commit_style s,
>> >bool
>> >dpms_test)
>> > 			continue;
>> > 		}
>> >
>> >-		test_content_protection_on_output(output, s, dpms_test);
>> >+		test_content_protection_on_output(output, s);
>> > 		valid_tests++;
>> > 	}
>> >
>> >@@ -359,16 +361,17 @@ igt_main
>> > 	}
>> >
>> > 	igt_subtest("legacy")
>> >-		test_content_protection(COMMIT_LEGACY, false);
>> >+		test_content_protection(COMMIT_LEGACY);
>> >
>> > 	igt_subtest("atomic") {
>> > 		igt_require(data.display.is_atomic);
>> >-		test_content_protection(COMMIT_ATOMIC, false);
>> >+		test_content_protection(COMMIT_ATOMIC);
>> > 	}
>> >
>> > 	igt_subtest("atomic-dpms") {
>> > 		igt_require(data.display.is_atomic);
>> >-		test_content_protection(COMMIT_ATOMIC, true);
>> >+		data.cp_tests = CP_DPMS;
>>
>> Not quite sure, but should we not reset this flag after the test so
>> that data_cp.tests have a clean slate for any other subtest to be executed later.
>
>Before the next test we are always assigning the required flags than editing the
>exiting one. So resetting the flag is not required.

Yeah, tests above DPMS don't even touch these flags, so there may be a case where
someone adds a test which doesn't require a FLAG. I feel it's good to reset the setting
once the test is done. Anyways it's just a nit-pick, will leave it to you - no hard objections.

This is,
Reviewed-by: Uma Shankar <uma.shankar at intel.com>

>-Ram
>>
>>
>> >+		test_content_protection(COMMIT_ATOMIC);
>> > 	}
>> >
>> > 	igt_fixture
>> >--
>> >2.19.1
>> >
>> >_______________________________________________
>> >igt-dev mailing list
>> >igt-dev at lists.freedesktop.org
>> >https://lists.freedesktop.org/mailman/listinfo/igt-dev