Re: [igt-dev] [PATCH i-g-t v2] tests/kms_cursor_crc: Limit cursor size based on platform capability.

["B S, Karthik" <karthik.b.s@intel.com>]

On 5/6/2019 5:46 PM, Ville Syrjälä wrote:
> On Mon, May 06, 2019 at 10:40:08AM +0530, Karthik B S wrote:
>> Limiting the cursor size of subtests to the maximum size listed as per
>> the platform capability, so that we can avoid skipping of subtests.
> Why?

Currently, cursor size is hard coded to have an upper limit of 512 and 
even though we've the max cursor size supported by the platform,

we still list all the sub-tests up to 512. And upon execution we skip 
them since the platform doesn't support it.

IMHO, this is redundant and we should be removing this hard coding.

Instead we should make use of the platform capability data, which we 
fetch from the kernel at run time.

So here the test is split into sub-tests based on structure(square and 
non square) instead of the actual dimension of the cursor.

And at run time the maximum size supported by the platform is fetched 
and the max cursor size used is limited based on this.
>
>> Instead of spliting into subtests based on cursor size,
>> here spliting of subtests is done based on functionality.
>> Maximum size of cursor is restricted at run time based
>> on platform capability.
>>
>> v2: Keep platform capability fetch inside igt_fixture.
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   tests/kms_cursor_crc.c | 102 ++++++++++++++++++++++++-------------------------
>>   1 file changed, 49 insertions(+), 53 deletions(-)
>>
>> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
>> index fd74fda..08a6c2a 100644
>> --- a/tests/kms_cursor_crc.c
>> +++ b/tests/kms_cursor_crc.c
>> @@ -44,6 +44,8 @@ IGT_TEST_DESCRIPTION(
>>   #ifndef DRM_CAP_CURSOR_HEIGHT
>>   #define DRM_CAP_CURSOR_HEIGHT 0x9
>>   #endif
>> +#define SQUARE    1
>> +#define NONSQUARE 0
>>   
>>   typedef struct {
>>   	int drm_fd;
>> @@ -470,9 +472,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int
>>   	enum pipe p;
>>   	int valid_tests = 0;
>>   
>> -	igt_require(cursor_w <= data->cursor_max_w &&
>> -		    cursor_h <= data->cursor_max_h);
>> -
>>   	for_each_pipe_with_valid_output(display, p, output) {
>>   		data->output = output;
>>   		data->pipe = p;
>> @@ -481,15 +480,15 @@ static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int
>>   
>>   		valid_tests++;
>>   
>> -		igt_info("Beginning %s on pipe %s, connector %s\n",
>> -			  igt_subtest_name(),
>> +		igt_info("Beginning %s with size %dx%d on pipe %s, connector %s\n",
>> +			  igt_subtest_name(), cursor_w, cursor_h,
>>   			  kmstest_pipe_name(data->pipe),
>>   			  igt_output_name(output));
>>   
>>   		testfunc(data);
>>   
>> -		igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
>> -			  igt_subtest_name(),
>> +		igt_info("\n%s with size %dx%d on pipe %s, connector %s: PASSED\n\n",
>> +			  igt_subtest_name(), cursor_w, cursor_h,
>>   			  kmstest_pipe_name(data->pipe),
>>   			  igt_output_name(output));
>>   
>> @@ -639,75 +638,72 @@ static void test_rapid_movement(data_t *data)
>>   
>>   }
>>   
>> -static void run_test_generic(data_t *data)
>> +static void test_cursor(data_t *data, void (*testfunc)(data_t *), bool square)
>>   {
>>   	int cursor_size;
>> -	for (cursor_size = 64; cursor_size <= 512; cursor_size *= 2) {
>> +	for (cursor_size = 64; cursor_size <= data->cursor_max_w;
>> +	     cursor_size *= 2) {
>>   		int w = cursor_size;
>>   		int h = cursor_size;
>>   
>> -		igt_fixture
>> -			create_cursor_fb(data, w, h);
>> -
>> -		/* Using created cursor FBs to test cursor support */
>> -		igt_subtest_f("cursor-%dx%d-onscreen", w, h)
>> -			run_test(data, test_crc_onscreen, w, h);
>> -		igt_subtest_f("cursor-%dx%d-offscreen", w, h)
>> -			run_test(data, test_crc_offscreen, w, h);
>> -		igt_subtest_f("cursor-%dx%d-sliding", w, h)
>> -			run_test(data, test_crc_sliding, w, h);
>> -		igt_subtest_f("cursor-%dx%d-random", w, h)
>> -			run_test(data, test_crc_random, w, h);
>> -		igt_subtest_f("cursor-%dx%d-dpms", w, h) {
>> +		/*
>> +		 * Test non-square cursors a bit on the platforms
>> +		 * that support such things. And make it a bit more
>> +		 * interesting by using a non-pot height.
>> +		 */
>> +		if (!square)
>> +			h /= 3;
>> +
>> +		create_cursor_fb(data, w, h);
>> +
>> +		run_test(data, testfunc, w, h);
>> +
>> +		igt_remove_fb(data->drm_fd, &data->fb);
>> +	}
>> +}
>> +
>> +static void run_test_generic(data_t *data)
>> +{
>> +		igt_subtest_f("square-cursor-onscreen")
>> +			test_cursor(data, test_crc_onscreen, SQUARE);
>> +		igt_subtest_f("square-cursor-offscreen")
>> +			test_cursor(data, test_crc_offscreen, SQUARE);
>> +		igt_subtest_f("square-cursor-sliding")
>> +			test_cursor(data, test_crc_sliding, SQUARE);
>> +		igt_subtest_f("square-cursor-random")
>> +			test_cursor(data, test_crc_random, SQUARE);
>> +		igt_subtest_f("square-cursor-dpms") {
>>   			data->flags = TEST_DPMS;
>> -			run_test(data, test_crc_random, w, h);
>> +			test_cursor(data, test_crc_random, SQUARE);
>>   			data->flags = 0;
>>   		}
>>   
>> -		igt_subtest_f("cursor-%dx%d-suspend", w, h) {
>> +		igt_subtest_f("square-cursor-suspend") {
>>   			data->flags = TEST_SUSPEND;
>> -			run_test(data, test_crc_random, w, h);
>> +			test_cursor(data, test_crc_random, SQUARE);
>>   			data->flags = 0;
>>   		}
>>   
>> -		igt_subtest_f("cursor-%dx%d-rapid-movement", w, h) {
>> -			run_test(data, test_rapid_movement, w, h);
>> +		igt_subtest_f("square-cursor-rapid-movement") {
>> +			test_cursor(data, test_rapid_movement, SQUARE);
>>   		}
>>   
>> -		igt_fixture
>> -			igt_remove_fb(data->drm_fd, &data->fb);
>> -
>> -		/*
>> -		 * Test non-square cursors a bit on the platforms
>> -		 * that support such things. And make it a bit more
>> -		 * interesting by using a non-pot height.
>> -		 */
>> -		h /= 3;
>> -
>> -		igt_fixture
>> -			create_cursor_fb(data, w, h);
>> -
>> -		/* Using created cursor FBs to test cursor support */
>> -		igt_subtest_f("cursor-%dx%d-onscreen", w, h) {
>> +		igt_subtest_f("non-square-cursor-onscreen") {
>>   			igt_require(has_nonsquare_cursors(data));
>> -			run_test(data, test_crc_onscreen, w, h);
>> +			test_cursor(data, test_crc_onscreen, NONSQUARE);
>>   		}
>> -		igt_subtest_f("cursor-%dx%d-offscreen", w, h) {
>> +		igt_subtest_f("non-square-cursor-offscreen") {
>>   			igt_require(has_nonsquare_cursors(data));
>> -			run_test(data, test_crc_offscreen, w, h);
>> +			test_cursor(data, test_crc_offscreen, NONSQUARE);
>>   		}
>> -		igt_subtest_f("cursor-%dx%d-sliding", w, h) {
>> +		igt_subtest_f("non-square-cursor-sliding") {
>>   			igt_require(has_nonsquare_cursors(data));
>> -			run_test(data, test_crc_sliding, w, h);
>> +			test_cursor(data, test_crc_sliding, NONSQUARE);
>>   		}
>> -		igt_subtest_f("cursor-%dx%d-random", w, h) {
>> +		igt_subtest_f("non-square-cursor-random") {
>>   			igt_require(has_nonsquare_cursors(data));
>> -			run_test(data, test_crc_random, w, h);
>> +			test_cursor(data, test_crc_random, NONSQUARE);
>>   		}
>> -
>> -		igt_fixture
>> -			igt_remove_fb(data->drm_fd, &data->fb);
>> -	}
>>   }
>>   
>>   static data_t data;
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev