[igt-dev] [PATCH] [i-g-t, v2] tests/kms_cursor_crc: skip pipe on invalid connector in cursor size test

[Bhadane, Dnyaneshwar]

Hello Petri,
> -----Original Message-----
> From: Latvala, Petri <petri.latvala at intel.com>
> Sent: Monday, January 16, 2023 8:12 PM
> To: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Kurmi, Suresh Kumar
> <suresh.kumar.kurmi at intel.com>; Heikkila, Juha-pekka <juha-
> pekka.heikkila at intel.com>
> Subject: Re: [igt-dev] [PATCH] [i-g-t, v2] tests/kms_cursor_crc: skip pipe on
> invalid connector in cursor size test
> 
> On Mon, Jan 16, 2023 at 08:02:19PM +0530, Dnyaneshwar Bhadane wrote:
> > Only the valid pipe connector combination reach to the igt commit.
> > Cursor max-size test will not affect existing flow as only skip for
> > invalid connector.
> > For cursor-dpms and cursor-suspend not require to check
> > require_cursor_size becuase the cursor height and width used from drm
> capablities.
> >
> > --v2
> > - Used for_each_pipe_with_single_output() to iterate for valid pipe and
> output.
> >
> > Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane at intel.com>
> > ---
> >  tests/kms_cursor_crc.c | 16 +++-------------
> >  1 file changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c index
> > d5a4b30b..a4afcff3 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -749,7 +749,7 @@ static void run_size_tests(data_t *data, int w, int h)
> >  				}
> >  			}
> >
> > -			for_each_pipe(&data->display, pipe) {
> > +			for_each_pipe_with_single_output(&data->display,
> pipe,
> > +data->output) {
> >  				data->pipe = pipe;
> >
> >  				if (require_cursor_size(data, w, h)) { @@ -
> 850,15 +850,10 @@
> > static void run_tests_on_pipe(data_t *data)
> >
> >  	igt_describe("Check random placement of a cursor with DPMS.");
> >  	igt_subtest_with_dynamic("cursor-dpms") {
> > -		for_each_pipe(&data->display, pipe) {
> > +		for_each_pipe_with_single_output(&data->display, pipe,
> > +data->output) {
> >  			data->pipe = pipe;
> >  			data->flags = TEST_DPMS;
> >
> > -			if (require_cursor_size(data, data->cursor_max_w,
> data->cursor_max_h)) {
> > -				igt_debug("Cursor size %dx%d not supported by
> driver\n",
> > -					  data->cursor_max_w, data-
> >cursor_max_h);
> > -				continue;
> > -			}
> >
> >  			igt_dynamic_f("pipe-%s-%s",
> >  				      kmstest_pipe_name(pipe),
> > @@ -871,15 +866,10 @@ static void run_tests_on_pipe(data_t *data)
> >
> >  	igt_describe("Check random placement of a cursor with suspend.");
> >  	igt_subtest_with_dynamic("cursor-suspend") {
> > -		for_each_pipe(&data->display, pipe) {
> > +		for_each_pipe_with_single_output(&data->display, pipe,
> > +data->output) {
> >  			data->pipe = pipe;
> >  			data->flags = TEST_SUSPEND;
> >
> > -			if (require_cursor_size(data, data->cursor_max_w,
> data->cursor_max_h)) {
> > -				igt_debug("Cursor size %dx%d not supported by
> driver\n",
> > -					  data->cursor_max_w, data-
> >cursor_max_h);
> > -				continue;
> > -			}
> 
> 
> These will effectively overwrite whatever data->output was set to in the fixture
> in run_tests_on_pipe. But, how did that fixture ever work?
> That fixture leaves 'pipe' variable uninitialized and then finds an output for that
> pipe?
> 
The pipe variable is default taking value 0 as it enum. To save the testcase time,
the kms_cursor_crc testcases has written for one output even multiple output 
are connected to it so it will select first possible output.

Rewriting the new micro that will not override the existing data->output.
Means no need to recalculate as it is already assigned in the fixture.

Dnyaneshwar Bhadane
Intel.
> Either way that fetching of an output in that fixture needs to be removed if it's
> done in the subtests instead.
> 
> --
> Petri Latvala