[igt-dev] [v3 5/9] tests/kms_plane_scaling: Upscaling on 2 planes

[Sharma, Swati2]

On 09-Feb-22 2:06 PM, Ville Syrjälä wrote:
> On Tue, Feb 08, 2022 at 11:20:35AM +0530, Swati Sharma wrote:
>> i915 driver supports 2 scalers per pipe. Added test case
>> to upscale 2 overlay planes simultaneously on single pipe.
>>
>> v2: -add check for min 2 scalers
>>      -add igt_require() for overlay planes, fixes CRASH on KBL
>>
>> v3: -test modified for n planes (Ville)
>>
>> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
>> ---
>>   tests/kms_plane_scaling.c | 74 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>
>> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
>> index 22563fac..f813c5b4 100644
>> --- a/tests/kms_plane_scaling.c
>> +++ b/tests/kms_plane_scaling.c
>> @@ -29,6 +29,11 @@
>>   
>>   IGT_TEST_DESCRIPTION("Test display plane scaling");
>>   
>> +/* Test flags. */
>> +enum {
>> +        TEST_UP_UP_SCALING = 1 << 0,
>> +};
>> +
>>   typedef struct {
>>   	uint32_t devid;
>>   	int drm_fd;
>> @@ -358,6 +363,67 @@ test_plane_downscaling(data_t *d, enum pipe pipe, igt_output_t *output)
>>   	}
>>   }
>>   
>> +static void
>> +__test_plane_up_up_scaling(data_t *d, enum pipe pipe, igt_output_t *output)
>> +{
>> +	drmModeModeInfo *mode;
>> +	int width, height;
>> +	igt_display_t *display = &d->display;
>> +
>> +	cleanup_crtc(d);
>> +
>> +	igt_output_set_pipe(output, pipe);
>> +	mode = igt_output_get_mode(output);
>> +	width = height = 20;
>> +
>> +	for (int k = 0; k < display->pipes[pipe].n_planes; k++) {
>> +		igt_plane_t *plane1, *plane2;
>> +
>> +		plane1 = &display->pipes[pipe].planes[k];
>> +		igt_require(plane1);
>> +		plane2 = &display->pipes[pipe].planes[k+1];
>> +		igt_require(plane2);
>> +
>> +		if (plane1->type == DRM_PLANE_TYPE_PRIMARY  || plane1->type == DRM_PLANE_TYPE_CURSOR ||
>> +		    plane2->type == DRM_PLANE_TYPE_PRIMARY || plane2->type ==DRM_PLANE_TYPE_CURSOR)
>> +				continue;
>> +
>> +		igt_create_color_pattern_fb(display->drm_fd,
>> +					    width, height,
>> +					    DRM_FORMAT_XRGB8888,
>> +					    I915_TILING_NONE,
>> +					    1.0, 0.0, 0.0, &d->fb[1]);
>> +		igt_create_color_pattern_fb(display->drm_fd,
>> +					    width, height,
>> +					    DRM_FORMAT_XRGB8888,
>> +					    I915_TILING_NONE,
>> +					    0.0, 1.0, 0.0, &d->fb[2]);
> 
> Recreating the same fbs every iteration seems pointless/expensive.

Fixed in v4.

> 
>> +
>> +		igt_plane_set_fb(plane1, &d->fb[1]);
>> +		igt_plane_set_fb(plane2, &d->fb[2]);
>> +
>> +		/* 1st overlay plane upscaling */
>> +		igt_plane_set_size(plane1, mode->hdisplay, mode->vdisplay);
>> +		/* 2nd overlay plane upscaling */
>> +		igt_plane_set_size(plane2, mode->hdisplay - 20, mode->vdisplay - 20);
>> +		igt_display_commit2(display, COMMIT_ATOMIC);
>> +
>> +		igt_plane_set_fb(plane1, NULL);
>> +		igt_plane_set_fb(plane2, NULL);
>> +		igt_remove_fb(display->drm_fd, &d->fb[1]);
>> +		igt_remove_fb(display->drm_fd, &d->fb[2]);
>> +	}
>> +}
>> +
>> +static void
>> +test_plane_scaling_combo(data_t *d, enum pipe pipe, igt_output_t *output, uint32_t flags)
>> +{
>> +	igt_require(get_num_scalers(d, pipe) >= 2);
>> +
>> +	if (flags & TEST_UP_UP_SCALING)
> 
> What's the point of this flag based dispatcher thing? Just call the
> right function directly?

okay.

> 
>> +		__test_plane_up_up_scaling(d, pipe, output);
>> +}
>> +
>>   static bool test_pipe_iteration(data_t *data, enum pipe pipe, int iteration)
>>   {
>>   	if (!is_i915_device(data->drm_fd) ||
>> @@ -689,6 +755,14 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>>   					test_plane_downscaling(&data, pipe, output);
>>   		}
>>   
>> +		igt_describe("Tests upscaling of 2 overlay planes.");
>> +		igt_subtest_with_dynamic("plane-up-up-scaling") {
> 
> Bit of a funny name that, but don't really have a great idea
> how to make it any better.

Any suggestions now? Already tried many names, did find any less funny :p
Can we go with plane-upscaling-upscaling ?
> 
>> +			for_each_pipe_with_single_output(&data.display, pipe, output)
>> +				igt_dynamic_f("pipe-%s-%s-plane-up-up-scaling",
>> +					      kmstest_pipe_name(pipe), igt_output_name(output))
>> +					test_plane_scaling_combo(&data, pipe, output, TEST_UP_UP_SCALING);
>> +		}
>> +
>>   		igt_describe("Tests scaling with pixel formats.");
>>   		igt_subtest_with_dynamic("scaler-with-pixel-format") {
>>   			for_each_pipe_with_single_output(&data.display, pipe, output)
>> -- 
>> 2.25.1
> 

-- 
~Swati Sharma