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

[Kazlauskas, Nicholas]

On 11/13/18 12:15 PM, Ville Syrjälä wrote:
> On Tue, Nov 13, 2018 at 05:02:04PM +0000, Kazlauskas, Nicholas wrote:
>> On 11/13/18 11:58 AM, Ville Syrjälä wrote:
>>> On Tue, Nov 13, 2018 at 10:23:48AM -0500, 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.
>>>>
>>>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>>> ---
>>>>    tests/kms_plane_multiple.c | 12 ++++++++++--
>>>>    1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
>>>> index 721afe59..7c8adf94 100644
>>>> --- a/tests/kms_plane_multiple.c
>>>> +++ b/tests/kms_plane_multiple.c
>>>> @@ -291,12 +291,16 @@ 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 (is_i915_device(data->drm_fd))
>>>> +		igt_skip_on(tiling == LOCAL_DRM_FORMAT_MOD_NONE);
>>>> +	else
>>>> +		igt_skip_on(tiling != LOCAL_DRM_FORMAT_MOD_NONE);
>>>
>>> Might be nice to switch this over to using igt_plane_has_format_mod().
>>> We'd need to push it down a bit further since we'd need to query each
>>> plane in turn.
>>
>> That definitely seems like a more elegant solution, I'll fix that up for
>> this and the other patch too.
> 
> I forget, does amggpu support modifiers? If not you will need this:
> https://patchwork.freedesktop.org/patch/239808/

There's no IN_FORMATS support for AMDGPU yet. I had tried adding it with 
a quick patch but it causes a lot of breakages in userspace so it's not 
as simple as I had hoped.

I have v2s that make use of the new helpers but without IN_FOMRATS 
support they skip. The patch series you linked would be really nice to 
have in this case since it still allows the helpers to be used without 
it. I don't mind leaving a R-B on the patch you linked.

But if there isn't any plan to have it merged I can always just do 
something like:

igt_skip_on(tiling != LOCAL_DRM_FORMAT_MOD_NONE && 
!igt_plane_has_format_mod(...))

Let me know what you think.

Nicholas Kazlauskas

> 
>>
>> Nicholas Kazlauskas
>>
>>>
>>> Alternatively we could do something a bit simpler:
>>> igt_require(igt_display_has_format_mod(display, DRM_FORMAT_XRGB888,
>>> 				       tiling));
>>> igt_require(igt_display_has_format_mod(display, DRM_FORMAT_ARGB888,
>>> 				       LOCAL_DRM_FORMAT_MOD_LINEAR));
>>>
>>>> +
>>>>    	if ((tiling == LOCAL_I915_FORMAT_MOD_Y_TILED ||
>>>>    	     tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED))
>>>> -		igt_require(AT_LEAST_GEN(devid, 9));
>>>> +		igt_require(AT_LEAST_GEN(intel_get_drm_devid(data->drm_fd), 9));
>>>>    
>>>>    	if (!opt.user_seed)
>>>>    		opt.seed = time(NULL);
>>>> @@ -344,6 +348,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;
>>>> -- 
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> igt-dev mailing list
>>>> igt-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>>>
>>
>