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

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

[Nicholas Kazlauskas]

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@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);
+
 	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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Ville Syrjälä]

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@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.

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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Kazlauskas, Nicholas]

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@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.

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@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

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

[Ville Syrjälä]

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@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/

> 
> 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@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> 

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [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@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@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

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

[Ville Syrjälä]

On Tue, Nov 27, 2018 at 06:36:53PM +0000, Kazlauskas, Nicholas wrote:
> 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@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.

I'd like to have that patch merged. So r-b away please.

IIRC the point of contention was whether my proposed behaviour is
actually acceptable for driver w/o modifiers. Since you seem to have
one I think an r-b from you would be a good indicator that my
plan was in fact sane ;)


-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 1/4] tests/kms_plane_scaling: Add support for testing AMDGPU

[Nicholas Kazlauskas]

The plane_scaling subtests are capable of running on AMDGPU when
not using i915 specific tiling formats and when the test only requires
one scaler per pipe.

This patch removes the forced i915 devid and gen checks from non i915
devices. It also adds logic for getting the number of scalers per pipe
in a way that doesn't only depend on devid. One scaler per pipe is
assumed for AMDGPU.

There isn't any specific reason that the x-tiled formats need to be
used on the non-rotation tests on i915 but this patch keeps the
existing test behavior. It's a little simpler to keep it this way for
the prepare_crtc helper that's shared between the scaling test
and the clipping/clamping test.

v2: Use igt_plane_has_format_mod helper (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 tests/kms_plane_scaling.c | 49 +++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index 773162ec..a3bc54fd 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -46,11 +46,14 @@ typedef struct {
 	igt_plane_t *plane4;
 } data_t;
 
-static int get_num_scalers(uint32_t devid, enum pipe pipe)
+static int get_num_scalers(data_t* d, enum pipe pipe)
 {
-	igt_require(intel_gen(devid) >= 9);
+	if (!is_i915_device(d->drm_fd))
+		return 1;
+
+	igt_require(intel_gen(d->devid) >= 9);
 
-	if (intel_gen(devid) >= 10)
+	if (intel_gen(d->devid) >= 10)
 		return 2;
 	else if (pipe != PIPE_C)
 		return 2;
@@ -79,6 +82,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 			igt_plane_t *plane, drmModeModeInfo *mode)
 {
 	igt_display_t *display = &data->display;
+	uint64_t tiling = is_i915_device(data->drm_fd) ?
+		LOCAL_I915_FORMAT_MOD_X_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
 
 	cleanup_crtc(data);
 
@@ -90,7 +95,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	/* allocate fb for plane 1 */
 	igt_create_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
 			      DRM_FORMAT_XRGB8888,
-			      LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
+			      tiling,
 			      &data->fb[0]);
 
 	igt_plane_set_fb(plane, &data->fb[0]);
@@ -129,6 +134,9 @@ static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
 	igt_output_set_pipe(output, pipe);
 	mode = igt_output_get_mode(output);
 
+	igt_skip_on(!igt_plane_has_format_mod(plane, pixel_format,
+					      tiling));
+
 	/* create buffer in the range of  min and max source side limit.*/
 	width = height = 8;
 	if (pixel_format == DRM_FORMAT_NV12)
@@ -170,6 +178,8 @@ static void test_scaler_with_rotation_pipe(data_t *d, enum pipe pipe,
 {
 	igt_display_t *display = &d->display;
 	igt_plane_t *plane;
+	uint64_t tiling = is_i915_device(d->drm_fd) ?
+		LOCAL_I915_FORMAT_MOD_Y_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
 
 	igt_output_set_pipe(output, pipe);
 	for_each_plane_on_pipe(display, pipe, plane) {
@@ -183,8 +193,8 @@ static void test_scaler_with_rotation_pipe(data_t *d, enum pipe pipe,
 				if (igt_fb_supported_format(format) &&
 				    can_rotate(d, format))
 					check_scaling_pipe_plane_rot(d, plane, format,
-								     LOCAL_I915_FORMAT_MOD_Y_TILED,
-								     pipe, output, rot);
+								     tiling, pipe,
+								     output, rot);
 			}
 		}
 	}
@@ -264,6 +274,8 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
 	igt_display_t *display = &d->display;
 	drmModeModeInfo *mode;
 	int primary_plane_scaling = 0; /* For now */
+	uint64_t tiling = is_i915_device(display->drm_fd) ?
+		LOCAL_I915_FORMAT_MOD_X_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
 
 	mode = igt_output_get_mode(output);
 
@@ -273,13 +285,13 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
 
 	igt_create_color_pattern_fb(display->drm_fd, 600, 600,
 				    DRM_FORMAT_XRGB8888,
-				    LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
+				    tiling,
 				    .5, .5, .5, &d->fb[1]);
 
 	igt_create_pattern_fb(d->drm_fd,
 			      mode->hdisplay, mode->vdisplay,
 			      DRM_FORMAT_XRGB8888,
-			      LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
+			      tiling,
 			      &d->fb[2]);
 
 	if (primary_plane_scaling) {
@@ -425,7 +437,7 @@ test_scaler_with_clipping_clamping_scenario(data_t *d, enum pipe pipe, igt_outpu
 {
 	drmModeModeInfo *mode;
 
-	igt_require(get_num_scalers(d->devid, pipe) >= 2);
+	igt_require(get_num_scalers(d, pipe) >= 2);
 
 	mode = igt_output_get_mode(output);
 	d->plane1 = &d->display.pipes[pipe].planes[0];
@@ -480,6 +492,8 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
 	igt_output_t *output1, *output2;
 	drmModeModeInfo *mode1, *mode2;
 	enum pipe pipe1, pipe2;
+	uint64_t tiling = is_i915_device(display->drm_fd) ?
+		LOCAL_I915_FORMAT_MOD_Y_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
 
 	cleanup_crtc(d);
 
@@ -492,28 +506,28 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
 	igt_output_set_pipe(output2, pipe2);
 
 	d->plane1 = igt_output_get_plane(output1, 0);
-	d->plane2 = get_num_scalers(d->devid, pipe1) >= 2 ? igt_output_get_plane(output1, 1) : NULL;
+	d->plane2 = get_num_scalers(d, pipe1) >= 2 ? igt_output_get_plane(output1, 1) : NULL;
 	d->plane3 = igt_output_get_plane(output2, 0);
-	d->plane4 = get_num_scalers(d->devid, pipe2) >= 2 ? igt_output_get_plane(output2, 1) : NULL;
+	d->plane4 = get_num_scalers(d, pipe2) >= 2 ? igt_output_get_plane(output2, 1) : NULL;
 
 	mode1 = igt_output_get_mode(output1);
 	mode2 = igt_output_get_mode(output2);
 
 	igt_create_pattern_fb(d->drm_fd, 600, 600,
 			      DRM_FORMAT_XRGB8888,
-			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[0]);
+			      tiling, &d->fb[0]);
 
 	igt_create_pattern_fb(d->drm_fd, 500, 500,
 			      DRM_FORMAT_XRGB8888,
-			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[1]);
+			      tiling, &d->fb[1]);
 
 	igt_create_pattern_fb(d->drm_fd, 700, 700,
 			      DRM_FORMAT_XRGB8888,
-			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[2]);
+			      tiling, &d->fb[2]);
 
 	igt_create_pattern_fb(d->drm_fd, 400, 400,
 			      DRM_FORMAT_XRGB8888,
-			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[3]);
+			      tiling, &d->fb[3]);
 
 	igt_plane_set_fb(d->plane1, &d->fb[0]);
 	if (d->plane2)
@@ -545,7 +559,8 @@ igt_main
 		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
 		igt_require_pipe_crc(data.drm_fd);
 		igt_display_require(&data.display, data.drm_fd);
-		data.devid = intel_get_drm_devid(data.drm_fd);
+		data.devid = is_i915_device(data.drm_fd) ?
+			intel_get_drm_devid(data.drm_fd) : 0;
 		igt_require(data.display.is_atomic);
 	}
 
@@ -555,7 +570,7 @@ igt_main
 		igt_fixture {
 			igt_display_require_output_on_pipe(&data.display, pipe);
 
-			igt_require(get_num_scalers(data.devid, pipe) > 0);
+			igt_require(get_num_scalers(&data, pipe) > 0);
 		}
 
 		igt_subtest_f("pipe-%s-plane-scaling", kmstest_pipe_name(pipe))
-- 
2.17.1

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

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

[Nicholas Kazlauskas]

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));
-
 	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;
-- 
2.17.1

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

[igt-dev] [PATCH i-g-t 3/4] tests/kms_plane_lowres: Don't fail tests when missing format/mod support

[Nicholas Kazlauskas]

The kms_plane_lowres subtests will fail on non-i915 hardware because
of the devid lookups and tiling format requirements.

This patch makes use of the igt_display_has_format_mod() helper to
check for support before failing fb creation.

The tests still won't fully run yet on i915 hardware because they'll
skip during calls to igt_assert_plane_visible - those require an i915
extension to get the CRTC/plane set for a given pipe.

v2: Use igt_display_has_format_mod helper (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 tests/kms_plane_lowres.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
index 0824ef8f..63a4e5b8 100644
--- a/tests/kms_plane_lowres.c
+++ b/tests/kms_plane_lowres.c
@@ -141,6 +141,12 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
 	int size;
 	int i = 1, x, y;
 	igt_plane_t *plane;
+	uint64_t plane_modifier;
+	uint32_t plane_format;
+
+	igt_skip_on(!igt_display_has_format_mod(&data->display,
+						DRM_FORMAT_XRGB8888,
+						modifier));
 
 	igt_output_set_pipe(output, pipe);
 
@@ -171,10 +177,19 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
 		x = 0;
 		y = mode->vdisplay - size;
 
+		plane_format = plane->type == DRM_PLANE_TYPE_CURSOR ?
+			DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888;
+
+		plane_modifier = plane->type == DRM_PLANE_TYPE_CURSOR ?
+			LOCAL_DRM_FORMAT_MOD_NONE : modifier;
+
+		igt_skip_on(!igt_plane_has_format_mod(plane, plane_format,
+						      plane_modifier));
+
 		igt_create_color_fb(data->drm_fd,
 				    size, size,
-				    plane->type == DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888,
-				    plane->type == DRM_PLANE_TYPE_CURSOR ? LOCAL_DRM_FORMAT_MOD_NONE : modifier,
+				    plane_format,
+				    plane_modifier,
 				    1.0, 1.0, 0.0,
 				    &data->fb[i]);
 
@@ -251,11 +266,6 @@ static void
 test_plane_position(data_t *data, enum pipe pipe, uint64_t modifier)
 {
 	igt_output_t *output;
-	const int gen = intel_gen(intel_get_drm_devid(data->drm_fd));
-
-	if (modifier == LOCAL_I915_FORMAT_MOD_Y_TILED ||
-	    modifier == LOCAL_I915_FORMAT_MOD_Yf_TILED)
-		igt_skip_on(gen < 9);
 
 	for_each_valid_output_on_pipe(&data->display, pipe, output)
 		test_plane_position_with_output(data, pipe, output, modifier);
-- 
2.17.1

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

[igt-dev] [PATCH i-g-t 4/4] tests: Enable plane tests for AMDGPU

[Nicholas Kazlauskas]

The i915 specific feature requirements that would have failed subtests
from kms_plane, kms_plane_multiple and kms_plane_scaling have been
conditionally guarded against. These tests can now be run on AMDGPU
with the i915 specific tests skipped appropriately.

Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 tests/kms_plane.c          | 2 +-
 tests/kms_plane_multiple.c | 2 +-
 tests/kms_plane_scaling.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 45e0a304..655b0839 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -590,7 +590,7 @@ igt_main
 	igt_skip_on_simulation();
 
 	igt_fixture {
-		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_AMDGPU);
 
 		kmstest_set_vt_graphics_mode();
 
diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index e67e21a5..41024cb2 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -401,7 +401,7 @@ int main(int argc, char *argv[])
 	igt_skip_on_simulation();
 
 	igt_fixture {
-		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_AMDGPU);
 		kmstest_set_vt_graphics_mode();
 		igt_require_pipe_crc(data.drm_fd);
 		igt_display_require(&data.display, data.drm_fd);
diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index a3bc54fd..de7165ea 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -556,7 +556,7 @@ igt_main
 	igt_skip_on_simulation();
 
 	igt_fixture {
-		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_AMDGPU);
 		igt_require_pipe_crc(data.drm_fd);
 		igt_display_require(&data.display, data.drm_fd);
 		data.devid = is_i915_device(data.drm_fd) ?
-- 
2.17.1

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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] tests/kms_plane_scaling: Add support for testing AMDGPU

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] tests/kms_plane_scaling: Add support for testing AMDGPU
URL   : https://patchwork.freedesktop.org/series/53171/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5213 -> IGTPW_2104
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53171/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2104 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6700hq:      PASS -> DMESG-WARN [fdo#105998]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  
#### Possible fixes ####

  * igt@gem_basic@create-fd-close:
    - fi-kbl-7560u:       INCOMPLETE -> PASS

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-n3050:       FAIL [fdo#108656] -> PASS

  * igt@kms_chamelium@hdmi-hpd-fast:
    - {fi-kbl-7500u}:     FAIL [fdo#108769] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656
  [fdo#108769]: https://bugs.freedesktop.org/show_bug.cgi?id=108769


Participating hosts (51 -> 44)
------------------------------

  Missing    (7): fi-kbl-soraka fi-kbl-7567u fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * IGT: IGT_4735 -> IGTPW_2104

  CI_DRM_5213: 5912c54d9804fb15d6a9fa2798bfef1e837c8938 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2104: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2104/
  IGT_4735: b05c028ccdb6ac8e8d8499a041bb14dfe358ee26 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_plane_multiple@atomic-pipe-a-tiling-none
+igt@kms_plane_multiple@atomic-pipe-b-tiling-none
+igt@kms_plane_multiple@atomic-pipe-c-tiling-none
+igt@kms_plane_multiple@atomic-pipe-d-tiling-none
+igt@kms_plane_multiple@atomic-pipe-e-tiling-none
+igt@kms_plane_multiple@atomic-pipe-f-tiling-none

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2104/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Wentland, Harry]

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

>  	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

Re: [igt-dev] [PATCH i-g-t 3/4] tests/kms_plane_lowres: Don't fail tests when missing format/mod support

[Wentland, Harry]

On 2018-11-28 9:56 a.m., Nicholas Kazlauskas wrote:
> The kms_plane_lowres subtests will fail on non-i915 hardware because
> of the devid lookups and tiling format requirements.
> 
> This patch makes use of the igt_display_has_format_mod() helper to
> check for support before failing fb creation.
> 
> The tests still won't fully run yet on i915 hardware because they'll
> skip during calls to igt_assert_plane_visible - those require an i915
> extension to get the CRTC/plane set for a given pipe.
> 
> v2: Use igt_display_has_format_mod helper (Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>  tests/kms_plane_lowres.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
> index 0824ef8f..63a4e5b8 100644
> --- a/tests/kms_plane_lowres.c
> +++ b/tests/kms_plane_lowres.c
> @@ -141,6 +141,12 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
>  	int size;
>  	int i = 1, x, y;
>  	igt_plane_t *plane;
> +	uint64_t plane_modifier;
> +	uint32_t plane_format;
> +
> +	igt_skip_on(!igt_display_has_format_mod(&data->display,
> +						DRM_FORMAT_XRGB8888,
> +						modifier));
>  
>  	igt_output_set_pipe(output, pipe);
>  
> @@ -171,10 +177,19 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
>  		x = 0;
>  		y = mode->vdisplay - size;
>  
> +		plane_format = plane->type == DRM_PLANE_TYPE_CURSOR ?
> +			DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888;
> +
> +		plane_modifier = plane->type == DRM_PLANE_TYPE_CURSOR ?
> +			LOCAL_DRM_FORMAT_MOD_NONE : modifier;
> +
> +		igt_skip_on(!igt_plane_has_format_mod(plane, plane_format,
> +						      plane_modifier));
> +
>  		igt_create_color_fb(data->drm_fd,
>  				    size, size,
> -				    plane->type == DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888,
> -				    plane->type == DRM_PLANE_TYPE_CURSOR ? LOCAL_DRM_FORMAT_MOD_NONE : modifier,
> +				    plane_format,
> +				    plane_modifier,
>  				    1.0, 1.0, 0.0,
>  				    &data->fb[i]);
>  
> @@ -251,11 +266,6 @@ static void
>  test_plane_position(data_t *data, enum pipe pipe, uint64_t modifier)
>  {
>  	igt_output_t *output;
> -	const int gen = intel_gen(intel_get_drm_devid(data->drm_fd));
> -
> -	if (modifier == LOCAL_I915_FORMAT_MOD_Y_TILED ||
> -	    modifier == LOCAL_I915_FORMAT_MOD_Yf_TILED)
> -		igt_skip_on(gen < 9);

Same comment as on patch 2: does this even have an impact on non-i915 drivers?

Harry

>  
>  	for_each_valid_output_on_pipe(&data->display, pipe, output)
>  		test_plane_position_with_output(data, pipe, output, modifier);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/4] tests: Enable plane tests for AMDGPU

[Wentland, Harry]

On 2018-11-28 9:56 a.m., Nicholas Kazlauskas wrote:
> The i915 specific feature requirements that would have failed subtests
> from kms_plane, kms_plane_multiple and kms_plane_scaling have been
> conditionally guarded against. These tests can now be run on AMDGPU
> with the i915 specific tests skipped appropriately.
> 
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Once review for the other patches in this series is settled this one is
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  tests/kms_plane.c          | 2 +-
>  tests/kms_plane_multiple.c | 2 +-
>  tests/kms_plane_scaling.c  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 45e0a304..655b0839 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -590,7 +590,7 @@ igt_main
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> -		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_AMDGPU);
>  
>  		kmstest_set_vt_graphics_mode();
>  
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index e67e21a5..41024cb2 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -401,7 +401,7 @@ int main(int argc, char *argv[])
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> -		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_AMDGPU);
>  		kmstest_set_vt_graphics_mode();
>  		igt_require_pipe_crc(data.drm_fd);
>  		igt_display_require(&data.display, data.drm_fd);
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index a3bc54fd..de7165ea 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -556,7 +556,7 @@ igt_main
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> -		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_AMDGPU);
>  		igt_require_pipe_crc(data.drm_fd);
>  		igt_display_require(&data.display, data.drm_fd);
>  		data.devid = is_i915_device(data.drm_fd) ?
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Kazlauskas, Nicholas]

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

Re: [igt-dev] [PATCH i-g-t 1/4] tests/kms_plane_scaling: Add support for testing AMDGPU

[Ville Syrjälä]

On Wed, Nov 28, 2018 at 09:56:17AM -0500, Nicholas Kazlauskas wrote:
> The plane_scaling subtests are capable of running on AMDGPU when
> not using i915 specific tiling formats and when the test only requires
> one scaler per pipe.
> 
> This patch removes the forced i915 devid and gen checks from non i915
> devices. It also adds logic for getting the number of scalers per pipe
> in a way that doesn't only depend on devid. One scaler per pipe is
> assumed for AMDGPU.
> 
> There isn't any specific reason that the x-tiled formats need to be
> used on the non-rotation tests on i915 but this patch keeps the
> existing test behavior. It's a little simpler to keep it this way for
> the prepare_crtc helper that's shared between the scaling test
> and the clipping/clamping test.

Fair enough. We may want to switch to linear with i915, but prorbably
better to do that as a separate patch.

> 
> v2: Use igt_plane_has_format_mod helper (Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>  tests/kms_plane_scaling.c | 49 +++++++++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 773162ec..a3bc54fd 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -46,11 +46,14 @@ typedef struct {
>  	igt_plane_t *plane4;
>  } data_t;
>  
> -static int get_num_scalers(uint32_t devid, enum pipe pipe)
> +static int get_num_scalers(data_t* d, enum pipe pipe)
>  {
> -	igt_require(intel_gen(devid) >= 9);
> +	if (!is_i915_device(d->drm_fd))
> +		return 1;
> +
> +	igt_require(intel_gen(d->devid) >= 9);
>  
> -	if (intel_gen(devid) >= 10)
> +	if (intel_gen(d->devid) >= 10)
>  		return 2;
>  	else if (pipe != PIPE_C)
>  		return 2;
> @@ -79,6 +82,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>  			igt_plane_t *plane, drmModeModeInfo *mode)
>  {
>  	igt_display_t *display = &data->display;
> +	uint64_t tiling = is_i915_device(data->drm_fd) ?
> +		LOCAL_I915_FORMAT_MOD_X_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
>  
>  	cleanup_crtc(data);
>  
> @@ -90,7 +95,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>  	/* allocate fb for plane 1 */
>  	igt_create_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
>  			      DRM_FORMAT_XRGB8888,
> -			      LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> +			      tiling,
>  			      &data->fb[0]);
>  
>  	igt_plane_set_fb(plane, &data->fb[0]);
> @@ -129,6 +134,9 @@ static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
>  	igt_output_set_pipe(output, pipe);
>  	mode = igt_output_get_mode(output);
>  
> +	igt_skip_on(!igt_plane_has_format_mod(plane, pixel_format,
> +					      tiling));
> +
>  	/* create buffer in the range of  min and max source side limit.*/
>  	width = height = 8;
>  	if (pixel_format == DRM_FORMAT_NV12)
> @@ -170,6 +178,8 @@ static void test_scaler_with_rotation_pipe(data_t *d, enum pipe pipe,
>  {
>  	igt_display_t *display = &d->display;
>  	igt_plane_t *plane;
> +	uint64_t tiling = is_i915_device(d->drm_fd) ?
> +		LOCAL_I915_FORMAT_MOD_Y_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
>  
>  	igt_output_set_pipe(output, pipe);
>  	for_each_plane_on_pipe(display, pipe, plane) {
> @@ -183,8 +193,8 @@ static void test_scaler_with_rotation_pipe(data_t *d, enum pipe pipe,
>  				if (igt_fb_supported_format(format) &&
>  				    can_rotate(d, format))
>  					check_scaling_pipe_plane_rot(d, plane, format,
> -								     LOCAL_I915_FORMAT_MOD_Y_TILED,
> -								     pipe, output, rot);
> +								     tiling, pipe,
> +								     output, rot);
>  			}
>  		}
>  	}
> @@ -264,6 +274,8 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>  	igt_display_t *display = &d->display;
>  	drmModeModeInfo *mode;
>  	int primary_plane_scaling = 0; /* For now */
> +	uint64_t tiling = is_i915_device(display->drm_fd) ?
> +		LOCAL_I915_FORMAT_MOD_X_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
>  
>  	mode = igt_output_get_mode(output);

Shouldn't we have a igt_skip_on(!has_format_mod()) here as well?

>  
> @@ -273,13 +285,13 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>  
>  	igt_create_color_pattern_fb(display->drm_fd, 600, 600,
>  				    DRM_FORMAT_XRGB8888,
> -				    LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> +				    tiling,
>  				    .5, .5, .5, &d->fb[1]);
>  
>  	igt_create_pattern_fb(d->drm_fd,
>  			      mode->hdisplay, mode->vdisplay,
>  			      DRM_FORMAT_XRGB8888,
> -			      LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> +			      tiling,
>  			      &d->fb[2]);
>  
>  	if (primary_plane_scaling) {
> @@ -425,7 +437,7 @@ test_scaler_with_clipping_clamping_scenario(data_t *d, enum pipe pipe, igt_outpu
>  {
>  	drmModeModeInfo *mode;
>  
> -	igt_require(get_num_scalers(d->devid, pipe) >= 2);
> +	igt_equire(get_num_scalers(d, pipe) >= 2);
>  
>  	mode = igt_output_get_mode(output);
>  	d->plane1 = &d->display.pipes[pipe].planes[0];
> @@ -480,6 +492,8 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
>  	igt_output_t *output1, *output2;
>  	drmModeModeInfo *mode1, *mode2;
>  	enum pipe pipe1, pipe2;
> +	uint64_t tiling = is_i915_device(display->drm_fd) ?
> +		LOCAL_I915_FORMAT_MOD_Y_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
>  

And here too?

>  	cleanup_crtc(d);
>  
> @@ -492,28 +506,28 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
>  	igt_output_set_pipe(output2, pipe2);
>  
>  	d->plane1 = igt_output_get_plane(output1, 0);
> -	d->plane2 = get_num_scalers(d->devid, pipe1) >= 2 ? igt_output_get_plane(output1, 1) : NULL;
> +	d->plane2 = get_num_scalers(d, pipe1) >= 2 ? igt_output_get_plane(output1, 1) : NULL;
>  	d->plane3 = igt_output_get_plane(output2, 0);
> -	d->plane4 = get_num_scalers(d->devid, pipe2) >= 2 ? igt_output_get_plane(output2, 1) : NULL;
> +	d->plane4 = get_num_scalers(d, pipe2) >= 2 ? igt_output_get_plane(output2, 1) : NULL;
>  
>  	mode1 = igt_output_get_mode(output1);
>  	mode2 = igt_output_get_mode(output2);
>  
>  	igt_create_pattern_fb(d->drm_fd, 600, 600,
>  			      DRM_FORMAT_XRGB8888,
> -			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[0]);
> +			      tiling, &d->fb[0]);
>  
>  	igt_create_pattern_fb(d->drm_fd, 500, 500,
>  			      DRM_FORMAT_XRGB8888,
> -			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[1]);
> +			      tiling, &d->fb[1]);
>  
>  	igt_create_pattern_fb(d->drm_fd, 700, 700,
>  			      DRM_FORMAT_XRGB8888,
> -			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[2]);
> +			      tiling, &d->fb[2]);
>  
>  	igt_create_pattern_fb(d->drm_fd, 400, 400,
>  			      DRM_FORMAT_XRGB8888,
> -			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[3]);
> +			      tiling, &d->fb[3]);
>  
>  	igt_plane_set_fb(d->plane1, &d->fb[0]);
>  	if (d->plane2)
> @@ -545,7 +559,8 @@ igt_main
>  		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
>  		igt_require_pipe_crc(data.drm_fd);
>  		igt_display_require(&data.display, data.drm_fd);
> -		data.devid = intel_get_drm_devid(data.drm_fd);
> +		data.devid = is_i915_device(data.drm_fd) ?
> +			intel_get_drm_devid(data.drm_fd) : 0;
>  		igt_require(data.display.is_atomic);
>  	}
>  
> @@ -555,7 +570,7 @@ igt_main
>  		igt_fixture {
>  			igt_display_require_output_on_pipe(&data.display, pipe);
>  
> -			igt_require(get_num_scalers(data.devid, pipe) > 0);
> +			igt_require(get_num_scalers(&data, pipe) > 0);
>  		}
>  
>  		igt_subtest_f("pipe-%s-plane-scaling", kmstest_pipe_name(pipe))
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Ville Syrjälä]

On Wed, Nov 28, 2018 at 09:56:18AM -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.
> 
> 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));
> -
>  	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);

I'm thinking we should s/none/linear/ all over. IIRC someone did that
in the kernel already, but igt hasn't gotten the same treatment.

Anyways patch lgtm so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
>  static data_t data;
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/4] tests/kms_plane_scaling: Add support for testing AMDGPU

[Kazlauskas, Nicholas]

On 11/30/18 8:51 AM, Ville Syrjälä wrote:
> On Wed, Nov 28, 2018 at 09:56:17AM -0500, Nicholas Kazlauskas wrote:
>> The plane_scaling subtests are capable of running on AMDGPU when
>> not using i915 specific tiling formats and when the test only requires
>> one scaler per pipe.
>>
>> This patch removes the forced i915 devid and gen checks from non i915
>> devices. It also adds logic for getting the number of scalers per pipe
>> in a way that doesn't only depend on devid. One scaler per pipe is
>> assumed for AMDGPU.
>>
>> There isn't any specific reason that the x-tiled formats need to be
>> used on the non-rotation tests on i915 but this patch keeps the
>> existing test behavior. It's a little simpler to keep it this way for
>> the prepare_crtc helper that's shared between the scaling test
>> and the clipping/clamping test.
> 
> Fair enough. We may want to switch to linear with i915, but prorbably
> better to do that as a separate patch.

Yeah, a separate patch was sort of what I felt was a better approach here.

> 
>>
>> v2: Use igt_plane_has_format_mod helper (Ville)
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>> ---
>>   tests/kms_plane_scaling.c | 49 +++++++++++++++++++++++++--------------
>>   1 file changed, 32 insertions(+), 17 deletions(-)
>>
>> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
>> index 773162ec..a3bc54fd 100644
>> --- a/tests/kms_plane_scaling.c
>> +++ b/tests/kms_plane_scaling.c
>> @@ -46,11 +46,14 @@ typedef struct {
>>   	igt_plane_t *plane4;
>>   } data_t;
>>   
>> -static int get_num_scalers(uint32_t devid, enum pipe pipe)
>> +static int get_num_scalers(data_t* d, enum pipe pipe)
>>   {
>> -	igt_require(intel_gen(devid) >= 9);
>> +	if (!is_i915_device(d->drm_fd))
>> +		return 1;
>> +
>> +	igt_require(intel_gen(d->devid) >= 9);
>>   
>> -	if (intel_gen(devid) >= 10)
>> +	if (intel_gen(d->devid) >= 10)
>>   		return 2;
>>   	else if (pipe != PIPE_C)
>>   		return 2;
>> @@ -79,6 +82,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>>   			igt_plane_t *plane, drmModeModeInfo *mode)
>>   {
>>   	igt_display_t *display = &data->display;
>> +	uint64_t tiling = is_i915_device(data->drm_fd) ?
>> +		LOCAL_I915_FORMAT_MOD_X_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
>>   
>>   	cleanup_crtc(data);
>>   
>> @@ -90,7 +95,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>>   	/* allocate fb for plane 1 */
>>   	igt_create_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
>>   			      DRM_FORMAT_XRGB8888,
>> -			      LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
>> +			      tiling,
>>   			      &data->fb[0]);
>>   
>>   	igt_plane_set_fb(plane, &data->fb[0]);
>> @@ -129,6 +134,9 @@ static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
>>   	igt_output_set_pipe(output, pipe);
>>   	mode = igt_output_get_mode(output);
>>   
>> +	igt_skip_on(!igt_plane_has_format_mod(plane, pixel_format,
>> +					      tiling));
>> +
>>   	/* create buffer in the range of  min and max source side limit.*/
>>   	width = height = 8;
>>   	if (pixel_format == DRM_FORMAT_NV12)
>> @@ -170,6 +178,8 @@ static void test_scaler_with_rotation_pipe(data_t *d, enum pipe pipe,
>>   {
>>   	igt_display_t *display = &d->display;
>>   	igt_plane_t *plane;
>> +	uint64_t tiling = is_i915_device(d->drm_fd) ?
>> +		LOCAL_I915_FORMAT_MOD_Y_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
>>   
>>   	igt_output_set_pipe(output, pipe);
>>   	for_each_plane_on_pipe(display, pipe, plane) {
>> @@ -183,8 +193,8 @@ static void test_scaler_with_rotation_pipe(data_t *d, enum pipe pipe,
>>   				if (igt_fb_supported_format(format) &&
>>   				    can_rotate(d, format))
>>   					check_scaling_pipe_plane_rot(d, plane, format,
>> -								     LOCAL_I915_FORMAT_MOD_Y_TILED,
>> -								     pipe, output, rot);
>> +								     tiling, pipe,
>> +								     output, rot);
>>   			}
>>   		}
>>   	}
>> @@ -264,6 +274,8 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>>   	igt_display_t *display = &d->display;
>>   	drmModeModeInfo *mode;
>>   	int primary_plane_scaling = 0; /* For now */
>> +	uint64_t tiling = is_i915_device(display->drm_fd) ?
>> +		LOCAL_I915_FORMAT_MOD_X_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
>>   
>>   	mode = igt_output_get_mode(output);
> 
> Shouldn't we have a igt_skip_on(!has_format_mod()) here as well?

Right now everything i915 supports X_TILED but I suppose that could 
potentially change in the future.

I don't mind adding the check.

> 
>>   
>> @@ -273,13 +285,13 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>>   
>>   	igt_create_color_pattern_fb(display->drm_fd, 600, 600,
>>   				    DRM_FORMAT_XRGB8888,
>> -				    LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
>> +				    tiling,
>>   				    .5, .5, .5, &d->fb[1]);
>>   
>>   	igt_create_pattern_fb(d->drm_fd,
>>   			      mode->hdisplay, mode->vdisplay,
>>   			      DRM_FORMAT_XRGB8888,
>> -			      LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
>> +			      tiling,
>>   			      &d->fb[2]);
>>   
>>   	if (primary_plane_scaling) {
>> @@ -425,7 +437,7 @@ test_scaler_with_clipping_clamping_scenario(data_t *d, enum pipe pipe, igt_outpu
>>   {
>>   	drmModeModeInfo *mode;
>>   
>> -	igt_require(get_num_scalers(d->devid, pipe) >= 2);
>> +	igt_equire(get_num_scalers(d, pipe) >= 2);
>>   
>>   	mode = igt_output_get_mode(output);
>>   	d->plane1 = &d->display.pipes[pipe].planes[0];
>> @@ -480,6 +492,8 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
>>   	igt_output_t *output1, *output2;
>>   	drmModeModeInfo *mode1, *mode2;
>>   	enum pipe pipe1, pipe2;
>> +	uint64_t tiling = is_i915_device(display->drm_fd) ?
>> +		LOCAL_I915_FORMAT_MOD_Y_TILED : LOCAL_DRM_FORMAT_MOD_NONE;
>>   
> 
> And here too?

...and to here as well.

> 
>>   	cleanup_crtc(d);
>>   
>> @@ -492,28 +506,28 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
>>   	igt_output_set_pipe(output2, pipe2);
>>   
>>   	d->plane1 = igt_output_get_plane(output1, 0);
>> -	d->plane2 = get_num_scalers(d->devid, pipe1) >= 2 ? igt_output_get_plane(output1, 1) : NULL;
>> +	d->plane2 = get_num_scalers(d, pipe1) >= 2 ? igt_output_get_plane(output1, 1) : NULL;
>>   	d->plane3 = igt_output_get_plane(output2, 0);
>> -	d->plane4 = get_num_scalers(d->devid, pipe2) >= 2 ? igt_output_get_plane(output2, 1) : NULL;
>> +	d->plane4 = get_num_scalers(d, pipe2) >= 2 ? igt_output_get_plane(output2, 1) : NULL;
>>   
>>   	mode1 = igt_output_get_mode(output1);
>>   	mode2 = igt_output_get_mode(output2);
>>   
>>   	igt_create_pattern_fb(d->drm_fd, 600, 600,
>>   			      DRM_FORMAT_XRGB8888,
>> -			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[0]);
>> +			      tiling, &d->fb[0]);
>>   
>>   	igt_create_pattern_fb(d->drm_fd, 500, 500,
>>   			      DRM_FORMAT_XRGB8888,
>> -			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[1]);
>> +			      tiling, &d->fb[1]);
>>   
>>   	igt_create_pattern_fb(d->drm_fd, 700, 700,
>>   			      DRM_FORMAT_XRGB8888,
>> -			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[2]);
>> +			      tiling, &d->fb[2]);
>>   
>>   	igt_create_pattern_fb(d->drm_fd, 400, 400,
>>   			      DRM_FORMAT_XRGB8888,
>> -			      LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb[3]);
>> +			      tiling, &d->fb[3]);
>>   
>>   	igt_plane_set_fb(d->plane1, &d->fb[0]);
>>   	if (d->plane2)
>> @@ -545,7 +559,8 @@ igt_main
>>   		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
>>   		igt_require_pipe_crc(data.drm_fd);
>>   		igt_display_require(&data.display, data.drm_fd);
>> -		data.devid = intel_get_drm_devid(data.drm_fd);
>> +		data.devid = is_i915_device(data.drm_fd) ?
>> +			intel_get_drm_devid(data.drm_fd) : 0;
>>   		igt_require(data.display.is_atomic);
>>   	}
>>   
>> @@ -555,7 +570,7 @@ igt_main
>>   		igt_fixture {
>>   			igt_display_require_output_on_pipe(&data.display, pipe);
>>   
>> -			igt_require(get_num_scalers(data.devid, pipe) > 0);
>> +			igt_require(get_num_scalers(&data, pipe) > 0);
>>   		}
>>   
>>   		igt_subtest_f("pipe-%s-plane-scaling", kmstest_pipe_name(pipe))
>> -- 
>> 2.17.1
> 

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

Re: [igt-dev] [PATCH i-g-t 3/4] tests/kms_plane_lowres: Don't fail tests when missing format/mod support

[Ville Syrjälä]

On Wed, Nov 28, 2018 at 09:56:19AM -0500, Nicholas Kazlauskas wrote:
> The kms_plane_lowres subtests will fail on non-i915 hardware because
> of the devid lookups and tiling format requirements.
> 
> This patch makes use of the igt_display_has_format_mod() helper to
> check for support before failing fb creation.
> 
> The tests still won't fully run yet on i915 hardware because they'll
> skip during calls to igt_assert_plane_visible - those require an i915
> extension to get the CRTC/plane set for a given pipe.
> 
> v2: Use igt_display_has_format_mod helper (Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>  tests/kms_plane_lowres.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
> index 0824ef8f..63a4e5b8 100644
> --- a/tests/kms_plane_lowres.c
> +++ b/tests/kms_plane_lowres.c
> @@ -141,6 +141,12 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
>  	int size;
>  	int i = 1, x, y;
>  	igt_plane_t *plane;
> +	uint64_t plane_modifier;
> +	uint32_t plane_format;

These could be moved into tighter scope.

Apart from that lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	igt_skip_on(!igt_display_has_format_mod(&data->display,
> +						DRM_FORMAT_XRGB8888,
> +						modifier));
>  
>  	igt_output_set_pipe(output, pipe);
>  
> @@ -171,10 +177,19 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
>  		x = 0;
>  		y = mode->vdisplay - size;
>  
> +		plane_format = plane->type == DRM_PLANE_TYPE_CURSOR ?
> +			DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888;
> +
> +		plane_modifier = plane->type == DRM_PLANE_TYPE_CURSOR ?
> +			LOCAL_DRM_FORMAT_MOD_NONE : modifier;
> +
> +		igt_skip_on(!igt_plane_has_format_mod(plane, plane_format,
> +						      plane_modifier));
> +
>  		igt_create_color_fb(data->drm_fd,
>  				    size, size,
> -				    plane->type == DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888,
> -				    plane->type == DRM_PLANE_TYPE_CURSOR ? LOCAL_DRM_FORMAT_MOD_NONE : modifier,
> +				    plane_format,
> +				    plane_modifier,
>  				    1.0, 1.0, 0.0,
>  				    &data->fb[i]);
>  
> @@ -251,11 +266,6 @@ static void
>  test_plane_position(data_t *data, enum pipe pipe, uint64_t modifier)
>  {
>  	igt_output_t *output;
> -	const int gen = intel_gen(intel_get_drm_devid(data->drm_fd));
> -
> -	if (modifier == LOCAL_I915_FORMAT_MOD_Y_TILED ||
> -	    modifier == LOCAL_I915_FORMAT_MOD_Yf_TILED)
> -		igt_skip_on(gen < 9);
>  
>  	for_each_valid_output_on_pipe(&data->display, pipe, output)
>  		test_plane_position_with_output(data, pipe, output, modifier);
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Kazlauskas, Nicholas]

On 11/30/18 8:55 AM, Ville Syrjälä wrote:
> On Wed, Nov 28, 2018 at 09:56:18AM -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.
>>
>> 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));
>> -
>>   	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);
> 
> I'm thinking we should s/none/linear/ all over. IIRC someone did that
> in the kernel already, but igt hasn't gotten the same treatment.
> 
> Anyways patch lgtm so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks for the review.

The linear/none naming seems kind of inconsistent so I just went with 
what the other plane tests were using for "convention".

Nicholas Kazlauskas

> 
>>   }
>>   
>>   static data_t data;
>> -- 
>> 2.17.1
> 

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

Re: [igt-dev] [PATCH i-g-t 3/4] tests/kms_plane_lowres: Don't fail tests when missing format/mod support

[Kazlauskas, Nicholas]

On 11/30/18 8:56 AM, Ville Syrjälä wrote:
> On Wed, Nov 28, 2018 at 09:56:19AM -0500, Nicholas Kazlauskas wrote:
>> The kms_plane_lowres subtests will fail on non-i915 hardware because
>> of the devid lookups and tiling format requirements.
>>
>> This patch makes use of the igt_display_has_format_mod() helper to
>> check for support before failing fb creation.
>>
>> The tests still won't fully run yet on i915 hardware because they'll
>> skip during calls to igt_assert_plane_visible - those require an i915
>> extension to get the CRTC/plane set for a given pipe.
>>
>> v2: Use igt_display_has_format_mod helper (Ville)
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>> ---
>>   tests/kms_plane_lowres.c | 24 +++++++++++++++++-------
>>   1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
>> index 0824ef8f..63a4e5b8 100644
>> --- a/tests/kms_plane_lowres.c
>> +++ b/tests/kms_plane_lowres.c
>> @@ -141,6 +141,12 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
>>   	int size;
>>   	int i = 1, x, y;
>>   	igt_plane_t *plane;
>> +	uint64_t plane_modifier;
>> +	uint32_t plane_format;
> 
> These could be moved into tighter scope.
> 
> Apart from that lgtm
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I'll fix that up then send the v3. Thanks.

Nicholas Kazlauskas

> 
>> +
>> +	igt_skip_on(!igt_display_has_format_mod(&data->display,
>> +						DRM_FORMAT_XRGB8888,
>> +						modifier));
>>   
>>   	igt_output_set_pipe(output, pipe);
>>   
>> @@ -171,10 +177,19 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
>>   		x = 0;
>>   		y = mode->vdisplay - size;
>>   
>> +		plane_format = plane->type == DRM_PLANE_TYPE_CURSOR ?
>> +			DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888;
>> +
>> +		plane_modifier = plane->type == DRM_PLANE_TYPE_CURSOR ?
>> +			LOCAL_DRM_FORMAT_MOD_NONE : modifier;
>> +
>> +		igt_skip_on(!igt_plane_has_format_mod(plane, plane_format,
>> +						      plane_modifier));
>> +
>>   		igt_create_color_fb(data->drm_fd,
>>   				    size, size,
>> -				    plane->type == DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888,
>> -				    plane->type == DRM_PLANE_TYPE_CURSOR ? LOCAL_DRM_FORMAT_MOD_NONE : modifier,
>> +				    plane_format,
>> +				    plane_modifier,
>>   				    1.0, 1.0, 0.0,
>>   				    &data->fb[i]);
>>   
>> @@ -251,11 +266,6 @@ static void
>>   test_plane_position(data_t *data, enum pipe pipe, uint64_t modifier)
>>   {
>>   	igt_output_t *output;
>> -	const int gen = intel_gen(intel_get_drm_devid(data->drm_fd));
>> -
>> -	if (modifier == LOCAL_I915_FORMAT_MOD_Y_TILED ||
>> -	    modifier == LOCAL_I915_FORMAT_MOD_Yf_TILED)
>> -		igt_skip_on(gen < 9);
>>   
>>   	for_each_valid_output_on_pipe(&data->display, pipe, output)
>>   		test_plane_position_with_output(data, pipe, output, modifier);
>> -- 
>> 2.17.1
> 

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