Re: [igt-dev] [PATCH i-g-t 3/3] kms/test: Pass correct pipe value to print pipe name

Khajapasha, Mohammed
2020-03-10T05:52:41Z

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Monday, March 9, 2020 2:19 PM
> To: Khajapasha, Mohammed <mohammed.khajapasha@intel.com>; igt-
> dev@lists.freedesktop.org
> Subject: Re: [igt-dev] [PATCH i-g-t 3/3] kms/test: Pass correct pipe value to
> print pipe name
>
> On Sun, 08 Mar 2020, Mohammed Khajapasha
> <mohammed.khajapasha@intel.com> wrote:
> > currently kmstest_pipe_name() fn gets the pipe index value to print
> > the pipe name, but with non-contiguous display, pipe index is not
> > always same as pipe name.
> >
> > for example: if PIPE_A is disabled in kernel, PIPE_B,C & D get
> > registered with 0,1 & 2 indexes in mode config's crtc list and
> > kmstest_pipe_name() always prints PIPE_A for 0 index which is actually
> > PIPE_B in kernel.
>
> IMO if you have enum pipe pipe in IGT, it should match the kernel pipe.
>
> If you have crtc indexes in igt, then you should call them crtc indexes,
> *not* pipes.
>
> Otherwise you'll end up completely lost when a pipe is a pipe and when it's
> not.
>
> > igt_info("Running test on pipe %s with resolution %dx%d and sprite
> size %dx%d alpha %i\n",
> > - kmstest_pipe_name(pipe), mode->hdisplay, mode-
> >vdisplay,
> > + kmstest_pipe_name(display->pipes[pipe].pipe), mode-
> >hdisplay,
> > +mode->vdisplay,
>
> For example in changes like this my only conclusion is that the pipe is not
> really a pipe, but something else. So you should probably not call it a pipe
> to begin with.

Are you saying, we should get rid of pipe enum and make it array index
based? Is my understanding correct?
At present, every IGT subtest iterates over pipe list using "enum pipe" and
this is getting passed down.
e.g.
In kms_atomic_transition test, each test case iterate over enum pipe to run the subtest.
snippet:
igt_main
{
igt_display_t display;
igt_output_t *output;
enum pipe pipe;
/*
*/
igt_subtest("plane-primary-toggle-with-vblank-wait")
for_each_pipe_with_valid_output(&display, pipe, output)
run_primary_test(&display, pipe, output);
/*
*/
}

In such case we might have to change all IGT test cases to handle pipe index based instead of enum pipe.

>
> BR,
> Jani.
>
> --
> Jani Nikula, Intel Open Source Graphics Center