Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_rotation_crc: Fix bad-pixel-format not to be run on gen11 and later

From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_rotation_crc: Fix bad-pixel-format not to be run on gen11 and later
Date: Wed, 29 Aug 2018 17:22:35 -0700	[thread overview]
Message-ID: <20180830002235.GX6418@intel.com> (raw)
In-Reply-To: <a31792b1-b7e9-5c36-e449-1d5e9743e714@gmail.com>

On Tue, Aug 28, 2018 at 04:38:08PM +0300, Juha-Pekka Heikkila wrote:
> On 27.08.2018 19:33, Rodrigo Vivi wrote:
> > On Mon, Aug 27, 2018 at 04:14:05PM +0300, Mika Kahola wrote:
> > > On Mon, 2018-08-27 at 15:27 +0300, Juha-Pekka Heikkila wrote:
> > > > gen11 enables 90/270 rotation for RGB565 mode. IGT would need support
> > > > for
> > > > C8 mode to enable running this subtest on gen11 and later.
> > > > 
> > > > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > > > ---
> > > >   tests/kms_rotation_crc.c | 6 ++++++
> > > >   1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > > > index b994cc5..35e6721 100644
> > > > --- a/tests/kms_rotation_crc.c
> > > > +++ b/tests/kms_rotation_crc.c
> > > > @@ -558,6 +558,12 @@ igt_main
> > > >   	igt_subtest_f("bad-pixel-format") {
> > > >   		igt_require(gen >= 9);
> > > > +		/*
> > > > +		 * gen11 enables RGB565 rotation for 90/270 degrees.
> > > > +		 * C8 mode need to be enabled for IGT if want to run
> > > > this test
> > > > +		 * on gen11 onwards.
> > > > +		 */
> > > > +		igt_require(gen < 11);
> > > Could we combine these two requirements? Comment might need to be
> > > rephrased a bit too.
> 
> I'll make another patch.
> 
> > 
> > also, isn't any other way to check the capability instead of spreading
> > gen checks?
> 
> This is at beginning of test, nothing has been generated yet. I don't know
> where at this stage I would check for capabilities. In any case I'd prefer
> skip early.
> 
> Out of curiosity, why to avoid gen checks? I think they are with IGT case
> hand-in-hand on what's written on kernel side?

because when we write same code on both sides we are suitable for making
same mistake twice.

But also to avoid duplication of effort like we have with pci ids across
different components of stack... a such simple thing that is a painful and
boring task :(

So I prefer when we check for capabilities and if it is there we test it.

> 
> > 
> > > 
> > > >   		data.rotation = IGT_ROTATION_90;
> > > >   		data.override_fmt = DRM_FORMAT_RGB565;
> > > >   		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> > > > true);
> > > -- 
> > > Mika Kahola - Intel OTC
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev