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

[Khajapasha, Mohammed]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Tuesday, March 17, 2020 6:07 PM
> To: Khajapasha, Mohammed <mohammed.khajapasha at intel.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>; igt-dev at lists.freedesktop.org
> Subject: Re: [igt-dev] [PATCH i-g-t 3/3] kms/test: Pass correct pipe value to
> print pipe name
> 
> On Tue, Mar 17, 2020 at 05:11:27AM +0000, Khajapasha, Mohammed wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Sent: Tuesday, March 10, 2020 5:45 PM
> > > To: Jani Nikula <jani.nikula at linux.intel.com>
> > > Cc: Khajapasha, Mohammed <mohammed.khajapasha at intel.com>; igt-
> > > dev at lists.freedesktop.org
> > > Subject: Re: [igt-dev] [PATCH i-g-t 3/3] kms/test: Pass correct pipe
> > > value to print pipe name
> > >
> > > On Tue, Mar 10, 2020 at 10:20:33AM +0200, Jani Nikula wrote:
> > > > On Tue, 10 Mar 2020, "Khajapasha, Mohammed"
> > > <mohammed.khajapasha at intel.com> wrote:
> > > > > 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.
> > > >
> > > > I am saying do not conflate enum pipe and crtc index.
> > > >
> > > > If you call something a pipe, it should be the pipe, not crtc index.
> > > > And vice versa.
> > > >
> > > > It gets really confusing really quick if you call something a
> > > > pipe, and then go on to convert it into a pipe, as if it wasn't a pipe
> already.
> > > >
> > > > In i915 we've removed all assumptions that pipe == crtc index. So
> > > > should igt.
> > >
> > > I tend to agree, especially as other hw doens't use the "pipe"
> > > terminology, nor do they necessarily refer to them with A/B/C/...
> > >
> > > The ugly part would be that we'd have maybe rename all tests with a
> > > pipe in the name. Should we just use the crtc index as a bare number
> > > in those cases?
> >
> > As per my understandings, yes, we need to use bare number of crtc
> > index to get the pipe in display->pipes[] array for for_each_pipe() macros.
> >
> > >
> > > This would also have some complications for being able to quickly
> > > see which pipe had a specific failure as you have to dig through the
> > > logs to get that (though we'd anyway need to do that if the pipes
> > > are fused off in a way that makes the crtc index not match the pipe).
> > >
> > > Also in some cases we may actually want to know which pipe we're
> > > dealing with, but currently there is no convenient way for userspace
> > > to find out which pipe maps to which crtc.
> > >
> > In this patch series, we are able to map the kernel pipe to IGT pipe
> > using GET_PIPE_FROM_CRTC_ID ioctl, with this we are able to print the
> > logs for failure pipes in  IGT.
> 
> Right. I forgot that we had that ioctl.

I have sent a sample RFC patch https://patchwork.freedesktop.org/patch/358020/ for review
by using bare crtc index for display->pipes[] array to get a pipe in kms atomic transition test case.
If the change is acceptable, will update all remaining test cases in this series.  

> --
> Ville Syrjälä
> Intel