[igt-dev] [PATCH i-g-t 2/2] tests/kms_chamelium: Don't fail random palnes tests on invalid config

[Arkadiusz Hiler]

On Thu, Mar 12, 2020 at 01:15:11PM +0200, Peres, Martin wrote:
> s/palnes/planes
> 
> On 2020-03-12 11:51, Hiler, Arkadiusz wrote:
> > It is possible to generate a configuration that the driver rejects
> > because it cannot be handled (e.g. exceeding the number of available
> > scalers). Until now the test was failing in such cases with:
> > 
> >   igt_kms-CRITICAL: Test assertion failure function igt_display_commit_atomic, file ../lib/igt_kms.c:3490:
> >   igt_kms-CRITICAL: Failed assertion: ret == 0
> >   igt_kms-CRITICAL: Last errno: 22, Invalid argument
> >   igt_kms-CRITICAL: error: -22 != 0
> > 
> > With this change we will just note that the atomic commit is invalid and
> > pass the test without causing random noise.
> 
> I was about to object strongly, but thinking about it further, I think
> this whole subtest makes no sense. We have a ton of tests for planes,
> and if the targeted HW does not have support for CRCs, then it should be
> emulated using pipe writeback or using chamelium as a source of CRC.
> 
> Anyway, fixing this test would require a loop of atomic tries until a
> suitable configuration would be found, until then, I would rather skip
> than give the idea that the test could test anything.

You would get a valid configuration most of the runs. It looks like the
test was originally coined as "maybe we hit some real issue sometime" as
exhaustive testing of all valid configurations is impossible.

> In summary, either:
> 
>  - nuke the subtest

I was thinking about it, but prefered to have someone else say that
aloud.

>  - make it skip upon atomic check failure

You would get a random skip flip-floper. This would make many people
quite grumpy.

>  - fix it by looking for valid configurations

Better to nuke it than to throw even more complexity at it.

> I am just against the idea of passing a test without it doing anything.