[igt-dev] [PATCH i-g-t] igt: Another combinatorial exercise for blits

[Chris Wilson]

Quoting Vanshidhar Konda (2019-11-05 23:50:57)
> On Tue, Nov 05, 2019 at 02:25:28PM +0100, Katarzyna Dec wrote:
> >On Tue, Nov 05, 2019 at 11:15:13AM +0000, Chris Wilson wrote:
> >> +    igt_subtest("basic") {
> >> +            struct buffer *src, *dst;
> >> +            unsigned int x, y;
> >> +
> >> +            for (unsigned int height = 1; height <= 16; height <<= 1) {
> >> +                    for (unsigned int y0 = ZERO; y0 <= (height > 2 ? BELOW : ZERO); y0++) {
> >> +                            for (unsigned int width = 1; width <= 64; width <<= 1) {
> >> +                                    for (unsigned int x0 = ZERO; x0 <= (width > 2 ? BELOW : ZERO); x0++) {
> >> +
> >> +                                            src = buffer_create(&device,
> >> +                                                                width * 16, height * 4);
> >> +                                            dst = buffer_create(&device,
> >> +                                                                width * 16, height * 4);
> >> +
> >> +                                            y = start_at(height, y0);
> >> +                                            for (unsigned int src_tiling = I915_TILING_NONE;
> >> +                                                 src_tiling <= (device.gen >= 6 ? I915_TILING_Y : I915_TILING_X);
> >> +                                                 src_tiling++) {
> >> +                                                    buffer_set_tiling(&device, src, src_tiling);
> >> +
> >> +                                                    x = start_at(width, x0);
> >> +                                                    for (unsigned int dst_tiling = I915_TILING_NONE;
> >> +                                                         dst_tiling <= (device.gen >= 6 ? I915_TILING_Y : I915_TILING_X);
> >> +                                                         dst_tiling++) {
> >> +                                                            buffer_set_tiling(&device, dst, dst_tiling);
> >> +
> >> +                                                            for (enum mode down = CPU; down <= WC; down++) {
> 
> It seems like mode is only used in the buffer_check function, unless I
> missed something. May be instead of looping over mode and doing the same
> blit operation multiple times we can just do mode = (width % 3)?

You are using the blit to set up the dirty state, then inspecting it via
different modes. One of those modes should probably be a mixture of the
others... Since how we flush the dirty state is a big part of the test,
I don't see it being easily reduced... It's about mixing it up and not
missing anything.

The art is all in picking the right parameter space to trick the kernel
& HW into making a mistake and being able to detect it. The real art is
in being thorough, but finding the bugs early! Think of this as only the
start!
-Chris