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

Ville Syrjälä
2020-03-17T12:36:54Z

On Tue, Mar 17, 2020 at 05:11:27AM +0000, Khajapasha, Mohammed wrote:
>
>
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Tuesday, March 10, 2020 5:45 PM
> > To: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: 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 Tue, Mar 10, 2020 at 10:20:33AM +0200, Jani Nikula wrote:
> > > On Tue, 10 Mar 2020, "Khajapasha, Mohammed"
> > <mohammed.khajapasha@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.

--
Ville Syrjälä
Intel