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

[Bhadane, Dnyaneshwar]

> -----Original Message-----
> From: Latvala, Petri <petri.latvala at intel.com>
> Sent: Monday, January 23, 2023 3:45 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>; Sharma, Swati2 <swati2.sharma at intel.com>;
> Modem, Bhanuprakash <bhanuprakash.modem 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 23, 2023 at 11:58:44AM +0200, Bhadane, Dnyaneshwar wrote:
> > 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.
> 
> It's accidentally 0, because CI builds with the default of
> buildtype=debugoptimized. It's an automatic variable, not a global.

Agreed, Since it declared as enum type so it default value is 0. I am assigning it 
with pipe "PIPE_A" constant which infers to 0 as enum value.

> 
> 
> --
> Petri Latvala