[PATCH i-g-t v2 3/4] tests/kms_rotation: Add extensive rotation test
Arthur Grillo
arthurgrillo at riseup.net
Sat Mar 16 12:28:51 UTC 2024
On 15/03/24 13:11, Louis Chauvet wrote:
> Le 14/03/24 - 14:42, Arthur Grillo a écrit :
>>
>>
>> On 13/03/24 14:09, Louis Chauvet wrote:
>>> +
>>> +/**
>>> + * create_fb() - Create a framebuffer
>>> + *
>>> + * This create a framebuffer and fill it with a pattern. This pattern is not invariant by rotation and reflection.
>>> + *
>>> + * The @width and @height parameters are automaticaly inverted according to @rotation if needed. If the requested plane
>>> + * is 1024*768 with IGT_ROTATION_90, the returned plane will have the size 768*1024.
>>> + *
>>> + * @data: Current test configuration
>>> + * @rotation: Rotation to apply to the pattern
>>> + * @format: Format of the requested plane
>>> + * @modifier: Modifier for the requested plane
>>> + * @width, @height: Size of the plane to use (without rotation)
>>> + * @width: Width of the requested plane
>>> + * @height: Height of the requested plane
>>> + * @fb: Place to store the created framebuffer
>>> + */
>>> +static void create_fb(struct data_t *data, igt_rotation_t rotation, uint32_t format, uint64_t modifier, int width,
>>> + int height, igt_fb_t *fb)
>>> +{
>>> + int step_x, step_y, color_index, current_n, current_m;
>>> + int offset_x, offset_y;
>>> + cairo_t *cr;
>>> +
>>> + switch (rotation & IGT_ROTATION_MASK) {
>>> + case IGT_ROTATION_90:
>>> + case IGT_ROTATION_270:
>>> + igt_swap(width, height);
>>> + break;
>>> + case IGT_ROTATION_0:
>>> + case IGT_ROTATION_180:
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + igt_create_fb(data->fd, width, height, format, modifier, fb);
>>> + cr = igt_get_cairo_ctx(data->fd, fb);
>>> +
>>> + step_x = (width) / ARRAY_SIZE(colors);
>>> + step_y = (height) / ARRAY_SIZE(colors);
>>> +
>>> + /*
>>> + * This pair of offset is used to ensure that a software rotated plane is the same as a hardware rotated plane.
>>> + */
>>> + switch ((int)rotation) {
>>> + case IGT_ROTATION_0:
>>> + case IGT_ROTATION_0 | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_90:
>>> + case IGT_ROTATION_90 | IGT_REFLECT_X:
>>> + case IGT_ROTATION_180 | IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_180 | IGT_REFLECT_X:
>>> + case IGT_ROTATION_270 | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_270 | IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + offset_x = 0;
>>> + break;
>>> + case IGT_ROTATION_0 | IGT_REFLECT_X:
>>> + case IGT_ROTATION_0 | IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_90 | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_90 | IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_180:
>>> + case IGT_ROTATION_180 | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_270:
>>> + case IGT_ROTATION_270 | IGT_REFLECT_X:
>>> + offset_x = width % step_x;
>>> + break;
>>> + }
>>> + switch ((int)rotation) {
>>> + case IGT_ROTATION_0:
>>> + case IGT_ROTATION_0 | IGT_REFLECT_X:
>>> + case IGT_ROTATION_90 | IGT_REFLECT_X:
>>> + case IGT_ROTATION_90 | IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_180 | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_180 | IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_270:
>>> + case IGT_ROTATION_270 | IGT_REFLECT_Y:
>>> + offset_y = 0;
>>> + break;
>>> + case IGT_ROTATION_0 | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_0 | IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_90:
>>> + case IGT_ROTATION_90 | IGT_REFLECT_Y:
>>> + case IGT_ROTATION_180:
>>> + case IGT_ROTATION_180 | IGT_REFLECT_X:
>>> + case IGT_ROTATION_270 | IGT_REFLECT_X:
>>> + case IGT_ROTATION_270 | IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + offset_y = height % step_y;
>>> + break;
>>> + }
>>> +
>>> + /* Iterate over all colors in x and y direction. The pattern is a colored chessboard */
>>> + for (int n = 0; n < ARRAY_SIZE(colors); n++) {
>>> + for (int m = 0; m < ARRAY_SIZE(colors); m++) {
>>> + color_index = (n + m) % ARRAY_SIZE(colors);
>>> + current_n = n;
>>> + current_m = m;
>>> +
>>> + /*
>>> + * If a reflexion and a rotation is requested, apply this to the pattern rendering
>>> + * If a reflexion is given, it must be applied before the rotation.
>>> + */
>>> + switch (rotation & IGT_REFLECT_MASK) {
>>> + case IGT_REFLECT_X:
>>> + current_n = ARRAY_SIZE(colors) - 1 - current_n;
>>> + break;
>>> + case IGT_REFLECT_Y:
>>> + current_m = ARRAY_SIZE(colors) - 1 - current_m;
>>> + break;
>>> + case IGT_REFLECT_X | IGT_REFLECT_Y:
>>> + current_n = ARRAY_SIZE(colors) - 1 - current_n;
>>> + current_m = ARRAY_SIZE(colors) - 1 - current_m;
>>> + break;
>>> + }
>>> + switch (rotation & IGT_ROTATION_MASK) {
>>> + case IGT_ROTATION_0:
>>> + break;
>>> + case IGT_ROTATION_90:
>>> + igt_swap(current_n, current_m);
>>> + current_m = ARRAY_SIZE(colors) - 1 - current_m;
>>> + break;
>>> + case IGT_ROTATION_180:
>>> + current_n = ARRAY_SIZE(colors) - 1 - current_n;
>>> + current_m = ARRAY_SIZE(colors) - 1 - current_m;
>>> + break;
>>> + case IGT_ROTATION_270:
>>> + igt_swap(current_n, current_m);
>>> + current_n = ARRAY_SIZE(colors) - 1 - current_n;
>>> + break;
>>> + }
>>> +
>>> + igt_paint_color(cr,
>>> + offset_x + current_n * step_x,
>>> + offset_y + current_m * step_y,
>>> + step_x,
>>> + step_y,
>>> + colors[color_index].d[0],
>>> + colors[color_index].d[1],
>>> + colors[color_index].d[2]);
>>> + }
>>> + }
>>
>>
>> Couldn't all this be done with a igt_create_fb_pattern() and an
>> cairo_rotate()?
>
> I agree they looked like a good fit for this purpose, but unfortunately I
> already tried these functions and they were either not working with YUV
> (see why below) or way too complex for my use case (TBH I did not manage
> to use cairo_rotate and cairo_scale was overly complex to handle).
>
> If you don't mind I would prefer to keep all this code with the same
> "idea" (i.e: use only cairo_* helpers or rotate "by hand", but not mixing
> them)
Yeah, I can go with that, If changing to cairo_* helpers would make more
the whole thing more complex than the other way around.
>
> The main issue with YUV formats is subsampling, and for example, NV12, the
> subsampling is horizontal. A "software rotated" and a "hardware
> rotated" will not use the same subsampling "direction", so the resulting
> CRC will not be the same. Using plain colors and choosing the correct size
> avoids those issues, and igt_fb_create_pattern use gradients, which are
> not working well.
>
>>> +
>>> +/**
>>> + * run_test() - Run the subtests for a specific plane
>>> + *
>>> + * @data: Test configuration
>>> + * @output, @pipe, @plane: Plane to use for the test
>>> + * @format, @modifier: Format description to test
>>> + */
>>> +static void run_test(struct data_t *data, igt_output_t *output, enum pipe pipe, igt_plane_t *plane, uint32_t format,
>>> + uint64_t modifier)
>>> +{
>>> + int width, height;
>>> + const struct format_desc_struct *format_description;
>>> + igt_crc_t ref_crc[ARRAY_SIZE(tested_rotation)];
>>> + igt_crc_t crc[ARRAY_SIZE(tested_rotation)];
>>> +
>>> + igt_display_reset(&data->display);
>>> + igt_output_set_pipe(output, pipe);
>>> + /* Check that the rotation is supported */
>>> + igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
>>> + format_description = lookup_drm_format(format);
>>> + width = ARRAY_SIZE(colors) * format_description->hsub * format_description->vsub * 6;
>>> + height = ARRAY_SIZE(colors) * format_description->hsub * format_description->vsub * 6;
>>
>> What's going on here?
>
> I forgot to put a comment here:
>
> To avoid issue with YUV subsampling, the size must be a multiple of the
> subsampling, and to avoid color issues, the size must also be a multiple
> of the number of colors.
> The multiplication by 6 as an arbitrary value to use an intermediate size (not too small
> because some drivers don't support it, not too big to reduce tests duration).
I understand, could you also add a #define for this arbitrary value?
Best Regards,
~Arthur Grillo
More information about the igt-dev
mailing list