Re: [igt-dev] [PATCH i-g-t 9/9] chamelium: Add a CRC-based display test for randomized planes

Maxime Ripard
2018-12-06T14:49:03Z

[-- Attachment #1.1: Type: text/plain, Size: 1426 bytes --]

Hi,

On Thu, Dec 06, 2018 at 03:11:32PM +0100, Paul Kocialkowski wrote:
> This introduces a new test for the Chamelium, that sets up planes
> with randomized properties such as the format, dimensions, position,
> in-framebuffer offsets and stride. The Chamelium capture is checked
> against the reference generated by cairo with a CRC.
>
> This test also includes testing for some VC4-specific features, such as
> T-tiled mode (in XR24 format), bandwidth limitation and underrun
> (that require kernel-side patches that are currently under review).

it's not really clear to me why both are part of the same test? You
don't really need to test the HVS bandwidth limitation with the
patterns, and you don't really need a pattern and a CRC to test the
HVS bandwith?

> Since this test does not share much with previous CRC-based display
> tests (especially regarding KMS configuration), most of the code is
> not shared with other tests.
>
> This test can be derived with reproducible properties for regression
> testing in the future. For now, it serves as a kind of fuzzing test.

That function is also pretty long. While sticking to function that are
under 50-80 lines is not always easy, this one is over 300 lines long
and would definitely benefit being split into many, smaller,
functions.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]