Re: [igt-dev] [PATCH i-g-t 2/4] tests/kms_plane_multiple: Add tiled-none plane position subtests

["Kazlauskas, Nicholas" <Nicholas.Kazlauskas@amd.com>]

On 11/29/18 4:48 PM, Wentland, Harry wrote:
> 
> 
> On 2018-11-28 9:56 a.m., Nicholas Kazlauskas wrote:
>> The per-pipe plane position subtests are capable of running on
>> AMDGPU as long as they're not using i915 specific tiling formats.
>>
>> The test setup already supports being invoked with different tiling
>> modes so this patch introduces the new 'tiled-none' subtest that runs
>> without any tiling.
>>
>> The tiled-none tests are skipped on i915 to retain existing test
>> coverage and behavior on i915.
>>
>> v2: Use igt_display_has_format_mod helpers (Ville)
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>> ---
>>   tests/kms_plane_multiple.c | 25 ++++++++++++++++++-------
>>   1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
>> index 721afe59..e67e21a5 100644
>> --- a/tests/kms_plane_multiple.c
>> +++ b/tests/kms_plane_multiple.c
>> @@ -157,6 +157,10 @@ create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo
>>   
>>   	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>>   
>> +	igt_skip_on(!igt_display_has_format_mod(&data->display,
>> +						DRM_FORMAT_XRGB8888,
>> +						tiling));
>> +
>>   	fb_id = igt_create_fb(data->drm_fd,
>>   			      mode->hdisplay, mode->vdisplay,
>>   			      DRM_FORMAT_XRGB8888,
>> @@ -210,6 +214,8 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
>>   	y[primary->index] = 0;
>>   	for (i = 0; i < max_planes; i++) {
>>   		igt_plane_t *plane = igt_output_get_plane(output, i);
>> +		uint32_t plane_format;
>> +		uint64_t plane_tiling;
>>   
>>   		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>>   			continue;
>> @@ -223,10 +229,16 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
>>   
>>   		data->plane[i] = plane;
>>   
>> +		plane_format = data->plane[i]->type == DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888;
>> +		plane_tiling = data->plane[i]->type == DRM_PLANE_TYPE_CURSOR ? LOCAL_DRM_FORMAT_MOD_NONE : tiling;
>> +
>> +		igt_skip_on(!igt_plane_has_format_mod(plane, plane_format,
>> +						      plane_tiling));
>> +
>>   		igt_create_color_fb(data->drm_fd,
>>   				    size[i], size[i],
>> -				    data->plane[i]->type == DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888,
>> -				    data->plane[i]->type == DRM_PLANE_TYPE_CURSOR ? LOCAL_DRM_FORMAT_MOD_NONE : tiling,
>> +				    plane_format,
>> +				    plane_tiling,
>>   				    color->red, color->green, color->blue,
>>   				    &data->fb[i]);
>>   
>> @@ -291,13 +303,8 @@ test_plane_position(data_t *data, enum pipe pipe, uint64_t tiling)
>>   {
>>   	igt_output_t *output;
>>   	int connected_outs;
>> -	int devid = intel_get_drm_devid(data->drm_fd);
>>   	int n_planes = data->display.pipes[pipe].n_planes;
>>   
>> -	if ((tiling == LOCAL_I915_FORMAT_MOD_Y_TILED ||
>> -	     tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED))
>> -		igt_require(AT_LEAST_GEN(devid, 9));
>> -
> 
> Should we leave this hunk as-is since we only do tiling-none on non-i915 for now?
> 
> Not sure what the implications are of taking this check out.
> 
> Harry

This is the common implementation for the other tests as well as you can 
see below:

test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_Yf_TILED);
...
test_plane_position(data, pipe, LOCAL_DRM_FORMAT_MOD_NONE);
...

Within those helpers is the intel_get_drm_devid - it fails the test when 
the drm_fd isn't i915.

The igt_plane_has_format_mod/igt_display_has_format_mod helpers take 
care of this check correctly (later in the test) since i915 does support 
IN_FORMATS.

This is the same case for the other patches in the series where I 
removed these checks and replaced them with the helpers.

Nicholas Kazlauskas

> 
>>   	if (!opt.user_seed)
>>   		opt.seed = time(NULL);
>>   
>> @@ -344,6 +351,10 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
>>   	igt_subtest_f("atomic-pipe-%s-tiling-yf", kmstest_pipe_name(pipe))
>>   		for_each_valid_output_on_pipe(&data->display, pipe, output)
>>   			test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_Yf_TILED);
>> +
>> +	igt_subtest_f("atomic-pipe-%s-tiling-none", kmstest_pipe_name(pipe))
>> +		for_each_valid_output_on_pipe(&data->display, pipe, output)
>> +			test_plane_position(data, pipe, LOCAL_DRM_FORMAT_MOD_NONE);
>>   }
>>   
>>   static data_t data;
>>

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev